[Xen-devel] [PATCH] tools/xentop: Display '-' when stats are not available.
From: Pritha Srivastava Displaying 0 is misleading. Signed-off-by: Pritha Srivastava Signed-off-by: Ronan Abhamon --- 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; +} + /* * 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.er
Re: [Xen-devel] [PATCH] tools/xentop: Display '-' when stats are not available.
On Wed, Feb 20, 2019 at 04:19:25PM +, Ronan Abhamon wrote: > From: Pritha Srivastava > > Displaying 0 is misleading. > > Signed-off-by: Pritha Srivastava > Signed-off-by: Ronan Abhamon > --- > 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; > +
Re: [Xen-devel] [PATCH] tools/xentop: Display '-' when stats are not available.
Le 21/02/2019 à 16:32, Wei Liu a écrit : On Wed, Feb 20, 2019 at 04:19:25PM +, Ronan Abhamon wrote: From: Pritha Srivastava Displaying 0 is misleading. Signed-off-by: Pritha Srivastava Signed-off-by: Ronan Abhamon --- 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. Thank you for your feedback, but xenstat_vbd is a private struct of libxenstat. Correct me if I am wrong but I can't include this file in the xentop folder. Ronan Abhamon ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] tools/xentop: Display '-' when stats are not available.
On Fri, Feb 22, 2019 at 10:47:24AM +, Ronan Abhamon wrote: > Le 21/02/2019 à 16:32, Wei Liu a écrit : > > > On Wed, Feb 20, 2019 at 04:19:25PM +, Ronan Abhamon wrote: > > > From: Pritha Srivastava > > > > > > Displaying 0 is misleading. > > > > > > Signed-off-by: Pritha Srivastava > > > Signed-off-by: Ronan Abhamon > > > --- > > > 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. > > Thank you for your feedback, but xenstat_vbd is a private struct of > libxenstat. > > Correct me if I am wrong but I can't include this file in the xentop folder. Oh, sorry. I misread. Yes then you need to keep this function. But I would like you to change the return type to bool if possible. Wei. > > > Ronan Abhamon > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel