Series comparison

-[Qemu-devel] [PULL for-2.9 0/1] Block patches
+[PULL for-6.1 0/3] Block patches
-The following changes since commit d84f714eafedd8bb9d4aaec8b76417bef8e3535e:
+The following changes since commit 3521ade3510eb5cefb2e27a101667f25dad89935:
-  Update version for v2.9.0-rc0 release (2017-03-14 19:18:23 +0000)
+  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:
+are available in the Git repository at:
-  git://github.com/stefanha/qemu.git tags/block-pull-request
+  https://gitlab.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to 9dc44aa5829eb3131a01378a738dee28a382bbc1:
+for you to fetch changes up to cc8eecd7f105a1dff5876adeb238a14696061a4a:
-  os: don't corrupt pre-existing memory-backend data with prealloc (2017-03-15 11:55:41 +0800)
+  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.
 ----------------------------------------------------------------
-----------------------------------------------------------------
+Fabian Ebner (1):
   block/io_uring: resubmit when result is -EAGAIN
-Daniel P. Berrange (1):
+Philippe Mathieu-Daudé (1):
-  os: don't corrupt pre-existing memory-backend data with prealloc
+  MAINTAINERS: Added myself as a reviewer for the NVMe Block Driver
- util/oslib-posix.c | 14 +++++++++++++-
+Stefano Garzarella (1):
-file changed, 13 insertions(+), 1 deletion(-)
+  MAINTAINERS: add Stefano Garzarella as io_uring reviewer
  MAINTAINERS      |  2 ++
  block/io_uring.c | 16 +++++++++++++++-
 files changed, 17 insertions(+), 1 deletion(-)
 --
-.9.3
+.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

-New patch
+[PULL for-6.1 2/3] block/io_uring: resubmit when result is -EAGAIN
+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 +++++++++++++++-
+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;
+             }
+--
+.31.1

-[Qemu-devel] [PULL for-2.9 1/1] os: don't corrupt pre-existing memory-backend data with prealloc
+[PULL for-6.1 3/3] MAINTAINERS: Added myself as a reviewer for the NVMe Block Driver
-From: "Daniel P. Berrange" <berrange@redhat.com>
+From: Philippe Mathieu-Daudé <philmd@redhat.com>
-When using a memory-backend object with prealloc turned on, QEMU
+I'm interested in following the activity around the NVMe bdrv.
 will memset() the first byte in every memory page to zero. While
 this might have been acceptable for memory backends associated
 with RAM, this corrupts application data for NVDIMMs.
-Instead of setting every page to zero, read the current byte
+Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
-value and then just write that same value back, so we are not
+Message-id: 20210728183340.2018313-1-philmd@redhat.com
 corrupting the original data. Directly write the value instead
 of memset()ing it, since there's no benefit to memset for a
 single byte write.
 Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
 Reviewed-by: Andrea Arcangeli <aarcange@redhat.com>
 Message-id: 20170303113255.28262-1-berrange@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- util/oslib-posix.c | 14 +++++++++++++-
+ MAINTAINERS | 1 +
-file changed, 13 insertions(+), 1 deletion(-)
+file changed, 1 insertion(+)
-diff --git a/util/oslib-posix.c b/util/oslib-posix.c
+diff --git a/MAINTAINERS b/MAINTAINERS
 index XXXXXXX..XXXXXXX 100644
---- a/util/oslib-posix.c
+--- a/MAINTAINERS
-+++ b/util/oslib-posix.c
++++ b/MAINTAINERS
-@@ -XXX,XX +XXX,XX @@ static void *do_touch_pages(void *arg)
+@@ -XXX,XX +XXX,XX @@ F: block/null.c
-         memset_thread_failed = true;
+ NVMe Block Driver
-     } else {
+ M: Stefan Hajnoczi <stefanha@redhat.com>
-         for (i = 0; i < numpages; i++) {
+ R: Fam Zheng <fam@euphon.net>
--            memset(addr, 0, 1);
++R: Philippe Mathieu-Daudé <philmd@redhat.com>
-+            /*
+ L: qemu-block@nongnu.org
-+             * Read & write back the same value, so we don't
+ S: Supported
-+             * corrupt existing user/app data that might be
+ F: block/nvme*
 +             * stored.
 +             *
 +             * 'volatile' to stop compiler optimizing this away
 +             * to a no-op
 +             *
 +             * TODO: get a better solution from kernel so we
 +             * don't need to write at all so we don't cause
 +             * wear on the storage backing the region...
 +             */
 +            *(volatile char *)addr = *addr;
              addr += hpagesize;
          }
      }
 --
-.9.3
+.31.1

From: "Daniel P. Berrange" <berrange@redhat.com>

When using a memory-backend object with prealloc turned on, QEMU
will memset() the first byte in every memory page to zero. While
this might have been acceptable for memory backends associated
with RAM, this corrupts application data for NVDIMMs.

Instead of setting every page to zero, read the current byte
value and then just write that same value back, so we are not
corrupting the original data. Directly write the value instead
of memset()ing it, since there's no benefit to memset for a
single byte write.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Andrea Arcangeli <aarcange@redhat.com>
Message-id: 20170303113255.28262-1-berrange@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/oslib-posix.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index XXXXXXX..XXXXXXX 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -XXX,XX +XXX,XX @@ static void *do_touch_pages(void *arg)
         memset_thread_failed = true;
     } else {
         for (i = 0; i < numpages; i++) {
-            memset(addr, 0, 1);
+            /*
+             * Read & write back the same value, so we don't
+             * corrupt existing user/app data that might be
+             * stored.
+             *
+             * 'volatile' to stop compiler optimizing this away
+             * to a no-op
+             *
+             * TODO: get a better solution from kernel so we
+             * don't need to write at all so we don't cause
+             * wear on the storage backing the region...
+             */
+            *(volatile char *)addr = *addr;
             addr += hpagesize;
         }
     }
-- 
2.9.3

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