6:50 a.m.
Peter Krempa (3):
qemu: Extract logic to determine the mlock limit size for VFIO
qemu: hotplug: Fix mlock limit handling on memory hotplug
qemu: hotplug: Reject VFIO hotplug if setting RLIMIT_MEMLOCK fails
src/qemu/qemu_command.c | 18 ++---------------
src/qemu/qemu_domain.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 3 +++
src/qemu/qemu_hotplug.c | 50 +++++++++++++++++++++++++++++----------------
4 files changed, 92 insertions(+), 33 deletions(-)
--
2.6.2
6:50 a.m.
New subject: [libvirt] [PATCH v2 1/3] qemu: Extract logic to determine the mlock limit size for VFIO
New function qemuDomainGetMlockLimitBytes will now handle the
calculation so that it unifies the logic to one place and allows later
reuse.
---
src/qemu/qemu_command.c | 18 ++----------------
src/qemu/qemu_domain.c | 27 +++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 2 ++
src/qemu/qemu_hotplug.c | 17 ++---------------
4 files changed, 33 insertions(+), 31 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8824541..9acf8e4 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -11447,22 +11447,8 @@ qemuBuildCommandLine(virConnectPtr conn,
goto error;
}
- if (mlock) {
- unsigned long long memKB;
-
- /* VFIO requires all of the guest's memory to be
- * locked resident, plus some amount for IO
- * space. Alex Williamson suggested adding 1GiB for IO
- * space just to be safe (some finer tuning might be
- * nice, though).
- */
- if (virMemoryLimitIsSet(def->mem.hard_limit))
- memKB = def->mem.hard_limit;
- else
- memKB = virDomainDefGetMemoryActual(def) + 1024 * 1024;
-
- virCommandSetMaxMemLock(cmd, memKB * 1024);
- }
+ if (mlock)
+ virCommandSetMaxMemLock(cmd, qemuDomainGetMlockLimitBytes(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 890d8ed..a06624e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3616,3 +3616,30 @@ qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
return 0;
}
+
+
+/**
+ * qemuDomainGetMlockLimitBytes:
+ *
+ * @def: domain definition
+ *
+ * Returns the size of the memory in bytes that needs to be set as
+ * RLIMIT_MEMLOCK for purpose of VFIO device passthrough. In cases where
+ * mem.hard_limit is set, the value is preferred,otherwise the value may depend
+ * on the device or architecture.
+ */
+unsigned long long
+qemuDomainGetMlockLimitBytes(virDomainDefPtr def)
+{
+ unsigned long long memKB;
+
+ /* VFIO requires all of the guest's memory to be locked resident, plus some
+ * amount for IO space. Alex Williamson suggested adding 1GiB for IO space
+ * just to be safe (some finer tuning might be nice, though). */
+ if (virMemoryLimitIsSet(def->mem.hard_limit))
+ memKB = def->mem.hard_limit;
+ else
+ memKB = virDomainDefGetMemoryActual(def) + 1024 * 1024;
+
+ return memKB << 10;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 64cd7e1..e34370b 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -482,4 +482,6 @@ bool qemuDomainMachineIsS390CCW(const virDomainDef *def);
int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
virDomainObjPtr vm);
+unsigned long long qemuDomainGetMlockLimitBytes(virDomainDefPtr def);
+
#endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8f2fda9..e7fc036 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1254,7 +1254,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
bool teardowncgroup = false;
bool teardownlabel = false;
int backend;
- unsigned long long memKB;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
unsigned int flags = 0;
@@ -1279,20 +1278,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
goto error;
}
- /* VFIO requires all of the guest's memory to be locked
- * resident (plus an additional 1GiB to cover IO space). During
- * hotplug, the guest's memory may already be locked, but it
- * doesn't hurt to "change" the limit to the same value.
- * NB: the domain's memory tuning parameters are stored as
- * Kibibytes, but virProcessSetMaxMemLock expects the value in
- * bytes.
- */
- if (virMemoryLimitIsSet(vm->def->mem.hard_limit))
- memKB = vm->def->mem.hard_limit;
- else
- memKB = virDomainDefGetMemoryActual(vm->def) + (1024 * 1024);
-
- virProcessSetMaxMemLock(vm->pid, memKB * 1024);
+ /* setup memory locking limits, that are necessary for VFIO */
+ virProcessSetMaxMemLock(vm->pid, qemuDomainGetMlockLimitBytes(vm->def));
break;
default:
--
2.6.2
12:09 p.m.
New subject: [libvirt] [PATCH v2 1/3] qemu: Extract logic to determine the mlock limit size for VFIO
On 11/09/2015 07:50 AM, Peter Krempa wrote:
New function qemuDomainGetMlockLimitBytes will now handle the
calculation so that it unifies the logic to one place and allows later
reuse.
---
src/qemu/qemu_command.c | 18 ++----------------
src/qemu/qemu_domain.c | 27 +++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 2 ++
src/qemu/qemu_hotplug.c | 17 ++---------------
4 files changed, 33 insertions(+), 31 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8824541..9acf8e4 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -11447,22 +11447,8 @@ qemuBuildCommandLine(virConnectPtr conn,
goto error;
}
- if (mlock) {
- unsigned long long memKB;
-
- /* VFIO requires all of the guest's memory to be
- * locked resident, plus some amount for IO
- * space. Alex Williamson suggested adding 1GiB for IO
- * space just to be safe (some finer tuning might be
- * nice, though).
- */
- if (virMemoryLimitIsSet(def->mem.hard_limit))
- memKB = def->mem.hard_limit;
- else
- memKB = virDomainDefGetMemoryActual(def) + 1024 * 1024;
-
- virCommandSetMaxMemLock(cmd, memKB * 1024);
- }
+ if (mlock)
+ virCommandSetMaxMemLock(cmd, qemuDomainGetMlockLimitBytes(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 890d8ed..a06624e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3616,3 +3616,30 @@ qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
return 0;
}
+
+
+/**
+ * qemuDomainGetMlockLimitBytes:
+ *
+ * @def: domain definition
+ *
+ * Returns the size of the memory in bytes that needs to be set as
+ * RLIMIT_MEMLOCK for purpose of VFIO device passthrough. In cases where
+ * mem.hard_limit is set, the value is preferred,otherwise the value may depend
+ * on the device or architecture.
How about?
Returns the size of memory in bytes to allow a process (domain) to be
locked into RAM (e.g setrlimit RLIMIT_MEMLOCK). If a mem.hard_limit is
set, then that value is preferred; otherwise, the value returned may
depend upon the architecture or devices present, such as a VFIO
passthrough device.
ACK -
John
+ */
+unsigned long long
+qemuDomainGetMlockLimitBytes(virDomainDefPtr def)
+{
+ unsigned long long memKB;
+
+ /* VFIO requires all of the guest's memory to be locked resident, plus some
+ * amount for IO space. Alex Williamson suggested adding 1GiB for IO space
+ * just to be safe (some finer tuning might be nice, though). */
+ if (virMemoryLimitIsSet(def->mem.hard_limit))
+ memKB = def->mem.hard_limit;
+ else
+ memKB = virDomainDefGetMemoryActual(def) + 1024 * 1024;
+
+ return memKB << 10;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 64cd7e1..e34370b 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -482,4 +482,6 @@ bool qemuDomainMachineIsS390CCW(const virDomainDef *def);
int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
virDomainObjPtr vm);
+unsigned long long qemuDomainGetMlockLimitBytes(virDomainDefPtr def);
+
#endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8f2fda9..e7fc036 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1254,7 +1254,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
bool teardowncgroup = false;
bool teardownlabel = false;
int backend;
- unsigned long long memKB;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
unsigned int flags = 0;
@@ -1279,20 +1278,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
goto error;
}
- /* VFIO requires all of the guest's memory to be locked
- * resident (plus an additional 1GiB to cover IO space). During
- * hotplug, the guest's memory may already be locked, but it
- * doesn't hurt to "change" the limit to the same value.
- * NB: the domain's memory tuning parameters are stored as
- * Kibibytes, but virProcessSetMaxMemLock expects the value in
- * bytes.
- */
- if (virMemoryLimitIsSet(vm->def->mem.hard_limit))
- memKB = vm->def->mem.hard_limit;
- else
- memKB = virDomainDefGetMemoryActual(vm->def) + (1024 * 1024);
-
- virProcessSetMaxMemLock(vm->pid, memKB * 1024);
+ /* setup memory locking limits, that are necessary for VFIO */
+ virProcessSetMaxMemLock(vm->pid, qemuDomainGetMlockLimitBytes(vm->def));
break;
default:
1:09 a.m.
New subject: [libvirt] [PATCH v2 1/3] qemu: Extract logic to determine the mlock limit size for VFIO
On Mon, Nov 09, 2015 at 13:09:59 -0500, John Ferlan wrote:
On 11/09/2015 07:50 AM, Peter Krempa wrote:
> New function qemuDomainGetMlockLimitBytes will now handle the
> calculation so that it unifies the logic to one place and allows later
> reuse.
> ---
[...]
> +/**
> + * qemuDomainGetMlockLimitBytes:
> + *
> + * @def: domain definition
> + *
> + * Returns the size of the memory in bytes that needs to be set as
> + * RLIMIT_MEMLOCK for purpose of VFIO device passthrough. In cases where
> + * mem.hard_limit is set, the value is preferred,otherwise the value may depend
> + * on the device or architecture.
How about?
Returns the size of memory in bytes to allow a process (domain) to be
That satement would be misleading. The size returned by this function
may not be enough to lock the complete process into RAM. Locking the
complete process would also include disk buffers, the binary itself and
other overhead of the process itself. This function clearly isn't
designed to do that.
Said that, it is actually used that way, since the botched revert of
qemuDomainMemoryLimit. Although I would like to remove that I'm afraid I
can't really do that, since it would break VMs that use locked memory
backing. It's wrong to use it that way though and I want to discourage
it, but I really don't want to break existing configs.
At this point I'd like to limit this to pure code extraction for reuse
without sliping into anything leading to further misuse of this
function. By the way, that was the original reason for not adding a
comment at all.
locked into RAM (e.g setrlimit RLIMIT_MEMLOCK). If a mem.hard_limit
is
Since this is a qemu specific function setrlimit(RLIMIT_MEMLOCK, ...) is
actually the only impl.
set, then that value is preferred; otherwise, the value returned may
depend upon the architecture or devices present, such as a VFIO
passthrough device.
Peter
2:12 a.m.
New subject: [libvirt] [PATCH v2 1/3] qemu: Extract logic to determine the mlock limit size for VFIO
On Mon, Nov 09, 2015 at 01:09:59PM -0500, John Ferlan wrote:
On 11/09/2015 07:50 AM, Peter Krempa wrote:
> New function qemuDomainGetMlockLimitBytes will now handle the
> calculation so that it unifies the logic to one place and allows later
> reuse.
> ---
> src/qemu/qemu_command.c | 18 ++----------------
> src/qemu/qemu_domain.c | 27 +++++++++++++++++++++++++++
> src/qemu/qemu_domain.h | 2 ++
> src/qemu/qemu_hotplug.c | 17 ++---------------
> 4 files changed, 33 insertions(+), 31 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 8824541..9acf8e4 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -11447,22 +11447,8 @@ qemuBuildCommandLine(virConnectPtr conn,
> goto error;
> }
>
> - if (mlock) {
> - unsigned long long memKB;
> -
> - /* VFIO requires all of the guest's memory to be
> - * locked resident, plus some amount for IO
> - * space. Alex Williamson suggested adding 1GiB for IO
> - * space just to be safe (some finer tuning might be
> - * nice, though).
> - */
> - if (virMemoryLimitIsSet(def->mem.hard_limit))
> - memKB = def->mem.hard_limit;
> - else
> - memKB = virDomainDefGetMemoryActual(def) + 1024 * 1024;
> -
> - virCommandSetMaxMemLock(cmd, memKB * 1024);
> - }
> + if (mlock)
> + virCommandSetMaxMemLock(cmd, qemuDomainGetMlockLimitBytes(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 890d8ed..a06624e 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3616,3 +3616,30 @@ qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
>
> return 0;
> }
> +
> +
> +/**
> + * qemuDomainGetMlockLimitBytes:
> + *
> + * @def: domain definition
> + *
> + * Returns the size of the memory in bytes that needs to be set as
> + * RLIMIT_MEMLOCK for purpose of VFIO device passthrough. In cases where
> + * mem.hard_limit is set, the value is preferred,otherwise the value may depend
> + * on the device or architecture.
How about?
Returns the size of memory in bytes to allow a process (domain) to be
locked into RAM (e.g setrlimit RLIMIT_MEMLOCK).
While this sentence is inaccurate,
If a mem.hard_limit is
set, then that value is preferred; otherwise, the value returned may
depend upon the architecture or devices present, such as a VFIO
passthrough device.
this one looks better than the original and it has proper spacing after
the comma.
ACK with the second sentence used.
Jan
6:50 a.m.
New subject: [libvirt] [PATCH v2 2/3] qemu: hotplug: Fix mlock limit handling on memory hotplug
If mlock is required either due to use of VFIO hostdevs or due to the
fact that it's enabled it needs to be tweaked prior to adding new memory
or after removing a module. Add a helper to determine when it's
necessary and reuse it both on hotplug and hotunplug.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1273491
---
src/qemu/qemu_domain.c | 27 +++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_hotplug.c | 32 +++++++++++++++++++++++++++++---
3 files changed, 57 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a06624e..6f95f76 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3643,3 +3643,30 @@ qemuDomainGetMlockLimitBytes(virDomainDefPtr def)
return memKB << 10;
}
+
+
+/**
+ * @def: domain definition
+ *
+ * Returns ture if the locked memory limit needs to be set or updated due to
+ * configuration or passthrough devices.
+ * */
+bool
+qemuDomainRequiresMlock(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;
+ }
+
+ return false;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index e34370b..4be998c 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -483,5 +483,6 @@ int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
virDomainObjPtr vm);
unsigned long long qemuDomainGetMlockLimitBytes(virDomainDefPtr def);
+bool qemuDomainRequiresMlock(virDomainDefPtr def);
#endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e7fc036..1e7a653 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1768,6 +1768,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
virJSONValuePtr props = NULL;
virObjectEventPtr event;
bool fix_balloon = false;
+ bool mlock = false;
int id;
int ret = -1;
@@ -1802,16 +1803,25 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
goto cleanup;
}
+ mlock = qemuDomainRequiresMlock(vm->def);
+
+ if (mlock &&
+ virProcessSetMaxMemLock(vm->pid,
+ qemuDomainGetMlockLimitBytes(vm->def)) < 0) {
+ mlock = false;
+ goto removedef;
+ }
+
qemuDomainObjEnterMonitor(driver, vm);
if (qemuMonitorAddObject(priv->mon, backendType, objalias, props) < 0)
- goto removedef;
+ goto exit_monitor;
if (qemuMonitorAddDevice(priv->mon, devstr) < 0) {
virErrorPtr err = virSaveLastError();
ignore_value(qemuMonitorDelObject(priv->mon, objalias));
virSetError(err);
virFreeError(err);
- goto removedef;
+ goto exit_monitor;
}
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
@@ -1845,17 +1855,27 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
virDomainMemoryDefFree(mem);
return ret;
- removedef:
+ exit_monitor:
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
mem = NULL;
goto audit;
}
+ removedef:
if ((id = virDomainMemoryFindByDef(vm->def, mem)) >= 0)
mem = virDomainMemoryRemove(vm->def, id);
else
mem = NULL;
+ /* reset the mlock limit */
+ if (mlock) {
+ virErrorPtr err = virSaveLastError();
+ ignore_value(virProcessSetMaxMemLock(vm->pid,
+ qemuDomainGetMlockLimitBytes(vm->def)));
+ virSetError(err);
+ virFreeError(err);
+ }
+
goto audit;
}
@@ -2947,6 +2967,12 @@ qemuDomainRemoveMemoryDevice(virQEMUDriverPtr driver,
virDomainMemoryRemove(vm->def, idx);
virDomainMemoryDefFree(mem);
+
+ /* decrease the mlock limit after memory unplug if necessary */
+ if (qemuDomainRequiresMlock(vm->def))
+ ignore_value(virProcessSetMaxMemLock(vm->pid,
+ qemuDomainGetMlockLimitBytes(vm->def)));
+
return 0;
}
--
2.6.2
12:11 p.m.
New subject: [libvirt] [PATCH v2 2/3] qemu: hotplug: Fix mlock limit handling on memory hotplug
On 11/09/2015 07:50 AM, Peter Krempa wrote:
If mlock is required either due to use of VFIO hostdevs or due to
the
fact that it's enabled it needs to be tweaked prior to adding new memory
or after removing a module. Add a helper to determine when it's
necessary and reuse it both on hotplug and hotunplug.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1273491
---
src/qemu/qemu_domain.c | 27 +++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_hotplug.c | 32 +++++++++++++++++++++++++++++---
3 files changed, 57 insertions(+), 3 deletions(-)
FWIW: My comments in v1 when someone sets hard_limit, doesn't have VFIO,
starts with some memory device objects and expectations. I had a slight
memory lapse as to what qemuDomainGetMlockLimitBytes returns. :-)
Now that I have recovered from that page fault, when a hard_limit is
set, regardless of what memory is added the value used in the call to
virProcessSetMaxMemLock won't change...
Still we could/should document in formatdomain that memory devices
present or added would need be accounted for when considering what
hard_limit is set.
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a06624e..6f95f76 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3643,3 +3643,30 @@ qemuDomainGetMlockLimitBytes(virDomainDefPtr def)
return memKB << 10;
}
+
+
+/**
+ * @def: domain definition
+ *
+ * Returns ture if the locked memory limit needs to be set or updated due to
+ * configuration or passthrough devices.
+ * */
+bool
+qemuDomainRequiresMlock(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;
+ }
+
+ return false;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index e34370b..4be998c 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -483,5 +483,6 @@ int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
virDomainObjPtr vm);
unsigned long long qemuDomainGetMlockLimitBytes(virDomainDefPtr def);
+bool qemuDomainRequiresMlock(virDomainDefPtr def);
#endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e7fc036..1e7a653 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1768,6 +1768,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
virJSONValuePtr props = NULL;
virObjectEventPtr event;
bool fix_balloon = false;
+ bool mlock = false;
int id;
int ret = -1;
@@ -1802,16 +1803,25 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
goto cleanup;
}
+ mlock = qemuDomainRequiresMlock(vm->def);
+
+ if (mlock &&
+ virProcessSetMaxMemLock(vm->pid,
+ qemuDomainGetMlockLimitBytes(vm->def)) < 0) {
+ mlock = false;
Coverity found a need for:
virJSONValueFree(props);
ACK - with the adjustment
John
BTW: If you think it matters - if we got the LimitBytes before inserting
vm->mem to the domain def, then we could know whether it changed and
whether we needed to Set it here and the error path. One would like to
believe an initial setting to hard_limit (such as 4G) wouldn't have a
failure if someone passed 4G again...
+ goto removedef;
+ }
+
qemuDomainObjEnterMonitor(driver, vm);
if (qemuMonitorAddObject(priv->mon, backendType, objalias, props) < 0)
- goto removedef;
+ goto exit_monitor;
if (qemuMonitorAddDevice(priv->mon, devstr) < 0) {
virErrorPtr err = virSaveLastError();
ignore_value(qemuMonitorDelObject(priv->mon, objalias));
virSetError(err);
virFreeError(err);
- goto removedef;
+ goto exit_monitor;
}
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
@@ -1845,17 +1855,27 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
virDomainMemoryDefFree(mem);
return ret;
- removedef:
+ exit_monitor:
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
mem = NULL;
goto audit;
}
+ removedef:
if ((id = virDomainMemoryFindByDef(vm->def, mem)) >= 0)
mem = virDomainMemoryRemove(vm->def, id);
else
mem = NULL;
+ /* reset the mlock limit */
+ if (mlock) {
+ virErrorPtr err = virSaveLastError();
+ ignore_value(virProcessSetMaxMemLock(vm->pid,
+
qemuDomainGetMlockLimitBytes(vm->def)));
+ virSetError(err);
+ virFreeError(err);
+ }
+
goto audit;
}
@@ -2947,6 +2967,12 @@ qemuDomainRemoveMemoryDevice(virQEMUDriverPtr driver,
virDomainMemoryRemove(vm->def, idx);
virDomainMemoryDefFree(mem);
+
+ /* decrease the mlock limit after memory unplug if necessary */
+ if (qemuDomainRequiresMlock(vm->def))
+ ignore_value(virProcessSetMaxMemLock(vm->pid,
+
qemuDomainGetMlockLimitBytes(vm->def)));
+
return 0;
}
1:16 a.m.
New subject: [libvirt] [PATCH v2 2/3] qemu: hotplug: Fix mlock limit handling on memory hotplug
On Mon, Nov 09, 2015 at 13:11:16 -0500, John Ferlan wrote:
On 11/09/2015 07:50 AM, Peter Krempa wrote:
> If mlock is required either due to use of VFIO hostdevs or due to the
> fact that it's enabled it needs to be tweaked prior to adding new memory
> or after removing a module. Add a helper to determine when it's
> necessary and reuse it both on hotplug and hotunplug.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1273491
> ---
> src/qemu/qemu_domain.c | 27 +++++++++++++++++++++++++++
> src/qemu/qemu_domain.h | 1 +
> src/qemu/qemu_hotplug.c | 32 +++++++++++++++++++++++++++++---
> 3 files changed, 57 insertions(+), 3 deletions(-)
>
[...]
> @@ -1802,16 +1803,25 @@ qemuDomainAttachMemory(virQEMUDriverPtr
driver,
> goto cleanup;
> }
>
> + mlock = qemuDomainRequiresMlock(vm->def);
> +
> + if (mlock &&
> + virProcessSetMaxMemLock(vm->pid,
> + qemuDomainGetMlockLimitBytes(vm->def)) < 0)
{
> + mlock = false;
Coverity found a need for:
virJSONValueFree(props);
ACK - with the adjustment
John
BTW: If you think it matters - if we got the LimitBytes before inserting
vm->mem to the domain def, then we could know whether it changed anda
In that case I'd rather make qemuDomainRequiresMlock less generic (it'd
include "memory hotplug" in some form in the name) which would return
false in an additional case when 'hard limit' was specified. I was
actually thinking of doing it this way but I've opted for the more
generic approach.
whether we needed to Set it here and the error path. One would like
to
believe an initial setting to hard_limit (such as 4G) wouldn't have a
failure if someone passed 4G again...
This actually should not happen (TM) while libvirtd is privileged.
Peter
6:50 a.m.
New subject: [libvirt] [PATCH v2 3/3] qemu: hotplug: Reject VFIO hotplug if setting RLIMIT_MEMLOCK fails
Check the return value of virCommandSetMaxMemLock when hotplugging VFIO
PCI hostdevs and reject the hotplug if the memory limit can't be set.
---
src/qemu/qemu_hotplug.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1e7a653..1a0d1e9 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1279,7 +1279,10 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
}
/* setup memory locking limits, that are necessary for VFIO */
- virProcessSetMaxMemLock(vm->pid, qemuDomainGetMlockLimitBytes(vm->def));
+ if (virProcessSetMaxMemLock(vm->pid,
+ qemuDomainGetMlockLimitBytes(vm->def)) < 0)
+ goto error;
+
break;
default:
--
2.6.2
12:12 p.m.
New subject: [libvirt] [PATCH v2 3/3] qemu: hotplug: Reject VFIO hotplug if setting RLIMIT_MEMLOCK fails
On 11/09/2015 07:50 AM, Peter Krempa wrote:
Check the return value of virCommandSetMaxMemLock when hotplugging
VFIO
PCI hostdevs and reject the hotplug if the memory limit can't be set.
---
src/qemu/qemu_hotplug.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
ACK
John
2:48 a.m.
On Mon, Nov 09, 2015 at 13:50:14 +0100, Peter Krempa wrote:
Peter Krempa (3):
qemu: Extract logic to determine the mlock limit size for VFIO
qemu: hotplug: Fix mlock limit handling on memory hotplug
qemu: hotplug: Reject VFIO hotplug if setting RLIMIT_MEMLOCK fails
src/qemu/qemu_command.c | 18 ++---------------
src/qemu/qemu_domain.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 3 +++
src/qemu/qemu_hotplug.c | 50 +++++++++++++++++++++++++++++----------------
4 files changed, 92 insertions(+), 33 deletions(-)
This series is pushed now after suggested tweaks.
Thanks.
Peter
3336
days inactive
3337
days old
10 comments
3 participants
participants (3)
-
John Ferlan -
Ján Tomko -
Peter Krempa