Re: [PATCH v3 2/2] tools/xenstore: set open file descriptor limit for xenstored

From: Ian Jackson <iwj@xenproject.org>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Julien Grall <julien@xen.org>, Juergen Gross <jgross@suse.com>,
	<xen-devel@lists.xenproject.org>, Wei Liu <wl@xen.org>
Subject: Re: [PATCH v3 2/2] tools/xenstore: set open file descriptor limit for xenstored
Date: Tue, 31 Aug 2021 15:22:56 +0100	[thread overview]
Message-ID: <24878.15168.981558.748531@mariner.uk.xensource.com> (raw)
In-Reply-To: <9352f82a-1b43-4bf6-8b0a-5916627b7537@citrix.com>

Andrew Cooper writes ("Re: [PATCH v3 2/2] tools/xenstore: set open file descriptor limit for xenstored"):
> xenstored is TCB.  It needs a large number of FDs, and can be trusted
> with unlimited.

I baseically agree with this.

> Also, like xenconsoled, we can calculate an upper bound, which is
> derived from the ABI limit of 32k domids.

IMO the default should support at leaat this much.

However, I don't think you are right, because xenstored offers console
connections to (possibly arbitrarily many) local socket connections.

> All you're haggling over is the error semantics in the case of:
> 1) the upper bound calculation is wrong, or
> 2) there is an fd leak
> 
> Personally, I think a fixed calculation is right, so fd leaks can be
> spotted more obviously.
> 
> An admin knob is not helpful - higher than the upper bound is just
> wasteful, while lower will cause malfunctions.

I don't agree.  Firstly, on a technical level, your statement is true
only if the admin does not know they will be running a much smaller
number of domains.  Secondly, we have had several people saying they
want this to be configurable.  I think if several people say they want
something to be configurable, we should respect that, even if we think
the use cases for it are marginal.  If there are hazards in bad
settings of such a know, that can be dealt with in the docs.

Julien's point about not having the limit set by xenstored itself is
very well taken.

ISTM that the following scheme is in the intersection of everyone's
requirements:

 * The limit will be adjusted/imposed in the startup script.
 * An /etc/{default,sysconfig} parameter will be provided to
   adjust the setting.
 * The default should be `unlimtied` since we cannot calculate
   a safe upper bound for all configurations.
 * Systems like Citrix Hypervisor (XenServer) which can calculate
   a safe upper bound can do so, and adjust the default, enabling
   them to spot fd leaks.

Ian.