[libvirt] [PATCH 3/6] qemu: Use qemuDomainRequiresMlock() when attaching PCI hostdev

[Peter Krempa]

On Wed, Nov 18, 2015 at 15:13:17 +0100, Andrea Bolognani wrote:
> The function is used everywhere else to check whether the locked
> memory limit should be set / updated, and it should be used here
> as well.
> 
> Moreover, qemuDomainGetMlockLimitBytes() expects the hostdev to
> have already been added to the domain definition, but we only do
> that at the end of qemuDomainAttachHostPCIDevice(). Work around
> the issue by adding the hostdev before adjusting the locked memory
> limit and removing it immediately afterwards.
> ---
>  src/qemu/qemu_hotplug.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 89e5c0d..0bd88ce 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1269,18 +1269,27 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
>                               "supported by this version of qemu"));
>              goto error;
>          }
> -
> -        /* setup memory locking limits, that are necessary for VFIO */
> -        if (virProcessSetMaxMemLock(vm->pid,
> -                                    qemuDomainGetMlockLimitBytes(vm->def)) < 0)
> -            goto error;
> -
>          break;
>  
>      default:
>          break;
>      }
>  
> +    /* Temporarily add the hostdev to the domain definition. This is needed
> +     * because qemuDomainRequiresMlock() and qemuDomainGetMlockLimitBytes()
> +     * require the hostdev to be already part of the domain definition, but
> +     * other functions like qemuAssignDeviceHostdevAlias() used below expect
> +     * it *not* to be there. A better way to handle this would be nice */
> +    vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
> +    if (qemuDomainRequiresMlock(vm->def)) {
> +        if (virProcessSetMaxMemLock(vm->pid,
> +                                    qemuDomainGetMlockLimitBytes(vm->def)) < 0) {
> +            vm->def->hostdevs[--(vm->def->nhostdevs)] = NULL;
> +            goto error;
> +        }
> +    }
> +    vm->def->hostdevs[--(vm->def->nhostdevs)] = NULL;
> +
>      if (qemuSetupHostdevCGroup(vm, hostdev) < 0)
>          goto error;
>      teardowncgroup = true;

Hmm, ugly, byt required for the PPC64 calculation in the following
patches. I guess reworking the hotplug code would be rather ugly so

relucant ACK.

Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151118/672529f9/attachment-0001.sig>