[libvirt] [PATCH] qemu: Increase RLIMIT_MEMLOCK when memoryBacking/locked is used

If a domain is configured to have all its memory locked, we need to increase RLIMIT_MEMLOCK so that QEMU is actually allowed to lock the memory. --- src/qemu/qemu_command.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c268a3a..8e2de09 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6440,6 +6440,7 @@ qemuBuildCommandLine(virConnectPtr conn, int spice = 0; int usbcontroller = 0; bool usblegacy = false; + bool mlock; int contOrder[] = { /* We don't add an explicit IDE or FD controller because the * provided PIIX4 device already includes one. It isn't possible to @@ -6551,6 +6552,7 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgFormat(cmd, "mlock=%s", def->mem.locked ? "on" : "off"); } + mlock = def->mem.locked; virCommandAddArg(cmd, "-smp"); if (!(smp = qemuBuildSmpArgStr(def, qemuCaps))) @@ -8191,22 +8193,13 @@ qemuBuildCommandLine(virConnectPtr conn, if (hostdev->source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { - unsigned long long memKB; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("VFIO PCI device assignment is not " "supported by this version of qemu")); goto error; } - /* 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). - */ - memKB = def->mem.max_balloon + (1024 * 1024); - virCommandSetMaxMemLock(cmd, memKB * 1024); + mlock = true; } if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { @@ -8425,6 +8418,24 @@ 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 memory hard_limit is configured, we can use it directly as + * there is no sense to set MEMLOCK limit beyond it. Also we can + * safely use it instead of any magically computed value. + */ + if (def->mem.hard_limit) + memKB = def->mem.hard_limit; + else + memKB = def->mem.max_balloon + (1024 * 1024); + virCommandSetMaxMemLock(cmd, memKB * 1024); + } + virObjectUnref(cfg); return cmd; -- 1.8.2.1

On 05/17/2013 07:03 AM, Jiri Denemark wrote:
If a domain is configured to have all its memory locked, we need to increase RLIMIT_MEMLOCK so that QEMU is actually allowed to lock the memory. --- src/qemu/qemu_command.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 05/17/2013 09:03 AM, Jiri Denemark wrote:
If a domain is configured to have all its memory locked, we need to increase RLIMIT_MEMLOCK so that QEMU is actually allowed to lock the memory. --- src/qemu/qemu_command.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c268a3a..8e2de09 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6440,6 +6440,7 @@ qemuBuildCommandLine(virConnectPtr conn, int spice = 0; int usbcontroller = 0; bool usblegacy = false; + bool mlock; int contOrder[] = { /* We don't add an explicit IDE or FD controller because the * provided PIIX4 device already includes one. It isn't possible to @@ -6551,6 +6552,7 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgFormat(cmd, "mlock=%s", def->mem.locked ? "on" : "off"); } + mlock = def->mem.locked;
virCommandAddArg(cmd, "-smp"); if (!(smp = qemuBuildSmpArgStr(def, qemuCaps))) @@ -8191,22 +8193,13 @@ qemuBuildCommandLine(virConnectPtr conn,
if (hostdev->source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { - unsigned long long memKB; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("VFIO PCI device assignment is not " "supported by this version of qemu")); goto error; } - /* 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). - */
I think it would be good to retain a short comment here, maybe just "VFIO requires all guest memory and IO space to be locked in RAM".
- memKB = def->mem.max_balloon + (1024 * 1024); - virCommandSetMaxMemLock(cmd, memKB * 1024); + mlock = true; }
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { @@ -8425,6 +8418,24 @@ 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 memory hard_limit is configured, we can use it directly as + * there is no sense to set MEMLOCK limit beyond it. Also we can + * safely use it instead of any magically computed value.
Does the setting of hard_limit take IO space into account? And what about the memory of the qemu process itself? Note that there is another place where virProcessSetMaxMemLock() is called (qemuDomainAttachHostPciDevice()). To be consistent, we should use the same logic there as here. Unfortunately, one calls virCommandSetMaxMemLock() (to set it in the future after the as-yet-uncreated process is running), and the other sets it immediately in an already running process, so if it's turned into a utility function, it has to be a function that takes a virDomainDef and returns the proper amount to lock. Not sure what that would be called...
+ */ + if (def->mem.hard_limit) + memKB = def->mem.hard_limit; + else + memKB = def->mem.max_balloon + (1024 * 1024); + virCommandSetMaxMemLock(cmd, memKB * 1024); + } + virObjectUnref(cfg); return cmd;

On Fri, May 17, 2013 at 10:26:17 -0400, Laine Stump wrote:
On 05/17/2013 09:03 AM, Jiri Denemark wrote:
If a domain is configured to have all its memory locked, we need to increase RLIMIT_MEMLOCK so that QEMU is actually allowed to lock the memory. --- src/qemu/qemu_command.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c268a3a..8e2de09 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6440,6 +6440,7 @@ qemuBuildCommandLine(virConnectPtr conn, int spice = 0; int usbcontroller = 0; bool usblegacy = false; + bool mlock; int contOrder[] = { /* We don't add an explicit IDE or FD controller because the * provided PIIX4 device already includes one. It isn't possible to @@ -6551,6 +6552,7 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgFormat(cmd, "mlock=%s", def->mem.locked ? "on" : "off"); } + mlock = def->mem.locked;
virCommandAddArg(cmd, "-smp"); if (!(smp = qemuBuildSmpArgStr(def, qemuCaps))) @@ -8191,22 +8193,13 @@ qemuBuildCommandLine(virConnectPtr conn,
if (hostdev->source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { - unsigned long long memKB; - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("VFIO PCI device assignment is not " "supported by this version of qemu")); goto error; } - /* 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). - */
I think it would be good to retain a short comment here, maybe just "VFIO requires all guest memory and IO space to be locked in RAM".
- memKB = def->mem.max_balloon + (1024 * 1024); - virCommandSetMaxMemLock(cmd, memKB * 1024); + mlock = true; }
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { @@ -8425,6 +8418,24 @@ 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 memory hard_limit is configured, we can use it directly as + * there is no sense to set MEMLOCK limit beyond it. Also we can + * safely use it instead of any magically computed value.
Does the setting of hard_limit take IO space into account? And what about the memory of the qemu process itself?
That's not our business as hard_limit is explicitly configured by a user/app and tells libvirt QEMU should never be allowed to eat more host memory. Thus it's up to them if it's computed correctly.
Note that there is another place where virProcessSetMaxMemLock() is called (qemuDomainAttachHostPciDevice()). To be consistent, we should use the same logic there as here.
Good point. And there's another magic computation of memory limit in qemuSetupCgroup, where we try to compute the right hard_limit if none was provided. I hate these magic computations, which is why I decided to be lazy and ignore that for now :-) Although it seems, we should really unify all these places and do just computation and use it consistently in all the places. Unfortunately, it seems the automatic hard_limit computations does not take VFIO into account (which means it may not work correctly if VFIO is used) so we can't just reuse that code without modifying it. Jirka
participants (3)
-
Eric Blake
-
Jiri Denemark
-
Laine Stump