[PATCH v2 0/2] libxl: change libxl__xs_mkdir() to libxl__xs_mknod()

[PATCH v2 0/2] libxl: change libxl__xs_mkdir() to libxl__xs_mknod()

[Paul Durrant]

Patch #1 is purely a search and replace
Patch #2 changes the underlying implementation

[PATCH v2 1/2] libxl: replace libxl__xs_mkdir() with libxl__xs_mknod()

[Paul Durrant]

This patch is purely cosmetic, it contains no functional change. A
change in the implementation of libxl__xs_mknod() will be made in a
subsequent patch.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_create.c   | 26 +++++++++++++-------------
 tools/libxl/libxl_internal.h |  2 +-
 tools/libxl/libxl_xshelp.c   |  2 +-
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 8770486..673e537 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -583,43 +583,43 @@ retry_transaction:
     t = xs_transaction_start(ctx->xsh);
 
     xs_rm(ctx->xsh, t, dom_path);
-    libxl__xs_mkdir(gc, t, dom_path, roperm, ARRAY_SIZE(roperm));
+    libxl__xs_mknod(gc, t, dom_path, roperm, ARRAY_SIZE(roperm));
 
     xs_rm(ctx->xsh, t, vm_path);
-    libxl__xs_mkdir(gc, t, vm_path, roperm, ARRAY_SIZE(roperm));
+    libxl__xs_mknod(gc, t, vm_path, roperm, ARRAY_SIZE(roperm));
 
     xs_rm(ctx->xsh, t, libxl_path);
-    libxl__xs_mkdir(gc, t, libxl_path, noperm, ARRAY_SIZE(noperm));
+    libxl__xs_mknod(gc, t, libxl_path, noperm, ARRAY_SIZE(noperm));
 
     xs_write(ctx->xsh, t, GCSPRINTF("%s/vm", dom_path), vm_path, strlen(vm_path));
     rc = libxl__domain_rename(gc, *domid, 0, info->name, t);
     if (rc)
         goto out;
 
