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

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

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@xxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
> Â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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel