[Xen-tools] Re: [PATCH] Make xs_rm and xs_mkdir idempotent

[Christian Limpach <Christian.Limpach@xxxxxxxxxxxx>]

Thanks!

On Sat, Sep 17, 2005 at 10:14:43AM +1000, Rusty Russell wrote:
> # HG changeset patch
> # User Rusty Russell <rusty@xxxxxxxxxxxxxxx>
> # Node ID 90b4e122d3407c7d0e28e6391ca89d46d5a81e52
> # Parent  fd19e760932d095b23d38e67eaec24dd02ba3aba
> Make xs_mkdir an xs_rm idempotent.
> When modifying libxenstore to transparently restart when the daemon dies,
> it became apparent that life is simpler when all commands can simply be
> restarted.  So this patch makes a slight semantic change to xs_rm and 
> xs_mkdir:
> xs_rm now succeeds if the file doesn't exist (as long as the parent exists),
> and xs_mkdir succeeds if the directory already exists.
> Noone should notice.
> 
> Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
> 
> diff -r fd19e760932d -r 90b4e122d340 tools/xenstore/testsuite/02directory.test
> --- a/tools/xenstore/testsuite/02directory.test       Thu Sep 15 19:46:14 2005
> +++ b/tools/xenstore/testsuite/02directory.test       Fri Sep 16 05:01:11 2005
> @@ -27,10 +27,8 @@
>  expect contents2
>  read /dir/test2
>  
> -# Creating dir over the top should fail.
> -expect mkdir failed: File exists
> +# Creating dir over the top should succeed.
>  mkdir /dir
> -expect mkdir failed: File exists
>  mkdir /dir/test2
>  
>  # Mkdir implicitly creates directories.
> diff -r fd19e760932d -r 90b4e122d340 tools/xenstore/testsuite/04rm.test
> --- a/tools/xenstore/testsuite/04rm.test      Thu Sep 15 19:46:14 2005
> +++ b/tools/xenstore/testsuite/04rm.test      Fri Sep 16 05:01:11 2005
> @@ -1,5 +1,4 @@
> -# Remove non-existant fails.
> -expect rm failed: No such file or directory
> +# Remove non-existant is OK, as long as parent exists
>  rm /test
>  expect rm failed: No such file or directory
>  rm /dir/test
> diff -r fd19e760932d -r 90b4e122d340 tools/xenstore/xenstored_core.c
> --- a/tools/xenstore/xenstored_core.c Thu Sep 15 19:46:14 2005
> +++ b/tools/xenstore/xenstored_core.c Fri Sep 16 05:01:11 2005
> @@ -961,6 +961,13 @@
>       return dir;
>  }
>  
> +static bool node_exists(struct connection *conn, const char *node)
> +{
> +     struct stat st;
> +
> +     return lstat(node_dir(conn->transaction, node), &st) == 0;
> +}
> +
>  /* path, flags, data... */
>  static void do_write(struct connection *conn, struct buffered_data *in)
>  {
> @@ -1050,7 +1057,6 @@
>  static void do_mkdir(struct connection *conn, const char *node)
>  {
>       char *dir;
> -     struct stat st;
>  
>       node = canonicalize(conn, node);
>       if (!check_node_perms(conn, node, XS_PERM_WRITE|XS_PERM_ENOENT_OK)) {
> @@ -1066,9 +1072,9 @@
>       if (transaction_block(conn, node))
>               return;
>  
> -     /* Must not already exist. */
> -     if (lstat(node_dir(conn->transaction, node), &st) == 0) {
> -             send_error(conn, EEXIST);
> +     /* If it already exists, fine. */
> +     if (node_exists(conn, node)) {
> +             send_ack(conn, XS_MKDIR);
>               return;
>       }
>  
> @@ -1089,6 +1095,15 @@
>  
>       node = canonicalize(conn, node);
>       if (!check_node_perms(conn, node, XS_PERM_WRITE)) {
> +             /* Didn't exist already?  Fine, if parent exists. */
> +             if (errno == ENOENT) {
> +                     if (node_exists(conn, get_parent(node))) {
> +                             send_ack(conn, XS_RM);
> +                             return;
> +                     }
> +                     /* Restore errno, just in case. */
> +                     errno = ENOENT;
> +             }
>               send_error(conn, errno);
>               return;
>       }
> diff -r fd19e760932d -r 90b4e122d340 tools/xenstore/xs.c
> --- a/tools/xenstore/xs.c     Thu Sep 15 19:46:14 2005
> +++ b/tools/xenstore/xs.c     Fri Sep 16 05:01:11 2005
> @@ -357,7 +357,7 @@
>  }
>  
>  /* Create a new directory.
> - * Returns false on failure.
> + * Returns false on failure, or success if it already exists.
>   */
>  bool xs_mkdir(struct xs_handle *h, const char *path)
>  {
> @@ -365,7 +365,7 @@
>  }
>  
>  /* Destroy a file or directory (directories must be empty).
> - * Returns false on failure.
> + * Returns false on failure, or success if it doesn't exist.
>   */
>  bool xs_rm(struct xs_handle *h, const char *path)
>  {
> diff -r fd19e760932d -r 90b4e122d340 tools/xenstore/xs.h
> --- a/tools/xenstore/xs.h     Thu Sep 15 19:46:14 2005
> +++ b/tools/xenstore/xs.h     Fri Sep 16 05:01:11 2005
> @@ -59,12 +59,12 @@
>             unsigned int len, int createflags);
>  
>  /* Create a new directory.
> - * Returns false on failure.
> + * Returns false on failure, or success if it already exists.
>   */
>  bool xs_mkdir(struct xs_handle *h, const char *path);
>  
>  /* Destroy a file or directory (and children).
> - * Returns false on failure.
> + * Returns false on failure, or success if it doesn't exist.
>   */
>  bool xs_rm(struct xs_handle *h, const char *path);
>  
> diff -r fd19e760932d -r 90b4e122d340 tools/xenstore/xs_random.c
> --- a/tools/xenstore/xs_random.c      Thu Sep 15 19:46:14 2005
> +++ b/tools/xenstore/xs_random.c      Fri Sep 16 05:01:11 2005
> @@ -385,7 +385,7 @@
>  
>       make_dirs(parent_filename(dirname));
>       if (mkdir(dirname, 0700) != 0)
> -             return false;
> +             return (errno == EEXIST);
>  
>       init_perms(dirname);
>       return true;
> @@ -401,8 +401,11 @@
>               return false;
>       }
>  
> -     if (lstat(filename, &st) != 0)
> -             return false;
> +     if (lstat(filename, &st) != 0) {
> +             if (lstat(parent_filename(filename), &st) != 0)
> +                     return false;
> +             return true;
> +     }
>  
>       if (!write_ok(info, path))
>               return false;
> 
> -- 
> A bad analogy is like a leaky screwdriver -- Richard Braakman
> 
> 

_______________________________________________
Xen-tools mailing list
Xen-tools@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-tools