Re: [pve-devel] [PATCH qemu-server 1/1] fix #4507 : increase qemu max openfiles limit

[Fiona Ebner <f.ebner@proxmox.com>]

Much of the cover letter would be nice to have as a commit message here.

Missing your Signed-off-by.

Am 10.12.23 um 15:49 schrieb Alexandre Derumier:
> ---
>  PVE/QemuServer.pm | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 2063e66..eba26f3 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5991,6 +5991,12 @@ sub vm_start_nolock {
>      eval { PVE::QemuServer::PCI::reserve_pci_usage($pci_reserve_list, $vmid, undef, $pid) };
>      warn $@ if $@;
>  
> +    #increase OpenFile limit

Nit: why is "OpenFile" capitalized like this? I'd rather put the bug
number in the comment, because the "what" is rather obvious while the
"why" is less so.

> +    eval {
> +	run_command(['/usr/bin/prlimit', '--nofile=524288:524288',  "--pid", $pid]);
> +    };

Some short mention that this is the default hard limit would be nice.

We don't usually write an absolute path to the binary with run_command.

> +    warn $@ if $@;
> +

Please use log_warn() so it will be visible as a task warning too.

>      if (defined($res->{migrate})) {
>  	print "migration listens on $res->{migrate}->{uri}\n";
>      } elsif ($statefile) {

I thought: can't we instead do this as part of setting up the systemd
scope/qemu.slice with LimitNOFILE?

But that would also affect the swtpm process spawned there :/ and the
systemd.exec man page says:

>            │LimitNOFILE=     │ ulimit -n         │ Number of File Descriptors │ Don't use. Be careful when raising the soft limit above 1024, since select() cannot │
>            │                 │                   │                            │ function with file descriptors above 1023 on Linux. Nowadays, the hard limit        │
>            │                 │                   │                            │ defaults to 524288, a very high value compared to historical defaults. Typically    │
>            │                 │                   │                            │ applications should increase their soft limit to the hard limit on their own, if    │
>            │                 │                   │                            │ they are OK with working with file descriptors above 1023, i.e. do not use          │
>            │                 │                   │                            │ select(). Note that file descriptors are nowadays accounted like any other form of  │
>            │                 │                   │                            │ memory, thus there should not be any need to lower the hard limit. Use MemoryMax=   │
>            │                 │                   │                            │ to control overall service memory use, including file descriptor memory.            │

So not sure if that's really nicer. This suggests QEMU should raise the
limit itself.