From 8e8886fd9576d9f78136050bff7750a38b9eccd3 Mon Sep 17 00:00:00 2001 From: Laurent Bigonville Date: Sun, 24 Nov 2013 16:26:31 +0100 Subject: [PATCH] d/p/0005-Provide-retry-options-for-upsdrvctl-and-drivers.patch, d/p/0006-ups-conf-maxretry.patch: By default, retry to start the drivers up-to three times, this should mitigate races with slow devices (Closes: #694717) --- debian/changelog | 6 +- ...ry-options-for-upsdrvctl-and-drivers.patch | 133 ++++++++++++++++++ debian/patches/0006-ups-conf-maxretry.patch | 34 +++++ debian/patches/series | 2 + 4 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 debian/patches/0005-Provide-retry-options-for-upsdrvctl-and-drivers.patch create mode 100644 debian/patches/0006-ups-conf-maxretry.patch diff --git a/debian/changelog b/debian/changelog index e7375ce..72f68fe 100644 --- a/debian/changelog +++ b/debian/changelog @@ -3,8 +3,12 @@ nut (2.7.1-1) UNRELEASED; urgency=low * New upstream release (Closes: #730183) - Refresh debian/patches/0004-fix-systemd-service.patch * debian/rules, debian/control: Enable SSL support using libnss3 + * d/p/0005-Provide-retry-options-for-upsdrvctl-and-drivers.patch, + d/p/0006-ups-conf-maxretry.patch: By default, retry to start the drivers + up-to three times, this should mitigate races with slow devices + (Closes: #694717) - -- Laurent Bigonville Sun, 24 Nov 2013 16:23:21 +0100 + -- Laurent Bigonville Sun, 24 Nov 2013 16:26:05 +0100 nut (2.6.5-4) unstable; urgency=low diff --git a/debian/patches/0005-Provide-retry-options-for-upsdrvctl-and-drivers.patch b/debian/patches/0005-Provide-retry-options-for-upsdrvctl-and-drivers.patch new file mode 100644 index 0000000..ee3cb8f --- /dev/null +++ b/debian/patches/0005-Provide-retry-options-for-upsdrvctl-and-drivers.patch @@ -0,0 +1,133 @@ +From ad1e78a22dc91b70bee871425cd5f74bb3c34c9f Mon Sep 17 00:00:00 2001 +From: Laurent Bigonville +Date: Thu, 21 Nov 2013 17:52:16 +0100 +Subject: [PATCH] Provide retry options for upsdrvctl and driver(s) + +As recently seen in Debian (bugs #694717 and #677143), it may be required to +have upsdrvctl retrying to start the driver in case of failure. More +specifically, a mix of init system (V and systemd), udev and USB device(s) can +result in the /dev entry not being available at driver startup, thus resulting +in a general failure to start NUT. This commit provides at least a way to +overcome this issue. A more suitable solution will require more work on NUT +design. + +This patch if based on Arnaud Quette proposal +--- + docs/man/ups.conf.txt | 15 +++++++++++++++ + docs/man/upsdrvctl.txt | 3 ++- + drivers/upsdrvctl.c | 37 ++++++++++++++++++++++++++++++++++--- + 3 files changed, 51 insertions(+), 4 deletions(-) + +diff --git a/docs/man/ups.conf.txt b/docs/man/ups.conf.txt +index 8a72a83..4877852 100644 +--- a/docs/man/ups.conf.txt ++++ b/docs/man/ups.conf.txt +@@ -58,6 +58,21 @@ directory, which is often /usr/local/ups/bin. + Optional. Same as the UPS field of the same name, but this is the + default for UPSes that don't have the field. + ++*maxretry*:: ++Optional. Specify the number of attempts to start the driver(s), in case of ++failure, before giving up. A delay of 'retrydelay' is inserted between each ++attempt. Caution should be taken when using this option, since it can ++impact the time taken by your system to start. +++ ++The default is 1 attempt. ++ ++*retrydelay*:: ++Optional. Specify the delay between each restart attempt of the driver(s), ++as specified by 'maxretry'. Caution should be taken when using this option, ++since it can impact the time taken by your system to start. +++ ++The default is 5 seconds. ++ + *pollinterval*:: + + Optional. The status of the UPS will be refreshed after a maximum +diff --git a/docs/man/upsdrvctl.txt b/docs/man/upsdrvctl.txt +index f19fd77..52e5509 100644 +--- a/docs/man/upsdrvctl.txt ++++ b/docs/man/upsdrvctl.txt +@@ -63,7 +63,8 @@ Without that argument, they operate on every UPS that is currently + configured. + + *start*:: +-Start the UPS driver(s). ++Start the UPS driver(s). In case of failure, further attempts may be executed ++by using the 'maxretry' and 'retrydelay' options - see linkman:ups.conf[5]. + + *stop*:: + Stop the UPS driver(s). +diff --git a/drivers/upsdrvctl.c b/drivers/upsdrvctl.c +index 623f753..0cdaa2c 100644 +--- a/drivers/upsdrvctl.c ++++ b/drivers/upsdrvctl.c +@@ -45,6 +45,12 @@ + /* timer - keeps us from getting stuck if a driver hangs */ + static int maxstartdelay = 45; + ++ /* counter - retry that many time(s) to start the driver if it fails to */ ++static int maxretry = 1; ++ ++ /* timer - delay between each restart attempt of the driver(s) */ ++static int retrydelay = 5; ++ + /* Directory where driver executables live */ + static char *driverpath = NULL; + +@@ -65,6 +71,12 @@ void do_upsconf_args(char *upsname, char *var, char *val) + driverpath = xstrdup(val); + } + ++ if (!strcmp(var, "maxretry")) ++ maxretry = atoi(val); ++ ++ if (!strcmp(var, "retrydelay")) ++ retrydelay = atoi(val); ++ + /* ignore anything else - it's probably for main */ + + return; +@@ -248,6 +260,7 @@ static void start_driver(const ups_t *ups) + char *argv[8]; + char dfn[SMALLBUF]; + int ret, arg = 0; ++ int initial_exec_error = exec_error, drv_maxretry = maxretry; + struct stat fs; + + upsdebugx(1, "Starting UPS: %s", ups->upsname); +@@ -276,10 +289,28 @@ static void start_driver(const ups_t *ups) + /* tie it off */ + argv[arg++] = NULL; + +- debugcmdline(2, "exec: ", argv); + +- if (!testmode) { +- forkexec(argv, ups); ++ while (drv_maxretry > 0) { ++ int cur_exec_error = exec_error; ++ ++ upsdebugx(2, "%i remaining attempts", drv_maxretry); ++ debugcmdline(2, "exec: ", argv); ++ drv_maxretry--; ++ ++ if (!testmode) { ++ forkexec(argv, ups); ++ } ++ ++ /* driver command succeeded */ ++ if (cur_exec_error == exec_error) { ++ drv_maxretry = 0; ++ exec_error = initial_exec_error; ++ } ++ else { ++ /* otherwise, retry if still needed */ ++ if (drv_maxretry > 0) ++ sleep (retrydelay); ++ } + } + } + +-- +1.8.4 + diff --git a/debian/patches/0006-ups-conf-maxretry.patch b/debian/patches/0006-ups-conf-maxretry.patch new file mode 100644 index 0000000..dd8ef93 --- /dev/null +++ b/debian/patches/0006-ups-conf-maxretry.patch @@ -0,0 +1,34 @@ +--- a/conf/ups.conf.sample ++++ b/conf/ups.conf.sample +@@ -40,6 +40,24 @@ + # + # Configuration directives + # ------------------------ ++# ++# These directives are used by upsdrvctl only and should be specified outside ++# of a driver definition: ++# ++# maxretry: Optional. Specify the number of attempts to start the driver(s), ++# in case of failure, before giving up. A delay of 'retrydelay' is ++# inserted between each attempt. Caution should be taken when using ++# this option, since it can impact the time taken by your system to ++# start. ++# ++# The default is 1 attempt. ++# ++# retrydelay: Optional. Specify the delay between each restart attempt of the ++# driver(s), as specified by 'maxretry'. Caution should be taken ++# when using this option, since it can impact the time taken by your ++# system to start. ++# ++# The default is 5 seconds. + # + # These directives are common to all drivers that support ups.conf: + # +@@ -102,3 +120,6 @@ + # + # To find out if your driver supports any extra settings, start it with + # the -h option and/or read the driver's documentation. ++ ++# Set maxretry to 3 by default, this should mitigate race with slow devices: ++maxretry = 3 diff --git a/debian/patches/series b/debian/patches/series index 540178d..aaccd1f 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -1,3 +1,5 @@ 0003-install-dev-files-in-usr.patch 0002-nut-monitor-paths.patch 0004-fix-systemd-service.patch +0005-Provide-retry-options-for-upsdrvctl-and-drivers.patch +0006-ups-conf-maxretry.patch