Bug 32138

Summary:

better progress reporting for dump(8)

Product:

Base System

Reporter:

Mikhail T. <freebsd-2024>

Component:

bin

Assignee:

iedowse

Status:

Closed FIXED

Severity:

Affects Only Me

Priority:

Normal

Version:

5.0-CURRENT

Hardware:

Any

OS:

Any

Attachments:

Description	Flags
file.diff	none

Description Mikhail T. 2001-11-20 17:00:00 UTC

	The included patch makes dump(8) update its proctitle to reflect
	what it is doing at the moment (including the percentage of the
	task completed). It also provides reasonable handling of SIGINFO,
	and removes the arbitrary (and confusing) requirement, that at
	least 500 blocks must be written before the next progress report
	line is output.

	The first version of the patch was critisized on -arch for doing
	things a signal handler is not allowed to do. This version simply
	alters the scheduled time of the next report to ensure it happens
	after the next block is being written.

Fix: Please, commit this, or agree to become my mentor for the src
	tree -- I'll be glad to commit this myself :-)

cvs server: Diffing .

Comment 1 arr freebsd_committer

2001-11-20 17:26:28 UTC

The signal handler in the original code was not signal safe and neither is
the code givenhere..  I'd like to see the code be fixed from this problem,
willing to make it signal safe?

Cheers,
Andrew

On Tue, 20 Nov 2001, Mikhail Teterin wrote:

:
:>Number:         32138
:>Category:       bin
:>Synopsis:       better progress reporting for dump(8)
:>Confidential:   no
:>Severity:       non-critical
:>Priority:       low
:>Responsible:    freebsd-bugs
:>State:          open
:>Quarter:        
:>Keywords:       
:>Date-Required:
:>Class:          change-request
:>Submitter-Id:   current-users
:>Arrival-Date:   Tue Nov 20 09:00:00 PST 2001
:>Closed-Date:
:>Last-Modified:
:>Originator:     Mikhail Teterin
:>Release:        FreeBSD 5.0-CURRENT i386
:>Organization:
:Virtual Estates, Inc.
:>Environment:
:System: FreeBSD aldan.algebra.com 5.0-CURRENT FreeBSD 5.0-CURRENT #6: Mon Nov 5 08:47:03 EST 2001 mi@aldan.algebra.com:/ccd/obj/ccd/src/sys/DEBUG i386
:
:
:>Description:
:
:	The included patch makes dump(8) update its proctitle to reflect
:	what it is doing at the moment (including the percentage of the
:	task completed). It also provides reasonable handling of SIGINFO,
:	and removes the arbitrary (and confusing) requirement, that at
:	least 500 blocks must be written before the next progress report
:	line is output.
:
:	The first version of the patch was critisized on -arch for doing
:	things a signal handler is not allowed to do. This version simply
:	alters the scheduled time of the next report to ensure it happens
:	after the next block is being written.
:
:>How-To-Repeat:
:
:>Fix:
:
:	Please, commit this, or agree to become my mentor for the src
:	tree -- I'll be glad to commit this myself :-)
:
:cvs server: Diffing .
:Index: dump.8
:===================================================================
:RCS file: /home/ncvs/src/sbin/dump/dump.8,v
:retrieving revision 1.39
:diff -U2 -r1.39 dump.8
:--- dump.8	15 Jul 2001 14:00:19 -0000	1.39
:+++ dump.8	20 Nov 2001 16:49:14 -0000
:@@ -320,5 +320,6 @@
: .Pp
: .Nm Dump
:-tells the operator what is going on at periodic intervals,
:+tells the operator what is going on at periodic intervals --
:+every 5 minutes, or promptly after receiving SIGINFO --
: including usually low estimates of the number of blocks to write,
: the number of tapes it will take, the time to completion, and
:Index: dump.h
:===================================================================
:RCS file: /home/ncvs/src/sbin/dump/dump.h,v
:retrieving revision 1.11
:diff -U2 -r1.11 dump.h
:--- dump.h	17 Nov 2001 00:06:55 -0000	1.11
:+++ dump.h	20 Nov 2001 16:49:14 -0000
:@@ -101,6 +101,7 @@
: int	query __P((char *question));
: void	quit __P((const char *fmt, ...)) __printflike(1, 2);
:-void	timeest __P((void));
:+void	timeest __P((int)); /* accepts the signal number */
: time_t	unctime __P((char *str));
:+void	title __P((void));
: 
: /* mapping rouintes */
:Index: optr.c
:===================================================================
:RCS file: /home/ncvs/src/sbin/dump/optr.c,v
:retrieving revision 1.16
:diff -U2 -r1.16 optr.c
:--- optr.c	17 Nov 2001 00:06:55 -0000	1.16
:+++ optr.c	20 Nov 2001 16:49:14 -0000
:@@ -52,4 +52,5 @@
: #include <string.h>
: #include <stdarg.h>
:+#include <signal.h>
: #include <unistd.h>
: #include <utmp.h>
:@@ -191,14 +192,26 @@
: 
: void
:-timeest()
:+timeest(signal)
: {
: 	time_t	tnow;
: 	int deltat;
: 
:-	(void) time((time_t *) &tnow);
:+	if (signal != 0) {
:+		/*
:+		 * We might be called to handle SIGINFO. Re-schedule
:+		 * the reporting to occur the next time we are called
:+		 * regularly and bail out -- don't do reporting as a
:+		 * signal handler -- it involves malloc-ing and other
:+		 * things signal handler are not supposed to do.
:+		 */
:+		/*
:+		 * 300 seconds is a constant only used in this function
:+		 */
:+		tschedule -= 300;
:+		return;
:+	}
:+	(void) time(&tnow);
: 	if (tnow >= tschedule) {
: 		tschedule = tnow + 300;
:-		if (blockswritten < 500)
:-			return;
: 		deltat = tstart_writing - tnow +
: 			(1.0 * (tnow - tstart_writing))
:@@ -207,4 +220,5 @@
: 			(blockswritten * 100.0) / tapesize,
: 			deltat / 3600, (deltat % 3600) / 60);
:+		title();
: 	}
: }
:@@ -235,4 +249,31 @@
: 	(void) vsnprintf(lastmsg, sizeof(lastmsg), fmt, ap);
: 	va_end(ap);
:+}
:+
:+/*
:+ * This function can be called to place, what msg() above pushed to
:+ * stderr, into the process title, viewable with the ps-command.
:+ * A side effect of this function, is it replaces the final '\n' (if any)
:+ * with the '\0' in the global variable lastmsg -- to avoid the literal
:+ * "\n" being put into the proctitle.
:+ * So, if the lastmsg needs to be output elsewhere, that should happen
:+ * before calling title().
:+ */
:+void title()
:+{
:+	int lastlen;
:+
:+	lastlen = strlen(lastmsg);
:+	if (lastmsg[lastlen-1] == '\n')
:+		lastmsg[lastlen-1] = '\0';
:+
:+	/*
:+	 * It would be unwise to run multiple dumps of same disk
:+	 * at  the  same time.  So  ``disk''  is sufficient  for
:+	 * identifying, to  which family of dump  processes this
:+	 * one belongs  -- the other processes  continue to have
:+	 * the original titles.
:+	 */
:+	setproctitle("%s: %s", disk, lastmsg);
: }
: 
:Index: tape.c
:===================================================================
:RCS file: /home/ncvs/src/sbin/dump/tape.c,v
:retrieving revision 1.14
:diff -U2 -r1.14 tape.c
:--- tape.c	17 Nov 2001 00:06:55 -0000	1.14
:+++ tape.c	20 Nov 2001 16:49:15 -0000
:@@ -309,5 +309,5 @@
: 		startnewtape(0);
: 	}
:-	timeest();
:+	timeest(0);
: }
: 
:@@ -522,4 +522,5 @@
: 	 *	All signals are inherited...
: 	 */
:+	setproctitle(NULL); /* restore the proctitle modified by title() */
: 	childpid = fork();
: 	if (childpid < 0) {
:@@ -612,4 +613,5 @@
: 
: 		enslave();  /* Share open tape file descriptor with slaves */
:+		signal(SIGINFO, timeest); /* report progress on SIGINFO */
: 
: 		asize = 0;
:>Release-Note:
:>Audit-Trail:
:>Unformatted:
:
:To Unsubscribe: send mail to majordomo@FreeBSD.org
:with "unsubscribe freebsd-bugs" in the body of the message
:

--
Andrew R. Reiter
arr@watson.org
arr@FreeBSD.org

Comment 2 iedowse 2001-11-20 17:41:50 UTC

In message <200111201651.fAKGp7s38773@aldan.algebra.com>, Mikhail Teterin write
s:
>-void	timeest __P((void));
>+void	timeest __P((int)); /* accepts the signal number */

>+		signal(SIGINFO, timeest); /* report progress on SIGINFO */

I think it would be cleaner to add a simple signal handler which
sets a flag, and then check this where necessary. Calling a signal
handler function directly is weird, and using a magic value for
the signal number is evil :-) What happens if multiple ^Ts are sent
and tschedule gets decremented a few times in a row?

