[PATCH] xentop: Dynamically expand some columns

[PATCH] xentop: Dynamically expand some columns

[Charles Arnold]

Allow certain xentop columns to automatically expand as the amount
of data reported gets larger.  The columns allowed to expand are:

NETTX(k), NETRX(k), VBD_RD, VBD_WR, VBD_RSECT, VBD_WSECT

Author: Markus Hauschild <Markus.Hauschild@rz.uni-regensburg.de>
Signed-off-by: Charles Arnold <carnold@suse.com>

diff --git a/tools/xenstat/xentop/xentop.c b/tools/xenstat/xentop/xentop.c
index dd11927..d087665 100644
--- a/tools/xenstat/xentop/xentop.c
+++ b/tools/xenstat/xentop/xentop.c
@@ -27,6 +27,7 @@
 
 #include <ctype.h>
 #include <errno.h>
+#include <math.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdarg.h>
@@ -127,7 +128,8 @@ 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 void reset_field_widths(void);
+static void adjust_field_widths(xenstat_domain *domain);
 
 /* Section printing functions */
 static void do_summary(void);
@@ -623,7 +625,7 @@ static int compare_net_tx(xenstat_domain *domain1, xenstat_domain *domain2)
 /* Prints number of total network tx bytes statistic */
 static void print_net_tx(xenstat_domain *domain)
 {
-	print("%8llu", tot_net_bytes(domain, FALSE)/1024);
+	print("%*llu", fields[FIELD_NET_TX-1].default_width, tot_net_bytes(domain, FALSE)/1024);
 }
 
 /* Compares number of total network rx bytes of two domains, returning -1,0,1
@@ -637,7 +639,7 @@ static int compare_net_rx(xenstat_domain *domain1, xenstat_domain *domain2)
 /* Prints number of total network rx bytes statistic */
 static void print_net_rx(xenstat_domain *domain)
 {
-	print("%8llu", tot_net_bytes(domain, TRUE)/1024);
+	print("%*llu", fields[FIELD_NET_RX-1].default_width, tot_net_bytes(domain, TRUE)/1024);
 }
 
 /* Gets number of total network bytes statistic, if rx true, then rx bytes
@@ -705,7 +707,7 @@ static int compare_vbd_rd(xenstat_domain *domain1, xenstat_domain *domain2)
 /* Prints number of total VBD READ requests statistic */
 static void print_vbd_rd(xenstat_domain *domain)
 {
-	print("%8llu", tot_vbd_reqs(domain, FIELD_VBD_RD));
+	print("%*llu", fields[FIELD_VBD_RD-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_RD));
 }
 
 /* Compares number of total VBD WRITE requests of two domains,
@@ -719,7 +721,7 @@ static int compare_vbd_wr(xenstat_domain *domain1, xenstat_domain *domain2)
 /* Prints number of total VBD WRITE requests statistic */
 static void print_vbd_wr(xenstat_domain *domain)
 {
-	print("%8llu", tot_vbd_reqs(domain, FIELD_VBD_WR));
+	print("%*llu", fields[FIELD_VBD_WR-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_WR));
 }
 
 /* Compares number of total VBD READ sectors of two domains,
@@ -733,7 +735,7 @@ static int compare_vbd_rsect(xenstat_domain *domain1, xenstat_domain *domain2)
 /* Prints number of total VBD READ sectors statistic */
 static void print_vbd_rsect(xenstat_domain *domain)
 {
-	print("%10llu", tot_vbd_reqs(domain, FIELD_VBD_RSECT));
+	print("%*llu", fields[FIELD_VBD_RSECT-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_RSECT));
 }
 
 /* Compares number of total VBD WRITE sectors of two domains,
@@ -747,7 +749,7 @@ static int compare_vbd_wsect(xenstat_domain *domain1, xenstat_domain *domain2)
 /* Prints number of total VBD WRITE sectors statistic */
 static void print_vbd_wsect(xenstat_domain *domain)
 {
-	print("%10llu", tot_vbd_reqs(domain, FIELD_VBD_WSECT));
+	print("%*llu", fields[FIELD_VBD_WSECT-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_WSECT));
 }
 
 
@@ -806,6 +808,48 @@ static void print_ssid(xenstat_domain *domain)
 	print("%4u", xenstat_domain_ssid(domain));
 }
 
+/* Resets default_width for fields with potentially large numbers */
+void reset_field_widths(void)
+{
+	fields[FIELD_NET_TX-1].default_width = 8;
+	fields[FIELD_NET_RX-1].default_width = 8;
+	fields[FIELD_VBD_RD-1].default_width = 8;
+	fields[FIELD_VBD_WR-1].default_width = 8;
+	fields[FIELD_VBD_RSECT-1].default_width = 10;
+	fields[FIELD_VBD_WSECT-1].default_width = 10;
+}
+
+/* Adjusts default_width for fields with potentially large numbers */
+void adjust_field_widths(xenstat_domain *domain)
+{
+	unsigned int length;
+
+	length = (unsigned int)(log10(tot_net_bytes(domain, FALSE)/1024) + 1);
+	if (length > fields[FIELD_NET_TX-1].default_width)
+		fields[FIELD_NET_TX-1].default_width = length;
+
+	length = (unsigned int)(log10(tot_net_bytes(domain, TRUE)/1024) + 1);
+	if (length > fields[FIELD_NET_RX-1].default_width)
+		fields[FIELD_NET_RX-1].default_width = length;
+
+	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_RD)) + 1);
+	if (length > fields[FIELD_VBD_RD-1].default_width)
+		fields[FIELD_VBD_RD-1].default_width = length;
+
+	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_WR)) + 1);
+	if (length > fields[FIELD_VBD_WR-1].default_width)
+		fields[FIELD_VBD_WR-1].default_width = length;
+
+	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_RSECT)) + 1);
+	if (length > fields[FIELD_VBD_RSECT-1].default_width)
+		fields[FIELD_VBD_RSECT-1].default_width = length;
+
+	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_WSECT)) + 1);
+	if (length > fields[FIELD_VBD_WSECT-1].default_width)
+		fields[FIELD_VBD_WSECT-1].default_width = length;
+}
+
+
 /* Section printing functions */
 /* Prints the top summary, above the domain table */
 void do_summary(void)
@@ -1088,6 +1132,12 @@ static void top(void)
 	if(first_domain_index >= num_domains)
 		first_domain_index = num_domains-1;
 
+	/* Adjust default_width for fields with potentially large numbers */
+	reset_field_widths();
+	for (i = first_domain_index; i < num_domains; i++) {
+		adjust_field_widths(domains[i]);
+	}
+
 	for (i = first_domain_index; i < num_domains; i++) {
 		if(!batch && current_row() == lines()-1)
 			break;

Re: [PATCH] xentop: Dynamically expand some columns

[Andrew Cooper]

On 02/10/14 16:58, Charles Arnold wrote:
> Allow certain xentop columns to automatically expand as the amount
> of data reported gets larger.  The columns allowed to expand are:
>
> NETTX(k), NETRX(k), VBD_RD, VBD_WR, VBD_RSECT, VBD_WSECT
>
> Author: Markus Hauschild <Markus.Hauschild@rz.uni-regensburg.de>
> Signed-off-by: Charles Arnold <carnold@suse.com>

In principle, very nice.  (I have wanted to see about doing this for a
while now, but very far down the todo list)

How about the NAME field? 9 characters isn't enough for some people.

~Andrew

>
> diff --git a/tools/xenstat/xentop/xentop.c b/tools/xenstat/xentop/xentop.c
> index dd11927..d087665 100644
> --- a/tools/xenstat/xentop/xentop.c
> +++ b/tools/xenstat/xentop/xentop.c
> @@ -27,6 +27,7 @@
>  
>  #include <ctype.h>
>  #include <errno.h>
> +#include <math.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <stdarg.h>
> @@ -127,7 +128,8 @@ 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 void reset_field_widths(void);
> +static void adjust_field_widths(xenstat_domain *domain);
>  
>  /* Section printing functions */
>  static void do_summary(void);
> @@ -623,7 +625,7 @@ static int compare_net_tx(xenstat_domain *domain1, xenstat_domain *domain2)
>  /* Prints number of total network tx bytes statistic */
>  static void print_net_tx(xenstat_domain *domain)
>  {
> -	print("%8llu", tot_net_bytes(domain, FALSE)/1024);
> +	print("%*llu", fields[FIELD_NET_TX-1].default_width, tot_net_bytes(domain, FALSE)/1024);
>  }
>  
>  /* Compares number of total network rx bytes of two domains, returning -1,0,1
> @@ -637,7 +639,7 @@ static int compare_net_rx(xenstat_domain *domain1, xenstat_domain *domain2)
>  /* Prints number of total network rx bytes statistic */
>  static void print_net_rx(xenstat_domain *domain)
>  {
> -	print("%8llu", tot_net_bytes(domain, TRUE)/1024);
> +	print("%*llu", fields[FIELD_NET_RX-1].default_width, tot_net_bytes(domain, TRUE)/1024);
>  }
>  
>  /* Gets number of total network bytes statistic, if rx true, then rx bytes
> @@ -705,7 +707,7 @@ static int compare_vbd_rd(xenstat_domain *domain1, xenstat_domain *domain2)
>  /* Prints number of total VBD READ requests statistic */
>  static void print_vbd_rd(xenstat_domain *domain)
>  {
> -	print("%8llu", tot_vbd_reqs(domain, FIELD_VBD_RD));
> +	print("%*llu", fields[FIELD_VBD_RD-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_RD));
>  }
>  
>  /* Compares number of total VBD WRITE requests of two domains,
> @@ -719,7 +721,7 @@ static int compare_vbd_wr(xenstat_domain *domain1, xenstat_domain *domain2)
>  /* Prints number of total VBD WRITE requests statistic */
>  static void print_vbd_wr(xenstat_domain *domain)
>  {
> -	print("%8llu", tot_vbd_reqs(domain, FIELD_VBD_WR));
> +	print("%*llu", fields[FIELD_VBD_WR-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_WR));
>  }
>  
>  /* Compares number of total VBD READ sectors of two domains,
> @@ -733,7 +735,7 @@ static int compare_vbd_rsect(xenstat_domain *domain1, xenstat_domain *domain2)
>  /* Prints number of total VBD READ sectors statistic */
>  static void print_vbd_rsect(xenstat_domain *domain)
>  {
> -	print("%10llu", tot_vbd_reqs(domain, FIELD_VBD_RSECT));
> +	print("%*llu", fields[FIELD_VBD_RSECT-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_RSECT));
>  }
>  
>  /* Compares number of total VBD WRITE sectors of two domains,
> @@ -747,7 +749,7 @@ static int compare_vbd_wsect(xenstat_domain *domain1, xenstat_domain *domain2)
>  /* Prints number of total VBD WRITE sectors statistic */
>  static void print_vbd_wsect(xenstat_domain *domain)
>  {
> -	print("%10llu", tot_vbd_reqs(domain, FIELD_VBD_WSECT));
> +	print("%*llu", fields[FIELD_VBD_WSECT-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_WSECT));
>  }
>  
>  
> @@ -806,6 +808,48 @@ static void print_ssid(xenstat_domain *domain)
>  	print("%4u", xenstat_domain_ssid(domain));
>  }
>  
> +/* Resets default_width for fields with potentially large numbers */
> +void reset_field_widths(void)
> +{
> +	fields[FIELD_NET_TX-1].default_width = 8;
> +	fields[FIELD_NET_RX-1].default_width = 8;
> +	fields[FIELD_VBD_RD-1].default_width = 8;
> +	fields[FIELD_VBD_WR-1].default_width = 8;
> +	fields[FIELD_VBD_RSECT-1].default_width = 10;
> +	fields[FIELD_VBD_WSECT-1].default_width = 10;
> +}
> +
> +/* Adjusts default_width for fields with potentially large numbers */
> +void adjust_field_widths(xenstat_domain *domain)
> +{
> +	unsigned int length;
> +
> +	length = (unsigned int)(log10(tot_net_bytes(domain, FALSE)/1024) + 1);
> +	if (length > fields[FIELD_NET_TX-1].default_width)
> +		fields[FIELD_NET_TX-1].default_width = length;
> +
> +	length = (unsigned int)(log10(tot_net_bytes(domain, TRUE)/1024) + 1);
> +	if (length > fields[FIELD_NET_RX-1].default_width)
> +		fields[FIELD_NET_RX-1].default_width = length;
> +
> +	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_RD)) + 1);
> +	if (length > fields[FIELD_VBD_RD-1].default_width)
> +		fields[FIELD_VBD_RD-1].default_width = length;
> +
> +	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_WR)) + 1);
> +	if (length > fields[FIELD_VBD_WR-1].default_width)
> +		fields[FIELD_VBD_WR-1].default_width = length;
> +
> +	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_RSECT)) + 1);
> +	if (length > fields[FIELD_VBD_RSECT-1].default_width)
> +		fields[FIELD_VBD_RSECT-1].default_width = length;
> +
> +	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_WSECT)) + 1);
> +	if (length > fields[FIELD_VBD_WSECT-1].default_width)
> +		fields[FIELD_VBD_WSECT-1].default_width = length;
> +}
> +
> +
>  /* Section printing functions */
>  /* Prints the top summary, above the domain table */
>  void do_summary(void)
> @@ -1088,6 +1132,12 @@ static void top(void)
>  	if(first_domain_index >= num_domains)
>  		first_domain_index = num_domains-1;
>  
> +	/* Adjust default_width for fields with potentially large numbers */
> +	reset_field_widths();
> +	for (i = first_domain_index; i < num_domains; i++) {
> +		adjust_field_widths(domains[i]);
> +	}
> +
>  	for (i = first_domain_index; i < num_domains; i++) {
>  		if(!batch && current_row() == lines()-1)
>  			break;
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH] xentop: Dynamically expand some columns

[Charles Arnold]

>>> On 10/2/2014 at 10:10 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote: 
> On 02/10/14 16:58, Charles Arnold wrote:
>> Allow certain xentop columns to automatically expand as the amount
>> of data reported gets larger.  The columns allowed to expand are:
>>
>> NETTX(k), NETRX(k), VBD_RD, VBD_WR, VBD_RSECT, VBD_WSECT
>>
>> Author: Markus Hauschild <Markus.Hauschild@rz.uni-regensburg.de>
>> Signed-off-by: Charles Arnold <carnold@suse.com>
> 
> In principle, very nice.  (I have wanted to see about doing this for a
> while now, but very far down the todo list)
> 
> How about the NAME field? 9 characters isn't enough for some people.

Sure, and any others that might need it.  But what to do about the '-f' flag
which says show me the full VM name but the flag doesn't adjust the entire
column.  By automatically showing the full name and adjusting the column
appropriately it makes this flag pointless (which I'm ok with). 

- Charles

>>
>> diff --git a/tools/xenstat/xentop/xentop.c b/tools/xenstat/xentop/xentop.c
>> index dd11927..d087665 100644
>> --- a/tools/xenstat/xentop/xentop.c
>> +++ b/tools/xenstat/xentop/xentop.c
>> @@ -27,6 +27,7 @@
>>  
>>  #include <ctype.h>
>>  #include <errno.h>
>> +#include <math.h>
>>  #include <stdio.h>
>>  #include <stdlib.h>
>>  #include <stdarg.h>
>> @@ -127,7 +128,8 @@ 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 void reset_field_widths(void);
>> +static void adjust_field_widths(xenstat_domain *domain);
>>  
>>  /* Section printing functions */
>>  static void do_summary(void);
>> @@ -623,7 +625,7 @@ static int compare_net_tx(xenstat_domain *domain1, 
> xenstat_domain *domain2)
>>  /* Prints number of total network tx bytes statistic */
>>  static void print_net_tx(xenstat_domain *domain)
>>  {
>> -	print("%8llu", tot_net_bytes(domain, FALSE)/1024);
>> +	print("%*llu", fields[FIELD_NET_TX-1].default_width, tot_net_bytes(domain, 
> FALSE)/1024);
>>  }
>>  
>>  /* Compares number of total network rx bytes of two domains, returning 
> -1,0,1
>> @@ -637,7 +639,7 @@ static int compare_net_rx(xenstat_domain *domain1, 
> xenstat_domain *domain2)
>>  /* Prints number of total network rx bytes statistic */
>>  static void print_net_rx(xenstat_domain *domain)
>>  {
>> -	print("%8llu", tot_net_bytes(domain, TRUE)/1024);
>> +	print("%*llu", fields[FIELD_NET_RX-1].default_width, tot_net_bytes(domain, 
> TRUE)/1024);
>>  }
>>  
>>  /* Gets number of total network bytes statistic, if rx true, then rx bytes
>> @@ -705,7 +707,7 @@ static int compare_vbd_rd(xenstat_domain *domain1, 
> xenstat_domain *domain2)
>>  /* Prints number of total VBD READ requests statistic */
>>  static void print_vbd_rd(xenstat_domain *domain)
>>  {
>> -	print("%8llu", tot_vbd_reqs(domain, FIELD_VBD_RD));
>> +	print("%*llu", fields[FIELD_VBD_RD-1].default_width, tot_vbd_reqs(domain, 
> FIELD_VBD_RD));
>>  }
>>  
>>  /* Compares number of total VBD WRITE requests of two domains,
>> @@ -719,7 +721,7 @@ static int compare_vbd_wr(xenstat_domain *domain1, 
> xenstat_domain *domain2)
>>  /* Prints number of total VBD WRITE requests statistic */
>>  static void print_vbd_wr(xenstat_domain *domain)
>>  {
>> -	print("%8llu", tot_vbd_reqs(domain, FIELD_VBD_WR));
>> +	print("%*llu", fields[FIELD_VBD_WR-1].default_width, tot_vbd_reqs(domain, 
> FIELD_VBD_WR));
>>  }
>>  
>>  /* Compares number of total VBD READ sectors of two domains,
>> @@ -733,7 +735,7 @@ static int compare_vbd_rsect(xenstat_domain *domain1, 
> xenstat_domain *domain2)
>>  /* Prints number of total VBD READ sectors statistic */
>>  static void print_vbd_rsect(xenstat_domain *domain)
>>  {
>> -	print("%10llu", tot_vbd_reqs(domain, FIELD_VBD_RSECT));
>> +	print("%*llu", fields[FIELD_VBD_RSECT-1].default_width, tot_vbd_reqs(domain, 
> FIELD_VBD_RSECT));
>>  }
>>  
>>  /* Compares number of total VBD WRITE sectors of two domains,
>> @@ -747,7 +749,7 @@ static int compare_vbd_wsect(xenstat_domain *domain1, 
> xenstat_domain *domain2)
>>  /* Prints number of total VBD WRITE sectors statistic */
>>  static void print_vbd_wsect(xenstat_domain *domain)
>>  {
>> -	print("%10llu", tot_vbd_reqs(domain, FIELD_VBD_WSECT));
>> +	print("%*llu", fields[FIELD_VBD_WSECT-1].default_width, tot_vbd_reqs(domain, 
> FIELD_VBD_WSECT));
>>  }
>>  
>>  
>> @@ -806,6 +808,48 @@ static void print_ssid(xenstat_domain *domain)
>>  	print("%4u", xenstat_domain_ssid(domain));
>>  }
>>  
>> +/* Resets default_width for fields with potentially large numbers */
>> +void reset_field_widths(void)
>> +{
>> +	fields[FIELD_NET_TX-1].default_width = 8;
>> +	fields[FIELD_NET_RX-1].default_width = 8;
>> +	fields[FIELD_VBD_RD-1].default_width = 8;
>> +	fields[FIELD_VBD_WR-1].default_width = 8;
>> +	fields[FIELD_VBD_RSECT-1].default_width = 10;
>> +	fields[FIELD_VBD_WSECT-1].default_width = 10;
>> +}
>> +
>> +/* Adjusts default_width for fields with potentially large numbers */
>> +void adjust_field_widths(xenstat_domain *domain)
>> +{
>> +	unsigned int length;
>> +
>> +	length = (unsigned int)(log10(tot_net_bytes(domain, FALSE)/1024) + 1);
>> +	if (length > fields[FIELD_NET_TX-1].default_width)
>> +		fields[FIELD_NET_TX-1].default_width = length;
>> +
>> +	length = (unsigned int)(log10(tot_net_bytes(domain, TRUE)/1024) + 1);
>> +	if (length > fields[FIELD_NET_RX-1].default_width)
>> +		fields[FIELD_NET_RX-1].default_width = length;
>> +
>> +	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_RD)) + 1);
>> +	if (length > fields[FIELD_VBD_RD-1].default_width)
>> +		fields[FIELD_VBD_RD-1].default_width = length;
>> +
>> +	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_WR)) + 1);
>> +	if (length > fields[FIELD_VBD_WR-1].default_width)
>> +		fields[FIELD_VBD_WR-1].default_width = length;
>> +
>> +	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_RSECT)) + 1);
>> +	if (length > fields[FIELD_VBD_RSECT-1].default_width)
>> +		fields[FIELD_VBD_RSECT-1].default_width = length;
>> +
>> +	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_WSECT)) + 1);
>> +	if (length > fields[FIELD_VBD_WSECT-1].default_width)
>> +		fields[FIELD_VBD_WSECT-1].default_width = length;
>> +}
>> +
>> +
>>  /* Section printing functions */
>>  /* Prints the top summary, above the domain table */
>>  void do_summary(void)
>> @@ -1088,6 +1132,12 @@ static void top(void)
>>  	if(first_domain_index >= num_domains)
>>  		first_domain_index = num_domains-1;
>>  
>> +	/* Adjust default_width for fields with potentially large numbers */
>> +	reset_field_widths();
>> +	for (i = first_domain_index; i < num_domains; i++) {
>> +		adjust_field_widths(domains[i]);
>> +	}
>> +
>>  	for (i = first_domain_index; i < num_domains; i++) {
>>  		if(!batch && current_row() == lines()-1)
>>  			break;
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel

Re: [PATCH] xentop: Dynamically expand some columns

[Konrad Rzeszutek Wilk]

On Thu, Oct 02, 2014 at 10:25:41AM -0600, Charles Arnold wrote:
> >>> On 10/2/2014 at 10:10 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote: 
> > On 02/10/14 16:58, Charles Arnold wrote:
> >> Allow certain xentop columns to automatically expand as the amount
> >> of data reported gets larger.  The columns allowed to expand are:
> >>
> >> NETTX(k), NETRX(k), VBD_RD, VBD_WR, VBD_RSECT, VBD_WSECT
> >>
> >> Author: Markus Hauschild <Markus.Hauschild@rz.uni-regensburg.de>
> >> Signed-off-by: Charles Arnold <carnold@suse.com>
> > 
> > In principle, very nice.  (I have wanted to see about doing this for a
> > while now, but very far down the todo list)
> > 
> > How about the NAME field? 9 characters isn't enough for some people.
> 
> Sure, and any others that might need it.  But what to do about the '-f' flag
> which says show me the full VM name but the flag doesn't adjust the entire
> column.  By automatically showing the full name and adjusting the column
> appropriately it makes this flag pointless (which I'm ok with). 

Perhaps this functionality should be under that option?

I am a bit hesistant about this as there are some users of xentop that
use it for their monitoring. What I can't remember is if they use the
batched mode or not - and if they scan for specific strings (and length).

This would (I think?) throw a wrench in that?

> 
> - Charles
> 
> >>
> >> diff --git a/tools/xenstat/xentop/xentop.c b/tools/xenstat/xentop/xentop.c
> >> index dd11927..d087665 100644
> >> --- a/tools/xenstat/xentop/xentop.c
> >> +++ b/tools/xenstat/xentop/xentop.c
> >> @@ -27,6 +27,7 @@
> >>  
> >>  #include <ctype.h>
> >>  #include <errno.h>
> >> +#include <math.h>
> >>  #include <stdio.h>
> >>  #include <stdlib.h>
> >>  #include <stdarg.h>
> >> @@ -127,7 +128,8 @@ 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 void reset_field_widths(void);
> >> +static void adjust_field_widths(xenstat_domain *domain);
> >>  
> >>  /* Section printing functions */
> >>  static void do_summary(void);
> >> @@ -623,7 +625,7 @@ static int compare_net_tx(xenstat_domain *domain1, 
> > xenstat_domain *domain2)
> >>  /* Prints number of total network tx bytes statistic */
> >>  static void print_net_tx(xenstat_domain *domain)
> >>  {
> >> -	print("%8llu", tot_net_bytes(domain, FALSE)/1024);
> >> +	print("%*llu", fields[FIELD_NET_TX-1].default_width, tot_net_bytes(domain, 
> > FALSE)/1024);
> >>  }
> >>  
> >>  /* Compares number of total network rx bytes of two domains, returning 
> > -1,0,1
> >> @@ -637,7 +639,7 @@ static int compare_net_rx(xenstat_domain *domain1, 
> > xenstat_domain *domain2)
> >>  /* Prints number of total network rx bytes statistic */
> >>  static void print_net_rx(xenstat_domain *domain)
> >>  {
> >> -	print("%8llu", tot_net_bytes(domain, TRUE)/1024);
> >> +	print("%*llu", fields[FIELD_NET_RX-1].default_width, tot_net_bytes(domain, 
> > TRUE)/1024);
> >>  }
> >>  
> >>  /* Gets number of total network bytes statistic, if rx true, then rx bytes
> >> @@ -705,7 +707,7 @@ static int compare_vbd_rd(xenstat_domain *domain1, 
> > xenstat_domain *domain2)
> >>  /* Prints number of total VBD READ requests statistic */
> >>  static void print_vbd_rd(xenstat_domain *domain)
> >>  {
> >> -	print("%8llu", tot_vbd_reqs(domain, FIELD_VBD_RD));
> >> +	print("%*llu", fields[FIELD_VBD_RD-1].default_width, tot_vbd_reqs(domain, 
> > FIELD_VBD_RD));
> >>  }
> >>  
> >>  /* Compares number of total VBD WRITE requests of two domains,
> >> @@ -719,7 +721,7 @@ static int compare_vbd_wr(xenstat_domain *domain1, 
> > xenstat_domain *domain2)
> >>  /* Prints number of total VBD WRITE requests statistic */
> >>  static void print_vbd_wr(xenstat_domain *domain)
> >>  {
> >> -	print("%8llu", tot_vbd_reqs(domain, FIELD_VBD_WR));
> >> +	print("%*llu", fields[FIELD_VBD_WR-1].default_width, tot_vbd_reqs(domain, 
> > FIELD_VBD_WR));
> >>  }
> >>  
> >>  /* Compares number of total VBD READ sectors of two domains,
> >> @@ -733,7 +735,7 @@ static int compare_vbd_rsect(xenstat_domain *domain1, 
> > xenstat_domain *domain2)
> >>  /* Prints number of total VBD READ sectors statistic */
> >>  static void print_vbd_rsect(xenstat_domain *domain)
> >>  {
> >> -	print("%10llu", tot_vbd_reqs(domain, FIELD_VBD_RSECT));
> >> +	print("%*llu", fields[FIELD_VBD_RSECT-1].default_width, tot_vbd_reqs(domain, 
> > FIELD_VBD_RSECT));
> >>  }
> >>  
> >>  /* Compares number of total VBD WRITE sectors of two domains,
> >> @@ -747,7 +749,7 @@ static int compare_vbd_wsect(xenstat_domain *domain1, 
> > xenstat_domain *domain2)
> >>  /* Prints number of total VBD WRITE sectors statistic */
> >>  static void print_vbd_wsect(xenstat_domain *domain)
> >>  {
> >> -	print("%10llu", tot_vbd_reqs(domain, FIELD_VBD_WSECT));
> >> +	print("%*llu", fields[FIELD_VBD_WSECT-1].default_width, tot_vbd_reqs(domain, 
> > FIELD_VBD_WSECT));
> >>  }
> >>  
> >>  
> >> @@ -806,6 +808,48 @@ static void print_ssid(xenstat_domain *domain)
> >>  	print("%4u", xenstat_domain_ssid(domain));
> >>  }
> >>  
> >> +/* Resets default_width for fields with potentially large numbers */
> >> +void reset_field_widths(void)
> >> +{
> >> +	fields[FIELD_NET_TX-1].default_width = 8;
> >> +	fields[FIELD_NET_RX-1].default_width = 8;
> >> +	fields[FIELD_VBD_RD-1].default_width = 8;
> >> +	fields[FIELD_VBD_WR-1].default_width = 8;
> >> +	fields[FIELD_VBD_RSECT-1].default_width = 10;
> >> +	fields[FIELD_VBD_WSECT-1].default_width = 10;
> >> +}
> >> +
> >> +/* Adjusts default_width for fields with potentially large numbers */
> >> +void adjust_field_widths(xenstat_domain *domain)
> >> +{
> >> +	unsigned int length;
> >> +
> >> +	length = (unsigned int)(log10(tot_net_bytes(domain, FALSE)/1024) + 1);
> >> +	if (length > fields[FIELD_NET_TX-1].default_width)
> >> +		fields[FIELD_NET_TX-1].default_width = length;
> >> +
> >> +	length = (unsigned int)(log10(tot_net_bytes(domain, TRUE)/1024) + 1);
> >> +	if (length > fields[FIELD_NET_RX-1].default_width)
> >> +		fields[FIELD_NET_RX-1].default_width = length;
> >> +
> >> +	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_RD)) + 1);
> >> +	if (length > fields[FIELD_VBD_RD-1].default_width)
> >> +		fields[FIELD_VBD_RD-1].default_width = length;
> >> +
> >> +	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_WR)) + 1);
> >> +	if (length > fields[FIELD_VBD_WR-1].default_width)
> >> +		fields[FIELD_VBD_WR-1].default_width = length;
> >> +
> >> +	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_RSECT)) + 1);
> >> +	if (length > fields[FIELD_VBD_RSECT-1].default_width)
> >> +		fields[FIELD_VBD_RSECT-1].default_width = length;
> >> +
> >> +	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_WSECT)) + 1);
> >> +	if (length > fields[FIELD_VBD_WSECT-1].default_width)
> >> +		fields[FIELD_VBD_WSECT-1].default_width = length;
> >> +}
> >> +
> >> +
> >>  /* Section printing functions */
> >>  /* Prints the top summary, above the domain table */
> >>  void do_summary(void)
> >> @@ -1088,6 +1132,12 @@ static void top(void)
> >>  	if(first_domain_index >= num_domains)
> >>  		first_domain_index = num_domains-1;
> >>  
> >> +	/* Adjust default_width for fields with potentially large numbers */
> >> +	reset_field_widths();
> >> +	for (i = first_domain_index; i < num_domains; i++) {
> >> +		adjust_field_widths(domains[i]);
> >> +	}
> >> +
> >>  	for (i = first_domain_index; i < num_domains; i++) {
> >>  		if(!batch && current_row() == lines()-1)
> >>  			break;
> >>
> >>
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@lists.xen.org
> >> http://lists.xen.org/xen-devel
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH] xentop: Dynamically expand some columns

