Series comparison

-[PULL 0/1] Block patches
+[PULL for-6.1 0/3] Block patches
-The following changes since commit 6157b0e19721aadb4c7fdcfe57b2924af6144b14:
+The following changes since commit 3521ade3510eb5cefb2e27a101667f25dad89935:
-  Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-6.0-pull-=
+  Merge remote-tracking branch 'remotes/thuth-gitlab/tags/pull-request-2021-07-29' into staging (2021-07-29 13:17:20 +0100)
 request' into staging (2021-03-14 17:47:49 +0000)
 are available in the Git repository at:
   https://gitlab.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to fb0b154c801e3447e505de420195fb7038695941:
+for you to fetch changes up to cc8eecd7f105a1dff5876adeb238a14696061a4a:
-  virtio-blk: Respect discard granularity (2021-03-15 09:48:53 +0000)
+  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
+.1.
 ----------------------------------------------------------------
-Akihiko Odaki (1):
+Fabian Ebner (1):
-  virtio-blk: Respect discard granularity
+  block/io_uring: resubmit when result is -EAGAIN
- include/hw/virtio/virtio-blk.h | 1 +
+Philippe Mathieu-Daudé (1):
- hw/block/virtio-blk.c          | 8 +++++++-
+  MAINTAINERS: Added myself as a reviewer for the NVMe Block Driver
  hw/core/machine.c              | 1 +
 files changed, 9 insertions(+), 1 deletion(-)
---=20
+Stefano Garzarella (1):
-.29.2
+  MAINTAINERS: add Stefano Garzarella as io_uring reviewer
+ MAINTAINERS      |  2 ++
+ block/io_uring.c | 16 +++++++++++++++-
+files changed, 17 insertions(+), 1 deletion(-)
+--
+.31.1

-New patch
+[PULL for-6.1 1/3] MAINTAINERS: add Stefano Garzarella as io_uring reviewer
+From: Stefano Garzarella <sgarzare@redhat.com>
+I've been working with io_uring for a while so I'd like to help
+with reviews.
+Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
+Message-Id: <20210728131515.131045-1-sgarzare@redhat.com>
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ MAINTAINERS | 1 +
+file changed, 1 insertion(+)
+diff --git a/MAINTAINERS b/MAINTAINERS
+index XXXXXXX..XXXXXXX 100644
+--- a/MAINTAINERS
++++ b/MAINTAINERS
+@@ -XXX,XX +XXX,XX @@ Linux io_uring
+ M: Aarushi Mehta <mehta.aaru20@gmail.com>
+ M: Julia Suvorova <jusual@redhat.com>
+ 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

-[PULL 1/1] virtio-blk: Respect discard granularity
+[PULL for-6.1 2/3] block/io_uring: resubmit when result is -EAGAIN
-From: Akihiko Odaki <akihiko.odaki@gmail.com>
+From: Fabian Ebner <f.ebner@proxmox.com>
-Report the configured granularity for discard operation to the
+Linux SCSI can throw spurious -EAGAIN in some corner cases in its
-guest. If this is not set use the block size.
+completion path, which will end up being the result in the completed
 io_uring request.
-Since until now we have ignored the configured discard granularity
+Resubmitting such requests should allow block jobs to complete, even
-and always reported the block size, let's add
+if such spurious errors are encountered.
 'report-discard-granularity' property and disable it for older
 machine types to avoid migration issues.
-Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.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>
-Message-Id: <20210225001239.47046-1-akihiko.odaki@gmail.com>
 ---
- include/hw/virtio/virtio-blk.h | 1 +
+ block/io_uring.c | 16 +++++++++++++++-
- hw/block/virtio-blk.c          | 8 +++++++-
+file changed, 15 insertions(+), 1 deletion(-)
  hw/core/machine.c              | 1 +
 files changed, 9 insertions(+), 1 deletion(-)
-diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
+diff --git a/block/io_uring.c b/block/io_uring.c
 index XXXXXXX..XXXXXXX 100644
