[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 17/20] qemu-io: Use bdrv_parse_cache_mode() in r
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 17/20] qemu-io: Use bdrv_parse_cache_mode() in reopen_f() |
Date: |
Tue, 29 Mar 2016 12:16:40 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 26.03.2016 um 22:05 hat Max Reitz geschrieben:
> 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).
bdrv_enable_write_cache() is only a wrapper around
blk_enable_write_cache(), so that wouldn't make any difference. It also
means that bdrv_reopen() checks the cache mode of the BlockBackend
rather than always returning true. This line exists precisely to avoid
the error message you mention.
And as you noticed, the line goes away anyway when BDRV_O_CACHE_WB is
removed later in the series.
Kevin
pgpmjEP4Ii40Y.pgp
Description: PGP signature
- Re: [Qemu-devel] [PATCH 13/20] iscsi: Support BDRV_REQ_FUA, (continued)
- [Qemu-devel] [PATCH 11/20] block/qapi: Use blk_enable_write_cache(), Kevin Wolf, 2016/03/18
- [Qemu-devel] [PATCH 14/20] nbd: Support BDRV_REQ_FUA, Kevin Wolf, 2016/03/18
- [Qemu-devel] [PATCH 18/20] block: Remove bdrv_parse_cache_flags(), Kevin Wolf, 2016/03/18
- [Qemu-devel] [PATCH 17/20] qemu-io: Use bdrv_parse_cache_mode() in reopen_f(), Kevin Wolf, 2016/03/18
- [Qemu-devel] [PATCH 20/20] block: Remove bdrv_(set_)enable_write_cache(), Kevin Wolf, 2016/03/18
- [Qemu-devel] [PATCH 19/20] block: Remove BDRV_O_CACHE_WB, Kevin Wolf, 2016/03/18