[Charles Arnold]

>>> On 10/2/2014 at 11:05 AM, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:

> On Thu, Oct 02, 2014 at 10:25:41AM -0600, Charles Arnold wrote:
>> >>> On 10/2/2014 at 10:10 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote: 
>> > On 02/10/14 16:58, Charles Arnold wrote:
>> >> Allow certain xentop columns to automatically expand as the amount
>> >> of data reported gets larger.  The columns allowed to expand are:
>> >>
>> >> NETTX(k), NETRX(k), VBD_RD, VBD_WR, VBD_RSECT, VBD_WSECT
>> >>
>> >> Author: Markus Hauschild <Markus.Hauschild@rz.uni-regensburg.de>
>> >> Signed-off-by: Charles Arnold <carnold@suse.com>
>> > 
>> > In principle, very nice.  (I have wanted to see about doing this for a
>> > while now, but very far down the todo list)
>> > 
>> > How about the NAME field? 9 characters isn't enough for some people.
>> 
>> Sure, and any others that might need it.  But what to do about the '-f' flag
>> which says show me the full VM name but the flag doesn't adjust the entire
>> column.  By automatically showing the full name and adjusting the column
>> appropriately it makes this flag pointless (which I'm ok with). 
> 
> Perhaps this functionality should be under that option?
> 
> I am a bit hesistant about this as there are some users of xentop that
> use it for their monitoring. What I can't remember is if they use the
> batched mode or not - and if they scan for specific strings (and length).

Right.  The original patch came from a customer doing the monitoring
you describe.  In their case, they originally wanted the columns to be
aligned no matter how big the data got and to know that there would be
at least one white space between values.

> 
> This would (I think?) throw a wrench in that?

If someone were counting on there being an exact number of spaces for each column
then the patch might be a problem - but - they would still have an issue with the
existing code if for example the NET_TX data exceeded the hardcoded default width of
the column.  If they only care about white space separating the values then they should
be fine with this change.

If you think this is ok, I'll post another patch that includes the expansion for the VM name.
Or alternately post a patch that only automatically expands the VM name if '-f' is passed
(which allows the flag to continue to be relevant).

- Charles

> 
>> 
>> - Charles
>> 
>> >>
>> >> diff --git a/tools/xenstat/xentop/xentop.c b/tools/xenstat/xentop/xentop.c
>> >> index dd11927..d087665 100644
>> >> --- a/tools/xenstat/xentop/xentop.c
>> >> +++ b/tools/xenstat/xentop/xentop.c
>> >> @@ -27,6 +27,7 @@
>> >>  
>> >>  #include <ctype.h>
>> >>  #include <errno.h>
>> >> +#include <math.h>
>> >>  #include <stdio.h>
>> >>  #include <stdlib.h>
>> >>  #include <stdarg.h>
>> >> @@ -127,7 +128,8 @@ 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 void reset_field_widths(void);
>> >> +static void adjust_field_widths(xenstat_domain *domain);
>> >>  
>> >>  /* Section printing functions */
>> >>  static void do_summary(void);
>> >> @@ -623,7 +625,7 @@ static int compare_net_tx(xenstat_domain *domain1, 
>> > xenstat_domain *domain2)
>> >>  /* Prints number of total network tx bytes statistic */
>> >>  static void print_net_tx(xenstat_domain *domain)
>> >>  {
>> >> -	print("%8llu", tot_net_bytes(domain, FALSE)/1024);
>> >> +	print("%*llu", fields[FIELD_NET_TX-1].default_width, tot_net_bytes(domain, 
>> > FALSE)/1024);
>> >>  }
>> >>  
>> >>  /* Compares number of total network rx bytes of two domains, returning 
>> > -1,0,1
>> >> @@ -637,7 +639,7 @@ static int compare_net_rx(xenstat_domain *domain1, 
>> > xenstat_domain *domain2)
>> >>  /* Prints number of total network rx bytes statistic */
>> >>  static void print_net_rx(xenstat_domain *domain)
>> >>  {
>> >> -	print("%8llu", tot_net_bytes(domain, TRUE)/1024);
>> >> +	print("%*llu", fields[FIELD_NET_RX-1].default_width, tot_net_bytes(domain, 
>> > TRUE)/1024);
>> >>  }
>> >>  
>> >>  /* Gets number of total network bytes statistic, if rx true, then rx bytes
>> >> @@ -705,7 +707,7 @@ static int compare_vbd_rd(xenstat_domain *domain1, 
>> > xenstat_domain *domain2)
>> >>  /* Prints number of total VBD READ requests statistic */
>> >>  static void print_vbd_rd(xenstat_domain *domain)
>> >>  {
>> >> -	print("%8llu", tot_vbd_reqs(domain, FIELD_VBD_RD));
>> >> +	print("%*llu", fields[FIELD_VBD_RD-1].default_width, tot_vbd_reqs(domain, 
>> > FIELD_VBD_RD));
>> >>  }
>> >>  
>> >>  /* Compares number of total VBD WRITE requests of two domains,
>> >> @@ -719,7 +721,7 @@ static int compare_vbd_wr(xenstat_domain *domain1, 
>> > xenstat_domain *domain2)
>> >>  /* Prints number of total VBD WRITE requests statistic */
>> >>  static void print_vbd_wr(xenstat_domain *domain)
>> >>  {
>> >> -	print("%8llu", tot_vbd_reqs(domain, FIELD_VBD_WR));
>> >> +	print("%*llu", fields[FIELD_VBD_WR-1].default_width, tot_vbd_reqs(domain, 
>> > FIELD_VBD_WR));
>> >>  }
>> >>  
>> >>  /* Compares number of total VBD READ sectors of two domains,
>> >> @@ -733,7 +735,7 @@ static int compare_vbd_rsect(xenstat_domain *domain1, 
>> > xenstat_domain *domain2)
>> >>  /* Prints number of total VBD READ sectors statistic */
>> >>  static void print_vbd_rsect(xenstat_domain *domain)
>> >>  {
>> >> -	print("%10llu", tot_vbd_reqs(domain, FIELD_VBD_RSECT));
>> >> +	print("%*llu", fields[FIELD_VBD_RSECT-1].default_width, 
> tot_vbd_reqs(domain, 
>> > FIELD_VBD_RSECT));
>> >>  }
>> >>  
>> >>  /* Compares number of total VBD WRITE sectors of two domains,
>> >> @@ -747,7 +749,7 @@ static int compare_vbd_wsect(xenstat_domain *domain1, 
>> > xenstat_domain *domain2)
>> >>  /* Prints number of total VBD WRITE sectors statistic */
>> >>  static void print_vbd_wsect(xenstat_domain *domain)
>> >>  {
>> >> -	print("%10llu", tot_vbd_reqs(domain, FIELD_VBD_WSECT));
>> >> +	print("%*llu", fields[FIELD_VBD_WSECT-1].default_width, 
> tot_vbd_reqs(domain, 
>> > FIELD_VBD_WSECT));
>> >>  }
>> >>  
>> >>  
>> >> @@ -806,6 +808,48 @@ static void print_ssid(xenstat_domain *domain)
>> >>  	print("%4u", xenstat_domain_ssid(domain));
>> >>  }
>> >>  
>> >> +/* Resets default_width for fields with potentially large numbers */
>> >> +void reset_field_widths(void)
>> >> +{
>> >> +	fields[FIELD_NET_TX-1].default_width = 8;
>> >> +	fields[FIELD_NET_RX-1].default_width = 8;
>> >> +	fields[FIELD_VBD_RD-1].default_width = 8;
>> >> +	fields[FIELD_VBD_WR-1].default_width = 8;
>> >> +	fields[FIELD_VBD_RSECT-1].default_width = 10;
>> >> +	fields[FIELD_VBD_WSECT-1].default_width = 10;
>> >> +}
>> >> +
>> >> +/* Adjusts default_width for fields with potentially large numbers */
>> >> +void adjust_field_widths(xenstat_domain *domain)
>> >> +{
>> >> +	unsigned int length;
>> >> +
>> >> +	length = (unsigned int)(log10(tot_net_bytes(domain, FALSE)/1024) + 1);
>> >> +	if (length > fields[FIELD_NET_TX-1].default_width)
>> >> +		fields[FIELD_NET_TX-1].default_width = length;
>> >> +
>> >> +	length = (unsigned int)(log10(tot_net_bytes(domain, TRUE)/1024) + 1);
>> >> +	if (length > fields[FIELD_NET_RX-1].default_width)
>> >> +		fields[FIELD_NET_RX-1].default_width = length;
>> >> +
>> >> +	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_RD)) + 1);
>> >> +	if (length > fields[FIELD_VBD_RD-1].default_width)
>> >> +		fields[FIELD_VBD_RD-1].default_width = length;
>> >> +
>> >> +	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_WR)) + 1);
>> >> +	if (length > fields[FIELD_VBD_WR-1].default_width)
>> >> +		fields[FIELD_VBD_WR-1].default_width = length;
>> >> +
>> >> +	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_RSECT)) + 1);
>> >> +	if (length > fields[FIELD_VBD_RSECT-1].default_width)
>> >> +		fields[FIELD_VBD_RSECT-1].default_width = length;
>> >> +
>> >> +	length = (unsigned int)(log10(tot_vbd_reqs(domain, FIELD_VBD_WSECT)) + 1);
>> >> +	if (length > fields[FIELD_VBD_WSECT-1].default_width)
>> >> +		fields[FIELD_VBD_WSECT-1].default_width = length;
>> >> +}
>> >> +
>> >> +
>> >>  /* Section printing functions */
>> >>  /* Prints the top summary, above the domain table */
>> >>  void do_summary(void)
>> >> @@ -1088,6 +1132,12 @@ static void top(void)
>> >>  	if(first_domain_index >= num_domains)
>> >>  		first_domain_index = num_domains-1;
>> >>  
>> >> +	/* Adjust default_width for fields with potentially large numbers */
>> >> +	reset_field_widths();
>> >> +	for (i = first_domain_index; i < num_domains; i++) {
>> >> +		adjust_field_widths(domains[i]);
>> >> +	}
>> >> +
>> >>  	for (i = first_domain_index; i < num_domains; i++) {
>> >>  		if(!batch && current_row() == lines()-1)
>> >>  			break;
>> >>
>> >>
>> >> _______________________________________________
>> >> Xen-devel mailing list
>> >> Xen-devel@lists.xen.org
>> >> http://lists.xen.org/xen-devel
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel

Re: [PATCH] xentop: Dynamically expand some columns

[Ian Campbell]

On Thu, 2014-10-02 at 13:05 -0400, Konrad Rzeszutek Wilk wrote:
> On Thu, Oct 02, 2014 at 10:25:41AM -0600, Charles Arnold wrote:
> > >>> On 10/2/2014 at 10:10 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote: 
> > > On 02/10/14 16:58, Charles Arnold wrote:
> > >> Allow certain xentop columns to automatically expand as the amount
> > >> of data reported gets larger.  The columns allowed to expand are:
> > >>
> > >> NETTX(k), NETRX(k), VBD_RD, VBD_WR, VBD_RSECT, VBD_WSECT
> > >>
> > >> Author: Markus Hauschild <Markus.Hauschild@rz.uni-regensburg.de>
> > >> Signed-off-by: Charles Arnold <carnold@suse.com>
> > > 
> > > In principle, very nice.  (I have wanted to see about doing this for a
> > > while now, but very far down the todo list)
> > > 
> > > How about the NAME field? 9 characters isn't enough for some people.
> > 
> > Sure, and any others that might need it.  But what to do about the '-f' flag
> > which says show me the full VM name but the flag doesn't adjust the entire
> > column.  By automatically showing the full name and adjusting the column
> > appropriately it makes this flag pointless (which I'm ok with). 
> 
> Perhaps this functionality should be under that option?
> 
> I am a bit hesistant about this as there are some users of xentop that
> use it for their monitoring. What I can't remember is if they use the
> batched mode or not - and if they scan for specific strings (and length).
> 
> This would (I think?) throw a wrench in that?

IMHO people who are parsing the output of tools such as this ought to be
prepared to deal with occasional changes in the precise content of the
output across Xen releases.

If they aren't happy with that then they should file wishlist bugs (or
better: patches) asking for an extensible/machine-readable format to be
output upon request, or for libxenstat to be exposed and made API
stable, or some other functionality which solves their need.

Otherwise we find ourselves in a position where useful patches like
Charles' and similar patches which add genuinely useful output for human
readers can never be accepted.

Ian.

Re: [PATCH] xentop: Dynamically expand some columns

[Konrad Rzeszutek Wilk]

On Fri, Oct 03, 2014 at 10:00:30AM +0100, Ian Campbell wrote:
> On Thu, 2014-10-02 at 13:05 -0400, Konrad Rzeszutek Wilk wrote:
> > On Thu, Oct 02, 2014 at 10:25:41AM -0600, Charles Arnold wrote:
> > > >>> On 10/2/2014 at 10:10 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote: 
> > > > On 02/10/14 16:58, Charles Arnold wrote:
> > > >> Allow certain xentop columns to automatically expand as the amount
> > > >> of data reported gets larger.  The columns allowed to expand are:
> > > >>
> > > >> NETTX(k), NETRX(k), VBD_RD, VBD_WR, VBD_RSECT, VBD_WSECT
> > > >>
> > > >> Author: Markus Hauschild <Markus.Hauschild@rz.uni-regensburg.de>
> > > >> Signed-off-by: Charles Arnold <carnold@suse.com>
> > > > 
> > > > In principle, very nice.  (I have wanted to see about doing this for a
> > > > while now, but very far down the todo list)
> > > > 
> > > > How about the NAME field? 9 characters isn't enough for some people.
> > > 
> > > Sure, and any others that might need it.  But what to do about the '-f' flag
> > > which says show me the full VM name but the flag doesn't adjust the entire
> > > column.  By automatically showing the full name and adjusting the column
> > > appropriately it makes this flag pointless (which I'm ok with). 
> > 
> > Perhaps this functionality should be under that option?
> > 
> > I am a bit hesistant about this as there are some users of xentop that
> > use it for their monitoring. What I can't remember is if they use the
> > batched mode or not - and if they scan for specific strings (and length).
> > 
> > This would (I think?) throw a wrench in that?
> 
> IMHO people who are parsing the output of tools such as this ought to be
> prepared to deal with occasional changes in the precise content of the
> output across Xen releases.
> 
> If they aren't happy with that then they should file wishlist bugs (or
> better: patches) asking for an extensible/machine-readable format to be
> output upon request, or for libxenstat to be exposed and made API
> stable, or some other functionality which solves their need.
> 
> Otherwise we find ourselves in a position where useful patches like
> Charles' and similar patches which add genuinely useful output for human
> readers can never be accepted.

I dug up why -f was added, and this:

mit 422f39c72e1a9890636a770cb93de05b93f0e605
Author: Keir Fraser <keir.fraser@citrix.com>
Date:   Wed Dec 2 18:43:28 2009 +0000
 xentop: Allow full domain name display
    
    Add a '-f' option to xentop to allow the full domain name to be
    displayed. This is the original behavior which can cause the display
    to be unaligned. Customers have requested this because only the
    trailing characters of their domain names are unique and therefore
    cannot be distinguished when the display is limited to a 10 character
    width.
    
    Signed-off-by: Charles Arnold <carnold@novell.com>

OK, so this is all about 'aligment' of columns - and wanting to preserve
that without the usage '-f'.

Looking over your v2 patch it seems that it would keep that aligment in
check. That is if the aligment MUST change (one of the guests has a huge amount
of data), then ALL of the fields for ALL guests change their aligment?

If that is the case, then V2 is the proper way to go.

Re: [PATCH] xentop: Dynamically expand some columns

[Charles Arnold]

>>> On 10/3/2014 at 03:00 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote: 
> On Thu, 2014-10-02 at 13:05 -0400, Konrad Rzeszutek Wilk wrote:
>> On Thu, Oct 02, 2014 at 10:25:41AM -0600, Charles Arnold wrote:
>> > >>> On 10/2/2014 at 10:10 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote: 
>> > > On 02/10/14 16:58, Charles Arnold wrote:
>> > >> Allow certain xentop columns to automatically expand as the amount
>> > >> of data reported gets larger.  The columns allowed to expand are:
>> > >>
>> > >> NETTX(k), NETRX(k), VBD_RD, VBD_WR, VBD_RSECT, VBD_WSECT
>> > >>
>> > >> Author: Markus Hauschild <Markus.Hauschild@rz.uni-regensburg.de>
>> > >> Signed-off-by: Charles Arnold <carnold@suse.com>
>> > > 
>> > > In principle, very nice.  (I have wanted to see about doing this for a
>> > > while now, but very far down the todo list)
>> > > 
>> > > How about the NAME field? 9 characters isn't enough for some people.
>> > 
>> > Sure, and any others that might need it.  But what to do about the '-f' 
> flag
>> > which says show me the full VM name but the flag doesn't adjust the entire
>> > column.  By automatically showing the full name and adjusting the column
>> > appropriately it makes this flag pointless (which I'm ok with). 
>> 
>> Perhaps this functionality should be under that option?
>> 
>> I am a bit hesistant about this as there are some users of xentop that
>> use it for their monitoring. What I can't remember is if they use the
>> batched mode or not - and if they scan for specific strings (and length).
>> 
>> This would (I think?) throw a wrench in that?
> 
> IMHO people who are parsing the output of tools such as this ought to be
> prepared to deal with occasional changes in the precise content of the
> output across Xen releases.
> 
> If they aren't happy with that then they should file wishlist bugs (or
> better: patches) asking for an extensible/machine-readable format to be
> output upon request, or for libxenstat to be exposed and made API
> stable, or some other functionality which solves their need.
> 
> Otherwise we find ourselves in a position where useful patches like
> Charles' and similar patches which add genuinely useful output for human
> readers can never be accepted.

So do we get a thumbs up on this?  If so can someone take the patch (I prefer v3 which aligns the name if -f is used, but v2 would satisfy the customer).

- Charles

Re: [PATCH] xentop: Dynamically expand some columns

[Konrad Rzeszutek Wilk]

On Mon, Oct 13, 2014 at 11:12:39AM -0600, Charles Arnold wrote:
> >>> On 10/3/2014 at 03:00 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote: 
> > On Thu, 2014-10-02 at 13:05 -0400, Konrad Rzeszutek Wilk wrote:
> >> On Thu, Oct 02, 2014 at 10:25:41AM -0600, Charles Arnold wrote:
> >> > >>> On 10/2/2014 at 10:10 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote: 
> >> > > On 02/10/14 16:58, Charles Arnold wrote:
> >> > >> Allow certain xentop columns to automatically expand as the amount
> >> > >> of data reported gets larger.  The columns allowed to expand are:
> >> > >>
> >> > >> NETTX(k), NETRX(k), VBD_RD, VBD_WR, VBD_RSECT, VBD_WSECT
> >> > >>
> >> > >> Author: Markus Hauschild <Markus.Hauschild@rz.uni-regensburg.de>
> >> > >> Signed-off-by: Charles Arnold <carnold@suse.com>
> >> > > 
> >> > > In principle, very nice.  (I have wanted to see about doing this for a
> >> > > while now, but very far down the todo list)
> >> > > 
> >> > > How about the NAME field? 9 characters isn't enough for some people.
> >> > 
> >> > Sure, and any others that might need it.  But what to do about the '-f' 
> > flag
> >> > which says show me the full VM name but the flag doesn't adjust the entire
> >> > column.  By automatically showing the full name and adjusting the column
> >> > appropriately it makes this flag pointless (which I'm ok with). 
> >> 
> >> Perhaps this functionality should be under that option?
> >> 
> >> I am a bit hesistant about this as there are some users of xentop that
> >> use it for their monitoring. What I can't remember is if they use the
> >> batched mode or not - and if they scan for specific strings (and length).
> >> 
> >> This would (I think?) throw a wrench in that?
> > 
> > IMHO people who are parsing the output of tools such as this ought to be
> > prepared to deal with occasional changes in the precise content of the
> > output across Xen releases.
> > 
> > If they aren't happy with that then they should file wishlist bugs (or
> > better: patches) asking for an extensible/machine-readable format to be
> > output upon request, or for libxenstat to be exposed and made API
> > stable, or some other functionality which solves their need.
> > 
> > Otherwise we find ourselves in a position where useful patches like
> > Charles' and similar patches which add genuinely useful output for human
> > readers can never be accepted.
> 
> So do we get a thumbs up on this?  If so can someone take the patch (I prefer v3 which aligns the name if -f is used, but v2 would satisfy the customer).

I am OK with either patch, so Released-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
on either v2 or v3.


> 
> - Charles
> 
> 

Re: [PATCH] xentop: Dynamically expand some columns

[Charles Arnold]

>>> On 10/13/2014 at 02:09 PM, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
wrote: 
> On Mon, Oct 13, 2014 at 11:12:39AM -0600, Charles Arnold wrote:
>> >>> On 10/3/2014 at 03:00 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote: 
>> > On Thu, 2014-10-02 at 13:05 -0400, Konrad Rzeszutek Wilk wrote:
>> >> On Thu, Oct 02, 2014 at 10:25:41AM -0600, Charles Arnold wrote:
>> >> > >>> On 10/2/2014 at 10:10 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote: 
>> >> > > On 02/10/14 16:58, Charles Arnold wrote:
>> >> > >> Allow certain xentop columns to automatically expand as the amount
>> >> > >> of data reported gets larger.  The columns allowed to expand are:
>> >> > >>
>> >> > >> NETTX(k), NETRX(k), VBD_RD, VBD_WR, VBD_RSECT, VBD_WSECT
>> >> > >>
>> >> > >> Author: Markus Hauschild <Markus.Hauschild@rz.uni-regensburg.de>
>> >> > >> Signed-off-by: Charles Arnold <carnold@suse.com>
>> >> > > 
>> >> > > In principle, very nice.  (I have wanted to see about doing this for a
>> >> > > while now, but very far down the todo list)
>> >> > > 
>> >> > > How about the NAME field? 9 characters isn't enough for some people.
>> >> > 
>> >> > Sure, and any others that might need it.  But what to do about the '-f' 
>> > flag
>> >> > which says show me the full VM name but the flag doesn't adjust the entire
>> >> > column.  By automatically showing the full name and adjusting the column
>> >> > appropriately it makes this flag pointless (which I'm ok with). 
>> >> 
>> >> Perhaps this functionality should be under that option?
>> >> 
>> >> I am a bit hesistant about this as there are some users of xentop that
>> >> use it for their monitoring. What I can't remember is if they use the
>> >> batched mode or not - and if they scan for specific strings (and length).
>> >> 
>> >> This would (I think?) throw a wrench in that?
>> > 
>> > IMHO people who are parsing the output of tools such as this ought to be
>> > prepared to deal with occasional changes in the precise content of the
>> > output across Xen releases.
>> > 
>> > If they aren't happy with that then they should file wishlist bugs (or
>> > better: patches) asking for an extensible/machine-readable format to be
>> > output upon request, or for libxenstat to be exposed and made API
>> > stable, or some other functionality which solves their need.
>> > 
>> > Otherwise we find ourselves in a position where useful patches like
>> > Charles' and similar patches which add genuinely useful output for human
>> > readers can never be accepted.
>> 
>> So do we get a thumbs up on this?  If so can someone take the patch (I 
> prefer v3 which aligns the name if -f is used, but v2 would satisfy the 
> customer).
> 
> I am OK with either patch, so Released-Acked-by: Konrad Rzeszutek Wilk 
> <konrad.wilk@oracle.com>
> on either v2 or v3.

Ian, what do you say?

[PATCH] xentop: Dynamically expand some columns

[Markus Hauschild]

Allow certain xentop columns to automatically expand as the amount
of data reported gets larger.  The columns allowed to auto expand are:

NETTX(k), NETRX(k), VBD_RD, VBD_WR, VBD_RSECT, VBD_WSECT

If the -f option is used to allow full length VM names, those names will
also be aligned based on the longest name in the NAME column.

The default minimum width of all columns remains unchanged.

Signed-off-by: Markus Hauschild <Markus.Hauschild@rz.uni-regensburg.de>
Signed-off-by: Charles Arnold <carnold@suse.com>

diff --git a/tools/xenstat/xentop/Makefile b/tools/xenstat/xentop/Makefile
index 18bccb6..076e44c 100644
--- a/tools/xenstat/xentop/Makefile
+++ b/tools/xenstat/xentop/Makefile
@@ -19,7 +19,7 @@ all install xentop:
 else
 
 CFLAGS += -DGCC_PRINTF -Werror $(CFLAGS_libxenstat)
-LDLIBS += $(LDLIBS_libxenstat) $(CURSES_LIBS) $(SOCKET_LIBS)
+LDLIBS += $(LDLIBS_libxenstat) $(CURSES_LIBS) $(SOCKET_LIBS) -lm
 CFLAGS += -DHOST_$(XEN_OS)
 
 # Include configure output (config.h) to headers search path
diff --git a/tools/xenstat/xentop/xentop.c b/tools/xenstat/xentop/xentop.c
index dd11927..3062cb5 100644
--- a/tools/xenstat/xentop/xentop.c
+++ b/tools/xenstat/xentop/xentop.c
@@ -27,6 +27,7 @@
 
 #include <ctype.h>
 #include <errno.h>
+#include <math.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdarg.h>
@@ -70,6 +71,8 @@
 #define curses_str_t const char *
 #endif
 
+#define INT_FIELD_WIDTH(n) ((unsigned int)(log10(n) + 1))
+
 /*
  * Function prototypes
  */
@@ -127,7 +130,8 @@ 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 void reset_field_widths(void);
+static void adjust_field_widths(xenstat_domain *domain);
 
 /* Section printing functions */
 static void do_summary(void);
@@ -444,7 +448,7 @@ int compare_name(xenstat_domain *domain1, xenstat_domain *domain2)
 void print_name(xenstat_domain *domain)
 {
 	if(show_full_name)
-		print("%10s", xenstat_domain_name(domain));
+		print("%*s", fields[FIELD_NAME-1].default_width, xenstat_domain_name(domain));
 	else
 		print("%10.10s", xenstat_domain_name(domain));
 }
@@ -623,7 +627,7 @@ static int compare_net_tx(xenstat_domain *domain1, xenstat_domain *domain2)
 /* Prints number of total network tx bytes statistic */
 static void print_net_tx(xenstat_domain *domain)
 {
-	print("%8llu", tot_net_bytes(domain, FALSE)/1024);
+	print("%*llu", fields[FIELD_NET_TX-1].default_width, tot_net_bytes(domain, FALSE)/1024);
 }
 
 /* Compares number of total network rx bytes of two domains, returning -1,0,1
@@ -637,7 +641,7 @@ static int compare_net_rx(xenstat_domain *domain1, xenstat_domain *domain2)
 /* Prints number of total network rx bytes statistic */
 static void print_net_rx(xenstat_domain *domain)
 {
-	print("%8llu", tot_net_bytes(domain, TRUE)/1024);
+	print("%*llu", fields[FIELD_NET_RX-1].default_width, tot_net_bytes(domain, TRUE)/1024);
 }
 
 /* Gets number of total network bytes statistic, if rx true, then rx bytes
@@ -705,7 +709,7 @@ static int compare_vbd_rd(xenstat_domain *domain1, xenstat_domain *domain2)
 /* Prints number of total VBD READ requests statistic */
 static void print_vbd_rd(xenstat_domain *domain)
 {
-	print("%8llu", tot_vbd_reqs(domain, FIELD_VBD_RD));
+	print("%*llu", fields[FIELD_VBD_RD-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_RD));
 }
 
 /* Compares number of total VBD WRITE requests of two domains,
@@ -719,7 +723,7 @@ static int compare_vbd_wr(xenstat_domain *domain1, xenstat_domain *domain2)
 /* Prints number of total VBD WRITE requests statistic */
 static void print_vbd_wr(xenstat_domain *domain)
 {
-	print("%8llu", tot_vbd_reqs(domain, FIELD_VBD_WR));
+	print("%*llu", fields[FIELD_VBD_WR-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_WR));
 }
 
 /* Compares number of total VBD READ sectors of two domains,
@@ -733,7 +737,7 @@ static int compare_vbd_rsect(xenstat_domain *domain1, xenstat_domain *domain2)
 /* Prints number of total VBD READ sectors statistic */
 static void print_vbd_rsect(xenstat_domain *domain)
 {
-	print("%10llu", tot_vbd_reqs(domain, FIELD_VBD_RSECT));
+	print("%*llu", fields[FIELD_VBD_RSECT-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_RSECT));
 }
 
 /* Compares number of total VBD WRITE sectors of two domains,
@@ -747,7 +751,7 @@ static int compare_vbd_wsect(xenstat_domain *domain1, xenstat_domain *domain2)
 /* Prints number of total VBD WRITE sectors statistic */
 static void print_vbd_wsect(xenstat_domain *domain)
 {
-	print("%10llu", tot_vbd_reqs(domain, FIELD_VBD_WSECT));
+	print("%*llu", fields[FIELD_VBD_WSECT-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_WSECT));
 }
 
 
@@ -806,6 +810,54 @@ static void print_ssid(xenstat_domain *domain)
 	print("%4u", xenstat_domain_ssid(domain));
 }
 
