In monitor mode, bhyve(8) has two processes per VM: the monitor process
and a VM process. The process started by the libvirt driver is a monitor
process, and its PID is stored as vm->pid. To obtain a VM process ID,
use "bhyvectl --get-vm-pid". We need this VM for shutting down a domain
by signal and querying process-specific statistics.

RCTL operations require privileges, so resource limits are not supported
in unprivileged mode. Reject RCTL-backed domain configuration and memory
parameter operations instead of applying them to the VM child.

Do not persist the VM process ID as it may change, for example when a
domain reboots.

Signed-off-by: Roman Bogorodskiy <[email protected]>
---
 src/bhyve/bhyve_command.c |  14 +++++
 src/bhyve/bhyve_command.h |   3 ++
 src/bhyve/bhyve_domain.c  |  14 +++--
 src/bhyve/bhyve_driver.c  |  25 ++++++++-
 src/bhyve/bhyve_process.c | 107 ++++++++++++++++++++++++++++++++++----
 src/bhyve/bhyve_process.h |   2 +
 tests/bhyvexml2argvtest.c |   8 +++
 tests/bhyvexml2xmltest.c  |   2 +
 8 files changed, 159 insertions(+), 16 deletions(-)

diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index e3133c4b62..6b7431ddaf 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -1358,6 +1358,20 @@ virBhyveProcessBuildResetCmd(struct _bhyveConn *driver 
G_GNUC_UNUSED,
     return cmd;
 }
 
