[libvirt] [PATCH v2 0/7] Memory locking limit improvements
During my testing, I've realized that even with the series applied there's still one case in which we're unable to restore the previous memory locking limit after removing the last PCI hostdev from the guest. If an x86 guest contains a PCI hostdev in its XML definition, then the memory locking limit will be set correctly, but virCommandGetMaxMemLock() will be used instead of virProcessGetMaxMemLock(), and the limit will be set right before calling exec() to spawn the qemu binary. In this context, we have no access to the virDomainObj or even virDomainDef instances, so I see no way of storing the original limit for later retrieval. The series is still an improvement as it covers all other cases. Still, I thought this was worth mentioning. Changes since v1[1]: * reorder commits according to John's suggestions * don't fail if we can't retrieve the current memory locking limit * small changes here and there as pointed out during review Cheers. [1] https://www.redhat.com/archives/libvir-list/2015-November/msg01021.html Andrea Bolognani (7): process: Allow virProcessPrLimit() to get current limit process: Add virProcessGetMaxMemLock() qemu: Add qemuDomainAdjustMaxMemLock() qemu: Use qemuDomainAdjustMaxMemLock() qemu: Reduce memlock limit after detaching PCI hostdev qemu: Allow qemuDomainAdjustMaxMemLock() to restore previous value qemu: Replace Mlock with MemLock in function names configure.ac | 2 +- src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_domain.c | 53 ++++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_domain.h | 5 ++-- src/qemu/qemu_hotplug.c | 45 ++++++++++++++--------------------- src/util/virprocess.c | 61 +++++++++++++++++++++++++++++++++++++++++++----- src/util/virprocess.h | 2 ++ 9 files changed, 135 insertions(+), 41 deletions(-) -- 2.5.0
The prlimit() function allows both getting and setting limits for a process; expose the same functionality in our wrapper. Add the const modifier for new_limit, in accordance with the prototype for prlimit(). --- src/util/virprocess.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 4b18903..9b38834 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -725,15 +725,19 @@ int virProcessSetNamespaces(size_t nfdlist, #if HAVE_PRLIMIT static int -virProcessPrLimit(pid_t pid, int resource, struct rlimit *rlim) +virProcessPrLimit(pid_t pid, + int resource, + const struct rlimit *new_limit, + struct rlimit *old_limit) { - return prlimit(pid, resource, rlim, NULL); + return prlimit(pid, resource, new_limit, old_limit); } #elif HAVE_SETRLIMIT static int virProcessPrLimit(pid_t pid ATTRIBUTE_UNUSED, int resource ATTRIBUTE_UNUSED, - struct rlimit *rlim ATTRIBUTE_UNUSED) + const struct rlimit *new_limit ATTRIBUTE_UNUSED, + struct rlimit *old_limit ATTRIBUTE_UNUSED) { errno = ENOSYS; return -1; @@ -758,7 +762,7 @@ virProcessSetMaxMemLock(pid_t pid, unsigned long long bytes) return -1; } } else { - if (virProcessPrLimit(pid, RLIMIT_MEMLOCK, &rlim) < 0) { + if (virProcessPrLimit(pid, RLIMIT_MEMLOCK, &rlim, NULL) < 0) { virReportSystemError(errno, _("cannot limit locked memory " "of process %lld to %llu"), @@ -803,7 +807,7 @@ virProcessSetMaxProcesses(pid_t pid, unsigned int procs) return -1; } } else { - if (virProcessPrLimit(pid, RLIMIT_NPROC, &rlim) < 0) { + if (virProcessPrLimit(pid, RLIMIT_NPROC, &rlim, NULL) < 0) { virReportSystemError(errno, _("cannot limit number of subprocesses " "of process %lld to %u"), @@ -851,7 +855,7 @@ virProcessSetMaxFiles(pid_t pid, unsigned int files) return -1; } } else { - if (virProcessPrLimit(pid, RLIMIT_NOFILE, &rlim) < 0) { + if (virProcessPrLimit(pid, RLIMIT_NOFILE, &rlim, NULL) < 0) { virReportSystemError(errno, _("cannot limit number of open files " "of process %lld to %u"), -- 2.5.0
This function can be used to retrieve the current locked memory limit for a process, so that the setting can be later restored. Add a configure check for getrlimit(), which we now use. --- configure.ac | 2 +- src/libvirt_private.syms | 1 + src/util/virprocess.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ src/util/virprocess.h | 2 ++ 4 files changed, 49 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 98cf210..25fdfe2 100644 --- a/configure.ac +++ b/configure.ac @@ -279,7 +279,7 @@ AC_CHECK_SIZEOF([long]) dnl Availability of various common functions (non-fatal if missing), dnl and various less common threadsafe functions AC_CHECK_FUNCS_ONCE([cfmakeraw fallocate geteuid getgid getgrnam_r \ - getmntent_r getpwuid_r getuid kill mmap newlocale posix_fallocate \ + getmntent_r getpwuid_r getrlimit getuid kill mmap newlocale posix_fallocate \ posix_memalign prlimit regexec sched_getaffinity setgroups setns \ setrlimit symlink sysctlbyname getifaddrs sched_setscheduler]) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 55822ae..0b5ddc1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2040,6 +2040,7 @@ virPortAllocatorSetUsed; virProcessAbort; virProcessExitWithStatus; virProcessGetAffinity; +virProcessGetMaxMemLock; virProcessGetNamespaces; virProcessGetPids; virProcessGetStartTime; diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 9b38834..277b3bc 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -788,6 +788,51 @@ virProcessSetMaxMemLock(pid_t pid ATTRIBUTE_UNUSED, unsigned long long bytes) } #endif /* ! (HAVE_SETRLIMIT && defined(RLIMIT_MEMLOCK)) */ +#if HAVE_GETRLIMIT && defined(RLIMIT_MEMLOCK) +int +virProcessGetMaxMemLock(pid_t pid, + unsigned long long *bytes) +{ + struct rlimit rlim; + + if (!bytes) + return 0; + + if (pid == 0) { + if (getrlimit(RLIMIT_MEMLOCK, &rlim) < 0) { + virReportSystemError(errno, + "%s", + _("cannot get locked memory limit")); + return -1; + } + } else { + if (virProcessPrLimit(pid, RLIMIT_MEMLOCK, NULL, &rlim) < 0) { + virReportSystemError(errno, + _("cannot get locked memory limit " + "of process %lld"), + (long long int) pid); + return -1; + } + } + + /* virProcessSetMaxMemLock() sets both rlim_cur and rlim_max to the + * same value, so we can retrieve just rlim_max here */ + *bytes = rlim.rlim_max; + + return 0; +} +#else /* ! (HAVE_GETRLIMIT && defined(RLIMIT_MEMLOCK)) */ +int +virProcessGetMaxMemLock(pid_t pid ATTRIBUTE_UNUSED, + unsigned long long *bytes) +{ + if (!bytes) + return 0; + + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return -1; +} +#endif /* ! (HAVE_GETRLIMIT && defined(RLIMIT_MEMLOCK)) */ #if HAVE_SETRLIMIT && defined(RLIMIT_NPROC) int diff --git a/src/util/virprocess.h b/src/util/virprocess.h index 1768009..a7a1fe9 100644 --- a/src/util/virprocess.h +++ b/src/util/virprocess.h @@ -76,6 +76,8 @@ int virProcessSetMaxMemLock(pid_t pid, unsigned long long bytes); int virProcessSetMaxProcesses(pid_t pid, unsigned int procs); int virProcessSetMaxFiles(pid_t pid, unsigned int files); +int virProcessGetMaxMemLock(pid_t pid, unsigned long long *bytes); + /* Callback to run code within the mount namespace tied to the given * pid. This function must use only async-signal-safe functions, as * it gets run after a fork of a multi-threaded process. The return -- 2.5.0
This function detects whether a domain needs RLIMIT_MEMLOCK to be set, and if so, uses an appropriate value. --- src/qemu/qemu_domain.c | 30 ++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 1 + 2 files changed, 31 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6f19d49..4b796ab 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -41,6 +41,7 @@ #include "virstring.h" #include "virthreadjob.h" #include "viratomic.h" +#include "virprocess.h" #include "logging/log_manager.h" #include "storage/storage_driver.h" @@ -4117,6 +4118,35 @@ qemuDomainRequiresMlock(virDomainDefPtr def) return false; } +/** + * qemuDomainAdjustMaxMemLock: + * @vm: domain + * + * Adjust the memory locking limit for the QEMU process associated to @vm, in + * order to comply with VFIO or architecture requirements. + * + * The limit will not be changed unless doing so is needed. + * + * Returns: 0 on success, <0 on failure + */ +int +qemuDomainAdjustMaxMemLock(virDomainObjPtr vm) +{ + unsigned long long bytes = 0; + int ret = -1; + + if (qemuDomainRequiresMlock(vm->def)) + bytes = qemuDomainGetMlockLimitBytes(vm->def); + + /* Trying to set the memory locking limit to zero is a no-op */ + if (virProcessSetMaxMemLock(vm->pid, bytes) < 0) + goto out; + + ret = 0; + + out: + return ret; +} /** * qemuDomainHasVcpuPids: diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 916d5d3..704351b 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -500,6 +500,7 @@ int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver, unsigned long long qemuDomainGetMlockLimitBytes(virDomainDefPtr def); bool qemuDomainRequiresMlock(virDomainDefPtr def); +int qemuDomainAdjustMaxMemLock(virDomainObjPtr vm); int qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, virQEMUCapsPtr qemuCaps, -- 2.5.0
Replace all uses of the qemuDomainRequiresMlock/virProcessSetMaxMemLock combination with the equivalent qemuDomainAdjustMaxMemLock() call. --- src/qemu/qemu_hotplug.c | 40 +++++++++++++--------------------------- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index a88c2f2..8d69647 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1282,17 +1282,14 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, } /* Temporarily add the hostdev to the domain definition. This is needed - * because qemuDomainRequiresMlock() and qemuDomainGetMlockLimitBytes() - * require the hostdev to be already part of the domain definition, but - * other functions like qemuAssignDeviceHostdevAlias() used below expect - * it *not* to be there. A better way to handle this would be nice */ + * because qemuDomainAdjustMaxMemLock() requires the hostdev to be already + * part of the domain definition, but other functions like + * qemuAssignDeviceHostdevAlias() used below expect it *not* to be there. + * A better way to handle this would be nice */ vm->def->hostdevs[vm->def->nhostdevs++] = hostdev; - if (qemuDomainRequiresMlock(vm->def)) { - if (virProcessSetMaxMemLock(vm->pid, - qemuDomainGetMlockLimitBytes(vm->def)) < 0) { - vm->def->hostdevs[--(vm->def->nhostdevs)] = NULL; - goto error; - } + if (qemuDomainAdjustMaxMemLock(vm) < 0) { + vm->def->hostdevs[--(vm->def->nhostdevs)] = NULL; + goto error; } vm->def->hostdevs[--(vm->def->nhostdevs)] = NULL; @@ -1778,7 +1775,6 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, virJSONValuePtr props = NULL; virObjectEventPtr event; bool fix_balloon = false; - bool mlock = false; int id; int ret = -1; @@ -1810,12 +1806,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, goto cleanup; } - mlock = qemuDomainRequiresMlock(vm->def); - - if (mlock && - virProcessSetMaxMemLock(vm->pid, - qemuDomainGetMlockLimitBytes(vm->def)) < 0) { - mlock = false; + if (qemuDomainAdjustMaxMemLock(vm) < 0) { virJSONValueFree(props); goto removedef; } @@ -1876,13 +1867,10 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, mem = NULL; /* reset the mlock limit */ - if (mlock) { - virErrorPtr err = virSaveLastError(); - ignore_value(virProcessSetMaxMemLock(vm->pid, - qemuDomainGetMlockLimitBytes(vm->def))); - virSetError(err); - virFreeError(err); - } + virErrorPtr err = virSaveLastError(); + ignore_value(qemuDomainAdjustMaxMemLock(vm)); + virSetError(err); + virFreeError(err); goto audit; } @@ -2976,9 +2964,7 @@ qemuDomainRemoveMemoryDevice(virQEMUDriverPtr driver, virDomainMemoryDefFree(mem); /* decrease the mlock limit after memory unplug if necessary */ - if (qemuDomainRequiresMlock(vm->def)) - ignore_value(virProcessSetMaxMemLock(vm->pid, - qemuDomainGetMlockLimitBytes(vm->def))); + ignore_value(qemuDomainAdjustMaxMemLock(vm)); return 0; } -- 2.5.0
We increase the limit before plugging in a PCI hostdev or a memory module because some memory might need to be locked due to eg. VFIO. Of course we should do the opposite after unplugging a device: this was already the case for memory modules, but not for PCI hostdevs. --- src/qemu/qemu_hotplug.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 8d69647..f2a268b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3051,6 +3051,10 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, switch ((virDomainHostdevSubsysType) hostdev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: qemuDomainRemovePCIHostDevice(driver, vm, hostdev); + /* QEMU might no longer need to lock as much memory, eg. we just + * detached the last VFIO device, so adjust the limit here */ + if (qemuDomainAdjustMaxMemLock(vm) < 0) + VIR_WARN("Failed to adjust locked memory limit"); break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: qemuDomainRemoveUSBHostDevice(driver, vm, hostdev); @@ -3076,6 +3080,7 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, networkReleaseActualDevice(vm->def, net); virDomainNetDefFree(net); } + ret = 0; cleanup: -- 2.5.0
When the function changes the memory lock limit for the first time, it will retrieve the current value and store it inside the virDomainObj for the domain. When the function is called again, if memory locking is no longer needed, it will be able to restore the memory locking limit to its original value. --- src/conf/domain_conf.h | 3 +++ src/qemu/qemu_domain.c | 21 +++++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cec681a..952d3cc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2420,6 +2420,9 @@ struct _virDomainObj { void (*privateDataFreeFunc)(void *); int taint; + + unsigned long long original_memlock; /* Original RLIMIT_MEMLOCK, zero if no + * restore will be required later */ }; typedef bool (*virDomainObjListACLFilter)(virConnectPtr conn, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4b796ab..1e1e57f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4125,7 +4125,10 @@ qemuDomainRequiresMlock(virDomainDefPtr def) * Adjust the memory locking limit for the QEMU process associated to @vm, in * order to comply with VFIO or architecture requirements. * - * The limit will not be changed unless doing so is needed. + * The limit will not be changed unless doing so is needed; the first time + * the limit is changed, the original (default) limit is stored in @vm and + * that value will be restored if qemuDomainAdjustMaxMemLock() is called once + * memory locking is no longer required. * * Returns: 0 on success, <0 on failure */ @@ -4135,8 +4138,22 @@ qemuDomainAdjustMaxMemLock(virDomainObjPtr vm) unsigned long long bytes = 0; int ret = -1; - if (qemuDomainRequiresMlock(vm->def)) + if (qemuDomainRequiresMlock(vm->def)) { + /* 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 + * failure, it just means we'll be unable to lower it later */ + if (!vm->original_memlock) { + if (virProcessGetMaxMemLock(vm->pid, &(vm->original_memlock)) < 0) + vm->original_memlock = 0; + } bytes = qemuDomainGetMlockLimitBytes(vm->def); + } else { + /* Once memory locking is no longer required, we can restore the + * original, usually very low, limit */ + bytes = vm->original_memlock; + vm->original_memlock = 0; + } /* Trying to set the memory locking limit to zero is a no-op */ if (virProcessSetMaxMemLock(vm->pid, bytes) < 0) -- 2.5.0
MemLock is already used in other modules and, while still an abbreviation, is not ambiguous. --- src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_domain.c | 10 +++++----- src/qemu/qemu_domain.h | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d2f37e4..692f088 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -11262,8 +11262,8 @@ qemuBuildCommandLine(virConnectPtr conn, /* 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 (qemuDomainRequiresMlock(def)) - virCommandSetMaxMemLock(cmd, qemuDomainGetMlockLimitBytes(def)); + if (qemuDomainRequiresMemLock(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 1e1e57f..bb8d47f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3964,7 +3964,7 @@ qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver, /** - * qemuDomainGetMlockLimitBytes: + * qemuDomainGetMemLockLimitBytes: * * @def: domain definition * @@ -3974,7 +3974,7 @@ qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver, * value returned may depend upon the architecture or devices present. */ unsigned long long -qemuDomainGetMlockLimitBytes(virDomainDefPtr def) +qemuDomainGetMemLockLimitBytes(virDomainDefPtr def) { unsigned long long memKB; @@ -4095,7 +4095,7 @@ qemuDomainGetMlockLimitBytes(virDomainDefPtr def) * requirements. * */ bool -qemuDomainRequiresMlock(virDomainDefPtr def) +qemuDomainRequiresMemLock(virDomainDefPtr def) { size_t i; @@ -4138,7 +4138,7 @@ qemuDomainAdjustMaxMemLock(virDomainObjPtr vm) unsigned long long bytes = 0; int ret = -1; - if (qemuDomainRequiresMlock(vm->def)) { + if (qemuDomainRequiresMemLock(vm->def)) { /* 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 @@ -4147,7 +4147,7 @@ qemuDomainAdjustMaxMemLock(virDomainObjPtr vm) if (virProcessGetMaxMemLock(vm->pid, &(vm->original_memlock)) < 0) vm->original_memlock = 0; } - bytes = qemuDomainGetMlockLimitBytes(vm->def); + 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 704351b..cff48d7 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -498,8 +498,8 @@ bool qemuDomainMachineHasBuiltinIDE(const virDomainDef *def); int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver, virDomainObjPtr vm); -unsigned long long qemuDomainGetMlockLimitBytes(virDomainDefPtr def); -bool qemuDomainRequiresMlock(virDomainDefPtr def); +unsigned long long qemuDomainGetMemLockLimitBytes(virDomainDefPtr def); +bool qemuDomainRequiresMemLock(virDomainDefPtr def); int qemuDomainAdjustMaxMemLock(virDomainObjPtr vm); int qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, -- 2.5.0
On 12/11/2015 09:45 AM, Andrea Bolognani wrote:
During my testing, I've realized that even with the series applied there's still one case in which we're unable to restore the previous memory locking limit after removing the last PCI hostdev from the guest.
If an x86 guest contains a PCI hostdev in its XML definition, then the memory locking limit will be set correctly, but virCommandGetMaxMemLock() will be used instead of virProcessGetMaxMemLock(), and the limit will be set right before calling exec() to spawn the qemu binary.
In this context, we have no access to the virDomainObj or even virDomainDef instances, so I see no way of storing the original limit for later retrieval.
The series is still an improvement as it covers all other cases. Still, I thought this was worth mentioning.
Changes since v1[1]:
* reorder commits according to John's suggestions * don't fail if we can't retrieve the current memory locking limit * small changes here and there as pointed out during review
Cheers.
[1] https://www.redhat.com/archives/libvir-list/2015-November/msg01021.html
Andrea Bolognani (7): process: Allow virProcessPrLimit() to get current limit process: Add virProcessGetMaxMemLock() qemu: Add qemuDomainAdjustMaxMemLock() qemu: Use qemuDomainAdjustMaxMemLock() qemu: Reduce memlock limit after detaching PCI hostdev qemu: Allow qemuDomainAdjustMaxMemLock() to restore previous value qemu: Replace Mlock with MemLock in function names
configure.ac | 2 +- src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_domain.c | 53 ++++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_domain.h | 5 ++-- src/qemu/qemu_hotplug.c | 45 ++++++++++++++--------------------- src/util/virprocess.c | 61 +++++++++++++++++++++++++++++++++++++++++++----- src/util/virprocess.h | 2 ++ 9 files changed, 135 insertions(+), 41 deletions(-)
ACK series, John
On Wed, 2015-12-16 at 14:32 -0500, John Ferlan wrote:
On 12/11/2015 09:45 AM, Andrea Bolognani wrote:
During my testing, I've realized that even with the series applied there's still one case in which we're unable to restore the previous memory locking limit after removing the last PCI hostdev from the guest.
If an x86 guest contains a PCI hostdev in its XML definition, then the memory locking limit will be set correctly, but virCommandGetMaxMemLock() will be used instead of virProcessGetMaxMemLock(), and the limit will be set right before calling exec() to spawn the qemu binary.
In this context, we have no access to the virDomainObj or even virDomainDef instances, so I see no way of storing the original limit for later retrieval.
The series is still an improvement as it covers all other cases. Still, I thought this was worth mentioning.
Changes since v1[1]:
* reorder commits according to John's suggestions * don't fail if we can't retrieve the current memory locking limit * small changes here and there as pointed out during review
Cheers.
[1] https://www.redhat.com/archives/libvir-list/2015-November/msg01021.html
Andrea Bolognani (7): process: Allow virProcessPrLimit() to get current limit process: Add virProcessGetMaxMemLock() qemu: Add qemuDomainAdjustMaxMemLock() qemu: Use qemuDomainAdjustMaxMemLock() qemu: Reduce memlock limit after detaching PCI hostdev qemu: Allow qemuDomainAdjustMaxMemLock() to restore previous value qemu: Replace Mlock with MemLock in function names
configure.ac | 2 +- src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_domain.c | 53 ++++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_domain.h | 5 ++-- src/qemu/qemu_hotplug.c | 45 ++++++++++++++--------------------- src/util/virprocess.c | 61 +++++++++++++++++++++++++++++++++++++++++++----- src/util/virprocess.h | 2 ++ 9 files changed, 135 insertions(+), 41 deletions(-)
ACK series,
Pushed, thanks. -- Andrea Bolognani Software Engineer - Virtualization Team
participants (2)
-
Andrea Bolognani -
John Ferlan