On 09/26/2014 06:39 PM, Federico Simoncelli wrote: > ----- Original Message ----- >> From: "Pádraig Brady" <[email protected]> >> To: "Federico Simoncelli" <[email protected]> >> Cc: "Bernhard Voelker" <[email protected]>, "Coreutils" >> <[email protected]> >> Sent: Friday, September 26, 2014 6:05:33 PM >> Subject: Re: dd SIGUSR1 race >> >>> Many other commands are providing an explicit flag: wget, curl, >>> qemu-img, etc. >> >> Yes, I see qemu-img supports both. >> If not too invasive, we could accept a status=progress patch to output >> stats every approx 1s (noting the caveats of being blocked behind large >> reads/writes) > > I'll try to investigate if I can easily use sigusr1 with your new patch, > meanwhile I came up with a patch adding status=progress. > > It's a simple draft and I am open to hear all comments about it, > especially on what's the format to report the progress. > > For example qemu-img uses '\r' so that the output is always on the same > line. > > Patch in attachment.
I like the idea of '\r', but it involved a little adjustment to deal with. I've updated your patch (attached) with the following changes: - Augmented the texinfo and NEWS entries - Added documentation to --help/man - Adjusted periodic output to use \r rather than \n - Used gethrxtime() rather than clock_gettime() which is not generally available - Avoided a race and CPU spinning in the test thanks, Pádraig.
>From 45d61c2dfb8fa01a4934f607fda5dfdf20e40266 Mon Sep 17 00:00:00 2001 From: Federico Simoncelli <[email protected]> Date: Fri, 26 Sep 2014 17:12:32 +0000 Subject: [PATCH] dd: new status=progress operand to print stats periodically * src/dd.c: Report the transfer progress every second when the new status=progress operand is used. * doc/corutils.texi (dd invocation): Document the new progress status operand. * tests/dd/stats.sh: Add new test for status=progress. * tests/dd/misc.sh: check status=none has precedence over 'progress'. * NEWS: Mention the feature. --- NEWS | 3 + doc/coreutils.texi | 6 +++ src/dd.c | 114 ++++++++++++++++++++++++++++++++++++++++------------ tests/dd/misc.sh | 3 + tests/dd/stats.sh | 7 +++ 5 files changed, 107 insertions(+), 26 deletions(-) diff --git a/NEWS b/NEWS index 3e10ac4..99cff31 100644 --- a/NEWS +++ b/NEWS @@ -19,6 +19,9 @@ GNU coreutils NEWS -*- outline -*- king directory. The new option is only permitted if the new root directory is the old "/", and therefore is useful with the --group and --userspec options. + dd accepts the new status=progress option to update data transfer statistics + on stderr approximately every second. + ** Changes in behavior chroot changes the current directory to "/" in again - unless the above new diff --git a/doc/coreutils.texi b/doc/coreutils.texi index 7d32af5..3092a08 100644 --- a/doc/coreutils.texi +++ b/doc/coreutils.texi @@ -8649,6 +8649,12 @@ that normally make up the last status line. Do not print any informational or warning messages to stderr. Error messages are output as normal. +@item progress +@opindex progress @r{dd status=} +Update the transfer rate and volume statistics on stderr, +when processing each input block. Statistics are output +at most once every second, and can be delayed when waiting on I/O. + @end table @item conv=@var{conversion}[,@var{conversion}]@dots{} diff --git a/src/dd.c b/src/dd.c index d22ec59..810379a 100644 --- a/src/dd.c +++ b/src/dd.c @@ -34,6 +34,7 @@ #include "long-options.h" #include "quote.h" #include "quotearg.h" +#include "verror.h" #include "xstrtol.h" #include "xtime.h" @@ -136,7 +137,8 @@ enum enum { STATUS_NOXFER = 01, - STATUS_NONE = 02 + STATUS_NONE = 02, + STATUS_PROGRESS = 04 }; /* The name of the input file, or NULL for the standard input. */ @@ -211,6 +213,12 @@ static uintmax_t w_bytes = 0; /* Time that dd started. */ static xtime_t start_time; +/* Previous time for periodic progress. */ +static xtime_t previous_time; + +/* Whether a '\n' is pending after writing progress. */ +static bool newline_pending; + /* True if input is seekable. */ static bool input_seekable; @@ -375,6 +383,7 @@ static struct symbol_value const statuses[] = { {"noxfer", STATUS_NOXFER}, {"none", STATUS_NONE}, + {"progress", STATUS_PROGRESS}, {"", 0} }; @@ -517,6 +526,25 @@ maybe_close_stdout (void) _exit (EXIT_FAILURE); } +/* Like error() but handle any pending newline. */ + +static void _GL_ATTRIBUTE_FORMAT ((__printf__, 3, 4)) +werror (int status, int errnum, const char *fmt, ...) +{ + if (newline_pending) + { + putc ('\n', stderr); + newline_pending = false; + } + + va_list ap; + va_start (ap, fmt); + verror (status, errnum, fmt, ap); + va_end (ap); +} + +#define error werror + void usage (int status) { @@ -546,8 +574,9 @@ Copy a file, converting and formatting according to the operands.\n\ oflag=FLAGS write as per the comma separated symbol list\n\ seek=N skip N obs-sized blocks at start of output\n\ skip=N skip N ibs-sized blocks at start of input\n\ - status=WHICH WHICH info to suppress outputting to stderr;\n\ - 'noxfer' suppresses transfer stats, 'none' suppresses all\n\ + status=WHICH WHICH info to print to stderr; 'progress' shows periodic\n\ + transfer stats, 'noxfer' suppresses final transfer stats,\n\ + 'none' suppresses all except warnings and errors\n\ "), stdout); fputs (_("\ \n\ @@ -724,8 +753,7 @@ multiple_bits_set (int i) /* Print transfer statistics. */ static void -print_stats (void) -{ +print_xfer_stats (xtime_t progress_time) { char hbuf[LONGEST_HUMAN_READABLE + 1]; int human_opts = (human_autoscale | human_round_to_nearest @@ -733,24 +761,6 @@ print_stats (void) double delta_s; char const *bytes_per_second; - if (status_flags & STATUS_NONE) - return; - - fprintf (stderr, - _("%"PRIuMAX"+%"PRIuMAX" records in\n" - "%"PRIuMAX"+%"PRIuMAX" records out\n"), - r_full, r_partial, w_full, w_partial); - - if (r_truncate != 0) - fprintf (stderr, - ngettext ("%"PRIuMAX" truncated record\n", - "%"PRIuMAX" truncated records\n", - select_plural (r_truncate)), - r_truncate); - - if (status_flags & STATUS_NOXFER) - return; - /* Use integer arithmetic to compute the transfer rate, since that makes it easy to use SI abbreviations. */ @@ -761,7 +771,8 @@ print_stats (void) w_bytes, human_readable (w_bytes, hbuf, human_opts, 1, 1)); - xtime_t now = gethrxtime (); + xtime_t now = progress_time ? progress_time : gethrxtime (); + if (start_time < now) { double XTIME_PRECISIONe0 = XTIME_PRECISION; @@ -787,7 +798,45 @@ print_stats (void) but that was incorrect for languages like Polish. To fix this bug we now use SI symbols even though they're a bit more confusing in English. */ - fprintf (stderr, _(", %g s, %s/s\n"), delta_s, bytes_per_second); + char const *time_fmt; + if (progress_time) + time_fmt = _(", %.6f s, %s/s%c"); /* OK with '\r' as increasing width. */ + else + time_fmt = _(", %g s, %s/s%c"); + fprintf (stderr, time_fmt, delta_s, bytes_per_second, + progress_time ? '\r' : '\n'); + + newline_pending = !!progress_time; +} + +static void +print_stats (void) +{ + if (status_flags & STATUS_NONE) + return; + + if (newline_pending) + { + putc ('\n', stderr); + newline_pending = false; + } + + fprintf (stderr, + _("%"PRIuMAX"+%"PRIuMAX" records in\n" + "%"PRIuMAX"+%"PRIuMAX" records out\n"), + r_full, r_partial, w_full, w_partial); + + if (r_truncate != 0) + fprintf (stderr, + ngettext ("%"PRIuMAX" truncated record\n", + "%"PRIuMAX" truncated records\n", + select_plural (r_truncate)), + r_truncate); + + if (status_flags & STATUS_NOXFER) + return; + + print_xfer_stats (0); } /* An ordinary signal was received; arrange for the program to exit. */ @@ -2029,6 +2078,19 @@ dd_copy (void) while (1) { + if ((status_flags & STATUS_PROGRESS) && !(status_flags & STATUS_NONE)) + { + xtime_t progress_time = gethrxtime (); + uintmax_t delta_xtime = progress_time; + delta_xtime -= previous_time; + double XTIME_PRECISIONe0 = XTIME_PRECISION; + if (delta_xtime / XTIME_PRECISIONe0 > 1) + { + print_xfer_stats (progress_time); + previous_time = progress_time; + } + } + if (r_partial + r_full >= max_records + !!max_bytes) break; @@ -2345,7 +2407,7 @@ main (int argc, char **argv) } } - start_time = gethrxtime (); + start_time = previous_time = gethrxtime (); exit_status = dd_copy (); diff --git a/tests/dd/misc.sh b/tests/dd/misc.sh index f877fdd..b141268 100755 --- a/tests/dd/misc.sh +++ b/tests/dd/misc.sh @@ -38,6 +38,9 @@ compare /dev/null err || fail=1 # check status=none is cumulative with status=noxfer dd status=none status=noxfer if=$tmp_in of=/dev/null 2> err || fail=1 compare /dev/null err || fail=1 +# check status=none has precedence over status=progress +dd status=none status=progress if=$tmp_in of=/dev/null 2> err || fail=1 +compare /dev/null err || fail=1 dd if=$tmp_in of=$tmp_out 2> /dev/null || fail=1 compare $tmp_in $tmp_out || fail=1 diff --git a/tests/dd/stats.sh b/tests/dd/stats.sh index 386752e..3a4bbad 100755 --- a/tests/dd/stats.sh +++ b/tests/dd/stats.sh @@ -54,4 +54,11 @@ for open in '' '1'; do grep '500000000 bytes .* copied' err || { cat err; fail=1; } done +progress_output() +{ + { sleep "$1"; echo 1; } | dd bs=1 status=noxfer,progress of=/dev/null 2>err + grep 'byte.*copied' err +} +retry_delay_ progress_output 1 4 || { cat err; fail=1; } + Exit $fail -- 1.7.7.6
