| Message ID | 20210608055839.10313-3-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tools/xenstore: set resource limits of xenstored | expand |
Am Tue, 8 Jun 2021 07:58:39 +0200
schrieb Juergen Gross <jgross@suse.com>:
> +#XENSTORED_MAX_N_DOMAINS=32768
This will break fillup.
Provide an empty variable like it is done for a few others in that file.
Olaf
On 08.06.21 18:39, Olaf Hering wrote: > Am Tue, 8 Jun 2021 07:58:39 +0200 > schrieb Juergen Gross <jgross@suse.com>: > >> +#XENSTORED_MAX_N_DOMAINS=32768 > > This will break fillup. Why? You realize that above is a comment just documenting the default? > Provide an empty variable like it is done for a few others in that file. I'm following the pattern of basically all variables in that file, BTW. Juergen
Am Fri, 11 Jun 2021 07:01:31 +0200
schrieb Juergen Gross <jgross@suse.com>:
> Why? You realize that above is a comment just documenting the default?
That depends on the context. See https://bugzilla.opensuse.org/show_bug.cgi?id=1185682 for a reason why it should become an empty variable. But yes, we can patch that one too.
Olaf
On 11.06.21 07:46, Olaf Hering wrote: > Am Fri, 11 Jun 2021 07:01:31 +0200 > schrieb Juergen Gross <jgross@suse.com>: > >> Why? You realize that above is a comment just documenting the default? > > That depends on the context. See https://bugzilla.opensuse.org/show_bug.cgi?id=1185682 for a reason why it should become an empty variable. But yes, we can patch that one too. Isn't that a bug in fillup or the related rpm-macro? A variable set in the existing /etc/sysconfig/xencommons file only should be preserved. In general I think we should be consistent in the file. In case there is no downside for other distributions I'd recommend to switch all variables to your suggested pattern. If there are disadvantages for others we should keep the current pattern as changing it now would break existing installations. Any thoughts? Juergen
Am Fri, 11 Jun 2021 09:07:24 +0200
schrieb Juergen Gross <jgross@suse.com>:
> Isn't that a bug in fillup or the related rpm-macro?
No. Fillup expects a certain pattern: a bunch of comments and a single key=var right after that. With such format it might be able to adjust the comment and leave the key=var as it is. Without key=var it will see it as a stale comment, and removes the entire section of comments during the next package update.
Olaf
diff --git a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in index 00cf7f91d4..516cd97092 100644 --- a/tools/hotplug/Linux/init.d/sysconfig.xencommons.in +++ b/tools/hotplug/Linux/init.d/sysconfig.xencommons.in @@ -32,6 +32,13 @@ # Changing this requires a reboot to take effect. #XENSTORED=@XENSTORED@ +## Type: integer +## Default: 32768 +# +# Select maximum number of domains supported by xenstored. +# Only evaluated if XENSTORETYPE is "daemon". +#XENSTORED_MAX_N_DOMAINS=32768 + ## Type: string ## Default: "" # diff --git a/tools/hotplug/Linux/launch-xenstore.in b/tools/hotplug/Linux/launch-xenstore.in index 3ad71e3d08..89149f98ee 100644 --- a/tools/hotplug/Linux/launch-xenstore.in +++ b/tools/hotplug/Linux/launch-xenstore.in @@ -54,12 +54,14 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF [ "$XENSTORETYPE" = "daemon" ] && { [ -z "$XENSTORED_TRACE" ] || XENSTORED_ARGS="$XENSTORED_ARGS -T @XEN_LOG_DIR@/xenstored-trace.log" + [ -z "$XENSTORED_MAX_N_DOMAINS" ] && XENSTORED_MAX_N_DOMAINS=32768 [ -z "$XENSTORED" ] && XENSTORED=@XENSTORED@ [ -x "$XENSTORED" ] || { echo "No xenstored found" exit 1 } rm -f @XEN_RUN_DIR@/xenstored.pid + N_FILES=$(($XENSTORED_MAX_N_DOMAINS * 5 + 100)) echo -n Starting $XENSTORED... $XENSTORED --pid-file @XEN_RUN_DIR@/xenstored.pid $XENSTORED_ARGS @@ -67,6 +69,7 @@ test -f @CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons && . @CONFIG_DIR@/@CONFIG_LEAF systemd-notify --booted 2>/dev/null || timeout_xenstore $XENSTORED || exit 1 XS_PID=`cat @XEN_RUN_DIR@/xenstored.pid` echo -500 >/proc/$XS_PID/oom_score_adj + prlimit --pid $XS_PID --nofile=$N_FILES exit 0 }
Add a configuration item for the maximum number of domains xenstored should support and set the limit of open file descriptors accordingly. For HVM domains there are up to 5 socket connections per domain (2 by the xl daemon process, and 3 by qemu). So set the ulimit for xenstored to 5 * XENSTORED_MAX_DOMAINS + 100 (the "+ 100" is for some headroom, like logging, event channel device, etc.). Signed-off-by: Juergen Gross <jgross@suse.com> --- V2: - set ulimit form launch script (Julien Grall) - split off from original patch (Julien Grall) --- tools/hotplug/Linux/init.d/sysconfig.xencommons.in | 7 +++++++ tools/hotplug/Linux/launch-xenstore.in | 3 +++ 2 files changed, 10 insertions(+)