Ian

Comment 3 Mikhail T. 2001-11-20 19:07:34 UTC

On 20 Nov, Ian Dowse wrote:
> In message <200111201651.fAKGp7s38773@aldan.algebra.com>, Mikhail Teterin write
> s:
>>-void	timeest __P((void));
>>+void	timeest __P((int)); /* accepts the signal number */
> 
>>+		signal(SIGINFO, timeest); /* report progress on SIGINFO */
 
> I think it would be cleaner to  add a simple signal handler which sets
> a flag, and then check this  where necessary. Calling a signal handler
> function directly  is weird, and  using a  magic value for  the signal
> number is evil :-)

Yes, a  weird evil  :) Ok, I'll  split it  -- I did  not want  to create
another global function, to be honest.

> What happens if multiple ^Ts are sent and tschedule gets decremented a
> few times in a row?

The  check  is only  for  tnow  >=  tschedule.  So, no  problem,  unless
tschedule  gets decremented  into  the positive  territory (imagine  the
number of ^T this would require),  in which case, the progress reporting
will  cease. Big  deal...  The version  I last  presented  to -arch  set
tschedule to the current time, but that required a call to time(), which
is not a problem, really, but is not needed either.

On 20 Nov, Andrew R. Reiter wrote:
> The  signal handler  in  the original  code was  not  signal safe  and
> neither is the code givenhere.. I'd like to see the code be fixed from
> this problem, willing to make it signal safe?

What's not safe about it? The only thing happening in the signal handler
is ``tschedule -= 300;''.  I think, it is quite safe --  see above I can
even make it ``if (tschedule > 300) tschedule -= 300;'' :-) Could you be
more specific? Thanks!

	-mi

How about this:
Index: dump.8
===================================================================
RCS file: /home/ncvs/src/sbin/dump/dump.8,v
retrieving revision 1.39
diff -U2 -r1.39 dump.8
--- dump.8	15 Jul 2001 14:00:19 -0000	1.39
+++ dump.8	20 Nov 2001 19:07:30 -0000
@@ -320,5 +320,6 @@
 .Pp
 .Nm Dump
-tells the operator what is going on at periodic intervals,
+tells the operator what is going on at periodic intervals --
+every 5 minutes, or promptly after receiving SIGINFO --
 including usually low estimates of the number of blocks to write,
 the number of tapes it will take, the time to completion, and
Index: dump.h
===================================================================
RCS file: /home/ncvs/src/sbin/dump/dump.h,v
retrieving revision 1.11
diff -U2 -r1.11 dump.h
--- dump.h	17 Nov 2001 00:06:55 -0000	1.11
+++ dump.h	20 Nov 2001 19:07:30 -0000
@@ -96,4 +96,5 @@
 /* operator interface functions */
 void	broadcast __P((char *message));
+void	infosch	__P((int));
 void	lastdump __P((int arg));	/* int should be char */
 void	msg __P((const char *fmt, ...)) __printflike(1, 2);
@@ -103,4 +104,5 @@
 void	timeest __P((void));
 time_t	unctime __P((char *str));
+void	title __P((void));
 
 /* mapping rouintes */
