| Message ID | ea11fea30810071044l6d8887e0udb52342f82412e4a@mail.gmail.com |
|---|---|
| State | Rejected, archived |
| Headers | show |
On Tue, Oct 07, 2008 at 11:14:11PM +0530, Manish Katiyar wrote: > Hi Ted, > > I am not sure why we wan't to background the logsave and keep retrying > opening the fd in case of failures. That's one of the main reason why logsave exists; the filesystem containing /var/log might not be mounted, or the root filesystem may be mounted read-only, and so the log file can't be written until the filesystem is remounted r/w or /var is mounted. > But there may be situations when we will never be able to succeed > and thus create unnecessary process. For example invoking it > > /home/mkatiyar/sbin> ./logsave /testfile ls The main use of logsave was in init.d scripts. So I didn't really worry about the permissoin denied case. Perhaps logsave should just fail hard and not even run the command if there is a permission denied error. That would certainly be simpler... > +static void should_background(int err, int *nobackground) { > + switch (err) { > + case EPERM: > + case EACCES: > + *nobackground = err; > + break; > + default : > + *nobackground = 0; > + } > + return ; > +} Why is this its own function? - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Oct 11, 2008 at 12:01 AM, Theodore Tso <tytso@mit.edu> wrote: > On Tue, Oct 07, 2008 at 11:14:11PM +0530, Manish Katiyar wrote: >> Hi Ted, >> >> I am not sure why we wan't to background the logsave and keep retrying >> opening the fd in case of failures. > > That's one of the main reason why logsave exists; the filesystem > containing /var/log might not be mounted, or the root filesystem may > be mounted read-only, and so the log file can't be written until the > filesystem is remounted r/w or /var is mounted. > >> But there may be situations when we will never be able to succeed >> and thus create unnecessary process. For example invoking it >> >> /home/mkatiyar/sbin> ./logsave /testfile ls > > The main use of logsave was in init.d scripts. So I didn't really > worry about the permissoin denied case. Perhaps logsave should just > fail hard and not even run the command if there is a permission denied > error. That would certainly be simpler... > >> +static void should_background(int err, int *nobackground) { >> + switch (err) { >> + case EPERM: >> + case EACCES: >> + *nobackground = err; >> + break; >> + default : >> + *nobackground = 0; >> + } >> + return ; >> +} > > Why is this its own function? I made it that way so that in future if we want to handle any more error numbers to bail out , we just need to add a case statement. Thanks - Manish > > - Ted > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/misc/logsave.c b/misc/logsave.c index f0011f8..77a0a16 100644 --- a/misc/logsave.c +++ b/misc/logsave.c @@ -203,6 +203,17 @@ static int copy_from_stdin(void) return 0; } +static void should_background(int err, int *nobackground) { + switch (err) { + case EPERM: + case EACCES: + *nobackground = err; + break; + default : + *nobackground = 0; + } + return ; +} int main(int argc, char **argv) @@ -211,7 +222,7 @@ int main(int argc, char **argv) char *outfn, **cpp; int openflags = O_CREAT|O_WRONLY|O_TRUNC; int send_flag = SEND_LOG; - int do_stdin; + int do_stdin, nobackground = 0; time_t t; while ((c = getopt(argc, argv, "+asv")) != EOF) { @@ -237,6 +248,8 @@ int main(int argc, char **argv) argc -= optind; outfd = open(outfn, openflags, 0644); + if (outfd < 0) + should_background(errno, &nobackground); do_stdin = !strcmp(argv[0], "-"); send_output("Log of ", 0, send_flag); @@ -263,7 +276,7 @@ int main(int argc, char **argv) send_output(ctime(&t), 0, send_flag); send_output("----------------\n", 0, send_flag); - if (outbuf) { + if (!nobackground) { pid = fork(); if (pid < 0) { perror("fork"); @@ -282,6 +295,8 @@ int main(int argc, char **argv) } write(outfd, outbuf, outbufsize); free(outbuf); + } else { + printf("Unable to save log to %s : %s\n", outfn, strerror(nobackground)); } close(outfd);