From: Bruce A. <ba...@di...> - 2003-01-14 16:20:17
|
Hi Tim, > Here's an attempt for a new feature. It's partially tested > (works for me). > > Note: Patch is against version 5.0.49 > > Purpose: To add a dump of "smartctl -a <device>" to any mail > message that smartd sends. > > Why? I find it useful to see warning messages with full context. > I cannot always log into the machine smartd has flagged up > because user may have booted it to MS Windows. Sounds very reasonable. As I said in an earlier email, as long as it's a user-controllable option, and turned off by default, I like it. > Caveats: Only uses /bin/mail (mailx package) as was originally used, > paths to /bin/mail and /usr/sbin/smartctl hardcoded in smartd.h . > No configuration option to enable - it's always on. I suggest we either fix this now or later with environment variables SMARTDSMARTCTL and SMARTDMAIL. Note that we may need to use a different member of the exec() family for this. > Implementation notes: > > I felt I should rewrite the mail sending bit not to use system() to > avoid problems with the output of smartctl -a confusing the > shell and maybe introducing undesireable behaviour. Good. I have used fork/exec anyway but am lazy. Anyway, using pipes to communicate is a good idea. It also means that we can get the stderr output of mail if there is a problem, and write it to syslog. > smartd.h and smartd.c now have helper functions added: > > 1 - int sendmail (const char* to, const char* subject, const char* body) > actually does the job, using a fork/exec/pipe > approach shamelessly lifted from the comp.unix.programmers (sp?) > FAQ. Fine. I need to get a copy of Stevens and check that it agrees with what comp.unix.programmers say, but I am sure it's reasonable. > 2 - int writeall (const int fd, const char* buf, const ssize_t buflen) > is a wrapper around write() to ensure all data is > stuffed down the pipe. > > 3 - ssize_t freadallmax (FILE* fd, char* buf, const ssize_t bufsize) > likewise wrapper for fread() > > 4 - int readsmartdata (const char* command, char* buf, const unsigned int bufsize) > uses popen() to run smartctl and stuff results into a string. > Should be buffer-overrun safe, but does not grow the buffer > if the output gets too long. (Could that happen??) Hmmm, I'll have to read your code carefully before giving an opinion. At the moment the output of smartctl can't exceed a certain length, because in the worst case it prints 5 ATA error structures of fixed length and 21 self-test error structures of fixed length. But in a future incarnation it will probably handle extended self-test and error logs that can be much bigger. So we should probably write a function that can handle arbitrary lengths. > Symantics of: > void printandmail(cfgfile *cfg, int which, int priority, char *fmt, ...) > changed to: > void printandmail(cfgfile *cfg, int which, int priority, const char* device, char *fmt, ...) > so I can get the device name every time. Hmmm I thought that the device name was always in cfg->name. But I remember that the pointer/info was in a couple of different places, so perhaps it's necessary to include this device name separately as well. > TODO (ie what's wrong with this): > > 1 - Maybe I have mis-placed the code - it may be better > not to mess with printandmail() and put the call to > smartctl in the caller?? I'll have to look more closely at what you've done so far. > 2 - Needs an extra smartd.conf option. Easy -- let's just modify/extend the arguments to -M as I proposed in an earlier message. > 3 - Mail handling is still kind of tied to /bin/mail. > Not going to get into a debate about this, save that > perhaps unix needs a common mail-send library with a > one off config and interfaces to libesmtp, /bin/mail, sendmail > etc. But that's another discussion for another place. > I tend to put full paths in everywhere, because I'm > paranoid about root processes - just my way. Perhaps > these could be derived at build time automagically. Do you like the idea of just controlling the path with an environment variable? > BUGS - Very likely. I said before, I'm a perl programmer > and I'm tainted now. Sorry. In looking through it, the only specific comments were: -- cfg->name (I think...) can replace one argument. -- printf() shouldn't be used. The printout function is designed to redirect output to either syslog or terminal depending upon the users command line options. I saw this in sendmail(). Likewise there is a replacement for perror() called syserror() that "does the right thing" with the output. -- Have you looked closely at the signal handling to be sure that it won't interfere with the existing use/handling of USR1, HUP and TERM? -- might be nice to have a pipe for stderr as well, so if mail has trouble we can report the error back to the user. In general I thought that the code looked very clean. Cheers, Bruce |
From: Bruce A. <ba...@di...> - 2003-01-27 14:15:59
|
Hi Guido, > > It's a different approach to the one that the two of you were going to > > persue. I haven't thought enough about it to have a strong opinion if it's > > more useful -- it might be nice to support both alternatives I suppose. > I wasn't aware that this is so much different from the things Tim and > Phil are implementing, otherwise I'd actually forwarded this to the list > - (seems I didn't read the archives carefully enough, sorry). I've thought abouT it a bit more since last night. In fact I think that Jamie's wish might in fact be accomodated under Tim and Phil's scheme, if they make one additional change. They would need to modify the code to EXPORT a number of environment variables before calling the mail program. These exported variables might include: SMARTDPATH (example: /dev/hda) SMARTDFAILURE (example: 2, meaning, failed smart usage attribute) SMARTDCOUNT (example: 3, number of times this problem reported) SMARTDFIRSTTIME (example: 724125622, unix epoch problem first reported) SMARTDRECIPIENT (example: root@localhost) SMARTDTYPE (example: ata) ... Then, since smartd will use the environment variable: SMARTDMAIL (by default /bin/mail but Jamie can set it to /usr/local/bin/myscript) to decide what mail program to run, Jamie could set SMARTDMAIL to point to the script that he wants to run, and make this script ignore it's command line arguments and stdin. [The command line arguments would be things like '-s "subject" root@localhost' and stdin is the body of the mail message.] This would allow the user (Jamie) to execute an arbitrary script, and by making the script use the environment variables listed above, it would had access to all the relevant info. > BTW you can "subscribe" to the bug reports filed against the Debian > package via > http://packages.qa.debian.org/s/smartmontools.html > so you get copies of each bug filed. I'll do this, thanks! I found reading the debian bug reports for smartsuite very useful. It had a big influence on how I rewrote parts of the code to make smartmontools. Cheers, Bruce |
From: Guido G. <ag...@si...> - 2003-01-27 15:26:27
|
Hi Bruce, On Mon, Jan 27, 2003 at 08:15:55AM -0600, Bruce Allen wrote: > In fact I think that Jamie's wish might in fact be accomodated under Tim > and Phil's scheme, if they make one additional change. They would need > to modify the code to EXPORT a number of environment variables before > calling the mail program. These exported variables might include: > > SMARTDPATH (example: /dev/hda) Can we call this SMARTD_DEVICE or SMARTD_DEV? I kind of associate SMARTDPATH with the location where smartd is located in the filesystem - but that's nitpicking. > Then, since smartd will use the environment variable: SMARTDMAIL (by > default /bin/mail but Jamie can set it to /usr/local/bin/myscript) to > decide what mail program to run, Jamie could set SMARTDMAIL to point > to the script that he wants to run, and make this script ignore it's > command line arguments and stdin. [The command line arguments would be > things like '-s "subject" root@localhost' and stdin is the body of the > mail message.] This would allow the user (Jamie) to execute an arbitrary > script, and by making the script use the environment variables listed > above, it would had access to all the relevant info. Hmm...wouldn't it be cleaner to rename SMARTD_MAIL to SMARTD_SCRIPT then and provide an example SMARTD_SCRIPT that sends the same mail as smartd currently does? This would actually make smartd simpler and move non crucial parts out of the daemon. Or said the other way around - must a daemon really send mail, can't it just call an external script that does: !/bin/bash cat <<EOF | mail -s "$SMARTD_DEV reported an error" $SMARTD_RECIPIENT $SMARTD_DRIVE drive $SMARTD_DEV failed with $SMARTD_ERROR. EOF (I added underscores to the env vars to increasre readability). The final script should be a bit more clever though. Sorry if this has been discussed before. Regards, -- Guido |
From: Bruce A. <ba...@di...> - 2003-01-27 15:24:18
|
Hi Guido, > On Mon, Jan 27, 2003 at 08:15:55AM -0600, Bruce Allen wrote: > > In fact I think that Jamie's wish might in fact be accomodated under Tim > > and Phil's scheme, if they make one additional change. They would need > > to modify the code to EXPORT a number of environment variables before > > calling the mail program. These exported variables might include: > > > > SMARTDPATH (example: /dev/hda) > Can we call this SMARTD_DEVICE or SMARTD_DEV? I kind of associate > SMARTDPATH with the location where smartd is located in the filesystem - > but that's nitpicking. That's fine with me. I was mostly trying to explain the idea -- I agree that some more thought should go into the names and the naming convention. > > Then, since smartd will use the environment variable: SMARTDMAIL (by > > default /bin/mail but Jamie can set it to /usr/local/bin/myscript) to > > decide what mail program to run, Jamie could set SMARTDMAIL to point > > to the script that he wants to run, and make this script ignore it's > > command line arguments and stdin. [The command line arguments would be > > things like '-s "subject" root@localhost' and stdin is the body of the > > mail message.] This would allow the user (Jamie) to execute an arbitrary > > script, and by making the script use the environment variables listed > > above, it would had access to all the relevant info. > Hmm...wouldn't it be cleaner to rename SMARTD_MAIL to SMARTD_SCRIPT then > and provide an example SMARTD_SCRIPT that sends the same mail as smartd > currently does? This would actually make smartd simpler and move non > crucial parts out of the daemon. Or said the other way around - must a > daemon really send mail, can't it just call an external script that > does: I did think about this, and I'm oppposed to it. I would like smartd to remain simple to use for the "ordinary" user. So by default I would like the -m option of smartd to "just send mail" using /bin/mail if SMARTDMAIL is not set by the user. I don't want this to have to rely on a script that the user has to understand or even copy to a standard location. But, this still allows the "sophisticated user" to do exactly what Jamie wants. The script that they write will receive some command line arguments that would normally go to /bin/mail (which they can use or ignore at their pleasure) and it will also receive some stdin, that would normally be the body of the mail message (that again, they can use or ignore at their pleasure). Guido, while I am against your suggestion of requiring a script file to get the mail functionality, this does suggest to me another alternative to implement Tim's original idea of getting the output of smartctl -a added to the mail message. Instead of building this into smartd, Tim could simply provide a script that (1) sends the mail and (2) adds the smartctl -a output to the mail message. Tim, what do you think of this idea? It would actually simplify smartd, by eliminating the -M exec option entirely. You would simply set SMARTDMAIL=/path/to/my/custom/script with a script something like this: !#/bin/bash # save body of mail message produced by smartd # by redirecting stdin to a temporary file: cat > /tmp/mailmessage # add output of smartctl -a to mail message: /usr/sbin/smartctl -a SMARTD_DRIVE >> /tmp/mailmessage # send mail message (re-use command line arguments): /bin/mail $@ < /tmp/mailmessage # remove temp file: rm -f /tmp/mailmessage You could of course use this to run sg_utils or whatever other executables you wanted as well. It seems to me like a good solution, that simplifies smartd and still enables sophisticated users to do what they want, easily. Cheers, Bruce |
From: Bruce A. <ba...@di...> - 2003-01-27 15:31:02
|
A couple small fixes to my script. I was sloppy: > Tim, what do you think of this idea? It would actually simplify smartd, by > eliminating the -M exec option entirely. You would simply set > SMARTDMAIL=/path/to/my/custom/script > with a script something like this: > > !#/bin/bash #! /bin/bash > # save body of mail message produced by smartd > # by redirecting stdin to a temporary file: > cat > /tmp/mailmessage > > # add output of smartctl -a to mail message: > /usr/sbin/smartctl -a SMARTD_DRIVE >> /tmp/mailmessage /usr/sbin/smartctl -a $SMARTD_DRIVE >> /tmp/mailmessage > # send mail message (re-use command line arguments): > /bin/mail $@ < /tmp/mailmessage > > # remove temp file: > rm -f /tmp/mailmessage |
From: Guido G. <ag...@si...> - 2003-01-27 16:32:49
|
On Mon, Jan 27, 2003 at 09:31:00AM -0600, Bruce Allen wrote: > > cat > /tmp/mailmessage > > > > # add output of smartctl -a to mail message: > > /usr/sbin/smartctl -a SMARTD_DRIVE >> /tmp/mailmessage > > /usr/sbin/smartctl -a $SMARTD_DRIVE >> /tmp/mailmessage We should use a directory where only root can write to here, otherwise were open to symlink attacks on /tmp. Regards, -- Guido |
From: Bruce A. <ba...@di...> - 2003-01-27 16:18:59
|
Hi Guido, > On Mon, Jan 27, 2003 at 09:31:00AM -0600, Bruce Allen wrote: > > > cat > /tmp/mailmessage > > > > > > # add output of smartctl -a to mail message: > > > /usr/sbin/smartctl -a SMARTD_DRIVE >> /tmp/mailmessage > > > > /usr/sbin/smartctl -a $SMARTD_DRIVE >> /tmp/mailmessage > We should use a directory where only root can write to here, otherwise > were open to symlink attacks on /tmp. Yes, you are right. Is there a standard directory to use for this sort of thing? In general, do you agree with the approach I am advocating? I am in favor of it because it would give the "sophisticated user" the opportunity to do whatever they want, whereas the "ordinary user" could still get the mail message warnings easily without writing/intalling/maintaining additional scripts. I am curious what Tim and Phil will think of this... Cheers, Bruce |
From: Guido G. <ag...@si...> - 2003-01-27 17:59:23
|
On Mon, Jan 27, 2003 at 10:17:14AM -0600, Bruce Allen wrote: [..snip..] > > > /usr/sbin/smartctl -a $SMARTD_DRIVE >> /tmp/mailmessage > > We should use a directory where only root can write to here, otherwise > > were open to symlink attacks on /tmp. > > Yes, you are right. Is there a standard directory to use for this sort > of thing? it'd use /var/lib/smartmontools or /var/lib/smartd. > > In general, do you agree with the approach I am advocating? > > I am in favor of it because it would give the "sophisticated user" > the opportunity to do whatever they want, whereas the "ordinary > user" could still get the mail message warnings easily without > writing/intalling/maintaining additional scripts. Well I'd still opt for removing the mail code from smartd and use an external script instead. We could make the '-m' option use a supplied script /usr/lib/smartmontools/mail (default path compiled into the binary, gets installed with the standard 'install' target of the makefile) and add a new '-s, --script' option that allows to specify an individual script, so '-m' would merely be a shortcut for '-s /usr/lib/smartmontools/mail' with the env vars set. This would probably be a cleaner interface than to expect people to understand that '-m' could also use a script but you have to ignore commandline args (mail subject, etc) and rely on env vars. Regards, -- Guido |
From: Bruce A. <ba...@di...> - 2003-01-27 16:48:37
|
> Date: Mon, 27 Jan 2003 17:31:33 +0100 Hi Guido, Thanks for the fast replies. > > Yes, you are right. Is there a standard directory to use for this sort > > of thing? > it'd use /var/lib/smartmontools or /var/lib/smartd. Interesting. I didn't know that this was the standard place for an application/daemon to write temp files. > > In general, do you agree with the approach I am advocating? > > > > I am in favor of it because it would give the "sophisticated user" > > the opportunity to do whatever they want, whereas the "ordinary > > user" could still get the mail message warnings easily without > > writing/intalling/maintaining additional scripts. > Well I'd still opt for removing the mail code from smartd and use an > external script instead. We could make the '-m' option use a supplied > script /usr/lib/smartmontools/mail (default path compiled into the > binary, gets installed with the standard 'install' target of the > makefile) and add a new '-s, --script' option that allows to specify an > individual script, so '-m' would merely be a shortcut for > '-s /usr/lib/smartmontools/mail' with the env vars set. This would > probably be a cleaner interface than to expect people to understand that > '-m' could also use a script but you have to ignore commandline args > (mail subject, etc) and rely on env vars. I am worried about increasing the number of directories and executables used, and the complexity of installing/using the package. As usual there is some conflict between "generality" on the one hand and "simplicity" on the other. Let's see what Tim and Phil think. Cheers, Bruce |
From: Jamie H. <ja...@au...> - 2003-01-27 19:34:33
|
> I am worried about increasing the number of directories and executables > used, and the complexity of installing/using the package. As usual there > is some conflict between "generality" on the one hand and "simplicity" > on the other. Consider getting a copy Mike Gancarz's book. Many small programs that do discreet things isn't just good, its the unix way. /bin/mail shouldn't ever be used by external programs, infact, /bin/mail is a lousy piece of software that in an ideal world would simply be removed entirely. You don't need any temp files, fork, exec, close up shop and call it a day. -- Jamie Heilman http://audible.transient.net/~jamie/ "...thats the metaphorical equivalent of flopping your wedding tackle into a lion's mouth and flicking his lovespuds with a wet towel, pure insanity..." -Rimmer |
From: Phil W. <ph...@su...> - 2003-01-28 20:12:26
|
On Mon, Jan 27, 2003 at 10:43:12AM -0600, Bruce Allen wrote: > > > In general, do you agree with the approach I am advocating? > > > > > > I am in favor of it because it would give the "sophisticated user" > > > the opportunity to do whatever they want, whereas the "ordinary > > > user" could still get the mail message warnings easily without > > > writing/intalling/maintaining additional scripts. > > > Well I'd still opt for removing the mail code from smartd and use an > > external script instead. We could make the '-m' option use a supplied > > script /usr/lib/smartmontools/mail (default path compiled into the > > binary, gets installed with the standard 'install' target of the > > makefile) and add a new '-s, --script' option that allows to specify an > > individual script, so '-m' would merely be a shortcut for > > '-s /usr/lib/smartmontools/mail' with the env vars set. This would > > probably be a cleaner interface than to expect people to understand that > > '-m' could also use a script but you have to ignore commandline args > > (mail subject, etc) and rely on env vars. > > I am worried about increasing the number of directories and executables > used, and the complexity of installing/using the package. As usual there > is some conflict between "generality" on the one hand and "simplicity" > on the other. > > Let's see what Tim and Phil think. I'm all for adding a '-s' Directive to allow the user to specify an arbitrary script for smartd to run and I'm all for removing the (incomplete) '-M exec' Directive and requiring the user to write their own script if they want to be mailed the output of 'smartctl -a' or whatever. I also think it's worth keeping '-m' for users who just want a basic email warning when one of their drives starts doing something weird. I suspect that most smartd users would use this feature and that it is adequate for the vast majority of users (Bruce, do you think that's right?), so I would prefer that we don't force them to write (or modify) a script for this. We definitely need to allow the user to specify the mail program though (something I know Bruce has mentioned before). Phil |
From: Bruce A. <ba...@gr...> - 2003-01-29 04:21:44
|
Hi Phil, > I'm all for adding a '-s' Directive to allow the user to specify an > arbitrary script for smartd to run and I'm all for removing the > (incomplete) '-M exec' Directive and requiring the user to write their > own script if they want to be mailed the output of 'smartctl -a' or > whatever. In fact we can accomplish all of this with really tiny changes to the code, and without adding a "-s" option at all. We make the following modifications. [Note: the environment variables names below are off-the-cuff, and there may be a better choices of names]: (1) if SMARTD_MAIL is set, use it instead of /bin/mail as the program to execute. This lets the user execute an arbitrary script. (2) before running SMARTD_MAIL, export environment variables: SMARTD_DEVICE SMARTD_DEVICETYPE SMARTD_ERRORTYPE SMARTD_ERRORNUMBER ... I think that this satisfies ALL requirements, while making the changes to the code minimal. (A) 'Ordinary' users just run the code "as is". It's easy to use and requires no scripting, additional configuration, etc. (B) 'Sophisticated' users (such as Tim) who want additional output set SMARTD_MAIL to point to their own script such as the one below #! /bin/bash file=/only/writeable/by/root.$SMARTD_DEVICE cat > $file /usr/sbin/smartctl -a $SMARTD_DRIVE >> $file /bin/mail $@ < $file (C) 'Strange' users who want to do something *else* can set SMARTD_MAIL to point to a script that does whatever: #! /bin/bash echo -e \a echo -e \a echo -e \a wall 'smartd warning: shutdown in 20 secs' sleep 20 /sbin/shutdown -hf now [This beeps the console three times, issues a warning to all users, and powers off the machine!] It seems to me that this satisfies ALL requirements: -- allows users to easily send mail using current scheme that works well for most people -- lets users use a different path to mail if they want -- lets users write a customized script that adds additional info to the mail message if they want -- lets users execute an arbitrary script if they want, including creating custom mail, using custom mailers, etc. In addition, it requires very minimal changes to the code. To specify the executable path with the environment variable SMARTD_MAIL is trivial, and we can export the values of the other variables either by using export SMARTD_VAR1=... syntax in the script argument to system() or by "doing it properly" with fork/execle. > I also think it's worth keeping '-m' for users who just want a basic > email warning when one of their drives starts doing something weird. > I suspect that most smartd users would use this feature and that it is > adequate for the vast majority of users (Bruce, do you think that's > right?) Yes, I do. > so I would prefer that we don't force them to write (or modify) a > script for this. We definitely need to allow the user to specify the > mail program though (something I know Bruce has mentioned before). I completely agree with this. Tim, what do you think? Cheers, Bruce |
From: Bruce A. <ba...@gr...> - 2003-01-29 16:10:26
|
For fun, I decided to try implementing what I've suggested below. It added ~ 10 lines to smartd.c. I've tested it and checked it into CVS; we can back it out if this turns out to be a bad idea. You can see the code I've added with: cvs diff -r 1.100 -r 1.101 smartd.c If you just run smartd "as is" you will see no change from the previous behavior. It sends mail as governed by -m and -M options. If you first do: export SMARTD_MAILER=/bin/true (for example) before running ./smartd, this will use /bin/true as the mailer program, and you'll get no mail at all. If you then set (warning! Example only) export SMARTD_MAILER=/root/myscript where /root/myscript contains the four lines: #!/bin/bash cat > /root/normal_mail_message /usr/sbin/smartctl -a $SMARTD_DEVICE >> /root/normal_mail_message /bin/mail -s "SMART errors detected on host: `hostname`" $SMARTD_ADDRESS < /root/normal_mail_message > /dev/null 2> /dev/null you will get the output of smartctl -a appended to the mailer output, which is what Tim had originally wanted. Note that the code currently exports the environment variables: > setenv("SMARTD_DEVICE", cfg->name, 1); > setenv("SMARTD_MESSAGE", message, 1); > setenv("SMARTD_ADDRESS", address, 1); > setenv("SMARTD_TFIRST", ctime(&(mail->firstsent)), 1); that can be used in a script. Additional variables can be added with one extra line per variable in smartd.c If we want the user to be able to run a different script for each device then we might want to retain the -M exec option, rather than using the environment variable SMARTD_MAILER. Or we could leave this up to the user, since of course the script can test the value of SMARTD_DEVICE to see which device is being used... The rule for writing scripts is the following: -- script should ignore any command-line arguments -- script receives the "normal" mail message on stdin. It can use this, ignore it, or dispose of it, as desired. -- Anything that the script writes to stdout or stderr is lost. So any output from the script should be mailed, logged to syslog, or written to a log file. -- script should return exit status 0 if all is well, else non-zero exit status if there is a problem This seems to me a simple change to the code that gives the user complete flexibility while maintaining ease-of-use for most people. Comments? Bruce On Tue, 28 Jan 2003, Bruce Allen wrote: > Hi Phil, > > > I'm all for adding a '-s' Directive to allow the user to specify an > > arbitrary script for smartd to run and I'm all for removing the > > (incomplete) '-M exec' Directive and requiring the user to write their > > own script if they want to be mailed the output of 'smartctl -a' or > > whatever. > > In fact we can accomplish all of this with really tiny changes to the > code, and without adding a "-s" option at all. We make the following > modifications. [Note: the environment variables names below are > off-the-cuff, and there may be a better choices of names]: > > (1) if SMARTD_MAIL is set, use it instead of /bin/mail as the program > to execute. This lets the user execute an arbitrary script. > > (2) before running SMARTD_MAIL, export environment variables: > SMARTD_DEVICE > SMARTD_DEVICETYPE > SMARTD_ERRORTYPE > SMARTD_ERRORNUMBER > ... > > I think that this satisfies ALL requirements, while making the changes to > the code minimal. > > (A) 'Ordinary' users just run the code "as is". It's easy to use and > requires no scripting, additional configuration, etc. > > (B) 'Sophisticated' users (such as Tim) who want additional output > set SMARTD_MAIL to point to their own script such as the one below > > #! /bin/bash > file=/only/writeable/by/root.$SMARTD_DEVICE > cat > $file > /usr/sbin/smartctl -a $SMARTD_DRIVE >> $file > /bin/mail $@ < $file > > (C) 'Strange' users who want to do something *else* can set > SMARTD_MAIL to point to a script that does whatever: > > #! /bin/bash > echo -e \a > echo -e \a > echo -e \a > wall 'smartd warning: shutdown in 20 secs' > sleep 20 > /sbin/shutdown -hf now > > [This beeps the console three times, issues a warning to > all users, and powers off the machine!] > > It seems to me that this satisfies ALL requirements: > > -- allows users to easily send mail using current scheme that works well > for most people > -- lets users use a different path to mail if they want > -- lets users write a customized script that adds additional info > to the mail message if they want > -- lets users execute an arbitrary script if they want, including > creating custom mail, using custom mailers, etc. > > In addition, it requires very minimal changes to the code. > To specify the executable path with the environment variable SMARTD_MAIL > is trivial, and we can export the values of the other variables either by > using > export SMARTD_VAR1=... > syntax in the script argument to system() or by "doing it properly" with > fork/execle. > > > I also think it's worth keeping '-m' for users who just want a basic > > email warning when one of their drives starts doing something weird. > > I suspect that most smartd users would use this feature and that it is > > adequate for the vast majority of users (Bruce, do you think that's > > right?) > > Yes, I do. > > > so I would prefer that we don't force them to write (or modify) a > > script for this. We definitely need to allow the user to specify the > > mail program though (something I know Bruce has mentioned before). > > I completely agree with this. > > Tim, what do you think? > > Cheers, > Bruce > > > > ------------------------------------------------------- > This SF.NET email is sponsored by: > SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See! > http://www.vasoftware.com > _______________________________________________ > Smartmontools-support mailing list > Sma...@li... > https://lists.sourceforge.net/lists/listinfo/smartmontools-support > |
From: Phil W. <ph...@su...> - 2003-01-29 20:22:32
|
Hi Bruce, > > I'm all for adding a '-s' Directive to allow the user to specify an > > arbitrary script for smartd to run and I'm all for removing the > > (incomplete) '-M exec' Directive and requiring the user to write their > > own script if they want to be mailed the output of 'smartctl -a' or > > whatever. > > In fact we can accomplish all of this with really tiny changes to the > code, and without adding a "-s" option at all. We make the following > modifications. [Note: the environment variables names below are > off-the-cuff, and there may be a better choices of names]: I still think we should add a '-s' directive. It would require very little more work than what you have suggested and it would make the interface and documentation cleaner. Phil |
From: Bruce A. <ba...@gr...> - 2003-01-29 21:41:54
|
Hi Phil, > > > I'm all for adding a '-s' Directive to allow the user to specify an > > > arbitrary script for smartd to run and I'm all for removing the > > > (incomplete) '-M exec' Directive and requiring the user to write their > > > own script if they want to be mailed the output of 'smartctl -a' or > > > whatever. > > > > In fact we can accomplish all of this with really tiny changes to the > > code, and without adding a "-s" option at all. We make the following > > modifications. [Note: the environment variables names below are > > off-the-cuff, and there may be a better choices of names]: > > I still think we should add a '-s' directive. It would require very little > more work than what you have suggested and it would make the interface > and documentation cleaner. I guess that this isn't clear to me. What I don't understand is if '-s' works in conjunction with, in parallel with, or instead of, the existing "mailing" routines. In other words, when is the executable pointed to by the "-s" argument actually called? And by what routine within smartd.c? Perhaps if you could be a bit more detailed about what you have in mind for "inside the code" it would be clearer to me. Or if it's simple and easy just go ahead and check it into CVS and I'll have a look. Cheers, Bruce |
From: Phil W. <ph...@su...> - 2003-01-29 22:37:31
|
Hi Bruce, > > I still think we should add a '-s' directive. It would require very little > > more work than what you have suggested and it would make the interface > > and documentation cleaner. > > I guess that this isn't clear to me. What I don't understand is if '-s' > works in conjunction with, in parallel with, or instead of, the existing > "mailing" routines. In other words, when is the executable pointed to by > the "-s" argument actually called? And by what routine within smartd.c? > > Perhaps if you could be a bit more detailed about what you have in mind > for "inside the code" it would be clearer to me. Or if it's simple and > easy just go ahead and check it into CVS and I'll have a look. Well, I haven't actually looked at the relevant code, but I'm assuming it's very little work :-) What I meant was: - '-m' will work exactly as it does now (well, as it did before your last check-in), except that the user will be able to specify the mail program, probably by setting an environment variable - there will be a new directive, '-s' (or whatever), that will accept as an argument an arbitrary string (presumed to be the path to a script) - immediately after the point in the code that a mail is sent to the user (whether we send mail or not), we attempt to execute the specified script (if any), making sure to arrange that the appropriate environment variables are set for that process (SMARTD_DEVICE, SMARTD_TYPE, etc.) So '-m' would provide a basic email warning and for anything else the user would have to write a script to do it. If they wanted a more informative email message (for example, including the output of 'smartctl -a'), they would have to write a script to do it and use '-s'. Similiarly, if they wanted to shut down the machine at the first hint of trouble, they'd write a script to do it and use '-s' just the same. Phil |
From: Bruce A. <ba...@gr...> - 2003-01-29 23:09:24
|
Hi Phil, Thanks very much for the clarification! > > Perhaps if you could be a bit more detailed about what you have in mind > > for "inside the code" it would be clearer to me. Or if it's simple and > > easy just go ahead and check it into CVS and I'll have a look. > > Well, I haven't actually looked at the relevant code, but I'm assuming it's > very little work :-) Looking ahead at what you said, I think that this is probably true! > What I meant was: > > - '-m' will work exactly as it does now (well, as it did before your last > check-in), except that the user will be able to specify the mail program, > probably by setting an environment variable OK, so we use SMARTD_MAILER for that, as in the current code. Easy. > - there will be a new directive, '-s' (or whatever), that will accept as an > argument an arbitrary string (presumed to be the path to a script) OK, clear enough. > - immediately after the point in the code that a mail is sent to the user > (whether we send mail or not), we attempt to execute the specified script > (if any), making sure to arrange that the appropriate environment variables > are set for that process (SMARTD_DEVICE, SMARTD_TYPE, etc.) So for example if the user does not want to send mail, then if I understand you correctly they must set SMARTD_MAILER to point to /bin/true, so that nothing happens. Then for example Tim has to write a program that generates the entire mail message himself, including the body of it, using all the SMARTD_* env variables. Because of course the input that normally would go to SMARTD_MAILER has been sent to /dev/null by /bin/true. Something that bugs me about this is that with your suggested way of coding this, the "-s" option is HEAVILY dependent upon the logic of the -m and -M options. Yet it appears to be a completely independent option. > So '-m' would provide a basic email warning and for anything else the > user would have to write a script to do it. If they wanted a more > informative email message (for example, including the output of > 'smartctl -a'), they would have to write a script to do it and use > '-s'. Similiarly, if they wanted to shut down the machine at the > first hint of trouble, they'd write a script to do it and use '-s' > just the same. I guess at this point I don't see what this gains over the code that I have already implemented. Here's a comparison: WITH -s OPTION: -- just get email -- trivial, leave off option -- get mail with different mailer: point SMARTD_MAILER to new mailer -- add smartctl output to mail: point SMARTD_MAILER to /bin/true point -s option to #!/bin/bash echo $SMARTD_MESSAGE > /tmp/smartdfile /usr/sbin/smartctl -a $SMARTD_DEVICE >> /tmp/smartdfile /bin/mail -s "SMART errors detected on host: `hostname`" $SMARTD_ADDRESS < /tmp/smartdfile > /dev/null 2> /dev/null exit 0 -- reboot machine point SMARTD_MAILER to /bin/true point -s to script: #! /bin/bash /sbin/shutdown -hf now WITHOUT -s OPTION: -- just get email -- trivial, don't define SMARTD_MAILER -- get mail with different mailer: point SMARTD_MAILER to new mailer -- add smartctl output to mail: point SMARTD_MAILER to: #!/bin/bash cat > /tmp/smartdfile /usr/sbin/smartctl -a $SMARTD_DEVICE >> /tmp/smartdfile /bin/mail -s "SMART errors detected on host: `hostname`" $SMARTD_ADDRESS < /tmp/smartdfile > /dev/null 2> /dev/null exit 0 -- reboot machine point SMARTD_MAILER to #! /bin/bash /sbin/shutdown -hf now In each case it seems to me that it's simpler to do it without the "-s" option. The problem I see with "-s" is that in the implementation you describe, it's intimately tied up with the -m and -M options, not independent of them. Can you suggest a situation where the -s proposal simplifies rather than complicates matters, as above? Cheers, Bruce |
From: Phil W. <ph...@su...> - 2003-01-29 23:47:39
|
Hi Bruce, > > - immediately after the point in the code that a mail is sent to the user > > (whether we send mail or not), we attempt to execute the specified script > > (if any), making sure to arrange that the appropriate environment variables > > are set for that process (SMARTD_DEVICE, SMARTD_TYPE, etc.) > > So for example if the user does not want to send mail, then if I > understand you correctly they must set SMARTD_MAILER to point to > /bin/true, so that nothing happens. They wouldn't use '-m' and wouldn't set SMARTD_MAILER, but would use '-s'. > Then for example Tim has to write a program that generates the entire mail > message himself, including the body of it, using all the SMARTD_* env > variables. Because of course the input that normally would go to > SMARTD_MAILER has been sent to /dev/null by /bin/true. Yes, apart from the last line (Tim presumably wouldn't use -m so the mail code wouldn't be executed). > Something that bugs me about this is that with your suggested way of > coding this, the "-s" option is HEAVILY dependent upon the logic of the -m > and -M options. Yet it appears to be a completely independent > option. I've not really been thinking about this from a coding perspective, but I don't think there are any serious coding issues. Let me summarise from my perspective: - if a failure is detected then smartd will take some sort of action - it is up to the user to specify this action by providing a script - smartd can be configured to repeat this action at predefined intervals - in the one very common case of sending a warning email, an additional directive, '-m', is provided for the user's convenience The documentation and naming of the directives would have to be updated to reflect this approach. Ideally the code would reflect this as well. > > So '-m' would provide a basic email warning and for anything else the > > user would have to write a script to do it. If they wanted a more > > informative email message (for example, including the output of > > 'smartctl -a'), they would have to write a script to do it and use > > '-s'. Similiarly, if they wanted to shut down the machine at the > > first hint of trouble, they'd write a script to do it and use '-s' > > just the same. > > I guess at this point I don't see what this gains over the code that I > have already implemented. Here's a comparison: > > WITH -s OPTION: > -- just get email -- trivial, leave off option > -- get mail with different mailer: > point SMARTD_MAILER to new mailer > -- add smartctl output to mail: > point SMARTD_MAILER to /bin/true > > point -s option to > #!/bin/bash > echo $SMARTD_MESSAGE > /tmp/smartdfile > /usr/sbin/smartctl -a $SMARTD_DEVICE >> /tmp/smartdfile > /bin/mail -s "SMART errors detected on > host: `hostname`" $SMARTD_ADDRESS < > /tmp/smartdfile > /dev/null 2> /dev/null > exit 0 > -- reboot machine > point SMARTD_MAILER to /bin/true > point -s to script: > #! /bin/bash > /sbin/shutdown -hf now > > > > WITHOUT -s OPTION: > -- just get email -- trivial, don't define SMARTD_MAILER > -- get mail with different mailer: > point SMARTD_MAILER to new mailer > -- add smartctl output to mail: > point SMARTD_MAILER to: > #!/bin/bash > cat > /tmp/smartdfile > /usr/sbin/smartctl -a $SMARTD_DEVICE >> /tmp/smartdfile > /bin/mail -s "SMART errors detected on > host: `hostname`" $SMARTD_ADDRESS < > /tmp/smartdfile > /dev/null 2> /dev/null > exit 0 > -- reboot machine > point SMARTD_MAILER to > #! /bin/bash > /sbin/shutdown -hf now > > > In each case it seems to me that it's simpler to do it without the "-s" > option. The problem I see with "-s" is that in the implementation you > describe, it's intimately tied up with the -m and -M options, not > independent of them. I hope I've answered this above. > Can you suggest a situation where the -s proposal simplifies rather than > complicates matters, as above? Looking at it from the user's perspective: Suppose the user wants to shut down the system if there is any indication of a problem on /dev/hda. Then using your suggested implementation they would write a script (/foo/bar, say) and add a line to smartd.conf like /dev/hda -m bogus@ddress and set SMARTD_MAILER to /foo/bar Using the implementation I just described, they would just have to add a line like: /dev/hda -s /foo/bar ... Which is much more intuitive. I don't think there are any significant differences between implementation complexity in the two approaches. Phil |
From: Bruce A. <ba...@gr...> - 2003-01-30 12:24:47
|
Hi Phil, > > > - immediately after the point in the code that a mail is sent to the user > > > (whether we send mail or not), we attempt to execute the specified script > > > (if any), making sure to arrange that the appropriate environment variables > > > are set for that process (SMARTD_DEVICE, SMARTD_TYPE, etc.) > > > > So for example if the user does not want to send mail, then if I > > understand you correctly they must set SMARTD_MAILER to point to > > /bin/true, so that nothing happens. > > They wouldn't use '-m' and wouldn't set SMARTD_MAILER, but would use '-s'. Here's where I have a problem. This description is NOT consistent with your description of the implementation. You described the implementation as "put the exec right after the point where mail is sent". But the problem is that without -m turned on, the routine in which the mail is sent is never called. So the "-s" argument would never get called. > > Then for example Tim has to write a program that generates the entire mail > > message himself, including the body of it, using all the SMARTD_* env > > variables. Because of course the input that normally would go to > > SMARTD_MAILER has been sent to /dev/null by /bin/true. > > Yes, apart from the last line (Tim presumably wouldn't use -m so the mail code > wouldn't be executed). Again, this implies a *different* implementation than you described in your previous email. I don't see "how to do it" in a straightforward way. > > Something that bugs me about this is that with your suggested way of > > coding this, the "-s" option is HEAVILY dependent upon the logic of the -m > > and -M options. Yet it appears to be a completely independent > > option. > > I've not really been thinking about this from a coding perspective, > but I don't think there are any serious coding issues. Here's where I am less clear than you. If the "-s" option is not coded as part of the mailing routine, then the implementation of it requires introducing additional global data structures for storing when the routine is called, and under what circumstances. > > In each case it seems to me that it's simpler to do it without the "-s" > > option. The problem I see with "-s" is that in the implementation you > > describe, it's intimately tied up with the -m and -M options, not > > independent of them. > > I hope I've answered this above. Well, yes and no. What you've described is an implementation that is "independent" of the existing printandmail() function. It's not equivalent to 'adding an exec of the "-s" argument right after mail gets sent'. > > Can you suggest a situation where the -s proposal simplifies rather than > > complicates matters, as above? > > Looking at it from the user's perspective: > > Suppose the user wants to shut down the system if there is any indication of > a problem on /dev/hda. Then using your suggested implementation they would > write a script (/foo/bar, say) and add a line to smartd.conf like > > /dev/hda -m bogus@ddress > > and set SMARTD_MAILER to /foo/bar Correct! > Using the implementation I just described, they would just have to add a line > like: > > /dev/hda -s /foo/bar ... > > Which is much more intuitive. But they'd have to do MUCH more!!! Since your suggested implementation of '-s' is independent of the '-m' implementation, they'd also need to set the switches like -S once -S test and so on that control when and how often this script is run. Then we'd need to add another layer of global variables and a whole body of routines (and corresponding bugs!) to smartd to implement this. If the main objection is that '-m' requires an address to turn it on, which the user might not need for their script, then there is another solution. How about we reintroduce the -M exec and allow the following possibilities: # just send email /dev/hda -m root@mydomain # send email via strange mailing script # note: in this case the mail script must get addresses, drive name, etc # from environment variables -- hence no %s type arguments! /dev/hda -m root@mydomain -M exec /my/custom/mail-script # send email using /bin/mail but which has a strange path /dev/hda -m root@mydomain -M exec /path/to/bin/mail # just execute weird script that doesn't send email at all /dev/hda -M exec /path/to/weird/script By the way, I am in favor of reintroducing -M exec because it allows the user to easily specify different "script" actions for each device. The user could do this using switch ($SMARTD_DEVICE){ ... } within the script but I think we ought to make it simpler than that. For example they might want different scripts for SCSI and ATA devices. > I don't think there are any significant differences between > implementation complexity in the two approaches. Here's where we disagree, I think. Can you send me or check in a proposed implementation? What I think is true is that if you want to implement '-s' by just adding something after the exising line that sends mail, you are going to be very closely bound to the various -M option arguments; it's not at all "independent" of the mailing code. Cheers, Bruce |
From: <ph...@su...> - 2003-01-30 15:11:43
|
Hi Bruce, sorry, I don't have time to reply in full now. > > > > - immediately after the point in the code that a mail is sent to the user > > > > (whether we send mail or not), we attempt to execute the specified script > > > > (if any), making sure to arrange that the appropriate environment variables > > > > are set for that process (SMARTD_DEVICE, SMARTD_TYPE, etc.) > > > > > > So for example if the user does not want to send mail, then if I > > > understand you correctly they must set SMARTD_MAILER to point to > > > /bin/true, so that nothing happens. > > > > They wouldn't use '-m' and wouldn't set SMARTD_MAILER, but would use '-s'. > > Here's where I have a problem. > > This description is NOT consistent with your description of the > implementation. You described the implementation as "put the exec right > after the point where mail is sent". But the problem is that without -m > turned on, the routine in which the mail is sent is never called. > > So the "-s" argument would never get called. OK, technically it's not consistent. I didn't mean that we actually execute the script on the next line of code after the mail is sent, just that conceptually it's the next thing smartd does. We would call a new routine after we have called the mail routine (or decided not to, if the user didn't use -m). > > > Then for example Tim has to write a program that generates the entire mail > > > message himself, including the body of it, using all the SMARTD_* env > > > variables. Because of course the input that normally would go to > > > SMARTD_MAILER has been sent to /dev/null by /bin/true. > > > > Yes, apart from the last line (Tim presumably wouldn't use -m so the mail code > > wouldn't be executed). > > Again, this implies a *different* implementation than you described in > your previous email. I don't see "how to do it" in a straightforward way. See my last comment (and the next). > > > Something that bugs me about this is that with your suggested way of > > > coding this, the "-s" option is HEAVILY dependent upon the logic of the -m > > > and -M options. Yet it appears to be a completely independent > > > option. > > > > I've not really been thinking about this from a coding perspective, > > but I don't think there are any serious coding issues. > > Here's where I am less clear than you. If the "-s" option is not coded as > part of the mailing routine, then the implementation of it requires > introducing additional global data structures for storing when the routine > is called, and under what circumstances. I actually imagined that the -M settings would be shared between -m and -s, though perhaps it makes more sense for them to be independent. Unless the -m and -s options are mutually exclusive (a renaming or something might be necessary for this to make sense to users). Phil |
From: Tim S. <ts...@do...> - 2003-01-30 15:33:43
|
Dear All, Sorry - my work life just went "bursty" - that happens randomly... I have been keeping up with the discussion - just no time to type anything rational :-( I'm going to answer in summary mode now for brevity but just on the subject of sending mail and/or calling user scripts. My opinion is that either -s option or SMARTD_MAILER together with the mail option both seem elegant and equally useful to advanced users. I also think that the use of environment variables is the best way to deal with the long standing problem of where OS dependent things like /bin/mail live. That deals with Jamie's problem - in the worst case you implement your own /bin/mail wrapper or call something else that works in a compatible way. Can't really do any better that that. I really don't feel I should have a strong opinion on the "look" of the command (using -s or otherwise) - I haven't been around long enough - so I'll go with the flow on this I'll pull the CVS tree down very soon and have a play with some of the ideas. Cheers Tim -- Mr Tim J Southerwood CSG, Dept of Computing, Imperial College, London Email personal: ts...@di... work: ts...@do... Tel home: 020-866-17388 mobile: 07949-487179 work: 020-759-48392 |
From: Bruce A. <ba...@gr...> - 2003-01-30 16:34:33
|
Hi Tim, > I'll pull the CVS tree down very soon and have a play with some of the ideas. I'm in bursty mode too! Give me a few hours and perhaps then you can test (and if needed, fix!) the code I've written to do this, please Cheers, Bruce |
From: Bruce A. <ba...@gr...> - 2003-01-30 16:47:35
|
Hi Phil, > > > > > - immediately after the point in the code that a mail is sent to the user > > > > > (whether we send mail or not), we attempt to execute the specified script > > > > > (if any), making sure to arrange that the appropriate environment variables > > > > > are set for that process (SMARTD_DEVICE, SMARTD_TYPE, etc.) > > > > > > > > So for example if the user does not want to send mail, then if I > > > > understand you correctly they must set SMARTD_MAILER to point to > > > > /bin/true, so that nothing happens. > > > > > > They wouldn't use '-m' and wouldn't set SMARTD_MAILER, but would use '-s'. > > > > Here's where I have a problem. > > > > This description is NOT consistent with your description of the > > implementation. You described the implementation as "put the exec right > > after the point where mail is sent". But the problem is that without -m > > turned on, the routine in which the mail is sent is never called. > > > > So the "-s" argument would never get called. > > OK, technically it's not consistent. I didn't mean that we actually > execute the script on the next line of code after the mail is sent, > just that conceptually it's the next thing smartd does. We would call > a new routine after we have called the mail routine (or decided not > to, if the user didn't use -m). OK, I understand now what you are suggesting. I've just implemeneted it and am testing it. Though I call it "-M exec" not "-s" because of the way that it is controlled by the other -M options. Here's how it works: If the user does not have -M exec then everything works as currently. If the user has -M exec then there are two cases: (1) The user has set -m address. In THIS case, the program pointed by exec is executed, with the normal mail message fed into stdin, For this case, the user should ONLY provide a script that does not use/require any command line arguments. [See below about env variables.] This makes it trivial to have a 3-line script that adds (for instance) the output of smartctl to the mail message. (2) If the user has NOT set -m address, then we simply execute their command within bash. Thus it can be an executable, a pipeline, or even a short script. Note that in both cases (1) and (2) I set a number of environment variables: "SMARTD_MAILER (this is OUTPUT not input now). It's either "mail" or the users argument to -M exec "SMARTD_DEVICE" "SMARTD_DEVICETYPE" "SMARTD_MESSAGE" "SMARTD_SUBJECT" "SMARTD_TFIRST" if -m has been provided then "SMARTD_ADDRESS" is also set I'll do some more testing, write a bit of documentation, then put this in CVS for further testing. > I actually imagined that the -M settings would be shared between -m > and -s, though perhaps it makes more sense for them to be independent. > Unless the -m and -s options are mutually exclusive (a renaming or > something might be necessary for this to make sense to users). I think the solution is simply to use -M exec, as I now do. Cheers, Bruce > |
From: Phil W. <ph...@su...> - 2003-01-30 22:10:31
|
Hi Bruce, > Here's how it works: > If the user does not have -M exec then everything works as currently. > > If the user has -M exec then there are two cases: > (1) The user has set -m address. In THIS case, the program pointed > by exec is executed, with the normal mail message fed into stdin, > For this case, the user should ONLY provide a script that does > not use/require any command line arguments. [See below about env > variables.] > This makes it trivial to have a 3-line script that adds (for > instance) the output of smartctl to the mail message. > (2) If the user has NOT set -m address, then we simply execute their > command within bash. Thus it can be an executable, a pipeline, or > even a short script. That sounds like a neat way of doing it. My only comment is that I don't think we should assume that the user has bash. > Note that in both cases (1) and (2) I set a number of environment > variables: > > "SMARTD_MAILER (this is OUTPUT not input now). It's either "mail" or the > users argument to -M exec My brain's not working right. Could you give me an example of what this would be used for? Phil |
From: Bruce A. <ba...@gr...> - 2003-01-30 22:41:08
|
> > (2) If the user has NOT set -m address, then we simply execute their > > command within bash. Thus it can be an executable, a pipeline, or > > even a short script. > > That sounds like a neat way of doing it. My only comment is that I > don't think we should assume that the user has bash. In fact the command is executed by system() so it uses /bin/sh. If that doesn't exist then it uses whatever else libc provides. The point is, it does provide an environment no matter what shell(s) your system has or lacks. > > Note that in both cases (1) and (2) I set a number of environment > > variables: > > > > "SMARTD_MAILER (this is OUTPUT not input now). It's either "mail" or the > > users argument to -M exec > > My brain's not working right. Could you give me an example of what > this would be used for? Ummm, actually I can't. But I thought that it might be useful to someone writing a script, to know exactly what had been specified on the smartd.conf command line. I'll download some code/documentation real soon now (TM). Cheers, Bruce |