[libvirt] [PATCH v2 1/3] qemu: Extract logic to determine the mlock limit size for VFIO

[John Ferlan]

On 11/09/2015 07:50 AM, Peter Krempa wrote:
> New function qemuDomainGetMlockLimitBytes will now handle the
> calculation so that it unifies the logic to one place and allows later
> reuse.
> ---
>  src/qemu/qemu_command.c | 18 ++----------------
>  src/qemu/qemu_domain.c  | 27 +++++++++++++++++++++++++++
>  src/qemu/qemu_domain.h  |  2 ++
>  src/qemu/qemu_hotplug.c | 17 ++---------------
>  4 files changed, 33 insertions(+), 31 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 8824541..9acf8e4 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -11447,22 +11447,8 @@ 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 (virMemoryLimitIsSet(def->mem.hard_limit))
> -            memKB = def->mem.hard_limit;
> -        else
> -            memKB = virDomainDefGetMemoryActual(def) + 1024 * 1024;
> -
> -        virCommandSetMaxMemLock(cmd, memKB * 1024);
> -    }
> +    if (mlock)
> +        virCommandSetMaxMemLock(cmd, qemuDomainGetMlockLimitBytes(def));
> 
>      if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MSG_TIMESTAMP) &&
>          cfg->logTimestamp)
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 890d8ed..a06624e 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3616,3 +3616,30 @@ qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
> 
>      return 0;
>  }
> +
> +
> +/**
> + * qemuDomainGetMlockLimitBytes:
> + *
> + * @def: domain definition
> + *
> + * Returns the size of the memory in bytes that needs to be set as
> + * RLIMIT_MEMLOCK for purpose of VFIO device passthrough. In cases where
> + * mem.hard_limit is set, the value is preferred,otherwise the value may depend
> + * on the device or architecture.


How about?

Returns the size of memory in bytes to allow a process (domain) to be
locked into RAM (e.g setrlimit RLIMIT_MEMLOCK). If a mem.hard_limit is
set, then that value is preferred; otherwise, the value returned may
depend upon the architecture or devices present, such as a VFIO
passthrough device.


ACK -

John
> + */
> +unsigned long long
> +qemuDomainGetMlockLimitBytes(virDomainDefPtr def)
> +{
> +    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 (virMemoryLimitIsSet(def->mem.hard_limit))
> +        memKB = def->mem.hard_limit;
> +    else
> +        memKB = virDomainDefGetMemoryActual(def) + 1024 * 1024;
> +
> +    return memKB << 10;
> +}
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 64cd7e1..e34370b 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -482,4 +482,6 @@ bool qemuDomainMachineIsS390CCW(const virDomainDef *def);
>  int qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
>                                        virDomainObjPtr vm);
> 
> +unsigned long long qemuDomainGetMlockLimitBytes(virDomainDefPtr def);
> +
>  #endif /* __QEMU_DOMAIN_H__ */
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 8f2fda9..e7fc036 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1254,7 +1254,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
>      bool teardowncgroup = false;
>      bool teardownlabel = false;
>      int backend;
> -    unsigned long long memKB;
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>      unsigned int flags = 0;
> 
> @@ -1279,20 +1278,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver,
>              goto error;
>          }
> 
> -        /* VFIO requires all of the guest's memory to be locked
> -         * resident (plus an additional 1GiB to cover IO space). During
> -         * hotplug, the guest's memory may already be locked, but it
> -         * doesn't hurt to "change" the limit to the same value.
> -         * NB: the domain's memory tuning parameters are stored as
> -         * Kibibytes, but virProcessSetMaxMemLock expects the value in
> -         * bytes.
> -         */
> -        if (virMemoryLimitIsSet(vm->def->mem.hard_limit))
> -            memKB = vm->def->mem.hard_limit;
> -        else
> -            memKB = virDomainDefGetMemoryActual(vm->def) + (1024 * 1024);
> -
> -        virProcessSetMaxMemLock(vm->pid, memKB * 1024);
> +        /* setup memory locking limits, that are necessary for VFIO */
> +        virProcessSetMaxMemLock(vm->pid, qemuDomainGetMlockLimitBytes(vm->def));
>          break;
> 
>      default:
>