Re: [Qemu-devel] [PATCH 2/3] qga: implement qmp_guest_get_vcpus() for Linux with sysfs

[mdroth]

On Mon, Mar 04, 2013 at 11:19:56PM +0100, Laszlo Ersek wrote:
> 
> Signed-off-by: Laszlo Ersek <address@hidden>
> ---
>  qga/commands-posix.c |   87 
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 87 insertions(+), 0 deletions(-)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 1ad231a..d4b6bdc 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -15,6 +15,9 @@
>  #include <sys/types.h>
>  #include <sys/ioctl.h>
>  #include <sys/wait.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <stdio.h>
>  #include "qga/guest-agent-core.h"
>  #include "qga-qmp-commands.h"
>  #include "qapi/qmp/qerror.h"
> @@ -1083,9 +1086,93 @@ void qmp_guest_fstrim(bool has_minimum, int64_t 
> minimum, Error **err)
>  }
>  #endif
> 
> +#if defined(__linux__)

There's a section in commands-posix.c set aside under "linux-specific
implementations" for these, and another underneath for stubs so we can
avoid having too many ifdef's.

> +#define SYSCONF_EXACT(name, err) sysconf_exact((name), #name, (err))
> +
> +static long sysconf_exact(int name, const char *name_str, Error **err)
> +{
> +    long ret;
> +
> +    errno = 0;
> +    ret = sysconf(name);
> +    if (ret == -1) {
> +        if (errno == 0) {
> +            error_setg(err, "sysconf(%s): value indefinite", name_str);
> +        } else {
> +            error_setg_errno(err, errno, "sysconf(%s)", name_str);
> +        }
> +    }
> +    return ret;
> +}
> +
> +/*
> + * Store a VCPU structure under the link, and return the link to store into
> + * at the next time.
> + */
> +static GuestLogicalProcessorList **
> +append_vcpu(int64_t logical_id, bool online, GuestLogicalProcessorList 
> **link)
> +{
> +    GuestLogicalProcessor *vcpu;
> +    GuestLogicalProcessorList *entry;
> +
> +    vcpu = g_malloc0(sizeof *vcpu);
> +    vcpu->logical_id = logical_id;
> +    vcpu->online = online;
> +
> +    entry = g_malloc0(sizeof *entry);
> +    entry->value = vcpu;
> +
> +    *link = entry;
> +    return &entry->next;
> +}
> +#endif
> +
>  GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp)
>  {
> +#if defined(__linux__)
> +    long current;
> +    GuestLogicalProcessorList **link, *head;
> +    long sc_max;
> +    Error *local_err = NULL;
> +
> +    current = 0;
> +    link = append_vcpu(current++, true, &head);
> +
> +    sc_max = SYSCONF_EXACT(_SC_NPROCESSORS_CONF, &local_err);
> +    while (local_err == NULL && current < sc_max) {
> +        char *buf;
> +        FILE *f;
> +
> +        buf = g_strdup_printf("/sys/devices/system/cpu/cpu%ld/online",
> +                              current);
> +        f = fopen(buf, "r");
> +        if (f == NULL) {
> +            error_setg_errno(&local_err, errno, "fopen(\"%s\", \"r\")", buf);
> +        } else {
> +            unsigned online;
> +
> +            if (fscanf(f, "%u", &online) != 1) {

On Fedora 18 and Ubuntu 12.04 at least there doesn't seem to be per-cpu
values for online/offline/etc, but instead just a 'global' entry at
/sys/devices/system/cpu/{online,offline} that provides a range. This is
what's currently described in
linux/Documentation/ABI/testing/sysfs-devices-system-cpu as well.

Is that file also available on the distro you're testing with? Hopefully
there's a single interfaces we can rely on.

> +                error_setg(&local_err, "failed to read or parse \"%s\"", 
> buf);
> +            } else {
> +                link = append_vcpu(current++, online != 0, link);
> +            }
> +
> +            if (fclose(f) == EOF && local_err == NULL) {
> +                error_setg_errno(&local_err, errno, "fclose(\"%s\")", buf);
> +            }
> +        }
> +        g_free(buf);
> +    }
> +
> +    if (local_err == NULL) {
> +        return head;
> +    }
> +
> +    qapi_free_GuestLogicalProcessorList(head);
> +    error_propagate(errp, local_err);
> +#else
>      error_set(errp, QERR_UNSUPPORTED);
> +#endif
>      return NULL;
>  }
> 
> -- 
> 1.7.1
> 
>