Re: [PATCH] Enable xenstat to use xenstore & fix bugzilla #311

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

There's a small memory leak in this.  xenstore_read returns a pointer 
that must be free()'d by the caller.  Just make sure that when the 
domain structure is cleaned up that name is free()'d and all should be well.

Regards,

Anthony Liguori

Jerone Young wrote:

>Signed-off-by: Jerone Young <jyoung5@us.ibm.com>
>  
>
>------------------------------------------------------------------------
>
># HG changeset patch
># User jeroney@localhost.localdomain
># Node ID 1b9608b5fea4cb4bfa0fa7ca48bc58aad6007243
># Parent  7f3f018a694f6a8befb14e547a6f0433223ff7ec
>* Enable xenstat to use xenstore
>* Have xentop display names instead of DomIDs
>  as requested in Bugzilla#311 filed by Ian
>
>diff -r 7f3f018a694f -r 1b9608b5fea4 tools/xenstat/libxenstat/Makefile
>--- a/tools/xenstat/libxenstat/Makefile	Tue Oct 25 16:09:28 2005
>+++ b/tools/xenstat/libxenstat/Makefile	Thu Oct 27 02:57:52 2005
>@@ -38,13 +38,13 @@
> 
> WARN_FLAGS=-Wall -Werror
> 
>-CFLAGS+=-Isrc -I$(XEN_LIBXC)
>+CFLAGS+=-Isrc -I$(XEN_LIBXC) -I$(XEN_XENSTORE)
> LDFLAGS+=-Lsrc
> 
> all: $(LIB)
> 
> $(LIB): $(OBJECTS)
>-	$(AR) rc $@ $^
>+	$(AR) rc $@ $^ $(XEN_XENSTORE)/libxenstore.so
> 	$(RANLIB) $@
> 
> $(SHLIB): $(OBJECTS)
>diff -r 7f3f018a694f -r 1b9608b5fea4 tools/xenstat/libxenstat/src/xenstat.c
>--- a/tools/xenstat/libxenstat/src/xenstat.c	Tue Oct 25 16:09:28 2005
>+++ b/tools/xenstat/libxenstat/src/xenstat.c	Thu Oct 27 02:57:52 2005
>@@ -21,6 +21,7 @@
> #include <string.h>
> #include <unistd.h>
> #include <xen-interface.h>
>+#include <xs.h>
> #include "xenstat.h"
> 
> /*
>@@ -31,6 +32,7 @@
> 
> struct xenstat_handle {
> 	xi_handle *xihandle;
>+	struct xs_handle *xshandle; /* xenstore handle */
> 	int page_size;
> 	FILE *procnetdev;
> 	char xen_version[VERSION_SIZE]; /* xen version running on this node */
>@@ -49,6 +51,7 @@
> 
> struct xenstat_domain {
> 	unsigned int id;
>+	char *name;
> 	unsigned int state;
> 	unsigned long long cpu_ns;
> 	unsigned int num_vcpus;		/* No. vcpus configured for domain */
>@@ -110,6 +113,7 @@
> static void xenstat_uninit_vcpus(xenstat_handle * handle);
> static void xenstat_uninit_networks(xenstat_handle * handle);
> static void xenstat_uninit_xen_version(xenstat_handle * handle);
>+static char *xenstat_get_domain_name(xenstat_handle * handle, unsigned int domain_id);
> 
> static xenstat_collector collectors[] = {
> 	{ XENSTAT_VCPU, xenstat_collect_vcpus,
>@@ -153,6 +157,13 @@
> 		return NULL;
> 	}
> 
>+	handle->xshandle = xs_daemon_open_readonly(); /* open handle to xenstore*/
>+	if (handle->xshandle == NULL) {
>+		perror("unable to open xenstore\n");
>+		free(handle);
>+		return NULL;
>+	}
>+
> 	return handle;
> }
> 
>@@ -163,6 +174,7 @@
> 		for (i = 0; i < NUM_COLLECTORS; i++)
> 			collectors[i].uninit(handle);
> 		xi_uninit(handle->xihandle);
>+		xs_daemon_close(handle->xshandle);
> 		free(handle);
> 	}
> }
>@@ -228,6 +240,7 @@
> 		for (i = 0; i < new_domains; i++) {
> 			/* Fill in domain using domaininfo[i] */
> 			domain->id = domaininfo[i].domain;
>+			domain->name = xenstat_get_domain_name(handle, domaininfo[i].domain);
> 			domain->state = domaininfo[i].flags;
> 			domain->cpu_ns = domaininfo[i].cpu_time;
> 			domain->num_vcpus = (domaininfo[i].max_vcpu_id+1);
>@@ -337,6 +350,12 @@
> unsigned xenstat_domain_id(xenstat_domain * domain)
> {
> 	return domain->id;
>+}
>+
>+/* Get the domain name for the domain */
>+char *xenstat_domain_name(xenstat_domain * domain)
>+{
>+	return domain->name;
> }
> 
> /* Get information about how much CPU time has been used */
>@@ -675,3 +694,25 @@
> static void xenstat_uninit_xen_version(xenstat_handle * handle)
> {
> }
>+
>+static char *xenstat_get_domain_name(xenstat_handle *handle, unsigned int domain_id)
>+{
>+	char path[80];
>+	char *name;
>+	unsigned int *len;
>+	struct xs_transaction_handle *xstranshandle;
>+
>+	snprintf(path, sizeof(path),"/local/domain/%i/name", domain_id);
>+	
>+	xstranshandle = xs_transaction_start(handle->xshandle);
>+	if (xstranshandle == NULL) {
>+		perror("Unable to get transcation handle from xenstore\n");
>+		exit(1); /* Change this */
>+	}
>+
>+	name = (char *) xs_read(handle->xshandle, xstranshandle, path, len);
>+	
>+	xs_transaction_end(handle->xshandle, xstranshandle, false);
>+
>+	return name;
>+}	
>diff -r 7f3f018a694f -r 1b9608b5fea4 tools/xenstat/libxenstat/src/xenstat.h
>--- a/tools/xenstat/libxenstat/src/xenstat.h	Tue Oct 25 16:09:28 2005
>+++ b/tools/xenstat/libxenstat/src/xenstat.h	Thu Oct 27 02:57:52 2005
>@@ -80,6 +80,9 @@
> /* Get the domain ID for this domain */
> unsigned xenstat_domain_id(xenstat_domain * domain);
> 
>+/* Set the domain name for the domain */
>+char *xenstat_domain_name(xenstat_domain * domain);
>+
> /* Get information about how much CPU time has been used */
> unsigned long long xenstat_domain_cpu_ns(xenstat_domain * domain);
> 
>diff -r 7f3f018a694f -r 1b9608b5fea4 tools/xenstat/xentop/Makefile
>--- a/tools/xenstat/xentop/Makefile	Tue Oct 25 16:09:28 2005
>+++ b/tools/xenstat/xentop/Makefile	Thu Oct 27 02:57:52 2005
>@@ -26,7 +26,7 @@
> man1dir=$(mandir)/man1
> sbindir=$(prefix)/sbin
> 
>-CFLAGS += -DGCC_PRINTF -Wall -Werror -I$(XEN_LIBXENSTAT)
>+CFLAGS += -DGCC_PRINTF -Wall -I$(XEN_LIBXENSTAT)
> LDFLAGS += -L$(XEN_LIBXENSTAT)
> LDLIBS += -lxenstat -lncurses
> 
>diff -r 7f3f018a694f -r 1b9608b5fea4 tools/xenstat/xentop/xentop.c
>--- a/tools/xenstat/xentop/xentop.c	Tue Oct 25 16:09:28 2005
>+++ b/tools/xenstat/xentop/xentop.c	Thu Oct 27 02:57:52 2005
>@@ -28,6 +28,7 @@
> #include <time.h>
> #include <unistd.h>
> 
>+#include <xs.h>
> #include <xenstat.h>
> 
> #define XENTOP_VERSION "1.0"
>@@ -91,6 +92,8 @@
> static void print_net_rx(xenstat_domain *domain);
> static int compare_ssid(xenstat_domain *domain1, xenstat_domain *domain2);
> static void print_ssid(xenstat_domain *domain);
>+static int compare_name(xenstat_domain *domain1, xenstat_domain *domain2);
>+static void print_name(xenstat_domain *domain);
> 
> /* Section printing functions */
> static void do_summary(void);
>@@ -104,6 +107,7 @@
> /* Field types */
> typedef enum field_id {
> 	FIELD_DOMID,
>+	FIELD_NAME,
> 	FIELD_STATE,
> 	FIELD_CPU,
> 	FIELD_CPU_PCT,
>@@ -127,7 +131,8 @@
> } field;
> 
> field fields[] = {
>-	{ FIELD_DOMID,   "DOMID",      5, compare_domid,   print_domid   },
>+//	{ FIELD_DOMID,   "DOMID",      5, compare_domid,   print_domid   },
>+	{ FIELD_NAME,    "NAME",      10, compare_name,    print_name    },
> 	{ FIELD_STATE,   "STATE",      6, compare_state,   print_state   },
> 	{ FIELD_CPU,     "CPU(sec)",  10, compare_cpu,     print_cpu     },
> 	{ FIELD_CPU_PCT, "CPU(%)",     6, compare_cpu_pct, print_cpu_pct },
>@@ -356,6 +361,18 @@
> 	print("%5u", xenstat_domain_id(domain));
> }
> 
>+/* Compare domain names, returning -1,0,1 for <,=,> */
>+int compare_name(xenstat_domain *domain1, xenstat_domain *domain2)
>+{
>+	return strcasecmp(xenstat_domain_name(domain1), xenstat_domain_name(domain2));
>+}
>+
>+/* Prints domain name */
>+void print_name(xenstat_domain *domain)
>+{
>+	print("%10s", xenstat_domain_name(domain));
>+}
>+
> struct {
> 	unsigned int (*get)(xenstat_domain *);
> 	char ch;
>  
>
>------------------------------------------------------------------------
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@lists.xensource.com
>http://lists.xensource.com/xen-devel
>  
>