[Libguestfs] [PATCH libnbd 1/2] copy: Handle --destination-is-zero in the dst->ops->asynch_zero

Currently our function to zero the destination (multi-thread-copying.c:fill_dst_range_with_zeroes) handles the --destination-is-zero flag by short-cutting. However the disadvantage of this is that the destination does not get to see the zero operation, making it impossible to handle coalescing of small requests into larger ones. That in turn is necessary in order to support the destination preferred block size. Push the handling of this flag down into dst->ops->asynch_zero. We only have to change file-ops and nbd-ops. null-ops already does nothing for zeroing, and pipe-ops is only called in the synchronous case and doesn't have an asynch_zero callback (that can be called). This slightly complicates the code, in exchange for making future changes possible. It is a refactoring that should not change the behaviour of the program. --- copy/file-ops.c | 11 +++++++++++ copy/multi-thread-copying.c | 7 +------ copy/nbd-ops.c | 9 +++++++++ 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/copy/file-ops.c b/copy/file-ops.c index ab3787545c..f4ec2ae445 100644 --- a/copy/file-ops.c +++ b/copy/file-ops.c @@ -614,8 +614,19 @@ file_asynch_zero (struct rw *rw, struct command *command, { int dummy = 0; + /* If the destination is already zero, do nothing. However we must + * call the callback so the command is freed. + */ + if (destination_is_zero) + goto do_callback; + + /* Files don't support async so synchronously zero (if supported) + * and call the callback so the command is freed. + */ if (!file_synch_zero (rw, command->offset, command->slice.len, allocate)) return false; + + do_callback: cb.callback (cb.user_data, &dummy); return true; } diff --git a/copy/multi-thread-copying.c b/copy/multi-thread-copying.c index 06cdb8eef6..1ec2e3dd1d 100644 --- a/copy/multi-thread-copying.c +++ b/copy/multi-thread-copying.c @@ -546,7 +546,7 @@ finished_read (void *vp, int *error) * command line flags this could mean: * * --destination-is-zero: - * do nothing + * do nothing (handled by dst->ops->asynch_zero) * * --allocated: write zeroes allocating space using an efficient * zeroing command or writing a command of zeroes @@ -563,9 +563,6 @@ fill_dst_range_with_zeroes (struct command *command) char *data; size_t data_size; - if (destination_is_zero) - goto free_and_return; - /* Try efficient zeroing. */ if (dst->ops->asynch_zero (dst, command, (nbd_completion_callback) { @@ -595,8 +592,6 @@ fill_dst_range_with_zeroes (struct command *command) command->offset += len; } free (data); - - free_and_return: free_command (command); } diff --git a/copy/nbd-ops.c b/copy/nbd-ops.c index 3bc26ba613..1ffe6a213f 100644 --- a/copy/nbd-ops.c +++ b/copy/nbd-ops.c @@ -336,6 +336,15 @@ nbd_ops_asynch_zero (struct rw *rw, struct command *command, { struct rw_nbd *rwn = (struct rw_nbd *) rw; + /* If the destination is already zero, do nothing. However we must + * call the callback so the command is freed. + */ + if (destination_is_zero) { + int dummy = 0; + cb.callback (cb.user_data, &dummy); + return true; + } + if (!rwn->can_zero) return false; -- 2.37.0.rc2