Series comparison

-[Qemu-devel] [PULL for-2.9 0/1] Block patches
+[PULL 0/2] Block patches
-The following changes since commit d84f714eafedd8bb9d4aaec8b76417bef8e3535e:
+The following changes since commit ca61fa4b803e5d0abaf6f1ceb690f23bb78a4def:
-  Update version for v2.9.0-rc0 release (2017-03-14 19:18:23 +0000)
+  Merge remote-tracking branch 'remotes/quic/tags/pull-hex-20211006' into staging (2021-10-06 12:11:14 -0700)
-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 1cc7eada97914f090125e588497986f6f7900514:
-  os: don't corrupt pre-existing memory-backend data with prealloc (2017-03-15 11:55:41 +0800)
+  iothread: use IOThreadParamInfo in iothread_[set|get]_param() (2021-10-07 15:29:50 +0100)
 ----------------------------------------------------------------
 Pull request
 ----------------------------------------------------------------
-----------------------------------------------------------------
+Stefano Garzarella (2):
   iothread: rename PollParamInfo to IOThreadParamInfo
   iothread: use IOThreadParamInfo in iothread_[set|get]_param()
-Daniel P. Berrange (1):
+ iothread.c | 28 +++++++++++++++-------------
-  os: don't corrupt pre-existing memory-backend data with prealloc
+file changed, 15 insertions(+), 13 deletions(-)
  util/oslib-posix.c | 14 +++++++++++++-
 file changed, 13 insertions(+), 1 deletion(-)
 --
-.9.3
+.31.1

