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 <bogorodskiy@gmail.com> --- 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