> -----Original Message-----
> From: Ian Campbell [mailto:ian.campb...@citrix.com]
> Sent: 16 November 2015 12:55
> To: Paul Durrant; xen-de...@lists.xenproject.org
> Cc: Ian Jackson; Stefano Stabellini; Wei Liu
> Subject: Re: [RFC PATCH 2/6] libxl: stop using libxl__xs_mkdir() for
> ~/control/shutdown
>
> 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.durr...@citrix.com>
> > Cc: Ian Jackson <ian.jack...@eu.citrix.com>
> > Cc: Stefano Stabellini <stefano.stabell...@eu.citrix.com>
> > Cc: Ian Campbell <ian.campb...@citrix.com>
> > Cc: Wei Liu <wei.l...@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?)
>
That's a good point. I would have thought wiring an xs_write() failure back
through to the caller would be a good idea (and same goes for
xs_set_permissions, I guess).
Paul
> > 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
