On 04/21/2017 08:25 AM, Kevin Wolf wrote: > Am 21.04.2017 um 05:55 hat Fam Zheng geschrieben: >> Similar to share-rw qdev property, this will force the opened images to >> allow shared write permission of other programs. >> >> Signed-off-by: Fam Zheng <f...@redhat.com> > > General observation: We were considering to make share-rw require > read-only. Some of the commands converted here always open the image > read-write, so if we go ahead with the restriction, will the option > become useless in many of the subcommands? >
>> static BlockBackend *img_open_opts(const char *optstr,
>> QemuOpts *opts, int flags, bool
>> writethrough,
>> - bool quiet)
>> + bool quiet, bool share_rw)
>> {
>> QDict *options;
>> Error *local_err = NULL;
>> BlockBackend *blk;
>> options = qemu_opts_to_qdict(opts, NULL);
>> + if (share_rw) {
>> + qdict_put(options, BDRV_OPT_FORCE_SHARED_WRITE,
>> qbool_from_bool(true));
>> + }
>
> It's interesting that you chose a conditional qdict_put for true rather
> than an unconditional one for share_rw here. The difference becomes
> visible when someone sets both -U and share-rw=off; we need to decide
> which one should take precedence.
>
> Generally, we always give explicit options the precedence, so if we were
> to follow suit here, we would set share-rw here only if the option isn't
> already set. For strings, we have qdict_set_default_str() to achieve
> this, for bools we probably need a new function (or does Eric's series
> which introduces qdict_put_bool() also introduce a similar function,
> like some qdict_set_default_bool?)
It doesn't (yet), but I could add one if it is a common pattern that we
envision using.
>> @@ -3126,6 +3182,9 @@ static int img_rebase(int argc, char **argv)
>> qdict_put(options, "driver",
>> qstring_from_str(bs->backing_format));
>> }
>>
>> + if (share_rw) {
>> + qdict_put(options, BDRV_OPT_FORCE_SHARED_WRITE,
>> qbool_from_bool(true));
>
> This is longer than 80 lines and wrapping wouldn't make it unreadable. I
> think there are more similar instances in this series (even though you
> replied to the patchew mail that they are intentional).
This one would indeed benefit from my patch for qdict_put_bool().
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
