This example failed to check the *error parameter to the completion and extent callbacks.
- If the source NBD server failed a read, we wrote stale data from the request buffer to the destination image, corrupting the image. - If the destination NBD server failed a write or zero command, we ignored the error, leaving previous content on the destination image, corrupting the image. - Error in the extents callbacks were ignored. I'm not sure if this was a real problem, but it is a very bad example. In all cases, the copy would end with zero exit code creating a corrupted image. Fixed by failing the copy if read, write, zero commands completed with non-zero *error parameter. If an extent callback completed with non-zero *error, we log the error and abort the callback, leaving the extents array as NULL. If extents completion callback completed with non-zero *error we don't need to check it, since we already handle a NULL extents array by disabling can_extents. Here is an example failure: $ ./copy-libev nbd+unix:///?socket=/tmp/src.sock nbd+unix:///?socket=/tmp/dst.sock copy-libev: r0: read failed: Input/output error $ echo $? 1 Signed-off-by: Nir Soffer <nsof...@redhat.com> --- examples/copy-libev.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/examples/copy-libev.c b/examples/copy-libev.c index 13db898a..ad50b64c 100644 --- a/examples/copy-libev.c +++ b/examples/copy-libev.c @@ -184,20 +184,26 @@ static int extent_callback (void *user_data, const char *metacontext, uint64_t offset, uint32_t *entries, size_t nr_entries, int *error) { struct request *r = user_data; if (strcmp (metacontext, LIBNBD_CONTEXT_BASE_ALLOCATION) != 0) { DEBUG ("Unexpected meta context: %s", metacontext); return 1; } + if (*error) { + DEBUG ("r%zu: extent callback for %s failed: %s", + r->index, LIBNBD_CONTEXT_BASE_ALLOCATION, strerror (*error)); + return 1; + } + /* Libnbd returns uint32_t pair (length, flags) for each extent. */ extents_len = nr_entries / 2; extents = malloc (extents_len * sizeof *extents); if (extents == NULL) FAIL ("Cannot allocated extents: %s", strerror (errno)); /* Copy libnbd entries to extents array. */ for (int i = 0, j = 0; i < extents_len; i++, j=i*2) { extents[i].length = entries[j]; @@ -240,20 +246,26 @@ static void wake_up_requests () static int extents_completed (void *user_data, int *error) { struct request *r = (struct request *)user_data; DEBUG ("r%zu: extents completed time=%.6f", r->index, ev_now (loop) - r->started); extents_in_progress = false; + /* + * If extents failed (*error != 0), the extent callback was not + * called, or called with an error, so we did not allocate new + * extents array. + */ + if (extents == NULL) { DEBUG ("r%zu: received no extents, disabling extents", r->index); src.can_extents = false; } /* Start the request to process recvievd extents. This must be done on the * next loop iteration, to avoid deadlock if we need to start a read. */ start_request_soon (r); @@ -434,20 +446,23 @@ start_read(struct request *r) 0); if (cookie == -1) FAIL ("Cannot start read: %s", nbd_get_error ()); } static int read_completed (void *user_data, int *error) { struct request *r = (struct request *)user_data; + if (*error) + FAIL ("r%zu: read failed: %s", r->index, strerror (*error)); + DEBUG ("r%zu: read completed offset=%" PRIi64 " len=%zu", r->index, r->offset, r->length); if (dst.can_zero && is_zero (r->data, r->length)) start_zero (r); else start_write (r); return 1; } @@ -489,20 +504,24 @@ start_zero(struct request *r) if (cookie == -1) FAIL ("Cannot start zero: %s", nbd_get_error ()); } /* Called when async copy or zero request completed. */ static int request_completed (void *user_data, int *error) { struct request *r = (struct request *)user_data; + if (*error) + FAIL ("r%zu: %s failed: %s", + r->index, request_state (r), strerror (*error)); + written += r->length; DEBUG ("r%zu: %s completed offset=%" PRIi64 " len=%zu, time=%.6f", r->index, request_state (r), r->offset, r->length, ev_now (loop) - r->started); if (written == size) { /* The last write completed. Stop all watchers and break out * from the event loop. */ -- 2.34.1 _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs