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

[Daniel P. Berrangé]

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