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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]