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] libxl: remove console 0 backend directory from x

To: Ian Campbell <ian.campbell@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] libxl: remove console 0 backend directory from xenstore on devices destroy
From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Date: Tue, 5 Oct 2010 14:39:53 +0100
Cc: Ian, Campbell <Ian.Campbell@xxxxxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Tue, 05 Oct 2010 06:40:20 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <9e0d9b5460dc1c3cad0f.1286207191@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: <9e0d9b5460dc1c3cad0f.1286207191@xxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Alpine 2.00 (DEB 1167 2008-08-23)
The patch is good but I think it would be probably cleaner to add a
function like:

libxl__get_fe_path(libxl__device_kinds kind, int devid)

that would be called by libxl__device_generic_add to figure out the fe
path, rather than adding yet another parameter to the function,
parameter that in most cases is NULL anyway.



On Mon, 4 Oct 2010, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@xxxxxxxxxx>
> # Date 1286207114 -3600
> # Node ID 9e0d9b5460dc1c3cad0f5576157ac10b4045b4de
> # Parent  a2c40fa3bc6c8471e1988e445f449975cf6f3822
> libxl: remove console 0 backend directory from xenstore on devices destroy
> 
> The first PV console device has an unusual frontend path for
> historical reasons (/local/domain/<domid>/console rather than
> /local/domain/<domid>/device/console/0).
> 
> Therefore we need to check this additional frontend path as well as
> those under /local/domain/<domid>/device when tearing down all
> backends or else we miss the backend for the console.
> 
> As part of this we need to ensure that the frontend directory has a
> valid link to the backend, currently this link does not exist.
> 
> Fix this by allowing libxl__device_generic_add to take an optional
> frontend path suffix relative to the dompath allowing us to use the
> function even for setting up the special case frontend.
> 
> This also fixes the link from backend to frontend which until now gave
> a dangling link to the normal device path instead of the exceptional
> one.
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> 
> diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c       Mon Oct 04 15:39:30 2010 +0100
> +++ b/tools/libxl/libxl.c       Mon Oct 04 16:45:14 2010 +0100
> @@ -1897,7 +1897,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
> 
>      libxl__device_generic_add(ctx, &device,
>                               libxl__xs_kvs_of_flexarray(&gc, back, boffset),
> -                             libxl__xs_kvs_of_flexarray(&gc, front, 
> foffset));
> +                             libxl__xs_kvs_of_flexarray(&gc, front, 
> foffset), NULL);
> 
>      rc = 0;
> 
> @@ -2047,7 +2047,7 @@ int libxl_device_nic_add(libxl_ctx *ctx,
> 
>      libxl__device_generic_add(ctx, &device,
>                               libxl__xs_kvs_of_flexarray(&gc, back, boffset),
> -                             libxl__xs_kvs_of_flexarray(&gc, front, 
> foffset));
> +                             libxl__xs_kvs_of_flexarray(&gc, front, 
> foffset), NULL);
> 
>      /* FIXME: wait for plug */
>      rc = 0;
> @@ -2232,7 +2232,7 @@ int libxl_device_net2_add(libxl_ctx *ctx
> 
>      libxl__device_generic_add(ctx, &device,
>                               libxl__xs_kvs_of_flexarray(&gc, back, boffset),
> -                             libxl__xs_kvs_of_flexarray(&gc, front, 
> foffset));
> +                             libxl__xs_kvs_of_flexarray(&gc, front, 
> foffset), NULL);
> 
>      /* FIXME: wait for plug */
>      rc = 0;
> @@ -2324,36 +2324,13 @@ int libxl_device_console_add(libxl_ctx *
>  int libxl_device_console_add(libxl_ctx *ctx, uint32_t domid, 
> libxl_device_console *console)
>  {
>      libxl__gc gc = LIBXL_INIT_GC(ctx);
> +    char *frontend_path = NULL;
>      flexarray_t *front;
>      flexarray_t *back;
>      unsigned int boffset = 0;
>      unsigned int foffset = 0;
>      libxl__device device;
>      int rc;
> -
> -    if (console->build_state) {
> -        xs_transaction_t t;
> -        char **ents = (char **) libxl__calloc(&gc, 11, sizeof(char *));
> -        ents[0] = "console/port";
> -        ents[1] = libxl__sprintf(&gc, "%"PRIu32, 
> console->build_state->console_port);
> -        ents[2] = "console/ring-ref";
> -        ents[3] = libxl__sprintf(&gc, "%lu", 
> console->build_state->console_mfn);
> -        ents[4] = "console/limit";
> -        ents[5] = libxl__sprintf(&gc, "%d", LIBXL_XENCONSOLE_LIMIT);
> -        ents[6] = "console/type";
> -        if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
> -            ents[7] = "xenconsoled";
> -        else
> -            ents[7] = "ioemu";
> -        ents[8] = "console/output";
> -        ents[9] = console->output;
> -retry_transaction:
> -        t = xs_transaction_start(ctx->xsh);
> -        libxl__xs_writev(&gc, t, libxl__xs_get_dompath(&gc, console->domid), 
> ents);
> -        if (!xs_transaction_end(ctx->xsh, t, 0))
> -            if (errno == EAGAIN)
> -                goto retry_transaction;
> -    }
> 
>      front = flexarray_make(16, 1);
>      if (!front) {
> @@ -2384,29 +2361,40 @@ retry_transaction:
>      flexarray_set(back, boffset++, "protocol");
>      flexarray_set(back, boffset++, LIBXL_XENCONSOLE_PROTOCOL);
> 
> -    /* if devid == 0 do not add the frontend to device/console/ because
> -     * it has already been added to console/ */
> -    if (device.devid > 0) {
> -        flexarray_set(front, foffset++, "backend-id");
> -        flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d", 
> console->backend_domid));
> +    flexarray_set(front, foffset++, "backend-id");
> +    flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d", 
> console->backend_domid));
> +    flexarray_set(front, foffset++, "limit");
> +    flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d", 
> LIBXL_XENCONSOLE_LIMIT));
> +    flexarray_set(front, foffset++, "type");
> +    if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
> +        flexarray_set(front, foffset++, "xenconsoled");
> +    else
> +        flexarray_set(front, foffset++, "ioemu");
> +    flexarray_set(front, foffset++, "output");
> +    flexarray_set(front, foffset++, console->output);
> +
> +    if (device.devid == 0) {
> +        if (console->build_state == NULL) {
> +            rc = ERROR_INVAL;
> +            goto out_free;
> +        }
> +        frontend_path = "console";
> +
> +        flexarray_set(front, foffset++, "port");
> +        flexarray_set(front, foffset++, libxl__sprintf(&gc, "%"PRIu32, 
> console->build_state->console_port));
> +        flexarray_set(front, foffset++, "ring-ref");
> +        flexarray_set(front, foffset++, libxl__sprintf(&gc, "%lu", 
> console->build_state->console_mfn));
> +    } else {
>          flexarray_set(front, foffset++, "state");
>          flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d", 1));
> -        flexarray_set(front, foffset++, "limit");
> -        flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d", 
> LIBXL_XENCONSOLE_LIMIT));
>          flexarray_set(front, foffset++, "protocol");
>          flexarray_set(front, foffset++, LIBXL_XENCONSOLE_PROTOCOL);
> -        flexarray_set(front, foffset++, "type");
> -        if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
> -            flexarray_set(front, foffset++, "xenconsoled");
> -        else
> -            flexarray_set(front, foffset++, "ioemu");
> -        flexarray_set(front, foffset++, "output");
> -        flexarray_set(front, foffset++, console->output);
>      }
> 
>      libxl__device_generic_add(ctx, &device,
>                               libxl__xs_kvs_of_flexarray(&gc, back, boffset),
> -                             libxl__xs_kvs_of_flexarray(&gc, front, 
> foffset));
> +                             libxl__xs_kvs_of_flexarray(&gc, front, foffset),
> +                             frontend_path);
>      rc = 0;
>  out_free:
>      flexarray_free(back);
> @@ -2461,7 +2449,7 @@ int libxl_device_vkb_add(libxl_ctx *ctx,
> 
>      libxl__device_generic_add(ctx, &device,
>                               libxl__xs_kvs_of_flexarray(&gc, back, boffset),
> -                             libxl__xs_kvs_of_flexarray(&gc, front, 
> foffset));
> +                             libxl__xs_kvs_of_flexarray(&gc, front, 
> foffset), NULL);
>      rc = 0;
>  out_free:
>      flexarray_free(back);
> @@ -2722,7 +2710,7 @@ int libxl_device_vfb_add(libxl_ctx *ctx,
> 
>      libxl__device_generic_add(ctx, &device,
>                               libxl__xs_kvs_of_flexarray(&gc, back, boffset),
> -                             libxl__xs_kvs_of_flexarray(&gc, front, 
> foffset));
> +                             libxl__xs_kvs_of_flexarray(&gc, front, 
> foffset), NULL);
>      rc = 0;
>  out_free:
>      flexarray_free(front);
> diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl_device.c
> --- a/tools/libxl/libxl_device.c        Mon Oct 04 15:39:30 2010 +0100
> +++ b/tools/libxl/libxl_device.c        Mon Oct 04 16:45:14 2010 +0100
> @@ -40,10 +40,10 @@ static const char *string_of_kinds[] = {
>  };
> 
>  int libxl__device_generic_add(libxl_ctx *ctx, libxl__device *device,
> -                             char **bents, char **fents)
> +                              char **bents, char **fents, char 
> *frontend_path)
>  {
>      libxl__gc gc = LIBXL_INIT_GC(ctx);
> -    char *dom_path_backend, *dom_path, *frontend_path, *backend_path;
> +    char *dom_path_backend, *dom_path, *backend_path;
>      xs_transaction_t t;
>      struct xs_permissions frontend_perms[2];
>      struct xs_permissions backend_perms[2];
> @@ -57,8 +57,12 @@ int libxl__device_generic_add(libxl_ctx
>      dom_path_backend = libxl__xs_get_dompath(&gc, device->backend_domid);
>      dom_path = libxl__xs_get_dompath(&gc, device->domid);
> 
> -    frontend_path = libxl__sprintf(&gc, "%s/device/%s/%d",
> -                                  dom_path, string_of_kinds[device->kind], 
> device->devid);
> +    if (frontend_path)
> +        frontend_path = libxl__sprintf(&gc, "%s/%s",
> +                                       dom_path, frontend_path);
> +    else
> +        frontend_path = libxl__sprintf(&gc, "%s/device/%s/%d",
> +                                       dom_path, 
> string_of_kinds[device->kind], device->devid);
>      backend_path = libxl__sprintf(&gc, "%s/backend/%s/%u/%d",
>                                   dom_path_backend, 
> string_of_kinds[device->backend_kind], device->domid, device->devid);
> 
> @@ -326,6 +330,16 @@ int libxl__devices_destroy(libxl_ctx *ct
>              }
>          }
>      }
> +
> +    /* console 0 frontend directory is not under 
> /local/domain/<domid>/device */
> +    fe_path = libxl__sprintf(&gc, "/local/domain/%d/console", domid);
> +    be_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc, 
> "%s/backend", fe_path));
> +    if (be_path && strcmp(be_path, "")) {
> +        if (libxl__device_destroy(ctx, be_path, force) > 0)
> +            n_watches++;
> +        flexarray_set(toremove, n++, libxl__dirname(&gc, be_path));
> +    }
> +
>      if (!force) {
>          /* Linux-ism. Most implementations leave the timeout
>           * untouched after select. Linux, however, will chip
> diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h      Mon Oct 04 15:39:30 2010 +0100
> +++ b/tools/libxl/libxl_internal.h      Mon Oct 04 16:45:14 2010 +0100
> @@ -171,7 +171,7 @@ _hidden int libxl__device_disk_dev_numbe
>  _hidden int libxl__device_disk_dev_number(char *virtpath);
> 
>  _hidden int libxl__device_generic_add(libxl_ctx *ctx, libxl__device *device,
> -                             char **bents, char **fents);
> +                                      char **bents, char **fents, char 
> *frontend_path);
>  _hidden int libxl__device_del(libxl_ctx *ctx, libxl__device *dev, int wait);
>  _hidden int libxl__device_destroy(libxl_ctx *ctx, char *be_path, int force);
>  _hidden int libxl__devices_destroy(libxl_ctx *ctx, uint32_t domid, int 
> force);
> diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl_pci.c
> --- a/tools/libxl/libxl_pci.c   Mon Oct 04 15:39:30 2010 +0100
> +++ b/tools/libxl/libxl_pci.c   Mon Oct 04 16:45:14 2010 +0100
> @@ -280,7 +280,7 @@ static int libxl_create_pci_backend(libx
> 
>      libxl__device_generic_add(ctx, &device,
>                               libxl__xs_kvs_of_flexarray(gc, back, boffset),
> -                             libxl__xs_kvs_of_flexarray(gc, front, foffset));
> +                             libxl__xs_kvs_of_flexarray(gc, front, foffset), 
> NULL);
> 
>      flexarray_free(back);
>      flexarray_free(front);
> 
> _______________________________________________
> 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