Series comparison

-[Qemu-devel] [PULL for-2.9 0/1] Block patches
+[PULL 0/1] Block patches
-The following changes since commit d84f714eafedd8bb9d4aaec8b76417bef8e3535e:
+The following changes since commit 61c265f0660ee476985808c8aa7915617c44fd53:
-  Update version for v2.9.0-rc0 release (2017-03-14 19:18:23 +0000)
+  Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20200313a' into staging (2020-03-13 10:33:04 +0000)
-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://github.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to 9dc44aa5829eb3131a01378a738dee28a382bbc1:
+for you to fetch changes up to 4ab78b19189a81038e744728ed949d09aa477550:
-  os: don't corrupt pre-existing memory-backend data with prealloc (2017-03-15 11:55:41 +0800)
+  block/io: fix bdrv_co_do_copy_on_readv (2020-03-16 11:46:11 +0000)
 ----------------------------------------------------------------
 Pull request
 ----------------------------------------------------------------
-----------------------------------------------------------------
+Vladimir Sementsov-Ogievskiy (1):
   block/io: fix bdrv_co_do_copy_on_readv
-Daniel P. Berrange (1):
+ block/io.c | 2 +-
-  os: don't corrupt pre-existing memory-backend data with prealloc
+file changed, 1 insertion(+), 1 deletion(-)
  util/oslib-posix.c | 14 +++++++++++++-
 file changed, 13 insertions(+), 1 deletion(-)
 --
-.9.3
+.24.1

-[Qemu-devel] [PULL for-2.9 1/1] os: don't corrupt pre-existing memory-backend data with prealloc
+[PULL 1/1] block/io: fix bdrv_co_do_copy_on_readv
-From: "Daniel P. Berrange" <berrange@redhat.com>
+From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
-When using a memory-backend object with prealloc turned on, QEMU
+Prior to 1143ec5ebf4 it was OK to qemu_iovec_from_buf() from aligned-up
-will memset() the first byte in every memory page to zero. While
+buffer to original qiov, as qemu_iovec_from_buf() will stop at qiov end
-this might have been acceptable for memory backends associated
+anyway.
 with RAM, this corrupts application data for NVDIMMs.
-Instead of setting every page to zero, read the current byte
+But after 1143ec5ebf4 we assume that bdrv_co_do_copy_on_readv works on
-value and then just write that same value back, so we are not
+part of original qiov, defined by qiov_offset and bytes. So we must not
-corrupting the original data. Directly write the value instead
+touch qiov behind qiov_offset+bytes bound. Fix it.
 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>
+Cc: qemu-stable@nongnu.org # v4.2
-Reviewed-by: Andrea Arcangeli <aarcange@redhat.com>
+Fixes: 1143ec5ebf4
-Message-id: 20170303113255.28262-1-berrange@redhat.com
+Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
 Reviewed-by: John Snow <jsnow@redhat.com>
 Message-id: 20200312081949.5350-1-vsementsov@virtuozzo.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- util/oslib-posix.c | 14 +++++++++++++-
+ block/io.c | 2 +-
-file changed, 13 insertions(+), 1 deletion(-)
+file changed, 1 insertion(+), 1 deletion(-)
-diff --git a/util/oslib-posix.c b/util/oslib-posix.c
+diff --git a/block/io.c b/block/io.c
 index XXXXXXX..XXXXXXX 100644
---- a/util/oslib-posix.c
+--- a/block/io.c
-+++ b/util/oslib-posix.c
++++ b/block/io.c
-@@ -XXX,XX +XXX,XX @@ static void *do_touch_pages(void *arg)
+@@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
-         memset_thread_failed = true;
+             if (!(flags & BDRV_REQ_PREFETCH)) {
-     } else {
+                 qemu_iovec_from_buf(qiov, qiov_offset + progress,
-         for (i = 0; i < numpages; i++) {
+                                     bounce_buffer + skip_bytes,
--            memset(addr, 0, 1);
+-                                    pnum - skip_bytes);
-+            /*
++                                    MIN(pnum - skip_bytes, bytes - progress));
-+             * Read & write back the same value, so we don't
+             }
-+             * corrupt existing user/app data that might be
+         } else if (!(flags & BDRV_REQ_PREFETCH)) {
-+             * stored.
+             /* Read directly into the destination */
 +             *
 +             * '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
+.24.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

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Prior to 1143ec5ebf4 it was OK to qemu_iovec_from_buf() from aligned-up
buffer to original qiov, as qemu_iovec_from_buf() will stop at qiov end
anyway.

But after 1143ec5ebf4 we assume that bdrv_co_do_copy_on_readv works on
part of original qiov, defined by qiov_offset and bytes. So we must not
touch qiov behind qiov_offset+bytes bound. Fix it.

Cc: qemu-stable@nongnu.org # v4.2
Fixes: 1143ec5ebf4
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 20200312081949.5350-1-vsementsov@virtuozzo.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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_do_copy_on_readv(BdrvChild *child,
             if (!(flags & BDRV_REQ_PREFETCH)) {
                 qemu_iovec_from_buf(qiov, qiov_offset + progress,
                                     bounce_buffer + skip_bytes,
-                                    pnum - skip_bytes);
+                                    MIN(pnum - skip_bytes, bytes - progress));
             }
         } else if (!(flags & BDRV_REQ_PREFETCH)) {
             /* Read directly into the destination */
-- 
2.24.1