[systemd-devel] [PATCH] watchdog: Don't require WDIOC_SETOPTIONS/WDIOS_ENABLECARD

[Jean Delvare]

Hi Lennart,

Thanks for your quick reply.

Le Monday 15 June 2015 à 18:16 +0200, Lennart Poettering a écrit :
> On Mon, 15.06.15 18:14, Jean Delvare (jdelvare at 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.
> 
> Isn't this something that should be fixed in the drivers?

This is a legitimate question and I pondered it myself too. However the
fact that over 20 drivers do not implement WDIOC_SETOPTIONS, together
with the fact that it's named, well, "set options", give me the feeling
that not implementing it is legitimate.

> > ---
> >  src/shared/watchdog.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > --- a/src/shared/watchdog.c
> > +++ b/src/shared/watchdog.c
> > @@ -64,7 +64,8 @@ static int update_timeout(void) {
> >  
> >                  flags = WDIOS_ENABLECARD;
> >                  r = ioctl(watchdog_fd, WDIOC_SETOPTIONS, &flags);
> > -                if (r < 0) {
> > +                /* ENOTTY means the watchdog is always enabled so we're fine */
> > +                if (r < 0 && errno != ENOTTY) {
> >                          log_warning("Failed to enable hardware watchdog: %m");
> >                          return -errno;
> 
> If this is something to fix in systemd, rather than fix in the
> drivers:  am pretty sure that we should log in all cases, but change
> the log level to LOG_DEBUG if its ENOTTY. i.e. use "log_full(errno ==
> ENOTTY ? LOG_DEBUG : LOG_WARNING"...

Sure, I can try that. Updated patch coming (likely tomorrow.)

Thanks for the review,
-- 
Jean Delvare
SUSE L3 Support