Re: [PATCH] /sys/hypervisor/uuid

[Anthony Liguori <aliguori@us.ibm.com>]

Markus Armbruster wrote:
> New /sys/hypervisor/uuid, containing this domain's UUID.
>
> Stripping off /vm/ from the value of vm to get the UUID isn't exactly
> nice.  The alternative is to add a XENVER_get_uuid code to
> HYPERVISOR_xen_version(), but I'm not sure that's worth it.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>   

I really don't think we should be mirroring things in sysfs that are 
available in userspace.  What benefit is there of having two interfaces 
to the same information?

If the argument is that we don't want to rely on libxenstore in a domU, 
then we should be exposing all of XenStore in sysfs.

The uuid parameter is a construction of Xend.  Xend is *not* a part of 
the supported guest interface.  By exposing the uuid in the kernel 
interface, you're tying an unsupported interface to what really should 
be a supported interface.

Plus, there's already a patch floating on LKML for a /sys/hypervisor.  
We shouldn't start adding attributes to this namespace as it could 
potentially conflict with the existing patch.

In the very least, we ought to stick this stuff in /sys/hypervisor/xen.

Regards,

Anthony Liguori

> diff -r ddba92a5cba9 drivers/xen/core/xen_sysfs.c
> --- a/drivers/xen/core/xen_sysfs.c	Tue May 09 12:41:38 2006 +0200
> +++ b/drivers/xen/core/xen_sysfs.c	Thu May 18 18:06:35 2006 +0200
> @@ -8,12 +8,14 @@
>   */
>  
>  #include <linux/config.h>
> +#include <linux/err.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <asm/hypervisor.h>
>  #include <xen/features.h>
>  #include <xen/hypervisor_sysfs.h>
> +#include <xen/xenbus.h>
>  
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Mike D. Day <ncmike@us.ibm.com>");
> @@ -92,6 +94,37 @@ static void xen_sysfs_version_destroy(vo
>  static void xen_sysfs_version_destroy(void)
>  {
>  	sysfs_remove_group(&hypervisor_subsys.kset.kobj, &version_group);
> +}
> +
> +/* UUID */
> +
> +static ssize_t uuid_show(struct hyp_sysfs_attr *attr, char *buffer)
> +{
> +	char *val;
> +	int ret;
> +
> +	val = xenbus_read(XBT_NULL, "vm", "", NULL);
> +	if (IS_ERR(val))
> +		ret = PTR_ERR(val);
> +	else if (strncmp(val, "/vm/", 4))
> +		ret = -EIO;
> +	else
> +		ret = sprintf(buffer, "%s\n", val + 4);
> +	kfree(val);
> +
> +	return ret;
> +}
> +
> +HYPERVISOR_ATTR_RO(uuid);
> +
> +static int __init xen_sysfs_uuid_init(void)
> +{
> +	return sysfs_create_file(&hypervisor_subsys.kset.kobj, &uuid_attr.attr);
> +}
> +
> +static void xen_sysfs_uuid_destroy(void)
> +{
> +	sysfs_remove_file(&hypervisor_subsys.kset.kobj, &uuid_attr.attr);
>  }
>  
>  /* xen compilation attributes */
> @@ -285,10 +318,15 @@ static int __init hyper_sysfs_init(void)
>  	ret = xen_compilation_init();
>  	if (ret)
>  		goto comp_out;
> +	ret = xen_sysfs_uuid_init();
> +	if (ret)
> +		goto uuid_out;
>  	ret = xen_properties_init();
>  	if (!ret)
>  		goto out;
>  
> +	xen_sysfs_uuid_destroy();
> +uuid_out:
>  	xen_compilation_destroy();
>  comp_out:
>  	xen_sysfs_version_destroy();
> @@ -302,6 +340,7 @@ static void hyper_sysfs_exit(void)
>  {
>  	xen_properties_destroy();
>  	xen_compilation_destroy();
> +	xen_sysfs_uuid_destroy();
>  	xen_sysfs_version_destroy();
>  	xen_sysfs_type_destroy();
>  
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>