[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



On Tue, 2010-10-05 at 14:39 +0100, Stefano Stabellini wrote:
> 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.

Yes, that's a good idea.

> 
> 
> 
> 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®.