Hi
On Thu, Nov 19, 2020 at 12:48 PM Lin Ma <[email protected]> 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
> {"execute":"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 <[email protected]>
> ---
> qga/commands-posix.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 3bffee99d4..accc893373 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2182,15 +2182,15 @@ GuestLogicalProcessorList
> *qmp_guest_get_vcpus(Error **errp)
> {
> int64_t current;
> GuestLogicalProcessorList *head, **link;
> - long sc_max;
> + long max_loop_count;
> Error *local_err = NULL;
>
> current = 0;
> head = NULL;
> link = &head;
> - sc_max = SYSCONF_EXACT(_SC_NPROCESSORS_CONF, &local_err);
> + max_loop_count = SYSCONF_EXACT(_SC_NPROCESSORS_CONF, &local_err);
>
> - while (local_err == NULL && current < sc_max) {
> + while (local_err == NULL && current < max_loop_count) {
> GuestLogicalProcessor *vcpu;
> GuestLogicalProcessorList *entry;
> int64_t id = current++;
> @@ -2206,6 +2206,8 @@ GuestLogicalProcessorList *qmp_guest_get_vcpus(Error
> **errp)
> entry->value = vcpu;
> *link = entry;
> link = &entry->next;
> + } else {
> + max_loop_count += 1;
>
This looks like a recipe for infinite loop on error.
Shouldn't we loop over all the /sys/devices/system/cpu/cpu#/ instead?
(possibly parse /sys/devices/system/cpu/present, but I doubt it's necessary)
--
Marc-André Lureau