+virCommand *
+virBhyveProcessBuildGetVMPidCmd(struct _bhyveConn *driver,
+                                virDomainDef *def)
+{
+    virCommand *cmd = virCommandNew("bhyvectl");
+
+    virCommandAddArg(cmd, "--get-vm-pid");
+    virCommandAddArgPair(cmd, "--vm", def->name);
+    if (!driver->privileged)
+        virCommandAddArgPair(cmd, "--rundir", driver->config->stateDir);
+
+    return cmd;
+}
+
 static void
 virAppendBootloaderArgs(virCommand *cmd, virDomainDef *def)
 {
diff --git a/src/bhyve/bhyve_command.h b/src/bhyve/bhyve_command.h
index e946090411..2ee3f3a607 100644
--- a/src/bhyve/bhyve_command.h
+++ b/src/bhyve/bhyve_command.h
@@ -43,3 +43,6 @@ virBhyveProcessBuildLoadCmd(struct _bhyveConn *driver, 
virDomainDef *def,
 
 virCommand *
 virBhyveProcessBuildResetCmd(struct _bhyveConn *driver, virDomainDef *def);
+
+virCommand *
+virBhyveProcessBuildGetVMPidCmd(struct _bhyveConn *driver, virDomainDef *def);
diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c
index 2513573e98..584257edba 100644
--- a/src/bhyve/bhyve_domain.c
+++ b/src/bhyve/bhyve_domain.c
@@ -636,9 +636,17 @@ bhyveDomainDefValidate(const virDomainDef *def,
     if (virMemoryLimitIsSet(def->mem.soft_limit) ||
         virMemoryLimitIsSet(def->mem.swap_hard_limit) ||
         def->mem.min_guarantee) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("Only 'hard_limit' memory tuning parameter is 
supported by bhyve"));
-            return -1;
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("Only 'hard_limit' memory tuning parameter is 
supported by bhyve"));
+        return -1;
+    }
+
+    if (!driver->privileged &&
+        (def->blkio.ndevices > 0 ||
+         virMemoryLimitIsSet(def->mem.hard_limit))) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("resource limits are not supported in unprivileged 
mode"));
+        return -1;
     }
 
     if (!driver->privileged && !def->os.loader) {
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 3058420bae..4c4ba9b0ba 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -1906,6 +1906,7 @@ bhyveDomainMemoryStats(virDomainPtr domain,
     unsigned long long rss;
     size_t i = 0;
     int ret = -1;
+    pid_t pid;
 
     virCheckFlags(0, -1);
 
@@ -1918,7 +1919,10 @@ bhyveDomainMemoryStats(virDomainPtr domain,
     if (virDomainMemoryStatsEnsureACL(domain->conn, vm->def) < 0)
         goto cleanup;
 
-    if (virProcessGetStatInfo(NULL, NULL, NULL, NULL, &rss, vm->pid, 0) < 0) {
+    if ((pid = virBhyveProcessGetBhyvePid(vm)) < 0)
+        goto cleanup;
+
+    if (virProcessGetStatInfo(NULL, NULL, NULL, NULL, &rss, pid, 0) < 0) {
         virReportError(VIR_ERR_OPERATION_FAILED, "%s",
                        _("cannot get RSS for domain"));
     } else {
@@ -1964,6 +1968,7 @@ bhyveDomainBlockStats(virDomainPtr domain,
     virDomainObj *vm;
     int ret = -1;
     g_autofree struct kinfo_proc *p = NULL;
+    pid_t pid;
 
     if (!(vm = bhyveDomObjFromDomain(domain)))
         goto cleanup;
@@ -1974,7 +1979,10 @@ bhyveDomainBlockStats(virDomainPtr domain,
     if (virDomainBlockStatsEnsureACL(domain->conn, vm->def) < 0)
         goto cleanup;
 
-    if ((p = bhyveDomainProcGetInfo(vm->pid)) == NULL)
+    if ((pid = virBhyveProcessGetBhyvePid(vm)) < 0)
+        goto cleanup;
+
+    if ((p = bhyveDomainProcGetInfo(pid)) == NULL)
         goto cleanup;
 
     stats->rd_req = p->ki_rusage.ru_inblock;
@@ -2305,6 +2313,7 @@ bhyveDomainGetMemoryParameters(virDomainPtr domain,
                                int *nparams,
                                unsigned int flags)
 {
+    struct _bhyveConn *privconn = domain->conn->privateData;
     virDomainObj *vm = NULL;
     virDomainDef *persistentDef = NULL;
     int ret = -1;
@@ -2320,6 +2329,12 @@ bhyveDomainGetMemoryParameters(virDomainPtr domain,
     if (virDomainGetMemoryParametersEnsureACL(domain->conn, vm->def) < 0)
         goto cleanup;
 
+    if (!privconn->privileged) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("memory parameters are not supported in unprivileged 
mode"));
+        goto cleanup;
+    }
+
     if (virDomainObjGetDefs(vm, flags, NULL, &persistentDef) < 0)
         goto cleanup;
 
@@ -2378,6 +2393,12 @@ bhyveDomainSetMemoryParameters(virDomainPtr domain,
     if (virDomainSetMemoryParametersEnsureACL(domain->conn, vm->def, flags) < 
0)
         goto cleanup;
 
+    if (!privconn->privileged) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("memory parameters are not supported in unprivileged 
mode"));
+        goto cleanup;
+    }
+
     if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0)
         goto cleanup;
 
diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
index 170889f651..0dc7d18289 100644
--- a/src/bhyve/bhyve_process.c
+++ b/src/bhyve/bhyve_process.c
@@ -138,6 +138,78 @@ bhyveProcessStopHook(struct _bhyveConn *driver,
                 VIR_HOOK_SUBOP_END, NULL, xml, NULL);
 }
 
+static pid_t
+bhyveProcessQueryVMPid(virDomainObj *vm)
+{
+    bhyveDomainObjPrivate *priv = vm->privateData;
+    g_autoptr(virCommand) cmd = NULL;
+    g_autofree char *output = NULL;
+    const char *pidstr;
+    char *end = NULL;
+    pid_t pid;
+
+    cmd = virBhyveProcessBuildGetVMPidCmd(priv->driver, vm->def);
+    virCommandSetOutputBuffer(cmd, &output);
+
+    if (virCommandRun(cmd, NULL) < 0)
+        return -1;
+
+    virStringTrimOptionalNewline(output);
+    if (!STRPREFIX(output, "vm pid:\t"))
+        goto malformed;
+
+    pidstr = output + strlen("vm pid:\t");
+    if (virStrToLong_i(pidstr, &end, 10, &pid) < 0 ||
+        pid <= 0 || *end != '\0')
+        goto malformed;
+
+    return pid;
+
+ malformed:
+    virReportError(VIR_ERR_INTERNAL_ERROR,
+                   _("Unable to parse bhyvectl output: %1$s"), output);
+    return -1;
+}
+
+
+static int
+bhyveProcessWaitForVMPid(virDomainObj *vm)
+{
+    virTimeBackOffVar timebackoff;
+
+    if (virTimeBackOffStart(&timebackoff, 1, 5000) < 0)
+        return -1;
+
+    while (virTimeBackOffWait(&timebackoff)) {
+        if (bhyveProcessQueryVMPid(vm) > 0)
+            return 0;
+
+        virResetLastError();
+    }
+
+    return bhyveProcessQueryVMPid(vm) < 0 ? -1 : 0;
+}
+
+
+pid_t
+virBhyveProcessGetBhyvePid(virDomainObj *vm)
+{
+    bhyveDomainObjPrivate *priv = vm->privateData;
+
+    if (vm->pid == 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Invalid PID %1$d for VM"),
+                       (int)vm->pid);
+        return -1;
+    }
+
+    if (priv->driver->privileged)
+        return vm->pid;
+
+    return bhyveProcessQueryVMPid(vm);
+}
+
+
 static int
 bhyveSetResourceLimits(struct _bhyveConn *driver, virDomainObj *vm)
 {
@@ -147,6 +219,12 @@ bhyveSetResourceLimits(struct _bhyveConn *driver, 
virDomainObj *vm)
         !virMemoryLimitIsSet(vm->def->mem.hard_limit))
         return 0;
 
+    if (!driver->privileged) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("resource limits are not supported in unprivileged 
mode"));
+        return -1;
+    }
+
     if ((bhyveDriverGetBhyveCaps(driver) & BHYVE_CAP_RCTL) == 0) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                        _("Cannot set resource limits: RACCT/RCTL is either not 
supported or not enabled"));
@@ -156,7 +234,8 @@ bhyveSetResourceLimits(struct _bhyveConn *driver, 
virDomainObj *vm)
     if (vm->def->blkio.ndevices == 1) {
         device = &vm->def->blkio.devices[0];
 
-        bhyveRctlSetIoLimits(vm->pid, device);
+        if (bhyveRctlSetIoLimits(vm->pid, device) < 0)
+            return -1;
     }
 
     /* rctl(8) uses bytes for these values and def->mem.* uses kibibytes */
@@ -414,6 +493,10 @@ virBhyveProcessStartImpl(struct _bhyveConn *driver,
         goto cleanup;
     }
 
+    /* /dev/vmm can appear before the monitor mode IPC socket is ready. */
+    if (!driver->privileged && bhyveProcessWaitForVMPid(vm) < 0)
+        goto cleanup;
+
     vm->def->id = vm->pid;
     virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, reason);
     if ((priv->mon = bhyveMonitorOpen(vm, driver)) == NULL)
@@ -754,20 +837,18 @@ virBhyveProcessStop(struct _bhyveConn *driver,
 int
 virBhyveProcessShutdown(virDomainObj *vm)
 {
-    if (vm->pid == 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Invalid PID %1$d for VM"),
-                       (int)vm->pid);
+    pid_t pid;
+
+    if ((pid = virBhyveProcessGetBhyvePid(vm)) < 0)
         return -1;
-    }
 
     /* Bhyve tries to perform ACPI shutdown when it receives
      * SIGTERM signal. So we just issue SIGTERM here and rely
      * on the bhyve monitor to clean things up if process disappears.
      */
-    if (virProcessKill(vm->pid, SIGTERM) != 0) {
-        VIR_WARN("Failed to terminate bhyve process for VM '%s': %s",
-                 vm->def->name, virGetLastErrorMessage());
+    if (virProcessKill(pid, SIGTERM) != 0) {
+        VIR_WARN("Failed to terminate bhyve process for VM '%s' (pid: %d): %s",
+                 vm->def->name, (int)pid, virGetLastErrorMessage());
         return -1;
     }
 
@@ -808,6 +889,10 @@ virBhyveGetDomainTotalCpuStats(virDomainObj *vm,
     g_autofree char *errbuf = g_new0(char, _POSIX2_LINE_MAX);
     int nprocs;
     int ret = -1;
+    pid_t pid;
+
+    if ((pid = virBhyveProcessGetBhyvePid(vm)) < 0)
+        return -1;
 
     if ((kd = kvm_openfiles(NULL, NULL, NULL, O_RDONLY, errbuf)) == NULL) {
         virReportError(VIR_ERR_SYSTEM_ERROR,
@@ -817,11 +902,11 @@ virBhyveGetDomainTotalCpuStats(virDomainObj *vm,
 
     }
 
-    kp = kvm_getprocs(kd, KERN_PROC_PID, vm->pid, &nprocs);
+    kp = kvm_getprocs(kd, KERN_PROC_PID, pid, &nprocs);
     if (kp == NULL || nprocs != 1) {
         virReportError(VIR_ERR_SYSTEM_ERROR,
                        _("Unable to obtain information about pid: %1$d"),
-                       (int)vm->pid);
+                       (int)pid);
         goto cleanup;
     }
 
diff --git a/src/bhyve/bhyve_process.h b/src/bhyve/bhyve_process.h
index 832925851c..2f0ce21167 100644
--- a/src/bhyve/bhyve_process.h
+++ b/src/bhyve/bhyve_process.h
@@ -55,6 +55,8 @@ int virBhyveProcessReset(struct _bhyveConn *driver,
 
 int virBhyveProcessShutdown(virDomainObj *vm);
 
+pid_t virBhyveProcessGetBhyvePid(virDomainObj *vm);
+
 int virBhyveGetDomainTotalCpuStats(virDomainObj *vm,
                                    unsigned long long *cpustats);
 
diff --git a/tests/bhyvexml2argvtest.c b/tests/bhyvexml2argvtest.c
index 06af9dec8a..f7676142af 100644
--- a/tests/bhyvexml2argvtest.c
+++ b/tests/bhyvexml2argvtest.c
@@ -21,6 +21,7 @@ typedef enum {
     FLAG_EXPECT_FAILURE         = 1 << 0,
     FLAG_EXPECT_PARSE_ERROR     = 1 << 1,
     FLAG_EXPECT_PREPARE_ERROR   = 1 << 2,
+    FLAG_UNPRIVILEGED           = 1 << 3,
 } virBhyveXMLToArgvTestFlags;
 
 static int testCompareXMLToArgvFiles(const char *xml,
@@ -141,6 +142,8 @@ testCompareXMLToArgvHelper(const void *data)
     dmargs = g_strdup_printf("%s/bhyvexml2argvdata/%s/bhyvexml2argv-%s.devmap",
                              abs_srcdir, arch, info->name);
 
+    driver.privileged = !(info->flags & FLAG_UNPRIVILEGED);
+
     return testCompareXMLToArgvFiles(xml, args, ldargs, dmargs, info->flags);
 }
 
@@ -192,6 +195,9 @@ mymain(void)
 # define DO_TEST_FAILURE(name) \
     DO_TEST_FULL(name, FLAG_EXPECT_FAILURE)
 
+# define DO_TEST_UNPRIVILEGED_FAILURE(name) \
+    DO_TEST_FULL(name, FLAG_EXPECT_FAILURE | FLAG_UNPRIVILEGED)
+
 # define DO_TEST_PARSE_ERROR(name) \
     DO_TEST_FULL(name, FLAG_EXPECT_PARSE_ERROR)
 
@@ -295,6 +301,7 @@ mymain(void)
     DO_TEST("virtio-scsi");
     DO_TEST("vcpupin");
     DO_TEST("blkiotune");
+    DO_TEST_UNPRIVILEGED_FAILURE("blkiotune");
     DO_TEST_FAILURE("blkiotune-invalid-device");
     DO_TEST_FAILURE("blkiotune-multiple-devices");
     DO_TEST_FAILURE("blkiotune-weight");
@@ -307,6 +314,7 @@ mymain(void)
     DO_TEST_FAILURE("virtio-console-invalid-name");
     DO_TEST_FAILURE("virtio-console-invalid-path");
     DO_TEST("memtune");
+    DO_TEST_UNPRIVILEGED_FAILURE("memtune");
     DO_TEST_FAILURE("memtune-unsupported-params");
     DO_TEST("sysinfo-smbios");
 
diff --git a/tests/bhyvexml2xmltest.c b/tests/bhyvexml2xmltest.c
index 92710b3f0f..96c2e7a3a4 100644
--- a/tests/bhyvexml2xmltest.c
+++ b/tests/bhyvexml2xmltest.c
@@ -64,6 +64,8 @@ mymain(void)
     if ((driver.caps = virBhyveCapsBuild()) == NULL)
         return EXIT_FAILURE;
 
+    driver.privileged = true;
+
     if ((driver.xmlopt = virBhyveDriverCreateXMLConf(&driver)) == NULL)
         return EXIT_FAILURE;
 
-- 
2.55.0

Reply via email to