Index: optr.c
===================================================================
RCS file: /home/ncvs/src/sbin/dump/optr.c,v
retrieving revision 1.16
diff -U2 -r1.16 optr.c
--- optr.c	17 Nov 2001 00:06:55 -0000	1.16
+++ optr.c	20 Nov 2001 19:07:30 -0000
@@ -52,4 +52,5 @@
 #include <string.h>
 #include <stdarg.h>
+#include <signal.h>
 #include <unistd.h>
 #include <utmp.h>
@@ -188,5 +189,5 @@
  */
 
-time_t	tschedule = 0;
+static time_t	tschedule = 0;
 
 void
@@ -196,9 +197,7 @@
 	int deltat;
 
-	(void) time((time_t *) &tnow);
+	(void) time(&tnow);
 	if (tnow >= tschedule) {
 		tschedule = tnow + 300;
-		if (blockswritten < 500)
-			return;
 		deltat = tstart_writing - tnow +
 			(1.0 * (tnow - tstart_writing))
@@ -207,7 +206,21 @@
 			(blockswritten * 100.0) / tapesize,
 			deltat / 3600, (deltat % 3600) / 60);
+		title();
 	}
 }
 
+/*
+ *	Reschedule the next printout of the estimate
+ */
+void
+infosch(int signal) {
+	/*
+	 * 300 seconds -- 5 minutes -- is the magical constant,
+	 * only used in this file
+	 */
+	if (tschedule > 300)
+		tschedule -= 300;
+}
+
 void
 #if __STDC__
@@ -235,4 +248,31 @@
 	(void) vsnprintf(lastmsg, sizeof(lastmsg), fmt, ap);
 	va_end(ap);
+}
+
+/*
+ * This function can be called to place, what msg() above pushed to
+ * stderr, into the process title, viewable with the ps-command.
+ * A side effect of this function, is it replaces the final '\n' (if any)
+ * with the '\0' in the global variable lastmsg -- to avoid the literal
+ * "\n" being put into the proctitle.
+ * So, if the lastmsg needs to be output elsewhere, that should happen
+ * before calling title().
+ */
+void title()
+{
+	int lastlen;
+
+	lastlen = strlen(lastmsg);
+	if (lastmsg[lastlen-1] == '\n')
+		lastmsg[lastlen-1] = '\0';
+
+	/*
+	 * It would be unwise to run multiple dumps of same disk
+	 * at  the  same time.  So  ``disk''  is sufficient  for
+	 * identifying, to  which family of dump  processes this
+	 * one belongs  -- the other processes  continue to have
+	 * the original titles.
+	 */
+	setproctitle("%s: %s", disk, lastmsg);
 }
 
Index: tape.c
===================================================================
RCS file: /home/ncvs/src/sbin/dump/tape.c,v
retrieving revision 1.14
diff -U2 -r1.14 tape.c
--- tape.c	17 Nov 2001 00:06:55 -0000	1.14
+++ tape.c	20 Nov 2001 19:07:31 -0000
@@ -522,4 +522,5 @@
 	 *	All signals are inherited...
 	 */
+	setproctitle(NULL); /* restore the proctitle modified by title() */
 	childpid = fork();
 	if (childpid < 0) {
@@ -612,4 +613,5 @@
 
 		enslave();  /* Share open tape file descriptor with slaves */
+		signal(SIGINFO, infosch); /* report progress on SIGINFO */
 
 		asize = 0;

Comment 4 arr freebsd_committer

2001-11-20 19:18:05 UTC

:
:What's not safe about it? The only thing happening in the signal handler
:is ``tschedule -= 300;''.  I think, it is quite safe --  see above I can
:even make it ``if (tschedule > 300) tschedule -= 300;'' :-) Could you be
:more specific? Thanks!
:
:	-mi

just quickly:
 - signal handlers really should just set a flag and then check the flag
at the appropriate time (depending on how your program flow is).
 - setproctitle(3) is not safe to use within a handler


