[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC PATCH 2/6] libxl: stop using libxl__xs_mkdir() for ~/control/shutdown
> -----Original Message----- > From: Ian Campbell [mailto:ian.campbell@xxxxxxxxxx] > Sent: 16 November 2015 12:55 > To: Paul Durrant; xen-devel@xxxxxxxxxxxxxxxxxxxx > 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.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?) > 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@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |