[PATCH] xentop: Adds options for tabs-separators, and including the domain ID in the output.

[PATCH] xentop: Adds options for tabs-separators, and including the domain ID in the output.

[Stefan Wieser]

This change adds two options to xentop:

        -T      adds a tabulator (\t) character after each field, to allow
                easier and more robust parsing. This affects batch mode only.
        -I      includes a column with the domain ID in the output (both the
                graphical output, and the batch output)

This makes the output easier to parse for automated tools.
If none of the options are passed, the output is unchanged, so none of them would break existing tools.

Signed-off-by: Stefan Wieser <swieser@edu.aau.at>
---
 tools/xenstat/xentop/xentop.c | 82 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 65 insertions(+), 17 deletions(-)

diff --git a/tools/xenstat/xentop/xentop.c b/tools/xenstat/xentop/xentop.c
index 2fd2b67..d6a927f 100644
--- a/tools/xenstat/xentop/xentop.c
+++ b/tools/xenstat/xentop/xentop.c
@@ -126,6 +126,9 @@ static int compare_vbd_rsect(xenstat_domain *domain1, xenstat_domain *domain2);
 static void print_vbd_rsect(xenstat_domain *domain);
 static int compare_vbd_wsect(xenstat_domain *domain1, xenstat_domain *domain2);
 static void print_vbd_wsect(xenstat_domain *domain);
+static int compare_domid(xenstat_domain *domain1, xenstat_domain *domain2);
+static void print_domid(xenstat_domain *domain);
+
 static void reset_field_widths(void);
 static void adjust_field_widths(xenstat_domain *domain);
 
@@ -172,6 +175,7 @@ typedef struct field {
 } field;
 
 field fields[] = {
+	{ FIELD_DOMID,     "DOMID",      6, 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     },
@@ -206,6 +210,8 @@ unsigned int delay = 3;
 unsigned int batch = 0;
 unsigned int loop = 1;
 unsigned int iterations = 0;
+int use_tabs = 0;
+int show_domid = 0;
 int show_vcpus = 0;
 int show_networks = 0;
 int show_vbds = 0;
@@ -239,6 +245,8 @@ static void usage(const char *program)
 	       "-r, --repeat-header  repeat table header before each domain\n"
 	       "-v, --vcpus          output vcpu data\n"
 	       "-b, --batch	     output in batch mode, no user input accepted\n"
+	       "-I, --include-domid  includes the domain ID column\n"
+	       "-T, --tabs           use tabs as a field-separator in batch mode\n"
 	       "-i, --iterations     number of iterations before exiting\n"
 	       "-f, --full-name      output the full domain name (not truncated)\n"
 	       "\n" XENTOP_BUGSTO,
@@ -426,6 +434,19 @@ static int compare(unsigned long long i1, unsigned long long i2)
 	return 0;
 }
 
+/* Compares domain ID of two domains, returning -1,0,1 for <,=,> */
+static int compare_domid(xenstat_domain *domain1, xenstat_domain *domain2)
+{
+	return -compare(xenstat_domain_id(domain1),
+		xenstat_domain_id(domain2));
+}
+
+/* Prints the domain ID */
+static void print_domid(xenstat_domain *domain)
+{
+	print("%6u", xenstat_domain_id(domain));
+}
+
 /* Comparison function for use with qsort.  Compares two domains using the
  * current sort field. */
 static int compare_domains(xenstat_domain **domain1, xenstat_domain **domain2)
@@ -914,12 +935,15 @@ void do_summary(void)
 void do_header(void)
 {
 	field_id i;
+	int fields_shown = 0;
 
 	/* Turn on REVERSE highlight attribute for headings */
 	xentop_attron(A_REVERSE);
-	for(i = 0; i < NUM_FIELDS; i++) {
-		if (i != 0)
-			print(" ");
+	for(i = 0; i < NUM_FIELDS; i++, fields_shown++) {
+		if (i == FIELD_DOMID && !show_domid)
+			continue;
+		if (fields_shown)
+			print("%c", use_tabs ? '\t' : ' ');
 		/* The BOLD attribute is turned on for the sort column */
 		if (i == sort_field)
 			xentop_attron(A_BOLD);
@@ -978,17 +1002,22 @@ void do_bottom_line(void)
 /* Prints Domain information */
 void do_domain(xenstat_domain *domain)
 {
-	unsigned int i;
-	for (i = 0; i < NUM_FIELDS; i++) {
-		if (i != 0)
-			print(" ");
+	field_id i;
+	int fields_shown = 0;
+
+	for (i = 0; i < NUM_FIELDS; i++, fields_shown++) {
+		/* Skip the domain ID field if it wasn't requested. */
+		if (i == FIELD_DOMID && !show_domid)
+			continue;
+		if (fields_shown)
+			print(use_tabs ? "\t" : " ");
 		if (i == sort_field)
 			xentop_attron(A_BOLD);
 		fields[i].print(domain);
 		if (i == sort_field)
 			xentop_attroff(A_BOLD);
 	}
-	print("\n");
+	print(use_tabs ? "\t" : "\n");
 }
 
 /* Output all vcpu information */
@@ -1007,13 +1036,13 @@ void do_vcpu(xenstat_domain *domain)
 		vcpu = xenstat_domain_vcpu(domain,i);
 
 		if (xenstat_vcpu_online(vcpu) > 0) {
-			if (i != 0 && (i%5)==0)
+			if (i != 0 && (i%5)==0 && !use_tabs)
 				print("\n        ");
 			print(" %2u: %10llus", i, 
 					xenstat_vcpu_ns(vcpu)/1000000000);
 		}
 	}
-	print("\n");
+	print(use_tabs ? "\t" : "\n");
 }
 
 /* Output all network information */
@@ -1038,11 +1067,12 @@ void do_network(xenstat_domain *domain)
 		      xenstat_network_rerrs(network),
 		      xenstat_network_rdrop(network));
 
-		print("TX: %8llubytes %8llupkts %8lluerr %8lludrop\n",
+		print("TX: %8llubytes %8llupkts %8lluerr %8lludrop%c",
 		      xenstat_network_tbytes(network),
 		      xenstat_network_tpackets(network),
 		      xenstat_network_terrs(network),
-		      xenstat_network_tdrop(network));
+		      xenstat_network_tdrop(network),
+		      use_tabs ? '\t' : '\n');
 	}
 }
 
@@ -1075,14 +1105,15 @@ void do_vbd(xenstat_domain *domain)
 			 MINOR(xenstat_vbd_dev(vbd)));
 #endif
 
-		print("VBD %s %4d %s OO: %8llu   RD: %8llu   WR: %8llu   RSECT: %10llu   WSECT: %10llu\n",
+		print("VBD %s %4d %s OO: %8llu   RD: %8llu   WR: %8llu   RSECT: %10llu   WSECT: %10llu%c",
 		      vbd_type[xenstat_vbd_type(vbd)],
 		      xenstat_vbd_dev(vbd), details,
 		      xenstat_vbd_oo_reqs(vbd),
 		      xenstat_vbd_rd_reqs(vbd),
 		      xenstat_vbd_wr_reqs(vbd),
 		      xenstat_vbd_rd_sects(vbd),
-		      xenstat_vbd_wr_sects(vbd));
+		      xenstat_vbd_wr_sects(vbd),
+		      use_tabs ? '\t' : '\n');
 	}
 }
 
@@ -1097,9 +1128,10 @@ void do_tmem(xenstat_domain *domain)
 
 	if (curr_eph_pages | succ_eph_gets | succ_pers_puts | succ_pers_gets)
 		print("Tmem:  Curr eph pages: %8llu   Succ eph gets: %8llu   "
-	              "Succ pers puts: %8llu   Succ pers gets: %8llu\n",
+	              "Succ pers puts: %8llu   Succ pers gets: %8llu%c",
 			curr_eph_pages, succ_eph_gets,
-			succ_pers_puts, succ_pers_gets);
+			succ_pers_puts, succ_pers_gets,
+			use_tabs ? '\t' : '\n');
 
 }
 
@@ -1157,6 +1189,10 @@ static void top(void)
 			do_vbd(domains[i]);
 		if (show_tmem)
 			do_tmem(domains[i]);
+
+		/* If we were told to separate with tabs, add a new-line once the domain is done. */
+		if (use_tabs) 
+			printf("\n");
 	}
 
 	if (!batch)
@@ -1188,9 +1224,11 @@ int main(int argc, char **argv)
 		{ "batch",	   no_argument,	      NULL, 'b' },
 		{ "iterations",	   required_argument, NULL, 'i' },
 		{ "full-name",     no_argument,       NULL, 'f' },
+		{ "tabs",          no_argument,       NULL, 'T' },
+		{ "include-domid", no_argument,       NULL, 'I' },
 		{ 0, 0, 0, 0 },
 	};
-	const char *sopts = "hVnxrvd:bi:f";
+	const char *sopts = "hVnxrvd:bi:fTI";
 
 	if (atexit(cleanup) != 0)
 		fail("Failed to install cleanup handler.\n");
@@ -1235,9 +1273,19 @@ int main(int argc, char **argv)
 		case 't':
 			show_tmem = 1;
 			break;
+		case 'T':
+			use_tabs = 1;
+			break;
+		case 'I':
+			show_domid = 1;
+			break;
 		}
 	}
 
+	if (use_tabs && !batch) {
+		fail("Cannot use tabs in interactive mode.\n");
+	}
+
 	/* Get xenstat handle */
 	xhandle = xenstat_init();
 	if (xhandle == NULL)
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] xentop: Adds options for tabs-separators, and including the domain ID in the output.

[Wei Liu]

On Sat, Aug 13, 2016 at 05:51:21PM +0000, Stefan Wieser wrote:
> This change adds two options to xentop:
> 
>         -T      adds a tabulator (\t) character after each field, to allow
>                 easier and more robust parsing. This affects batch mode only.
>         -I      includes a column with the domain ID in the output (both the
>                 graphical output, and the batch output)
> 
> This makes the output easier to parse for automated tools.
> If none of the options are passed, the output is unchanged, so none of them would break existing tools.

Line too long.

And thanks for having compatibility in mind.

> @@ -1235,9 +1273,19 @@ int main(int argc, char **argv)
>  		case 't':
>  			show_tmem = 1;
>  			break;
> +		case 'T':
> +			use_tabs = 1;
> +			break;
> +		case 'I':
> +			show_domid = 1;
> +			break;
>  		}
>  	}
>  
> +	if (use_tabs && !batch) {
> +		fail("Cannot use tabs in interactive mode.\n");
> +	}
> +

Minor nit: no need to use {} here.

No need to resend, though. Should be easy to fix if I am to commit this
patch.

All in all I have no opinion for a few new options. I will wait a few
gays so that other people can express their opinions.

Wei.

>  	/* Get xenstat handle */
>  	xhandle = xenstat_init();
>  	if (xhandle == NULL)
> -- 
> 1.9.1
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] xentop: Adds options for tabs-separators, and including the domain ID in the output.

[Wei Liu]

On Mon, Aug 15, 2016 at 01:09:32PM +0100, Wei Liu wrote:
> On Sat, Aug 13, 2016 at 05:51:21PM +0000, Stefan Wieser wrote:
> > This change adds two options to xentop:
> > 
> >         -T      adds a tabulator (\t) character after each field, to allow
> >                 easier and more robust parsing. This affects batch mode only.
> >         -I      includes a column with the domain ID in the output (both the
> >                 graphical output, and the batch output)
> > 
> > This makes the output easier to parse for automated tools.
> > If none of the options are passed, the output is unchanged, so none of them would break existing tools.
> 
> Line too long.
> 
> And thanks for having compatibility in mind.
> 
> > @@ -1235,9 +1273,19 @@ int main(int argc, char **argv)
> >  		case 't':
> >  			show_tmem = 1;
> >  			break;
> > +		case 'T':
> > +			use_tabs = 1;
> > +			break;
> > +		case 'I':
> > +			show_domid = 1;
> > +			break;
> >  		}
> >  	}
> >  
> > +	if (use_tabs && !batch) {
> > +		fail("Cannot use tabs in interactive mode.\n");
> > +	}
> > +
> 
> Minor nit: no need to use {} here.
> 
> No need to resend, though. Should be easy to fix if I am to commit this
> patch.
> 
> All in all I have no opinion for a few new options. I will wait a few
> gays so that other people can express their opinions.
> 

^ days, sorry...

> Wei.
> 
> >  	/* Get xenstat handle */
> >  	xhandle = xenstat_init();
> >  	if (xhandle == NULL)
> > -- 
> > 1.9.1
> > 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] xentop: Adds options for tabs-separators, and including the domain ID in the output.

[Ian Jackson]

Wei Liu writes ("Re: [PATCH] xentop: Adds options for tabs-separators, and including the domain ID in the output."):
> On Sat, Aug 13, 2016 at 05:51:21PM +0000, Stefan Wieser wrote:
> > This change adds two options to xentop:
> > 
> >         -T      adds a tabulator (\t) character after each field,
> >                 to allow easier and more robust parsing. This
> >                 affects batch mode only.
...
> All in all I have no opinion for a few new options. I will wait a few
> gays so that other people can express their opinions.

In the absence of a better way of collecting this data in a
machine-readable fashion, I think something like this patch is a good
idea.

Stefan, I think you need to do something about the possibility that a
domain name could contain \t.  I suggest \-escaping the domname
(\ => \\; tab => \t; space left unchanged; bytes 0-31 and 127 encoded
with \xXX; assumption is that output may be UTF-8, which will be
passed unchanged if it contains no control chars).

I think this behaviour should be enabled by -T.

Also, I think you could profitably add something to the docs
explaining how to parse the output.  In particular, that a parser must
look at the column headings and not assume the columns will always be
the same columsn in the same order.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel