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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list