Re: [PATCH v4 08/10] libxl: call hotplug scripts for disk devices from libxl

[Roger Pau Monne <roger.pau@citrix.com>]

Ian Jackson wrote:
>> +    return;
>> +
>> +out:
>> +    libxl__ev_time_deregister(gc,&aodev->ev);
>> +    aodev->rc = rc;
>> +    aodev->callback(egc, aodev);
>> +    return;
>> +}
>
> Shouldn't something set aodev->state to FINISHED in this error path ?
>
> It might be better to have a helper function to call when declaring
> the aodev finished.  That helper function would also do the
> _ev_time_deregister, and assert !libxl__ev_child_inuse.

Setting the aodev to finished is always done in 
libxl__ao_device_check_last, which should be called by the user.

>
>> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
>> index 45b776c..da5b02b 100644
>> --- a/tools/libxl/libxl_internal.h
>> +++ b/tools/libxl/libxl_internal.h
> ...
>> +    /* device hotplug execution */
>> +    pid_t pid;
>> +    char *what;
>> +    libxl__ev_time ev;
>> +    libxl__ev_child child;
>
> Is the pid inside child not sufficient ?

Probably, I just wasn't sure if the user of libxl__ev_child was allowed 
to pick values inside the struct, now I saw the comment :)