Jerone Young
2005-Oct-27 03:04 UTC
[Xen-devel] [PATCH] Enable xenstat to use xenstore & fix bugzilla #311
Signed-off-by: Jerone Young <jyoung5@us.ibm.com> -- Jerone Young IBM Linux Technology Center jyoung5@us.ibm.com 512-838-1157 (T/L: 678-1157) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony Liguori
2005-Oct-27 14:38 UTC
Re: [Xen-devel] [PATCH] Enable xenstat to use xenstore & fix bugzilla #311
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 > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ewan Mellor
2005-Oct-27 15:21 UTC
Re: [Xen-devel] [PATCH] Enable xenstat to use xenstore & fix bugzilla #311
On Thu, Oct 27, 2005 at 09:38:41AM -0500, Anthony Liguori wrote:> 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.Thanks Anthony, well spotted. I''ll patch that. Ewan. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel