|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 2/7] libxl: add a transaction parameter to libxl__device_generic_add
On Mon, 2012-04-16 at 19:33 +0100, Stefano Stabellini wrote:
> Add a xs_transaction_t parameter to libxl__device_generic_add, if it is
> XBT_NULL, allocate a proper one.
>
> Update all the callers.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> ---
> tools/libxl/libxl.c | 10 +++++-----
> tools/libxl/libxl_device.c | 14 +++++++-------
> tools/libxl/libxl_internal.h | 4 ++--
> tools/libxl/libxl_pci.c | 2 +-
> 4 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index e645d2a..85082eb 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1401,7 +1401,7 @@ int libxl_device_disk_add(libxl_ctx *ctx, uint32_t
> domid, libxl_device_disk *dis
> flexarray_append(front, "device-type");
> flexarray_append(front, disk->is_cdrom ? "cdrom" : "disk");
>
> - libxl__device_generic_add(gc, &device,
> + libxl__device_generic_add(gc, XBT_NULL, &device,
> libxl__xs_kvs_of_flexarray(gc, back,
> back->count),
> libxl__xs_kvs_of_flexarray(gc, front,
> front->count));
>
> @@ -1795,7 +1795,7 @@ int libxl_device_nic_add(libxl_ctx *ctx, uint32_t
> domid, libxl_device_nic *nic)
> flexarray_append(front, "mac");
> flexarray_append(front, libxl__sprintf(gc,
> LIBXL_MAC_FMT,
> LIBXL_MAC_BYTES(nic->mac)));
> - libxl__device_generic_add(gc, &device,
> + libxl__device_generic_add(gc, XBT_NULL, &device,
> libxl__xs_kvs_of_flexarray(gc, back,
> back->count),
> libxl__xs_kvs_of_flexarray(gc, front,
> front->count));
>
> @@ -2073,7 +2073,7 @@ int libxl__device_console_add(libxl__gc *gc, uint32_t
> domid,
> flexarray_append(front, LIBXL_XENCONSOLE_PROTOCOL);
> }
>
> - libxl__device_generic_add(gc, &device,
> + libxl__device_generic_add(gc, XBT_NULL, &device,
> libxl__xs_kvs_of_flexarray(gc, back,
> back->count),
> libxl__xs_kvs_of_flexarray(gc, front,
> front->count));
> rc = 0;
> @@ -2144,7 +2144,7 @@ int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t
> domid, libxl_device_vkb *vkb)
> flexarray_append(front, "state");
> flexarray_append(front, libxl__sprintf(gc, "%d", 1));
>
> - libxl__device_generic_add(gc, &device,
> + libxl__device_generic_add(gc, XBT_NULL, &device,
> libxl__xs_kvs_of_flexarray(gc, back,
> back->count),
> libxl__xs_kvs_of_flexarray(gc, front,
> front->count));
> rc = 0;
> @@ -2277,7 +2277,7 @@ int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t
> domid, libxl_device_vfb *vfb)
> libxl__sprintf(gc, "%d", vfb->backend_domid));
> flexarray_append_pair(front, "state", libxl__sprintf(gc, "%d", 1));
>
> - libxl__device_generic_add(gc, &device,
> + libxl__device_generic_add(gc, XBT_NULL, &device,
> libxl__xs_kvs_of_flexarray(gc, back,
> back->count),
> libxl__xs_kvs_of_flexarray(gc, front,
> front->count));
> rc = 0;
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index c7e057d..05909c5 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -58,14 +58,14 @@ int libxl__parse_backend_path(libxl__gc *gc,
> return libxl__device_kind_from_string(strkind, &dev->backend_kind);
> }
>
> -int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
> - char **bents, char **fents)
> +int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
> + libxl__device *device, char **bents, char **fents)
> {
> libxl_ctx *ctx = libxl__gc_owner(gc);
> char *frontend_path, *backend_path;
> - xs_transaction_t t;
> struct xs_permissions frontend_perms[2];
> struct xs_permissions backend_perms[2];
> + int create_transaction = t == XBT_NULL;
>
> frontend_path = libxl__device_frontend_path(gc, device);
> backend_path = libxl__device_backend_path(gc, device);
> @@ -81,7 +81,8 @@ int libxl__device_generic_add(libxl__gc *gc, libxl__device
> *device,
> backend_perms[1].perms = XS_PERM_READ;
>
> retry_transaction:
> - t = xs_transaction_start(ctx->xsh);
> + if (create_transaction)
> + t = xs_transaction_start(ctx->xsh);
> /* FIXME: read frontend_path and check state before removing stuff */
>
> if (fents) {
> @@ -101,13 +102,12 @@ retry_transaction:
> }
>
> if (!xs_transaction_end(ctx->xsh, t, 0)) {
Do we really want to end the transaction for caller provided t? (i.e.
when create_transaction == False)
It would seem more expected to me to return to the caller and expect
them to complete the transaction and perform error handling etc. If the
caller doesn't have things of its own to do in the transaction then why
does it have one in hand to pass in?
> - if (errno == EAGAIN)
> + if (errno == EAGAIN && create_transaction)
> goto retry_transaction;
> else
> LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xs transaction failed");
> }
> -
> - return 0;
> + return ERROR_FAIL;
Where is the success exit path in this function now?
> }
>
> typedef struct {
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 4e0d203..c0991eb 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -681,8 +681,8 @@ _hidden int libxl__device_console_add(libxl__gc *gc,
> uint32_t domid,
> libxl__device_console *console,
> libxl__domain_build_state *state);
>
> -_hidden int libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
> - char **bents, char **fents);
> +_hidden int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
> + libxl__device *device, char **bents, char **fents);
> _hidden char *libxl__device_backend_path(libxl__gc *gc, libxl__device
> *device);
> _hidden char *libxl__device_frontend_path(libxl__gc *gc, libxl__device
> *device);
> _hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> index e8b8839..202a101 100644
> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -102,7 +102,7 @@ int libxl__create_pci_backend(libxl__gc *gc, uint32_t
> domid,
> flexarray_append_pair(front, "backend-id", libxl__sprintf(gc, "%d", 0));
> flexarray_append_pair(front, "state", libxl__sprintf(gc, "%d", 1));
>
> - libxl__device_generic_add(gc, &device,
> + libxl__device_generic_add(gc, XBT_NULL, &device,
> libxl__xs_kvs_of_flexarray(gc, back,
> back->count),
> libxl__xs_kvs_of_flexarray(gc, front,
> front->count));
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |