Re: [Qemu-devel] [PATCH] qga: implement the guest-set-vcpus for windows

[Michael Roth <mdroth@linux.vnet.ibm.com>]

Quoting Gal Hammer (2015-11-02 08:49:48)
> Signed-off-by: Gal Hammer <ghammer@redhat.com>
> ---
>  qga/commands-win32.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 64 insertions(+), 2 deletions(-)
> 
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index d9de23b..a8eb4a0 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -1181,7 +1181,69 @@ void qmp_guest_set_time(bool has_time, int64_t time_ns, Error **errp)
> 
>  GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp)
>  {
> -    error_setg(errp, QERR_UNSUPPORTED);
> +    PSYSTEM_LOGICAL_PROCESSOR_INFORMATION pslpi, ptr;
> +    DWORD length;
> +    GuestLogicalProcessorList *head, **link;
> +    Error *local_err = NULL;
> +    int64_t current;
> +
> +    ptr = pslpi = NULL;
> +    length = 0;
> +    current = 0;
> +    head = NULL;
> +    link = &head;
> +
> +    if ((GetLogicalProcessorInformation(pslpi, &length) == FALSE) &&
> +        (GetLastError() == ERROR_INSUFFICIENT_BUFFER) &&
> +        (length > sizeof(SYSTEM_LOGICAL_PROCESSOR_INFORMATION))) {
> +        ptr = pslpi = g_malloc0(length);
> +        if (GetLogicalProcessorInformation(pslpi, &length) == FALSE) {
> +            error_setg(&local_err, "Failed to get processor information: %d",
> +                       (int)GetLastError());
> +        }
> +    } else {
> +        error_setg(&local_err,
> +                   "Failed to get processor information buffer length: %d",
> +                   (int)GetLastError());
> +    }
> +
> +    while ((local_err == NULL) && (length > 0)) {
> +        if (pslpi->Relationship == RelationProcessorCore) {
> +            ULONG_PTR cpu_bits = pslpi->ProcessorMask;
> +
> +            while (cpu_bits > 0) {
> +                if (!!(cpu_bits & 1)) {
> +                    GuestLogicalProcessor *vcpu;
> +                    GuestLogicalProcessorList *entry;
> +
> +                    vcpu = g_malloc0(sizeof *vcpu);
> +                    vcpu->logical_id = current++;
> +                    vcpu->online = true;
> +                    vcpu->has_can_offline = false;
> +
> +                    entry = g_malloc0(sizeof *entry);
> +                    entry->value = vcpu;
> +
> +                    *link = entry;
> +                    link = &entry->next;
> +                }
> +                cpu_bits >>= 1;
> +            }
> +        }
> +        length -= sizeof(SYSTEM_LOGICAL_PROCESSOR_INFORMATION);
> +        pslpi++; /* next entry */
> +    }
> +
> +    g_free(ptr);
> +
> +    if (local_err == NULL) {
> +        /* there's no guest with zero VCPUs */
> +        g_assert(head != NULL);

I think we should report that via error_setg() rather than assert.

Since it's a minor I can fix that up on my end though:

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

Sorry for the delay in getting to this. Need to test, but plan on pulling
this in for 2.6.

> +        return head;
> +    }
> +
> +    qapi_free_GuestLogicalProcessorList(head);
> +    error_propagate(errp, local_err);
>      return NULL;
>  }
> 
> @@ -1295,7 +1357,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
>  {
>      const char *list_unsupported[] = {
>          "guest-suspend-hybrid",
> -        "guest-get-vcpus", "guest-set-vcpus",
> +        "guest-set-vcpus",
>          "guest-get-memory-blocks", "guest-set-memory-blocks",
>          "guest-get-memory-block-size",
>          "guest-fsfreeze-freeze-list",
> -- 
> 2.1.0
>