Xen devel - Nov 2011 - [PATCH 1/2] xenstore: xenbus cannot be opened read-only

In order to read keys from xenstore, the xenstore libraries need to
write the request to the xenbus socket. This means that the socket
cannot be opened read-only.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 tools/xenstore/xs.c |   18 ++++++++----------
 1 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c
index c72ea83..df270f7 100644
--- a/tools/xenstore/xs.c
+++ b/tools/xenstore/xs.c
@@ -182,15 +182,13 @@ error:
 	return -1;
 }
 
-static int get_dev(const char *connect_to, unsigned long flags)
+static int get_dev(const char *connect_to)
 {
-	if (flags & XS_OPEN_READONLY)
-		return open(connect_to, O_RDONLY);
-	else
-		return open(connect_to, O_RDWR);
+	/* We cannot open read-only because requests are writes */
+	return open(connect_to, O_RDWR);
 }
 
-static struct xs_handle *get_handle(const char *connect_to, unsigned long
flags)
+static struct xs_handle *get_handle(const char *connect_to)
 {
 	struct stat buf;
 	struct xs_handle *h = NULL;
@@ -202,7 +200,7 @@ static struct xs_handle *get_handle(const char *connect_to,
unsigned long flags)
 	if (S_ISSOCK(buf.st_mode))
 		fd = get_socket(connect_to);
 	else
-		fd = get_dev(connect_to, flags);
+		fd = get_dev(connect_to);
 
 	if (fd == -1)
 		return NULL;
@@ -258,12 +256,12 @@ struct xs_handle *xs_open(unsigned long flags)
 	struct xs_handle *xsh = NULL;
 
 	if (flags & XS_OPEN_READONLY)
-		xsh = get_handle(xs_daemon_socket_ro(), flags);
+		xsh = get_handle(xs_daemon_socket_ro());
 	else
-		xsh = get_handle(xs_daemon_socket(), flags);
+		xsh = get_handle(xs_daemon_socket());
 
 	if (!xsh && !(flags & XS_OPEN_SOCKETONLY))
-		xsh = get_handle(xs_domain_dev(), flags);
+		xsh = get_handle(xs_domain_dev());
 
 	return xsh;
 }
-- 
1.7.6.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

The domain name stored in /local/domain/$domid/name is simpler to
access and is the only domain name modified by "xl rename". Use this
domain name in libxenstat''s reporting.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 tools/xenstat/libxenstat/src/xenstat.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/tools/xenstat/libxenstat/src/xenstat.c
b/tools/xenstat/libxenstat/src/xenstat.c
index 2791cc1..104655d 100644
--- a/tools/xenstat/libxenstat/src/xenstat.c
+++ b/tools/xenstat/libxenstat/src/xenstat.c
@@ -739,17 +739,9 @@ unsigned long long xenstat_tmem_succ_pers_gets(xenstat_tmem
*tmem)
 
 static char *xenstat_get_domain_name(xenstat_handle *handle, unsigned int
domain_id)
 {
-	char path[80], *vmpath;
+	char path[80];
 
-	snprintf(path, sizeof(path),"/local/domain/%i/vm", domain_id);
-
-	vmpath = xs_read(handle->xshandle, XBT_NULL, path, NULL);
-
-	if (vmpath == NULL)
-		return NULL;
-
-	snprintf(path, sizeof(path),"%s/name", vmpath);
-	free(vmpath);
+	snprintf(path, sizeof(path),"/local/domain/%i/name", domain_id);
 
 	return xs_read(handle->xshandle, XBT_NULL, path, NULL);
 }
-- 
1.7.6.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Wed, 2011-11-09 at 18:11 +0000, Daniel De Graaf
wrote:
> The domain name stored in /local/domain/$domid/name is simpler to
> access and is the only domain name modified by "xl rename". Use
this
> domain name in libxenstat''s reporting.
As it happens I noticed this today too since it causes dom0 not to show
up in xentop. My workaround was to add to my initscripts:

xenstore-write /vm/00000000-0000-0000-0000-000000000000/name
"Domain-0"
xenstore-write /local/domain/0/vm /vm/00000000-0000-0000-0000-000000000000

I wondered what xend does and it seems that it also creates
both /local/domain/N/name and /vm/UUID/name but only updates the latter
on "xm rename". So it seems that xend and xl behave exactly opposite
wrt
which one they change on rename. Leaving aside that it seems buggy to a)
record the name twice and b) only update one copy on rename I think "xl
rename" should either do the same thing as "xm rename" or it
should
update both names in xenstore (or maybe we should drop one).

Ian.
> 
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> ---
>  tools/xenstat/libxenstat/src/xenstat.c |   12 ++----------
>  1 files changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/xenstat/libxenstat/src/xenstat.c
b/tools/xenstat/libxenstat/src/xenstat.c
> index 2791cc1..104655d 100644
> --- a/tools/xenstat/libxenstat/src/xenstat.c
> +++ b/tools/xenstat/libxenstat/src/xenstat.c
> @@ -739,17 +739,9 @@ unsigned long long
xenstat_tmem_succ_pers_gets(xenstat_tmem *tmem)
>  
>  static char *xenstat_get_domain_name(xenstat_handle *handle, unsigned int
domain_id)
>  {
> -	char path[80], *vmpath;
> +	char path[80];
>  
> -	snprintf(path, sizeof(path),"/local/domain/%i/vm", domain_id);
> -
> -	vmpath = xs_read(handle->xshandle, XBT_NULL, path, NULL);
> -
> -	if (vmpath == NULL)
> -		return NULL;
> -
> -	snprintf(path, sizeof(path),"%s/name", vmpath);
> -	free(vmpath);
> +	snprintf(path, sizeof(path),"/local/domain/%i/name",
domain_id);
>  
>  	return xs_read(handle->xshandle, XBT_NULL, path, NULL);
>  }


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Ian Campbell writes ("Re: [Xen-devel] [PATCH 2/2] xenstat: Use local domain
names"):
> I wondered what xend does and it seems that it also creates
> both /local/domain/N/name and /vm/UUID/name but only updates the latter
> on "xm rename". So it seems that xend and xl behave exactly
opposite wrt
> which one they change on rename. Leaving aside that it seems buggy to a)
> record the name twice and b) only update one copy on rename I think
"xl
> rename" should either do the same thing as "xm rename" or it
should
> update both names in xenstore (or maybe we should drop one).
I think libxl should store the name only in /local/domain/$domid.
In libxl we identify domains by domid, in general, not uuid.

Since libxl is becoming the preferred toolstack I think we should
change xenstat to look in the libxl location - ie, apply Daniel''s
2/2.  But I''ll wait for further comments.

Ian.