Re: [Qemu-block] [PATCH 17/20] qemu-io: Use bdrv_parse_cache_mode() in reopen_f()

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> We must forbid changing the WCE flag in bdrv_reopen() in the same patch,
> as otherwise the behaviour would change so that the flag takes
> precedence over the explicitly specified option.
> 
> The correct value of the WCE flag depends on the BlockBackend user (e.g.
> guest device) and isn't a decision that the QMP client makes, so this
> change is what we want.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  block.c                    | 18 ++++++------------
>  qemu-io-cmds.c             | 14 +++++++++++++-
>  tests/qemu-iotests/142     |  2 +-
>  tests/qemu-iotests/142.out |  2 +-
>  4 files changed, 21 insertions(+), 15 deletions(-)
> 

[...]

> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index e929d24..7de3754 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c

[...]

> @@ -2136,14 +2137,25 @@ static int reopen_f(BlockBackend *blk, int argc, char 
> **argv)
>          return qemuio_command_usage(&reopen_cmd);
>      }
>  
> +    if (writethrough != blk_enable_write_cache(blk) &&
> +        blk_get_attached_dev(blk))
> +    {
> +        error_report("Cannot change cache.writeback: Device attached");
> +        qemu_opts_reset(&reopen_opts);
> +        return 0;
> +    }
> +
>      qopts = qemu_opts_find(&reopen_opts, NULL);
>      opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL;
>      qemu_opts_reset(&reopen_opts);
>  
> +    flags |= blk_enable_write_cache(blk) ? BDRV_O_CACHE_WB : 0;

Shouldn't this be bdrv_enable_write_cache(bs)?

With blk_enable_write_cache(blk), reopening a non-WB BB should always
fail because bdrv_reopen_multiple() accuses us of trying to change the
WB mode (whereas we really don't want to change the BDS's mode).

Max

>      brq = bdrv_reopen_queue(NULL, bs, opts, flags);
>      bdrv_reopen_multiple(brq, &local_err);
>      if (local_err) {
>          error_report_err(local_err);
> +    } else {
> +        blk_set_enable_write_cache(blk, !writethrough);
>      }
>  
>      return 0;

signature.asc
Description: OpenPGP digital signature