| Message ID | 20190802153606.32061-18-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libxl refactoring to use ev_qmp (with API changes) | expand |
Anthony PERARD writes ("[PATCH 17/35] libxl: Add libxl__ev_qmp to libxl__ao_device"):
> `aodev->qmp' is initialised in libxl__prepare_ao_device(), but since
> there isn't a single exit path for a `libxl__ao_device', users of this
> new `qmp' field will have to disposed of it.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
It might be nice to add a comment about the field(s) of
libxl__ao_device which callers of libxl__prepare_ao_device are
expected to clean up themselves.
Ian.
On Tue, Sep 17, 2019 at 06:07:31PM +0100, Ian Jackson wrote: > Anthony PERARD writes ("[PATCH 17/35] libxl: Add libxl__ev_qmp to libxl__ao_device"): > > `aodev->qmp' is initialised in libxl__prepare_ao_device(), but since > > there isn't a single exit path for a `libxl__ao_device', users of this > > new `qmp' field will have to disposed of it. > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> > > It might be nice to add a comment about the field(s) of > libxl__ao_device which callers of libxl__prepare_ao_device are > expected to clean up themselves. Something like that? diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -2581,6 +2581,10 @@ typedef void libxl__device_callback(libxl__egc*, libxl__ao_device*); * Once _prepare has been called on a libxl__ao_device, it is safe to just * discard this struct, there's no need to call any destroy function. * _prepare can also be called multiple times with the same libxl__ao_device. + * + * But if any of the fields `backend_ds', `timeout', `xswait', `qmp' is + * used by a caller of _prepare, the caller will have to arrange to clean + * or dispose of them. */ _hidden void libxl__prepare_ao_device(libxl__ao *ao, libxl__ao_device *aodev);
Anthony PERARD writes ("Re: [PATCH 17/35] libxl: Add libxl__ev_qmp to libxl__ao_device"):
> On Tue, Sep 17, 2019 at 06:07:31PM +0100, Ian Jackson wrote:
> > It might be nice to add a comment about the field(s) of
> > libxl__ao_device which callers of libxl__prepare_ao_device are
> > expected to clean up themselves.
>
> Something like that?
>
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -2581,6 +2581,10 @@ typedef void libxl__device_callback(libxl__egc*, libxl__ao_device*);
> * Once _prepare has been called on a libxl__ao_device, it is safe to just
> * discard this struct, there's no need to call any destroy function.
> * _prepare can also be called multiple times with the same libxl__ao_device.
> + *
> + * But if any of the fields `backend_ds', `timeout', `xswait', `qmp' is
> + * used by a caller of _prepare, the caller will have to arrange to clean
> + * or dispose of them.
LGTM
Thanks,
Ian.
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c index 1941fe780a..1402b61a81 100644 --- a/tools/libxl/libxl_device.c +++ b/tools/libxl/libxl_device.c @@ -644,6 +644,8 @@ void libxl__prepare_ao_device(libxl__ao *ao, libxl__ao_device *aodev) * without actually calling any hotplug script */ libxl__async_exec_init(&aodev->aes); libxl__ev_child_init(&aodev->child); + + libxl__ev_qmp_init(&aodev->qmp); } /* multidev */ diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 7cabfd61b7..cc9318f33a 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -2622,6 +2622,7 @@ struct libxl__ao_device { bool update_json; /* for asynchronous execution of synchronous-only syscalls etc. */ libxl__ev_child child; + libxl__ev_qmp qmp; }; /*
`aodev->qmp' is initialised in libxl__prepare_ao_device(), but since there isn't a single exit path for a `libxl__ao_device', users of this new `qmp' field will have to disposed of it. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- tools/libxl/libxl_device.c | 2 ++ tools/libxl/libxl_internal.h | 1 + 2 files changed, 3 insertions(+)