diff mbox series

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

Message ID 20200915164411.20590-8-vsementsov@virtuozzo.com
State New
Headers show
Series coroutines: generate wrapper code | expand

Commit Message

Vladimir Sementsov-Ogievskiy Sept. 15, 2020, 4:44 p.m. UTC
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(-)

Comments

Eric Blake Sept. 23, 2020, 8:10 p.m. UTC | #1
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.

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

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org
Stefan Hajnoczi Sept. 24, 2020, 12:16 p.m. UTC | #2
On Tue, Sep 15, 2020 at 07:44:11PM +0300, 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(-)


Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff mbox series

Patch

diff --git a/block/coroutines.h b/block/coroutines.h
index 6c63a819c9..f69179f5ef 100644
--- a/block/coroutines.h
+++ b/block/coroutines.h
@@ -57,11 +57,9 @@  bdrv_common_block_status_above(BlockDriverState *bs,
                                int64_t *map,
                                BlockDriverState **file);
 
-int coroutine_fn
-bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
-                   bool is_read);
-int generated_co_wrapper
-bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
-                bool is_read);
+int coroutine_fn bdrv_co_readv_vmstate(BlockDriverState *bs,
+                                       QEMUIOVector *qiov, int64_t pos);
+int coroutine_fn bdrv_co_writev_vmstate(BlockDriverState *bs,
+                                        QEMUIOVector *qiov, int64_t pos);
 
 #endif /* BLOCK_COROUTINES_INT_H */
diff --git a/include/block/block.h b/include/block/block.h
index b8b4c177de..6cd789724b 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -572,8 +572,10 @@  int path_has_protocol(const char *path);
 int path_is_absolute(const char *path);
 char *path_combine(const char *base_path, const char *filename);
 
-int bdrv_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos);
-int bdrv_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos);
+int generated_co_wrapper
+bdrv_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos);
+int generated_co_wrapper
+bdrv_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos);
 int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf,
                       int64_t pos, int size);
 
diff --git a/block/io.c b/block/io.c
index 68d7d9cf80..84f82bc069 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2491,66 +2491,67 @@  int bdrv_is_allocated_above(BlockDriverState *top,
 }
 
 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);
     } 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);
+    } else if (bs->file) {
+        ret = bdrv_co_writev_vmstate(bs->file->bs, qiov, pos);
+    }
+
+    bdrv_dec_in_flight(bs);
+
+    return ret;
 }
 
-int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
+int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf,
                       int64_t pos, int size)
 {
     QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, size);
-    int ret;
-
-    ret = bdrv_readv_vmstate(bs, &qiov, pos);
-    if (ret < 0) {
-        return ret;
-    }
+    int ret = bdrv_writev_vmstate(bs, &qiov, pos);
 
-    return size;
+    return ret < 0 ? ret : size;
 }
 
-int bdrv_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos)
+int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
+                      int64_t pos, int size)
 {
-    return bdrv_rw_vmstate(bs, qiov, pos, true);
+    QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, size);
+    int ret = bdrv_readv_vmstate(bs, &qiov, pos);
+
+    return ret < 0 ? ret : size;
 }
 
 /**************************************************************/