* [PATCH v3] qga: Correct loop count in qmp_guest_get_vcpus()
@ 2020-11-20 12:27 Lin Ma
2020-11-20 12:33 ` Marc-André Lureau
0 siblings, 1 reply; 4+ messages in thread
From: Lin Ma @ 2020-11-20 12:27 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, marcandre.lureau, Lin Ma
The guest-get-vcpus returns incorrect vcpu info in case we hotunplug vcpus(not
the last one).
e.g.:
A VM has 4 VCPUs: cpu0 + 3 hotunpluggable online vcpus(cpu1, cpu2 and cpu3).
Hotunplug cpu2, Now only cpu0, cpu1 and cpu3 are present & online.
./qmp-shell /tmp/qmp-monitor.sock
(QEMU) query-hotpluggable-cpus
{"return": [
{"props": {"core-id": 0, "thread-id": 0, "socket-id": 3}, "vcpus-count": 1,
"qom-path": "/machine/peripheral/cpu3", "type": "host-x86_64-cpu"},
{"props": {"core-id": 0, "thread-id": 0, "socket-id": 2}, "vcpus-count": 1,
"qom-path": "/machine/peripheral/cpu2", "type": "host-x86_64-cpu"},
{"props": {"core-id": 0, "thread-id": 0, "socket-id": 1}, "vcpus-count": 1,
"qom-path": "/machine/peripheral/cpu1", "type": "host-x86_64-cpu"},
{"props": {"core-id": 0, "thread-id": 0, "socket-id": 0}, "vcpus-count": 1,
"qom-path": "/machine/unattached/device[0]", "type": "host-x86_64-cpu"}
]}
(QEMU) device_del id=cpu2
{"return": {}}
(QEMU) query-hotpluggable-cpus
{"return": [
{"props": {"core-id": 0, "thread-id": 0, "socket-id": 3}, "vcpus-count": 1,
"qom-path": "/machine/peripheral/cpu3", "type": "host-x86_64-cpu"},
{"props": {"core-id": 0, "thread-id": 0, "socket-id": 2}, "vcpus-count": 1,
"type": "host-x86_64-cpu"},
{"props": {"core-id": 0, "thread-id": 0, "socket-id": 1}, "vcpus-count": 1,
"qom-path": "/machine/peripheral/cpu1", "type": "host-x86_64-cpu"},
{"props": {"core-id": 0, "thread-id": 0, "socket-id": 0}, "vcpus-count": 1,
"qom-path": "/machine/unattached/device[0]", "type": "host-x86_64-cpu"}
]}
Before:
./qmp-shell -N /tmp/qmp-ga.sock
Welcome to the QMP low-level shell!
Connected
(QEMU) guest-get-vcpus
{"return": [
{"online": true, "can-offline": false, "logical-id": 0},
{"online": true, "can-offline": true, "logical-id": 1}]}
After:
./qmp-shell -N /tmp/qmp-ga.sock
Welcome to the QMP low-level shell!
Connected
(QEMU) guest-get-vcpus
{"return": [
{"online": true, "can-offline": false, "logical-id": 0},
{"online": true, "can-offline": true, "logical-id": 1},
{"online": true, "can-offline": true, "logical-id": 3}]}
Signed-off-by: Lin Ma <lma@suse.com>
---
qga/commands-posix.c | 45 ++++++++++++++++----------------------------
1 file changed, 16 insertions(+), 29 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index c089e38120..ee05e694d3 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2376,24 +2376,6 @@ error:
return NULL;
}
-#define SYSCONF_EXACT(name, errp) sysconf_exact((name), #name, (errp))
-
-static long sysconf_exact(int name, const char *name_str, Error **errp)
-{
- long ret;
-
- errno = 0;
- ret = sysconf(name);
- if (ret == -1) {
- if (errno == 0) {
- error_setg(errp, "sysconf(%s): value indefinite", name_str);
- } else {
- error_setg_errno(errp, errno, "sysconf(%s)", name_str);
- }
- }
- return ret;
-}
-
/* Transfer online/offline status between @vcpu and the guest system.
*
* On input either @errp or *@errp must be NULL.
@@ -2464,24 +2446,29 @@ static void transfer_vcpu(GuestLogicalProcessor *vcpu, bool sys2vcpu,
GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp)
{
- int64_t current;
GuestLogicalProcessorList *head, **link;
- long sc_max;
+ g_autofree char *cpu_dir = NULL;
+ const gchar *line;
+ GDir *cpu_gdir = NULL;
Error *local_err = NULL;
- current = 0;
head = NULL;
link = &head;
- sc_max = SYSCONF_EXACT(_SC_NPROCESSORS_CONF, &local_err);
+ cpu_dir = g_strdup_printf("/sys/devices/system/cpu");
+ cpu_gdir = g_dir_open(cpu_dir, 0, NULL);
+
+ if (cpu_gdir == NULL) {
+ error_setg_errno(errp, errno, "failed to list entries: %s", cpu_dir);
+ return NULL;
+ }
- while (local_err == NULL && current < sc_max) {
+ while (local_err == NULL && (line = g_dir_read_name(cpu_gdir)) != NULL) {
GuestLogicalProcessor *vcpu;
GuestLogicalProcessorList *entry;
- int64_t id = current++;
- char *path = g_strdup_printf("/sys/devices/system/cpu/cpu%" PRId64 "/",
- id);
-
- if (g_file_test(path, G_FILE_TEST_EXISTS)) {
+ int64_t id;
+ if (sscanf(line, "cpu%ld", &id)) {
+ g_autofree char *path = g_strdup_printf("/sys/devices/system/cpu/"
+ "cpu%" PRId64 "/", id);
vcpu = g_malloc0(sizeof *vcpu);
vcpu->logical_id = id;
vcpu->has_can_offline = true; /* lolspeak ftw */
@@ -2491,8 +2478,8 @@ GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp)
*link = entry;
link = &entry->next;
}
- g_free(path);
}
+ g_dir_close(cpu_gdir);
if (local_err == NULL) {
/* there's no guest with zero VCPUs */
--
2.26.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] qga: Correct loop count in qmp_guest_get_vcpus()
2020-11-20 12:27 [PATCH v3] qga: Correct loop count in qmp_guest_get_vcpus() Lin Ma
@ 2020-11-20 12:33 ` Marc-André Lureau
2020-11-24 14:11 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 4+ messages in thread
From: Marc-André Lureau @ 2020-11-20 12:33 UTC (permalink / raw)
To: Lin Ma; +Cc: QEMU, Michael Roth
[-- Attachment #1: Type: text/plain, Size: 5449 bytes --]
Hi
On Fri, Nov 20, 2020 at 4:27 PM Lin Ma <lma@suse.com> wrote:
> The guest-get-vcpus returns incorrect vcpu info in case we hotunplug
> vcpus(not
> the last one).
> e.g.:
> A VM has 4 VCPUs: cpu0 + 3 hotunpluggable online vcpus(cpu1, cpu2 and
> cpu3).
> Hotunplug cpu2, Now only cpu0, cpu1 and cpu3 are present & online.
>
> ./qmp-shell /tmp/qmp-monitor.sock
> (QEMU) query-hotpluggable-cpus
> {"return": [
> {"props": {"core-id": 0, "thread-id": 0, "socket-id": 3}, "vcpus-count": 1,
> "qom-path": "/machine/peripheral/cpu3", "type": "host-x86_64-cpu"},
> {"props": {"core-id": 0, "thread-id": 0, "socket-id": 2}, "vcpus-count": 1,
> "qom-path": "/machine/peripheral/cpu2", "type": "host-x86_64-cpu"},
> {"props": {"core-id": 0, "thread-id": 0, "socket-id": 1}, "vcpus-count": 1,
> "qom-path": "/machine/peripheral/cpu1", "type": "host-x86_64-cpu"},
> {"props": {"core-id": 0, "thread-id": 0, "socket-id": 0}, "vcpus-count": 1,
> "qom-path": "/machine/unattached/device[0]", "type": "host-x86_64-cpu"}
> ]}
>
> (QEMU) device_del id=cpu2
> {"return": {}}
>
> (QEMU) query-hotpluggable-cpus
> {"return": [
> {"props": {"core-id": 0, "thread-id": 0, "socket-id": 3}, "vcpus-count": 1,
> "qom-path": "/machine/peripheral/cpu3", "type": "host-x86_64-cpu"},
> {"props": {"core-id": 0, "thread-id": 0, "socket-id": 2}, "vcpus-count": 1,
> "type": "host-x86_64-cpu"},
> {"props": {"core-id": 0, "thread-id": 0, "socket-id": 1}, "vcpus-count": 1,
> "qom-path": "/machine/peripheral/cpu1", "type": "host-x86_64-cpu"},
> {"props": {"core-id": 0, "thread-id": 0, "socket-id": 0}, "vcpus-count": 1,
> "qom-path": "/machine/unattached/device[0]", "type": "host-x86_64-cpu"}
> ]}
>
> Before:
> ./qmp-shell -N /tmp/qmp-ga.sock
> Welcome to the QMP low-level shell!
> Connected
> (QEMU) guest-get-vcpus
> {"return": [
> {"online": true, "can-offline": false, "logical-id": 0},
> {"online": true, "can-offline": true, "logical-id": 1}]}
>
> After:
> ./qmp-shell -N /tmp/qmp-ga.sock
> Welcome to the QMP low-level shell!
> Connected
> (QEMU) guest-get-vcpus
> {"return": [
> {"online": true, "can-offline": false, "logical-id": 0},
> {"online": true, "can-offline": true, "logical-id": 1},
> {"online": true, "can-offline": true, "logical-id": 3}]}
>
> Signed-off-by: Lin Ma <lma@suse.com>
> ---
> qga/commands-posix.c | 45 ++++++++++++++++----------------------------
> 1 file changed, 16 insertions(+), 29 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index c089e38120..ee05e694d3 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2376,24 +2376,6 @@ error:
> return NULL;
> }
>
> -#define SYSCONF_EXACT(name, errp) sysconf_exact((name), #name, (errp))
> -
> -static long sysconf_exact(int name, const char *name_str, Error **errp)
> -{
> - long ret;
> -
> - errno = 0;
> - ret = sysconf(name);
> - if (ret == -1) {
> - if (errno == 0) {
> - error_setg(errp, "sysconf(%s): value indefinite", name_str);
> - } else {
> - error_setg_errno(errp, errno, "sysconf(%s)", name_str);
> - }
> - }
> - return ret;
> -}
> -
> /* Transfer online/offline status between @vcpu and the guest system.
> *
> * On input either @errp or *@errp must be NULL.
> @@ -2464,24 +2446,29 @@ static void transfer_vcpu(GuestLogicalProcessor
> *vcpu, bool sys2vcpu,
>
> GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp)
> {
> - int64_t current;
> GuestLogicalProcessorList *head, **link;
> - long sc_max;
> + g_autofree char *cpu_dir = NULL;
> + const gchar *line;
> + GDir *cpu_gdir = NULL;
> Error *local_err = NULL;
>
> - current = 0;
> head = NULL;
> link = &head;
> - sc_max = SYSCONF_EXACT(_SC_NPROCESSORS_CONF, &local_err);
> + cpu_dir = g_strdup_printf("/sys/devices/system/cpu");
>
Why strdup?
+ cpu_gdir = g_dir_open(cpu_dir, 0, NULL);
> +
> + if (cpu_gdir == NULL) {
> + error_setg_errno(errp, errno, "failed to list entries: %s",
> cpu_dir);
> + return NULL;
> + }
>
> - while (local_err == NULL && current < sc_max) {
> + while (local_err == NULL && (line = g_dir_read_name(cpu_gdir)) !=
> NULL) {
> GuestLogicalProcessor *vcpu;
> GuestLogicalProcessorList *entry;
> - int64_t id = current++;
> - char *path = g_strdup_printf("/sys/devices/system/cpu/cpu%"
> PRId64 "/",
> - id);
> -
> - if (g_file_test(path, G_FILE_TEST_EXISTS)) {
> + int64_t id;
> + if (sscanf(line, "cpu%ld", &id)) {
> + g_autofree char *path =
> g_strdup_printf("/sys/devices/system/cpu/"
> + "cpu%" PRId64 "/",
> id);
> vcpu = g_malloc0(sizeof *vcpu);
> vcpu->logical_id = id;
> vcpu->has_can_offline = true; /* lolspeak ftw */
> @@ -2491,8 +2478,8 @@ GuestLogicalProcessorList *qmp_guest_get_vcpus(Error
> **errp)
> *link = entry;
> link = &entry->next;
> }
> - g_free(path);
> }
> + g_dir_close(cpu_gdir);
>
> if (local_err == NULL) {
> /* there's no guest with zero VCPUs */
> --
> 2.26.0
>
>
thanks for the changes, that seems better to me
--
Marc-André Lureau
[-- Attachment #2: Type: text/html, Size: 7636 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] qga: Correct loop count in qmp_guest_get_vcpus()
2020-11-20 12:33 ` Marc-André Lureau
@ 2020-11-24 14:11 ` Philippe Mathieu-Daudé
2020-11-24 22:05 ` Peter Maydell
0 siblings, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-11-24 14:11 UTC (permalink / raw)
To: Marc-André Lureau, Lin Ma; +Cc: QEMU, Michael Roth
Hi,
On 11/20/20 1:33 PM, Marc-André Lureau wrote:
> Hi
>
> On Fri, Nov 20, 2020 at 4:27 PM Lin Ma <lma@suse.com
> <mailto:lma@suse.com>> wrote:
>
> The guest-get-vcpus returns incorrect vcpu info in case we hotunplug
> vcpus(not
> the last one).
> e.g.:
> A VM has 4 VCPUs: cpu0 + 3 hotunpluggable online vcpus(cpu1, cpu2
> and cpu3).
> Hotunplug cpu2, Now only cpu0, cpu1 and cpu3 are present & online.
>
> ./qmp-shell /tmp/qmp-monitor.sock
> (QEMU) query-hotpluggable-cpus
> {"return": [
> {"props": {"core-id": 0, "thread-id": 0, "socket-id": 3},
> "vcpus-count": 1,
> "qom-path": "/machine/peripheral/cpu3", "type": "host-x86_64-cpu"},
> {"props": {"core-id": 0, "thread-id": 0, "socket-id": 2},
> "vcpus-count": 1,
> "qom-path": "/machine/peripheral/cpu2", "type": "host-x86_64-cpu"},
> {"props": {"core-id": 0, "thread-id": 0, "socket-id": 1},
> "vcpus-count": 1,
> "qom-path": "/machine/peripheral/cpu1", "type": "host-x86_64-cpu"},
> {"props": {"core-id": 0, "thread-id": 0, "socket-id": 0},
> "vcpus-count": 1,
> "qom-path": "/machine/unattached/device[0]", "type": "host-x86_64-cpu"}
> ]}
>
> (QEMU) device_del id=cpu2
> {"return": {}}
>
> (QEMU) query-hotpluggable-cpus
> {"return": [
> {"props": {"core-id": 0, "thread-id": 0, "socket-id": 3},
> "vcpus-count": 1,
> "qom-path": "/machine/peripheral/cpu3", "type": "host-x86_64-cpu"},
> {"props": {"core-id": 0, "thread-id": 0, "socket-id": 2},
> "vcpus-count": 1,
> "type": "host-x86_64-cpu"},
> {"props": {"core-id": 0, "thread-id": 0, "socket-id": 1},
> "vcpus-count": 1,
> "qom-path": "/machine/peripheral/cpu1", "type": "host-x86_64-cpu"},
> {"props": {"core-id": 0, "thread-id": 0, "socket-id": 0},
> "vcpus-count": 1,
> "qom-path": "/machine/unattached/device[0]", "type": "host-x86_64-cpu"}
> ]}
>
> Before:
> ./qmp-shell -N /tmp/qmp-ga.sock
> Welcome to the QMP low-level shell!
> Connected
> (QEMU) guest-get-vcpus
> {"return": [
> {"online": true, "can-offline": false, "logical-id": 0},
> {"online": true, "can-offline": true, "logical-id": 1}]}
>
> After:
> ./qmp-shell -N /tmp/qmp-ga.sock
> Welcome to the QMP low-level shell!
> Connected
> (QEMU) guest-get-vcpus
> {"return": [
> {"online": true, "can-offline": false, "logical-id": 0},
> {"online": true, "can-offline": true, "logical-id": 1},
> {"online": true, "can-offline": true, "logical-id": 3}]}
>
> Signed-off-by: Lin Ma <lma@suse.com <mailto:lma@suse.com>>
> ---
> qga/commands-posix.c | 45 ++++++++++++++++----------------------------
> 1 file changed, 16 insertions(+), 29 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index c089e38120..ee05e694d3 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2376,24 +2376,6 @@ error:
> return NULL;
> }
>
> -#define SYSCONF_EXACT(name, errp) sysconf_exact((name), #name, (errp))
> -
> -static long sysconf_exact(int name, const char *name_str, Error **errp)
> -{
> - long ret;
> -
> - errno = 0;
> - ret = sysconf(name);
> - if (ret == -1) {
> - if (errno == 0) {
> - error_setg(errp, "sysconf(%s): value indefinite",
> name_str);
> - } else {
> - error_setg_errno(errp, errno, "sysconf(%s)", name_str);
> - }
> - }
> - return ret;
> -}
> -
> /* Transfer online/offline status between @vcpu and the guest system.
> *
> * On input either @errp or *@errp must be NULL.
> @@ -2464,24 +2446,29 @@ static void
> transfer_vcpu(GuestLogicalProcessor *vcpu, bool sys2vcpu,
>
> GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp)
> {
> - int64_t current;
> GuestLogicalProcessorList *head, **link;
> - long sc_max;
> + g_autofree char *cpu_dir = NULL;
> + const gchar *line;
> + GDir *cpu_gdir = NULL;
> Error *local_err = NULL;
>
> - current = 0;
> head = NULL;
> link = &head;
> - sc_max = SYSCONF_EXACT(_SC_NPROCESSORS_CONF, &local_err);
> + cpu_dir = g_strdup_printf("/sys/devices/system/cpu");
>
>
> Why strdup?
As we are going to tag 5.2-rc3, what is the status of this fix?
Thanks,
Phil.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] qga: Correct loop count in qmp_guest_get_vcpus()
2020-11-24 14:11 ` Philippe Mathieu-Daudé
@ 2020-11-24 22:05 ` Peter Maydell
0 siblings, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2020-11-24 22:05 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Michael Roth, Marc-André Lureau, QEMU, Lin Ma
On Tue, 24 Nov 2020 at 14:12, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> As we are going to tag 5.2-rc3, what is the status of this fix?
'git blame' says none of the code changed here is newer than 2018,
so it seems unlikely that this is a regression since 5.1. rc3 is
now ready to tag, so this is going to get postponed to 6.0
unless somebody has a really solid argument for why it is
a release-critical bug.
thanks
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-11-24 22:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-20 12:27 [PATCH v3] qga: Correct loop count in qmp_guest_get_vcpus() Lin Ma
2020-11-20 12:33 ` Marc-André Lureau
2020-11-24 14:11 ` Philippe Mathieu-Daudé
2020-11-24 22:05 ` Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.