---- a/include/hw/virtio/virtio-blk.h
+--- a/block/io_uring.c
-+++ b/include/hw/virtio/virtio-blk.h
++++ b/block/io_uring.c
-@@ -XXX,XX +XXX,XX @@ struct VirtIOBlkConf
+@@ -XXX,XX +XXX,XX @@ static void luring_process_completions(LuringState *s)
-     uint16_t num_queues;
+         total_bytes = ret + luringcb->total_read;
-     uint16_t queue_size;
-     bool seg_max_adjust;
+         if (ret < 0) {
-+    bool report_discard_granularity;
+-            if (ret == -EINTR) {
-     uint32_t max_discard_sectors;
++            /*
-     uint32_t max_write_zeroes_sectors;
++             * Only writev/readv/fsync requests on regular files or host block
-     bool x_enable_wce_if_config_wce;
++             * devices are submitted. Therefore -EAGAIN is not expected but it's
-diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
++             * known to happen sometimes with Linux SCSI. Submit again and hope
-index XXXXXXX..XXXXXXX 100644
++             * the request completes successfully.
---- a/hw/block/virtio-blk.c
++             *
-+++ b/hw/block/virtio-blk.c
++             * For more information, see:
-@@ -XXX,XX +XXX,XX @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
++             * https://lore.kernel.org/io-uring/20210727165811.284510-3-axboe@kernel.dk/T/#u
-     blkcfg.wce = blk_enable_write_cache(s->blk);
++             *
-     virtio_stw_p(vdev, &blkcfg.num_queues, s->conf.num_queues);
++             * If the code is changed to submit other types of requests in the
-     if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_DISCARD)) {
++             * future, then this workaround may need to be extended to deal with
-+        uint32_t discard_granularity = conf->discard_granularity;
++             * genuine -EAGAIN results that should not be resubmitted
-+        if (discard_granularity == -1 || !s->conf.report_discard_granularity) {
++             * immediately.
-+            discard_granularity = blk_size;
++             */
-+        }
++            if (ret == -EINTR || ret == -EAGAIN) {
-         virtio_stl_p(vdev, &blkcfg.max_discard_sectors,
+                 luring_resubmit(s, luringcb);
-                      s->conf.max_discard_sectors);
+                 continue;
-         virtio_stl_p(vdev, &blkcfg.discard_sector_alignment,
+             }
 -                     blk_size >> BDRV_SECTOR_BITS);
 +                     discard_granularity >> BDRV_SECTOR_BITS);
          /*
           * We support only one segment per request since multiple segments
           * are not widely used and there are no userspace APIs that allow
@@ -XXX,XX +XXX,XX @@ static Property virtio_blk_properties[] = {
                       IOThread *),
      DEFINE_PROP_BIT64("discard", VirtIOBlock, host_features,
                        VIRTIO_BLK_F_DISCARD, true),
 +    DEFINE_PROP_BOOL("report-discard-granularity", VirtIOBlock,
 +                     conf.report_discard_granularity, true),
      DEFINE_PROP_BIT64("write-zeroes", VirtIOBlock, host_features,
                        VIRTIO_BLK_F_WRITE_ZEROES, true),
      DEFINE_PROP_UINT32("max-discard-sectors", VirtIOBlock,
 diff --git a/hw/core/machine.c b/hw/core/machine.c
 index XXXXXXX..XXXXXXX 100644
 --- a/hw/core/machine.c
 +++ b/hw/core/machine.c
@@ -XXX,XX +XXX,XX @@
  GlobalProperty hw_compat_5_2[] = {
      { "ICH9-LPC", "smm-compat", "on"},
      { "PIIX4_PM", "smm-compat", "on"},
 +    { "virtio-blk-device", "report-discard-granularity", "off" },
  };
  const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
 --
-.29.2
+.31.1

-New patch
+[PULL for-6.1 3/3] MAINTAINERS: Added myself as a reviewer for the NVMe Block Driver
+From: Philippe Mathieu-Daudé <philmd@redhat.com>
+I'm interested in following the activity around the NVMe bdrv.
+Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Message-id: 20210728183340.2018313-1-philmd@redhat.com
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ MAINTAINERS | 1 +
+file changed, 1 insertion(+)
+diff --git a/MAINTAINERS b/MAINTAINERS
+index XXXXXXX..XXXXXXX 100644
+--- a/MAINTAINERS
++++ b/MAINTAINERS
+@@ -XXX,XX +XXX,XX @@ F: block/null.c
+ NVMe Block Driver
+ 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

From: Akihiko Odaki <akihiko.odaki@gmail.com>

Report the configured granularity for discard operation to the
guest. If this is not set use the block size.

Since until now we have ignored the configured discard granularity
and always reported the block size, let's add
'report-discard-granularity' property and disable it for older
machine types to avoid migration issues.

Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210225001239.47046-1-akihiko.odaki@gmail.com>
---
 include/hw/virtio/virtio-blk.h | 1 +
 hw/block/virtio-blk.c          | 8 +++++++-
 hw/core/machine.c              | 1 +
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index XXXXXXX..XXXXXXX 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -XXX,XX +XXX,XX @@ struct VirtIOBlkConf
     uint16_t num_queues;
     uint16_t queue_size;
     bool seg_max_adjust;
+    bool report_discard_granularity;
     uint32_t max_discard_sectors;
     uint32_t max_write_zeroes_sectors;
     bool x_enable_wce_if_config_wce;
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -XXX,XX +XXX,XX @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
     blkcfg.wce = blk_enable_write_cache(s->blk);
     virtio_stw_p(vdev, &blkcfg.num_queues, s->conf.num_queues);
     if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_DISCARD)) {
+        uint32_t discard_granularity = conf->discard_granularity;
+        if (discard_granularity == -1 || !s->conf.report_discard_granularity) {
+            discard_granularity = blk_size;
+        }
         virtio_stl_p(vdev, &blkcfg.max_discard_sectors,
                      s->conf.max_discard_sectors);
         virtio_stl_p(vdev, &blkcfg.discard_sector_alignment,
-                     blk_size >> BDRV_SECTOR_BITS);
+                     discard_granularity >> BDRV_SECTOR_BITS);
         /*
          * We support only one segment per request since multiple segments
          * are not widely used and there are no userspace APIs that allow
@@ -XXX,XX +XXX,XX @@ static Property virtio_blk_properties[] = {
                      IOThread *),
     DEFINE_PROP_BIT64("discard", VirtIOBlock, host_features,
                       VIRTIO_BLK_F_DISCARD, true),
+    DEFINE_PROP_BOOL("report-discard-granularity", VirtIOBlock,
+                     conf.report_discard_granularity, true),
     DEFINE_PROP_BIT64("write-zeroes", VirtIOBlock, host_features,
                       VIRTIO_BLK_F_WRITE_ZEROES, true),
     DEFINE_PROP_UINT32("max-discard-sectors", VirtIOBlock,
diff --git a/hw/core/machine.c b/hw/core/machine.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -XXX,XX +XXX,XX @@
 GlobalProperty hw_compat_5_2[] = {
     { "ICH9-LPC", "smm-compat", "on"},
     { "PIIX4_PM", "smm-compat", "on"},
+    { "virtio-blk-device", "report-discard-granularity", "off" },
 };
 const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
 
-- 
2.29.2

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