Re: [Xen-devel] [PATCH 18 of 27 v3] libxl: merge libxl__device_del into libxl__device_remove

[Roger Pau Monnà <roger.pau@xxxxxxxxxxxxx>]

2011/10/18 Ian Campbell <ian.campbell@xxxxxxxxxx>:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@xxxxxxxxxx>
> # Date 1318941402 -3600
> # Node ID 4020531a53fcb1a5579de3225b99a29a7619c7df
> # Parent Â5547b8593511cb37b830f6ec73f480e607f639c2
> libxl: merge libxl__device_del into libxl__device_remove
>
> Note that the "wait" parameter added to libxl_device_remove is different to 
> the
> wait paramter previously used by similar functions. In the past not-wait meant
> forced whereas now in means wait for a graceful shutdown, as opposed to 
> setting
> off a graceful shutdown but not waiting.
>
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
>
> diff -r 5547b8593511 -r 4020531a53fc tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c    Tue Oct 18 13:36:42 2011 +0100
> +++ b/tools/libxl/libxl.c    Tue Oct 18 13:36:42 2011 +0100
> @@ -1075,7 +1075,7 @@ int libxl_device_disk_del(libxl_ctx *ctx
>   device.devid      Â= devid;
>   device.kind       = LIBXL__DEVICE_KIND_VBD;
> Â Â if (wait)
> - Â Â Â Ârc = libxl__device_del(&gc, &device);
> + Â Â Â Ârc = libxl__device_remove(&gc, &device, wait);
> Â Â else
> Â Â Â Â rc = libxl__device_destroy(&gc, &device);
> Âout_free:
> @@ -1290,7 +1290,7 @@ int libxl_device_nic_del(libxl_ctx *ctx,
>   device.kind       = LIBXL__DEVICE_KIND_VIF;
>
> Â Â if (wait)
> - Â Â Â Ârc = libxl__device_del(&gc, &device);
> + Â Â Â Ârc = libxl__device_remove(&gc, &device, wait);
> Â Â else
> Â Â Â Â rc = libxl__device_destroy(&gc, &device);
>
> diff -r 5547b8593511 -r 4020531a53fc tools/libxl/libxl_device.c
> --- a/tools/libxl/libxl_device.c    ÂTue Oct 18 13:36:42 2011 +0100
> +++ b/tools/libxl/libxl_device.c    ÂTue Oct 18 13:36:42 2011 +0100
> @@ -367,57 +367,6 @@ int libxl__device_disk_dev_number(const
> Â Â return -1;
> Â}
>
> -int libxl__device_remove(libxl__gc *gc, libxl__device *dev)
> -{
> - Â Âlibxl_ctx *ctx = libxl__gc_owner(gc);
> - Â Âxs_transaction_t t;
> - Â Âchar *be_path = libxl__device_backend_path(gc, dev);
> - Â Âchar *state_path = libxl__sprintf(gc, "%s/state", be_path);
> - Â Âchar *state = libxl__xs_read(gc, XBT_NULL, state_path);
> - Â Âint rc = 0;
> -
> - Â Âif (!state)
> - Â Â Â Âgoto out;
> - Â Âif (atoi(state) != 4) {
> - Â Â Â Âlibxl__device_destroy_tapdisk(gc, be_path);
> - Â Â Â Âxs_rm(ctx->xsh, XBT_NULL, be_path);
> - Â Â Â Âgoto out;
> - Â Â}
> -
> -retry_transaction:
> - Â Ât = xs_transaction_start(ctx->xsh);
> - Â Âxs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/online", be_path), "0", 
> strlen("0"));
> - Â Âxs_write(ctx->xsh, t, state_path, "5", strlen("5"));
> - Â Âif (!xs_transaction_end(ctx->xsh, t, 0)) {
> - Â Â Â Âif (errno == EAGAIN)
> - Â Â Â Â Â Âgoto retry_transaction;
> - Â Â Â Âelse {
> - Â Â Â Â Â Ârc = -1;
> - Â Â Â Â Â Âgoto out;
> - Â Â Â Â}
> - Â Â}
> -
> - Â Âxs_watch(ctx->xsh, state_path, be_path);
> - Â Âlibxl__device_destroy_tapdisk(gc, be_path);
> - Â Ârc = 1;
> -out:
> - Â Âreturn rc;
> -}
> -
> -int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
> -{
> - Â Âlibxl_ctx *ctx = libxl__gc_owner(gc);
> - Â Âchar *be_path = libxl__device_backend_path(gc, dev);
> - Â Âchar *fe_path = libxl__device_frontend_path(gc, dev);
> -
> - Â Âxs_rm(ctx->xsh, XBT_NULL, be_path);
> - Â Âxs_rm(ctx->xsh, XBT_NULL, fe_path);
> -
> - Â Âlibxl__device_destroy_tapdisk(gc, be_path);
> -
> - Â Âreturn 0;
> -}
> -
> Âstatic int wait_for_dev_destroy(libxl__gc *gc, struct timeval *tv)
> Â{
> Â Â libxl_ctx *ctx = libxl__gc_owner(gc);
> @@ -446,6 +395,71 @@ static int wait_for_dev_destroy(libxl__g
> Â Â return rc;
> Â}
>
> +/*
> + * Returns 0 (device already destroyed) or 1 (caller must
> + * wait_for_dev_destroy) on success, ERROR_* on fail.
> + */
> +int libxl__device_remove(libxl__gc *gc, libxl__device *dev, int wait)
> +{
> + Â Âlibxl_ctx *ctx = libxl__gc_owner(gc);
> + Â Âxs_transaction_t t;
> + Â Âchar *be_path = libxl__device_backend_path(gc, dev);
> + Â Âchar *state_path = libxl__sprintf(gc, "%s/state", be_path);
> + Â Âchar *state = libxl__xs_read(gc, XBT_NULL, state_path);
> + Â Âint rc = 0;
> +
> + Â Âif (!state)
> + Â Â Â Âgoto out;
> + Â Âif (atoi(state) != 4) {
> + Â Â Â Âlibxl__device_destroy_tapdisk(gc, be_path);
> + Â Â Â Âxs_rm(ctx->xsh, XBT_NULL, be_path);
> + Â Â Â Âgoto out;
> + Â Â}
> +
> +retry_transaction:
> + Â Ât = xs_transaction_start(ctx->xsh);
> + Â Âxs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/online", be_path), "0", 
> strlen("0"));
> + Â Âxs_write(ctx->xsh, t, state_path, "5", strlen("5"));
> + Â Âif (!xs_transaction_end(ctx->xsh, t, 0)) {
> + Â Â Â Âif (errno == EAGAIN)
> + Â Â Â Â Â Âgoto retry_transaction;
> + Â Â Â Âelse {
> + Â Â Â Â Â Ârc = ERROR_FAIL;
> + Â Â Â Â Â Âgoto out;
> + Â Â Â Â}
> + Â Â}
> +
> + Â Âxs_watch(ctx->xsh, state_path, be_path);
> + Â Âlibxl__device_destroy_tapdisk(gc, be_path);
> +
> + Â Âif (wait) {
> + Â Â Â Âstruct timeval tv;
> + Â Â Â Âtv.tv_sec = LIBXL_DESTROY_TIMEOUT;
> + Â Â Â Âtv.tv_usec = 0;
> + Â Â Â Â(void)wait_for_dev_destroy(gc, &tv);

I think we should check the return value of wait_for_dev_destroy here,
since it might timeout, and the device is not removed then, so
libxl__device_remove should return 1 in that case (whereas it always
returns 0 now in that case, even when the device is still being
watched).

> + Â Â Â Âxs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(gc, dev));
> + Â Â} else {
> + Â Â Â Ârc = 1; /* Caller must wait_for_dev_destroy */
> + Â Â}
> +
> +out:
> + Â Âreturn rc;
> +}
> +
> +int libxl__device_destroy(libxl__gc *gc, libxl__device *dev)
> +{
> + Â Âlibxl_ctx *ctx = libxl__gc_owner(gc);
> + Â Âchar *be_path = libxl__device_backend_path(gc, dev);
> + Â Âchar *fe_path = libxl__device_frontend_path(gc, dev);
> +
> + Â Âxs_rm(ctx->xsh, XBT_NULL, be_path);
> + Â Âxs_rm(ctx->xsh, XBT_NULL, fe_path);
> +
> + Â Âlibxl__device_destroy_tapdisk(gc, be_path);
> +
> + Â Âreturn 0;
> +}
> +
> Âint libxl__devices_destroy(libxl__gc *gc, uint32_t domid, int force)
> Â{
> Â Â libxl_ctx *ctx = libxl__gc_owner(gc);
> @@ -485,8 +499,12 @@ int libxl__devices_destroy(libxl__gc *gc
> Â Â Â Â Â Â Â Â if (force) {
> Â Â Â Â Â Â Â Â Â Â libxl__device_destroy(gc, &dev);
> Â Â Â Â Â Â Â Â } else {
> - Â Â Â Â Â Â Â Â Â Âif (libxl__device_remove(gc, &dev) > 0)
> - Â Â Â Â Â Â Â Â Â Â Â Ân_watches++;
> + Â Â Â Â Â Â Â Â Â Âint rc = libxl__device_remove(gc, &dev, 0);
> + Â Â Â Â Â Â Â Â Â Âif (rc < 0)
> + Â Â Â Â Â Â Â Â Â Â Â ÂLIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "cannot remove device %s\n", path);
> + Â Â Â Â Â Â Â Â Â Âelse
> + Â Â Â Â Â Â Â Â Â Â Â Ân_watches += rc;
> Â Â Â Â Â Â Â Â }
> Â Â Â Â Â Â }
> Â Â Â Â }
> @@ -504,8 +522,12 @@ int libxl__devices_destroy(libxl__gc *gc
> Â Â Â Â if (force) {
> Â Â Â Â Â Â libxl__device_destroy(gc, &dev);
> Â Â Â Â } else {
> - Â Â Â Â Â Âif (libxl__device_remove(gc, &dev) > 0)
> - Â Â Â Â Â Â Â Ân_watches++;
> + Â Â Â Â Â Âint rc = libxl__device_remove(gc, &dev, 0);
> + Â Â Â Â Â Âif (rc < 0)
> + Â Â Â Â Â Â Â ÂLIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â "cannot remove device %s\n", path);
> + Â Â Â Â Â Âelse
> + Â Â Â Â Â Â Â Ân_watches += rc;
> Â Â Â Â }
> Â Â }
>
> @@ -530,29 +552,6 @@ out:
> Â Â return 0;
> Â}
>
> -int libxl__device_del(libxl__gc *gc, libxl__device *dev)
> -{
> - Â Âlibxl_ctx *ctx = libxl__gc_owner(gc);
> - Â Âstruct timeval tv;
> - Â Âint rc;
> -
> - Â Ârc = libxl__device_remove(gc, dev);
> - Â Âif (rc == -1) {
> - Â Â Â Ârc = ERROR_FAIL;
> - Â Â Â Âgoto out;
> - Â Â}
> -
> - Â Âtv.tv_sec = LIBXL_DESTROY_TIMEOUT;
> - Â Âtv.tv_usec = 0;
> - Â Â(void)wait_for_dev_destroy(gc, &tv);
> -
> - Â Âxs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(gc, dev));
> - Â Ârc = 0;
> -
> -out:
> - Â Âreturn rc;
> -}
> -
> Âint libxl__wait_for_device_model(libxl__gc *gc,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âuint32_t domid, char *state,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âlibxl__spawn_starting *spawning,
> diff -r 5547b8593511 -r 4020531a53fc tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h   ÂTue Oct 18 13:36:42 2011 +0100
> +++ b/tools/libxl/libxl_internal.h   ÂTue Oct 18 13:36:42 2011 +0100
> @@ -242,8 +242,7 @@ _hidden char *libxl__device_backend_path
> Â_hidden char *libxl__device_frontend_path(libxl__gc *gc, libxl__device 
> *device);
> Â_hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â libxl__device *dev);
> -_hidden int libxl__device_del(libxl__gc *gc, libxl__device *dev);
> -_hidden int libxl__device_remove(libxl__gc *gc, libxl__device *dev);
> +_hidden int libxl__device_remove(libxl__gc *gc, libxl__device *dev, int 
> wait);
> Â_hidden int libxl__device_destroy(libxl__gc *gc, libxl__device *dev);
> Â_hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid, int force);
> Â_hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char 
> *state);
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel