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

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

2011/10/13 Ian Campbell <ian.campbell@xxxxxxxxxx>:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@xxxxxxxxxx>
> # Date 1318499565 -3600
> # Node ID 95b2f3977d439bba3a01db4a4403a4307f238377
> # Parent Âf60954a04df9acbddf68af4f2d46c3910f3b0afc
> 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 f60954a04df9 -r 95b2f3977d43 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c    Thu Oct 13 10:52:45 2011 +0100
> +++ b/tools/libxl/libxl.c    Thu Oct 13 10:52:45 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 f60954a04df9 -r 95b2f3977d43 tools/libxl/libxl_device.c
> --- a/tools/libxl/libxl_device.c    ÂThu Oct 13 10:52:45 2011 +0100
> +++ b/tools/libxl/libxl_device.c    ÂThu Oct 13 10:52:45 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,67 @@ static int wait_for_dev_destroy(libxl__g
> Â Â return rc;
> Â}
>
> +/* Returns 0 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);

I think here we should return something different than 0 (possibly 1?)
so the number of watches (n_watches) is not increased.

> + Â Â Â Â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);
> + Â Â Â Âxs_rm(ctx->xsh, XBT_NULL, libxl__device_frontend_path(gc, dev));

Here we should check if the device is removed correctly or not, so
that the number of watches is not increased:

if(wait_for_dev_destroy(gc, &tv) != 0) /* device destroyed */
    rc = 1;

> + Â Â}
> +
> + Â Ârc = 0;

This should also be removed, since rc is initialized to 0 already.

> +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,7 +495,7 @@ int libxl__devices_destroy(libxl__gc *gc
> Â Â Â Â Â Â Â Â if (force) {
> Â Â Â Â Â Â Â Â Â Â libxl__device_destroy(gc, &dev);
> Â Â Â Â Â Â Â Â } else {
> - Â Â Â Â Â Â Â Â Â Âif (libxl__device_remove(gc, &dev) > 0)
> + Â Â Â Â Â Â Â Â Â Âif (libxl__device_remove(gc, &dev, 0) == 0)
> Â Â Â Â Â Â Â Â Â Â Â Â n_watches++;
> Â Â Â Â Â Â Â Â }
> Â Â Â Â Â Â }
> @@ -504,7 +514,7 @@ int libxl__devices_destroy(libxl__gc *gc
> Â Â Â Â if (force) {
> Â Â Â Â Â Â libxl__device_destroy(gc, &dev);
> Â Â Â Â } else {
> - Â Â Â Â Â Âif (libxl__device_remove(gc, &dev) > 0)
> + Â Â Â Â Â Âif (libxl__device_remove(gc, &dev, 0) == 0)
> Â Â Â Â Â Â Â Â n_watches++;
> Â Â Â Â }
> Â Â }
> @@ -530,29 +540,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 f60954a04df9 -r 95b2f3977d43 tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h   ÂThu Oct 13 10:52:45 2011 +0100
> +++ b/tools/libxl/libxl_internal.h   ÂThu Oct 13 10:52:45 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