:
:How about this:
:Index: dump.8
:===================================================================
:RCS file: /home/ncvs/src/sbin/dump/dump.8,v
:retrieving revision 1.39
:diff -U2 -r1.39 dump.8
:--- dump.8	15 Jul 2001 14:00:19 -0000	1.39
:+++ dump.8	20 Nov 2001 19:07:30 -0000
:@@ -320,5 +320,6 @@
: .Pp
: .Nm Dump
:-tells the operator what is going on at periodic intervals,
:+tells the operator what is going on at periodic intervals --
:+every 5 minutes, or promptly after receiving SIGINFO --
: including usually low estimates of the number of blocks to write,
: the number of tapes it will take, the time to completion, and
:Index: dump.h
:===================================================================
:RCS file: /home/ncvs/src/sbin/dump/dump.h,v
:retrieving revision 1.11
:diff -U2 -r1.11 dump.h
:--- dump.h	17 Nov 2001 00:06:55 -0000	1.11
:+++ dump.h	20 Nov 2001 19:07:30 -0000
:@@ -96,4 +96,5 @@
: /* operator interface functions */
: void	broadcast __P((char *message));
:+void	infosch	__P((int));
: void	lastdump __P((int arg));	/* int should be char */
: void	msg __P((const char *fmt, ...)) __printflike(1, 2);
:@@ -103,4 +104,5 @@
: void	timeest __P((void));
: time_t	unctime __P((char *str));
:+void	title __P((void));
: 
: /* mapping rouintes */
:Index: optr.c
:===================================================================
:RCS file: /home/ncvs/src/sbin/dump/optr.c,v
:retrieving revision 1.16
:diff -U2 -r1.16 optr.c
:--- optr.c	17 Nov 2001 00:06:55 -0000	1.16
:+++ optr.c	20 Nov 2001 19:07:30 -0000
:@@ -52,4 +52,5 @@
: #include <string.h>
: #include <stdarg.h>
:+#include <signal.h>
: #include <unistd.h>
: #include <utmp.h>
:@@ -188,5 +189,5 @@
:  */
: 
:-time_t	tschedule = 0;
:+static time_t	tschedule = 0;
: 
: void
:@@ -196,9 +197,7 @@
: 	int deltat;
: 
:-	(void) time((time_t *) &tnow);
:+	(void) time(&tnow);
: 	if (tnow >= tschedule) {
: 		tschedule = tnow + 300;
:-		if (blockswritten < 500)
:-			return;
: 		deltat = tstart_writing - tnow +
: 			(1.0 * (tnow - tstart_writing))
:@@ -207,7 +206,21 @@
: 			(blockswritten * 100.0) / tapesize,
: 			deltat / 3600, (deltat % 3600) / 60);
:+		title();
: 	}
: }
: 
:+/*
:+ *	Reschedule the next printout of the estimate
:+ */
:+void
:+infosch(int signal) {
:+	/*
:+	 * 300 seconds -- 5 minutes -- is the magical constant,
:+	 * only used in this file
:+	 */
:+	if (tschedule > 300)
:+		tschedule -= 300;
:+}
:+
: void
: #if __STDC__
:@@ -235,4 +248,31 @@
: 	(void) vsnprintf(lastmsg, sizeof(lastmsg), fmt, ap);
: 	va_end(ap);
:+}
:+
:+/*
:+ * This function can be called to place, what msg() above pushed to
:+ * stderr, into the process title, viewable with the ps-command.
:+ * A side effect of this function, is it replaces the final '\n' (if any)
:+ * with the '\0' in the global variable lastmsg -- to avoid the literal
:+ * "\n" being put into the proctitle.
:+ * So, if the lastmsg needs to be output elsewhere, that should happen
:+ * before calling title().
:+ */
:+void title()
:+{
:+	int lastlen;
:+
:+	lastlen = strlen(lastmsg);
:+	if (lastmsg[lastlen-1] == '\n')
:+		lastmsg[lastlen-1] = '\0';
:+
:+	/*
:+	 * It would be unwise to run multiple dumps of same disk
:+	 * at  the  same time.  So  ``disk''  is sufficient  for
:+	 * identifying, to  which family of dump  processes this
:+	 * one belongs  -- the other processes  continue to have
:+	 * the original titles.
:+	 */
:+	setproctitle("%s: %s", disk, lastmsg);
: }
: 
:Index: tape.c
:===================================================================
:RCS file: /home/ncvs/src/sbin/dump/tape.c,v
:retrieving revision 1.14
:diff -U2 -r1.14 tape.c
:--- tape.c	17 Nov 2001 00:06:55 -0000	1.14
:+++ tape.c	20 Nov 2001 19:07:31 -0000
:@@ -522,4 +522,5 @@
: 	 *	All signals are inherited...
: 	 */
:+	setproctitle(NULL); /* restore the proctitle modified by title() */
: 	childpid = fork();
: 	if (childpid < 0) {
:@@ -612,4 +613,5 @@
: 
: 		enslave();  /* Share open tape file descriptor with slaves */
:+		signal(SIGINFO, infosch); /* report progress on SIGINFO */
: 
: 		asize = 0;
:
:
:

--
Andrew R. Reiter
arr@watson.org
arr@FreeBSD.org

Comment 5 iedowse 2001-11-20 19:29:46 UTC

In message <200111201907.fAKJ7bO40025@aldan.algebra.com>, Mikhail Teterin write
s:

>How about this:

Much better! Two comments:

>+void
>+infosch(int signal) {
>+	/*
>+	 * 300 seconds -- 5 minutes -- is the magical constant,
>+	 * only used in this file
>+	 */
>+	if (tschedule > 300)
>+		tschedule -= 300;
>+}

If "tschedule = 0" will work here instead, then I'd suggest using
that to avoid the magic constant.

>+ * A side effect of this function, is it replaces the final '\n' (if any)
>+ * with the '\0' in the global variable lastmsg -- to avoid the literal
>+ * "\n" being put into the proctitle.
>+ * So, if the lastmsg needs to be output elsewhere, that should happen
>+ * before calling title().

You can avoid this side-effect by telling setproctitle not to print
the last character, i.e:

	lastlen = strlen(lastmsg);
	if (lastlen > 0 && lastmsg[lastlen - 1] == '\n')
		lastlen--;
	setproctitle("%s: %.*s", disk, lastlen, lastmsg);

Ian

Comment 6 arr freebsd_committer

2001-11-20 19:30:20 UTC

On Tue, 20 Nov 2001, Ian Dowse wrote:

:In message <200111201907.fAKJ7bO40025@aldan.algebra.com>, Mikhail Teterin write
:s:
:
:>How about this:
:
:Much better! Two comments:
:
:>+void
:>+infosch(int signal) {
:>+	/*
:>+	 * 300 seconds -- 5 minutes -- is the magical constant,
:>+	 * only used in this file
:>+	 */
:>+	if (tschedule > 300)
:>+		tschedule -= 300;
:>+}
:
:If "tschedule = 0" will work here instead, then I'd suggest using
:that to avoid the magic constant.
:
:>+ * A side effect of this function, is it replaces the final '\n' (if any)
:>+ * with the '\0' in the global variable lastmsg -- to avoid the literal
:>+ * "\n" being put into the proctitle.
:>+ * So, if the lastmsg needs to be output elsewhere, that should happen
:>+ * before calling title().
:
:You can avoid this side-effect by telling setproctitle not to print
:the last character, i.e:
:
:	lastlen = strlen(lastmsg);
:	if (lastlen > 0 && lastmsg[lastlen - 1] == '\n')
:		lastlen--;
:	setproctitle("%s: %.*s", disk, lastlen, lastmsg);
:
:Ian
:

setproctitle() should not even be used in the handler.

--
Andrew R. Reiter
arr@watson.org
arr@FreeBSD.org

Comment 7 iedowse 2001-11-20 19:35:18 UTC

In message <Pine.NEB.3.96L.1011120143005.92014C-100000@fledge.watson.org>, "And
rew R. Reiter" writes:
>
>setproctitle() should not even be used in the handler.

Is it being called from a signal handler? I only looked briefly at
the code, and it appears to be called by timeest() which is called
by flushtape(), but I didn't trace back any further to see if that
is called from a signal handler.

Ian

Comment 8 Mikhail T. 2001-11-20 20:06:20 UTC

On 20 Nov, Ian Dowse wrote:
> In message <Pine.NEB.3.96L.1011120143005.92014C-100000@fledge.watson.org>,
>	"Andrew R. Reiter" writes:
>>
>>setproctitle() should not even be used in the handler.
> 
> Is it being called from a signal handler?

It is not. Not now, not in the original PR, not in the version last sent
to -arch (three weeks ago).

>You can avoid this side-effect by telling setproctitle not to print the
>last character, i.e:

>	lastlen = strlen(lastmsg);
>	if (lastlen > 0 && lastmsg[lastlen - 1] == '\n')
>		lastlen--;
>	setproctitle("%s: %.*s", disk, lastlen, lastmsg);

Yes! That's a good one... Thanks,

	-mi

Comment 9 Mikhail Teterin freebsd_committer

2001-12-27 20:54:03 UTC

Responsible Changed
From-To: freebsd-bugs->iadowse

Comment 10 Mikhail Teterin freebsd_committer

2001-12-27 21:24:22 UTC

Responsible Changed
From-To: iadowse->iedowse

Ian has different aliases...

Comment 11 Mikhail T. 2002-02-05 18:57:24 UTC

This is  the current version of  the patch... There is  some controversy
over using the global variable lastmsg in the new title(). I feel, it is
alright. The variable existed before this patch, and is only used in one
file optr.c. In fact, this patch makes  it static -- aiking to a private
member of the "optr class".

	-mi

cvs server: Diffing .
Index: dump.8
===================================================================
RCS file: /home/ncvs/src/sbin/dump/dump.8,v
retrieving revision 1.39
diff -U2 -r1.39 dump.8
--- dump.8	15 Jul 2001 14:00:19 -0000	1.39
+++ dump.8	5 Feb 2002 18:55:20 -0000
@@ -320,5 +320,6 @@
 .Pp
 .Nm Dump
-tells the operator what is going on at periodic intervals,
+tells the operator what is going on at periodic intervals --
+every 5 minutes, or promptly after receiving SIGINFO --
 including usually low estimates of the number of blocks to write,
 the number of tapes it will take, the time to completion, and
Index: dump.h
===================================================================
RCS file: /home/ncvs/src/sbin/dump/dump.h,v
retrieving revision 1.11
diff -U2 -r1.11 dump.h
--- dump.h	17 Nov 2001 00:06:55 -0000	1.11
+++ dump.h	5 Feb 2002 18:55:20 -0000
@@ -96,4 +96,5 @@
 /* operator interface functions */
 void	broadcast __P((char *message));
+void	infosch	__P((int));
 void	lastdump __P((int arg));	/* int should be char */
 void	msg __P((const char *fmt, ...)) __printflike(1, 2);
@@ -103,4 +104,5 @@
 void	timeest __P((void));
 time_t	unctime __P((char *str));
+void	title __P((void));
 
 /* mapping rouintes */
Index: main.c
===================================================================
RCS file: /home/ncvs/src/sbin/dump/main.c,v
retrieving revision 1.31
diff -U2 -r1.31 main.c
--- main.c	19 Jan 2002 23:19:59 -0000	1.31
+++ main.c	5 Feb 2002 18:55:21 -0000
@@ -364,7 +364,9 @@
 
 	msg("mapping (Pass I) [regular files]\n");
+	title();
 	anydirskipped = mapfiles(maxino, &tapesize);
 
 	msg("mapping (Pass II) [directories]\n");
+	title();
 	while (anydirskipped) {
 		anydirskipped = mapdirs(maxino, &tapesize);
@@ -416,4 +418,5 @@
 		    tapesize, fetapes);
 	}
