All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roger Pau Monne <roger.pau@citrix.com>
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
Subject: Re: [PATCH v3 2/5] libxl: add libxl__xs_path_cleanup
Date: Wed, 2 May 2012 10:44:40 +0100	[thread overview]
Message-ID: <4FA10208.6060908@citrix.com> (raw)
In-Reply-To: <20373.35017.147406.379559@mariner.uk.xensource.com>

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.

  parent reply	other threads:[~2012-05-02  9:44 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-20 13:23 [PATCH v3 0/5] libxl: call hotplug scripts from libxl Roger Pau Monne
2012-04-20 13:23 ` [PATCH v3 1/5] libxl: allow libxl__exec to take a parameter containing the env variables Roger Pau Monne
2012-04-23 15:37   ` Ian Jackson
2012-04-20 13:23 ` [PATCH v3 2/5] libxl: add libxl__xs_path_cleanup Roger Pau Monne
2012-04-23 15:33   ` Ian Jackson
2012-04-23 15:42     ` Roger Pau Monne
2012-04-23 16:49       ` Ian Jackson
2012-04-23 16:52   ` Ian Jackson
2012-04-25 13:19     ` Roger Pau Monne
2012-05-02  9:44     ` Roger Pau Monne [this message]
2012-04-20 13:23 ` [PATCH v3 3/5] libxl: call hotplug scripts from libxl for vbd Roger Pau Monne
2012-04-23 16:48   ` Ian Jackson
2012-04-24  9:16     ` Ian Campbell
2012-04-24 10:09       ` Ian Jackson
2012-04-24 10:17         ` Ian Campbell
2012-04-24 10:27           ` Ian Jackson
2012-04-24 10:29             ` Ian Campbell
2012-04-24 10:37               ` Ian Jackson
2012-04-25 13:53                 ` Roger Pau Monne
2012-04-25 14:59                   ` Ian Campbell
     [not found]     ` <4F982F18.2080902@citrix.com>
2012-04-26 11:48       ` Fwd: " Roger Pau Monne
2012-04-26 12:06         ` Ian Campbell
2012-04-26 12:12           ` Roger Pau Monne
2012-04-26 13:02         ` Ian Jackson
2012-04-26 13:09           ` Ian Campbell
2012-05-11 16:06             ` Ian Jackson
2012-05-11 16:26               ` Ian Campbell
2012-04-20 13:23 ` [PATCH v3 4/5] libxl: call hotplug scripts from libxl for vif Roger Pau Monne
2012-04-23 16:59   ` Ian Jackson
2012-04-24  9:18     ` Ian Campbell
2012-04-25 11:19   ` Ian Campbell
2012-04-25 11:24     ` Roger Pau Monne
2012-04-20 13:23 ` [PATCH v3 5/5] libxl: add "downscript=no" to Qemu call Roger Pau Monne
2012-04-23 15:38   ` Ian Jackson
2012-04-23 12:12 ` [PATCH v3 0/5] libxl: call hotplug scripts from libxl Marek Marczykowski
2012-04-23 13:31   ` Roger Pau Monne
2012-04-23 13:47     ` Marek Marczykowski
2012-04-23 13:59       ` Non-dom0 block backends (was: Re: [PATCH v3 0/5] libxl: call hotplug scripts from libxl) Joanna Rutkowska
2012-04-23 14:02     ` [PATCH v3 0/5] libxl: call hotplug scripts from libxl Ian Campbell
2012-04-23 16:25       ` Roger Pau Monne
2012-04-24  9:18         ` Ian Campbell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FA10208.6060908@citrix.com \
    --to=roger.pau@citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=xen-devel@lists.xen.org \
    --subject='Re: [PATCH v3 2/5] libxl: add libxl__xs_path_cleanup' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.