Series comparison

-[Qemu-devel] [PULL 0/3] Block patches
+[PULL for-6.1 0/3] Block patches
-The following changes since commit ab08440a4ee09032d1a9cb22fdcab23bc7e1c656:
+The following changes since commit 3521ade3510eb5cefb2e27a101667f25dad89935:
-  Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20180702' into staging (2018-07-02 17:57:46 +0100)
+  Merge remote-tracking branch 'remotes/thuth-gitlab/tags/pull-request-2021-07-29' into staging (2021-07-29 13:17:20 +0100)
 are available in the Git repository at:
-  git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request
+  https://gitlab.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to 9ded4a0114968e98b41494fc035ba14f84cdf700:
+for you to fetch changes up to cc8eecd7f105a1dff5876adeb238a14696061a4a:
-  backup: Use copy offloading (2018-07-02 23:23:45 -0400)
+  MAINTAINERS: Added myself as a reviewer for the NVMe Block Driver (2021-07-29 17:17:34 +0100)
 ----------------------------------------------------------------
-Block backup patches
+Pull request
 The main fix here is for io_uring. Spurious -EAGAIN errors can happen and the
 request needs to be resubmitted.
 The MAINTAINERS changes carry no risk and we might as well include them in QEMU
 .1.
 ----------------------------------------------------------------
-Fam Zheng (3):
+Fabian Ebner (1):
-  block: Fix parameter checking in bdrv_co_copy_range_internal
+  block/io_uring: resubmit when result is -EAGAIN
   block: Honour BDRV_REQ_NO_SERIALISING in copy range
   backup: Use copy offloading
- block/backup.c        | 150 ++++++++++++++++++++++++++++++------------
+Philippe Mathieu-Daudé (1):
- block/io.c            |  35 +++++-----
+  MAINTAINERS: Added myself as a reviewer for the NVMe Block Driver
- block/trace-events    |   1 +
- include/block/block.h |   5 +-
+Stefano Garzarella (1):
-files changed, 132 insertions(+), 59 deletions(-)
+  MAINTAINERS: add Stefano Garzarella as io_uring reviewer
  MAINTAINERS      |  2 ++
  block/io_uring.c | 16 +++++++++++++++-
 files changed, 17 insertions(+), 1 deletion(-)
 --
-.17.1
+.31.1

-[Qemu-devel] [PULL 3/3] backup: Use copy offloading
+[PULL for-6.1 1/3] MAINTAINERS: add Stefano Garzarella as io_uring reviewer
-From: Fam Zheng <famz@redhat.com>
+From: Stefano Garzarella <sgarzare@redhat.com>
-The implementation is similar to the 'qemu-img convert'. In the
+I've been working with io_uring for a while so I'd like to help
-beginning of the job, offloaded copy is attempted. If it fails, further
+with reviews.
 I/O will go through the existing bounce buffer code path.
-Then, as Kevin pointed out, both this and qemu-img convert can benefit
+Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
-from a local check if one request fails because of, for example, the
+Message-Id: <20210728131515.131045-1-sgarzare@redhat.com>
-offset is beyond EOF, but another may well be accepted by the protocol
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-layer. This will be implemented separately.
+---
  MAINTAINERS | 1 +
 file changed, 1 insertion(+)
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+diff --git a/MAINTAINERS b/MAINTAINERS
-Signed-off-by: Fam Zheng <famz@redhat.com>
+index XXXXXXX..XXXXXXX 100644
-Message-id: 20180703023758.14422-4-famz@redhat.com
+--- a/MAINTAINERS
-Signed-off-by: Jeff Cody <jcody@redhat.com>
++++ b/MAINTAINERS
----
+@@ -XXX,XX +XXX,XX @@ Linux io_uring
- block/backup.c     | 150 ++++++++++++++++++++++++++++++++-------------
+ M: Aarushi Mehta <mehta.aaru20@gmail.com>
- block/trace-events |   1 +
+ M: Julia Suvorova <jusual@redhat.com>
-files changed, 110 insertions(+), 41 deletions(-)
+ M: Stefan Hajnoczi <stefanha@redhat.com>
 +R: Stefano Garzarella <sgarzare@redhat.com>
  L: qemu-block@nongnu.org
  S: Maintained
  F: block/io_uring.c
 --
 .31.1
-diff --git a/block/backup.c b/block/backup.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/backup.c
-+++ b/block/backup.c
-@@ -XXX,XX +XXX,XX @@ typedef struct BackupBlockJob {
-     QLIST_HEAD(, CowRequest) inflight_reqs;
-     HBitmap *copy_bitmap;
-+    bool use_copy_range;
-+    int64_t copy_range_size;
- } BackupBlockJob;
- static const BlockJobDriver backup_job_driver;
-@@ -XXX,XX +XXX,XX @@ static void cow_request_end(CowRequest *req)
-     qemu_co_queue_restart_all(&req->wait_queue);
- }
-+/* Copy range to target with a bounce buffer and return the bytes copied. If
-+ * error occured, return a negative error number */
-+static int coroutine_fn backup_cow_with_bounce_buffer(BackupBlockJob *job,
-+                                                      int64_t start,
-+                                                      int64_t end,
-+                                                      bool is_write_notifier,
-+                                                      bool *error_is_read,
-+                                                      void **bounce_buffer)
-+{
-+    int ret;
-+    struct iovec iov;
-+    QEMUIOVector qiov;
-+    BlockBackend *blk = job->common.blk;
-+    int nbytes;
-+
-+    hbitmap_reset(job->copy_bitmap, start / job->cluster_size, 1);
-+    nbytes = MIN(job->cluster_size, job->len - start);
-+    if (!*bounce_buffer) {
-+        *bounce_buffer = blk_blockalign(blk, job->cluster_size);
-+    }
-+    iov.iov_base = *bounce_buffer;
-+    iov.iov_len = nbytes;
-+    qemu_iovec_init_external(&qiov, &iov, 1);
-+
-+    ret = blk_co_preadv(blk, start, qiov.size, &qiov,
-+                        is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0);
-+    if (ret < 0) {
-+        trace_backup_do_cow_read_fail(job, start, ret);
-+        if (error_is_read) {
-+            *error_is_read = true;
-+        }
-+        goto fail;
-+    }
-+
-+    if (qemu_iovec_is_zero(&qiov)) {
-+        ret = blk_co_pwrite_zeroes(job->target, start,
-+                                   qiov.size, BDRV_REQ_MAY_UNMAP);
-+    } else {
-+        ret = blk_co_pwritev(job->target, start,
-+                             qiov.size, &qiov,
-+                             job->compress ? BDRV_REQ_WRITE_COMPRESSED : 0);
-+    }
-+    if (ret < 0) {
-+        trace_backup_do_cow_write_fail(job, start, ret);
-+        if (error_is_read) {
-+            *error_is_read = false;
-+        }
-+        goto fail;
-+    }
-+
-+    return nbytes;
-+fail:
-+    hbitmap_set(job->copy_bitmap, start / job->cluster_size, 1);
-+    return ret;
-+
-+}
-+
-+/* Copy range to target and return the bytes copied. If error occured, return a
-+ * negative error number. */
-+static int coroutine_fn backup_cow_with_offload(BackupBlockJob *job,
-+                                                int64_t start,
-+                                                int64_t end,
-+                                                bool is_write_notifier)
-+{
-+    int ret;
-+    int nr_clusters;
-+    BlockBackend *blk = job->common.blk;
-+    int nbytes;
-+
-+    assert(QEMU_IS_ALIGNED(job->copy_range_size, job->cluster_size));
-+    nbytes = MIN(job->copy_range_size, end - start);
-+    nr_clusters = DIV_ROUND_UP(nbytes, job->cluster_size);
-+    hbitmap_reset(job->copy_bitmap, start / job->cluster_size,
-+                  nr_clusters);
-+    ret = blk_co_copy_range(blk, start, job->target, start, nbytes,
-+                            is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0);
-+    if (ret < 0) {
-+        trace_backup_do_cow_copy_range_fail(job, start, ret);
-+        hbitmap_set(job->copy_bitmap, start / job->cluster_size,
-+                    nr_clusters);
-+        return ret;
-+    }
-+
-+    return nbytes;
-+}
-+
- static int coroutine_fn backup_do_cow(BackupBlockJob *job,
-                                       int64_t offset, uint64_t bytes,
-                                       bool *error_is_read,
-                                       bool is_write_notifier)
- {
--    BlockBackend *blk = job->common.blk;
-     CowRequest cow_request;
--    struct iovec iov;
--    QEMUIOVector bounce_qiov;
--    void *bounce_buffer = NULL;
-     int ret = 0;
-     int64_t start, end; /* bytes */
--    int n; /* bytes */
-+    void *bounce_buffer = NULL;
-     qemu_co_rwlock_rdlock(&job->flush_rwlock);
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
-     wait_for_overlapping_requests(job, start, end);
-     cow_request_begin(&cow_request, job, start, end);
--    for (; start < end; start += job->cluster_size) {
-+    while (start < end) {
-         if (!hbitmap_get(job->copy_bitmap, start / job->cluster_size)) {
-             trace_backup_do_cow_skip(job, start);
-+            start += job->cluster_size;
-             continue; /* already copied */
-         }
--        hbitmap_reset(job->copy_bitmap, start / job->cluster_size, 1);
-         trace_backup_do_cow_process(job, start);
--        n = MIN(job->cluster_size, job->len - start);
--
--        if (!bounce_buffer) {
--            bounce_buffer = blk_blockalign(blk, job->cluster_size);
--        }
--        iov.iov_base = bounce_buffer;
--        iov.iov_len = n;
--        qemu_iovec_init_external(&bounce_qiov, &iov, 1);
--
--        ret = blk_co_preadv(blk, start, bounce_qiov.size, &bounce_qiov,
--                            is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0);
--        if (ret < 0) {
--            trace_backup_do_cow_read_fail(job, start, ret);
--            if (error_is_read) {
--                *error_is_read = true;
-+        if (job->use_copy_range) {
-+            ret = backup_cow_with_offload(job, start, end, is_write_notifier);
-+            if (ret < 0) {
-+                job->use_copy_range = false;
-             }
--            hbitmap_set(job->copy_bitmap, start / job->cluster_size, 1);
--            goto out;
-         }
--
--        if (buffer_is_zero(iov.iov_base, iov.iov_len)) {
--            ret = blk_co_pwrite_zeroes(job->target, start,
--                                       bounce_qiov.size, BDRV_REQ_MAY_UNMAP);
--        } else {
--            ret = blk_co_pwritev(job->target, start,
--                                 bounce_qiov.size, &bounce_qiov,
--                                 job->compress ? BDRV_REQ_WRITE_COMPRESSED : 0);
-+        if (!job->use_copy_range) {
-+            ret = backup_cow_with_bounce_buffer(job, start, end, is_write_notifier,
-+                                                error_is_read, &bounce_buffer);
-         }
-         if (ret < 0) {
--            trace_backup_do_cow_write_fail(job, start, ret);
--            if (error_is_read) {
--                *error_is_read = false;
--            }
--            hbitmap_set(job->copy_bitmap, start / job->cluster_size, 1);
--            goto out;
-+            break;
-         }
-         /* Publish progress, guest I/O counts as progress too.  Note that the
-          * offset field is an opaque progress value, it is not a disk offset.
-          */
--        job->bytes_read += n;
--        job_progress_update(&job->common.job, n);
-+        start += ret;
-+        job->bytes_read += ret;
-+        job_progress_update(&job->common.job, ret);
-+        ret = 0;
-     }
--out:
-     if (bounce_buffer) {
-         qemu_vfree(bounce_buffer);
-     }
-@@ -XXX,XX +XXX,XX @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
-     } else {
-         job->cluster_size = MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
-     }
-+    job->use_copy_range = true;
-+    job->copy_range_size = MIN_NON_ZERO(blk_get_max_transfer(job->common.blk),
-+                                        blk_get_max_transfer(job->target));
-+    job->copy_range_size = MAX(job->cluster_size,
-+                               QEMU_ALIGN_UP(job->copy_range_size,
-+                                             job->cluster_size));
-     /* Required permissions are already taken with target's blk_new() */
-     block_job_add_bdrv(&job->common, "target", target, 0, BLK_PERM_ALL,
-diff --git a/block/trace-events b/block/trace-events
-index XXXXXXX..XXXXXXX 100644
---- a/block/trace-events
-+++ b/block/trace-events
-@@ -XXX,XX +XXX,XX @@ backup_do_cow_skip(void *job, int64_t start) "job %p start %"PRId64
- backup_do_cow_process(void *job, int64_t start) "job %p start %"PRId64
- backup_do_cow_read_fail(void *job, int64_t start, int ret) "job %p start %"PRId64" ret %d"
- backup_do_cow_write_fail(void *job, int64_t start, int ret) "job %p start %"PRId64" ret %d"
-+backup_do_cow_copy_range_fail(void *job, int64_t start, int ret) "job %p start %"PRId64" ret %d"
- # blockdev.c
- qmp_block_job_cancel(void *job) "job %p"
---
-.17.1

-[Qemu-devel] [PULL 2/3] block: Honour BDRV_REQ_NO_SERIALISING in copy range
+[PULL for-6.1 2/3] block/io_uring: resubmit when result is -EAGAIN
-From: Fam Zheng <famz@redhat.com>
+From: Fabian Ebner <f.ebner@proxmox.com>
-This semantics is needed by drive-backup so implement it before using
+Linux SCSI can throw spurious -EAGAIN in some corner cases in its
-this API there.
+completion path, which will end up being the result in the completed
 io_uring request.
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+Resubmitting such requests should allow block jobs to complete, even
-Signed-off-by: Fam Zheng <famz@redhat.com>
+if such spurious errors are encountered.
-Message-id: 20180703023758.14422-3-famz@redhat.com
-Signed-off-by: Jeff Cody <jcody@redhat.com>
+Co-authored-by: Stefan Hajnoczi <stefanha@gmail.com>
 Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
 Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
 Message-id: 20210729091029.65369-1-f.ebner@proxmox.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/io.c            | 6 ++++--
+ block/io_uring.c | 16 +++++++++++++++-
- include/block/block.h | 5 +++--
+file changed, 15 insertions(+), 1 deletion(-)
 files changed, 7 insertions(+), 4 deletions(-)
-diff --git a/block/io.c b/block/io.c
+diff --git a/block/io_uring.c b/block/io_uring.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/io.c
+--- a/block/io_uring.c
-+++ b/block/io.c
++++ b/block/io_uring.c
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_copy_range_internal(BdrvChild *src,
+@@ -XXX,XX +XXX,XX @@ static void luring_process_completions(LuringState *s)
-     tracked_request_begin(&dst_req, dst->bs, dst_offset,
+         total_bytes = ret + luringcb->total_read;
-                           bytes, BDRV_TRACKED_WRITE);
+         if (ret < 0) {
--    wait_serialising_requests(&src_req);
+-            if (ret == -EINTR) {
--    wait_serialising_requests(&dst_req);
++            /*
-+    if (!(flags & BDRV_REQ_NO_SERIALISING)) {
++             * Only writev/readv/fsync requests on regular files or host block
-+        wait_serialising_requests(&src_req);
++             * devices are submitted. Therefore -EAGAIN is not expected but it's
-+        wait_serialising_requests(&dst_req);
++             * known to happen sometimes with Linux SCSI. Submit again and hope
-+    }
++             * the request completes successfully.
-     if (recurse_src) {
++             *
-         ret = src->bs->drv->bdrv_co_copy_range_from(src->bs,
++             * For more information, see:
-                                                     src, src_offset,
++             * https://lore.kernel.org/io-uring/20210727165811.284510-3-axboe@kernel.dk/T/#u
-diff --git a/include/block/block.h b/include/block/block.h
++             *
-index XXXXXXX..XXXXXXX 100644
++             * If the code is changed to submit other types of requests in the
---- a/include/block/block.h
++             * future, then this workaround may need to be extended to deal with
-+++ b/include/block/block.h
++             * genuine -EAGAIN results that should not be resubmitted
-@@ -XXX,XX +XXX,XX @@ void bdrv_unregister_buf(BlockDriverState *bs, void *host);
++             * immediately.
-  * @dst: Destination child to copy data to
++             */
-  * @dst_offset: offset in @dst image to write data
++            if (ret == -EINTR || ret == -EAGAIN) {
-  * @bytes: number of bytes to copy
+                 luring_resubmit(s, luringcb);
-- * @flags: request flags. Must be one of:
+                 continue;
-- *         0 - actually read data from src;
+             }
 + * @flags: request flags. Supported flags:
   *         BDRV_REQ_ZERO_WRITE - treat the @src range as zero data and do zero
   *                               write on @dst as if bdrv_co_pwrite_zeroes is
   *                               called. Used to simplify caller code, or
   *                               during BlockDriver.bdrv_co_copy_range_from()
   *                               recursion.
 + *         BDRV_REQ_NO_SERIALISING - do not serialize with other overlapping
 + *                                   requests currently in flight.
   *
   * Returns: 0 if succeeded; negative error code if failed.
   **/
 --
-.17.1
+.31.1

-[Qemu-devel] [PULL 1/3] block: Fix parameter checking in bdrv_co_copy_range_internal
+[PULL for-6.1 3/3] MAINTAINERS: Added myself as a reviewer for the NVMe Block Driver
-From: Fam Zheng <famz@redhat.com>
+From: Philippe Mathieu-Daudé <philmd@redhat.com>
-src may be NULL if BDRV_REQ_ZERO_WRITE flag is set, in this case only
+I'm interested in following the activity around the NVMe bdrv.
 check dst and dst->bs. This bug was introduced when moving in the
 request tracking code from bdrv_co_copy_range, in 37aec7d75eb.
-This especially fixes the possible segfault when initializing src_bs
+Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
-with a NULL src.
+Message-id: 20210728183340.2018313-1-philmd@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  MAINTAINERS | 1 +
 file changed, 1 insertion(+)
-Signed-off-by: Fam Zheng <famz@redhat.com>
+diff --git a/MAINTAINERS b/MAINTAINERS
-Message-id: 20180703023758.14422-2-famz@redhat.com
+index XXXXXXX..XXXXXXX 100644
-Reviewed-by: Jeff Cody <jcody@redhat.com>
+--- a/MAINTAINERS
-Signed-off-by: Jeff Cody <jcody@redhat.com>
++++ b/MAINTAINERS
----
+@@ -XXX,XX +XXX,XX @@ F: block/null.c
- block/io.c | 29 +++++++++++++++--------------
+ NVMe Block Driver
-file changed, 15 insertions(+), 14 deletions(-)
+ M: Stefan Hajnoczi <stefanha@redhat.com>
  R: Fam Zheng <fam@euphon.net>
 +R: Philippe Mathieu-Daudé <philmd@redhat.com>
  L: qemu-block@nongnu.org
  S: Supported
  F: block/nvme*
 --
 .31.1
-diff --git a/block/io.c b/block/io.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/io.c
-+++ b/block/io.c
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_copy_range_internal(BdrvChild *src,
-                                                     bool recurse_src)
- {
-     BdrvTrackedRequest src_req, dst_req;
--    BlockDriverState *src_bs = src->bs;
--    BlockDriverState *dst_bs = dst->bs;
-     int ret;
--    if (!src || !dst || !src->bs || !dst->bs) {
-+    if (!dst || !dst->bs) {
-         return -ENOMEDIUM;
-     }
--    ret = bdrv_check_byte_request(src->bs, src_offset, bytes);
--    if (ret) {
--        return ret;
--    }
--
-     ret = bdrv_check_byte_request(dst->bs, dst_offset, bytes);
-     if (ret) {
-         return ret;
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_copy_range_internal(BdrvChild *src,
-         return bdrv_co_pwrite_zeroes(dst, dst_offset, bytes, flags);
-     }
-+    if (!src || !src->bs) {
-+        return -ENOMEDIUM;
-+    }
-+    ret = bdrv_check_byte_request(src->bs, src_offset, bytes);
-+    if (ret) {
-+        return ret;
-+    }
-+
-     if (!src->bs->drv->bdrv_co_copy_range_from
-         || !dst->bs->drv->bdrv_co_copy_range_to
-         || src->bs->encrypted || dst->bs->encrypted) {
-         return -ENOTSUP;
-     }
--    bdrv_inc_in_flight(src_bs);
--    bdrv_inc_in_flight(dst_bs);
--    tracked_request_begin(&src_req, src_bs, src_offset,
-+    bdrv_inc_in_flight(src->bs);
-+    bdrv_inc_in_flight(dst->bs);
-+    tracked_request_begin(&src_req, src->bs, src_offset,
-                           bytes, BDRV_TRACKED_READ);
--    tracked_request_begin(&dst_req, dst_bs, dst_offset,
-+    tracked_request_begin(&dst_req, dst->bs, dst_offset,
-                           bytes, BDRV_TRACKED_WRITE);
-     wait_serialising_requests(&src_req);
-@@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_copy_range_internal(BdrvChild *src,
-     }
-     tracked_request_end(&src_req);
-     tracked_request_end(&dst_req);
--    bdrv_dec_in_flight(src_bs);
--    bdrv_dec_in_flight(dst_bs);
-+    bdrv_dec_in_flight(src->bs);
-+    bdrv_dec_in_flight(dst->bs);
-     return ret;
- }
---
-.17.1

From: Fam Zheng <famz@redhat.com>

src may be NULL if BDRV_REQ_ZERO_WRITE flag is set, in this case only
check dst and dst->bs. This bug was introduced when moving in the
request tracking code from bdrv_co_copy_range, in 37aec7d75eb.

This especially fixes the possible segfault when initializing src_bs
with a NULL src.

Signed-off-by: Fam Zheng <famz@redhat.com>
Message-id: 20180703023758.14422-2-famz@redhat.com
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/io.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/block/io.c b/block/io.c
index XXXXXXX..XXXXXXX 100644
--- a/block/io.c
+++ b/block/io.c
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_copy_range_internal(BdrvChild *src,
                                                     bool recurse_src)
 {
     BdrvTrackedRequest src_req, dst_req;
-    BlockDriverState *src_bs = src->bs;
-    BlockDriverState *dst_bs = dst->bs;
     int ret;
 
-    if (!src || !dst || !src->bs || !dst->bs) {
+    if (!dst || !dst->bs) {
         return -ENOMEDIUM;
     }
-    ret = bdrv_check_byte_request(src->bs, src_offset, bytes);
-    if (ret) {
-        return ret;
-    }
-
     ret = bdrv_check_byte_request(dst->bs, dst_offset, bytes);
     if (ret) {
         return ret;
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_copy_range_internal(BdrvChild *src,
         return bdrv_co_pwrite_zeroes(dst, dst_offset, bytes, flags);
     }
 
+    if (!src || !src->bs) {
+        return -ENOMEDIUM;
+    }
+    ret = bdrv_check_byte_request(src->bs, src_offset, bytes);
+    if (ret) {
+        return ret;
+    }
+
     if (!src->bs->drv->bdrv_co_copy_range_from
         || !dst->bs->drv->bdrv_co_copy_range_to
         || src->bs->encrypted || dst->bs->encrypted) {
         return -ENOTSUP;
     }
-    bdrv_inc_in_flight(src_bs);
-    bdrv_inc_in_flight(dst_bs);
-    tracked_request_begin(&src_req, src_bs, src_offset,
+    bdrv_inc_in_flight(src->bs);
+    bdrv_inc_in_flight(dst->bs);
+    tracked_request_begin(&src_req, src->bs, src_offset,
                           bytes, BDRV_TRACKED_READ);
-    tracked_request_begin(&dst_req, dst_bs, dst_offset,
+    tracked_request_begin(&dst_req, dst->bs, dst_offset,
                           bytes, BDRV_TRACKED_WRITE);
 
     wait_serialising_requests(&src_req);
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_copy_range_internal(BdrvChild *src,
     }
     tracked_request_end(&src_req);
     tracked_request_end(&dst_req);
-    bdrv_dec_in_flight(src_bs);
-    bdrv_dec_in_flight(dst_bs);
+    bdrv_dec_in_flight(src->bs);
+    bdrv_dec_in_flight(dst->bs);
     return ret;
 }
 
-- 
2.17.1

From: Fam Zheng <famz@redhat.com>

This semantics is needed by drive-backup so implement it before using
this API there.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-id: 20180703023758.14422-3-famz@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/io.c            | 6 ++++--
 include/block/block.h | 5 +++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/block/io.c b/block/io.c
index XXXXXXX..XXXXXXX 100644
--- a/block/io.c
+++ b/block/io.c
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_copy_range_internal(BdrvChild *src,
     tracked_request_begin(&dst_req, dst->bs, dst_offset,
                           bytes, BDRV_TRACKED_WRITE);
 
-    wait_serialising_requests(&src_req);
-    wait_serialising_requests(&dst_req);
+    if (!(flags & BDRV_REQ_NO_SERIALISING)) {
+        wait_serialising_requests(&src_req);
+        wait_serialising_requests(&dst_req);
+    }
     if (recurse_src) {
         ret = src->bs->drv->bdrv_co_copy_range_from(src->bs,
                                                     src, src_offset,
diff --git a/include/block/block.h b/include/block/block.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -XXX,XX +XXX,XX @@ void bdrv_unregister_buf(BlockDriverState *bs, void *host);
  * @dst: Destination child to copy data to
  * @dst_offset: offset in @dst image to write data
  * @bytes: number of bytes to copy
- * @flags: request flags. Must be one of:
- *         0 - actually read data from src;
+ * @flags: request flags. Supported flags:
  *         BDRV_REQ_ZERO_WRITE - treat the @src range as zero data and do zero
  *                               write on @dst as if bdrv_co_pwrite_zeroes is
  *                               called. Used to simplify caller code, or
  *                               during BlockDriver.bdrv_co_copy_range_from()
  *                               recursion.
+ *         BDRV_REQ_NO_SERIALISING - do not serialize with other overlapping
+ *                                   requests currently in flight.
  *
  * Returns: 0 if succeeded; negative error code if failed.
  **/
-- 
2.17.1

From: Fam Zheng <famz@redhat.com>

The implementation is similar to the 'qemu-img convert'. In the
beginning of the job, offloaded copy is attempted. If it fails, further
I/O will go through the existing bounce buffer code path.

Then, as Kevin pointed out, both this and qemu-img convert can benefit
from a local check if one request fails because of, for example, the
offset is beyond EOF, but another may well be accepted by the protocol
layer. This will be implemented separately.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-id: 20180703023758.14422-4-famz@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/backup.c     | 150 ++++++++++++++++++++++++++++++++-------------
 block/trace-events |   1 +
 2 files changed, 110 insertions(+), 41 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index XXXXXXX..XXXXXXX 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -XXX,XX +XXX,XX @@ typedef struct BackupBlockJob {
     QLIST_HEAD(, CowRequest) inflight_reqs;
 
     HBitmap *copy_bitmap;
+    bool use_copy_range;
+    int64_t copy_range_size;
 } BackupBlockJob;
 
 static const BlockJobDriver backup_job_driver;
@@ -XXX,XX +XXX,XX @@ static void cow_request_end(CowRequest *req)
     qemu_co_queue_restart_all(&req->wait_queue);
 }
 
+/* Copy range to target with a bounce buffer and return the bytes copied. If
+ * error occured, return a negative error number */
+static int coroutine_fn backup_cow_with_bounce_buffer(BackupBlockJob *job,
+                                                      int64_t start,
+                                                      int64_t end,
+                                                      bool is_write_notifier,
+                                                      bool *error_is_read,
+                                                      void **bounce_buffer)
+{
+    int ret;
+    struct iovec iov;
+    QEMUIOVector qiov;
+    BlockBackend *blk = job->common.blk;
+    int nbytes;
+
+    hbitmap_reset(job->copy_bitmap, start / job->cluster_size, 1);
+    nbytes = MIN(job->cluster_size, job->len - start);
+    if (!*bounce_buffer) {
+        *bounce_buffer = blk_blockalign(blk, job->cluster_size);
+    }
+    iov.iov_base = *bounce_buffer;
+    iov.iov_len = nbytes;
+    qemu_iovec_init_external(&qiov, &iov, 1);
+
+    ret = blk_co_preadv(blk, start, qiov.size, &qiov,
+                        is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0);
+    if (ret < 0) {
+        trace_backup_do_cow_read_fail(job, start, ret);
+        if (error_is_read) {
+            *error_is_read = true;
+        }
+        goto fail;
+    }
+
+    if (qemu_iovec_is_zero(&qiov)) {
+        ret = blk_co_pwrite_zeroes(job->target, start,
+                                   qiov.size, BDRV_REQ_MAY_UNMAP);
+    } else {
+        ret = blk_co_pwritev(job->target, start,
+                             qiov.size, &qiov,
+                             job->compress ? BDRV_REQ_WRITE_COMPRESSED : 0);
+    }
+    if (ret < 0) {
+        trace_backup_do_cow_write_fail(job, start, ret);
+        if (error_is_read) {
+            *error_is_read = false;
+        }
+        goto fail;
+    }
+
+    return nbytes;
+fail:
+    hbitmap_set(job->copy_bitmap, start / job->cluster_size, 1);
+    return ret;
+
+}
+
+/* Copy range to target and return the bytes copied. If error occured, return a
+ * negative error number. */
+static int coroutine_fn backup_cow_with_offload(BackupBlockJob *job,
+                                                int64_t start,
+                                                int64_t end,
+                                                bool is_write_notifier)
+{
+    int ret;
+    int nr_clusters;
+    BlockBackend *blk = job->common.blk;
+    int nbytes;
+
+    assert(QEMU_IS_ALIGNED(job->copy_range_size, job->cluster_size));
+    nbytes = MIN(job->copy_range_size, end - start);
+    nr_clusters = DIV_ROUND_UP(nbytes, job->cluster_size);
+    hbitmap_reset(job->copy_bitmap, start / job->cluster_size,
+                  nr_clusters);
+    ret = blk_co_copy_range(blk, start, job->target, start, nbytes,
+                            is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0);
+    if (ret < 0) {
+        trace_backup_do_cow_copy_range_fail(job, start, ret);
+        hbitmap_set(job->copy_bitmap, start / job->cluster_size,
+                    nr_clusters);
+        return ret;
+    }
+
+    return nbytes;
+}
+
 static int coroutine_fn backup_do_cow(BackupBlockJob *job,
                                       int64_t offset, uint64_t bytes,
                                       bool *error_is_read,
                                       bool is_write_notifier)
 {
-    BlockBackend *blk = job->common.blk;
     CowRequest cow_request;
-    struct iovec iov;
-    QEMUIOVector bounce_qiov;
-    void *bounce_buffer = NULL;
     int ret = 0;
     int64_t start, end; /* bytes */
-    int n; /* bytes */
+    void *bounce_buffer = NULL;
 
     qemu_co_rwlock_rdlock(&job->flush_rwlock);
 
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
     wait_for_overlapping_requests(job, start, end);
     cow_request_begin(&cow_request, job, start, end);
 
-    for (; start < end; start += job->cluster_size) {
+    while (start < end) {
         if (!hbitmap_get(job->copy_bitmap, start / job->cluster_size)) {
             trace_backup_do_cow_skip(job, start);
+            start += job->cluster_size;
             continue; /* already copied */
         }
-        hbitmap_reset(job->copy_bitmap, start / job->cluster_size, 1);
 
         trace_backup_do_cow_process(job, start);
 
-        n = MIN(job->cluster_size, job->len - start);
-
-        if (!bounce_buffer) {
-            bounce_buffer = blk_blockalign(blk, job->cluster_size);
-        }
-        iov.iov_base = bounce_buffer;
-        iov.iov_len = n;
-        qemu_iovec_init_external(&bounce_qiov, &iov, 1);
-
-        ret = blk_co_preadv(blk, start, bounce_qiov.size, &bounce_qiov,
-                            is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0);
-        if (ret < 0) {
-            trace_backup_do_cow_read_fail(job, start, ret);
-            if (error_is_read) {
-                *error_is_read = true;
+        if (job->use_copy_range) {
+            ret = backup_cow_with_offload(job, start, end, is_write_notifier);
+            if (ret < 0) {
+                job->use_copy_range = false;
             }
-            hbitmap_set(job->copy_bitmap, start / job->cluster_size, 1);
-            goto out;
         }
-
-        if (buffer_is_zero(iov.iov_base, iov.iov_len)) {
-            ret = blk_co_pwrite_zeroes(job->target, start,
-                                       bounce_qiov.size, BDRV_REQ_MAY_UNMAP);
-        } else {
-            ret = blk_co_pwritev(job->target, start,
-                                 bounce_qiov.size, &bounce_qiov,
-                                 job->compress ? BDRV_REQ_WRITE_COMPRESSED : 0);
+        if (!job->use_copy_range) {
+            ret = backup_cow_with_bounce_buffer(job, start, end, is_write_notifier,
+                                                error_is_read, &bounce_buffer);
         }
         if (ret < 0) {
-            trace_backup_do_cow_write_fail(job, start, ret);
-            if (error_is_read) {
-                *error_is_read = false;
-            }
-            hbitmap_set(job->copy_bitmap, start / job->cluster_size, 1);
-            goto out;
+            break;
         }
 
         /* Publish progress, guest I/O counts as progress too.  Note that the
          * offset field is an opaque progress value, it is not a disk offset.
          */
-        job->bytes_read += n;
-        job_progress_update(&job->common.job, n);
+        start += ret;
+        job->bytes_read += ret;
+        job_progress_update(&job->common.job, ret);
+        ret = 0;
     }
 
-out:
     if (bounce_buffer) {
         qemu_vfree(bounce_buffer);
     }
@@ -XXX,XX +XXX,XX @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
     } else {
         job->cluster_size = MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size);
     }
+    job->use_copy_range = true;
+    job->copy_range_size = MIN_NON_ZERO(blk_get_max_transfer(job->common.blk),
+                                        blk_get_max_transfer(job->target));
+    job->copy_range_size = MAX(job->cluster_size,
+                               QEMU_ALIGN_UP(job->copy_range_size,
+                                             job->cluster_size));
 
     /* Required permissions are already taken with target's blk_new() */
     block_job_add_bdrv(&job->common, "target", target, 0, BLK_PERM_ALL,
diff --git a/block/trace-events b/block/trace-events
index XXXXXXX..XXXXXXX 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -XXX,XX +XXX,XX @@ backup_do_cow_skip(void *job, int64_t start) "job %p start %"PRId64
 backup_do_cow_process(void *job, int64_t start) "job %p start %"PRId64
 backup_do_cow_read_fail(void *job, int64_t start, int ret) "job %p start %"PRId64" ret %d"
 backup_do_cow_write_fail(void *job, int64_t start, int ret) "job %p start %"PRId64" ret %d"
+backup_do_cow_copy_range_fail(void *job, int64_t start, int ret) "job %p start %"PRId64" ret %d"
 
 # blockdev.c
 qmp_block_job_cancel(void *job) "job %p"
-- 
2.17.1

The following changes since commit 3521ade3510eb5cefb2e27a101667f25dad89935:

Merge remote-tracking branch 'remotes/thuth-gitlab/tags/pull-request-2021-07-29' into staging (2021-07-29 13:17:20 +0100)

are available in the Git repository at:

https://gitlab.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to cc8eecd7f105a1dff5876adeb238a14696061a4a:

MAINTAINERS: Added myself as a reviewer for the NVMe Block Driver (2021-07-29 17:17:34 +0100)

----------------------------------------------------------------
Pull request

The main fix here is for io_uring. Spurious -EAGAIN errors can happen and the
request needs to be resubmitted.

The MAINTAINERS changes carry no risk and we might as well include them in QEMU
6.1.

----------------------------------------------------------------

Fabian Ebner (1):
  block/io_uring: resubmit when result is -EAGAIN

Philippe Mathieu-Daudé (1):
  MAINTAINERS: Added myself as a reviewer for the NVMe Block Driver

Stefano Garzarella (1):
  MAINTAINERS: add Stefano Garzarella as io_uring reviewer

MAINTAINERS      |  2 ++
 block/io_uring.c | 16 +++++++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

-- 
2.31.1

From: Fabian Ebner <f.ebner@proxmox.com>

Linux SCSI can throw spurious -EAGAIN in some corner cases in its
completion path, which will end up being the result in the completed
io_uring request.

Resubmitting such requests should allow block jobs to complete, even
if such spurious errors are encountered.

Co-authored-by: Stefan Hajnoczi <stefanha@gmail.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Message-id: 20210729091029.65369-1-f.ebner@proxmox.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/io_uring.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/block/io_uring.c b/block/io_uring.c
index XXXXXXX..XXXXXXX 100644
--- a/block/io_uring.c
+++ b/block/io_uring.c
@@ -XXX,XX +XXX,XX @@ static void luring_process_completions(LuringState *s)
         total_bytes = ret + luringcb->total_read;
 
         if (ret < 0) {
-            if (ret == -EINTR) {
+            /*
+             * Only writev/readv/fsync requests on regular files or host block
+             * devices are submitted. Therefore -EAGAIN is not expected but it's
+             * known to happen sometimes with Linux SCSI. Submit again and hope
+             * the request completes successfully.
+             *
+             * For more information, see:
+             * https://lore.kernel.org/io-uring/20210727165811.284510-3-axboe@kernel.dk/T/#u
+             *
+             * If the code is changed to submit other types of requests in the
+             * future, then this workaround may need to be extended to deal with
+             * genuine -EAGAIN results that should not be resubmitted
+             * immediately.
+             */
+            if (ret == -EINTR || ret == -EAGAIN) {
                 luring_resubmit(s, luringcb);
                 continue;
             }
-- 
2.31.1