On Thu, 23 Mar 2017 19:16:40 +0100 Andrea Bolognani <abologna(a)redhat.com&gt; wrote: Actually, it seems that limits.conf doesn't work with libvirt as mentioned by Daniel in another thread. I didn't know this myself btw. This makes this series even more important because only through libvirt we can set this limit to infinity.

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 */ diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 1f266bf..b9c8f16 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -681,7 +681,6 @@ int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver, virDomainObjPtr vm); unsigned long long qemuDomainGetMemLockLimitBytes(virDomainDefPtr def); -bool qemuDomainRequiresMemLock(virDomainDefPtr def); int qemuDomainAdjustMaxMemLock(virDomainObjPtr vm); int qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, -- 2.7.4

On Mon, 2017-03-27 at 14:19 +0200, Martin Kletzander wrote: [...] Nope, it's guest memory + 1 GiB regardless of the number of VFIO devices. There should be a 'goto done' after setting memKB to quit the loop early, though. Consider that squashed in. Yeah, it's slightly confusing even to me, and I'm the one who wrote it! If you have any suggestions on how to improve it, I'm all ears :) --  Andrea Bolognani / Red Hat / Virtualization

On Thu, Mar 23, 2017 at 07:16:42PM +0100, Andrea Bolognani wrote: So there is no way how one can limit the size of the memlock, other than setting the hard limit? Are you planning on adding new element to the domain XML which would allow setting this number as well?

On Mon, Mar 27, 2017 at 03:17:12PM +0200, Andrea Bolognani wrote: Well, honestly, then I feel really bad about forcing people do different choices and changing it between releases. I don't think anyone wants to be checking all documentation changes every release. But since there are only two releases with the patch being in, it's probably okay. But I would *at least* be nice to mention that the lock limit is on it's way to the XML.

On Thu, 23 Mar 2017 19:16:43 +0100 Andrea Bolognani <abologna(a)redhat.com&gt; wrote: I know I'm not very smart, but I had trouble parsing this. What about: if (virMemoryLimitIsInfinity(bytes)) rlim.rlim_cur = rlim.rlim_max = RLIM_INFINITY; ... This reads better, and avoids using virMemoryLimitIsSet() which seems very error-prone. It doesn't check for zero and it's strange that "limit < infinity" means "limit is set".

This will be used later on in the test suite. --- src/libvirt_private.syms | 1 + src/util/vircommand.c | 9 +++++++++ src/util/vircommand.h | 2 ++ 3 files changed, 12 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a1c7624..a978d61 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1433,6 +1433,7 @@ virCommandDaemonize; virCommandDoAsyncIO; virCommandExec; virCommandFree; +virCommandGetMaxMemLock; virCommandHandshakeNotify; virCommandHandshakeWait; virCommandNew; diff --git a/src/util/vircommand.c b/src/util/vircommand.c index e1bbc05..af97049 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -1083,6 +1083,15 @@ virCommandSetUID(virCommandPtr cmd, uid_t uid) cmd->uid = uid; } +unsigned long long +virCommandGetMaxMemLock(virCommandPtr cmd) +{ + if (!cmd || cmd->has_error) + return 0; + + return cmd->maxMemLock; +} + void virCommandSetMaxMemLock(virCommandPtr cmd, unsigned long long bytes) { diff --git a/src/util/vircommand.h b/src/util/vircommand.h index 99dcdeb..0fb4c8d 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -72,6 +72,8 @@ void virCommandSetGID(virCommandPtr cmd, gid_t gid); void virCommandSetUID(virCommandPtr cmd, uid_t uid); +unsigned long long virCommandGetMaxMemLock(virCommandPtr cmd); + void virCommandSetMaxMemLock(virCommandPtr cmd, unsigned long long bytes); void virCommandSetMaxProcesses(virCommandPtr cmd, unsigned int procs); void virCommandSetMaxFiles(virCommandPtr cmd, unsigned int files); -- 2.7.4

These tests cover a number of scenarios where we care about the memory locking limit being set correctly for the guest to work properly. --- tests/Makefile.am | 9 ++ .../qemumemlock-pc-hardlimit+hostdev.xml | 20 +++ .../qemumemlock-pc-hardlimit+locked+hostdev.xml | 23 +++ .../qemumemlock-pc-hardlimit+locked.xml | 17 ++ tests/qemumemlockdata/qemumemlock-pc-hardlimit.xml | 14 ++ tests/qemumemlockdata/qemumemlock-pc-hostdev.xml | 17 ++ tests/qemumemlockdata/qemumemlock-pc-kvm.xml | 11 ++ .../qemumemlock-pc-locked+hostdev.xml | 20 +++ tests/qemumemlockdata/qemumemlock-pc-locked.xml | 14 ++ tests/qemumemlockdata/qemumemlock-pc-tcg.xml | 11 ++ .../qemumemlock-pseries-hardlimit+hostdev.xml | 20 +++ ...emumemlock-pseries-hardlimit+locked+hostdev.xml | 23 +++ .../qemumemlock-pseries-hardlimit+locked.xml | 17 ++ .../qemumemlock-pseries-hardlimit.xml | 14 ++ .../qemumemlock-pseries-hostdev.xml | 17 ++ tests/qemumemlockdata/qemumemlock-pseries-kvm.xml | 11 ++ .../qemumemlock-pseries-locked+hostdev.xml | 20 +++ .../qemumemlockdata/qemumemlock-pseries-locked.xml | 14 ++ tests/qemumemlockdata/qemumemlock-pseries-tcg.xml | 11 ++ tests/qemumemlocktest.c | 172 +++++++++++++++++++++ 20 files changed, 475 insertions(+) create mode 100644 tests/qemumemlockdata/qemumemlock-pc-hardlimit+hostdev.xml create mode 100644 tests/qemumemlockdata/qemumemlock-pc-hardlimit+locked+hostdev.xml create mode 100644 tests/qemumemlockdata/qemumemlock-pc-hardlimit+locked.xml create mode 100644 tests/qemumemlockdata/qemumemlock-pc-hardlimit.xml create mode 100644 tests/qemumemlockdata/qemumemlock-pc-hostdev.xml create mode 100644 tests/qemumemlockdata/qemumemlock-pc-kvm.xml create mode 100644 tests/qemumemlockdata/qemumemlock-pc-locked+hostdev.xml create mode 100644 tests/qemumemlockdata/qemumemlock-pc-locked.xml create mode 100644 tests/qemumemlockdata/qemumemlock-pc-tcg.xml create mode 100644 tests/qemumemlockdata/qemumemlock-pseries-hardlimit+hostdev.xml create mode 100644 tests/qemumemlockdata/qemumemlock-pseries-hardlimit+locked+hostdev.xml create mode 100644 tests/qemumemlockdata/qemumemlock-pseries-hardlimit+locked.xml create mode 100644 tests/qemumemlockdata/qemumemlock-pseries-hardlimit.xml create mode 100644 tests/qemumemlockdata/qemumemlock-pseries-hostdev.xml create mode 100644 tests/qemumemlockdata/qemumemlock-pseries-kvm.xml create mode 100644 tests/qemumemlockdata/qemumemlock-pseries-locked+hostdev.xml create mode 100644 tests/qemumemlockdata/qemumemlock-pseries-locked.xml create mode 100644 tests/qemumemlockdata/qemumemlock-pseries-tcg.xml create mode 100644 tests/qemumemlocktest.c diff --git a/tests/Makefile.am b/tests/Makefile.am index af69a3a..ccf5ee1 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -127,6 +127,7 @@ EXTRA_DIST = \ qemumonitorjsondata \ qemuxml2argvdata \ qemuxml2xmloutdata \ + qemumemlockdata \ secretxml2xmlin \ securityselinuxhelperdata \ securityselinuxlabeldata \ @@ -276,6 +277,7 @@ test_programs += qemuxml2argvtest qemuxml2xmltest \ qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest \ qemumonitortest qemumonitorjsontest qemuhotplugtest \ qemuagenttest qemucapabilitiestest qemucaps2xmltest \ + qemumemlocktest \ qemucommandutiltest test_helpers += qemucapsprobe test_libraries += libqemumonitortestutils.la \ @@ -658,6 +660,12 @@ domainsnapshotxml2xmltest_SOURCES = \ domainsnapshotxml2xmltest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h domainsnapshotxml2xmltest_LDADD = $(qemu_LDADDS) $(LDADDS) + +qemumemlocktest_SOURCES = \ + qemumemlocktest.c \ + testutilsqemu.c testutilsqemu.h \ + testutils.c testutils.h +qemumemlocktest_LDADD = $(qemu_LDADDS) $(LDADDS) else ! WITH_QEMU EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \ qemuhelptest.c domainsnapshotxml2xmltest.c \ @@ -665,6 +673,7 @@ EXTRA_DIST += qemuxml2argvtest.c qemuxml2xmltest.c qemuargv2xmltest.c \ qemumonitorjsontest.c qemuhotplugtest.c \ qemuagenttest.c qemucapabilitiestest.c \ qemucaps2xmltest.c qemucommandutiltest.c \ + qemumemlocktest.c \ $(QEMUMONITORTESTUTILS_SOURCES) endif ! WITH_QEMU diff --git a/tests/qemumemlockdata/qemumemlock-pc-hardlimit+hostdev.xml b/tests/qemumemlockdata/qemumemlock-pc-hardlimit+hostdev.xml new file mode 100644 index 0000000..5443145 --- /dev/null +++ b/tests/qemumemlockdata/qemumemlock-pc-hardlimit+hostdev.xml @@ -0,0 +1,20 @@ +<domain type='kvm'> + <name>guest</name> + <memory unit='KiB'>1048576</memory> + <memtune> + <hard_limit unit='KiB'>2097152</hard_limit> + </memtune> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0000' bus='0x04' slot='0x02' function='0x0'/> + </source> + </hostdev> + </devices> +</domain> diff --git a/tests/qemumemlockdata/qemumemlock-pc-hardlimit+locked+hostdev.xml b/tests/qemumemlockdata/qemumemlock-pc-hardlimit+locked+hostdev.xml new file mode 100644 index 0000000..8184eef --- /dev/null +++ b/tests/qemumemlockdata/qemumemlock-pc-hardlimit+locked+hostdev.xml @@ -0,0 +1,23 @@ +<domain type='kvm'> + <name>guest</name> + <memory unit='KiB'>1048576</memory> + <memtune> + <hard_limit unit='KiB'>2097152</hard_limit> + </memtune> + <memoryBacking> + <locked/> + </memoryBacking> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0000' bus='0x04' slot='0x02' function='0x0'/> + </source> + </hostdev> + </devices> +</domain> diff --git a/tests/qemumemlockdata/qemumemlock-pc-hardlimit+locked.xml b/tests/qemumemlockdata/qemumemlock-pc-hardlimit+locked.xml new file mode 100644 index 0000000..78bee62 --- /dev/null +++ b/tests/qemumemlockdata/qemumemlock-pc-hardlimit+locked.xml @@ -0,0 +1,17 @@ +<domain type='kvm'> + <name>guest</name> + <memory unit='KiB'>1048576</memory> + <memtune> + <hard_limit unit='KiB'>2097152</hard_limit> + </memtune> + <memoryBacking> + <locked/> + </memoryBacking> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemumemlockdata/qemumemlock-pc-hardlimit.xml b/tests/qemumemlockdata/qemumemlock-pc-hardlimit.xml new file mode 100644 index 0000000..b1e3867 --- /dev/null +++ b/tests/qemumemlockdata/qemumemlock-pc-hardlimit.xml @@ -0,0 +1,14 @@ +<domain type='kvm'> + <name>guest</name> + <memory unit='KiB'>1048576</memory> + <memtune> + <hard_limit unit='KiB'>2097152</hard_limit> + </memtune> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemumemlockdata/qemumemlock-pc-hostdev.xml b/tests/qemumemlockdata/qemumemlock-pc-hostdev.xml new file mode 100644 index 0000000..6c058a9 --- /dev/null +++ b/tests/qemumemlockdata/qemumemlock-pc-hostdev.xml @@ -0,0 +1,17 @@ +<domain type='kvm'> + <name>guest</name> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0000' bus='0x04' slot='0x02' function='0x0'/> + </source> + </hostdev> + </devices> +</domain> diff --git a/tests/qemumemlockdata/qemumemlock-pc-kvm.xml b/tests/qemumemlockdata/qemumemlock-pc-kvm.xml new file mode 100644 index 0000000..7fa4e24 --- /dev/null +++ b/tests/qemumemlockdata/qemumemlock-pc-kvm.xml @@ -0,0 +1,11 @@ +<domain type='kvm'> + <name>guest</name> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemumemlockdata/qemumemlock-pc-locked+hostdev.xml b/tests/qemumemlockdata/qemumemlock-pc-locked+hostdev.xml new file mode 100644 index 0000000..fbc1dc3 --- /dev/null +++ b/tests/qemumemlockdata/qemumemlock-pc-locked+hostdev.xml @@ -0,0 +1,20 @@ +<domain type='kvm'> + <name>guest</name> + <memory unit='KiB'>1048576</memory> + <memoryBacking> + <locked/> + </memoryBacking> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0000' bus='0x04' slot='0x02' function='0x0'/> + </source> + </hostdev> + </devices> +</domain> diff --git a/tests/qemumemlockdata/qemumemlock-pc-locked.xml b/tests/qemumemlockdata/qemumemlock-pc-locked.xml new file mode 100644 index 0000000..febb036 --- /dev/null +++ b/tests/qemumemlockdata/qemumemlock-pc-locked.xml @@ -0,0 +1,14 @@ +<domain type='kvm'> + <name>guest</name> + <memory unit='KiB'>1048576</memory> + <memoryBacking> + <locked/> + </memoryBacking> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemumemlockdata/qemumemlock-pc-tcg.xml b/tests/qemumemlockdata/qemumemlock-pc-tcg.xml new file mode 100644 index 0000000..1934269 --- /dev/null +++ b/tests/qemumemlockdata/qemumemlock-pc-tcg.xml @@ -0,0 +1,11 @@ +<domain type='qemu'> + <name>guest</name> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + </devices> +</domain> diff --git a/tests/qemumemlockdata/qemumemlock-pseries-hardlimit+hostdev.xml b/tests/qemumemlockdata/qemumemlock-pseries-hardlimit+hostdev.xml new file mode 100644 index 0000000..ddd3b47 --- /dev/null +++ b/tests/qemumemlockdata/qemumemlock-pseries-hardlimit+hostdev.xml @@ -0,0 +1,20 @@ +<domain type='kvm'> + <name>guest</name> + <memory unit='KiB'>1048576</memory> + <memtune> + <hard_limit unit='KiB'>2097152</hard_limit> + </memtune> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0000' bus='0x04' slot='0x02' function='0x0'/> + </source> + </hostdev> + </devices> +</domain> diff --git a/tests/qemumemlockdata/qemumemlock-pseries-hardlimit+locked+hostdev.xml b/tests/qemumemlockdata/qemumemlock-pseries-hardlimit+locked+hostdev.xml new file mode 100644 index 0000000..73c28c1 --- /dev/null +++ b/tests/qemumemlockdata/qemumemlock-pseries-hardlimit+locked+hostdev.xml @@ -0,0 +1,23 @@ +<domain type='kvm'> + <name>guest</name> + <memory unit='KiB'>1048576</memory> + <memtune> + <hard_limit unit='KiB'>2097152</hard_limit> + </memtune> + <memoryBacking> + <locked/> + </memoryBacking> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0000' bus='0x04' slot='0x02' function='0x0'/> + </source> + </hostdev> + </devices> +</domain> diff --git a/tests/qemumemlockdata/qemumemlock-pseries-hardlimit+locked.xml b/tests/qemumemlockdata/qemumemlock-pseries-hardlimit+locked.xml new file mode 100644 index 0000000..fe984a2 --- /dev/null +++ b/tests/qemumemlockdata/qemumemlock-pseries-hardlimit+locked.xml @@ -0,0 +1,17 @@ +<domain type='kvm'> + <name>guest</name> + <memory unit='KiB'>1048576</memory> + <memtune> + <hard_limit unit='KiB'>2097152</hard_limit> + </memtune> + <memoryBacking> + <locked/> + </memoryBacking> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + </devices> +</domain> diff --git a/tests/qemumemlockdata/qemumemlock-pseries-hardlimit.xml b/tests/qemumemlockdata/qemumemlock-pseries-hardlimit.xml new file mode 100644 index 0000000..b23de1d --- /dev/null +++ b/tests/qemumemlockdata/qemumemlock-pseries-hardlimit.xml @@ -0,0 +1,14 @@ +<domain type='kvm'> + <name>guest</name> + <memory unit='KiB'>1048576</memory> + <memtune> + <hard_limit unit='KiB'>2097152</hard_limit> + </memtune> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + </devices> +</domain> diff --git a/tests/qemumemlockdata/qemumemlock-pseries-hostdev.xml b/tests/qemumemlockdata/qemumemlock-pseries-hostdev.xml new file mode 100644 index 0000000..daf70a4 --- /dev/null +++ b/tests/qemumemlockdata/qemumemlock-pseries-hostdev.xml @@ -0,0 +1,17 @@ +<domain type='kvm'> + <name>guest</name> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0000' bus='0x04' slot='0x02' function='0x0'/> + </source> + </hostdev> + </devices> +</domain> diff --git a/tests/qemumemlockdata/qemumemlock-pseries-kvm.xml b/tests/qemumemlockdata/qemumemlock-pseries-kvm.xml new file mode 100644 index 0000000..5a065da --- /dev/null +++ b/tests/qemumemlockdata/qemumemlock-pseries-kvm.xml @@ -0,0 +1,11 @@ +<domain type='kvm'> + <name>guest</name> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + </devices> +</domain> diff --git a/tests/qemumemlockdata/qemumemlock-pseries-locked+hostdev.xml b/tests/qemumemlockdata/qemumemlock-pseries-locked+hostdev.xml new file mode 100644 index 0000000..74212f1 --- /dev/null +++ b/tests/qemumemlockdata/qemumemlock-pseries-locked+hostdev.xml @@ -0,0 +1,20 @@ +<domain type='kvm'> + <name>guest</name> + <memory unit='KiB'>1048576</memory> + <memoryBacking> + <locked/> + </memoryBacking> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <hostdev mode='subsystem' type='pci' managed='yes'> + <driver name='vfio'/> + <source> + <address domain='0x0000' bus='0x04' slot='0x02' function='0x0'/> + </source> + </hostdev> + </devices> +</domain> diff --git a/tests/qemumemlockdata/qemumemlock-pseries-locked.xml b/tests/qemumemlockdata/qemumemlock-pseries-locked.xml new file mode 100644 index 0000000..befaefd --- /dev/null +++ b/tests/qemumemlockdata/qemumemlock-pseries-locked.xml @@ -0,0 +1,14 @@ +<domain type='kvm'> + <name>guest</name> + <memory unit='KiB'>1048576</memory> + <memoryBacking> + <locked/> + </memoryBacking> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + </devices> +</domain> diff --git a/tests/qemumemlockdata/qemumemlock-pseries-tcg.xml b/tests/qemumemlockdata/qemumemlock-pseries-tcg.xml new file mode 100644 index 0000000..a3b03dd --- /dev/null +++ b/tests/qemumemlockdata/qemumemlock-pseries-tcg.xml @@ -0,0 +1,11 @@ +<domain type='qemu'> + <name>guest</name> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + </devices> +</domain> diff --git a/tests/qemumemlocktest.c b/tests/qemumemlocktest.c new file mode 100644 index 0000000..9e65622 --- /dev/null +++ b/tests/qemumemlocktest.c @@ -0,0 +1,172 @@ +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <string.h> + +#include <sys/types.h> +#include <fcntl.h> + +#include "testutils.h" + +#ifdef WITH_QEMU + +# include "datatypes.h" +# include "internal.h" +# include "virstring.h" +# include "conf/domain_conf.h" +# include "qemu/qemu_capabilities.h" +# include "qemu/qemu_process.h" + +# include "testutilsqemu.h" + +# define VIR_FROM_THIS VIR_FROM_QEMU + +static const char *abs_top_srcdir; +static virQEMUDriver driver; + +struct testInfo { + const char *name; + unsigned long long memlock; +}; + +static int +testCompareMemLock(const void *data) +{ + const struct testInfo *info = data; + virConnectPtr conn = NULL; + virDomainObjPtr vm = NULL; + virCommandPtr cmd = NULL; + char *xml = NULL; + int ret = -1; + + if (!(conn = virGetConnect())) + goto cleanup; + + if (virAsprintf(&xml, "%s/qemumemlockdata/qemumemlock-%s.xml", + abs_srcdir, info->name) < 0) + goto cleanup; + + if (!(vm = virDomainObjNew(driver.xmlopt))) + goto cleanup; + + if (!(vm->def = virDomainDefParseFile(xml, driver.caps, driver.xmlopt, NULL, + VIR_DOMAIN_DEF_PARSE_INACTIVE))) { + goto cleanup; + } + + if (!(cmd = qemuProcessCreatePretendCmd(conn, &driver, vm, NULL, 0, false, + VIR_QEMU_PROCESS_START_COLD))) { + goto cleanup; + } + + if (virTestCompareToULL(info->memlock, virCommandGetMaxMemLock(cmd)) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virCommandFree(cmd); + virObjectUnref(vm); + virObjectUnref(conn); + VIR_FREE(xml); + + return ret; +} + + +int +main(void) +{ + virQEMUCapsPtr qemuCaps = NULL; + int ret = 0; + + abs_top_srcdir = getenv("abs_top_srcdir"); + if (!abs_top_srcdir) + abs_top_srcdir = abs_srcdir "/.."; + + if (qemuTestDriverInit(&driver) < 0) + return EXIT_FAILURE; + + driver.privileged = true; + + /* QEMU capabilites are the same for all tests */ + if (!(qemuCaps = virQEMUCapsNew())) + return EXIT_FAILURE; + + virQEMUCapsSetList(qemuCaps, + QEMU_CAPS_KVM, QEMU_CAPS_REALTIME_MLOCK, + QEMU_CAPS_DEVICE_VFIO_PCI); + + if (qemuTestCapsCacheInsert(driver.qemuCapsCache, "qemumemlock", qemuCaps) < 0) + return EXIT_FAILURE; + + virObjectUnref(qemuCaps); + +# define DO_TEST(name, memlock) \ + do { \ + static struct testInfo info = { \ + name, memlock \ + }; \ + if (virTestRun("QEMU MEMLOCK " name, testCompareMemLock, &info) < 0) \ + ret = -1; \ + } while (0) + + /* The tests below make sure that the memory locking limit is being + * calculated correctly in a number of situations. Each test is + * performed both on x86_64/pc and ppc64/pseries in order to account + * for some architecture-specific details. + * + * kvm: simple KMV guest + * tcg: simple TCG guest + * + * hardlimit: guest where <memtune><hard_limit> has been configured + * locked: guest where <memoryBacking><locked> has been enabled + * hostdev: guest that has some hostdev assigned + * + * The remaining tests cover different combinations of the above to + * ensure settings are prioritized as expected. + */ + + qemuTestSetHostArch(driver.caps, VIR_ARCH_X86_64); + + DO_TEST("pc-kvm", 0); + DO_TEST("pc-tcg", 0); + + DO_TEST("pc-hardlimit", 2147483648); + DO_TEST("pc-locked", VIR_DOMAIN_MEMORY_PARAM_UNLIMITED); + DO_TEST("pc-hostdev", 2147483648); + + DO_TEST("pc-hardlimit+locked", 2147483648); + DO_TEST("pc-hardlimit+hostdev", 2147483648); + DO_TEST("pc-hardlimit+locked+hostdev", 2147483648); + DO_TEST("pc-locked+hostdev", VIR_DOMAIN_MEMORY_PARAM_UNLIMITED); + + qemuTestSetHostArch(driver.caps, VIR_ARCH_PPC64); + + DO_TEST("pseries-kvm", 20971520); + DO_TEST("pseries-tcg", 0); + + DO_TEST("pseries-hardlimit", 2147483648); + DO_TEST("pseries-locked", VIR_DOMAIN_MEMORY_PARAM_UNLIMITED); + DO_TEST("pseries-hostdev", 2168455168); + + DO_TEST("pseries-hardlimit+locked", 2147483648); + DO_TEST("pseries-hardlimit+hostdev", 2147483648); + DO_TEST("pseries-hardlimit+locked+hostdev", 2147483648); + DO_TEST("pseries-locked+hostdev", VIR_DOMAIN_MEMORY_PARAM_UNLIMITED); + + qemuTestDriverFree(&driver); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +#else + +int main(void) +{ + return EXIT_AM_SKIP; +} + +#endif /* WITH_QEMU */ -- 2.7.4

On Thu, Mar 23, 2017 at 07:16:46PM +0100, Andrea Bolognani wrote: [...] Going through all this, just to check one number seems too much. And it doesn't actually test much more than simple call to qemuDomainGetMemLockLimitBytes(def) would. Use different name (e.g. mymain) and VIRT_TEST_MAIN() You should add QEMU_CAPS_LAST here. Do you even need capabilities with the upper part removed? Other than that it looks fine.

--- docs/formatdomain.html.in | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 4a3123e..5906de6 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -937,14 +937,18 @@ <dt><code>locked</code></dt> <dd>When set and supported by the hypervisor, memory pages belonging to the domain will be locked in host's memory and the host will not - be allowed to swap them out. For QEMU/KVM this requires - <code>hard_limit</code> <a href="#elementsMemoryTuning">memory tuning</a> - element to be used and set to the maximum memory configured for the - domain plus any memory consumed by the QEMU process itself. Beware of - setting the memory limit too high (and thus allowing the domain to lock - most of the host's memory). Doing so may be dangerous to both the - domain and the host itself since the host's kernel may run out of - memory. <span class="since">Since 1.0.6</span></dd> + be allowed to swap them out, which might be required for some + workloads such as RT. For QEMU/KVM guests, the memory used by the QEMU + process itself will be locked too: unlike guest memory, this is an + amount libvirt has no way of figuring out in advance, so it has to + remove the limit on locked memory altogether. This can be very + dangerous as the host might run out of memory and be unable to reclaim + it from the guest, so using this option is discouraged unless your + workload demands it; even then, it's highly recommended to set an + <code>hard_limit</code> (see + <a href="#elementsMemoryTuning">memory tuning</a>) on memory allocation + suitable for the specific environment at the same time to mitigate + the risks described above. <span class="since">Since 1.0.6</span></dd> <dt><code>source</code></dt> <dd>In this attribute you can switch to file memorybacking or keep default anonymous.</dd> <dt><code>access</code></dt> @@ -989,12 +993,18 @@ <dt><code>hard_limit</code></dt> <dd> The optional <code>hard_limit</code> element is the maximum memory the guest can use. The units for this value are kibibytes (i.e. blocks - of 1024 bytes). <strong>However, users of QEMU and KVM are strongly - advised not to set this limit as domain may get killed by the kernel - if the guess is too low. To determine the memory needed for a process - to run is an + of 1024 bytes). Users of QEMU and KVM are strongly advised not to set + this limit as domain may get killed by the kernel if the guess is too + low, and determining the memory needed for a process to run is an <a href="http://en.wikipedia.org/wiki/Undecidable_problem"> - undecidable problem</a>.</strong></dd> + undecidable problem</a>; that said, if you already set + <code>locked</code> in + <a href="#elementsMemoryBacking">memory backing</a> because your + workload demands it, you'll have to take into account the specifics of + your deployment and figure out a value for <code>hard_limit</code> that + balances the risk of your guest being killed because the limit was set + too low and the risk of your host crashing because it cannot reclaim + the memory used by the guest due to <code>locked</code>. Good luck!</dd> <dt><code>soft_limit</code></dt> <dd> The optional <code>soft_limit</code> element is the memory limit to enforce during memory contention. The units for this value are -- 2.7.4