Re: [PATCH v2 02/22] qga: move linux vcpu command impls to commands-linux.c

[Konstantin Kostiuk <kkostiuk@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 11158 bytes --]

Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>

On Thu, Jun 13, 2024 at 6:02 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> The qmp_guest_set_vcpus and qmp_guest_get_vcpus command impls in
> commands-posix.c are surrounded by '#ifdef __linux__' so should
> instead live in commands-linux.c
>
> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  qga/commands-linux.c | 141 +++++++++++++++++++++++++++++++++++++++++++
>  qga/commands-posix.c | 139 ------------------------------------------
>  2 files changed, 141 insertions(+), 139 deletions(-)
>
> diff --git a/qga/commands-linux.c b/qga/commands-linux.c
> index 214e408fcd..78580ac39d 100644
> --- a/qga/commands-linux.c
> +++ b/qga/commands-linux.c
> @@ -13,6 +13,7 @@
>
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
> +#include "qga-qapi-commands.h"
>  #include "commands-common.h"
>  #include "cutils.h"
>  #include <mntent.h>
> @@ -284,3 +285,143 @@ int qmp_guest_fsfreeze_do_thaw(Error **errp)
>      return i;
>  }
>  #endif /* CONFIG_FSFREEZE */
> +
> +/* Transfer online/offline status between @vcpu and the guest system.
> + *
> + * On input either @errp or *@errp must be NULL.
> + *
> + * In system-to-@vcpu direction, the following @vcpu fields are accessed:
> + * - R: vcpu->logical_id
> + * - W: vcpu->online
> + * - W: vcpu->can_offline
> + *
> + * In @vcpu-to-system direction, the following @vcpu fields are accessed:
> + * - R: vcpu->logical_id
> + * - R: vcpu->online
> + *
> + * Written members remain unmodified on error.
> + */
> +static void transfer_vcpu(GuestLogicalProcessor *vcpu, bool sys2vcpu,
> +                          char *dirpath, Error **errp)
> +{
> +    int fd;
> +    int res;
> +    int dirfd;
> +    static const char fn[] = "online";
> +
> +    dirfd = open(dirpath, O_RDONLY | O_DIRECTORY);
> +    if (dirfd == -1) {
> +        error_setg_errno(errp, errno, "open(\"%s\")", dirpath);
> +        return;
> +    }
> +
> +    fd = openat(dirfd, fn, sys2vcpu ? O_RDONLY : O_RDWR);
> +    if (fd == -1) {
> +        if (errno != ENOENT) {
> +            error_setg_errno(errp, errno, "open(\"%s/%s\")", dirpath, fn);
> +        } else if (sys2vcpu) {
> +            vcpu->online = true;
> +            vcpu->can_offline = false;
> +        } else if (!vcpu->online) {
> +            error_setg(errp, "logical processor #%" PRId64 " can't be "
> +                       "offlined", vcpu->logical_id);
> +        } /* otherwise pretend successful re-onlining */
> +    } else {
> +        unsigned char status;
> +
> +        res = pread(fd, &status, 1, 0);
> +        if (res == -1) {
> +            error_setg_errno(errp, errno, "pread(\"%s/%s\")", dirpath,
> fn);
> +        } else if (res == 0) {
> +            error_setg(errp, "pread(\"%s/%s\"): unexpected EOF", dirpath,
> +                       fn);
> +        } else if (sys2vcpu) {
> +            vcpu->online = (status != '0');
> +            vcpu->can_offline = true;
> +        } else if (vcpu->online != (status != '0')) {
> +            status = '0' + vcpu->online;
> +            if (pwrite(fd, &status, 1, 0) == -1) {
> +                error_setg_errno(errp, errno, "pwrite(\"%s/%s\")",
> dirpath,
> +                                 fn);
> +            }
> +        } /* otherwise pretend successful re-(on|off)-lining */
> +
> +        res = close(fd);
> +        g_assert(res == 0);
> +    }
> +
> +    res = close(dirfd);
> +    g_assert(res == 0);
> +}
> +
> +GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp)
> +{
> +    GuestLogicalProcessorList *head, **tail;
> +    const char *cpu_dir = "/sys/devices/system/cpu";
> +    const gchar *line;
> +    g_autoptr(GDir) cpu_gdir = NULL;
> +    Error *local_err = NULL;
> +
> +    head = NULL;
> +    tail = &head;
> +    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 && (line = g_dir_read_name(cpu_gdir)) !=
> NULL) {
> +        GuestLogicalProcessor *vcpu;
> +        int64_t id;
> +        if (sscanf(line, "cpu%" PRId64, &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 */
> +            transfer_vcpu(vcpu, true, path, &local_err);
> +            QAPI_LIST_APPEND(tail, vcpu);
> +        }
> +    }
> +
> +    if (local_err == NULL) {
> +        /* there's no guest with zero VCPUs */
> +        g_assert(head != NULL);
> +        return head;
> +    }
> +
> +    qapi_free_GuestLogicalProcessorList(head);
> +    error_propagate(errp, local_err);
> +    return NULL;
> +}
> +
> +int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error
> **errp)
> +{
> +    int64_t processed;
> +    Error *local_err = NULL;
> +
> +    processed = 0;
> +    while (vcpus != NULL) {
> +        char *path = g_strdup_printf("/sys/devices/system/cpu/cpu%"
> PRId64 "/",
> +                                     vcpus->value->logical_id);
> +
> +        transfer_vcpu(vcpus->value, false, path, &local_err);
> +        g_free(path);
> +        if (local_err != NULL) {
> +            break;
> +        }
> +        ++processed;
> +        vcpus = vcpus->next;
> +    }
> +
> +    if (local_err != NULL) {
> +        if (processed == 0) {
> +            error_propagate(errp, local_err);
> +        } else {
> +            error_free(local_err);
> +        }
> +    }
> +
> +    return processed;
> +}
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 76af98ba32..a8ef41f175 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2003,145 +2003,6 @@ void qmp_guest_suspend_hybrid(Error **errp)
>      guest_suspend(SUSPEND_MODE_HYBRID, errp);
>  }
>
> -/* Transfer online/offline status between @vcpu and the guest system.
> - *
> - * On input either @errp or *@errp must be NULL.
> - *
> - * In system-to-@vcpu direction, the following @vcpu fields are accessed:
> - * - R: vcpu->logical_id
> - * - W: vcpu->online
> - * - W: vcpu->can_offline
> - *
> - * In @vcpu-to-system direction, the following @vcpu fields are accessed:
> - * - R: vcpu->logical_id
> - * - R: vcpu->online
> - *
> - * Written members remain unmodified on error.
> - */
> -static void transfer_vcpu(GuestLogicalProcessor *vcpu, bool sys2vcpu,
> -                          char *dirpath, Error **errp)
> -{
> -    int fd;
> -    int res;
> -    int dirfd;
> -    static const char fn[] = "online";
> -
> -    dirfd = open(dirpath, O_RDONLY | O_DIRECTORY);
> -    if (dirfd == -1) {
> -        error_setg_errno(errp, errno, "open(\"%s\")", dirpath);
> -        return;
> -    }
> -
> -    fd = openat(dirfd, fn, sys2vcpu ? O_RDONLY : O_RDWR);
> -    if (fd == -1) {
> -        if (errno != ENOENT) {
> -            error_setg_errno(errp, errno, "open(\"%s/%s\")", dirpath, fn);
> -        } else if (sys2vcpu) {
> -            vcpu->online = true;
> -            vcpu->can_offline = false;
> -        } else if (!vcpu->online) {
> -            error_setg(errp, "logical processor #%" PRId64 " can't be "
> -                       "offlined", vcpu->logical_id);
> -        } /* otherwise pretend successful re-onlining */
> -    } else {
> -        unsigned char status;
> -
> -        res = pread(fd, &status, 1, 0);
> -        if (res == -1) {
> -            error_setg_errno(errp, errno, "pread(\"%s/%s\")", dirpath,
> fn);
> -        } else if (res == 0) {
> -            error_setg(errp, "pread(\"%s/%s\"): unexpected EOF", dirpath,
> -                       fn);
> -        } else if (sys2vcpu) {
> -            vcpu->online = (status != '0');
> -            vcpu->can_offline = true;
> -        } else if (vcpu->online != (status != '0')) {
> -            status = '0' + vcpu->online;
> -            if (pwrite(fd, &status, 1, 0) == -1) {
> -                error_setg_errno(errp, errno, "pwrite(\"%s/%s\")",
> dirpath,
> -                                 fn);
> -            }
> -        } /* otherwise pretend successful re-(on|off)-lining */
> -
> -        res = close(fd);
> -        g_assert(res == 0);
> -    }
> -
> -    res = close(dirfd);
> -    g_assert(res == 0);
> -}
> -
> -GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp)
> -{
> -    GuestLogicalProcessorList *head, **tail;
> -    const char *cpu_dir = "/sys/devices/system/cpu";
> -    const gchar *line;
> -    g_autoptr(GDir) cpu_gdir = NULL;
> -    Error *local_err = NULL;
> -
> -    head = NULL;
> -    tail = &head;
> -    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 && (line = g_dir_read_name(cpu_gdir)) !=
> NULL) {
> -        GuestLogicalProcessor *vcpu;
> -        int64_t id;
> -        if (sscanf(line, "cpu%" PRId64, &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 */
> -            transfer_vcpu(vcpu, true, path, &local_err);
> -            QAPI_LIST_APPEND(tail, vcpu);
> -        }
> -    }
> -
> -    if (local_err == NULL) {
> -        /* there's no guest with zero VCPUs */
> -        g_assert(head != NULL);
> -        return head;
> -    }
> -
> -    qapi_free_GuestLogicalProcessorList(head);
> -    error_propagate(errp, local_err);
> -    return NULL;
> -}
> -
> -int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error
> **errp)
> -{
> -    int64_t processed;
> -    Error *local_err = NULL;
> -
> -    processed = 0;
> -    while (vcpus != NULL) {
> -        char *path = g_strdup_printf("/sys/devices/system/cpu/cpu%"
> PRId64 "/",
> -                                     vcpus->value->logical_id);
> -
> -        transfer_vcpu(vcpus->value, false, path, &local_err);
> -        g_free(path);
> -        if (local_err != NULL) {
> -            break;
> -        }
> -        ++processed;
> -        vcpus = vcpus->next;
> -    }
> -
> -    if (local_err != NULL) {
> -        if (processed == 0) {
> -            error_propagate(errp, local_err);
> -        } else {
> -            error_free(local_err);
> -        }
> -    }
> -
> -    return processed;
> -}
>  #endif /* __linux__ */
>
>  #if defined(__linux__) || defined(__FreeBSD__)
> --
> 2.45.1
>
>

[-- Attachment #2: Type: text/html, Size: 13855 bytes --]