Re: [Xen-devel] [PATCH] tools/xentop: Display '-' when stats are not available.

[Wei Liu]

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

Re: [Xen-devel] [PATCH] tools/xentop: Display '-' when stats are not available.

[Ronan Abhamon]

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.

[Wei Liu]

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, , );
>   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", _reqs)) != 1))
> - {
> - continue;
> - }
> -
> - if((read_attributes_vbd(dp->d_name, "statistics/rd_req", buf, 
> 256)<=0)
> -|| ((ret = sscanf(buf, "%llu", _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", _reqs)) != 1))
> - {
> - continue;
> - }
> -
> - if((read_attributes_vbd(dp->d_name, "statistics/rd_sect", buf, 
> 256)<=0)
> -|| ((ret = sscanf(buf, "%llu", _sects)) != 1))
> - {
> - continue;
> + vbd.error = 0;
> +
> + if ((read_attributes_vbd(dp->d_name, 
> "statistics/oo_req", buf, 256)<=0) ||
> + ((ret = sscanf(buf, "%llu", _reqs)) != 
> 1) ||
> + (read_attributes_vbd(dp->d_name, 
> "statistics/rd_req", buf, 256)<=0) ||
> + ((ret = sscanf(buf, "%llu", _reqs)) != 
> 1) ||
> + (read_attributes_vbd(dp->d_name, 
> "statistics/wr_req", buf, 256)<=0) ||
> + ((ret = sscanf(buf, "%llu", _reqs)) != 
> 1) ||
> + (read_attributes_vbd(dp->d_name, 
> "statistics/rd_sect", buf, 256)<=0) ||
> + ((ret = sscanf(buf, "%llu", _sects)) != 
> 1) ||
> + (read_attributes_vbd(dp->d_name, 
> "statistics/wr_sect", buf, 256)<=0) ||
> + ((ret = sscanf(buf, "%llu", _sects)) != 
> 1))
> + {
> + vbd.error = 1;
> + }
>   }
> -
> - 

[Xen-devel] [PATCH] tools/xentop: Display '-' when stats are not available.

[Ronan Abhamon]

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, , );
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", _reqs)) != 1))
-   {
-   continue;
-   }
-
-   if((read_attributes_vbd(dp->d_name, "statistics/rd_req", buf, 
256)<=0)
-  || ((ret = sscanf(buf, "%llu", _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", _reqs)) != 1))
-   {
-   continue;
-   }
-
-   if((read_attributes_vbd(dp->d_name, "statistics/rd_sect", buf, 
256)<=0)
-  || ((ret = sscanf(buf, "%llu", _sects)) != 1))
-   {
-   continue;
+   vbd.error = 0;
+
+   if ((read_attributes_vbd(dp->d_name, 
"statistics/oo_req", buf, 256)<=0) ||
+   ((ret = sscanf(buf, "%llu", _reqs)) != 
1) ||
+   (read_attributes_vbd(dp->d_name, 
"statistics/rd_req", buf, 256)<=0) ||
+   ((ret = sscanf(buf, "%llu", _reqs)) != 
1) ||
+   (read_attributes_vbd(dp->d_name, 
"statistics/wr_req", buf, 256)<=0) ||
+   ((ret = sscanf(buf, "%llu", _reqs)) != 
1) ||
+   (read_attributes_vbd(dp->d_name, 
"statistics/rd_sect", buf, 256)<=0) ||
+   ((ret = sscanf(buf, "%llu", _sects)) != 
1) ||
+   (read_attributes_vbd(dp->d_name, 
"statistics/wr_sect", buf, 256)<=0) ||
+   ((ret = sscanf(buf, "%llu", _sects)) != 
1))
+   {
+   vbd.error = 1;
+   }
}
-
-   if((read_attributes_vbd(dp->d_name, "statistics/wr_sect", buf, 
256)<=0)
-  || ((ret = sscanf(buf, "%llu", _sects)) != 1))
+   else
{
-   continue;
+   vbd.error = 1;
}
-
if ((xenstat_save_vbd(domain, )) ==