Re: [Xen-devel] [PATCH 14 of 23] libxl: use libxl__device in libxl_devices_destroy and libxl__device_pci_remove_xenstore

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Fri, 2011-10-07 at 13:18 +0100, Roger Pau Monnà wrote:
> 2011/9/30 Ian Campbell <ian.campbell@xxxxxxxxxx>:

> > @@ -445,22 +467,27 @@ int libxl__devices_destroy(libxl__gc *gc
> >         num1 = 0;
> >     }
> >     for (i = 0; i < num1; i++) {
> > -        if (!strcmp("vfs", l1[i]))
> > +        if (libxl__device_kind_from_string(l1[i], &kind))
> > +            continue;
> > +        if (kind == LIBXL__DEVICE_KIND_VBD)
> >             continue;
> 
> This should not be there, or hotplug scripts for VBD device types are
> not called. I know Linux doesn't need to call hotplug scripts for VBD,
> but NetBSD does. Should we add some kind of helper function or ifdef,
> or is it safe to remove this and handle VBD devices normally?

This is a bug -- I removed a check for "vfs" and replaced it with one
for "vbd" which is not the same!

"vfs" was the fs-backend thing which was remove by 22716:3c78729b6f06 so
I think the check can just be removed and I will do so in the next
posting.


Ian
> 
> >         path = libxl__sprintf(gc, "/local/domain/%d/device/%s", domid, 
> > l1[i]);
> >         l2 = libxl__xs_directory(gc, XBT_NULL, path, &num2);
> >         if (!l2)
> >             continue;
> >         for (j = 0; j < num2; j++) {
> > -            fe_path = libxl__sprintf(gc, "/local/domain/%d/device/%s/%s", 
> > domid, l1[i], l2[j]);
> > -            be_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, 
> > "%s/backend", fe_path));
> > -            if (be_path != NULL) {
> > +            path = libxl__sprintf(gc, 
> > "/local/domain/%d/device/%s/%s/backend",
> > +                                  domid, l1[i], l2[j]);
> > +            path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, path));
> > +            if (path && libxl__parse_backend_path(gc, path, &dev) == 0) {
> > +                dev.domid = domid;
> > +                dev.kind = kind;
> > +                dev.devid = atoi(l2[j]);
> > +
> >                 if (force) {
> > -                    xs_rm(ctx->xsh, XBT_NULL, be_path);
> > -                    xs_rm(ctx->xsh, XBT_NULL, fe_path);
> > -                    libxl__device_destroy_tapdisk(gc, be_path);
> > +                    libxl__device_force_remove(gc, &dev);
> >                 } else {
> > -                    if (libxl__device_remove(gc, be_path) > 0)
> > +                    if (libxl__device_remove(gc, &dev) > 0)
> >                         n_watches++;
> >                 }
> >             }
> > @@ -468,14 +495,18 @@ int libxl__devices_destroy(libxl__gc *gc
> >     }
> >
> >     /* 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, "")) {
> > +    path = libxl__sprintf(gc, "/local/domain/%d/console/backend", domid);
> > +    path = libxl__xs_read(gc, XBT_NULL, path);
> > +    if (path && strcmp(path, "") &&
> > +        libxl__parse_backend_path(gc, path, &dev) == 0) {
> > +        dev.domid = domid;
> > +        dev.kind = LIBXL__DEVICE_KIND_CONSOLE;
> > +        dev.devid = 0;
> > +
> >         if (force) {
> > -            xs_rm(ctx->xsh, XBT_NULL, be_path);
> > -            xs_rm(ctx->xsh, XBT_NULL, fe_path);
> > +            libxl__device_force_remove(gc, &dev);
> >         } else {
> > -            if (libxl__device_remove(gc, be_path) > 0)
> > +            if (libxl__device_remove(gc, &dev) > 0)
> >                 n_watches++;
> >         }
> >     }
> > @@ -505,12 +536,9 @@ int libxl__device_del(libxl__gc *gc, lib
> >  {
> >     libxl_ctx *ctx = libxl__gc_owner(gc);
> >     struct timeval tv;
> > -    char *backend_path;
> >     int rc;
> >
> > -    backend_path = libxl__device_backend_path(gc, dev);
> > -
> > -    rc = libxl__device_remove(gc, backend_path);
> > +    rc = libxl__device_remove(gc, dev);
> >     if (rc == -1) {
> >         rc = ERROR_FAIL;
> >         goto out;
> > diff -r e5a70a3b61a1 -r ec28ee6dace5 tools/libxl/libxl_internal.h
> > --- a/tools/libxl/libxl_internal.h      Fri Sep 30 14:27:28 2011 +0100
> > +++ b/tools/libxl/libxl_internal.h      Fri Sep 30 14:27:28 2011 +0100
> > @@ -240,8 +240,10 @@ _hidden int libxl__device_generic_add(li
> >                              char **bents, char **fents);
> >  _hidden char *libxl__device_backend_path(libxl__gc *gc, libxl__device 
> > *device);
> >  _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, char *be_path);
> > +_hidden int libxl__device_remove(libxl__gc *gc, libxl__device *dev);
> >  _hidden int libxl__device_force_remove(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);
> > diff -r e5a70a3b61a1 -r ec28ee6dace5 tools/libxl/libxl_pci.c
> > --- a/tools/libxl/libxl_pci.c   Fri Sep 30 14:27:28 2011 +0100
> > +++ b/tools/libxl/libxl_pci.c   Fri Sep 30 14:27:28 2011 +0100
> > @@ -410,9 +410,15 @@ retry_transaction2:
> >             goto retry_transaction2;
> >
> >     if (num == 1) {
> > -        char *fe_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, 
> > "%s/frontend", be_path));
> > -        xs_rm(ctx->xsh, XBT_NULL, be_path);
> > -        xs_rm(ctx->xsh, XBT_NULL, fe_path);
> > +        libxl__device dev;
> > +        if (libxl__parse_backend_path(gc, be_path, &dev) != 0)
> > +            return ERROR_FAIL;
> > +
> > +        dev.domid = domid;
> > +        dev.kind = LIBXL__DEVICE_KIND_PCI;
> > +        dev.devid = 0;
> > +
> > +        libxl__device_force_remove(gc, &dev);
> >         return 0;
> >     }
> >
> >
> > _______________________________________________
> > 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