Re: [Xen-devel] [PATCH v2] fix rename: xenstore not fully updated

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-11-18 at 14:49 +0000, Ian Jackson wrote:
> Chunyan Liu writes ("[PATCH v2] fix rename: xenstore not fully updated"):
> > Currently libxl__domain_rename only update /local/domain/<domid>/name,
> > still some places in xenstore are not updated, including:
> > /vm/<uuid>/name and /local/domain/0/backend/<device>/<domid>/.../domain.
> 
> Thanks.  The principle is correct now and so is the broad approach.
> 
> > This patch updates /vm/<uuid>/name in xenstore, and removes the unusual
> > 'domain' field under backend directory (the affected are backend/console,
> > backend/vfb, backend/vkb).
> 
> I think this should be a separate patch.
> 
> > Signed-off-by: Chunyan Liu <cyliu@xxxxxxxx>
> > ---
> > Changes:
> >   * remove unusual 'domain' field from backend dir
> >   * get uuid from hypervisor rather then from xenstore
> > 
> >  tools/libxl/libxl.c | 23 ++++++++++++++++++-----
> >  1 file changed, 18 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index f7961f6..197433c 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -359,6 +359,9 @@ int libxl__domain_rename(libxl__gc *gc, uint32_t domid,
> > +    /* update /vm/<uuid>/name */
> > +    rc = libxl_domain_info(ctx, &info, domid);
> > +    if (rc) {
> > +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> > +                  "fail to get domain info for domain %d", domid);
> > +        goto x_fail;
> > +    }
> 
> I don't think you need to log here since libxl_domain_info already
> does so.  Likewise libxl__xs_write_checked.  But before deleting this,
> please let's wait and see whether Wei and Ian C agree.

I'm all for removing redundant logging.

Ian

> 
> If you do want to add logging here, it's probably better to use use
> LOG rather than LIBXL__LOG.
> 
> > +    uuid = GCSPRINTF(LIBXL_UUID_FMT, LIBXL_UUID_BYTES(info.uuid));
> > +    vm_name_path = GCSPRINTF("/vm/%s/name", uuid);
> > +    if (libxl__xs_write_checked(gc, trans, vm_name_path, new_name)) {
> > +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "failed to write new name `%s'"
> > +                   " to %s", new_name, vm_name_path);
> > +        goto x_fail;
> 
> I don't think it is necessary to LIBXL__LOG here, since
> libxl__xs_write_checked does so.  (See the doc comment in
> libxl_internal.h.)
> 
> Thanks,
> Ian.



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