[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] libxl: remove console 0 backend directory from xenstore on devices destroy



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.