Re: [PATCH] tools/xentop: Add VBD3 support to xentop

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.02.2024 15:12, Fouad Hilly wrote:
> From: Pritha Srivastava <pritha.srivastava@xxxxxxxxxx>
> 
> xl now knows how to drive tapdisk, so modified libxenstat to
> understand vbd3 statistics.
> 
> Signed-off-by: Jorge Martin <jorge.martin@xxxxxxxxxx>
> Signed-off-by: Pritha Srivastava <pritha.srivastava@xxxxxxxxxx>

Just a formal question (I'm not really qualified to review this change):
With the above two S-o-b and the earlier From: - who is the original
author of this patch? The general expectation is for the 1st S-o-b to
be the author's.

Jan

> Signed-off-by: Fouad Hilly <fouad.hilly@xxxxxxxxx>
> ---
> CC: Wei Liu <wl@xxxxxxx>
> CC: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> CC: Juergen Gross <jgross@xxxxxxxx>
> ---
>  tools/libs/stat/xenstat_linux.c | 69 ++++++++++++++++++++++++++++++++-
>  tools/libs/stat/xenstat_priv.h  | 16 ++++++++
>  tools/xentop/xentop.c           |  1 +
>  3 files changed, 85 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libs/stat/xenstat_linux.c b/tools/libs/stat/xenstat_linux.c
> index cbba54aa83ee..5a4a03634182 100644
> --- a/tools/libs/stat/xenstat_linux.c
> +++ b/tools/libs/stat/xenstat_linux.c
> @@ -390,6 +390,39 @@ void xenstat_uninit_networks(xenstat_handle * handle)
>               fclose(priv->procnetdev);
>  }
>  
> +static int read_attributes_vbd3(char *vbd3_path, xenstat_vbd *vbd)
> +{
> +     FILE *fp;
> +     struct vbd3_stats vbd3_stats;
> +
> +     fp = fopen(vbd3_path, "rb");
> +
> +     if (fp == NULL)
> +     {
> +             return -1;
> +     }
> +
> +     if (fread(&vbd3_stats, sizeof(struct vbd3_stats), 1, fp) != 1) {
> +             fclose(fp);
> +             return -1;
> +     }
> +
> +     if (vbd3_stats.version != 1) {
> +             fclose(fp);
> +             return -1;
> +     }
> +
> +     vbd->oo_reqs = vbd3_stats.oo_reqs;
> +     vbd->rd_reqs = vbd3_stats.read_reqs_submitted;
> +     vbd->rd_sects = vbd3_stats.read_sectors;
> +     vbd->wr_reqs = vbd3_stats.write_reqs_submitted;
> +     vbd->wr_sects = vbd3_stats.write_sectors;
> +
> +     fclose(fp);
> +
> +     return 0;
> +}
> +
>  static int read_attributes_vbd(const char *vbd_directory, const char *what, 
> char *ret, int cap)
>  {
>       static char file_name[80];
> @@ -438,7 +471,7 @@ int xenstat_collect_vbds(xenstat_node * node)
>               int ret;
>               char buf[256];
>  
> -             ret = sscanf(dp->d_name, "%3s-%u-%u", buf, &domid, &vbd.dev);
> +             ret = sscanf(dp->d_name, "%255[^-]-%u-%u", buf, &domid, 
> &vbd.dev);
>               if (ret != 3)
>                       continue;
>               if (!(strstr(buf, "vbd")) && !(strstr(buf, "tap")))
> @@ -448,6 +481,8 @@ int xenstat_collect_vbds(xenstat_node * node)
>                       vbd.back_type = 1;
>               else if (strcmp(buf,"tap") == 0)
>                       vbd.back_type = 2;
> +             else if (strcmp(buf,"vbd3") == 0)
> +                     vbd.back_type = 3;
>               else
>                       vbd.back_type = 0;
>  
> @@ -479,6 +514,38 @@ int xenstat_collect_vbds(xenstat_node * node)
>                               vbd.error = 1;
>                       }
>               }
> +             else if (vbd.back_type == 3)
> +             {
> +
> +                     char *td3_pid;
> +                     char *path;
> +
> +                     vbd.back_type = 3;
> +                     vbd.error = 0;
> +
> +                     if (asprintf(&path, 
> "/local/domain/0/backend/vbd3/%u/%u/kthread-pid", domid, vbd.dev) < 0)
> +                             continue;
> +
> +                     td3_pid = xs_read(node->handle->xshandle, XBT_NULL, 
> path, NULL);
> +
> +                     if (td3_pid == NULL) {
> +                             free(path);
> +                             continue;
> +                     }
> +
> +                     free(path);
> +
> +                     if (asprintf(&path, "/dev/shm/td3-%s/vbd-%u-%u", 
> td3_pid, domid, vbd.dev) < 0) {
> +                             free(td3_pid);
> +                             continue;
> +                     }
> +
> +                     if (read_attributes_vbd3(path, &vbd) < 0)
> +                             vbd.error = 1;
> +
> +                     free(td3_pid);
> +                     free(path);
> +             }
>               else
>               {
>                       vbd.error = 1;
> diff --git a/tools/libs/stat/xenstat_priv.h b/tools/libs/stat/xenstat_priv.h
> index 4eb44a8ebb84..c3a9635240e9 100644
> --- a/tools/libs/stat/xenstat_priv.h
> +++ b/tools/libs/stat/xenstat_priv.h
> @@ -98,6 +98,22 @@ struct xenstat_vbd {
>       unsigned long long wr_sects;
>  };
>  
> +struct vbd3_stats {
> +     uint32_t version;
> +     uint32_t __pad;
> +     uint64_t oo_reqs;
> +     uint64_t read_reqs_submitted;
> +     uint64_t read_reqs_completed;
> +     uint64_t read_sectors;
> +     uint64_t read_total_ticks;
> +     uint64_t write_reqs_submitted;
> +     uint64_t write_reqs_completed;
> +     uint64_t write_sectors;
> +     uint64_t write_total_ticks;
> +     uint64_t io_errors;
> +     uint64_t flags;
> +};
> +
>  extern int xenstat_collect_networks(xenstat_node * node);
>  extern void xenstat_uninit_networks(xenstat_handle * handle);
>  extern int xenstat_collect_vbds(xenstat_node * node);
> diff --git a/tools/xentop/xentop.c b/tools/xentop/xentop.c
> index 0a2fab7f15a3..f5a456fd4dfd 100644
> --- a/tools/xentop/xentop.c
> +++ b/tools/xentop/xentop.c
> @@ -1124,6 +1124,7 @@ void do_vbd(xenstat_domain *domain)
>               "Unidentified",           /* number 0 */
>               "BlkBack",           /* number 1 */
>               "BlkTap",            /* number 2 */
> +             "Tapdisk3"           /* number 3 */
>       };
>  
>       num_vbds = xenstat_domain_num_vbds(domain);