On Wed, 12 May 2021 15:49:57 +0800 Zhang Chen <chen.zh...@intel.com> wrote:
> Fix the issue from this patch:
> [PATCH] block: Flush all children in generic code
> From 883833e29cb800b4d92b5d4736252f4004885191
>
> Quorum driver do not have the primary child.
> It will caused guest block flush issue when use quorum and NBD.
> The vm guest flushes failed，and then guest filesystem is shutdown.
Hi,
I think the problem is rather that the quorum driver provides
.bdrv_co_flush_to_disk (which predates .bdrv_co_flush) instead of
.bdrv_co_flush. Can you try with the following patch instead?
diff --git a/block/quorum.c b/block/quorum.c
index cfc1436abb..f2c0805000 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1279,7 +1279,7 @@ static BlockDriver bdrv_quorum = {
.bdrv_dirname = quorum_dirname,
.bdrv_co_block_status = quorum_co_block_status,
- .bdrv_co_flush_to_disk = quorum_co_flush,
+ .bdrv_co_flush = quorum_co_flush,
.bdrv_getlength = quorum_getlength,
> Signed-off-by: Zhang Chen <chen.zh...@intel.com>
> Reported-by: Minghao Yuan <me...@qq.com>
> ---
> block/io.c | 31 ++++++++++++++++++++++---------
> 1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/block/io.c b/block/io.c
> index 35b6c56efc..4dc1873cb9 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -2849,6 +2849,13 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
> BdrvChild *child;
> int current_gen;
> int ret = 0;
> + bool no_primary_child = false;
> +
> + /* Quorum drivers do not have the primary child. */
> + if (!primary_child) {
> + primary_child = bs->file;
> + no_primary_child = true;
> + }
>
> bdrv_inc_in_flight(bs);
>
> @@ -2886,12 +2893,12 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
>
> /* But don't actually force it to the disk with cache=unsafe */
> if (bs->open_flags & BDRV_O_NO_FLUSH) {
> - goto flush_children;
> + goto flush_data;
> }
>
> /* Check if we really need to flush anything */
> if (bs->flushed_gen == current_gen) {
> - goto flush_children;
> + goto flush_data;
> }
>
> BLKDBG_EVENT(primary_child, BLKDBG_FLUSH_TO_DISK);
> @@ -2938,13 +2945,19 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
> /* Now flush the underlying protocol. It will also have BDRV_O_NO_FLUSH
> * in the case of cache=unsafe, so there are no useless flushes.
> */
> -flush_children:
> - ret = 0;
> - QLIST_FOREACH(child, &bs->children, next) {
> - if (child->perm & (BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED)) {
> - int this_child_ret = bdrv_co_flush(child->bs);
> - if (!ret) {
> - ret = this_child_ret;
> +flush_data:
> + if (no_primary_child) {
> + /* Flush parent */
> + ret = bs->file ? bdrv_co_flush(bs->file->bs) : 0;
> + } else {
> + /* Flush childrens */
> + ret = 0;
> + QLIST_FOREACH(child, &bs->children, next) {
> + if (child->perm & (BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED)) {
> + int this_child_ret = bdrv_co_flush(child->bs);
> + if (!ret) {
> + ret = this_child_ret;
> + }
> }
> }
> }
--
pgpVzJ_9v8bx1.pgp
Description: OpenPGP digital signature
