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

[Manos Pitsidianakis <manos.pitsidianakis@linaro.org>]

On Tue, 04 Jun 2024 16:49, "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
>
>Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>---


Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

> 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
>
>