-New patch
+[PULL 1/2] iothread: rename PollParamInfo to IOThreadParamInfo
+From: Stefano Garzarella <sgarzare@redhat.com>
+Commit 1793ad0247 ("iothread: add aio-max-batch parameter") added
+a new parameter (aio-max-batch) to IOThread and used PollParamInfo
+structure to handle it.
+Since it is not a parameter of the polling mechanism, we rename the
+structure to a more generic IOThreadParamInfo.
+Suggested-by: Kevin Wolf <kwolf@redhat.com>
+Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Message-id: 20210727145936.147032-2-sgarzare@redhat.com
+Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+---
+ iothread.c | 14 +++++++-------
+file changed, 7 insertions(+), 7 deletions(-)
+diff --git a/iothread.c b/iothread.c
+index XXXXXXX..XXXXXXX 100644
+--- a/iothread.c
++++ b/iothread.c
+@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
+ typedef struct {
+     const char *name;
+     ptrdiff_t offset; /* field's byte offset in IOThread struct */
+-} PollParamInfo;
++} IOThreadParamInfo;
+-static PollParamInfo poll_max_ns_info = {
++static IOThreadParamInfo poll_max_ns_info = {
+     "poll-max-ns", offsetof(IOThread, poll_max_ns),
+ };
+-static PollParamInfo poll_grow_info = {
++static IOThreadParamInfo poll_grow_info = {
+     "poll-grow", offsetof(IOThread, poll_grow),
+ };
+-static PollParamInfo poll_shrink_info = {
++static IOThreadParamInfo poll_shrink_info = {
+     "poll-shrink", offsetof(IOThread, poll_shrink),
+ };
+-static PollParamInfo aio_max_batch_info = {
++static IOThreadParamInfo aio_max_batch_info = {
+     "aio-max-batch", offsetof(IOThread, aio_max_batch),
+ };
+@@ -XXX,XX +XXX,XX @@ static void iothread_get_param(Object *obj, Visitor *v,
+         const char *name, void *opaque, Error **errp)
+ {
+     IOThread *iothread = IOTHREAD(obj);
+-    PollParamInfo *info = opaque;
++    IOThreadParamInfo *info = opaque;
+     int64_t *field = (void *)iothread + info->offset;
+     visit_type_int64(v, name, field, errp);
+@@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v,
+         const char *name, void *opaque, Error **errp)
+ {
+     IOThread *iothread = IOTHREAD(obj);
+-    PollParamInfo *info = opaque;
++    IOThreadParamInfo *info = opaque;
+     int64_t *field = (void *)iothread + info->offset;
+     int64_t value;
+--
+.31.1

-[Qemu-devel] [PULL for-2.9 1/1] os: don't corrupt pre-existing memory-backend data with prealloc
+[PULL 2/2] iothread: use IOThreadParamInfo in iothread_[set|get]_param()
-From: "Daniel P. Berrange" <berrange@redhat.com>
+From: Stefano Garzarella <sgarzare@redhat.com>
-When using a memory-backend object with prealloc turned on, QEMU
+Commit 0445409d74 ("iothread: generalize
-will memset() the first byte in every memory page to zero. While
+iothread_set_param/iothread_get_param") moved common code to set and
-this might have been acceptable for memory backends associated
+get IOThread parameters in two new functions.
 with RAM, this corrupts application data for NVDIMMs.
-Instead of setting every page to zero, read the current byte
+These functions are called inside callbacks, so we don't need to use an
-value and then just write that same value back, so we are not
+opaque pointer. Let's replace `void *opaque` parameter with
-corrupting the original data. Directly write the value instead
+`IOThreadParamInfo *info`.
 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>
+Suggested-by: Kevin Wolf <kwolf@redhat.com>
-Reviewed-by: Andrea Arcangeli <aarcange@redhat.com>
+Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
-Message-id: 20170303113255.28262-1-berrange@redhat.com
+Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
 Message-id: 20210727145936.147032-3-sgarzare@redhat.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- util/oslib-posix.c | 14 +++++++++++++-
+ iothread.c | 18 ++++++++++--------
-file changed, 13 insertions(+), 1 deletion(-)
+file changed, 10 insertions(+), 8 deletions(-)
-diff --git a/util/oslib-posix.c b/util/oslib-posix.c
+diff --git a/iothread.c b/iothread.c
 index XXXXXXX..XXXXXXX 100644
---- a/util/oslib-posix.c
+--- a/iothread.c
-+++ b/util/oslib-posix.c
++++ b/iothread.c
-@@ -XXX,XX +XXX,XX @@ static void *do_touch_pages(void *arg)
+@@ -XXX,XX +XXX,XX @@ static IOThreadParamInfo aio_max_batch_info = {
-         memset_thread_failed = true;
+ };
-     } else {
-         for (i = 0; i < numpages; i++) {
+ static void iothread_get_param(Object *obj, Visitor *v,
--            memset(addr, 0, 1);
+-        const char *name, void *opaque, Error **errp)
-+            /*
++        const char *name, IOThreadParamInfo *info, Error **errp)
-+             * Read & write back the same value, so we don't
+ {
-+             * corrupt existing user/app data that might be
+     IOThread *iothread = IOTHREAD(obj);
-+             * stored.
+-    IOThreadParamInfo *info = opaque;
-+             *
+     int64_t *field = (void *)iothread + info->offset;
-+             * 'volatile' to stop compiler optimizing this away
-+             * to a no-op
+     visit_type_int64(v, name, field, errp);
-+             *
+ }
-+             * TODO: get a better solution from kernel so we
-+             * don't need to write at all so we don't cause
+ static bool iothread_set_param(Object *obj, Visitor *v,
-+             * wear on the storage backing the region...
+-        const char *name, void *opaque, Error **errp)
-+             */
++        const char *name, IOThreadParamInfo *info, Error **errp)
-+            *(volatile char *)addr = *addr;
+ {
-             addr += hpagesize;
+     IOThread *iothread = IOTHREAD(obj);
-         }
+-    IOThreadParamInfo *info = opaque;
      int64_t *field = (void *)iothread + info->offset;
      int64_t value;
@@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v,
  static void iothread_get_poll_param(Object *obj, Visitor *v,
          const char *name, void *opaque, Error **errp)
  {
 +    IOThreadParamInfo *info = opaque;
 -    iothread_get_param(obj, v, name, opaque, errp);
 +    iothread_get_param(obj, v, name, info, errp);
  }
  static void iothread_set_poll_param(Object *obj, Visitor *v,
          const char *name, void *opaque, Error **errp)
  {
      IOThread *iothread = IOTHREAD(obj);
 +    IOThreadParamInfo *info = opaque;
 -    if (!iothread_set_param(obj, v, name, opaque, errp)) {
 +    if (!iothread_set_param(obj, v, name, info, errp)) {
          return;
      }
+@@ -XXX,XX +XXX,XX @@ static void iothread_set_poll_param(Object *obj, Visitor *v,
+ static void iothread_get_aio_param(Object *obj, Visitor *v,
+         const char *name, void *opaque, Error **errp)
+ {
++    IOThreadParamInfo *info = opaque;
+-    iothread_get_param(obj, v, name, opaque, errp);
++    iothread_get_param(obj, v, name, info, errp);
+ }
+ static void iothread_set_aio_param(Object *obj, Visitor *v,
+         const char *name, void *opaque, Error **errp)
+ {
+     IOThread *iothread = IOTHREAD(obj);
++    IOThreadParamInfo *info = opaque;
+-    if (!iothread_set_param(obj, v, name, opaque, errp)) {
++    if (!iothread_set_param(obj, v, name, info, errp)) {
+         return;
+     }
 --
-.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

From: Stefano Garzarella <sgarzare@redhat.com>

Commit 1793ad0247 ("iothread: add aio-max-batch parameter") added
a new parameter (aio-max-batch) to IOThread and used PollParamInfo
structure to handle it.

Since it is not a parameter of the polling mechanism, we rename the
structure to a more generic IOThreadParamInfo.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210727145936.147032-2-sgarzare@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 iothread.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
 typedef struct {
     const char *name;
     ptrdiff_t offset; /* field's byte offset in IOThread struct */
-} PollParamInfo;
+} IOThreadParamInfo;
 
-static PollParamInfo poll_max_ns_info = {
+static IOThreadParamInfo poll_max_ns_info = {
     "poll-max-ns", offsetof(IOThread, poll_max_ns),
 };
-static PollParamInfo poll_grow_info = {
+static IOThreadParamInfo poll_grow_info = {
     "poll-grow", offsetof(IOThread, poll_grow),
 };
-static PollParamInfo poll_shrink_info = {
+static IOThreadParamInfo poll_shrink_info = {
     "poll-shrink", offsetof(IOThread, poll_shrink),
 };
-static PollParamInfo aio_max_batch_info = {
+static IOThreadParamInfo aio_max_batch_info = {
     "aio-max-batch", offsetof(IOThread, aio_max_batch),
 };
 
@@ -XXX,XX +XXX,XX @@ static void iothread_get_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
 {
     IOThread *iothread = IOTHREAD(obj);
-    PollParamInfo *info = opaque;
+    IOThreadParamInfo *info = opaque;
     int64_t *field = (void *)iothread + info->offset;
 
     visit_type_int64(v, name, field, errp);
@@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
 {
     IOThread *iothread = IOTHREAD(obj);
-    PollParamInfo *info = opaque;
+    IOThreadParamInfo *info = opaque;
     int64_t *field = (void *)iothread + info->offset;
     int64_t value;
 
-- 
2.31.1

From: Stefano Garzarella <sgarzare@redhat.com>

Commit 0445409d74 ("iothread: generalize
iothread_set_param/iothread_get_param") moved common code to set and
get IOThread parameters in two new functions.

These functions are called inside callbacks, so we don't need to use an
opaque pointer. Let's replace `void *opaque` parameter with
`IOThreadParamInfo *info`.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20210727145936.147032-3-sgarzare@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 iothread.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/iothread.c b/iothread.c
index XXXXXXX..XXXXXXX 100644
--- a/iothread.c
+++ b/iothread.c
@@ -XXX,XX +XXX,XX @@ static IOThreadParamInfo aio_max_batch_info = {
 };
 
 static void iothread_get_param(Object *obj, Visitor *v,
-        const char *name, void *opaque, Error **errp)
+        const char *name, IOThreadParamInfo *info, Error **errp)
 {
     IOThread *iothread = IOTHREAD(obj);
-    IOThreadParamInfo *info = opaque;
     int64_t *field = (void *)iothread + info->offset;
 
     visit_type_int64(v, name, field, errp);
 }
 
 static bool iothread_set_param(Object *obj, Visitor *v,
-        const char *name, void *opaque, Error **errp)
+        const char *name, IOThreadParamInfo *info, Error **errp)
 {
     IOThread *iothread = IOTHREAD(obj);
-    IOThreadParamInfo *info = opaque;
     int64_t *field = (void *)iothread + info->offset;
     int64_t value;
 
@@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v,
 static void iothread_get_poll_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
 {
+    IOThreadParamInfo *info = opaque;
 
-    iothread_get_param(obj, v, name, opaque, errp);
+    iothread_get_param(obj, v, name, info, errp);
 }
 
 static void iothread_set_poll_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
 {
     IOThread *iothread = IOTHREAD(obj);
+    IOThreadParamInfo *info = opaque;
 
-    if (!iothread_set_param(obj, v, name, opaque, errp)) {
+    if (!iothread_set_param(obj, v, name, info, errp)) {
         return;
     }
 
@@ -XXX,XX +XXX,XX @@ static void iothread_set_poll_param(Object *obj, Visitor *v,
 static void iothread_get_aio_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
 {
+    IOThreadParamInfo *info = opaque;
 
-    iothread_get_param(obj, v, name, opaque, errp);
+    iothread_get_param(obj, v, name, info, errp);
 }
 
 static void iothread_set_aio_param(Object *obj, Visitor *v,
         const char *name, void *opaque, Error **errp)
 {
     IOThread *iothread = IOTHREAD(obj);
+    IOThreadParamInfo *info = opaque;
 
-    if (!iothread_set_param(obj, v, name, opaque, errp)) {
+    if (!iothread_set_param(obj, v, name, info, errp)) {
         return;
     }
 
-- 
2.31.1