Re: [Xen-devel] [PATCH v3 2/5] libxl: add libxl__xs_path_cleanup

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

Ian Jackson escribió:
> Roger Pau Monne writes ("[Xen-devel] [PATCH v3 2/5] libxl: add 
> libxl__xs_path_cleanup"):
> > diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> > index c7e057d..36d58cd 100644
> > --- a/tools/libxl/libxl_device.c
> > +++ b/tools/libxl/libxl_device.c
> > @@ -356,7 +356,6 @@ int libxl__device_disk_dev_number(const char *virtpath, int 
> > *pdisk,
> >       return -1;
> >   }
> >
> > -
> Unrelated whitespace change.
Done.

> > +/*
> > + * Perfrom recursive cleanup of xenstore path, from top to bottom
> > + * just like xenstore-rm -t
> > + */
> > +_hidden int libxl__xs_path_cleanup(libxl__gc *gc, char *path);
> I think, following my confusion, that this needs some better
> documentation comment.
Ok, I've tried to change to something more self-explaining.

> > diff --git a/tools/libxl/libxl_xshelp.c b/tools/libxl/libxl_xshelp.c
> > index 3ea8d08..0b1b844 100644
> > --- a/tools/libxl/libxl_xshelp.c
> > +++ b/tools/libxl/libxl_xshelp.c
> > @@ -135,6 +135,28 @@ char *libxl__xs_libxl_path(libxl__gc *gc, uint32_t domid)
> >       return s;
> >   }
> >
> > +int libxl__xs_path_cleanup(libxl__gc *gc, char *path)
> > +{
> > +    libxl_ctx *ctx = libxl__gc_owner(gc);
> > +    unsigned int nb = 0;
> > +    char *last;
> > +
> > +    if (!path)
> > +        return 0;
> > +
> > +    xs_rm(ctx->xsh, XBT_NULL, path);
> > +
> > +    for (last = strrchr(path, '/'); last != NULL; last = strrchr(path, '/')) {
> If the path is relative, this won't work correctly.
Are you sure? From what I've tested it seems to work ok.

> Also this whole thing needs to take place in a transaction, or it is
> racy.  Probably a transaction supplied by the caller, in which case
> you should assert it.
Ok, will add another lock.

> > +        *last = '\0';
> > +        if (!libxl__xs_directory(gc, XBT_NULL, path,&nb))
> > +            continue;
> If this fails, it should be a fatal error; we should not just blunder
> on.
>
> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel