Re: [libvirt] [PATCH] qemu: Increase RLIMIT_MEMLOCK when memoryBacking/locked is used
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