[PATCH v3 8/8] qemu: Enable for vCPUs on hotplug

Michal Privoznik posted 8 patches 3 years, 5 months ago
There is a newer version of this series
[PATCH v3 8/8] qemu: Enable for vCPUs on hotplug
Posted by Michal Privoznik 3 years, 5 months ago
As advertised in the previous commit, QEMU_SCHED_CORE_VCPUS case
is implemented for hotplug case. The implementation is very
similar to the cold boot case, except here we fork off for every
vCPU (because the implementation is done in
qemuProcessSetupVcpu() which is also the function that's called
from hotplug code). But that's okay because our hotplug APIs
allow hotplugging one device at the time.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2074559
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_hotplug.c |  2 +-
 src/qemu/qemu_process.c | 63 +++++++++++++++++++++++++++++++++++++++--
 src/qemu/qemu_process.h |  3 +-
 3 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 30f146f2f4..ff7f87f362 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -6240,7 +6240,7 @@ qemuDomainHotplugAddVcpu(virQEMUDriver *driver,
         vcpuinfo->online = true;
 
         if (vcpupriv->tid > 0 &&
-            qemuProcessSetupVcpu(vm, i) < 0)
+            qemuProcessSetupVcpu(vm, i, true) < 0)
             return -1;
     }
 
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f9c4f72496..8892bf40e4 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5817,10 +5817,48 @@ qemuProcessNetworkPrepareDevices(virQEMUDriver *driver,
 }
 
 
+struct qemuProcessSetupVcpuSchedCoreHelperData {
+    pid_t vcpupid;
+    pid_t dummypid;
+};
+
+static int
+qemuProcessSetupVcpuSchedCoreHelper(pid_t ppid G_GNUC_UNUSED,
+                                    void *opaque)
+{
+    struct qemuProcessSetupVcpuSchedCoreHelperData *data = opaque;
+
+    if (data->dummypid != -1) {
+        if (virProcessSchedCoreShareFrom(data->dummypid) < 0) {
+            virReportSystemError(errno,
+                                 _("unable to share scheduling cookie from %lld"),
+                                 (long long) data->dummypid);
+            return -1;
+        }
+    } else {
+        if (virProcessSchedCoreCreate() < 0) {
+            virReportSystemError(errno, "%s",
+                                 _("unable to create new scheduling group"));
+            return -1;
+        }
+    }
+
+    if (virProcessSchedCoreShareTo(data->vcpupid) < 0) {
+        virReportSystemError(errno,
+                             _("unable to share scheduling cookie to %lld"),
+                             (long long) data->vcpupid);
+        return -1;
+    }
+
+    return 0;
+}
+
+
 /**
  * qemuProcessSetupVcpu:
  * @vm: domain object
  * @vcpuid: id of VCPU to set defaults
+ * @schedCore: whether to set scheduling group
  *
  * This function sets resource properties (cgroups, affinity, scheduler) for a
  * vCPU. This function expects that the vCPU is online and the vCPU pids were
@@ -5830,8 +5868,11 @@ qemuProcessNetworkPrepareDevices(virQEMUDriver *driver,
  */
 int
 qemuProcessSetupVcpu(virDomainObj *vm,
-                     unsigned int vcpuid)
+                     unsigned int vcpuid,
+                     bool schedCore)
 {
+    qemuDomainObjPrivate *priv = vm->privateData;
+    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver);
     pid_t vcpupid = qemuDomainGetVcpuPid(vm, vcpuid);
     virDomainVcpuDef *vcpu = virDomainDefGetVcpu(vm->def, vcpuid);
     virDomainResctrlMonDef *mon = NULL;
@@ -5844,6 +5885,24 @@ qemuProcessSetupVcpu(virDomainObj *vm,
                             &vcpu->sched) < 0)
         return -1;
 
+    if (schedCore &&
+        cfg->schedCore == QEMU_SCHED_CORE_VCPUS) {
+        struct qemuProcessSetupVcpuSchedCoreHelperData data = { .vcpupid = vcpupid,
+            .dummypid = -1 };
+
+        for (i = 0; i < virDomainDefGetVcpusMax(vm->def); i++) {
+            pid_t temptid = qemuDomainGetVcpuPid(vm, i);
+
+            if (temptid > 0) {
+                data.dummypid = temptid;
+                break;
+            }
+        }
+
+        if (virProcessRunInFork(qemuProcessSetupVcpuSchedCoreHelper, &data) < 0)
+            return -1;
+    }
+
     for (i = 0; i < vm->def->nresctrls; i++) {
         size_t j = 0;
         virDomainResctrlDef *ct = vm->def->resctrls[i];
@@ -5950,7 +6009,7 @@ qemuProcessSetupVcpus(virDomainObj *vm)
         if (!vcpu->online)
             continue;
 
-        if (qemuProcessSetupVcpu(vm, i) < 0)
+        if (qemuProcessSetupVcpu(vm, i, false) < 0)
             return -1;
     }
 
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 421efc6016..4dfb2485c0 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -187,7 +187,8 @@ int qemuConnectAgent(virQEMUDriver *driver, virDomainObj *vm);
 
 
 int qemuProcessSetupVcpu(virDomainObj *vm,
-                         unsigned int vcpuid);
+                         unsigned int vcpuid,
+                         bool schedCore);
 int qemuProcessSetupIOThread(virDomainObj *vm,
                              virDomainIOThreadIDDef *iothread);
 
