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 v2] libxl: merge libxl__device_del into

To: Ian Campbell <ian.campbell@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 18 of 27 v2] libxl: merge libxl__device_del into libxl__device_remove
From: Roger Pau Monné <roger.pau@xxxxxxxxxxxxx>
Date: Mon, 17 Oct 2011 16:34:49 +0200
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Mon, 17 Oct 2011 07:35:21 -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=rpfqJ8tulPBvFvQxMSkiXZEyf0bJgEVVhnX2ilOwLjE=; b=NGt0tODqCdpIJD1OSh1QTjXv8onD18QLV4fpMek5WeoTi0L4PIPzeM/8H3LfHqhpn4 rokyCNWEwwq5sIvKGW4qO05R7L+td25ZsiEMVokrJYqaQqiDY4qxiAfZ6eAEvjK2ksVY 2Vg4xW8PopqHK/s28crHrOLg9ErrATPlSCFW8=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <95b2f3977d439bba3a01.1318499623@xxxxxxxxxxxxxxxxxxxxx>
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.1318499605@xxxxxxxxxxxxxxxxxxxxx> <95b2f3977d439bba3a01.1318499623@xxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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

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