[PATCH RFC] xen/sysfs: Use XENVER_guest_handle to query UUID

[PATCH RFC] xen/sysfs: Use XENVER_guest_handle to query UUID

[Daniel De Graaf]

The /sys/hypervisor/uuid path relies on Xenstore to query the domain's
UUID (handle) even when the hypervisor exposes an interface to more
directly and efficiently query this. The xenstore path /vm/UUID which is
used for the current query is being discussed for possible removal as
most of the information under this path is only useful for the
toolstack, not the VM.

The UUID fetched from xenstore may also not be properly formatted as a
UUID for the domain if the UUID has been reused (this is most often seen
in domain 0, which if xenstored does not clean up /vm properly, can end
up with a UUID field like "00000000-0000-0000-0000-000000000000-5").

----8<-----------------------------------------------------

This hypercall has been present since Xen 3.1, and is the preferred
method for a domain to obtain its UUID.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 drivers/xen/sys-hypervisor.c    | 20 +++++---------------
 include/xen/interface/version.h |  3 +++
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/xen/sys-hypervisor.c b/drivers/xen/sys-hypervisor.c
index 4c7db7d..416fa01 100644
--- a/drivers/xen/sys-hypervisor.c
+++ b/drivers/xen/sys-hypervisor.c
@@ -116,22 +116,12 @@ static void xen_sysfs_version_destroy(void)
 
 static ssize_t uuid_show(struct hyp_sysfs_attr *attr, char *buffer)
 {
-	char *vm, *val;
+	xen_domain_handle_t uuid;
 	int ret;
-	extern int xenstored_ready;
-
-	if (!xenstored_ready)
-		return -EBUSY;
-
-	vm = xenbus_read(XBT_NIL, "vm", "", NULL);
-	if (IS_ERR(vm))
-		return PTR_ERR(vm);
-	val = xenbus_read(XBT_NIL, vm, "uuid", NULL);
-	kfree(vm);
-	if (IS_ERR(val))
-		return PTR_ERR(val);
-	ret = sprintf(buffer, "%s\n", val);
-	kfree(val);
+	ret = HYPERVISOR_xen_version(XENVER_guest_handle, uuid);
+	if (ret)
+		return ret;
+	ret = sprintf(buffer, "%pU\n", uuid);
 	return ret;
 }
 