+	title();
 
 	/*
@@ -429,4 +432,5 @@
 
 	msg("dumping (Pass III) [directories]\n");
+	title();
 	dirty = 0;		/* XXX just to get gcc to shut up */
 	for (map = dumpdirmap, ino = 1; ino < maxino; ino++) {
@@ -447,4 +451,5 @@
 
 	msg("dumping (Pass IV) [regular files]\n");
+	title();
 	for (map = dumpinomap, ino = 1; ino < maxino; ino++) {
 		int mode;
@@ -476,4 +481,5 @@
 		    spcl.c_tapea, spcl.c_volume,
 		    (spcl.c_volume == 1) ? "" : "s");
+	title();
 
 	/* report dump performance, avoid division through zero */
@@ -484,4 +490,5 @@
 		    tend_writing - tstart_writing,
 		    spcl.c_tapea / (tend_writing - tstart_writing));
+	title();
 
 	putdumptime();
@@ -489,4 +496,5 @@
 	broadcast("DUMP IS DONE!\a\a\n");
 	msg("DUMP IS DONE\n");
+	title();
 	Exit(X_FINOK);
 	/* NOTREACHED */
@@ -542,4 +550,5 @@
 			quit("Signal on pipe: cannot recover\n");
 		msg("Rewriting attempted as response to unknown signal.\n");