+/* Resets default_width for fields with potentially large numbers */
+void reset_field_widths(void)
+{
+	fields[FIELD_NET_TX-1].default_width = 8;
+	fields[FIELD_NET_RX-1].default_width = 8;
+	fields[FIELD_VBD_RD-1].default_width = 8;
+	fields[FIELD_VBD_WR-1].default_width = 8;
+	fields[FIELD_VBD_RSECT-1].default_width = 10;
+	fields[FIELD_VBD_WSECT-1].default_width = 10;
+}
+
+/* Adjusts default_width for fields with potentially large numbers */
+void adjust_field_widths(xenstat_domain *domain)
+{
+	unsigned int length;
+
+	if (show_full_name) {
+		length = strlen(xenstat_domain_name(domain));
+		if (length > fields[FIELD_NAME-1].default_width)
+			fields[FIELD_NAME-1].default_width = length;
+	}
+
+	length = INT_FIELD_WIDTH((tot_net_bytes(domain, FALSE)/1024) + 1);
+	if (length > fields[FIELD_NET_TX-1].default_width)
+		fields[FIELD_NET_TX-1].default_width = length;
+
+	length = INT_FIELD_WIDTH((tot_net_bytes(domain, TRUE)/1024) + 1);
+	if (length > fields[FIELD_NET_RX-1].default_width)
+		fields[FIELD_NET_RX-1].default_width = length;
+
+	length = INT_FIELD_WIDTH((tot_vbd_reqs(domain, FIELD_VBD_RD)) + 1);
+	if (length > fields[FIELD_VBD_RD-1].default_width)
+		fields[FIELD_VBD_RD-1].default_width = length;
+
+	length = INT_FIELD_WIDTH((tot_vbd_reqs(domain, FIELD_VBD_WR)) + 1);
+	if (length > fields[FIELD_VBD_WR-1].default_width)
+		fields[FIELD_VBD_WR-1].default_width = length;
+
+	length = INT_FIELD_WIDTH((tot_vbd_reqs(domain, FIELD_VBD_RSECT)) + 1);
+	if (length > fields[FIELD_VBD_RSECT-1].default_width)
+		fields[FIELD_VBD_RSECT-1].default_width = length;
+
+	length = INT_FIELD_WIDTH((tot_vbd_reqs(domain, FIELD_VBD_WSECT)) + 1);
+	if (length > fields[FIELD_VBD_WSECT-1].default_width)
+		fields[FIELD_VBD_WSECT-1].default_width = length;
+}
+
+
 /* Section printing functions */
 /* Prints the top summary, above the domain table */
 void do_summary(void)
