* [PATCH v2 0/1] qga: add command 'guest-get-cpustats' @ 2022-07-04 2:36 zhenwei pi 2022-07-04 2:36 ` [PATCH v2 1/1] " zhenwei pi 0 siblings, 1 reply; 9+ messages in thread From: zhenwei pi @ 2022-07-04 2:36 UTC (permalink / raw) To: michael.roth, kkostiuk; +Cc: qemu-devel, marcandre.lureau, zhenwei pi v1 -> v2: - Konstantin & Marc-André pointed out that the structure 'GuestCpuStats' is too *linux style*, so re-define it to 'GuestLinuxCpuStats', and use an union type of 'GuestCpuStats'. - Modify comment info from 'man proc', also add linux version infomation. - Test sscanf return value by '(i == EOF)' (To Marc-André: name is declared as 'char name[64];', so we can't test '!name'). - Suggested by Marc-André, use 'int clk_tck = sysconf(_SC_CLK_TCK);' instead of hard code. v1: - Implement guest agent command 'guest-get-cpustats' Zhenwei Pi (1): qga: add command 'guest-get-cpustats' qga/commands-posix.c | 89 ++++++++++++++++++++++++++++++++++++++++++++ qga/commands-win32.c | 6 +++ qga/qapi-schema.json | 81 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 176 insertions(+) -- 2.20.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/1] qga: add command 'guest-get-cpustats' 2022-07-04 2:36 [PATCH v2 0/1] qga: add command 'guest-get-cpustats' zhenwei pi @ 2022-07-04 2:36 ` zhenwei pi 2022-07-04 7:28 ` Marc-André Lureau 0 siblings, 1 reply; 9+ messages in thread From: zhenwei pi @ 2022-07-04 2:36 UTC (permalink / raw) To: michael.roth, kkostiuk; +Cc: qemu-devel, marcandre.lureau, zhenwei pi A vCPU thread always reaches 100% utilization when: - guest uses idle=poll - disable HLT vm-exit - enable MWAIT Add new guest agent command 'guest-get-cpustats' to get guest CPU statistics, we can know the guest workload and how busy the CPU is. Signed-off-by: zhenwei pi <pizhenwei@bytedance.com> --- qga/commands-posix.c | 89 ++++++++++++++++++++++++++++++++++++++++++++ qga/commands-win32.c | 6 +++ qga/qapi-schema.json | 81 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 176 insertions(+) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 0469dc409d..e51f4c8f03 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -2893,6 +2893,90 @@ GuestDiskStatsInfoList *qmp_guest_get_diskstats(Error **errp) return guest_get_diskstats(errp); } +GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp) +{ + GuestCpuStatsList *head = NULL, **tail = &head; + const char *cpustats = "/proc/stat"; + int clk_tck = sysconf(_SC_CLK_TCK); + FILE *fp; + size_t n; + char *line = NULL; + + fp = fopen(cpustats, "r"); + if (fp == NULL) { + error_setg_errno(errp, errno, "open(\"%s\")", cpustats); + return NULL; + } + + while (getline(&line, &n, fp) != -1) { + GuestCpuStats *cpustat = NULL; + GuestLinuxCpuStats *linuxcpustat; + int i; + unsigned long user, system, idle, iowait, irq, softirq, steal, guest; + unsigned long nice, guest_nice; + char name[64]; + + i = sscanf(line, "%s %lu %lu %lu %lu %lu %lu %lu %lu %lu %lu", + name, &user, &nice, &system, &idle, &iowait, &irq, &softirq, + &steal, &guest, &guest_nice); + + /* drop "cpu 1 2 3 ...", get "cpuX 1 2 3 ..." only */ + if ((i == EOF) || strncmp(name, "cpu", 3) || (name[3] == '\0')) { + continue; + } + + if (i < 5) { + slog("Parsing cpu stat from %s failed, see \"man proc\"", cpustats); + break; + } + + cpustat = g_new0(GuestCpuStats, 1); + cpustat->type = GUEST_OS_TYPE_LINUXOS; + + linuxcpustat = &cpustat->u.linuxos; + linuxcpustat->cpu = atoi(&name[3]); + linuxcpustat->user = user * 1000 / clk_tck; + linuxcpustat->nice = nice * 1000 / clk_tck; + linuxcpustat->system = system * 1000 / clk_tck; + linuxcpustat->idle = idle * 1000 / clk_tck; + + if (i > 5) { + linuxcpustat->has_iowait = true; + linuxcpustat->iowait = iowait * 1000 / clk_tck; + } + + if (i > 6) { + linuxcpustat->has_irq = true; + linuxcpustat->irq = irq * 1000 / clk_tck; + linuxcpustat->has_softirq = true; + linuxcpustat->softirq = softirq * 1000 / clk_tck; + } + + if (i > 8) { + linuxcpustat->has_steal = true; + linuxcpustat->steal = steal * 1000 / clk_tck; + } + + if (i > 9) { + linuxcpustat->has_guest = true; + linuxcpustat->guest = guest * 1000 / clk_tck; + } + + if (i > 10) { + linuxcpustat->has_guest = true; + linuxcpustat->guest = guest * 1000 / clk_tck; + linuxcpustat->has_guestnice = true; + linuxcpustat->guestnice = guest_nice * 1000 / clk_tck; + } + + QAPI_LIST_APPEND(tail, cpustat); + } + + free(line); + fclose(fp); + return head; +} + #else /* defined(__linux__) */ void qmp_guest_suspend_disk(Error **errp) @@ -3247,6 +3331,11 @@ GuestDiskStatsInfoList *qmp_guest_get_diskstats(Error **errp) return NULL; } +GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp) +{ + error_setg(errp, QERR_UNSUPPORTED); + return NULL; +} #endif /* CONFIG_FSFREEZE */ diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 36f94c0f9c..7ed7664715 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -2543,3 +2543,9 @@ GuestDiskStatsInfoList *qmp_guest_get_diskstats(Error **errp) error_setg(errp, QERR_UNSUPPORTED); return NULL; } + +GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp) +{ + error_setg(errp, QERR_UNSUPPORTED); + return NULL; +} diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index 9fa20e791b..3b795222e9 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -1576,3 +1576,84 @@ { 'command': 'guest-get-diskstats', 'returns': ['GuestDiskStatsInfo'] } + +## +# @GuestOsType: +# +# An enumeration of OS type +# +# Since: 7.1 +## +{ 'enum': 'GuestOsType', + 'data': [ 'linuxos', 'windowsos' ] } + + +## +# @GuestLinuxCpuStats: +# +# CPU statistics of Linux +# +# @cpu: CPU index in guest OS +# +# @user: Time spent in user mode +# +# @nice: Time spent in user mode with low priority (nice) +# +# @system: Time spent in system mode +# +# @idle: Time spent in the idle task +# +# @iowait: Time waiting for I/O to complete (since Linux 2.5.41) +# +# @irq: Time servicing interrupts (since Linux 2.6.0-test4) +# +# @softirq: Time servicing softirqs (since Linux 2.6.0-test4) +# +# @steal: Stolen time by host (since Linux 2.6.11) +# +# @guest: ime spent running a virtual CPU for guest operating systems under +# the control of the Linux kernel (since Linux 2.6.24) +# +# @guestnice: Time spent running a niced guest (since Linux 2.6.33) +# +# Since: 7.1 +## +{ 'struct': 'GuestLinuxCpuStats', + 'data': {'cpu': 'int', + 'user': 'uint64', + 'nice': 'uint64', + 'system': 'uint64', + 'idle': 'uint64', + '*iowait': 'uint64', + '*irq': 'uint64', + '*softirq': 'uint64', + '*steal': 'uint64', + '*guest': 'uint64', + '*guestnice': 'uint64' + } } + +## +# @GuestCpuStats: +# +# Get statistics of each CPU in millisecond. +# +# - @linux: Linux style CPU statistics +# +# Since: 7.1 +## +{ 'union': 'GuestCpuStats', + 'base': { 'type': 'GuestOsType' }, + 'discriminator': 'type', + 'data': { 'linuxos': 'GuestLinuxCpuStats' } } + +## +# @guest-get-cpustats: +# +# Retrieve information about CPU stats. +# Returns: List of CPU stats of guest. +# +# Since: 7.1 +## +{ 'command': 'guest-get-cpustats', + 'returns': ['GuestCpuStats'] +} -- 2.20.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/1] qga: add command 'guest-get-cpustats' 2022-07-04 2:36 ` [PATCH v2 1/1] " zhenwei pi @ 2022-07-04 7:28 ` Marc-André Lureau 2022-07-04 7:29 ` Marc-André Lureau 2022-07-04 8:00 ` zhenwei pi 0 siblings, 2 replies; 9+ messages in thread From: Marc-André Lureau @ 2022-07-04 7:28 UTC (permalink / raw) To: zhenwei pi, Markus Armbruster; +Cc: Michael Roth, Konstantin Kostiuk, QEMU [-- Attachment #1: Type: text/plain, Size: 7211 bytes --] Hi On Mon, Jul 4, 2022 at 6:42 AM zhenwei pi <pizhenwei@bytedance.com> wrote: > A vCPU thread always reaches 100% utilization when: > - guest uses idle=poll > - disable HLT vm-exit > - enable MWAIT > > Add new guest agent command 'guest-get-cpustats' to get guest CPU > statistics, we can know the guest workload and how busy the CPU is. > > Signed-off-by: zhenwei pi <pizhenwei@bytedance.com> > --- > qga/commands-posix.c | 89 ++++++++++++++++++++++++++++++++++++++++++++ > qga/commands-win32.c | 6 +++ > qga/qapi-schema.json | 81 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 176 insertions(+) > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index 0469dc409d..e51f4c8f03 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -2893,6 +2893,90 @@ GuestDiskStatsInfoList > *qmp_guest_get_diskstats(Error **errp) > return guest_get_diskstats(errp); > } > > +GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp) > +{ > + GuestCpuStatsList *head = NULL, **tail = &head; > + const char *cpustats = "/proc/stat"; > + int clk_tck = sysconf(_SC_CLK_TCK); > + FILE *fp; > + size_t n; > + char *line = NULL; > + > + fp = fopen(cpustats, "r"); > + if (fp == NULL) { > + error_setg_errno(errp, errno, "open(\"%s\")", cpustats); > + return NULL; > + } > + > + while (getline(&line, &n, fp) != -1) { > + GuestCpuStats *cpustat = NULL; > + GuestLinuxCpuStats *linuxcpustat; > + int i; > + unsigned long user, system, idle, iowait, irq, softirq, steal, > guest; > + unsigned long nice, guest_nice; > + char name[64]; > + > + i = sscanf(line, "%s %lu %lu %lu %lu %lu %lu %lu %lu %lu %lu", > + name, &user, &nice, &system, &idle, &iowait, &irq, > &softirq, > + &steal, &guest, &guest_nice); > + > + /* drop "cpu 1 2 3 ...", get "cpuX 1 2 3 ..." only */ > + if ((i == EOF) || strncmp(name, "cpu", 3) || (name[3] == '\0')) { > + continue; > + } > + > + if (i < 5) { > + slog("Parsing cpu stat from %s failed, see \"man proc\"", > cpustats); > + break; > + } > + > + cpustat = g_new0(GuestCpuStats, 1); > + cpustat->type = GUEST_OS_TYPE_LINUXOS; > + > + linuxcpustat = &cpustat->u.linuxos; > + linuxcpustat->cpu = atoi(&name[3]); > + linuxcpustat->user = user * 1000 / clk_tck; > + linuxcpustat->nice = nice * 1000 / clk_tck; > + linuxcpustat->system = system * 1000 / clk_tck; > + linuxcpustat->idle = idle * 1000 / clk_tck; > + > + if (i > 5) { > + linuxcpustat->has_iowait = true; > + linuxcpustat->iowait = iowait * 1000 / clk_tck; > + } > + > + if (i > 6) { > + linuxcpustat->has_irq = true; > + linuxcpustat->irq = irq * 1000 / clk_tck; > + linuxcpustat->has_softirq = true; > + linuxcpustat->softirq = softirq * 1000 / clk_tck; > + } > + > + if (i > 8) { > + linuxcpustat->has_steal = true; > + linuxcpustat->steal = steal * 1000 / clk_tck; > + } > + > + if (i > 9) { > + linuxcpustat->has_guest = true; > + linuxcpustat->guest = guest * 1000 / clk_tck; > + } > + > + if (i > 10) { > + linuxcpustat->has_guest = true; > + linuxcpustat->guest = guest * 1000 / clk_tck; > + linuxcpustat->has_guestnice = true; > + linuxcpustat->guestnice = guest_nice * 1000 / clk_tck; > + } > + > + QAPI_LIST_APPEND(tail, cpustat); > + } > + > + free(line); > + fclose(fp); > + return head; > +} > + > #else /* defined(__linux__) */ > > void qmp_guest_suspend_disk(Error **errp) > @@ -3247,6 +3331,11 @@ GuestDiskStatsInfoList > *qmp_guest_get_diskstats(Error **errp) > return NULL; > } > > +GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp) > +{ > + error_setg(errp, QERR_UNSUPPORTED); > + return NULL; > +} > > #endif /* CONFIG_FSFREEZE */ > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > index 36f94c0f9c..7ed7664715 100644 > --- a/qga/commands-win32.c > +++ b/qga/commands-win32.c > @@ -2543,3 +2543,9 @@ GuestDiskStatsInfoList > *qmp_guest_get_diskstats(Error **errp) > error_setg(errp, QERR_UNSUPPORTED); > return NULL; > } > + > +GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp) > +{ > + error_setg(errp, QERR_UNSUPPORTED); > + return NULL; > +} > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json > index 9fa20e791b..3b795222e9 100644 > --- a/qga/qapi-schema.json > +++ b/qga/qapi-schema.json > @@ -1576,3 +1576,84 @@ > { 'command': 'guest-get-diskstats', > 'returns': ['GuestDiskStatsInfo'] > } > + > +## > +# @GuestOsType: > +# > +# An enumeration of OS type > +# > +# Since: 7.1 > +## > +{ 'enum': 'GuestOsType', > + 'data': [ 'linuxos', 'windowsos' ] } > I would rather keep this enum specific to GuestCpuStats, "GuestLinuxCpuStatsType"? I would also drop the "os" suffix + > + > +## > +# @GuestLinuxCpuStats: > +# > +# CPU statistics of Linux > +# > +# @cpu: CPU index in guest OS > +# > +# @user: Time spent in user mode > +# > +# @nice: Time spent in user mode with low priority (nice) > +# > +# @system: Time spent in system mode > +# > +# @idle: Time spent in the idle task > +# > +# @iowait: Time waiting for I/O to complete (since Linux 2.5.41) > +# > +# @irq: Time servicing interrupts (since Linux 2.6.0-test4) > +# > +# @softirq: Time servicing softirqs (since Linux 2.6.0-test4) > +# > +# @steal: Stolen time by host (since Linux 2.6.11) > +# > +# @guest: ime spent running a virtual CPU for guest operating systems > under > +# the control of the Linux kernel (since Linux 2.6.24) > +# > +# @guestnice: Time spent running a niced guest (since Linux 2.6.33) > +# > +# Since: 7.1 > +## > +{ 'struct': 'GuestLinuxCpuStats', > + 'data': {'cpu': 'int', > + 'user': 'uint64', > + 'nice': 'uint64', > + 'system': 'uint64', > + 'idle': 'uint64', > + '*iowait': 'uint64', > + '*irq': 'uint64', > + '*softirq': 'uint64', > + '*steal': 'uint64', > + '*guest': 'uint64', > + '*guestnice': 'uint64' > + } } > + > +## > +# @GuestCpuStats: > +# > +# Get statistics of each CPU in millisecond. > +# > +# - @linux: Linux style CPU statistics > +# > +# Since: 7.1 > +## > +{ 'union': 'GuestCpuStats', > + 'base': { 'type': 'GuestOsType' }, > + 'discriminator': 'type', > + 'data': { 'linuxos': 'GuestLinuxCpuStats' } } > + > +## > +# @guest-get-cpustats: > +# > +# Retrieve information about CPU stats. > +# Returns: List of CPU stats of guest. > +# > +# Since: 7.1 > +## > +{ 'command': 'guest-get-cpustats', > + 'returns': ['GuestCpuStats'] > +} > -- > 2.20.1 > > > Looks good to me otherwise. thanks -- Marc-André Lureau [-- Attachment #2: Type: text/html, Size: 9369 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/1] qga: add command 'guest-get-cpustats' 2022-07-04 7:28 ` Marc-André Lureau @ 2022-07-04 7:29 ` Marc-André Lureau 2022-07-04 8:00 ` zhenwei pi 1 sibling, 0 replies; 9+ messages in thread From: Marc-André Lureau @ 2022-07-04 7:29 UTC (permalink / raw) To: zhenwei pi, Markus Armbruster; +Cc: Michael Roth, Konstantin Kostiuk, QEMU [-- Attachment #1: Type: text/plain, Size: 7638 bytes --] On Mon, Jul 4, 2022 at 11:28 AM Marc-André Lureau < marcandre.lureau@gmail.com> wrote: > Hi > > On Mon, Jul 4, 2022 at 6:42 AM zhenwei pi <pizhenwei@bytedance.com> wrote: > >> A vCPU thread always reaches 100% utilization when: >> - guest uses idle=poll >> - disable HLT vm-exit >> - enable MWAIT >> >> Add new guest agent command 'guest-get-cpustats' to get guest CPU >> statistics, we can know the guest workload and how busy the CPU is. >> >> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com> >> --- >> qga/commands-posix.c | 89 ++++++++++++++++++++++++++++++++++++++++++++ >> qga/commands-win32.c | 6 +++ >> qga/qapi-schema.json | 81 ++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 176 insertions(+) >> >> diff --git a/qga/commands-posix.c b/qga/commands-posix.c >> index 0469dc409d..e51f4c8f03 100644 >> --- a/qga/commands-posix.c >> +++ b/qga/commands-posix.c >> @@ -2893,6 +2893,90 @@ GuestDiskStatsInfoList >> *qmp_guest_get_diskstats(Error **errp) >> return guest_get_diskstats(errp); >> } >> >> +GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp) >> +{ >> + GuestCpuStatsList *head = NULL, **tail = &head; >> + const char *cpustats = "/proc/stat"; >> + int clk_tck = sysconf(_SC_CLK_TCK); >> + FILE *fp; >> + size_t n; >> + char *line = NULL; >> + >> + fp = fopen(cpustats, "r"); >> + if (fp == NULL) { >> + error_setg_errno(errp, errno, "open(\"%s\")", cpustats); >> + return NULL; >> + } >> + >> + while (getline(&line, &n, fp) != -1) { >> + GuestCpuStats *cpustat = NULL; >> + GuestLinuxCpuStats *linuxcpustat; >> + int i; >> + unsigned long user, system, idle, iowait, irq, softirq, steal, >> guest; >> + unsigned long nice, guest_nice; >> + char name[64]; >> + >> + i = sscanf(line, "%s %lu %lu %lu %lu %lu %lu %lu %lu %lu %lu", >> + name, &user, &nice, &system, &idle, &iowait, &irq, >> &softirq, >> + &steal, &guest, &guest_nice); >> + >> + /* drop "cpu 1 2 3 ...", get "cpuX 1 2 3 ..." only */ >> + if ((i == EOF) || strncmp(name, "cpu", 3) || (name[3] == '\0')) { >> + continue; >> + } >> + >> + if (i < 5) { >> + slog("Parsing cpu stat from %s failed, see \"man proc\"", >> cpustats); >> + break; >> + } >> + >> + cpustat = g_new0(GuestCpuStats, 1); >> + cpustat->type = GUEST_OS_TYPE_LINUXOS; >> + >> + linuxcpustat = &cpustat->u.linuxos; >> + linuxcpustat->cpu = atoi(&name[3]); >> + linuxcpustat->user = user * 1000 / clk_tck; >> + linuxcpustat->nice = nice * 1000 / clk_tck; >> + linuxcpustat->system = system * 1000 / clk_tck; >> + linuxcpustat->idle = idle * 1000 / clk_tck; >> + >> + if (i > 5) { >> + linuxcpustat->has_iowait = true; >> + linuxcpustat->iowait = iowait * 1000 / clk_tck; >> + } >> + >> + if (i > 6) { >> + linuxcpustat->has_irq = true; >> + linuxcpustat->irq = irq * 1000 / clk_tck; >> + linuxcpustat->has_softirq = true; >> + linuxcpustat->softirq = softirq * 1000 / clk_tck; >> + } >> + >> + if (i > 8) { >> + linuxcpustat->has_steal = true; >> + linuxcpustat->steal = steal * 1000 / clk_tck; >> + } >> + >> + if (i > 9) { >> + linuxcpustat->has_guest = true; >> + linuxcpustat->guest = guest * 1000 / clk_tck; >> + } >> + >> + if (i > 10) { >> + linuxcpustat->has_guest = true; >> + linuxcpustat->guest = guest * 1000 / clk_tck; >> + linuxcpustat->has_guestnice = true; >> + linuxcpustat->guestnice = guest_nice * 1000 / clk_tck; >> + } >> + >> + QAPI_LIST_APPEND(tail, cpustat); >> + } >> + >> + free(line); >> + fclose(fp); >> + return head; >> +} >> + >> #else /* defined(__linux__) */ >> >> void qmp_guest_suspend_disk(Error **errp) >> @@ -3247,6 +3331,11 @@ GuestDiskStatsInfoList >> *qmp_guest_get_diskstats(Error **errp) >> return NULL; >> } >> >> +GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp) >> +{ >> + error_setg(errp, QERR_UNSUPPORTED); >> + return NULL; >> +} >> >> #endif /* CONFIG_FSFREEZE */ >> >> diff --git a/qga/commands-win32.c b/qga/commands-win32.c >> index 36f94c0f9c..7ed7664715 100644 >> --- a/qga/commands-win32.c >> +++ b/qga/commands-win32.c >> @@ -2543,3 +2543,9 @@ GuestDiskStatsInfoList >> *qmp_guest_get_diskstats(Error **errp) >> error_setg(errp, QERR_UNSUPPORTED); >> return NULL; >> } >> + >> +GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp) >> +{ >> + error_setg(errp, QERR_UNSUPPORTED); >> + return NULL; >> +} >> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json >> index 9fa20e791b..3b795222e9 100644 >> --- a/qga/qapi-schema.json >> +++ b/qga/qapi-schema.json >> @@ -1576,3 +1576,84 @@ >> { 'command': 'guest-get-diskstats', >> 'returns': ['GuestDiskStatsInfo'] >> } >> + >> +## >> +# @GuestOsType: >> +# >> +# An enumeration of OS type >> +# >> +# Since: 7.1 >> +## >> +{ 'enum': 'GuestOsType', >> + 'data': [ 'linuxos', 'windowsos' ] } >> > > I would rather keep this enum specific to GuestCpuStats, > "GuestLinuxCpuStatsType"? > Oops, I meant "GuestCpuStatsType" > I would also drop the "os" suffix > > + >> + >> +## >> +# @GuestLinuxCpuStats: >> +# >> +# CPU statistics of Linux >> +# >> +# @cpu: CPU index in guest OS >> +# >> +# @user: Time spent in user mode >> +# >> +# @nice: Time spent in user mode with low priority (nice) >> +# >> +# @system: Time spent in system mode >> +# >> +# @idle: Time spent in the idle task >> +# >> +# @iowait: Time waiting for I/O to complete (since Linux 2.5.41) >> +# >> +# @irq: Time servicing interrupts (since Linux 2.6.0-test4) >> +# >> +# @softirq: Time servicing softirqs (since Linux 2.6.0-test4) >> +# >> +# @steal: Stolen time by host (since Linux 2.6.11) >> +# >> +# @guest: ime spent running a virtual CPU for guest operating systems >> under >> +# the control of the Linux kernel (since Linux 2.6.24) >> +# >> +# @guestnice: Time spent running a niced guest (since Linux 2.6.33) >> +# >> +# Since: 7.1 >> +## >> +{ 'struct': 'GuestLinuxCpuStats', >> + 'data': {'cpu': 'int', >> + 'user': 'uint64', >> + 'nice': 'uint64', >> + 'system': 'uint64', >> + 'idle': 'uint64', >> + '*iowait': 'uint64', >> + '*irq': 'uint64', >> + '*softirq': 'uint64', >> + '*steal': 'uint64', >> + '*guest': 'uint64', >> + '*guestnice': 'uint64' >> + } } >> + >> +## >> +# @GuestCpuStats: >> +# >> +# Get statistics of each CPU in millisecond. >> +# >> +# - @linux: Linux style CPU statistics >> +# >> +# Since: 7.1 >> +## >> +{ 'union': 'GuestCpuStats', >> + 'base': { 'type': 'GuestOsType' }, >> + 'discriminator': 'type', >> + 'data': { 'linuxos': 'GuestLinuxCpuStats' } } >> + >> +## >> +# @guest-get-cpustats: >> +# >> +# Retrieve information about CPU stats. >> +# Returns: List of CPU stats of guest. >> +# >> +# Since: 7.1 >> +## >> +{ 'command': 'guest-get-cpustats', >> + 'returns': ['GuestCpuStats'] >> +} >> -- >> 2.20.1 >> >> >> > Looks good to me otherwise. > thanks > > -- > Marc-André Lureau > -- Marc-André Lureau [-- Attachment #2: Type: text/html, Size: 10127 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Re: [PATCH v2 1/1] qga: add command 'guest-get-cpustats' 2022-07-04 7:28 ` Marc-André Lureau 2022-07-04 7:29 ` Marc-André Lureau @ 2022-07-04 8:00 ` zhenwei pi 2022-07-06 3:04 ` zhenwei pi 1 sibling, 1 reply; 9+ messages in thread From: zhenwei pi @ 2022-07-04 8:00 UTC (permalink / raw) To: Marc-André Lureau, Markus Armbruster Cc: Michael Roth, Konstantin Kostiuk, QEMU On 7/4/22 15:28, Marc-André Lureau wrote: > Hi > > On Mon, Jul 4, 2022 at 6:42 AM zhenwei pi <pizhenwei@bytedance.com > <mailto:pizhenwei@bytedance.com>> wrote: > > A vCPU thread always reaches 100% utilization when: > - guest uses idle=poll > - disable HLT vm-exit > - enable MWAIT > > Add new guest agent command 'guest-get-cpustats' to get guest CPU > statistics, we can know the guest workload and how busy the CPU is. > > Signed-off-by: zhenwei pi <pizhenwei@bytedance.com > <mailto:pizhenwei@bytedance.com>> > --- > qga/commands-posix.c | 89 ++++++++++++++++++++++++++++++++++++++++++++ > qga/commands-win32.c | 6 +++ > qga/qapi-schema.json | 81 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 176 insertions(+) > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index 0469dc409d..e51f4c8f03 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -2893,6 +2893,90 @@ GuestDiskStatsInfoList > *qmp_guest_get_diskstats(Error **errp) > return guest_get_diskstats(errp); > } > > +GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp) > +{ > + GuestCpuStatsList *head = NULL, **tail = &head; > + const char *cpustats = "/proc/stat"; > + int clk_tck = sysconf(_SC_CLK_TCK); > + FILE *fp; > + size_t n; > + char *line = NULL; > + > + fp = fopen(cpustats, "r"); > + if (fp == NULL) { > + error_setg_errno(errp, errno, "open(\"%s\")", cpustats); > + return NULL; > + } > + > + while (getline(&line, &n, fp) != -1) { > + GuestCpuStats *cpustat = NULL; > + GuestLinuxCpuStats *linuxcpustat; > + int i; > + unsigned long user, system, idle, iowait, irq, softirq, > steal, guest; > + unsigned long nice, guest_nice; > + char name[64]; > + > + i = sscanf(line, "%s %lu %lu %lu %lu %lu %lu %lu %lu %lu %lu", > + name, &user, &nice, &system, &idle, &iowait, > &irq, &softirq, > + &steal, &guest, &guest_nice); > + > + /* drop "cpu 1 2 3 ...", get "cpuX 1 2 3 ..." only */ > + if ((i == EOF) || strncmp(name, "cpu", 3) || (name[3] == > '\0')) { > + continue; > + } > + > + if (i < 5) { > + slog("Parsing cpu stat from %s failed, see \"man > proc\"", cpustats); > + break; > + } > + > + cpustat = g_new0(GuestCpuStats, 1); > + cpustat->type = GUEST_OS_TYPE_LINUXOS; > + > + linuxcpustat = &cpustat->u.linuxos; > + linuxcpustat->cpu = atoi(&name[3]); > + linuxcpustat->user = user * 1000 / clk_tck; > + linuxcpustat->nice = nice * 1000 / clk_tck; > + linuxcpustat->system = system * 1000 / clk_tck; > + linuxcpustat->idle = idle * 1000 / clk_tck; > + > + if (i > 5) { > + linuxcpustat->has_iowait = true; > + linuxcpustat->iowait = iowait * 1000 / clk_tck; > + } > + > + if (i > 6) { > + linuxcpustat->has_irq = true; > + linuxcpustat->irq = irq * 1000 / clk_tck; > + linuxcpustat->has_softirq = true; > + linuxcpustat->softirq = softirq * 1000 / clk_tck; > + } > + > + if (i > 8) { > + linuxcpustat->has_steal = true; > + linuxcpustat->steal = steal * 1000 / clk_tck; > + } > + > + if (i > 9) { > + linuxcpustat->has_guest = true; > + linuxcpustat->guest = guest * 1000 / clk_tck; > + } > + > + if (i > 10) { > + linuxcpustat->has_guest = true; > + linuxcpustat->guest = guest * 1000 / clk_tck; > + linuxcpustat->has_guestnice = true; > + linuxcpustat->guestnice = guest_nice * 1000 / clk_tck; > + } > + > + QAPI_LIST_APPEND(tail, cpustat); > + } > + > + free(line); > + fclose(fp); > + return head; > +} > + > #else /* defined(__linux__) */ > > void qmp_guest_suspend_disk(Error **errp) > @@ -3247,6 +3331,11 @@ GuestDiskStatsInfoList > *qmp_guest_get_diskstats(Error **errp) > return NULL; > } > > +GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp) > +{ > + error_setg(errp, QERR_UNSUPPORTED); > + return NULL; > +} > > #endif /* CONFIG_FSFREEZE */ > > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > index 36f94c0f9c..7ed7664715 100644 > --- a/qga/commands-win32.c > +++ b/qga/commands-win32.c > @@ -2543,3 +2543,9 @@ GuestDiskStatsInfoList > *qmp_guest_get_diskstats(Error **errp) > error_setg(errp, QERR_UNSUPPORTED); > return NULL; > } > + > +GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp) > +{ > + error_setg(errp, QERR_UNSUPPORTED); > + return NULL; > +} > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json > index 9fa20e791b..3b795222e9 100644 > --- a/qga/qapi-schema.json > +++ b/qga/qapi-schema.json > @@ -1576,3 +1576,84 @@ > { 'command': 'guest-get-diskstats', > 'returns': ['GuestDiskStatsInfo'] > } > + > +## > +# @GuestOsType: > +# > +# An enumeration of OS type > +# > +# Since: 7.1 > +## > +{ 'enum': 'GuestOsType', > + 'data': [ 'linuxos', 'windowsos' ] } > > > I would rather keep this enum specific to GuestCpuStats, > "GuestLinuxCpuStatsType"? > Hi, 'GuestOsType' may be re-used in the future, not only for the CPU statistics case. > I would also drop the "os" suffix > I'm afraid we can not drop "os" suffix, build this without "os" suffix: qga/qga-qapi-types.h:948:28: error: expected member name or ';' after declaration specifiers GuestLinuxCpuStats linux; ~~~~~~~~~~~~~~~~~~ ^ <built-in>:336:15: note: expanded from here #define linux 1 > + > + > +## > +# @GuestLinuxCpuStats: > +# > +# CPU statistics of Linux > +# > +# @cpu: CPU index in guest OS > +# > +# @user: Time spent in user mode > +# > +# @nice: Time spent in user mode with low priority (nice) > +# > +# @system: Time spent in system mode > +# > +# @idle: Time spent in the idle task > +# > +# @iowait: Time waiting for I/O to complete (since Linux 2.5.41) > +# > +# @irq: Time servicing interrupts (since Linux 2.6.0-test4) > +# > +# @softirq: Time servicing softirqs (since Linux 2.6.0-test4) > +# > +# @steal: Stolen time by host (since Linux 2.6.11) > +# > +# @guest: ime spent running a virtual CPU for guest operating > systems under > +# the control of the Linux kernel (since Linux 2.6.24) > +# > +# @guestnice: Time spent running a niced guest (since Linux 2.6.33) > +# > +# Since: 7.1 > +## > +{ 'struct': 'GuestLinuxCpuStats', > + 'data': {'cpu': 'int', > + 'user': 'uint64', > + 'nice': 'uint64', > + 'system': 'uint64', > + 'idle': 'uint64', > + '*iowait': 'uint64', > + '*irq': 'uint64', > + '*softirq': 'uint64', > + '*steal': 'uint64', > + '*guest': 'uint64', > + '*guestnice': 'uint64' > + } } > + > +## > +# @GuestCpuStats: > +# > +# Get statistics of each CPU in millisecond. > +# > +# - @linux: Linux style CPU statistics > +# > +# Since: 7.1 > +## > +{ 'union': 'GuestCpuStats', > + 'base': { 'type': 'GuestOsType' }, > + 'discriminator': 'type', > + 'data': { 'linuxos': 'GuestLinuxCpuStats' } } > + > +## > +# @guest-get-cpustats: > +# > +# Retrieve information about CPU stats. > +# Returns: List of CPU stats of guest. > +# > +# Since: 7.1 > +## > +{ 'command': 'guest-get-cpustats', > + 'returns': ['GuestCpuStats'] > +} > -- > 2.20.1 > > > > Looks good to me otherwise. > thanks > > -- > Marc-André Lureau -- zhenwei pi ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/1] qga: add command 'guest-get-cpustats' 2022-07-04 8:00 ` zhenwei pi @ 2022-07-06 3:04 ` zhenwei pi 2022-07-06 7:20 ` Marc-André Lureau 0 siblings, 1 reply; 9+ messages in thread From: zhenwei pi @ 2022-07-06 3:04 UTC (permalink / raw) To: Marc-André Lureau, Markus Armbruster Cc: Michael Roth, Konstantin Kostiuk, QEMU On 7/4/22 16:00, zhenwei pi wrote: > > >> +## >> +# @GuestOsType: >> +# >> +# An enumeration of OS type >> +# >> +# Since: 7.1 >> +## >> +{ 'enum': 'GuestOsType', >> + 'data': [ 'linuxos', 'windowsos' ] } >> >> >> I would rather keep this enum specific to GuestCpuStats, >> "GuestLinuxCpuStatsType"? >> > > Hi, > > 'GuestOsType' may be re-used in the future, not only for the CPU > statistics case. > >> I would also drop the "os" suffix >> > I'm afraid we can not drop "os" suffix, build this without "os" suffix: > qga/qga-qapi-types.h:948:28: error: expected member name or ';' after > declaration specifiers > GuestLinuxCpuStats linux; > ~~~~~~~~~~~~~~~~~~ ^ > <built-in>:336:15: note: expanded from here > #define linux 1 > Hi, Marc Could you please give any hint about this issue? >> + >> + >> >> >> >> Looks good to me otherwise. >> thanks >> >> -- >> Marc-André Lureau > -- zhenwei pi ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/1] qga: add command 'guest-get-cpustats' 2022-07-06 3:04 ` zhenwei pi @ 2022-07-06 7:20 ` Marc-André Lureau 2022-07-06 7:44 ` zhenwei pi 0 siblings, 1 reply; 9+ messages in thread From: Marc-André Lureau @ 2022-07-06 7:20 UTC (permalink / raw) To: zhenwei pi; +Cc: Markus Armbruster, Michael Roth, Konstantin Kostiuk, QEMU [-- Attachment #1: Type: text/plain, Size: 1863 bytes --] Hi On Wed, Jul 6, 2022 at 7:09 AM zhenwei pi <pizhenwei@bytedance.com> wrote: > On 7/4/22 16:00, zhenwei pi wrote: > > > > > >> +## > >> +# @GuestOsType: > >> +# > >> +# An enumeration of OS type > >> +# > >> +# Since: 7.1 > >> +## > >> +{ 'enum': 'GuestOsType', > >> + 'data': [ 'linuxos', 'windowsos' ] } > >> > >> > >> I would rather keep this enum specific to GuestCpuStats, > >> "GuestLinuxCpuStatsType"? > >> > > > > Hi, > > > > 'GuestOsType' may be re-used in the future, not only for the CPU > > statistics case. > > > >> I would also drop the "os" suffix > >> > > I'm afraid we can not drop "os" suffix, build this without "os" suffix: > > qga/qga-qapi-types.h:948:28: error: expected member name or ';' after > > declaration specifiers > > GuestLinuxCpuStats linux; > > ~~~~~~~~~~~~~~~~~~ ^ > > <built-in>:336:15: note: expanded from here > > #define linux 1 > > > > Hi, Marc > > Could you please give any hint about this issue? > Yes, it looks like we need to add "linux" to the "polluted_words": diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py index 489273574aee..737b059e6291 100644 --- a/scripts/qapi/common.py +++ b/scripts/qapi/common.py @@ -114,7 +114,7 @@ def c_name(name: str, protect: bool = True) -> str: 'and', 'and_eq', 'bitand', 'bitor', 'compl', 'not', 'not_eq', 'or', 'or_eq', 'xor', 'xor_eq']) # namespace pollution: - polluted_words = set(['unix', 'errno', 'mips', 'sparc', 'i386']) + polluted_words = set(['unix', 'errno', 'mips', 'sparc', 'i386', 'linux']) > >> + > >> + > >> > >> > >> > >> Looks good to me otherwise. > >> thanks > >> > >> -- > >> Marc-André Lureau > > > > -- > zhenwei pi > -- Marc-André Lureau [-- Attachment #2: Type: text/html, Size: 3146 bytes --] ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: Re: [PATCH v2 1/1] qga: add command 'guest-get-cpustats' 2022-07-06 7:20 ` Marc-André Lureau @ 2022-07-06 7:44 ` zhenwei pi 2022-07-06 8:57 ` Marc-André Lureau 0 siblings, 1 reply; 9+ messages in thread From: zhenwei pi @ 2022-07-06 7:44 UTC (permalink / raw) To: Marc-André Lureau Cc: Markus Armbruster, Michael Roth, Konstantin Kostiuk, QEMU On 7/6/22 15:20, Marc-André Lureau wrote: > Hi > > On Wed, Jul 6, 2022 at 7:09 AM zhenwei pi <pizhenwei@bytedance.com > <mailto:pizhenwei@bytedance.com>> wrote: > > On 7/4/22 16:00, zhenwei pi wrote: > > > > > >> +## > >> +# @GuestOsType: > >> +# > >> +# An enumeration of OS type > >> +# > >> +# Since: 7.1 > >> +## > >> +{ 'enum': 'GuestOsType', > >> + 'data': [ 'linuxos', 'windowsos' ] } > >> > >> > >> I would rather keep this enum specific to GuestCpuStats, > >> "GuestLinuxCpuStatsType"? > >> > > > > Hi, > > > > 'GuestOsType' may be re-used in the future, not only for the CPU > > statistics case. > > > >> I would also drop the "os" suffix > >> > > I'm afraid we can not drop "os" suffix, build this without "os" > suffix: > > qga/qga-qapi-types.h:948:28: error: expected member name or ';' > after > > declaration specifiers > > GuestLinuxCpuStats linux; > > ~~~~~~~~~~~~~~~~~~ ^ > > <built-in>:336:15: note: expanded from here > > #define linux 1 > > > > Hi, Marc > > Could you please give any hint about this issue? > > > Yes, it looks like we need to add "linux" to the "polluted_words": > OK, I'll fix this in the next versoin. By the way, 'GuestCpuStatsType' seems to be used for CPU statistics only, but 'data': [ 'linux', 'windows' ] } is quite common, it may be used for other OS specified commands in the future. Should I use 'GuestCpuStatsType' instead of 'GuestOsType'? > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py > index 489273574aee..737b059e6291 100644 > --- a/scripts/qapi/common.py > +++ b/scripts/qapi/common.py > @@ -114,7 +114,7 @@ def c_name(name: str, protect: bool = True) -> str: > 'and', 'and_eq', 'bitand', 'bitor', 'compl', 'not', > 'not_eq', 'or', 'or_eq', 'xor', 'xor_eq']) > # namespace pollution: > - polluted_words = set(['unix', 'errno', 'mips', 'sparc', 'i386']) > + polluted_words = set(['unix', 'errno', 'mips', 'sparc', 'i386', > 'linux']) > > > >> + > >> + > >> > >> > >> > >> Looks good to me otherwise. > >> thanks > >> > >> -- > >> Marc-André Lureau > > > > -- > zhenwei pi > > > > -- > Marc-André Lureau -- zhenwei pi ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Re: [PATCH v2 1/1] qga: add command 'guest-get-cpustats' 2022-07-06 7:44 ` zhenwei pi @ 2022-07-06 8:57 ` Marc-André Lureau 0 siblings, 0 replies; 9+ messages in thread From: Marc-André Lureau @ 2022-07-06 8:57 UTC (permalink / raw) To: zhenwei pi; +Cc: Markus Armbruster, Michael Roth, Konstantin Kostiuk, QEMU [-- Attachment #1: Type: text/plain, Size: 3067 bytes --] Hi On Wed, Jul 6, 2022 at 11:49 AM zhenwei pi <pizhenwei@bytedance.com> wrote: > > > On 7/6/22 15:20, Marc-André Lureau wrote: > > Hi > > > > On Wed, Jul 6, 2022 at 7:09 AM zhenwei pi <pizhenwei@bytedance.com > > <mailto:pizhenwei@bytedance.com>> wrote: > > > > On 7/4/22 16:00, zhenwei pi wrote: > > > > > > > > >> +## > > >> +# @GuestOsType: > > >> +# > > >> +# An enumeration of OS type > > >> +# > > >> +# Since: 7.1 > > >> +## > > >> +{ 'enum': 'GuestOsType', > > >> + 'data': [ 'linuxos', 'windowsos' ] } > > >> > > >> > > >> I would rather keep this enum specific to GuestCpuStats, > > >> "GuestLinuxCpuStatsType"? > > >> > > > > > > Hi, > > > > > > 'GuestOsType' may be re-used in the future, not only for the CPU > > > statistics case. > > > > > >> I would also drop the "os" suffix > > >> > > > I'm afraid we can not drop "os" suffix, build this without "os" > > suffix: > > > qga/qga-qapi-types.h:948:28: error: expected member name or ';' > > after > > > declaration specifiers > > > GuestLinuxCpuStats linux; > > > ~~~~~~~~~~~~~~~~~~ ^ > > > <built-in>:336:15: note: expanded from here > > > #define linux 1 > > > > > > > Hi, Marc > > > > Could you please give any hint about this issue? > > > > > > Yes, it looks like we need to add "linux" to the "polluted_words": > > > > OK, I'll fix this in the next versoin. > > By the way, 'GuestCpuStatsType' seems to be used for CPU statistics > only, but 'data': [ 'linux', 'windows' ] } is quite common, it may be > used for other OS specified commands in the future. Should I use > 'GuestCpuStatsType' instead of 'GuestOsType'? > We can always generalize later, but for now I think this may just create confusion on the usage of the enum, I'd make it GuestCpuStatsType for now. (for example GuestOSInfo can't use GuestOsType) > > > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py > > index 489273574aee..737b059e6291 100644 > > --- a/scripts/qapi/common.py > > +++ b/scripts/qapi/common.py > > @@ -114,7 +114,7 @@ def c_name(name: str, protect: bool = True) -> str: > > 'and', 'and_eq', 'bitand', 'bitor', 'compl', > 'not', > > 'not_eq', 'or', 'or_eq', 'xor', 'xor_eq']) > > # namespace pollution: > > - polluted_words = set(['unix', 'errno', 'mips', 'sparc', 'i386']) > > + polluted_words = set(['unix', 'errno', 'mips', 'sparc', 'i386', > > 'linux']) > > > > > > >> + > > >> + > > >> > > >> > > >> > > >> Looks good to me otherwise. > > >> thanks > > >> > > >> -- > > >> Marc-André Lureau > > > > > > > -- > > zhenwei pi > > > > > > > > -- > > Marc-André Lureau > > -- > zhenwei pi > -- Marc-André Lureau [-- Attachment #2: Type: text/html, Size: 5010 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-07-06 9:42 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-07-04 2:36 [PATCH v2 0/1] qga: add command 'guest-get-cpustats' zhenwei pi 2022-07-04 2:36 ` [PATCH v2 1/1] " zhenwei pi 2022-07-04 7:28 ` Marc-André Lureau 2022-07-04 7:29 ` Marc-André Lureau 2022-07-04 8:00 ` zhenwei pi 2022-07-06 3:04 ` zhenwei pi 2022-07-06 7:20 ` Marc-André Lureau 2022-07-06 7:44 ` zhenwei pi 2022-07-06 8:57 ` Marc-André Lureau
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).