WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Ian Campbell <ian.campbell@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 18 of 27 v3] libxl: merge libxl__device_del into libxl__device_remove
From: Roger Pau Monné <roger.pau@xxxxxxxxxxxxx>
Date: Wed, 19 Oct 2011 12:22:56 +0200
Cc: ian.jackson@xxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Wed, 19 Oct 2011 03:24:10 -0700
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=k1V0r8EjSif+/fk6nQ51BSNsdDEdhrvaGvhJlHI08+k=; b=h24hdMFli5IxgnYOOgyc5il5/lBts9n2+1GFB2VRGeEOlo6YQRTAMD+bMExb7shQsj 7G3x7y1fkT1d0ZiZusqVu659CyFYWH1cnS13hp6dWcdpD2JBdvMhSTl/I/FsSe/py4ef LXVzIOsSkBKAGLzXNVoCMdaYKMPRadXYONWUM=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4020531a53fcb1a5579d.1318942513@xxxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <patchbomb.1318942495@xxxxxxxxxxxxxxxxxxxxxxxxx> <4020531a53fcb1a5579d.1318942513@xxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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

<Prev in Thread] Current Thread [Next in Thread>