-    libxl__xs_mkdir(gc, t,
+    libxl__xs_mknod(gc, t,
                     GCSPRINTF("%s/cpu", dom_path),
                     roperm, ARRAY_SIZE(roperm));
-    libxl__xs_mkdir(gc, t,
+    libxl__xs_mknod(gc, t,
                     GCSPRINTF("%s/memory", dom_path),
                     roperm, ARRAY_SIZE(roperm));
-    libxl__xs_mkdir(gc, t,
+    libxl__xs_mknod(gc, t,
                     GCSPRINTF("%s/device", dom_path),
                     roperm, ARRAY_SIZE(roperm));
-    libxl__xs_mkdir(gc, t,
+    libxl__xs_mknod(gc, t,
                     GCSPRINTF("%s/control", dom_path),
                     roperm, ARRAY_SIZE(roperm));
     if (info->type == LIBXL_DOMAIN_TYPE_HVM)
-        libxl__xs_mkdir(gc, t,
+        libxl__xs_mknod(gc, t,
                         GCSPRINTF("%s/hvmloader", dom_path),
                         roperm, ARRAY_SIZE(roperm));
 
-    libxl__xs_mkdir(gc, t,
+    libxl__xs_mknod(gc, t,
                     GCSPRINTF("%s/control/shutdown", dom_path),
                     rwperm, ARRAY_SIZE(rwperm));
-    libxl__xs_mkdir(gc, t,
+    libxl__xs_mknod(gc, t,
                     GCSPRINTF("%s/device/suspend/event-channel", dom_path),
                     rwperm, ARRAY_SIZE(rwperm));
-    libxl__xs_mkdir(gc, t,
+    libxl__xs_mknod(gc, t,
                     GCSPRINTF("%s/data", dom_path),
                     rwperm, ARRAY_SIZE(rwperm));
 
@@ -628,13 +628,13 @@ retry_transaction:
          * Create a local "libxl" directory for each guest, since we might want
          * to use libxl from inside the guest
          */
-        libxl__xs_mkdir(gc, t, GCSPRINTF("%s/libxl", dom_path), rwperm,
+        libxl__xs_mknod(gc, t, GCSPRINTF("%s/libxl", dom_path), rwperm,
                         ARRAY_SIZE(rwperm));
         /*
          * Create a local "device-model" directory for each guest, since we
          * might want to use Qemu from inside the guest
          */
-        libxl__xs_mkdir(gc, t, GCSPRINTF("%s/device-model", dom_path), rwperm,
+        libxl__xs_mknod(gc, t, GCSPRINTF("%s/device-model", dom_path), rwperm,
                         ARRAY_SIZE(rwperm));
     }
 
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 58d07cd..a671a61 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -680,7 +680,7 @@ _hidden char *libxl__xs_read(libxl__gc *gc, xs_transaction_t t,
 _hidden char **libxl__xs_directory(libxl__gc *gc, xs_transaction_t t,
                                    const char *path, unsigned int *nb);
    /* On error: returns NULL, sets errno (no logging) */
-_hidden bool libxl__xs_mkdir(libxl__gc *gc, xs_transaction_t t,
+_hidden bool libxl__xs_mknod(libxl__gc *gc, xs_transaction_t t,
                              const char *path, struct xs_permissions *perms,
 			     unsigned int num_perms);
 
diff --git a/tools/libxl/libxl_xshelp.c b/tools/libxl/libxl_xshelp.c
index bc60b9a..cb6a559 100644
--- a/tools/libxl/libxl_xshelp.c
+++ b/tools/libxl/libxl_xshelp.c
@@ -147,7 +147,7 @@ char **libxl__xs_directory(libxl__gc *gc, xs_transaction_t t,
     return ret;
 }
 
-bool libxl__xs_mkdir(libxl__gc *gc, xs_transaction_t t,
+bool libxl__xs_mknod(libxl__gc *gc, xs_transaction_t t,
                      const char *path, struct xs_permissions *perms,
 			         unsigned int num_perms)
 {
-- 
2.1.4

[PATCH v2 2/2] libxl: implement libxl__xs_mknod using XS_WRITE rather than XS_MKDIR

[Paul Durrant]

This patch modifies the implentation of libxl__xs_mknod() to use XS_WRITE
rather than XS_MKDIR since passing an empty value to the former will
ensure that the path is both existent and empty upon return, rather than
merely existent. The function return type is also changed to a libxl
error value rather than a boolean, it's declaration is accordingly moved
into the 'checked' section in libxl_internal.h, and a comment is added to
clarify its semantics.

This patch also contains as small whitespace fix in the definition of
libxl__xs_mknod() and the addition of 'ok' to CODING_STYLE as the
canonical variable name for holding return values from boolean functions.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---

v2:
 - Add logging should libxl__xs_mknod() fail
 - Clarify semantics of libxl__xs_mknod() in libxl_internal.h
 - Re-word use ok 'ok' in libxl/CODING_STYLE
---
 tools/libxl/CODING_STYLE     |  1 +
 tools/libxl/libxl_internal.h |  9 +++++----
 tools/libxl/libxl_xshelp.c   | 24 ++++++++++++++++++------
 3 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/tools/libxl/CODING_STYLE b/tools/libxl/CODING_STYLE
index 919bcc6..522d1c9 100644
--- a/tools/libxl/CODING_STYLE
+++ b/tools/libxl/CODING_STYLE
@@ -35,6 +35,7 @@ The following local variable names should be used where applicable:
 
   int rc;    /* a libxl error code - and not anything else */
   int r;     /* the return value from a system call (or libxc call) */
+  bool ok;   /* the success return value from a boolean function */
 
   uint32_t domid;
   libxl__gc *gc;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index a671a61..d2bda0a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -680,10 +680,6 @@ _hidden char *libxl__xs_read(libxl__gc *gc, xs_transaction_t t,
 _hidden char **libxl__xs_directory(libxl__gc *gc, xs_transaction_t t,
                                    const char *path, unsigned int *nb);
    /* On error: returns NULL, sets errno (no logging) */
-_hidden bool libxl__xs_mknod(libxl__gc *gc, xs_transaction_t t,
-                             const char *path, struct xs_permissions *perms,
-			     unsigned int num_perms);
-
 _hidden char *libxl__xs_libxl_path(libxl__gc *gc, uint32_t domid);
 
 
@@ -692,6 +688,11 @@ _hidden char *libxl__xs_libxl_path(libxl__gc *gc, uint32_t domid);
  * fails it logs and returns ERROR_FAIL.
  */
 
+/* On success, path will exist and will have an empty value */
+int libxl__xs_mknod(libxl__gc *gc, xs_transaction_t t,
+                    const char *path, struct xs_permissions *perms,
+                    unsigned int num_perms);
+
 /* On success, *result_out came from the gc.
  * On error, *result_out is undefined.
  * ENOENT counts as success but sets *result_out=0
diff --git a/tools/libxl/libxl_xshelp.c b/tools/libxl/libxl_xshelp.c
index cb6a559..8554ee5 100644
--- a/tools/libxl/libxl_xshelp.c
+++ b/tools/libxl/libxl_xshelp.c
@@ -147,14 +147,26 @@ char **libxl__xs_directory(libxl__gc *gc, xs_transaction_t t,
     return ret;
 }
 
-bool libxl__xs_mknod(libxl__gc *gc, xs_transaction_t t,
-                     const char *path, struct xs_permissions *perms,
-			         unsigned int num_perms)
+int libxl__xs_mknod(libxl__gc *gc, xs_transaction_t t,
+                    const char *path, struct xs_permissions *perms,
+                    unsigned int num_perms)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
-    if (!xs_mkdir(ctx->xsh, t, path))
-        return false;
-    return xs_set_permissions(ctx->xsh, t, path, perms, num_perms);
+    bool ok;
+
+    ok = xs_write(ctx->xsh, t, path, "", 0);
+    if (!ok) {
+        LOGE(ERROR, "xenstore write failed: `%s' = ''", path);
+        return ERROR_FAIL;
+    }
+
+    ok = xs_set_permissions(ctx->xsh, t, path, perms, num_perms);
+    if (!ok) {
+        LOGE(ERROR, "xenstore set permissions failed on `%s'", path);
+        return ERROR_FAIL;
+    }
+
+    return 0;
 }
 
 char *libxl__xs_libxl_path(libxl__gc *gc, uint32_t domid)
-- 
2.1.4

Re: [PATCH v2 2/2] libxl: implement libxl__xs_mknod using XS_WRITE rather than XS_MKDIR

[Ian Jackson]

Paul Durrant writes ("[PATCH v2 2/2] libxl: implement libxl__xs_mknod using XS_WRITE rather than XS_MKDIR"):
> This patch modifies the implentation of libxl__xs_mknod() to use XS_WRITE
> rather than XS_MKDIR since passing an empty value to the former will
> ensure that the path is both existent and empty upon return, rather than
> merely existent. The function return type is also changed to a libxl
> error value rather than a boolean, it's declaration is accordingly moved
> into the 'checked' section in libxl_internal.h, and a comment is added to
> clarify its semantics.
...

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Re: [PATCH v2 0/2] libxl: change libxl__xs_mkdir() to libxl__xs_mknod()

[Ian Campbell]

On Wed, 2015-11-25 at 14:50 +0000, Paul Durrant wrote:
> Patch #1 is purely a search and replace
> Patch #2 changes the underlying implementation

Applied both.

I think these used to be part of "libxl: xenstore related cleanup", did
this series completely replace that or are there still aspects yet to come
and/or be reposted?


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

Re: [PATCH v2 0/2] libxl: change libxl__xs_mkdir() to libxl__xs_mknod()

[Paul Durrant]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: 01 December 2015 12:37
> To: Paul Durrant; xen-devel@lists.xenproject.org
> Subject: Re: [Xen-devel] [PATCH v2 0/2] libxl: change libxl__xs_mkdir() to
> libxl__xs_mknod()
> 
> On Wed, 2015-11-25 at 14:50 +0000, Paul Durrant wrote:
> > Patch #1 is purely a search and replace
> > Patch #2 changes the underlying implementation
> 
> Applied both.
> 

Thanks.

> I think these used to be part of "libxl: xenstore related cleanup", did
> this series completely replace that or are there still aspects yet to come
> and/or be reposted?
> 

I do have a couple more clean up patches to post. I should hopefully  be able to do that shortly.

  Paul

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