Re: [systemd-devel] [PATCH v2] watchdog: Don't require WDIOC_SETOPTIONS/WDIOS_ENABLECARD
On Wed, 17 Jun 2015 12:02:27 +0200, Lennart Poettering wrote:
On Wed, 17.06.15 11:26, Jean Delvare (jdelv...@suse.de) wrote:
Not all watchdog drivers implement WDIOC_SETOPTIONS. Drivers which do
not implement it have their device always enabled. So it's fine to
report an error if WDIOS_DISABLECARD is passed and the ioctl is not
implemented, however failing when WDIOS_ENABLECARD is passed and the
ioctl is not implemented is not good: if the device was already
enabled then WDIOS_ENABLECARD was a no-op and wasn't needed in the
first place. So we can just ignore the error and continue.
---
Lennart, would that be OK with you?
Generally yes.
But: indentation is borked, there some whitespace issues, I cannot
even apply this.
Please have a look at CODING_STYLE.
Oops, sorry about that. I had read CODING_STYLE, and paid attention for
the first submission, but then forgot to double-check the indentation
after doing the requested changes. All other projects I'm working on
use tabs, not spaces, so that's my editor does unless I fix it manually.
Also, please build the stuff locally before commit and send to the
ML. We have a git commit hook that checks for weird whitespace at time
of commit. It's enables as soon as you run autogen.sh once...
I always test-build. But I won't get to commit the patch myself, which
is why I did not get the warning.
Changes since v1:
* Log WDIOC_SETOPTIONS/WDIOS_ENABLECARD failure at debug level if
ENOTTY is returned. Suggested by Lennart Poettering.
src/shared/watchdog.c |9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
--- systemd.orig/src/shared/watchdog.c 2015-06-15
18:51:40.811989627 +0200 +++ systemd/src/shared/watchdog.c
2015-06-17 11:20:01.798331790 +0200 @@ -60,8 +60,13 @@ static int
update_timeout(void) {
flags = WDIOS_ENABLECARD;
r = ioctl(watchdog_fd, WDIOC_SETOPTIONS, flags);
-if (r 0)
-return log_warning_errno(errno, Failed to
enable hardware watchdog: %m);
+if (r 0) {
+ /* ENOTTY means the watchdog is always
enabled so we're fine */
multiples of 8 space indenting, please.
+log_full(errno == ENOTTY ? LOG_DEBUG :
LOG_WARNING,
+ Failed to enable hardware
watchdog: %m);
+if (errno != ENOTTY)
+ return errno;
multiples of 8 space indenting, please.
+}
r = ioctl(watchdog_fd, WDIOC_KEEPALIVE, 0);
if (r 0)
Both fixed, I'll resubmit shortly, sorry again and thanks for the
review.
--
Jean Delvare
SUSE L3 Support
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH v2] watchdog: Don't require WDIOC_SETOPTIONS/WDIOS_ENABLECARD
On Wed, 17.06.15 11:26, Jean Delvare (jdelv...@suse.de) wrote:
Not all watchdog drivers implement WDIOC_SETOPTIONS. Drivers which do
not implement it have their device always enabled. So it's fine to
report an error if WDIOS_DISABLECARD is passed and the ioctl is not
implemented, however failing when WDIOS_ENABLECARD is passed and the
ioctl is not implemented is not good: if the device was already
enabled then WDIOS_ENABLECARD was a no-op and wasn't needed in the
first place. So we can just ignore the error and continue.
---
Lennart, would that be OK with you?
Generally yes.
But: indentation is borked, there some whitespace issues, I cannot
even apply this.
Please have a look at CODING_STYLE.
Also, please build the stuff locally before commit and send to the
ML. We have a git commit hook that checks for weird whitespace at time
of commit. It's enables as soon as you run autogen.sh once...
Changes since v1:
* Log WDIOC_SETOPTIONS/WDIOS_ENABLECARD failure at debug level if
ENOTTY is returned. Suggested by Lennart Poettering.
src/shared/watchdog.c |9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
--- systemd.orig/src/shared/watchdog.c2015-06-15 18:51:40.811989627
+0200
+++ systemd/src/shared/watchdog.c 2015-06-17 11:20:01.798331790 +0200
@@ -60,8 +60,13 @@ static int update_timeout(void) {
flags = WDIOS_ENABLECARD;
r = ioctl(watchdog_fd, WDIOC_SETOPTIONS, flags);
-if (r 0)
-return log_warning_errno(errno, Failed to enable
hardware watchdog: %m);
+if (r 0) {
+ /* ENOTTY means the watchdog is always enabled
so we're fine */
multiples of 8 space indenting, please.
+log_full(errno == ENOTTY ? LOG_DEBUG : LOG_WARNING,
+ Failed to enable hardware watchdog: %m);
+if (errno != ENOTTY)
+ return errno;
multiples of 8 space indenting, please.
+}
r = ioctl(watchdog_fd, WDIOC_KEEPALIVE, 0);
if (r 0)
--
Jean Delvare
SUSE L3 Support
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2] watchdog: Don't require WDIOC_SETOPTIONS/WDIOS_ENABLECARD
Not all watchdog drivers implement WDIOC_SETOPTIONS. Drivers which do
not implement it have their device always enabled. So it's fine to
report an error if WDIOS_DISABLECARD is passed and the ioctl is not
implemented, however failing when WDIOS_ENABLECARD is passed and the
ioctl is not implemented is not good: if the device was already
enabled then WDIOS_ENABLECARD was a no-op and wasn't needed in the
first place. So we can just ignore the error and continue.
---
Lennart, would that be OK with you?
Changes since v1:
* Log WDIOC_SETOPTIONS/WDIOS_ENABLECARD failure at debug level if
ENOTTY is returned. Suggested by Lennart Poettering.
src/shared/watchdog.c |9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
--- systemd.orig/src/shared/watchdog.c 2015-06-15 18:51:40.811989627 +0200
+++ systemd/src/shared/watchdog.c 2015-06-17 11:20:01.798331790 +0200
@@ -60,8 +60,13 @@ static int update_timeout(void) {
flags = WDIOS_ENABLECARD;
r = ioctl(watchdog_fd, WDIOC_SETOPTIONS, flags);
-if (r 0)
-return log_warning_errno(errno, Failed to enable
hardware watchdog: %m);
+if (r 0) {
+ /* ENOTTY means the watchdog is always enabled so we're
fine */
+log_full(errno == ENOTTY ? LOG_DEBUG : LOG_WARNING,
+ Failed to enable hardware watchdog: %m);
+if (errno != ENOTTY)
+ return errno;
+}
r = ioctl(watchdog_fd, WDIOC_KEEPALIVE, 0);
if (r 0)
--
Jean Delvare
SUSE L3 Support
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
