[Qemu-devel] [PATCH v5 19/42] block: Use CAF in bdrv_co_rw_vmstate()

Max Reitz posted 42 patches 6 years, 8 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v5 19/42] block: Use CAF in bdrv_co_rw_vmstate()
Posted by Max Reitz 6 years, 8 months ago
If a node whose driver does not provide VM state functions has a
metadata child, the VM state should probably go there; if it is a
filter, the VM state should probably go there.  It follows that we
should generally go down to the primary child.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/io.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/io.c b/block/io.c
index 659ea0c52a..14f99e1c00 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2395,6 +2395,7 @@ bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
                    bool is_read)
 {
     BlockDriver *drv = bs->drv;
+    BlockDriverState *child_bs = bdrv_primary_bs(bs);
     int ret = -ENOTSUP;
 
     bdrv_inc_in_flight(bs);
@@ -2407,8 +2408,8 @@ bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
         } else {
             ret = drv->bdrv_save_vmstate(bs, qiov, pos);
         }
-    } else if (bs->file) {
-        ret = bdrv_co_rw_vmstate(bs->file->bs, qiov, pos, is_read);
+    } else if (child_bs) {
+        ret = bdrv_co_rw_vmstate(child_bs, qiov, pos, is_read);
     }
 
     bdrv_dec_in_flight(bs);
-- 
2.21.0


Re: [Qemu-devel] [PATCH v5 19/42] block: Use CAF in bdrv_co_rw_vmstate()
Posted by Vladimir Sementsov-Ogievskiy 6 years, 7 months ago
13.06.2019 1:09, Max Reitz wrote:
> If a node whose driver does not provide VM state functions has a
> metadata child, the VM state should probably go there; if it is a
> filter, the VM state should probably go there.  It follows that we
> should generally go down to the primary child.

Hmm, as I understand vmstate is something stored in file and invisible for actual file user,
which may be guest or format node.. So actually it doesn't matter in which
child to store it, it should be transparent for the parent.. Maybe the right
thing is to loop through children and use first which supports storing vmstate.

But I'm OK with this too.

(hmm you assume that vmstate should go to metadata child,
but the only format which has separate metadata and storage child stores vmstate to
storage child)

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


> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   block/io.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index 659ea0c52a..14f99e1c00 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -2395,6 +2395,7 @@ bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
>                      bool is_read)
>   {
>       BlockDriver *drv = bs->drv;
> +    BlockDriverState *child_bs = bdrv_primary_bs(bs);
>       int ret = -ENOTSUP;
>   
>       bdrv_inc_in_flight(bs);
> @@ -2407,8 +2408,8 @@ bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
>           } else {
>               ret = drv->bdrv_save_vmstate(bs, qiov, pos);
>           }
> -    } else if (bs->file) {
> -        ret = bdrv_co_rw_vmstate(bs->file->bs, qiov, pos, is_read);
> +    } else if (child_bs) {
> +        ret = bdrv_co_rw_vmstate(child_bs, qiov, pos, is_read);
>       }
>   
>       bdrv_dec_in_flight(bs);
> 


-- 
Best regards,
Vladimir