qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v8 7/7] block/io: refactor save/load vmstate


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH v8 7/7] block/io: refactor save/load vmstate
Date: Thu, 24 Sep 2020 10:20:25 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.2.2

23.09.2020 23:10, Eric Blake wrote:
On 9/15/20 11:44 AM, Vladimir Sementsov-Ogievskiy wrote:
Like for read/write in a previous commit, drop extra indirection layer,
generate directly bdrv_readv_vmstate() and bdrv_writev_vmstate().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
  block/coroutines.h    | 10 +++----
  include/block/block.h |  6 ++--
  block/io.c            | 67 ++++++++++++++++++++++---------------------
  3 files changed, 42 insertions(+), 41 deletions(-)

diff --git a/block/coroutines.h b/block/coroutines.h

  int coroutine_fn
-bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
-                   bool is_read)
+bdrv_co_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
  {
      BlockDriver *drv = bs->drv;
      int ret = -ENOTSUP;
+    if (!drv) {
+        return -ENOMEDIUM;
+    }
+
      bdrv_inc_in_flight(bs);
-    if (!drv) {
-        ret = -ENOMEDIUM;
-    } else if (drv->bdrv_load_vmstate) {
-        if (is_read) {
-            ret = drv->bdrv_load_vmstate(bs, qiov, pos);
-        } else {
-            ret = drv->bdrv_save_vmstate(bs, qiov, pos);
-        }
+    if (drv->bdrv_load_vmstate) {
+        ret = drv->bdrv_load_vmstate(bs, qiov, pos);

This one makes sense;

      } else if (bs->file) {
-        ret = bdrv_co_rw_vmstate(bs->file->bs, qiov, pos, is_read);
+        ret = bdrv_co_readv_vmstate(bs->file->bs, qiov, pos);
      }
      bdrv_dec_in_flight(bs);
+
      return ret;
  }
-int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf,
-                      int64_t pos, int size)
+int coroutine_fn
+bdrv_co_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
  {
-    QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, size);
-    int ret;
+    BlockDriver *drv = bs->drv;
+    int ret = -ENOTSUP;
-    ret = bdrv_writev_vmstate(bs, &qiov, pos);
-    if (ret < 0) {
-        return ret;
+    if (!drv) {
+        return -ENOMEDIUM;
      }
-    return size;
-}
+    bdrv_inc_in_flight(bs);
-int bdrv_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
-{
-    return bdrv_rw_vmstate(bs, qiov, pos, false);
+    if (drv->bdrv_load_vmstate) {
+        ret = drv->bdrv_save_vmstate(bs, qiov, pos);

but this one looks awkward. It represents the pre-patch logic, but it would be 
nicer to check for bdrv_save_vmstate.  With that tweak, my R-b still stands.

Agree.


I had an interesting time applying this patch due to merge conflicts with the 
new bdrv_primary_bs() changes that landed in the meantime.


Thanks a lot!

To clarify: did you finally staged the series to send a pull request? Or Stefan 
should do it? Should I make a v9?

--
Best regards,
Vladimir



reply via email to

[Prev in Thread] Current Thread [Next in Thread]