Messages in this thread | | | Date | Fri, 06 Jun 2008 09:16:13 +0530 | From | Balbir Singh <> | Subject | Re: [PATCH 2.6.25-4] getdelays.c: signal handling for log rotation |
| |
Scott Wiersdorf wrote: > This adds a USR1 signal handler to getdelays.c, which causes getdelays > to close its logfile and reopen it (if '-w logfile' is > specified). This is useful in situations when getdelays is running for > a long time (i.e, the log file growing) and you need to rotate the > logs but don't want to lose any log data. > > Signed-off-by: Scott Wiersdorf <scott@perlcode.org> > --- > --- Documentation/accounting/getdelays.c 2008-05-15 09:00:12.000000000 -0600 > +++ Documentation/accounting/getdelays.c 2008-06-05 02:23:57.000000000 -0600 > @@ -50,6 +50,7 @@ int dbg; > int print_delays; > int print_io_accounting; > int print_task_context_switch_counts; > +volatile sig_atomic_t reopen_log = 0;
> __u64 stime, utime; > > #define PRINTF(fmt, arg...) { \ > @@ -80,6 +81,7 @@ static void usage(void) > fprintf(stderr, " -l: listen forever\n"); > fprintf(stderr, " -v: debug on\n"); > fprintf(stderr, " -C: container path\n"); > + fprintf(stderr, "\nSend USR1 to reopen the logfile if -w is used.\n");
Please mention that old data will be lost and that SIGUSR1 will take affect after some data is received.
> } > > /* > @@ -231,6 +233,30 @@ void print_ioacct(struct taskstats *t) > (unsigned long long)t->cancelled_write_bytes); > } > > +void catch_usr1(int sig) > +{ > + reopen_log = 1; > + signal(sig, catch_usr1); > +} > +
Aren't we better of using the newer sigaction primitives? IIRC, signal can be racy. The man page states "Avoid its use"
> +int reopen_logfile(int fd, char *logfile) > +{ > + if (fd) { > + PRINTF("USR1 received. Closing logfile.\n"); > + close(fd);
So sending USR1 causes data to be lost?
> + } > + fd = open(logfile, O_WRONLY | O_CREAT | O_TRUNC, > + S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); > + if (fd == -1) { > + perror("Cannot open output file\n"); > + exit(1); > + } > + > + reopen_log = 0; > + > + return fd; > +} > + > int main(int argc, char *argv[]) > { > int c, rc, rep_len, aggr_len, len2, cmd_type; > @@ -320,12 +346,8 @@ int main(int argc, char *argv[]) > } > > if (write_file) { > - fd = open(logfile, O_WRONLY | O_CREAT | O_TRUNC, > - S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); > - if (fd == -1) { > - perror("Cannot open output file\n"); > - exit(1); > - } > + fd = reopen_logfile(fd, logfile); > + signal(SIGUSR1, catch_usr1); /* only set when write_file is set */ > } > > if ((nl_sd = create_nl_socket(NETLINK_GENERIC)) < 0) > @@ -444,6 +466,9 @@ int main(int argc, char *argv[]) > err(1,"write error\n"); > } > } > + if (reopen_log) { > + fd = reopen_logfile(fd, logfile); > + }
This seems way of the 80 character space. You have braces that are not required.
> if (!loop) > goto done; > break; >
-- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL
| |