[v4] qga: add guest-get-diskstats command for Linux guests

Add a new 'guest-get-diskstats' command for report disk io statistics
for Linux guests. This can be useful for getting io flow or handling
IO fault, no need to enter guests.

Signed-off-by: luzhipeng <luzhipeng@cestc.cn>
---
 Changes v3->v4: https://patchew.org/QEMU/20220515095437.1291-1-luzhipeng@cestc.cn/
 Changes v2->v3: bugfix for memory leak 
 Changes v1->v2: v1:https://patchew.org/QEMU/20220512011930.214-1-luzhipeng@cestc.cn/
 
 qga/commands-posix.c | 123 +++++++++++++++++++++++++++++++++++++++++++
 qga/commands-win32.c |   6 +++
 qga/qapi-schema.json |  86 ++++++++++++++++++++++++++++++
 3 files changed, 215 insertions(+)

On Thu, May 19, 2022 at 3:28 PM luzhipeng <luzhipeng@cestc.cn> wrote:
>
> Add a new 'guest-get-diskstats' command for report disk io statistics
> for Linux guests. This can be useful for getting io flow or handling
> IO fault, no need to enter guests.
>
> Signed-off-by: luzhipeng <luzhipeng@cestc.cn>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  Changes v3->v4: https://patchew.org/QEMU/20220515095437.1291-1-luzhipeng@cestc.cn/
>  Changes v2->v3: bugfix for memory leak
>  Changes v1->v2: v1:https://patchew.org/QEMU/20220512011930.214-1-luzhipeng@cestc.cn/
>
>  qga/commands-posix.c | 123 +++++++++++++++++++++++++++++++++++++++++++
>  qga/commands-win32.c |   6 +++
>  qga/qapi-schema.json |  86 ++++++++++++++++++++++++++++++
>  3 files changed, 215 insertions(+)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 69f209af87..12b50b7124 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2783,6 +2783,122 @@ GuestMemoryBlockInfo *qmp_guest_get_memory_block_info(Error **errp)
>      return info;
>  }
>
> +#define MAX_NAME_LEN 128
> +static GuestDiskStatsInfoList *guest_get_diskstats(Error **errp)
> +{
> +#ifdef CONFIG_LINUX
> +    GuestDiskStatsInfoList *head = NULL, **tail = &head;
> +    const char *diskstats = "/proc/diskstats";
> +    FILE *fp;
> +    size_t n;
> +    char *line = NULL;
> +
> +    fp = fopen(diskstats, "r");
> +    if (fp  == NULL) {
> +        error_setg_errno(errp, errno, "open(\"%s\")", diskstats);
> +        return NULL;
> +    }
> +
> +    while (getline(&line, &n, fp) != -1) {
> +        g_autofree GuestDiskStatsInfo *diskstatinfo = NULL;
> +        g_autofree GuestDiskStats *diskstat = NULL;
> +        char dev_name[MAX_NAME_LEN];
> +        unsigned int ios_pgr, tot_ticks, rq_ticks, wr_ticks, dc_ticks, fl_ticks;
> +        unsigned long rd_ios, rd_merges_or_rd_sec, rd_ticks_or_wr_sec, wr_ios;
> +        unsigned long wr_merges, rd_sec_or_wr_ios, wr_sec;
> +        unsigned long dc_ios, dc_merges, dc_sec, fl_ios;
> +        unsigned int major, minor;
> +        int i;
> +
> +        i = sscanf(line, "%u %u %s %lu %lu %lu"
> +                   "%lu %lu %lu %lu %u %u %u %u"
> +                   "%lu %lu %lu %u %lu %u",
> +                   &major, &minor, dev_name,
> +                   &rd_ios, &rd_merges_or_rd_sec, &rd_sec_or_wr_ios,
> +                   &rd_ticks_or_wr_sec, &wr_ios, &wr_merges, &wr_sec,
> +                   &wr_ticks, &ios_pgr, &tot_ticks, &rq_ticks,
> +                   &dc_ios, &dc_merges, &dc_sec, &dc_ticks,
> +                   &fl_ios, &fl_ticks);
> +
> +        if (i < 7) {
> +            continue;
> B
> +        }
> +
> +        diskstatinfo = g_new0(GuestDiskStatsInfo, 1);
> +        diskstatinfo->name = g_strdup(dev_name);
> +        diskstatinfo->major = major;
> +        diskstatinfo->minor = minor;
> +
> +        diskstat = g_new0(GuestDiskStats, 1);
> +        if (i == 7) {
> +            diskstat->has_read_ios = true;
> +            diskstat->read_ios = rd_ios;
> +            diskstat->has_read_sectors = true;
> +            diskstat->read_sectors = rd_merges_or_rd_sec;
> +            diskstat->has_write_ios = true;
> +            diskstat->write_ios = rd_sec_or_wr_ios;
> +            diskstat->has_write_sectors = true;
> +            diskstat->write_sectors = rd_ticks_or_wr_sec;
> +        }
> +        if (i >= 14) {
> +            diskstat->has_read_ios = true;
> +            diskstat->read_ios = rd_ios;
> +            diskstat->has_read_sectors = true;
> +            diskstat->read_sectors = rd_sec_or_wr_ios;
> +            diskstat->has_read_merges = true;
> +            diskstat->read_merges = rd_merges_or_rd_sec;
> +            diskstat->has_read_ticks = true;
> +            diskstat->read_ticks = rd_ticks_or_wr_sec;
> +            diskstat->has_write_ios = true;
> +            diskstat->write_ios = wr_ios;
> +            diskstat->has_write_sectors = true;
> +            diskstat->write_sectors = wr_sec;
> +            diskstat->has_write_merges = true;
> +            diskstat->write_merges = wr_merges;
> +            diskstat->has_write_ticks = true;
> +            diskstat->write_ticks = wr_ticks;
> +            diskstat->has_ios_pgr = true;
> +            diskstat->ios_pgr = ios_pgr;
> +            diskstat->has_total_ticks = true;
> +            diskstat->total_ticks = tot_ticks;
> +            diskstat->has_weight_ticks = true;
> +            diskstat->weight_ticks = rq_ticks;
> +        }
> +        if (i >= 18) {
> +            diskstat->has_discard_ios = true;
> +            diskstat->discard_ios = dc_ios;
> +            diskstat->has_discard_merges = true;
> +            diskstat->discard_merges = dc_merges;
> +            diskstat->has_discard_sectors = true;
> +            diskstat->discard_sectors = dc_sec;
> +            diskstat->has_discard_ticks = true;
> +            diskstat->discard_ticks = dc_ticks;
> +        }
> +        if (i >= 20) {
> +            diskstat->has_flush_ios = true;
> +            diskstat->flush_ios = fl_ios;
> +            diskstat->has_flush_ticks = true;
> +            diskstat->flush_ticks = fl_ticks;
> +        }
> +
> +        diskstatinfo->stats = g_steal_pointer(&diskstat);
> +        QAPI_LIST_APPEND(tail, diskstatinfo);
> +        diskstatinfo = NULL;
> +    }
> +    free(line);
> +    fclose(fp);
> +    return head;
> +#else
> +    g_debug("disk stats reporting available only for Linux");
> +    return NULL;
> +#endif
> +}
> +
> +GuestDiskStatsInfoList *qmp_guest_get_diskstats(Error **errp)
> +{
> +    return guest_get_diskstats(errp);
> +}
> +
>  #else /* defined(__linux__) */
>
>  void qmp_guest_suspend_disk(Error **errp)
> @@ -3131,6 +3247,13 @@ GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
>      return NULL;
>  }
>
> +GuestDiskStatsInfoList *qmp_guest_get_diskstats(Error **errp)
> +{
> +    error_setg(errp, QERR_UNSUPPORTED);
> +    return NULL;
> +}
> +
> +
>  #endif /* CONFIG_FSFREEZE */
>
>  #if !defined(CONFIG_FSTRIM)
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index d56b5fd2a7..dcdeb76a68 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -2532,3 +2532,9 @@ char *qga_get_host_name(Error **errp)
>
>      return g_utf16_to_utf8(tmp, size, NULL, NULL, NULL);
>  }
> +
> +GuestDiskStatsInfoList *qmp_guest_get_diskstats(Error **errp)
> +{
> +    error_setg(errp, QERR_UNSUPPORTED);
> +    return NULL;
> +}
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index 4d8e506c9e..75d2bddf4b 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -1490,3 +1490,89 @@
>  { 'command': 'guest-ssh-remove-authorized-keys',
>    'data': { 'username': 'str', 'keys': ['str'] },
>    'if': 'CONFIG_POSIX' }
> +
> +##
> +# @GuestDiskStats:
> +#
> +# @read-sectors: sectors read
> +#
> +# @write-sectors: sectors written
> +#
> +# @discard-sectors: sectors discarded
> +#
> +# @read-ios: reads completed successfully
> +#
> +# @read-merges: Number of read requests merged
> +#
> +# @write-ios: writes completed
> +#
> +# @write-merges: Number of write requests merged
> +#
> +# @discard-ios: Number of discards completed successfully
> +#
> +# @discard-merges: NUmber of discard requests merged
> +#
> +# @flush-ios: Number of flush requests completed successfully
> +#
> +# @read-ticks: time spent reading(ms)
> +#
> +# @write-ticks: time spent writing(ms)
> +#
> +# @discard-ticks: time spent discarding(ms)
> +#
> +# @flush-ticks: time spent flushing(ms)
> +#
> +# @ios-pgr: Number of I/Os currently in flight
> +#
> +# @total-ticks: time spent doing I/Os (ms)
> +#
> +# @weight-ticks: weighted time spent doing I/Os since the last update of this field(ms)
> +#
> +# Since: 7.1
> +##
> +{ 'struct': 'GuestDiskStats',
> +  'data': {'*read-sectors': 'uint64',
> +           '*write-sectors': 'uint64',
> +           '*discard-sectors': 'uint64',
> +           '*read-ios': 'uint64',
> +           '*read-merges': 'uint64',
> +           '*write-ios': 'uint64',
> +           '*write-merges': 'uint64',
> +           '*discard-ios': 'uint64',
> +           '*discard-merges': 'uint64',
> +           '*flush-ios': 'uint64',
> +           '*read-ticks': 'uint64',
> +           '*write-ticks': 'uint64',
> +           '*discard-ticks': 'uint64',
> +           '*flush-ticks': 'uint64',
> +           '*ios-pgr': 'uint64',
> +           '*total-ticks': 'uint64',
> +           '*weight-ticks': 'uint64'
> +           } }
> +
> +##
> +# @GuestDiskStatsInfo:
> +#
> +# @name disk name
> +#
> +# @major major of disk
> +#
> +# @minor minor of disk
> +##
> +{ 'struct': 'GuestDiskStatsInfo',
> +  'data': {'name': 'str',
> +           'major': 'uint64',
> +           'minor': 'uint64',
> +           'stats': 'GuestDiskStats' } }
> +
> +##
> +# @guest-get-diskstats:
> +#
> +# Retrieve information about disk stats.
> +# Returns: List of disk stats of guest.
> +#
> +# Since: 7.1
> +##
> +{ 'command': 'guest-get-diskstats',
> +  'returns': ['GuestDiskStatsInfo']
> +}
> --
> 2.31.1
>
>
>

luzhipeng <luzhipeng@cestc.cn> writes:

> Add a new 'guest-get-diskstats' command for report disk io statistics
> for Linux guests. This can be useful for getting io flow or handling
> IO fault, no need to enter guests.
>
> Signed-off-by: luzhipeng <luzhipeng@cestc.cn>

[...]

> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index 4d8e506c9e..75d2bddf4b 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -1490,3 +1490,89 @@
>  { 'command': 'guest-ssh-remove-authorized-keys',
>    'data': { 'username': 'str', 'keys': ['str'] },
>    'if': 'CONFIG_POSIX' }
> +
> +##
> +# @GuestDiskStats:
> +#
> +# @read-sectors: sectors read
> +#
> +# @write-sectors: sectors written
> +#
> +# @discard-sectors: sectors discarded
> +#
> +# @read-ios: reads completed successfully
> +#
> +# @read-merges: Number of read requests merged
> +#
> +# @write-ios: writes completed
> +#
> +# @write-merges: Number of write requests merged
> +#
> +# @discard-ios: Number of discards completed successfully
> +#
> +# @discard-merges: NUmber of discard requests merged

Typo in "Number".

> +#
> +# @flush-ios: Number of flush requests completed successfully
> +#
> +# @read-ticks: time spent reading(ms)
> +#
> +# @write-ticks: time spent writing(ms)
> +#
> +# @discard-ticks: time spent discarding(ms)
> +#
> +# @flush-ticks: time spent flushing(ms)
> +#
> +# @ios-pgr: Number of I/Os currently in flight
> +#
> +# @total-ticks: time spent doing I/Os (ms)
> +#
> +# @weight-ticks: weighted time spent doing I/Os since the last update of this field(ms)

Some descriptions start with lower case, some with upper case.
Recommend to pick one and stick with it.  Lower case seems to be more
common in this schema.

> +#
> +# Since: 7.1
> +##
> +{ 'struct': 'GuestDiskStats',
> +  'data': {'*read-sectors': 'uint64',
> +           '*write-sectors': 'uint64',
> +           '*discard-sectors': 'uint64',
> +           '*read-ios': 'uint64',
> +           '*read-merges': 'uint64',
> +           '*write-ios': 'uint64',
> +           '*write-merges': 'uint64',
> +           '*discard-ios': 'uint64',
> +           '*discard-merges': 'uint64',
> +           '*flush-ios': 'uint64',
> +           '*read-ticks': 'uint64',
> +           '*write-ticks': 'uint64',
> +           '*discard-ticks': 'uint64',
> +           '*flush-ticks': 'uint64',
> +           '*ios-pgr': 'uint64',
> +           '*total-ticks': 'uint64',
> +           '*weight-ticks': 'uint64'
> +           } }
> +
> +##
> +# @GuestDiskStatsInfo:
> +#
> +# @name disk name
> +#
> +# @major major of disk
> +#
> +# @minor minor of disk

Major and minor *what*?

> +##
> +{ 'struct': 'GuestDiskStatsInfo',
> +  'data': {'name': 'str',
> +           'major': 'uint64',
> +           'minor': 'uint64',
> +           'stats': 'GuestDiskStats' } }
> +
> +##
> +# @guest-get-diskstats:
> +#
> +# Retrieve information about disk stats.
> +# Returns: List of disk stats of guest.
> +#
> +# Since: 7.1
> +##
> +{ 'command': 'guest-get-diskstats',
> +  'returns': ['GuestDiskStatsInfo']
> +}