Re: [PATCH v8 7/7] block/io: refactor save/load vmstate
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 > Reviewed-by: Eric Blake > --- > block/coroutines.h| 10 +++ > include/block/block.h | 6 ++-- > block/io.c| 67 ++- > 3 files changed, 42 insertions(+), 41 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [PATCH v8 7/7] block/io: refactor save/load vmstate
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
Reviewed-by: Eric Blake
---
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, , 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
Re: [PATCH v8 7/7] block/io: refactor save/load vmstate
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
Reviewed-by: Eric Blake
---
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, , 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
[PATCH v8 7/7] block/io: refactor save/load vmstate
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
Reviewed-by: Eric Blake
---
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
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, , 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, , pos);
-if (ret < 0) {
-return ret;
-}
+int ret = bdrv_writev_vmstate(bs, , 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, , pos);
+
+return ret < 0 ? ret : size;
}
