From 14ce83224985b5f3a692a7dd1b9fe3e28f01421c Mon Sep 17 00:00:00 2001 From: Andrew Wood Date: Sat, 2 Sep 2023 01:21:18 +0100 Subject: [PATCH] New "--error-skip-block" option. --- doc/ACKNOWLEDGEMENTS.md | 1 + doc/NEWS.md | 1 + doc/TODO.md | 1 - doc/pv.1 | 18 ++++++++++++++++++ doc/pv.1.md | 14 ++++++++++++++ src/include/options.h | 1 + src/include/pv-internal.h | 1 + src/include/pv.h | 1 + src/main/help.c | 3 +++ src/main/main.c | 1 + src/main/options.c | 12 +++++++++++- src/pv/state.c | 5 +++++ src/pv/transfer.c | 21 +++++++++++++++------ 13 files changed, 72 insertions(+), 8 deletions(-) diff --git a/doc/ACKNOWLEDGEMENTS.md b/doc/ACKNOWLEDGEMENTS.md index 206baea..135a6e0 100644 --- a/doc/ACKNOWLEDGEMENTS.md +++ b/doc/ACKNOWLEDGEMENTS.md @@ -86,5 +86,6 @@ is acknowledged and greatly appreciated: * [fuschia74](https://github.com/fuchsia74) - provided "`--enable-static`" patch for "`configure`" * [Wilhelm von Thiele](https://github.com/TurtleWilly) - assisted with OS X cleanups ([#73](https://codeberg.org/a-j-wood/pv/issues/73), [#74](https://codeberg.org/a-j-wood/pv/issues/74)) * Matějů Miroslav, Ing. - suggested fix for ETA and elapsed time faults when suspending and resuming a machine ([#13](https://codeberg.org/a-j-wood/pv/issues/13)) + * Anthony DeRobertis - suggested the "`--error-skip-block`" option ([#37](https://codeberg.org/a-j-wood/pv/issues/37)) --- diff --git a/doc/NEWS.md b/doc/NEWS.md index 96622b3..9aea99c 100644 --- a/doc/NEWS.md +++ b/doc/NEWS.md @@ -3,6 +3,7 @@ * feature: new "`--discard`" option to discard input as if writing to */dev/null* ([#42](https://codeberg.org/a-j-wood/pv/issues/42)) * feature: use `posix_fadvise()` like `cat`(1) does, to improve efficiency ([#39](https://codeberg.org/a-j-wood/pv/issues/39)) * feature: new "`--enable-static`" option to "`configure`" for static builds ([#75](https://codeberg.org/a-j-wood/pv/pull/75)) + * feature: new "`--error-skip-block`" option to make "`--skip-errors`" skip whole blocks ([#37](https://codeberg.org/a-j-wood/pv/issues/37)) * security: with "`--pidfile`", write to a temporary file and rename it into place, to improve security * security: keep self-contained copies of name and format string in PV internal state for memory safety * fix: only report errors about missing files when starting to transfer from them, not while calculating size, and behave more like `cat`(1) by skipping them and moving on diff --git a/doc/TODO.md b/doc/TODO.md index a124509..5c9bbe9 100644 --- a/doc/TODO.md +++ b/doc/TODO.md @@ -23,7 +23,6 @@ Feature requests * ([#22](https://codeberg.org/a-j-wood/pv/issues/22)) Options to skip input and seek on output (Jason A. Pfeil, Feb 2022) * ([#25](https://codeberg.org/a-j-wood/pv/issues/25)) Normalise progress to 100% on overrun (Andrej Gantvorg) * ([#35](https://codeberg.org/a-j-wood/pv/issues/35)) Allow decimal values for "`-s`", "`-L`", "`-B`" (Thomas Watson - Aug 2020) - * ([#37](https://codeberg.org/a-j-wood/pv/issues/37)) Debian #839796 - Allow "`-E`" to take a block size argument so errors cause a skip to the next block (Anthony DeRobertis - Oct 2016) * ([#38](https://codeberg.org/a-j-wood/pv/issues/38)) Reset ETA on *SIGUSR1* (Jacek Wielemborek - Jan 2019) * ([#40](https://codeberg.org/a-j-wood/pv/issues/40)) Permit "`-c`" with "`-d PID:FD`", reject "`-N`" with "`-d PID`" (Norman Rasmussen - Nov 2020) * ([#43](https://codeberg.org/a-j-wood/pv/issues/43)) Differentiate between "`--eta`" and "`--fineta`" in display (André Stapf - Apr 2017) diff --git a/doc/pv.1 b/doc/pv.1 index 97c8e11..3712410 100644 --- a/doc/pv.1 +++ b/doc/pv.1 @@ -402,6 +402,24 @@ Specify twice to only report a read error once per file, instead of reporting each byte range skipped. .TP +.BI \-Z\ BYTES \fR,\ \fB\-\-error\-skip\-block\ BYTES +When ignoring read errors with +.BR \-E , +instead of trying to adaptively skip by reading small amounts and skipping +progressively larger sections until a read succeeds, move to the next file +block of +.I BYTES +bytes as soon as an error occurs. There may still be some shorter skips +where the block being skipped coincides with the end of the transfer buffer. +.TP +.B "" +This option can only be used with +.B \-E +and is intended for use when reading from a block device, such as +.B \-E\ \-Z\ 4K +to skip in 4 kibibyte blocks. This will speed up reads from faulty media, +at the expense of potentially losing more data. +.TP .B \-S, \-\-stop-at-size If a size was specified with .BR \-s , diff --git a/doc/pv.1.md b/doc/pv.1.md index 132232d..93c8e79 100644 --- a/doc/pv.1.md +++ b/doc/pv.1.md @@ -290,6 +290,20 @@ are unreadable, the total size will not be calculated. : Specify **-E** twice to only report a read error once per file, instead of reporting each byte range skipped. +**-Z ***BYTES***, **\--error-skip-block *****BYTES* + +: When ignoring read errors with **-E**, instead of trying to + adaptively skip by reading small amounts and skipping progressively + larger sections until a read succeeds, move to the next file block + of *BYTES* bytes as soon as an error occurs. There may still be some + shorter skips where the block being skipped coincides with the end + of the transfer buffer. + +: This option can only be used with **-E** and is intended for use + when reading from a block device, such as **-E -Z 4K** to skip in 4 + kibibyte blocks. This will speed up reads from faulty media, at the + expense of potentially losing more data. + **-S, \--stop-at-size** : If a size was specified with **-s**, stop transferring data once diff --git a/src/include/options.h b/src/include/options.h index fca9903..0748401 100644 --- a/src/include/options.h +++ b/src/include/options.h @@ -42,6 +42,7 @@ struct opts_s { /* structure describing run-time options */ unsigned long long size; /* total size of data */ bool no_splice; /* flag set if never to use splice */ unsigned int skip_errors; /* skip read errors counter */ + unsigned long long error_skip_block; /* skip block size, 0 for adaptive */ bool stop_at_size; /* set if we stop at "size" bytes */ bool sync_after_write; /* set if we sync after every write */ bool direct_io; /* set if O_DIRECT is to be used */ diff --git a/src/include/pv-internal.h b/src/include/pv-internal.h index 4292273..a7fd931 100644 --- a/src/include/pv-internal.h +++ b/src/include/pv-internal.h @@ -93,6 +93,7 @@ struct pvstate_s { bool null_terminated_lines; /* lines are null-terminated */ bool no_display; /* do nothing other than pipe data */ unsigned int skip_errors; /* skip read errors counter */ + unsigned long long error_skip_block; /* skip block size, 0 for adaptive */ bool stop_at_size; /* set if we stop at "size" bytes */ bool sync_after_write; /* set if we sync after every write */ bool direct_io; /* set if O_DIRECT is to be used */ diff --git a/src/include/pv.h b/src/include/pv.h index 4d3a4d1..79a5b0f 100644 --- a/src/include/pv.h +++ b/src/include/pv.h @@ -148,6 +148,7 @@ extern void pv_state_bits_set(pvstate_t, bool); extern void pv_state_null_terminated_lines_set(pvstate_t, bool); extern void pv_state_no_display_set(pvstate_t, bool); extern void pv_state_skip_errors_set(pvstate_t, unsigned int); +extern void pv_state_error_skip_block_set(pvstate_t, unsigned long long); extern void pv_state_stop_at_size_set(pvstate_t, bool); extern void pv_state_sync_after_write_set(pvstate_t, bool); extern void pv_state_direct_io_set(pvstate_t, bool); diff --git a/src/main/help.c b/src/main/help.c index 41c5a0a..85c8a2c 100644 --- a/src/main/help.c +++ b/src/main/help.c @@ -368,6 +368,9 @@ void display_help(void) { "-E", "--skip-errors", NULL, N_("skip read errors in input"), { 0, 0, 0, 0} }, + { "-Z", "--error-skip-block", N_("BYTES"), + N_("skip errors in BYTES blocks at a time"), + { 0, 0, 0, 0} }, { "-S", "--stop-at-size", NULL, N_("stop after --size bytes have been transferred"), { 0, 0, 0, 0} }, diff --git a/src/main/main.c b/src/main/main.c index ada0b20..ec428f8 100644 --- a/src/main/main.c +++ b/src/main/main.c @@ -271,6 +271,7 @@ int main(int argc, char **argv) pv_state_bits_set(state, opts->bits); pv_state_null_terminated_lines_set(state, opts->null_terminated_lines); pv_state_skip_errors_set(state, opts->skip_errors); + pv_state_error_skip_block_set(state, opts->error_skip_block); pv_state_stop_at_size_set(state, opts->stop_at_size); pv_state_sync_after_write_set(state, opts->sync_after_write); pv_state_direct_io_set(state, opts->direct_io); diff --git a/src/main/options.c b/src/main/options.c index 94956e9..5a2aee9 100644 --- a/src/main/options.c +++ b/src/main/options.c @@ -169,6 +169,7 @@ opts_t opts_parse(unsigned int argc, char **argv) { "buffer-size", 1, NULL, (int) 'B' }, { "no-splice", 0, NULL, (int) 'C' }, { "skip-errors", 0, NULL, (int) 'E' }, + { "error-skip-block", 1, NULL, (int) 'Z' }, { "stop-at-size", 0, NULL, (int) 'S' }, { "sync", 0, NULL, (int) 'Y' }, { "direct-io", 0, NULL, (int) 'K' }, @@ -185,7 +186,7 @@ opts_t opts_parse(unsigned int argc, char **argv) /*@+nullassign@ */ int option_index = 0; #endif /* HAVE_GETOPT_LONG */ - char *short_options = "hVpteIrab8TA:fnqcWD:s:l0i:w:H:N:F:L:B:CESYKXR:P:d:m:" + char *short_options = "hVpteIrab8TA:fnqcWD:s:l0i:w:H:N:F:L:B:CEZ:SYKXR:P:d:m:" #ifdef ENABLE_DEBUGGING "!:" #endif @@ -281,6 +282,8 @@ opts_t opts_parse(unsigned int argc, char **argv) case 'R': /*@fallthrough@ */ case 'm': + /*@fallthrough@ */ + case 'Z': if (pv_getnum_check(optarg, PV_NUMTYPE_INTEGER) != 0) { /*@-mustfreefresh@ *//* see above */ fprintf(stderr, "%s: -%c: %s\n", opts->program_name, c, _("integer argument expected")); @@ -461,6 +464,9 @@ opts_t opts_parse(unsigned int argc, char **argv) case 'E': opts->skip_errors++; break; + case 'Z': + opts->error_skip_block = pv_getnum_ull(optarg); + break; case 'S': opts->stop_at_size = true; break; @@ -605,6 +611,10 @@ opts_t opts_parse(unsigned int argc, char **argv) opts->bytes = true; } + /* If -Z was given but not -E, pretend one -E was given too. */ + if (opts->error_skip_block > 0 && 0 == opts->skip_errors) + opts->skip_errors = 1; + /* * Store remaining command-line arguments. */ diff --git a/src/pv/state.c b/src/pv/state.c index bcbf00a..1dccd03 100644 --- a/src/pv/state.c +++ b/src/pv/state.c @@ -207,6 +207,11 @@ void pv_state_skip_errors_set(pvstate_t state, unsigned int val) state->skip_errors = val; }; +void pv_state_error_skip_block_set(pvstate_t state, unsigned long long val) +{ + state->error_skip_block = val; +} + void pv_state_stop_at_size_set(pvstate_t state, bool val) { state->stop_at_size = val; diff --git a/src/pv/transfer.c b/src/pv/transfer.c index c289e8d..ceeaf6e 100644 --- a/src/pv/transfer.c +++ b/src/pv/transfer.c @@ -402,12 +402,21 @@ static int pv__transfer_read(pvstate_t state, int fd, int *eof_in, int *eof_out, return 1; } - if (state->read_errors_in_a_row < 10) { - amount_to_skip = state->read_errors_in_a_row < 5 ? 1 : 2; - } else if (state->read_errors_in_a_row < 20) { - amount_to_skip = 1 << (state->read_errors_in_a_row - 10); - } else { - amount_to_skip = 512; + /* + * If a non-zero error skip block size was given, just use that, + * otherwise start small and ramp up based on the number of errors + * in a row. + */ + if (state->error_skip_block > 0) { + amount_to_skip = state->error_skip_block; + } else { + if (state->read_errors_in_a_row < 10) { + amount_to_skip = state->read_errors_in_a_row < 5 ? 1 : 2; + } else if (state->read_errors_in_a_row < 20) { + amount_to_skip = 1 << (state->read_errors_in_a_row - 10); + } else { + amount_to_skip = 512; + } } /*