[PATCH] tools/xenconsoled: Increase file descriptor limit

[PATCH] tools/xenconsoled: Increase file descriptor limit

[Andrew Cooper]

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>
---
 tools/console/daemon/main.c |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/tools/console/daemon/main.c b/tools/console/daemon/main.c
index 92d2fc4..759c061 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,34 @@ static void version(char *name)
 	printf("Xen Console Daemon 3.0\n");
 }
 
+/*
+ * Xenconsoled requires one file descriptor for each PV console, 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;
+	long nr_open = 4096;
+	int rc;
+
+	rc = getrlimit(RLIMIT_NOFILE, &lim);
+	if (rc)
+		return;
+
+	fs_nr_open = fopen("/proc/sys/fs/nr_open", "r");
+	if (fs_nr_open) {
+		fscanf(fs_nr_open, "%ld", &nr_open);
+		fclose(fs_nr_open);
+	}
+
+	if ((nr_open > lim.rlim_cur) || (nr_open > lim.rlim_max)) {
+		lim.rlim_cur = lim.rlim_max = nr_open;
+		setrlimit(RLIMIT_NOFILE, &lim);
+	}
+}
+
 int main(int argc, char **argv)
 {
 	const char *sopts = "hVvit:o:";
@@ -154,6 +183,8 @@ int main(int argc, char **argv)
 	openlog("xenconsoled", syslog_option, LOG_DAEMON);
 	setlogmask(syslog_mask);
 
+	increase_fd_limit();
+
 	if (!is_interactive) {
 		daemonize(pidfile ? pidfile : "/var/run/xenconsoled.pid");
 	}
-- 
1.7.10.4

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

[Don Slutz]

On 02/16/15 13:17, 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>
> ---
>  tools/console/daemon/main.c |   31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/tools/console/daemon/main.c b/tools/console/daemon/main.c
> index 92d2fc4..759c061 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,34 @@ static void version(char *name)
>  	printf("Xen Console Daemon 3.0\n");
>  }
>  
> +/*
> + * Xenconsoled requires one file descriptor for each PV console, 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;
> +	long nr_open = 4096;
> +	int rc;
> +
> +	rc = getrlimit(RLIMIT_NOFILE, &lim);
> +	if (rc)
> +		return;
> +
> +	fs_nr_open = fopen("/proc/sys/fs/nr_open", "r");
> +	if (fs_nr_open) {
> +		fscanf(fs_nr_open, "%ld", &nr_open);
> +		fclose(fs_nr_open);
> +	}
> +
> +	if ((nr_open > lim.rlim_cur) || (nr_open > lim.rlim_max)) {
> +		lim.rlim_cur = lim.rlim_max = nr_open;
> +		setrlimit(RLIMIT_NOFILE, &lim);
> +	}
> +}
> +

I am not sure that Xenconsoled is required to have the CAP_SYS_RESOURCE
capability.  With out this, the "soft" (rlim_cur) will most like not be
changed.  Does it make sense to include:

    lim.rlim_cur = lim.rlim_max;
    rc = setrlimit(RLIMIT_NOFILE, &lim);
    if (rc)
        return;

before "fs_nr_open = fopen(..."?

   -Don Slutz


>  int main(int argc, char **argv)
>  {
>  	const char *sopts = "hVvit:o:";
> @@ -154,6 +183,8 @@ int main(int argc, char **argv)
>  	openlog("xenconsoled", syslog_option, LOG_DAEMON);
>  	setlogmask(syslog_mask);
>  
> +	increase_fd_limit();
> +
>  	if (!is_interactive) {
>  		daemonize(pidfile ? pidfile : "/var/run/xenconsoled.pid");
>  	}
> 

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

[Andrew Cooper]

On 16/02/2015 20:44, Don Slutz wrote:
> On 02/16/15 13:17, 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>
>> ---
>>  tools/console/daemon/main.c |   31 +++++++++++++++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>>
>> diff --git a/tools/console/daemon/main.c b/tools/console/daemon/main.c
>> index 92d2fc4..759c061 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,34 @@ static void version(char *name)
>>  	printf("Xen Console Daemon 3.0\n");
>>  }
>>  
>> +/*
>> + * Xenconsoled requires one file descriptor for each PV console, 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;
>> +	long nr_open = 4096;
>> +	int rc;
>> +
>> +	rc = getrlimit(RLIMIT_NOFILE, &lim);
>> +	if (rc)
>> +		return;
>> +
>> +	fs_nr_open = fopen("/proc/sys/fs/nr_open", "r");
>> +	if (fs_nr_open) {
>> +		fscanf(fs_nr_open, "%ld", &nr_open);
>> +		fclose(fs_nr_open);
>> +	}
>> +
>> +	if ((nr_open > lim.rlim_cur) || (nr_open > lim.rlim_max)) {
>> +		lim.rlim_cur = lim.rlim_max = nr_open;
>> +		setrlimit(RLIMIT_NOFILE, &lim);
>> +	}
>> +}
>> +
> I am not sure that Xenconsoled is required to have the CAP_SYS_RESOURCE
> capability.  With out this, the "soft" (rlim_cur) will most like not be
> changed.  Does it make sense to include:
>
>     lim.rlim_cur = lim.rlim_max;
>     rc = setrlimit(RLIMIT_NOFILE, &lim);
>     if (rc)
>         return;
>
> before "fs_nr_open = fopen(..."?

It probably does make sense, although xenconsoled cannot realistically
run without privilege because of privcmd and /dev/ptmx.

I shall respin.

~Andrew