On Thu 15 Sep 2016 12:51:27 PM CEST, Kevin Wolf wrote:
>> @@ -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.
You're right, I assumed that with read-only=on,snapshot=on the temporary
file would still be r/w.
I'll fix it.
>> + 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.)
Yeah, I was mirroring blockdev_init() (there it's easier to keep it in
qemu_common_drive_opts), but in this case there's no need to do it.
I'll fix it too.
Thanks!
Berto
