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

[Jiri Denemark]

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