Re: [libvirt PATCH 15/17] qemu: Add external_limit_manager config knob

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 3/5/21 8:14 PM, Andrea Bolognani wrote:
> This will be useful when libvirtd is running in a containerized
> environment with limited capabilities, and in order to make
> things like VFIO device assignment still work an external
> privileged process changes the limits from outside of the
> container. KubeVirt is an example of this setup.
>
> Signed-off-by: Andrea Bolognani <abologna@xxxxxxxxxx>
> ---
>   src/qemu/libvirtd_qemu.aug         |  1 +
>   src/qemu/qemu.conf                 | 12 ++++++++++++
>   src/qemu/qemu_conf.c               |  4 ++++
>   src/qemu/qemu_conf.h               |  1 +
>   src/qemu/test_libvirtd_qemu.aug.in |  1 +
>   5 files changed, 19 insertions(+)
>
> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
> index 3c1045858b..f1b024a37f 100644
> --- a/src/qemu/libvirtd_qemu.aug
> +++ b/src/qemu/libvirtd_qemu.aug
> @@ -104,6 +104,7 @@ module Libvirtd_qemu =
>                    | str_entry "slirp_helper"
>                    | str_entry "dbus_daemon"
>                    | bool_entry "set_process_name"
> +                 | bool_entry "external_limit_manager"
>                    | int_entry "max_processes"
>                    | int_entry "max_files"
>                    | limits_entry "max_core"
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index 0c1054f198..15cbc3ba38 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -662,6 +662,18 @@
>   #
>   #set_process_name = 1
>   
> +# If enabled, libvirt will not attempt to change process limits (as
> +# configured with the max_processes, max_files and max_core settings
> +# below) itself but will instead expect an external entity to perform
> +# this task.
> +#
> +# This also applies to the memory locking limit, which cannot be
> +# configured here and is instead calculated dynamically based on the
> +# exact guest configuration: if an external limit manager is in use,
> +# then libvirt will merely check that the limit has been set
> +# appropriately.
> +#
> +#external_limit_manager = 1
>   
>   # If max_processes is set to a positive integer, libvirt will use
>   # it to set the maximum number of processes that can be run by qemu
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 2bbc75024c..ee95c124dd 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -673,6 +673,10 @@ virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfigPtr cfg,
>   
>       if (virConfGetValueBool(conf, "set_process_name", &cfg->setProcessName) < 0)
>           return -1;
> +
> +    if (virConfGetValueBool(conf, "external_limit_manager", &cfg->externalLimitManager) < 0)
> +        return -1;
> +
>       if (virConfGetValueUInt(conf, "max_processes", &cfg->maxProcesses) < 0)
>           return -1;
>       if (virConfGetValueUInt(conf, "max_files", &cfg->maxFiles) < 0)

I think we could error out if external_limit_manager is set and one of 
these max_processes, max_files, max_core is set also. It's not really a 
combination we can make sense of, is it? It's perfectly okay to write 
that in a separate patch though.

> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index 7025b5222e..15e0353253 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -176,6 +176,7 @@ struct _virQEMUDriverConfig {
>       bool nogfxAllowHostAudio;
>       bool setProcessName;
>   
> +    bool externalLimitManager;
>       unsigned int maxProcesses;
>       unsigned int maxFiles;
>       unsigned int maxThreadsPerProc;
> diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
> index 9310dcec1c..73be55febe 100644
> --- a/src/qemu/test_libvirtd_qemu.aug.in
> +++ b/src/qemu/test_libvirtd_qemu.aug.in
> @@ -77,6 +77,7 @@ module Test_libvirtd_qemu =
>   { "hugetlbfs_mount" = "/dev/hugepages" }
>   { "bridge_helper" = "/usr/libexec/qemu-bridge-helper" }
>   { "set_process_name" = "1" }
> +{ "external_limit_manager" = "1" }
>   { "max_processes" = "0" }
>   { "max_files" = "0" }
>   { "max_threads_per_process" = "0" }

Michal