@@ -1088,6 +1140,12 @@ static void top(void)
 	if(first_domain_index >= num_domains)
 		first_domain_index = num_domains-1;
 
+	/* Adjust default_width for fields with potentially large numbers */
+	reset_field_widths();
+	for (i = first_domain_index; i < num_domains; i++) {
+		adjust_field_widths(domains[i]);
+	}
+
 	for (i = first_domain_index; i < num_domains; i++) {
 		if(!batch && current_row() == lines()-1)
 			break;

Re: [PATCH] xentop: Dynamically expand some columns

[Ian Campbell]

On Wed, 2014-10-29 at 12:24 +0100, Markus Hauschild wrote:
> Allow certain xentop columns to automatically expand as the amount
> of data reported gets larger.  The columns allowed to auto expand are:
> 
> NETTX(k), NETRX(k), VBD_RD, VBD_WR, VBD_RSECT, VBD_WSECT
> 
> If the -f option is used to allow full length VM names, those names will
> also be aligned based on the longest name in the NAME column.
> 
> The default minimum width of all columns remains unchanged.
> 
> Signed-off-by: Markus Hauschild <Markus.Hauschild@rz.uni-regensburg.de>
> Signed-off-by: Charles Arnold <carnold@suse.com>

Acked-by: Ian Campbell <Ian.Campbell@citrix.com>

Konrad, I think you were previously intending this to go in for 4.5, is
that still the case?

> 
> diff --git a/tools/xenstat/xentop/Makefile b/tools/xenstat/xentop/Makefile
> index 18bccb6..076e44c 100644
> --- a/tools/xenstat/xentop/Makefile
> +++ b/tools/xenstat/xentop/Makefile
> @@ -19,7 +19,7 @@ all install xentop:
>  else
>  
>  CFLAGS += -DGCC_PRINTF -Werror $(CFLAGS_libxenstat)
> -LDLIBS += $(LDLIBS_libxenstat) $(CURSES_LIBS) $(SOCKET_LIBS)
> +LDLIBS += $(LDLIBS_libxenstat) $(CURSES_LIBS) $(SOCKET_LIBS) -lm
>  CFLAGS += -DHOST_$(XEN_OS)
>  
>  # Include configure output (config.h) to headers search path
> diff --git a/tools/xenstat/xentop/xentop.c b/tools/xenstat/xentop/xentop.c
> index dd11927..3062cb5 100644
> --- a/tools/xenstat/xentop/xentop.c
> +++ b/tools/xenstat/xentop/xentop.c
> @@ -27,6 +27,7 @@
>  
>  #include <ctype.h>
>  #include <errno.h>
> +#include <math.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <stdarg.h>
> @@ -70,6 +71,8 @@
>  #define curses_str_t const char *
>  #endif
>  
> +#define INT_FIELD_WIDTH(n) ((unsigned int)(log10(n) + 1))
> +
>  /*
>   * Function prototypes
>   */
> @@ -127,7 +130,8 @@ 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 void reset_field_widths(void);
> +static void adjust_field_widths(xenstat_domain *domain);
>  
>  /* Section printing functions */
>  static void do_summary(void);
> @@ -444,7 +448,7 @@ int compare_name(xenstat_domain *domain1, xenstat_domain *domain2)
>  void print_name(xenstat_domain *domain)
>  {
>  	if(show_full_name)
> -		print("%10s", xenstat_domain_name(domain));
> +		print("%*s", fields[FIELD_NAME-1].default_width, xenstat_domain_name(domain));
>  	else
>  		print("%10.10s", xenstat_domain_name(domain));
>  }
> @@ -623,7 +627,7 @@ static int compare_net_tx(xenstat_domain *domain1, xenstat_domain *domain2)
>  /* Prints number of total network tx bytes statistic */
>  static void print_net_tx(xenstat_domain *domain)
>  {
> -	print("%8llu", tot_net_bytes(domain, FALSE)/1024);
> +	print("%*llu", fields[FIELD_NET_TX-1].default_width, tot_net_bytes(domain, FALSE)/1024);
>  }
>  
>  /* Compares number of total network rx bytes of two domains, returning -1,0,1
> @@ -637,7 +641,7 @@ static int compare_net_rx(xenstat_domain *domain1, xenstat_domain *domain2)
>  /* Prints number of total network rx bytes statistic */
>  static void print_net_rx(xenstat_domain *domain)
>  {
> -	print("%8llu", tot_net_bytes(domain, TRUE)/1024);
> +	print("%*llu", fields[FIELD_NET_RX-1].default_width, tot_net_bytes(domain, TRUE)/1024);
>  }
>  
>  /* Gets number of total network bytes statistic, if rx true, then rx bytes
> @@ -705,7 +709,7 @@ static int compare_vbd_rd(xenstat_domain *domain1, xenstat_domain *domain2)
>  /* Prints number of total VBD READ requests statistic */
>  static void print_vbd_rd(xenstat_domain *domain)
>  {
> -	print("%8llu", tot_vbd_reqs(domain, FIELD_VBD_RD));
> +	print("%*llu", fields[FIELD_VBD_RD-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_RD));
>  }
>  
>  /* Compares number of total VBD WRITE requests of two domains,
> @@ -719,7 +723,7 @@ static int compare_vbd_wr(xenstat_domain *domain1, xenstat_domain *domain2)
>  /* Prints number of total VBD WRITE requests statistic */
>  static void print_vbd_wr(xenstat_domain *domain)
>  {
> -	print("%8llu", tot_vbd_reqs(domain, FIELD_VBD_WR));
> +	print("%*llu", fields[FIELD_VBD_WR-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_WR));
>  }
>  
>  /* Compares number of total VBD READ sectors of two domains,
> @@ -733,7 +737,7 @@ static int compare_vbd_rsect(xenstat_domain *domain1, xenstat_domain *domain2)
>  /* Prints number of total VBD READ sectors statistic */
>  static void print_vbd_rsect(xenstat_domain *domain)
>  {
> -	print("%10llu", tot_vbd_reqs(domain, FIELD_VBD_RSECT));
> +	print("%*llu", fields[FIELD_VBD_RSECT-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_RSECT));
>  }
>  
>  /* Compares number of total VBD WRITE sectors of two domains,
> @@ -747,7 +751,7 @@ static int compare_vbd_wsect(xenstat_domain *domain1, xenstat_domain *domain2)
>  /* Prints number of total VBD WRITE sectors statistic */
>  static void print_vbd_wsect(xenstat_domain *domain)
>  {
> -	print("%10llu", tot_vbd_reqs(domain, FIELD_VBD_WSECT));
> +	print("%*llu", fields[FIELD_VBD_WSECT-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_WSECT));
>  }
>  
> 
> @@ -806,6 +810,54 @@ static void print_ssid(xenstat_domain *domain)
>  	print("%4u", xenstat_domain_ssid(domain));
>  }
>  
> +/* Resets default_width for fields with potentially large numbers */
> +void reset_field_widths(void)
> +{
> +	fields[FIELD_NET_TX-1].default_width = 8;
> +	fields[FIELD_NET_RX-1].default_width = 8;
> +	fields[FIELD_VBD_RD-1].default_width = 8;
> +	fields[FIELD_VBD_WR-1].default_width = 8;
> +	fields[FIELD_VBD_RSECT-1].default_width = 10;
> +	fields[FIELD_VBD_WSECT-1].default_width = 10;
> +}
> +
> +/* Adjusts default_width for fields with potentially large numbers */
> +void adjust_field_widths(xenstat_domain *domain)
> +{
> +	unsigned int length;
> +
> +	if (show_full_name) {
> +		length = strlen(xenstat_domain_name(domain));
> +		if (length > fields[FIELD_NAME-1].default_width)
> +			fields[FIELD_NAME-1].default_width = length;
> +	}
> +
> +	length = INT_FIELD_WIDTH((tot_net_bytes(domain, FALSE)/1024) + 1);
> +	if (length > fields[FIELD_NET_TX-1].default_width)
> +		fields[FIELD_NET_TX-1].default_width = length;
> +
> +	length = INT_FIELD_WIDTH((tot_net_bytes(domain, TRUE)/1024) + 1);
> +	if (length > fields[FIELD_NET_RX-1].default_width)
> +		fields[FIELD_NET_RX-1].default_width = length;
> +
> +	length = INT_FIELD_WIDTH((tot_vbd_reqs(domain, FIELD_VBD_RD)) + 1);
> +	if (length > fields[FIELD_VBD_RD-1].default_width)
> +		fields[FIELD_VBD_RD-1].default_width = length;
> +
> +	length = INT_FIELD_WIDTH((tot_vbd_reqs(domain, FIELD_VBD_WR)) + 1);
> +	if (length > fields[FIELD_VBD_WR-1].default_width)
> +		fields[FIELD_VBD_WR-1].default_width = length;
> +
> +	length = INT_FIELD_WIDTH((tot_vbd_reqs(domain, FIELD_VBD_RSECT)) + 1);
> +	if (length > fields[FIELD_VBD_RSECT-1].default_width)
> +		fields[FIELD_VBD_RSECT-1].default_width = length;
> +
> +	length = INT_FIELD_WIDTH((tot_vbd_reqs(domain, FIELD_VBD_WSECT)) + 1);
> +	if (length > fields[FIELD_VBD_WSECT-1].default_width)
> +		fields[FIELD_VBD_WSECT-1].default_width = length;
> +}
> +
> +
>  /* Section printing functions */
>  /* Prints the top summary, above the domain table */
>  void do_summary(void)
> @@ -1088,6 +1140,12 @@ static void top(void)
>  	if(first_domain_index >= num_domains)
>  		first_domain_index = num_domains-1;
>  
> +	/* Adjust default_width for fields with potentially large numbers */
> +	reset_field_widths();
> +	for (i = first_domain_index; i < num_domains; i++) {
> +		adjust_field_widths(domains[i]);
> +	}
> +
>  	for (i = first_domain_index; i < num_domains; i++) {
>  		if(!batch && current_row() == lines()-1)
>  			break;
> 

Re: [PATCH] xentop: Dynamically expand some columns

[Konrad Rzeszutek Wilk]

On Tue, Nov 04, 2014 at 10:08:55AM +0000, Ian Campbell wrote:
> On Wed, 2014-10-29 at 12:24 +0100, Markus Hauschild wrote:
> > Allow certain xentop columns to automatically expand as the amount
> > of data reported gets larger.  The columns allowed to auto expand are:
> > 
> > NETTX(k), NETRX(k), VBD_RD, VBD_WR, VBD_RSECT, VBD_WSECT
> > 
> > If the -f option is used to allow full length VM names, those names will
> > also be aligned based on the longest name in the NAME column.
> > 
> > The default minimum width of all columns remains unchanged.
> > 
> > Signed-off-by: Markus Hauschild <Markus.Hauschild@rz.uni-regensburg.de>
> > Signed-off-by: Charles Arnold <carnold@suse.com>
> 
> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
> 
> Konrad, I think you were previously intending this to go in for 4.5, is
> that still the case?

Correct.
> 
> > 
> > diff --git a/tools/xenstat/xentop/Makefile b/tools/xenstat/xentop/Makefile
> > index 18bccb6..076e44c 100644
> > --- a/tools/xenstat/xentop/Makefile
> > +++ b/tools/xenstat/xentop/Makefile
> > @@ -19,7 +19,7 @@ all install xentop:
> >  else
> >  
> >  CFLAGS += -DGCC_PRINTF -Werror $(CFLAGS_libxenstat)
> > -LDLIBS += $(LDLIBS_libxenstat) $(CURSES_LIBS) $(SOCKET_LIBS)
> > +LDLIBS += $(LDLIBS_libxenstat) $(CURSES_LIBS) $(SOCKET_LIBS) -lm
> >  CFLAGS += -DHOST_$(XEN_OS)
> >  
> >  # Include configure output (config.h) to headers search path
> > diff --git a/tools/xenstat/xentop/xentop.c b/tools/xenstat/xentop/xentop.c
> > index dd11927..3062cb5 100644
> > --- a/tools/xenstat/xentop/xentop.c
> > +++ b/tools/xenstat/xentop/xentop.c
> > @@ -27,6 +27,7 @@
> >  
> >  #include <ctype.h>
> >  #include <errno.h>
> > +#include <math.h>
> >  #include <stdio.h>
> >  #include <stdlib.h>
> >  #include <stdarg.h>
> > @@ -70,6 +71,8 @@
> >  #define curses_str_t const char *
> >  #endif
> >  
> > +#define INT_FIELD_WIDTH(n) ((unsigned int)(log10(n) + 1))
> > +
> >  /*
> >   * Function prototypes
> >   */
> > @@ -127,7 +130,8 @@ 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 void reset_field_widths(void);
> > +static void adjust_field_widths(xenstat_domain *domain);
> >  
> >  /* Section printing functions */
> >  static void do_summary(void);
> > @@ -444,7 +448,7 @@ int compare_name(xenstat_domain *domain1, xenstat_domain *domain2)
> >  void print_name(xenstat_domain *domain)
> >  {
> >  	if(show_full_name)
> > -		print("%10s", xenstat_domain_name(domain));
> > +		print("%*s", fields[FIELD_NAME-1].default_width, xenstat_domain_name(domain));
> >  	else
> >  		print("%10.10s", xenstat_domain_name(domain));
> >  }
> > @@ -623,7 +627,7 @@ static int compare_net_tx(xenstat_domain *domain1, xenstat_domain *domain2)
> >  /* Prints number of total network tx bytes statistic */
> >  static void print_net_tx(xenstat_domain *domain)
> >  {
> > -	print("%8llu", tot_net_bytes(domain, FALSE)/1024);
> > +	print("%*llu", fields[FIELD_NET_TX-1].default_width, tot_net_bytes(domain, FALSE)/1024);
> >  }
> >  
> >  /* Compares number of total network rx bytes of two domains, returning -1,0,1
> > @@ -637,7 +641,7 @@ static int compare_net_rx(xenstat_domain *domain1, xenstat_domain *domain2)
> >  /* Prints number of total network rx bytes statistic */
> >  static void print_net_rx(xenstat_domain *domain)
> >  {
> > -	print("%8llu", tot_net_bytes(domain, TRUE)/1024);
> > +	print("%*llu", fields[FIELD_NET_RX-1].default_width, tot_net_bytes(domain, TRUE)/1024);
> >  }
> >  
> >  /* Gets number of total network bytes statistic, if rx true, then rx bytes
> > @@ -705,7 +709,7 @@ static int compare_vbd_rd(xenstat_domain *domain1, xenstat_domain *domain2)
> >  /* Prints number of total VBD READ requests statistic */
> >  static void print_vbd_rd(xenstat_domain *domain)
> >  {
> > -	print("%8llu", tot_vbd_reqs(domain, FIELD_VBD_RD));
> > +	print("%*llu", fields[FIELD_VBD_RD-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_RD));
> >  }
> >  
> >  /* Compares number of total VBD WRITE requests of two domains,
> > @@ -719,7 +723,7 @@ static int compare_vbd_wr(xenstat_domain *domain1, xenstat_domain *domain2)
> >  /* Prints number of total VBD WRITE requests statistic */
> >  static void print_vbd_wr(xenstat_domain *domain)
> >  {
> > -	print("%8llu", tot_vbd_reqs(domain, FIELD_VBD_WR));
> > +	print("%*llu", fields[FIELD_VBD_WR-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_WR));
> >  }
> >  
> >  /* Compares number of total VBD READ sectors of two domains,
> > @@ -733,7 +737,7 @@ static int compare_vbd_rsect(xenstat_domain *domain1, xenstat_domain *domain2)
> >  /* Prints number of total VBD READ sectors statistic */
> >  static void print_vbd_rsect(xenstat_domain *domain)
> >  {
> > -	print("%10llu", tot_vbd_reqs(domain, FIELD_VBD_RSECT));
> > +	print("%*llu", fields[FIELD_VBD_RSECT-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_RSECT));
> >  }
> >  
> >  /* Compares number of total VBD WRITE sectors of two domains,
> > @@ -747,7 +751,7 @@ static int compare_vbd_wsect(xenstat_domain *domain1, xenstat_domain *domain2)
> >  /* Prints number of total VBD WRITE sectors statistic */
> >  static void print_vbd_wsect(xenstat_domain *domain)
> >  {
> > -	print("%10llu", tot_vbd_reqs(domain, FIELD_VBD_WSECT));
> > +	print("%*llu", fields[FIELD_VBD_WSECT-1].default_width, tot_vbd_reqs(domain, FIELD_VBD_WSECT));
> >  }
> >  
> > 
> > @@ -806,6 +810,54 @@ static void print_ssid(xenstat_domain *domain)
> >  	print("%4u", xenstat_domain_ssid(domain));
> >  }
> >  
> > +/* Resets default_width for fields with potentially large numbers */
> > +void reset_field_widths(void)
> > +{
> > +	fields[FIELD_NET_TX-1].default_width = 8;
> > +	fields[FIELD_NET_RX-1].default_width = 8;
> > +	fields[FIELD_VBD_RD-1].default_width = 8;
> > +	fields[FIELD_VBD_WR-1].default_width = 8;
> > +	fields[FIELD_VBD_RSECT-1].default_width = 10;
> > +	fields[FIELD_VBD_WSECT-1].default_width = 10;
> > +}
> > +
> > +/* Adjusts default_width for fields with potentially large numbers */
> > +void adjust_field_widths(xenstat_domain *domain)
> > +{
> > +	unsigned int length;
> > +
> > +	if (show_full_name) {
> > +		length = strlen(xenstat_domain_name(domain));
> > +		if (length > fields[FIELD_NAME-1].default_width)
> > +			fields[FIELD_NAME-1].default_width = length;
> > +	}
> > +
> > +	length = INT_FIELD_WIDTH((tot_net_bytes(domain, FALSE)/1024) + 1);
> > +	if (length > fields[FIELD_NET_TX-1].default_width)
> > +		fields[FIELD_NET_TX-1].default_width = length;
> > +
> > +	length = INT_FIELD_WIDTH((tot_net_bytes(domain, TRUE)/1024) + 1);
> > +	if (length > fields[FIELD_NET_RX-1].default_width)
> > +		fields[FIELD_NET_RX-1].default_width = length;
> > +
> > +	length = INT_FIELD_WIDTH((tot_vbd_reqs(domain, FIELD_VBD_RD)) + 1);
> > +	if (length > fields[FIELD_VBD_RD-1].default_width)
> > +		fields[FIELD_VBD_RD-1].default_width = length;
> > +
> > +	length = INT_FIELD_WIDTH((tot_vbd_reqs(domain, FIELD_VBD_WR)) + 1);
> > +	if (length > fields[FIELD_VBD_WR-1].default_width)
> > +		fields[FIELD_VBD_WR-1].default_width = length;
> > +
> > +	length = INT_FIELD_WIDTH((tot_vbd_reqs(domain, FIELD_VBD_RSECT)) + 1);
> > +	if (length > fields[FIELD_VBD_RSECT-1].default_width)
> > +		fields[FIELD_VBD_RSECT-1].default_width = length;
> > +
> > +	length = INT_FIELD_WIDTH((tot_vbd_reqs(domain, FIELD_VBD_WSECT)) + 1);
> > +	if (length > fields[FIELD_VBD_WSECT-1].default_width)
> > +		fields[FIELD_VBD_WSECT-1].default_width = length;
> > +}
> > +
> > +
> >  /* Section printing functions */
> >  /* Prints the top summary, above the domain table */
> >  void do_summary(void)
> > @@ -1088,6 +1140,12 @@ static void top(void)
> >  	if(first_domain_index >= num_domains)
> >  		first_domain_index = num_domains-1;
> >  
> > +	/* Adjust default_width for fields with potentially large numbers */
> > +	reset_field_widths();
> > +	for (i = first_domain_index; i < num_domains; i++) {
> > +		adjust_field_widths(domains[i]);
> > +	}
> > +
> >  	for (i = first_domain_index; i < num_domains; i++) {
> >  		if(!batch && current_row() == lines()-1)
> >  			break;
> > 
> 
> 

Re: [PATCH] xentop: Dynamically expand some columns

[Ian Campbell]

On Tue, 2014-11-04 at 12:10 -0500, Konrad Rzeszutek Wilk wrote:
> On Tue, Nov 04, 2014 at 10:08:55AM +0000, Ian Campbell wrote:
> > On Wed, 2014-10-29 at 12:24 +0100, Markus Hauschild wrote:
> > > Allow certain xentop columns to automatically expand as the amount
> > > of data reported gets larger.  The columns allowed to auto expand are:
> > > 
> > > NETTX(k), NETRX(k), VBD_RD, VBD_WR, VBD_RSECT, VBD_WSECT
> > > 
> > > If the -f option is used to allow full length VM names, those names will
> > > also be aligned based on the longest name in the NAME column.
> > > 
> > > The default minimum width of all columns remains unchanged.
> > > 
> > > Signed-off-by: Markus Hauschild <Markus.Hauschild@rz.uni-regensburg.de>
> > > Signed-off-by: Charles Arnold <carnold@suse.com>
> > 
> > Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
> > 
> > Konrad, I think you were previously intending this to go in for 4.5, is
> > that still the case?
> 
> Correct.

Applied.