Re: [Xen-devel] [PATCH 0/2 V3] fix rename: xenstore not fully updated

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Tue, Nov 25, 2014 at 02:13:01PM +0000, Ian Campbell wrote:
> On Fri, 2014-11-21 at 12:01 -0500, Konrad Rzeszutek Wilk wrote:
> > On Thu, Nov 20, 2014 at 03:28:30PM +0000, Ian Campbell wrote:
> > > On Wed, 2014-11-19 at 16:25 -0500, Konrad Rzeszutek Wilk wrote:
> > > > On Wed, Nov 19, 2014 at 11:26:32AM +0000, Ian Jackson wrote:
> > > > > Hi Konrad, I have another release ack request:
> > > > > 
> > > > > Chunyan Liu writes ("[PATCH 0/2 V3] 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.
> > > > > > This patch series updates /vm/<uuid>/name in xenstore,
> > > > > 
> > > > > This ("[PATCH 2/2 V3] fix rename: xenstore not fully updated") is a
> > > > > bugfix which I think should go into Xen 4.5.
> > > > > 
> > > > > The risk WITHOUT this patch is that there are out-of-tree tools which
> > > > > look here for the domain name and will get confused after it is
> > > > > renamed.
> > > > 
> > > > When was this introduced? Did it exist with Xend?
> > > 
> > > Based on:
> > >  git grep domain\" RELEASE-4.4.0  tools/python/
> > >  git grep domain\' RELEASE-4.4.0 tools/python/
> > > it doesn't appear so, but someone with a xend install would be needed to
> > > confirm for sure.
> > > 
> > > Given that this has always been wrong for a libxl domain after migration
> > > it seems likely to me that noone is looking at this field.
> > 
> > And this is not a regression though.
> > 
> > What I am understanding is that we advertise to out-side toolstack
> > users for a couple of releases something which is misleading and wrong.
> 
> ...and also basically useless, there is really no reason to be looking
> for the domain's name under a subset of the backend nodes (only vkb, vfb
> and console have this key at all). None of the helper function which we
> provide do this.
> 
> Also these nodes are not advertised as supported either via
> docs/misc/xenstore-paths.markdown or xen/include/public/io/*.h.
> 
> > My fear is that there such toolstack users out there who will
> > get their pitchforks out when this shows up.
> > 
> > But perhaps there is a way to mitigate this. Is there another way
> > (and can it be in the commit description) to get the proper
> > domain name? I presume it is just looking at /local/domain/%d/name ?
> > 
> > As such could this be added:
> > 
> > "The proper way to get the domain name is to get it from
> > /local/domain/%d/name instead from this field."
> 
> The proper way is to use libxl_domid_to_name, not to go scrobbling
> around in xenstore. We could say this (or both) in the commit message
> though if it would help to reassure you.

That (both) would most certainly reassure me. Thank you!

> 
> Either way I think it really would be best to take this fix rather than
> worrying overly about the infinitesimal possibility that someone might
> be using this key.

Right, so with the reassurance text added on:
Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> 
> Ian.
> 

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