+		title();
 		(void)fflush(stderr);
 		(void)fflush(stdout);
Index: optr.c
===================================================================
RCS file: /home/ncvs/src/sbin/dump/optr.c,v
retrieving revision 1.16
diff -U2 -r1.16 optr.c
--- optr.c	17 Nov 2001 00:06:55 -0000	1.16
+++ optr.c	5 Feb 2002 18:55:21 -0000
@@ -52,4 +52,5 @@
 #include <string.h>
 #include <stdarg.h>
+#include <signal.h>
 #include <unistd.h>
 #include <utmp.h>
@@ -117,5 +118,5 @@
 }
 
-char lastmsg[BUFSIZ];
+static char lastmsg[BUFSIZ];
 
 /*
@@ -188,5 +189,5 @@
  */
 
-time_t	tschedule = 0;
+static time_t	tschedule = 0;
 
 void
@@ -196,9 +197,7 @@
 	int deltat;
 
-	(void) time((time_t *) &tnow);
+	(void) time(&tnow);
 	if (tnow >= tschedule) {
 		tschedule = tnow + 300;
-		if (blockswritten < 500)
-			return;
 		deltat = tstart_writing - tnow +
 			(1.0 * (tnow - tstart_writing))
@@ -207,7 +206,18 @@
 			(blockswritten * 100.0) / tapesize,
 			deltat / 3600, (deltat % 3600) / 60);
