[libvirt] [PATCH 3/3] qemu: Adjust max memlock on mdev hotplug

Eric Farman posted 3 patches 6 years, 3 months ago
[libvirt] [PATCH 3/3] qemu: Adjust max memlock on mdev hotplug
Posted by Eric Farman 6 years, 3 months ago
When starting a domain, we use the presence of a vfio-pci or
mdev hostdev to determine if the memlock maximum needs to be
increased.  But if we hotplug either of these devices, only the
vfio-pci path gets that love.  This means that attaching a, say,
vfio-ccw device will appear to succeed but the device may be
unusable as the guest may see I/O errors on long CCW chains.
The host, meanwhile, would be flooded with these messages:

  vfio_pin_page_external: Task qemu-system-s39 (11584) RLIMIT_MEMLOCK (65536) exceeded

Let's adjust the maximum memlock value in the mdev hotplug path,
so that the domain has the same value as if it were started with
one or more mdev devices in its configuration.

(Note: I started trying to refactor qemuDomainAdjustMaxMemLockHostdev()
to address the "A better way to handle this would be nice" comment,
but the result was getting way too involved for the problem I was
trying to fix.  Maybe another day.)

Signed-off-by: Eric Farman <farman@linux.ibm.com>
---
 src/qemu/qemu_hotplug.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 979e97b608..ae7c5395d2 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2755,6 +2755,7 @@ qemuDomainAttachMediatedDevice(virQEMUDriverPtr driver,
     bool teardowncgroup = false;
     bool teardownlabel = false;
     bool teardowndevice = false;
+    bool teardownmemlock = false;
     qemuDomainObjPrivatePtr priv = vm->privateData;
     virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_HOSTDEV,
                                 { .hostdev = hostdev } };
@@ -2798,6 +2799,10 @@ qemuDomainAttachMediatedDevice(virQEMUDriverPtr driver,
     if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0)
         goto cleanup;
 
+    if (qemuDomainAdjustMaxMemLockHostdev(vm, hostdev) < 0)
+        goto cleanup;
+    teardownmemlock = true;
+
     qemuDomainObjEnterMonitor(driver, vm);
     ret = qemuMonitorAddDevice(priv->mon, devstr);
     if (qemuDomainObjExitMonitor(driver, vm) < 0) {
@@ -2813,6 +2818,8 @@ qemuDomainAttachMediatedDevice(virQEMUDriverPtr driver,
     ret = 0;
  cleanup:
     if (ret < 0) {
+        if (teardownmemlock && qemuDomainAdjustMaxMemLock(vm) < 0)
+            VIR_WARN("Unable to reset maximum locked memory on hotplug fail");
         if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0)
             VIR_WARN("Unable to remove host device cgroup ACL on hotplug fail");
         if (teardownlabel &&
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu: Adjust max memlock on mdev hotplug
Posted by Daniel Henrique Barboza 6 years, 3 months ago

On 9/3/19 5:09 PM, Eric Farman wrote:
> When starting a domain, we use the presence of a vfio-pci or
> mdev hostdev to determine if the memlock maximum needs to be
> increased.  But if we hotplug either of these devices, only the
> vfio-pci path gets that love.  This means that attaching a, say,
> vfio-ccw device will appear to succeed but the device may be
> unusable as the guest may see I/O errors on long CCW chains.
> The host, meanwhile, would be flooded with these messages:
>
>    vfio_pin_page_external: Task qemu-system-s39 (11584) RLIMIT_MEMLOCK (65536) exceeded
>
> Let's adjust the maximum memlock value in the mdev hotplug path,
> so that the domain has the same value as if it were started with
> one or more mdev devices in its configuration.
>
> (Note: I started trying to refactor qemuDomainAdjustMaxMemLockHostdev()
> to address the "A better way to handle this would be nice" comment,
> but the result was getting way too involved for the problem I was
> trying to fix.  Maybe another day.)
>
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>


>   src/qemu/qemu_hotplug.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 979e97b608..ae7c5395d2 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -2755,6 +2755,7 @@ qemuDomainAttachMediatedDevice(virQEMUDriverPtr driver,
>       bool teardowncgroup = false;
>       bool teardownlabel = false;
>       bool teardowndevice = false;
> +    bool teardownmemlock = false;
>       qemuDomainObjPrivatePtr priv = vm->privateData;
>       virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_HOSTDEV,
>                                   { .hostdev = hostdev } };
> @@ -2798,6 +2799,10 @@ qemuDomainAttachMediatedDevice(virQEMUDriverPtr driver,
>       if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0)
>           goto cleanup;
>   
> +    if (qemuDomainAdjustMaxMemLockHostdev(vm, hostdev) < 0)
> +        goto cleanup;
> +    teardownmemlock = true;
> +
>       qemuDomainObjEnterMonitor(driver, vm);
>       ret = qemuMonitorAddDevice(priv->mon, devstr);
>       if (qemuDomainObjExitMonitor(driver, vm) < 0) {
> @@ -2813,6 +2818,8 @@ qemuDomainAttachMediatedDevice(virQEMUDriverPtr driver,
>       ret = 0;
>    cleanup:
>       if (ret < 0) {
> +        if (teardownmemlock && qemuDomainAdjustMaxMemLock(vm) < 0)
> +            VIR_WARN("Unable to reset maximum locked memory on hotplug fail");
>           if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0)
>               VIR_WARN("Unable to remove host device cgroup ACL on hotplug fail");
>           if (teardownlabel &&

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu: Adjust max memlock on mdev hotplug
Posted by Pavel Hrdina 6 years, 3 months ago
On Tue, Sep 03, 2019 at 10:09:48PM +0200, Eric Farman wrote:
> When starting a domain, we use the presence of a vfio-pci or
> mdev hostdev to determine if the memlock maximum needs to be
> increased.  But if we hotplug either of these devices, only the
> vfio-pci path gets that love.  This means that attaching a, say,
> vfio-ccw device will appear to succeed but the device may be
> unusable as the guest may see I/O errors on long CCW chains.
> The host, meanwhile, would be flooded with these messages:
> 
>   vfio_pin_page_external: Task qemu-system-s39 (11584) RLIMIT_MEMLOCK (65536) exceeded
> 
> Let's adjust the maximum memlock value in the mdev hotplug path,
> so that the domain has the same value as if it were started with
> one or more mdev devices in its configuration.
> 
> (Note: I started trying to refactor qemuDomainAdjustMaxMemLockHostdev()
> to address the "A better way to handle this would be nice" comment,
> but the result was getting way too involved for the problem I was
> trying to fix.  Maybe another day.)

This paragraph doesn't belong into the commit message, I'll remove it
before pushing.

Otherwise looks good.

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list