|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] tools/xentop: Display '-' when stats are not available.
On Wed, Feb 20, 2019 at 04:19:25PM +0000, Ronan Abhamon wrote:
> From: Pritha Srivastava <pritha.srivastava@xxxxxxxxxx>
>
> Displaying 0 is misleading.
>
> Signed-off-by: Pritha Srivastava <pritha.srivastava@xxxxxxxxxx>
> Signed-off-by: Ronan Abhamon <ronan.abhamon@xxxxxxxx>
> ---
> tools/xenstat/libxenstat/src/xenstat.c | 6 +
> tools/xenstat/libxenstat/src/xenstat.h | 3 +
> tools/xenstat/libxenstat/src/xenstat_linux.c | 47 +++---
> tools/xenstat/libxenstat/src/xenstat_priv.h | 1 +
> tools/xenstat/xentop/xentop.c | 159 +++++++++++++++----
> 5 files changed, 158 insertions(+), 58 deletions(-)
>
> diff --git a/tools/xenstat/libxenstat/src/xenstat.c
> b/tools/xenstat/libxenstat/src/xenstat.c
> index fbe44f3c56..8fa12d7bc0 100644
> --- a/tools/xenstat/libxenstat/src/xenstat.c
> +++ b/tools/xenstat/libxenstat/src/xenstat.c
> @@ -729,6 +729,12 @@ unsigned long long xenstat_vbd_wr_sects(xenstat_vbd *
> vbd)
> return vbd->wr_sects;
> }
>
> +/* Returns error while getting stats (1 if error happened, 0 otherwise) */
> +unsigned int xenstat_vbd_error(xenstat_vbd * vbd)
> +{
> + return vbd->error;
> +}
> +
You can do away with this function by accessing vbd->error directly in
code.
> /*
> * Tmem functions
> */
> diff --git a/tools/xenstat/libxenstat/src/xenstat.h
> b/tools/xenstat/libxenstat/src/xenstat.h
> index 47ec60e14d..fe13b65727 100644
> --- a/tools/xenstat/libxenstat/src/xenstat.h
> +++ b/tools/xenstat/libxenstat/src/xenstat.h
> @@ -193,6 +193,9 @@ unsigned long long xenstat_vbd_wr_reqs(xenstat_vbd * vbd);
> unsigned long long xenstat_vbd_rd_sects(xenstat_vbd * vbd);
> unsigned long long xenstat_vbd_wr_sects(xenstat_vbd * vbd);
>
> +/* Returns error while getting stats (1 if error happened, 0 otherwise) */
> +unsigned int xenstat_vbd_error(xenstat_vbd * vbd);
> +
> /*
> * Tmem functions - extract tmem information
> */
> diff --git a/tools/xenstat/libxenstat/src/xenstat_linux.c
> b/tools/xenstat/libxenstat/src/xenstat_linux.c
> index 7cdd3bf91f..9421ca43c8 100644
> --- a/tools/xenstat/libxenstat/src/xenstat_linux.c
> +++ b/tools/xenstat/libxenstat/src/xenstat_linux.c
> @@ -436,13 +436,15 @@ int xenstat_collect_vbds(xenstat_node * node)
> ret = sscanf(dp->d_name, "%3s-%u-%u", buf, &domid, &vbd.dev);
> if (ret != 3)
> continue;
> + if (!(strstr(buf, "vbd")) && !(strstr(buf, "tap")))
> + continue;
>
> if (strcmp(buf,"vbd") == 0)
> vbd.back_type = 1;
> else if (strcmp(buf,"tap") == 0)
> vbd.back_type = 2;
> else
> - continue;
> + vbd.back_type = 0;
>
> domain = xenstat_node_domain(node, domid);
> if (domain == NULL) {
> @@ -453,36 +455,29 @@ int xenstat_collect_vbds(xenstat_node * node)
> continue;
> }
>
> - if((read_attributes_vbd(dp->d_name, "statistics/oo_req", buf,
> 256)<=0)
> - || ((ret = sscanf(buf, "%llu", &vbd.oo_reqs)) != 1))
> - {
> - continue;
> - }
> -
> - if((read_attributes_vbd(dp->d_name, "statistics/rd_req", buf,
> 256)<=0)
> - || ((ret = sscanf(buf, "%llu", &vbd.rd_reqs)) != 1))
> + if (vbd.back_type == 1 || vbd.back_type == 2)
> {
> - continue;
> - }
>
> - if((read_attributes_vbd(dp->d_name, "statistics/wr_req", buf,
> 256)<=0)
> - || ((ret = sscanf(buf, "%llu", &vbd.wr_reqs)) != 1))
> - {
> - continue;
> - }
> -
> - if((read_attributes_vbd(dp->d_name, "statistics/rd_sect", buf,
> 256)<=0)
> - || ((ret = sscanf(buf, "%llu", &vbd.rd_sects)) != 1))
> - {
> - continue;
> + vbd.error = 0;
> +
> + if ((read_attributes_vbd(dp->d_name,
> "statistics/oo_req", buf, 256)<=0) ||
> + ((ret = sscanf(buf, "%llu", &vbd.oo_reqs)) !=
> 1) ||
> + (read_attributes_vbd(dp->d_name,
> "statistics/rd_req", buf, 256)<=0) ||
> + ((ret = sscanf(buf, "%llu", &vbd.rd_reqs)) !=
> 1) ||
> + (read_attributes_vbd(dp->d_name,
> "statistics/wr_req", buf, 256)<=0) ||
> + ((ret = sscanf(buf, "%llu", &vbd.wr_reqs)) !=
> 1) ||
> + (read_attributes_vbd(dp->d_name,
> "statistics/rd_sect", buf, 256)<=0) ||
> + ((ret = sscanf(buf, "%llu", &vbd.wr_sects)) !=
> 1) ||
> + (read_attributes_vbd(dp->d_name,
> "statistics/wr_sect", buf, 256)<=0) ||
> + ((ret = sscanf(buf, "%llu", &vbd.wr_sects)) !=
> 1))
> + {
> + vbd.error = 1;
> + }
> }
> -
> - if((read_attributes_vbd(dp->d_name, "statistics/wr_sect", buf,
> 256)<=0)
> - || ((ret = sscanf(buf, "%llu", &vbd.wr_sects)) != 1))
> + else
> {
> - continue;
> + vbd.error = 1;
> }
> -
> if ((xenstat_save_vbd(domain, &vbd)) == NULL) {
> perror("Allocation error");
> return 0;
> diff --git a/tools/xenstat/libxenstat/src/xenstat_priv.h
> b/tools/xenstat/libxenstat/src/xenstat_priv.h
> index 74e0774a5e..ebfcd0fff6 100644
> --- a/tools/xenstat/libxenstat/src/xenstat_priv.h
> +++ b/tools/xenstat/libxenstat/src/xenstat_priv.h
> @@ -98,6 +98,7 @@ struct xenstat_network {
> struct xenstat_vbd {
> unsigned int back_type;
> unsigned int dev;
> + unsigned int error;
> unsigned long long oo_reqs;
> unsigned long long rd_reqs;
> unsigned long long wr_reqs;
> diff --git a/tools/xenstat/xentop/xentop.c b/tools/xenstat/xentop/xentop.c
> index c46581062b..8ef6f49ff9 100644
> --- a/tools/xenstat/xentop/xentop.c
> +++ b/tools/xenstat/xentop/xentop.c
> @@ -32,6 +32,7 @@
> #include <time.h>
> #include <unistd.h>
> #include <signal.h>
> +#include <stdbool.h>
> #if defined(__linux__)
> #include <linux/kdev_t.h>
> #endif
> @@ -87,7 +88,7 @@ static int handle_key(int);
> static int compare(unsigned long long, unsigned long long);
> static int compare_domains(xenstat_domain **, xenstat_domain **);
> static unsigned long long tot_net_bytes( xenstat_domain *, int);
> -static unsigned long long tot_vbd_reqs( xenstat_domain *, int);
> +static bool tot_vbd_reqs( xenstat_domain *, int, unsigned long long *);
Since you're touching this anyway, I would like to request the
extraneous space be removed.
>
> /* Field functions */
> static int compare_state(xenstat_domain *domain1, xenstat_domain *domain2);
> @@ -685,70 +686,140 @@ static void print_vbds(xenstat_domain *domain)
> returning -1,0,1 * for <,=,> */
> static int compare_vbd_oo(xenstat_domain *domain1, xenstat_domain *domain2)
> {
> - return -compare(tot_vbd_reqs(domain1, FIELD_VBD_OO),
> - tot_vbd_reqs(domain2, FIELD_VBD_OO));
> + unsigned long long dom1_vbd_oo = 0, dom2_vbd_oo = 0;
> +
> + tot_vbd_reqs(domain1, FIELD_VBD_OO, &dom1_vbd_oo);
> + tot_vbd_reqs(domain1, FIELD_VBD_OO, &dom2_vbd_oo);
> +
> + return -compare(dom1_vbd_oo,
> + dom2_vbd_oo);
This now fits into one line.
There are other instances as well. Please fix them in the next version.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |