Re: [RFC PATCH 2/6] libxl: stop using libxl__xs_mkdir() for ~/control/shutdown

[Ian Campbell <ian.campbell@citrix.com>]

On Wed, 2015-11-11 at 16:18 +0000, Paul Durrant wrote:
> As documented in docs/misc/xenstore, the XS_MKDIR operation merely makes
> sure a path exists whereas ~/control/shutdown needs to start empty. Also
> using XS_MKDIR for a node which is never supposed to have children is
> somewhat counterintuitive.
> 
> This patch introduces a new libxl__xs_printf_perms() function analogous
> to libxl__xs_printf() but taking explicit permissions arguments, and then
> uses this to create an empty ~/control/shutdown node.
> 
> 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>
> ---
>  tools/libxl/libxl_create.c   | 16 ++++++++++------
>  tools/libxl/libxl_internal.h | 10 ++++++++++
>  tools/libxl/libxl_xshelp.c   | 44
> ++++++++++++++++++++++++++++++++++++++------
>  3 files changed, 58 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 921d155..279deda 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -484,7 +484,7 @@ int libxl__domain_make(libxl__gc *gc,
> libxl_domain_config *d_config,
>      libxl_ctx *ctx = libxl__gc_owner(gc);
>      int flags, ret, rc, nb_vm;
>      char *uuid_string;
> -    char *dom_path, *vm_path, *libxl_path;
> +    char *dom_path, *vm_path, *libxl_path, *control_path;
>      struct xs_permissions roperm[2];
>      struct xs_permissions rwperm[1];
>      struct xs_permissions noperm[1];
> @@ -605,17 +605,21 @@ retry_transaction:
>      libxl__xs_mkdir(gc, t,
>                      libxl__sprintf(gc, "%s/device", dom_path),
>                      roperm, ARRAY_SIZE(roperm));
> -    libxl__xs_mkdir(gc, t,
> -                    libxl__sprintf(gc, "%s/control", dom_path),
> +
> +    control_path = libxl__sprintf(gc, "%s/control", dom_path);
> +
> +    libxl__xs_mkdir(gc, t, control_path,
>                      roperm, ARRAY_SIZE(roperm));
>      if (info->type == LIBXL_DOMAIN_TYPE_HVM)
>          libxl__xs_mkdir(gc, t,
>                          libxl__sprintf(gc, "%s/hvmloader", dom_path),
>                          roperm, ARRAY_SIZE(roperm));
>  
> -    libxl__xs_mkdir(gc, t,
> -                    libxl__sprintf(gc, "%s/control/shutdown", dom_path),
> -                    rwperm, ARRAY_SIZE(rwperm));
> +    libxl__xs_printf_perms(gc, t,
> +                           libxl__sprintf(gc, "%s/shutdown",
> control_path),
> +                           rwperm, ARRAY_SIZE(rwperm),
> +                           "");
> +
>      libxl__xs_mkdir(gc, t,
>                      libxl__sprintf(gc, "%s/device/suspend/event-
> channel", dom_path),
>                      rwperm, ARRAY_SIZE(rwperm));
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 4710804..04d8a29 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -666,6 +666,16 @@ _hidden int libxl__xs_writev_perms(libxl__gc *gc,
> xs_transaction_t t,
>  _hidden int libxl__xs_writev_atonce(libxl__gc *gc,
>                               const char *dir, char **kvs);
>  
> +_hidden int libxl__xs_vprintf_perms(libxl__gc *gc, xs_transaction_t t,
> +                                    const char *path,
> +                                    struct xs_permissions *perms,
> +                                    unsigned int num_perms,
> +                                    const char *fmt, va_list ap);
> +_hidden int libxl__xs_printf_perms(libxl__gc *gc, xs_transaction_t t,
> +                                   const char *path,
> +                                   struct xs_permissions *perms,
> +                                   unsigned int num_perms,
> +                                   const char *fmt, ...)
> PRINTF_ATTRIBUTE(6, 7);
>  _hidden int libxl__xs_printf(libxl__gc *gc, xs_transaction_t t,
>                               const char *path, const char *fmt, ...)
> PRINTF_ATTRIBUTE(4, 5);
>     /* Each fn returns 0 on success.
> diff --git a/tools/libxl/libxl_xshelp.c b/tools/libxl/libxl_xshelp.c
> index 3cac4f2..0b56f8b 100644
> --- a/tools/libxl/libxl_xshelp.c
> +++ b/tools/libxl/libxl_xshelp.c
> @@ -96,25 +96,57 @@ out:
>  
>  }
>  
> -int libxl__xs_printf(libxl__gc *gc, xs_transaction_t t,
> -                     const char *path, const char *fmt, ...)
> +int libxl__xs_vprintf_perms(libxl__gc *gc, xs_transaction_t t,
> +                            const char *path,
> +                            struct xs_permissions *perms,
> +                            unsigned int num_perms,
> +                            const char *fmt,
> +                            va_list ap)
>  {
>      libxl_ctx *ctx = libxl__gc_owner(gc);
>      char *s;
> -    va_list ap;
>      int ret;
> -    va_start(ap, fmt);
> -    ret = vasprintf(&s, fmt, ap);
> -    va_end(ap);
>  
> +    ret = vasprintf(&s, fmt, ap);
>      if (ret == -1) {
>          return -1;
>      }
>      xs_write(ctx->xsh, t, path, s, ret);
> +    if (perms)
> +        xs_set_permissions(ctx->xsh, t, path, perms, num_perms);

This can fail, can't it? (OTOH so can xs_write, so maybe there is some
reason we apparently don't care for such things here?)

>      free(s);
>      return 0;
>  }
>  
> +int libxl__xs_printf_perms(libxl__gc *gc, xs_transaction_t t,
> +                           const char *path,
> +                           struct xs_permissions *perms,
> +                           unsigned int num_perms,
> +                           const char *fmt, ...)
> +{
> +    va_list ap;
> +    int ret;
> +
> +    va_start(ap, fmt);
> +    ret = libxl__xs_vprintf_perms(gc, t, path, perms, num_perms, fmt,
> ap);
> +    va_end(ap);
> +
> +    return ret;
> +}
> +
> +int libxl__xs_printf(libxl__gc *gc, xs_transaction_t t,
> +                     const char *path, const char *fmt, ...)
> +{
> +    va_list ap;
> +    int ret;
> +
> +    va_start(ap, fmt);
> +    ret = libxl__xs_vprintf_perms(gc, t, path, NULL, 0, fmt, ap);
> +    va_end(ap);
> +
> +    return ret;
> +}
> +
>  char * libxl__xs_read(libxl__gc *gc, xs_transaction_t t, const char
> *path)
>  {
>      libxl_ctx *ctx = libxl__gc_owner(gc);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel