On Fri, May 13, 2022 at 1:50 PM Konstantin Kostiuk <kkost...@redhat.com> wrote:
> > > On Fri, May 13, 2022 at 1:40 PM luzhipeng <luzhip...@cestc.cn> wrote: > >> Add a new 'guest-get-diskstats' command for report disk io statistics >> for Linux guests. This can be usefull for getting io flow or handling >> IO fault, no need to enter guests. >> >> Signed-off-by: luzhipeng <luzhip...@cestc.cn> >> >> --- >> Changes v1->v2: >> v1:https://patchew.org/QEMU/20220512011930.214-1-luzhip...@cestc.cn/ >> >> qga/commands-posix.c | 97 ++++++++++++++++++++++++++++++++++++++++++++ >> qga/commands-win32.c | 6 +++ >> qga/qapi-schema.json | 86 +++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 189 insertions(+) >> >> diff --git a/qga/commands-posix.c b/qga/commands-posix.c >> index 69f209af87..7a16d84e3a 100644 >> --- a/qga/commands-posix.c >> +++ b/qga/commands-posix.c >> @@ -2783,6 +2783,96 @@ 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; >> + char dev_name[MAX_NAME_LEN]; >> + int i; >> + 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; >> + >> + fp = fopen(diskstats, "r"); >> + if (fp == NULL) { >> + error_setg_errno(errp, errno, "open(\"%s\")", diskstats); >> + return NULL; >> + } >> + while (getline(&line, &n, fp) != -1) { >> + GuestDiskStatsInfo *diskstatinfo; >> + GuestDiskStats *diskstat; >> + 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); >> + >> + diskstatinfo = g_new0(GuestDiskStatsInfo, 1); >> + diskstat = g_new0(GuestDiskStats, 1); >> > + if (i < 7) { >> > > diskstatinfo and diskstat pointers do not have g_autoptr attribute and > will be not added to the results list. > So, looks like we have a memory leak in this case. > checked with valgrind. we have a memory leak of diskstatinfo and diskstat when i < 7 I suggest allocating these variables after this condition. > > >> + continue; >> + } >> + diskstatinfo->name = g_strdup(dev_name); >> + diskstatinfo->major = major; >> + diskstatinfo->minor = minor; >> + if (i == 7) { >> + diskstat->read_ios = rd_ios; >> + diskstat->read_sectors = rd_merges_or_rd_sec; >> + diskstat->write_ios = rd_sec_or_wr_ios; >> + diskstat->write_sectors = rd_ticks_or_wr_sec; >> + } >> + if (i >= 14) { >> + diskstat->read_ios = rd_ios; >> + diskstat->read_sectors = rd_sec_or_wr_ios; >> + diskstat->read_merges = rd_merges_or_rd_sec; >> + diskstat->read_ticks = rd_ticks_or_wr_sec; >> + diskstat->write_ios = wr_ios; >> + diskstat->write_sectors = wr_sec; >> + diskstat->write_merges = wr_merges; >> + diskstat->write_ticks = wr_ticks; >> + diskstat->ios_pgr = ios_pgr; >> + diskstat->total_ticks = tot_ticks; >> + diskstat->weight_ticks = rq_ticks; >> + } >> + if (i >= 18) { >> + diskstat->discard_ios = dc_ios; >> + diskstat->discard_merges = dc_merges; >> + diskstat->discard_sectors = dc_sec; >> + diskstat->discard_ticks = dc_ticks; >> + } >> + if (i >= 20) { >> + diskstat->flush_ios = fl_ios; >> + diskstat->flush_ticks = fl_ticks; >> + } >> + >> + diskstatinfo->stats = diskstat; >> + QAPI_LIST_APPEND(tail, diskstatinfo); >> + } >> + g_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 +3221,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..94aad4f2ae 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 >> >> >> >>
