On 06/28/2013 11:04 AM, Jiri Denemark wrote: I know you're just doing a cut-paste here, but I'm curious how we arrived at this formula. On systems using vfio, it ends up locking *way* more memory than previously. For my test guest with 4GB of ram assigned, the old computation would lock 5GB of ram for the qemu process. With the new method in this patch it ends up locking just about 7GB. I don't know if that's a case of the original simple VFIO amount being inadequate, or the new method goind overboard; just thought there should be at least minimal discussion before it went in (note that I tried it on a guest with VFIO passthrough and it does work without problems. Aside from that, ACK to the idea.

--- src/qemu/qemu_command.c | 17 +++++++---------- src/qemu/qemu_domain.c | 16 ++++++++++++++++ src/qemu/qemu_hotplug.c | 18 ++++++++---------- 3 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4d70004..f902501 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6683,6 +6683,7 @@ qemuBuildCommandLine(virConnectPtr conn, int spice = 0; int usbcontroller = 0; bool usblegacy = false; + bool mlock = false; 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 @@ -8337,22 +8338,15 @@ 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); + /* VFIO requires all of the guest's memory to be locked + * resident */ + mlock = true; } if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { @@ -8572,6 +8566,9 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } + if (mlock) + virCommandSetMaxMemLock(cmd, qemuDomainMemoryLimit(def) * 1024); + virObjectUnref(cfg); return cmd; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 77b94ec..8952a79 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2199,6 +2199,10 @@ qemuDomainMemoryLimit(virDomainDefPtr def) * cache per each disk) + F * where k = 0.5 and F = 200MB. The cache for disks is important as * kernel cache on the host side counts into the RSS limit. + * + * Moreover, VFIO requires some amount for IO space. Alex Williamson + * suggested adding 1GiB for IO space just to be safe (some finer + * tuning might be nice, though). */ mem = def->mem.max_balloon; for (i = 0; i < def->nvideos; i++) @@ -2206,6 +2210,18 @@ qemuDomainMemoryLimit(virDomainDefPtr def) mem *= 1.5; mem += def->ndisks * 32768; mem += 204800; + + for (i = 0; i < def->nhostdevs; i++) { + virDomainHostdevDefPtr hostdev = def->hostdevs[i]; + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == + VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && + hostdev->source.subsys.u.pci.backend == + VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { + mem += 1024 * 1024; + break; + } + } } return mem; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 46875ad..a350059 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1054,23 +1054,21 @@ int qemuDomainAttachHostPciDevice(virQEMUDriverPtr driver, if (hostdev->source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) { - unsigned long long memKB; - if (!virQEMUCapsGet(priv->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). - * In this case, the guest's memory may already be locked, but - * it doesn't hurt to "change" the limit to the same value. + + /* VFIO requires all of the guest's memory to be locked resident. + * In this case, the guest's memory may already be locked, but it + * doesn't hurt to "change" the limit to the same value. */ - memKB = vm->def->mem.max_balloon + (1024 * 1024); - virProcessSetMaxMemLock(vm->pid, memKB * 1024); + vm->def->hostdevs[vm->def->nhostdevs++] = hostdev; + virProcessSetMaxMemLock(vm->pid, + qemuDomainMemoryLimit(vm->def) * 1024); + vm->def->hostdevs[vm->def->nhostdevs--] = NULL; } if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { -- 1.8.2.1

If a domain is configured to have all its memory locked, we need to set RLIMIT_MEMLOCK so that QEMU is actually allowed to lock the memory. --- src/qemu/qemu_command.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f902501..278d2f2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6795,6 +6795,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))) -- 1.8.2.1