Re: [PATCH v2] tools/xenconsoled: Increase file descriptor limit

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 17/02/15 16:28, Wei Liu wrote:
> On Tue, Feb 17, 2015 at 04:21:24PM +0000, Andrew Cooper wrote:
>> XenServer's VM density testing uncovered a regression when moving from
>> sysvinit to systemd where the file descriptor limit dropped from 4096 to
>> 1024. (XenServer had previously inserted a ulimit statement into its
>> initscripts.)
>>
>> One solution is to use LimitNOFILE=4096 in xenconsoled.service to match the
>> lost ulimit, but that is only a stopgap solution.
>>
>> As Xenconsoled genuinely needs a large number of file descriptors if a large
>> number of domains are running, and is well behaved with its descriptors,
>> attempt to up the limit to the system maximum.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>>
>> ---
>> v2:
>>  * Always increase soft limit to hard limit
>>  * Correct commment regarding number of file descriptors
>>  * long -> unsigned long as that appears to be the underlying type of an rlim_t
>> ---
>>  tools/console/daemon/main.c |   45 +++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 45 insertions(+)
>>
>> diff --git a/tools/console/daemon/main.c b/tools/console/daemon/main.c
>> index 92d2fc4..0bb9d8a 100644
>> --- a/tools/console/daemon/main.c
>> +++ b/tools/console/daemon/main.c
>> @@ -26,6 +26,7 @@
>>  #include <string.h>
>>  #include <signal.h>
>>  #include <sys/types.h>
>> +#include <sys/resource.h>
>>  
>>  #include "xenctrl.h"
>>  
>> @@ -55,6 +56,48 @@ static void version(char *name)
>>  	printf("Xen Console Daemon 3.0\n");
>>  }
>>  
>> +/*
>> + * Xenconsoled requires two file descriptors for each PV console (pty and log
>> + * file), which can easily exceed the default of 1024 if many guests are
>> + * running.  Try to set the fd limit to the system maximum, falling back to a
>> + * default of 4096.
>> + */
>> +static void increase_fd_limit(void)
>> +{
>> +	FILE *fs_nr_open;
>> +	struct rlimit lim;
>> +	unsigned long nr_open = 4096;
>> +
>> +	if (getrlimit(RLIMIT_NOFILE, &lim) < 0)
>> +		return;
>> +
>> +	/* Increase the soft limit to the current hard limit. */
>> +	if (lim.rlim_cur < lim.rlim_max) {
>> +		lim.rlim_cur = lim.rlim_max;
>> +		if (setrlimit(RLIMIT_NOFILE, &lim) < 0)
>> +			return;
>> +	}
>> +
>> +	/* Attempt to locate the system maximum. */
>> +	fs_nr_open = fopen("/proc/sys/fs/nr_open", "r");
>> +	if (fs_nr_open) {
>> +		unsigned long nr;
>> +
>> +		if (fscanf(fs_nr_open, "%lu", &nr) == 1)
>> +			nr_open = nr;
>> +		fclose(fs_nr_open);
>> +	}
>> +
>> +	/*
>> +	 * In the likely case that we are a root process with
>> +	 * CAP_SYS_RESOURCE, attempt to up our hard limit.
>> +	 */
>> +	if (nr_open > lim.rlim_max) {
>> +		lim.rlim_cur = lim.rlim_max = nr_open;
>> +		setrlimit(RLIMIT_NOFILE, &lim);
>> +	}
>> +}
>> +
> This function looks Linux centric (/proc and CAP_SYSRESOURCE). Please be
> considerate to other Unices. :-)
>
> Also you might want to log failures along the line.
>
> Wei.

setrlimit() is posix.  From the manpage:

CONFORMING TO
       getrlimit(), setrlimit(): SVr4, 4.3BSD, POSIX.1-2001.

"/proc/sys/fs/nr_open" is likely Linux only, but not fatal, and doesn't
prevent an arbitrary grab for 4096 descriptors.


I am not sure what an admin could usefully do with a logged failure
message here.  Xenconsoled will most likely not function in an
environment where it is not sufficiently privileged to make this limit
adjustment (use of privcmd and /dev/ptmx).

~Andrew