+		title();
 	}
 }
 
+/*
+ *	Reschedule the next printout of the estimate.
+ */
+void
+infosch(signal)
+	int signal;
+{
+	tschedule = 0;
+}
+
 void
 #if __STDC__
@@ -235,4 +245,25 @@
 	(void) vsnprintf(lastmsg, sizeof(lastmsg), fmt, ap);
 	va_end(ap);
+}
+
+/*
+ * This function can be called to place, whatever msg() above pushed
+ * to stderr, into the process  title, viewable with the ps-command.
+ */
+void title() {
+	int lastlen;
+
+	lastlen = strlen(lastmsg);
+	if (lastlen > 0 && lastmsg[lastlen-1] == '\n')
+		lastlen--;
+
+	/*
+	 * It would be unwise to run multiple dumps of the same
+	 * disk at the same time. So ``disk'' is sufficient for
+	 * identifying, to which family of dump processes this one
+	 * belongs -- the other processes of the family continue to
+	 * have the original titles.
+	 */
+	setproctitle("%s: %.*s", disk, lastlen, lastmsg);
 }
 
Index: tape.c
===================================================================
RCS file: /home/ncvs/src/sbin/dump/tape.c,v
retrieving revision 1.14
diff -U2 -r1.14 tape.c
--- tape.c	17 Nov 2001 00:06:55 -0000	1.14
+++ tape.c	5 Feb 2002 18:55:21 -0000
@@ -522,4 +522,5 @@
 	 *	All signals are inherited...
 	 */
+	setproctitle(NULL); /* restore the proctitle modified by title() */
 	childpid = fork();
 	if (childpid < 0) {
@@ -612,4 +613,5 @@
 
 		enslave();  /* Share open tape file descriptor with slaves */
+		signal(SIGINFO, infosch);
 
 		asize = 0;

Comment 12 iedowse freebsd_committer

2002-02-23 22:32:59 UTC

State Changed
From-To: open->closed


Committed to -CURRENT and -STABLE.