-- 
2.35.1
Re: [PATCH v3 8/8] qemu: Enable for vCPUs on hotplug
Posted by Daniel P. Berrangé 3 years, 5 months ago
On Fri, Aug 12, 2022 at 12:04:23PM +0200, Michal Privoznik wrote:
> As advertised in the previous commit, QEMU_SCHED_CORE_VCPUS case
> is implemented for hotplug case. The implementation is very
> similar to the cold boot case, except here we fork off for every
> vCPU (because the implementation is done in
> qemuProcessSetupVcpu() which is also the function that's called
> from hotplug code). But that's okay because our hotplug APIs
> allow hotplugging one device at the time.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2074559
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_hotplug.c |  2 +-
>  src/qemu/qemu_process.c | 63 +++++++++++++++++++++++++++++++++++++++--
>  src/qemu/qemu_process.h |  3 +-
>  3 files changed, 64 insertions(+), 4 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 30f146f2f4..ff7f87f362 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -6240,7 +6240,7 @@ qemuDomainHotplugAddVcpu(virQEMUDriver *driver,
>          vcpuinfo->online = true;
>  
>          if (vcpupriv->tid > 0 &&
> -            qemuProcessSetupVcpu(vm, i) < 0)
> +            qemuProcessSetupVcpu(vm, i, true) < 0)
>              return -1;
>      }
>  
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index f9c4f72496..8892bf40e4 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -5817,10 +5817,48 @@ qemuProcessNetworkPrepareDevices(virQEMUDriver *driver,
>  }
>  
>  
> +struct qemuProcessSetupVcpuSchedCoreHelperData {
> +    pid_t vcpupid;
> +    pid_t dummypid;
> +};
> +
> +static int
> +qemuProcessSetupVcpuSchedCoreHelper(pid_t ppid G_GNUC_UNUSED,
> +                                    void *opaque)
> +{
> +    struct qemuProcessSetupVcpuSchedCoreHelperData *data = opaque;
> +
> +    if (data->dummypid != -1) {
> +        if (virProcessSchedCoreShareFrom(data->dummypid) < 0) {
> +            virReportSystemError(errno,
> +                                 _("unable to share scheduling cookie from %lld"),
> +                                 (long long) data->dummypid);
> +            return -1;
> +        }
> +    } else {
> +        if (virProcessSchedCoreCreate() < 0) {
> +            virReportSystemError(errno, "%s",
> +                                 _("unable to create new scheduling group"));
> +            return -1;
> +        }

Surely this branch must be dead/unreachable code....

> +    }
> +
> +    if (virProcessSchedCoreShareTo(data->vcpupid) < 0) {
> +        virReportSystemError(errno,
> +                             _("unable to share scheduling cookie to %lld"),
> +                             (long long) data->vcpupid);
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +
>  /**
>   * qemuProcessSetupVcpu:
>   * @vm: domain object
>   * @vcpuid: id of VCPU to set defaults
> + * @schedCore: whether to set scheduling group
>   *
>   * This function sets resource properties (cgroups, affinity, scheduler) for a
>   * vCPU. This function expects that the vCPU is online and the vCPU pids were
> @@ -5830,8 +5868,11 @@ qemuProcessNetworkPrepareDevices(virQEMUDriver *driver,
>   */
>  int
>  qemuProcessSetupVcpu(virDomainObj *vm,
> -                     unsigned int vcpuid)
> +                     unsigned int vcpuid,
> +                     bool schedCore)
>  {
> +    qemuDomainObjPrivate *priv = vm->privateData;
> +    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver);
>      pid_t vcpupid = qemuDomainGetVcpuPid(vm, vcpuid);
>      virDomainVcpuDef *vcpu = virDomainDefGetVcpu(vm->def, vcpuid);
>      virDomainResctrlMonDef *mon = NULL;
> @@ -5844,6 +5885,24 @@ qemuProcessSetupVcpu(virDomainObj *vm,
>                              &vcpu->sched) < 0)
>          return -1;
>  
> +    if (schedCore &&
> +        cfg->schedCore == QEMU_SCHED_CORE_VCPUS) {
> +        struct qemuProcessSetupVcpuSchedCoreHelperData data = { .vcpupid = vcpupid,
> +            .dummypid = -1 };
> +
> +        for (i = 0; i < virDomainDefGetVcpusMax(vm->def); i++) {
> +            pid_t temptid = qemuDomainGetVcpuPid(vm, i);
> +
> +            if (temptid > 0) {
> +                data.dummypid = temptid;
> +                break;
> +            }

....since when hotplugging a CPU there *must* always be at least
1 pre-existing vCPU - can't haave an existing running guest
with 0 vCPUs. 

> +        }

So don't we just need to raise an error here if we see dummypid
is still -1 ?

> +
> +        if (virProcessRunInFork(qemuProcessSetupVcpuSchedCoreHelper, &data) < 0)
> +            return -1;
> +    }
> +
>      for (i = 0; i < vm->def->nresctrls; i++) {
>          size_t j = 0;
>          virDomainResctrlDef *ct = vm->def->resctrls[i];

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|