diff --git a/include/xen/interface/version.h b/include/xen/interface/version.h
index e8b6519..dd58cf5 100644
--- a/include/xen/interface/version.h
+++ b/include/xen/interface/version.h
@@ -60,4 +60,7 @@ struct xen_feature_info {
 /* arg == NULL; returns host memory page size. */
 #define XENVER_pagesize 7
 
+/* arg == xen_domain_handle_t. */
+#define XENVER_guest_handle 8
+
 #endif /* __XEN_PUBLIC_VERSION_H__ */
-- 
1.7.11.2

Re: [PATCH RFC] xen/sysfs: Use XENVER_guest_handle to query UUID

[Matt Wilson]

On Thu, Aug 16, 2012 at 11:25:15AM -0700, Daniel De Graaf wrote:
> The /sys/hypervisor/uuid path relies on Xenstore to query the domain's
> UUID (handle) even when the hypervisor exposes an interface to more
> directly and efficiently query this. The xenstore path /vm/UUID which is
> used for the current query is being discussed for possible removal as
> most of the information under this path is only useful for the
> toolstack, not the VM.
> 
> The UUID fetched from xenstore may also not be properly formatted as a
> UUID for the domain if the UUID has been reused (this is most often seen
> in domain 0, which if xenstored does not clean up /vm properly, can end
> up with a UUID field like "00000000-0000-0000-0000-000000000000-5").
> 
> ----8<-----------------------------------------------------
> 
> This hypercall has been present since Xen 3.1, and is the preferred
> method for a domain to obtain its UUID.

Hi Daniel,

What do you think about retaining a fallback of looking in xenstore if
the hypercall fails?

Matt

> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
>  drivers/xen/sys-hypervisor.c    | 20 +++++---------------
>  include/xen/interface/version.h |  3 +++
>  2 files changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/xen/sys-hypervisor.c b/drivers/xen/sys-hypervisor.c
> index 4c7db7d..416fa01 100644
> --- a/drivers/xen/sys-hypervisor.c
> +++ b/drivers/xen/sys-hypervisor.c
> @@ -116,22 +116,12 @@ static void xen_sysfs_version_destroy(void)
>  
>  static ssize_t uuid_show(struct hyp_sysfs_attr *attr, char *buffer)
>  {
> -	char *vm, *val;
> +	xen_domain_handle_t uuid;
>  	int ret;
> -	extern int xenstored_ready;
> -
> -	if (!xenstored_ready)
> -		return -EBUSY;
> -
> -	vm = xenbus_read(XBT_NIL, "vm", "", NULL);
> -	if (IS_ERR(vm))
> -		return PTR_ERR(vm);
> -	val = xenbus_read(XBT_NIL, vm, "uuid", NULL);
> -	kfree(vm);
> -	if (IS_ERR(val))
> -		return PTR_ERR(val);
> -	ret = sprintf(buffer, "%s\n", val);
> -	kfree(val);
> +	ret = HYPERVISOR_xen_version(XENVER_guest_handle, uuid);
> +	if (ret)
> +		return ret;
> +	ret = sprintf(buffer, "%pU\n", uuid);
>  	return ret;
>  }
>  
> diff --git a/include/xen/interface/version.h b/include/xen/interface/version.h
> index e8b6519..dd58cf5 100644
> --- a/include/xen/interface/version.h
> +++ b/include/xen/interface/version.h
> @@ -60,4 +60,7 @@ struct xen_feature_info {
>  /* arg == NULL; returns host memory page size. */
>  #define XENVER_pagesize 7
>  
> +/* arg == xen_domain_handle_t. */
> +#define XENVER_guest_handle 8
> +
>  #endif /* __XEN_PUBLIC_VERSION_H__ */

[PATCH v2] xen/sysfs: Use XENVER_guest_handle to query UUID

[Daniel De Graaf]

On 08/16/2012 04:22 PM, Matt Wilson wrote:
> 
> Hi Daniel,
> 
> What do you think about retaining a fallback of looking in xenstore if
> the hypercall fails?
> 
> Matt
> 

That sounds good; there's little cost to leaving the fallback in.

----8<-----------------------------------------------------

This hypercall has been present since Xen 3.1, and is the preferred
method for a domain to obtain its UUID. Fall back to the xenstore method
if using an older version of Xen (which returns -ENOSYS).

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 drivers/xen/sys-hypervisor.c    | 13 ++++++++++++-
 include/xen/interface/version.h |  3 +++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/sys-hypervisor.c b/drivers/xen/sys-hypervisor.c
index 4c7db7d..284df8a 100644
--- a/drivers/xen/sys-hypervisor.c
+++ b/drivers/xen/sys-hypervisor.c
@@ -114,7 +114,7 @@ static void xen_sysfs_version_destroy(void)
 
 /* UUID */
 
-static ssize_t uuid_show(struct hyp_sysfs_attr *attr, char *buffer)
+static ssize_t uuid_show_fallback(struct hyp_sysfs_attr *attr, char *buffer)
 {
 	char *vm, *val;
 	int ret;
@@ -135,6 +135,17 @@ static ssize_t uuid_show(struct hyp_sysfs_attr *attr, char *buffer)
 	return ret;
 }
 
+static ssize_t uuid_show(struct hyp_sysfs_attr *attr, char *buffer)
+{
+	xen_domain_handle_t uuid;
+	int ret;
+	ret = HYPERVISOR_xen_version(XENVER_guest_handle, uuid);
+	if (ret)
+		return uuid_show_fallback(attr, buffer);
+	ret = sprintf(buffer, "%pU\n", uuid);
+	return ret;
+}
+
 HYPERVISOR_ATTR_RO(uuid);
 
 static int __init xen_sysfs_uuid_init(void)
diff --git a/include/xen/interface/version.h b/include/xen/interface/version.h
index e8b6519..dd58cf5 100644
--- a/include/xen/interface/version.h
+++ b/include/xen/interface/version.h
@@ -60,4 +60,7 @@ struct xen_feature_info {
 /* arg == NULL; returns host memory page size. */
 #define XENVER_pagesize 7
 
+/* arg == xen_domain_handle_t. */
+#define XENVER_guest_handle 8
+
 #endif /* __XEN_PUBLIC_VERSION_H__ */
-- 
1.7.11.2

Re: [PATCH v2] xen/sysfs: Use XENVER_guest_handle to query UUID

[Ian Campbell]

On Thu, 2012-08-16 at 21:40 +0100, Daniel De Graaf wrote:
> On 08/16/2012 04:22 PM, Matt Wilson wrote:
> > 
> > Hi Daniel,
> > 
> > What do you think about retaining a fallback of looking in xenstore if
> > the hypercall fails?
> > 
> > Matt
> > 
> 
> That sounds good; there's little cost to leaving the fallback in.
> 
> ----8<-----------------------------------------------------
> 
> This hypercall has been present since Xen 3.1,

Do a pvops kernels run on a hypervisor of that vintage? I have a vague
recollection of the pvops kernels requiring 3.4+ or something. Maybe
that was only for dom0 though.

Ian.

Re: [PATCH v2] xen/sysfs: Use XENVER_guest_handle to query UUID

[Pasi Kärkkäinen]

On Fri, Aug 17, 2012 at 08:51:20AM +0100, Ian Campbell wrote:
> On Thu, 2012-08-16 at 21:40 +0100, Daniel De Graaf wrote:
> > On 08/16/2012 04:22 PM, Matt Wilson wrote:
> > > 
> > > Hi Daniel,
> > > 
> > > What do you think about retaining a fallback of looking in xenstore if
> > > the hypercall fails?
> > > 
> > > Matt
> > > 
> > 
> > That sounds good; there's little cost to leaving the fallback in.
> > 
> > ----8<-----------------------------------------------------
> > 
> > This hypercall has been present since Xen 3.1,
> 
> Do a pvops kernels run on a hypervisor of that vintage? I have a vague
> recollection of the pvops kernels requiring 3.4+ or something. Maybe
> that was only for dom0 though.
> 

The Xen requirement is only for pvops dom0.

pvops kernels, for example the Linux 3.3.x and 3.5.x in Fedora 17, run OK on RHEL5 Xen, which is Xen 3.1.2 based.

-- Pasi

Re: [PATCH v2] xen/sysfs: Use XENVER_guest_handle to query UUID

[Pasi Kärkkäinen]

On Fri, Aug 17, 2012 at 12:36:37PM +0300, Pasi Kärkkäinen wrote:
> On Fri, Aug 17, 2012 at 08:51:20AM +0100, Ian Campbell wrote:
> > On Thu, 2012-08-16 at 21:40 +0100, Daniel De Graaf wrote:
> > > On 08/16/2012 04:22 PM, Matt Wilson wrote:
> > > > 
> > > > Hi Daniel,
> > > > 
> > > > What do you think about retaining a fallback of looking in xenstore if
> > > > the hypercall fails?
> > > > 
> > > > Matt
> > > > 
> > > 
> > > That sounds good; there's little cost to leaving the fallback in.
> > > 
> > > ----8<-----------------------------------------------------
> > > 
> > > This hypercall has been present since Xen 3.1,
> > 
> > Do a pvops kernels run on a hypervisor of that vintage? I have a vague
> > recollection of the pvops kernels requiring 3.4+ or something. Maybe
> > that was only for dom0 though.
> > 
> 
> The Xen requirement is only for pvops dom0.
> 
> pvops kernels, for example the Linux 3.3.x and 3.5.x in Fedora 17, run OK as domU on RHEL5 Xen, which is Xen 3.1.2 based.

I forget to write "as domU" earlier.. added.

-- Pasi

Re: [PATCH v2] xen/sysfs: Use XENVER_guest_handle to query UUID

[Konrad Rzeszutek Wilk]

On Thu, Aug 16, 2012 at 04:40:26PM -0400, Daniel De Graaf wrote:
> On 08/16/2012 04:22 PM, Matt Wilson wrote:
> > 
> > Hi Daniel,
> > 
> > What do you think about retaining a fallback of looking in xenstore if
> > the hypercall fails?
> > 
> > Matt
> > 
> 
> That sounds good; there's little cost to leaving the fallback in.

applied
> 
> ----8<-----------------------------------------------------
> 
> This hypercall has been present since Xen 3.1, and is the preferred
> method for a domain to obtain its UUID. Fall back to the xenstore method
> if using an older version of Xen (which returns -ENOSYS).
> 
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
>  drivers/xen/sys-hypervisor.c    | 13 ++++++++++++-
>  include/xen/interface/version.h |  3 +++
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/xen/sys-hypervisor.c b/drivers/xen/sys-hypervisor.c
> index 4c7db7d..284df8a 100644
> --- a/drivers/xen/sys-hypervisor.c
> +++ b/drivers/xen/sys-hypervisor.c
> @@ -114,7 +114,7 @@ static void xen_sysfs_version_destroy(void)
>  
>  /* UUID */
>  
> -static ssize_t uuid_show(struct hyp_sysfs_attr *attr, char *buffer)
> +static ssize_t uuid_show_fallback(struct hyp_sysfs_attr *attr, char *buffer)
>  {
>  	char *vm, *val;
>  	int ret;
> @@ -135,6 +135,17 @@ static ssize_t uuid_show(struct hyp_sysfs_attr *attr, char *buffer)
>  	return ret;
>  }
>  
> +static ssize_t uuid_show(struct hyp_sysfs_attr *attr, char *buffer)
> +{
> +	xen_domain_handle_t uuid;
> +	int ret;
> +	ret = HYPERVISOR_xen_version(XENVER_guest_handle, uuid);
> +	if (ret)
> +		return uuid_show_fallback(attr, buffer);
> +	ret = sprintf(buffer, "%pU\n", uuid);
> +	return ret;
> +}
> +
>  HYPERVISOR_ATTR_RO(uuid);
>  
>  static int __init xen_sysfs_uuid_init(void)
> diff --git a/include/xen/interface/version.h b/include/xen/interface/version.h
> index e8b6519..dd58cf5 100644
> --- a/include/xen/interface/version.h
> +++ b/include/xen/interface/version.h
> @@ -60,4 +60,7 @@ struct xen_feature_info {
>  /* arg == NULL; returns host memory page size. */
>  #define XENVER_pagesize 7
>  
> +/* arg == xen_domain_handle_t. */
> +#define XENVER_guest_handle 8
> +
>  #endif /* __XEN_PUBLIC_VERSION_H__ */
> -- 
> 1.7.11.2