[PATCH v3 4/8] qemu_domain: Introduce qemuDomainSchedCoreStart()

[Daniel P. Berrangé]

On Fri, Aug 12, 2022 at 12:04:19PM +0200, Michal Privoznik wrote:
> The aim of this helper function is to spawn a child process in
> which new scheduling group is created. This dummy process will
> then used to distribute scheduling group from (e.g. when starting
> helper processes or QEMU itself). The process is not needed for
> QEMU_SCHED_CORE_NONE case (obviously) nor for
> QEMU_SCHED_CORE_VCPUS case (because in that case a slightly
> different child will be forked off).
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_domain.c  | 105 ++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_domain.h  |  12 +++++
>  src/qemu/qemu_process.c |   4 ++
>  3 files changed, 121 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 9d5dd07958..6b30aec67d 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1747,6 +1747,8 @@ qemuDomainObjPrivateFree(void *data)
>          g_object_unref(priv->eventThread);
>      }
>  
> +    qemuDomainSchedCoreStop(priv);
> +
>      g_free(priv);
>  }
>  
> @@ -1778,6 +1780,9 @@ qemuDomainObjPrivateAlloc(void *opaque)
>      priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX;
>      priv->driver = opaque;
>  
> +    priv->schedCoreChildPID = -1;
> +    priv->schedCoreChildFD = -1;
> +
>      return g_steal_pointer(&priv);
>  }
>  
> @@ -11691,3 +11696,103 @@ qemuDomainObjWait(virDomainObj *vm)
>  
>      return 0;
>  }
> +
> +
> +int
> +qemuDomainSchedCoreStart(virQEMUDriverConfig *cfg,
> +                         virDomainObj *vm)
> +{
> +    qemuDomainObjPrivate *priv = vm->privateData;
> +    int waitfd[2] = { -1, -1 };
> +    int syncfd[2] = { -1, -1 };
> +    pid_t child = -1;
> +
> +    if (cfg->schedCore == QEMU_SCHED_CORE_NONE ||
> +        cfg->schedCore == QEMU_SCHED_CORE_VCPUS) {
> +        /* We don't need any dummy process for any of these two variants. */
> +        return 0;
> +    }
> +
> +    if (virPipe(waitfd) < 0 ||
> +        virPipe(syncfd) < 0)
> +        return -1;
> +
> +    if ((child = virFork()) < 0)
> +        goto error;
> +
> +    if (child == 0) {
> +        /* child */
> +        int rc;
> +        char c;
> +
> +        VIR_FORCE_CLOSE(waitfd[1]);
> +        VIR_FORCE_CLOSE(syncfd[0]);
> +
> +        errno = 0;
> +        rc = virProcessSchedCoreCreate();
> +        c = errno;
> +        ignore_value(safewrite(syncfd[1], &c, 1));
> +        VIR_FORCE_CLOSE(syncfd[1]);
> +
> +        if (rc < 0) {
> +            virReportSystemError(errno, "%s",
> +                                 _("Unable to set SCHED_CORE"));
> +            goto error;
> +        }
> +
> +        ignore_value(saferead(waitfd[0], &c, 1));
> +
> +        VIR_FORCE_CLOSE(waitfd[0]);

Surely we need _exit(0) - otherwise this code falls through and
runs waaaaay to much other stuff in the child process.

THe above "goto error" needs to be an exit too.


> +    } else {
> +        /* parent */
> +        char c = '\0';
> +        VIR_FORCE_CLOSE(waitfd[0]);
> +        VIR_FORCE_CLOSE(syncfd[1]);
> +
> +        if (saferead(syncfd[0], &c, 1) < 0) {
> +            virReportSystemError(errno, "%s",
> +                                 _("unable to read from pipe"));
> +            goto error;
> +        }
> +        VIR_FORCE_CLOSE(syncfd[0]);
> +
> +        if (c != 0) {
> +            virReportSystemError(c, "%s",
> +                                 _("Unable to set SCHED_CORE"));
> +            goto error;
> +        }
> +
> +        VIR_DEBUG("Spawned dummy process for schedCore (%s) pid=%lld fd=%d",
> +                  virQEMUSchedCoreTypeToString(cfg->schedCore),
> +                  (long long) child, waitfd[1]);
> +
> +        priv->schedCoreChildPID = child;
> +        priv->schedCoreChildFD = waitfd[1];
> +    }
> +
> +    return 0;
> +
> + error:
> +    VIR_FORCE_CLOSE(waitfd[0]);
> +    VIR_FORCE_CLOSE(waitfd[1]);
> +    VIR_FORCE_CLOSE(syncfd[0]);
> +    VIR_FORCE_CLOSE(syncfd[1]);
> +    return -1;
> +}
> +
> +
> +void
> +qemuDomainSchedCoreStop(qemuDomainObjPrivate *priv)
> +{
> +    if (priv->schedCoreChildFD != -1) {
> +        ignore_value(safewrite(priv->schedCoreChildFD, "q", 1));
> +        VIR_FORCE_CLOSE(priv->schedCoreChildFD);
> +    }
> +
> +    if (priv->schedCoreChildPID != -1) {
> +        VIR_DEBUG("Killing dummy procces for schedCore pid=%lld",
> +                  (long long) priv->schedCoreChildPID);
> +        virProcessAbort(priv->schedCoreChildPID);
> +        priv->schedCoreChildPID = -1;
> +    }
> +}
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 592ee9805b..620830ccf5 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -244,6 +244,11 @@ struct _qemuDomainObjPrivate {
>  
>      unsigned long long originalMemlock; /* Original RLIMIT_MEMLOCK, zero if no
>                                           * restore will be required later */
> +
> +    /* Info on dummy process for schedCore. A short lived process used only
> +     * briefly when starting a guest. Don't save/parse into XML. */
> +    pid_t schedCoreChildPID;
> +    pid_t schedCoreChildFD;
>  };
>  
>  #define QEMU_DOMAIN_PRIVATE(vm) \
> @@ -1098,3 +1103,10 @@ qemuDomainRemoveLogs(virQEMUDriver *driver,
>  
>  int
>  qemuDomainObjWait(virDomainObj *vm);
> +
> +int
> +qemuDomainSchedCoreStart(virQEMUDriverConfig *cfg,
> +                         virDomainObj *vm);
> +
> +void
> +qemuDomainSchedCoreStop(qemuDomainObjPrivate *priv);
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index cfcf879f59..8b027f2d39 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -7502,6 +7502,9 @@ qemuProcessLaunch(virConnectPtr conn,
>      if (qemuProcessGenID(vm, flags) < 0)
>          goto cleanup;
>  
> +    if (qemuDomainSchedCoreStart(cfg, vm) < 0)
> +        goto cleanup;
> +
>      if (qemuExtDevicesStart(driver, vm, incoming != NULL) < 0)
>          goto cleanup;
>  
> @@ -7774,6 +7777,7 @@ qemuProcessLaunch(virConnectPtr conn,
>      ret = 0;
>  
>   cleanup:
> +    qemuDomainSchedCoreStop(priv);
>      qemuDomainSecretDestroy(vm);
>      return ret;
>  }
> -- 
> 2.35.1
> 

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