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.
>
> > +/*
> > + * 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.
>
> > 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.
>
> 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.
This cannot be done inside of a transaction, because we cannot check 
that the directory is empty if the remove has not actually taken place, 
and checking that there are zero or one elements (the one we 'had' 
removed) can lead to unexpected results, as someone might be deleting 
elements on our back and we might actually delete a directory that still 
has valid entries.
> > +        *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