Re: [Xen-devel] [PATCH 0/2 V3] fix rename: xenstore not fully updated
On Tue, 2014-11-25 at 11:03 -0500, Konrad Rzeszutek Wilk wrote: On Tue, Nov 25, 2014 at 03:58:33PM +, Ian Campbell wrote: On Tue, 2014-11-25 at 10:51 -0500, Konrad Rzeszutek Wilk wrote: Right, so with the reassurance text added on: Release-Acked-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com Thanks. Chunyan, I propose to commit adding the following to the commit text of [PATCH 1/2 V3] remove domain field in xenstore backend dir: The correct way to obtain a domain's name is via libxl_domid_to_name(), or by reading from /local/domain/$DOMID/name for toolstacks not using libxl. Does that sound OK to you? Yes. Thanks, that was really a question for Chunyan, but rather than wait any longer I've applied with that change. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/2 V3] fix rename: xenstore not fully updated
On Fri, 2014-11-21 at 12:01 -0500, Konrad Rzeszutek Wilk wrote: On Thu, Nov 20, 2014 at 03:28:30PM +, 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 +, 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. 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. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/2 V3] fix rename: xenstore not fully updated
On Tue, Nov 25, 2014 at 02:13:01PM +, 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 +, 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 +, 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.w...@oracle.com Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/2 V3] fix rename: xenstore not fully updated
On Tue, 2014-11-25 at 10:51 -0500, Konrad Rzeszutek Wilk wrote: Right, so with the reassurance text added on: Release-Acked-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com Thanks. Chunyan, I propose to commit adding the following to the commit text of [PATCH 1/2 V3] remove domain field in xenstore backend dir: The correct way to obtain a domain's name is via libxl_domid_to_name(), or by reading from /local/domain/$DOMID/name for toolstacks not using libxl. Does that sound OK to you? Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/2 V3] fix rename: xenstore not fully updated
On Tue, Nov 25, 2014 at 03:58:33PM +, Ian Campbell wrote: On Tue, 2014-11-25 at 10:51 -0500, Konrad Rzeszutek Wilk wrote: Right, so with the reassurance text added on: Release-Acked-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com Thanks. Chunyan, I propose to commit adding the following to the commit text of [PATCH 1/2 V3] remove domain field in xenstore backend dir: The correct way to obtain a domain's name is via libxl_domid_to_name(), or by reading from /local/domain/$DOMID/name for toolstacks not using libxl. Does that sound OK to you? Yes. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/2 V3] fix rename: xenstore not fully updated
On Wed, 2014-11-19 at 16:25 -0500, Konrad Rzeszutek Wilk wrote: On Wed, Nov 19, 2014 at 11:26:32AM +, 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. The risk WITH this patch is that the implementation could be wrong somehow, in which case the code would need to be updated again. But it's a very small patch and has been fully reviewed. I checked QEMU and didn't find anything in there. Great. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/2 V3] fix rename: xenstore not fully updated
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. The risk WITH this patch is that the implementation could be wrong somehow, in which case the code would need to be updated again. But it's a very small patch and has been fully reviewed. and removes the unusual 'domain' field under backend directory. This is a reference to [PATCH 1/2 V3] remove domain field in xenstore backend dir. The change to libxl is that it no longer writes /local/domain/0/backend/vfb/3/0/domain = name of frontend domain It seems hardly conceivable that anyone could be using this field. Existing users will not work after the domain is renamed, anyway. The risk on both sides of the decision lies entirely with out-of-tree software which looks here for the domain name for some reason. We don't think any such tools exist. Note that the domain name cannot be used directly by a non-dom0 programs because the mapping between domids and domain names is in a part of xenstore which is not accessible to guests. (It is possible that a guest would read this value merely to display it.) If such out-of-tree software exists: The risk WITHOUT this patch is that it might report, or (worse) operate on, the wrong domain entirely. The risk WITH this patch is that it (or some subset of its functionality) would stop working right away. An alternative would be to update all of these entries on rename. That's a large and somewhat fiddly patch which we don't think is appropriate given that the presence of this key is a mistake. Thanks, ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
