Am 14.09.2016 um 17:52 hat Alberto Garcia geschrieben: > This adds the "read-only" option to the QDict. One important effect of > this change is that when a child inherits options from its parent, the > existing "read-only" mode can be preserved if it was explicitly set > previously. > > This addresses scenarios like this: > > [E] <- [D] <- [C] <- [B] <- [A] > > In this case, if we reopen [D] with read-only=off, and later reopen > [B], then [D] will not inherit read-only=on from its parent during the > bdrv_reopen_queue_child() stage. > > The BDRV_O_RDWR flag is not removed yet, but its keep in sync with the > value of the "read-only" option. > > Signed-off-by: Alberto Garcia <be...@igalia.com> > --- > block.c | 35 ++++++++++++++++++++++++++++++++--- > block/vvfat.c | 3 ++- > blockdev.c | 16 +++++++++++----- > include/block/block.h | 1 + > 4 files changed, 46 insertions(+), 9 deletions(-) > > diff --git a/block.c b/block.c > index f56d703..cf9c513 100644 > --- a/block.c > +++ b/block.c > @@ -707,6 +707,9 @@ static void bdrv_inherited_options(int *child_flags, > QDict *child_options, > qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT); > qdict_copy_default(child_options, parent_options, > BDRV_OPT_CACHE_NO_FLUSH); > > + /* Inherit the read-only option from the parent if it's not set */ > + qdict_copy_default(child_options, parent_options, BDRV_OPT_READ_ONLY); > + > /* Our block drivers take care to send flushes and respect unmap policy, > * so we can default to enable both on lower layers regardless of the > * corresponding parent options. */
We need another qdict_copy_default() in bdrv_temp_snapshot_options(), I think, so that flags and options stay consistent there. Currently it seems to be working because we still have the flag and the flag is what is actually used in bdrv_open_common(). This means that we parse the "read-only" option into the QemuOpts there, but we never read it from there. Shouldn't we use the option rather than the flags? > @@ -677,11 +679,15 @@ static BlockDriverState *bds_tree_init(QDict *bs_opts, > Error **errp) > goto fail; > } > > + read_only = qemu_opt_get_bool(opts, BDRV_OPT_READ_ONLY, false); > + > /* bdrv_open() defaults to the values in bdrv_flags (for compatibility > * with other callers) rather than what we want as the real defaults. > * Apply the defaults here instead. */ > qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_DIRECT, "off"); > qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_NO_FLUSH, "off"); > + qdict_set_default_str(bs_opts, BDRV_OPT_READ_ONLY, > + read_only ? "on" : "off"); Why do you parse read_only into the QemuOpts just to add it right back to bs_opts? Wouldn't it be easier to remove it from qemu_root_bds_opts and do a simple set_default_str("on") here? (Which is how the cache options work.) Kevin