[PATCH RFC 07/10] virCommand: Introduce APIs for core scheduling

Michal Privoznik posted 10 patches 3 years, 9 months ago
There is a newer version of this series
[PATCH RFC 07/10] virCommand: Introduce APIs for core scheduling
Posted by Michal Privoznik 3 years, 9 months ago
There are two modes of core scheduling that are handy wrt
virCommand:

1) create new trusted group when executing a virCommand

2) place freshly executed virCommand into the trusted group of
   another process.

Therefore, implement these two new operations as new APIs:
virCommandSetRunAlone() and virCommandSetRunAmong(),
respectively.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/libvirt_private.syms |  2 ++
 src/util/vircommand.c    | 74 ++++++++++++++++++++++++++++++++++++++++
 src/util/vircommand.h    |  5 +++
 3 files changed, 81 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 252d7e029f..8f2b789cee 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2079,6 +2079,8 @@ virCommandSetOutputBuffer;
 virCommandSetOutputFD;
 virCommandSetPidFile;
 virCommandSetPreExecHook;
+virCommandSetRunAlone;
+virCommandSetRunAmong;
 virCommandSetSELinuxLabel;
 virCommandSetSendBuffer;
 virCommandSetUID;
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 41cf552d7b..db20620f7c 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -148,6 +148,9 @@ struct _virCommand {
 #endif
     int mask;
 
+    bool schedCore;
+    pid_t schedCorePID;
+
     virCommandSendBuffer *sendBuffers;
     size_t numSendBuffers;
 };
