Re: [libvirt] [PATCH 2/8] qemu: Remove qemuDomainRequiresMemLock()

Instead of having a separate function, we can simply return zero from the existing qemuDomainGetMemLockLimitBytes() to signal the caller that the memory locking limit doesn't need to be set for the guest. Having a single function instead of two makes it less likely that we will use the wrong value, which is exactly what happened when we started applying the limit that was meant for VFIO-using guests to <memoryBacking><locked>-using guests. --- src/qemu/qemu_command.c | 4 +--- src/qemu/qemu_domain.c | 62 ++++++++++++++++++------------------------------- src/qemu/qemu_domain.h | 1 - 3 files changed, 24 insertions(+), 43 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2045c2e..52f6e00 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9740,7 +9740,6 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); unsigned int bootHostdevNet = 0; - VIR_DEBUG("driver=%p def=%p mon=%p json=%d " "qemuCaps=%p migrateURI=%s snapshot=%p vmop=%d", driver, def, monitor_chr, monitor_json, @@ -9966,8 +9965,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, /* In some situations, eg. VFIO passthrough, QEMU might need to lock a * significant amount of memory, so we need to set the limit accordingly */ - if (qemuDomainRequiresMemLock(def)) - virCommandSetMaxMemLock(cmd, qemuDomainGetMemLockLimitBytes(def)); + virCommandSetMaxMemLock(cmd, qemuDomainGetMemLockLimitBytes(def)); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MSG_TIMESTAMP) && cfg->logTimestamp) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8fa43f2..2e2ba4f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6199,18 +6199,20 @@ qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver, /** * qemuDomainGetMemLockLimitBytes: - * * @def: domain definition * - * Returns the size of the memory in bytes that needs to be set as - * RLIMIT_MEMLOCK for the QEMU process. - * If a mem.hard_limit is set, then that value is preferred; otherwise, the - * value returned may depend upon the architecture or devices present. + * Calculate the memory locking limit that needs to be set in order for + * the guest to operate properly. The limit depends on a number of factors, + * including certain configuration options and less immediately apparent ones + * such as the guest architecture or the use of certain devices. + * + * Returns: the memory locking limit, or 0 if setting the limit is not needed */ unsigned long long qemuDomainGetMemLockLimitBytes(virDomainDefPtr def) { - unsigned long long memKB; + unsigned long long memKB = 0; + size_t i; /* prefer the hard limit */ if (virMemoryLimitIsSet(def->mem.hard_limit)) { @@ -6218,13 +6220,17 @@ qemuDomainGetMemLockLimitBytes(virDomainDefPtr def) goto done; } - if (ARCH_IS_PPC64(def->os.arch)) { + if (def->mem.locked) { + memKB = virDomainDefGetMemoryTotal(def) + 1024 * 1024; + goto done; + } + + if (ARCH_IS_PPC64(def->os.arch) && def->virtType == VIR_DOMAIN_VIRT_KVM) { unsigned long long maxMemory; unsigned long long memory; unsigned long long baseLimit; unsigned long long passthroughLimit; size_t nPCIHostBridges; - size_t i; bool usesVFIO = false; /* TODO: Detect at runtime once we start using more than just @@ -6314,44 +6320,21 @@ qemuDomainGetMemLockLimitBytes(virDomainDefPtr def) * * Note that this may not be valid for all platforms. */ - memKB = virDomainDefGetMemoryTotal(def) + 1024 * 1024; - - done: - return memKB << 10; -} - - -/** - * @def: domain definition - * - * Returns true if the locked memory limit needs to be set or updated because - * of domain configuration, VFIO passthrough devices or architecture-specific - * requirements. - * */ -bool -qemuDomainRequiresMemLock(virDomainDefPtr def) -{ - size_t i; - - if (def->mem.locked) - return true; - for (i = 0; i < def->nhostdevs; i++) { virDomainHostdevDefPtr dev = def->hostdevs[i]; if (dev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && dev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && - dev->source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) - return true; + dev->source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { + memKB = virDomainDefGetMemoryTotal(def) + 1024 * 1024;

+ } } - /* ppc64 KVM domains need to lock some memory even when VFIO is not used */ - if (ARCH_IS_PPC64(def->os.arch) && def->virtType == VIR_DOMAIN_VIRT_KVM) - return true; - - return false; + done: + return memKB << 10; } + /** * qemuDomainAdjustMaxMemLock: * @vm: domain @@ -6372,7 +6355,9 @@ qemuDomainAdjustMaxMemLock(virDomainObjPtr vm) unsigned long long bytes = 0; int ret = -1; - if (qemuDomainRequiresMemLock(vm->def)) { + bytes = qemuDomainGetMemLockLimitBytes(vm->def); + + if (bytes) { /* If this is the first time adjusting the limit, save the current * value so that we can restore it once memory locking is no longer * required. Failing to obtain the current limit is not a critical @@ -6381,7 +6366,6 @@ qemuDomainAdjustMaxMemLock(virDomainObjPtr vm) if (virProcessGetMaxMemLock(vm->pid, &(vm->original_memlock)) < 0) vm->original_memlock = 0; } - bytes = qemuDomainGetMemLockLimitBytes(vm->def); } else { /* Once memory locking is no longer required, we can restore the * original, usually very low, limit */