@@ -434,6 +437,22 @@ virCommandHandshakeChild(virCommand *cmd)
 static int
 virExecCommon(virCommand *cmd, gid_t *groups, int ngroups)
 {
+    /* Do this before dropping capabilities. */
+    if (cmd->schedCore &&
+        virProcessSchedCoreCreate() < 0) {
+        virReportSystemError(errno, "%s",
+                             _("Unable to set SCHED_CORE"));
+        return -1;
+    }
+
+    if (cmd->schedCorePID >= 0 &&
+        virProcessSchedCoreShareFrom(cmd->schedCorePID) < 0) {
+        virReportSystemError(errno,
+                             _("Unable to run among %llu"),
+                             (unsigned long long) cmd->schedCorePID);
+        return -1;
+    }
+
     if (cmd->uid != (uid_t)-1 || cmd->gid != (gid_t)-1 ||
         cmd->capabilities || (cmd->flags & VIR_EXEC_CLEAR_CAPS)) {
         VIR_DEBUG("Setting child uid:gid to %d:%d with caps %llx",
@@ -964,6 +983,7 @@ virCommandNewArgs(const char *const*args)
     cmd->pid = -1;
     cmd->uid = -1;
     cmd->gid = -1;
+    cmd->schedCorePID = -1;
 
     virCommandAddArgSet(cmd, args);
 
@@ -3437,3 +3457,57 @@ virCommandRunNul(virCommand *cmd G_GNUC_UNUSED,
     return -1;
 }
 #endif /* WIN32 */
+
+/**
+ * virCommandSetRunAlone:
+ *
+ * Create new trusted group when running the command. In other words, the
+ * process won't be scheduled to run on a core among with processes from
+ * another, untrusted group.
+ */
+void
+virCommandSetRunAlone(virCommand *cmd)
+{
+    if (virCommandHasError(cmd))
+        return;
+
+    if (cmd->schedCorePID >= 0) {
+        /* Can't mix these two. */
+        cmd->has_error = -1;
+        VIR_DEBUG("cannot mix with virCommandSetRunAmong()");
+        return;
+    }
+
+    cmd->schedCore = true;
+}
+
+/**
+ * virCommandSetRunAmong:
+ * @pid: pid from a trusted group
+ *
+ * When spawning the command place it into the trusted group of @pid so that
+ * these two processes can run on Hyper Threads of a single core at the same
+ * time.
+ */
+void
+virCommandSetRunAmong(virCommand *cmd,
+                      pid_t pid)
+{
+    if (virCommandHasError(cmd))
+        return;
+
+    if (cmd->schedCore) {
+        /* Can't mix these two. */
+        VIR_DEBUG("cannot mix with virCommandSetRunAlone()");
+        cmd->has_error = -1;
+        return;
+    }
+
+    if (pid < 0) {
+        VIR_DEBUG("invalid pid value: %lld", (long long) pid);
+        cmd->has_error = -1;
+        return;
+    }
+
+    cmd->schedCorePID = pid;
+}
diff --git a/src/util/vircommand.h b/src/util/vircommand.h
index 600806a987..0b03ea005c 100644
--- a/src/util/vircommand.h
+++ b/src/util/vircommand.h
@@ -225,4 +225,9 @@ int virCommandRunNul(virCommand *cmd,
                      virCommandRunNulFunc func,
                      void *data);
 
+void virCommandSetRunAlone(virCommand *cmd);
+
+void virCommandSetRunAmong(virCommand *cmd,
+                           pid_t pid);
+
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCommand, virCommandFree);
-- 
2.35.1
Re: [PATCH RFC 07/10] virCommand: Introduce APIs for core scheduling
Posted by Daniel P. Berrangé 3 years, 8 months ago
On Mon, May 09, 2022 at 05:02:14PM +0200, Michal Privoznik wrote:
> There are two modes of core scheduling that are handy wrt
> virCommand:
> 
> 1) create new trusted group when executing a virCommand
> 
> 2) place freshly executed virCommand into the trusted group of
>    another process.
> 
> Therefore, implement these two new operations as new APIs:
> virCommandSetRunAlone() and virCommandSetRunAmong(),
> respectively.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/libvirt_private.syms |  2 ++
>  src/util/vircommand.c    | 74 ++++++++++++++++++++++++++++++++++++++++
>  src/util/vircommand.h    |  5 +++
>  3 files changed, 81 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 252d7e029f..8f2b789cee 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2079,6 +2079,8 @@ virCommandSetOutputBuffer;
>  virCommandSetOutputFD;
>  virCommandSetPidFile;
>  virCommandSetPreExecHook;
> +virCommandSetRunAlone;
> +virCommandSetRunAmong;
>  virCommandSetSELinuxLabel;
>  virCommandSetSendBuffer;
>  virCommandSetUID;
> diff --git a/src/util/vircommand.c b/src/util/vircommand.c
> index 41cf552d7b..db20620f7c 100644
> --- a/src/util/vircommand.c
> +++ b/src/util/vircommand.c
> @@ -148,6 +148,9 @@ struct _virCommand {
>  #endif
>      int mask;
>  
> +    bool schedCore;
> +    pid_t schedCorePID;
> +
>      virCommandSendBuffer *sendBuffers;
>      size_t numSendBuffers;
>  };
> @@ -434,6 +437,22 @@ virCommandHandshakeChild(virCommand *cmd)
>  static int
>  virExecCommon(virCommand *cmd, gid_t *groups, int ngroups)
>  {
> +    /* Do this before dropping capabilities. */
> +    if (cmd->schedCore &&
> +        virProcessSchedCoreCreate() < 0) {
> +        virReportSystemError(errno, "%s",
> +                             _("Unable to set SCHED_CORE"));
> +        return -1;
> +    }
> +
> +    if (cmd->schedCorePID >= 0 &&
> +        virProcessSchedCoreShareFrom(cmd->schedCorePID) < 0) {
> +        virReportSystemError(errno,
> +                             _("Unable to run among %llu"),
> +                             (unsigned long long) cmd->schedCorePID);
> +        return -1;
> +    }
> +
>      if (cmd->uid != (uid_t)-1 || cmd->gid != (gid_t)-1 ||
>          cmd->capabilities || (cmd->flags & VIR_EXEC_CLEAR_CAPS)) {
>          VIR_DEBUG("Setting child uid:gid to %d:%d with caps %llx",
> @@ -964,6 +983,7 @@ virCommandNewArgs(const char *const*args)
>      cmd->pid = -1;
>      cmd->uid = -1;
>      cmd->gid = -1;
> +    cmd->schedCorePID = -1;
>  
>      virCommandAddArgSet(cmd, args);
>  
> @@ -3437,3 +3457,57 @@ virCommandRunNul(virCommand *cmd G_GNUC_UNUSED,
>      return -1;
>  }
>  #endif /* WIN32 */
> +
> +/**
> + * virCommandSetRunAlone:
> + *
> + * Create new trusted group when running the command. In other words, the
> + * process won't be scheduled to run on a core among with processes from
> + * another, untrusted group.
> + */
> +void
> +virCommandSetRunAlone(virCommand *cmd)
> +{
> +    if (virCommandHasError(cmd))
> +        return;
> +
> +    if (cmd->schedCorePID >= 0) {
> +        /* Can't mix these two. */
> +        cmd->has_error = -1;
> +        VIR_DEBUG("cannot mix with virCommandSetRunAmong()");
> +        return;
> +    }
> +
> +    cmd->schedCore = true;
> +}
> +
> +/**
> + * virCommandSetRunAmong:
> + * @pid: pid from a trusted group
> + *
> + * When spawning the command place it into the trusted group of @pid so that
> + * these two processes can run on Hyper Threads of a single core at the same
> + * time.
> + */
> +void
> +virCommandSetRunAmong(virCommand *cmd,
> +                      pid_t pid)
> +{
> +    if (virCommandHasError(cmd))
> +        return;
> +
> +    if (cmd->schedCore) {
> +        /* Can't mix these two. */
> +        VIR_DEBUG("cannot mix with virCommandSetRunAlone()");
> +        cmd->has_error = -1;
> +        return;
> +    }
> +
> +    if (pid < 0) {
> +        VIR_DEBUG("invalid pid value: %lld", (long long) pid);
> +        cmd->has_error = -1;
> +        return;
> +    }
> +
> +    cmd->schedCorePID = pid;
> +}

It strikes me that we can handle this with only 1 variable and
thus avoid the possibility of mutually incompatible settings.

PID == 0 is not a valid PID for sharing a group with so we
can have

  pid_t schedCore;

with

  0 == no core scheduling
  1 == don't be silly we shouldn't ever copy 'init's PID :-)
  >1 == copy scheduling group from said PID
  -1 == create a new scheduling group


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