Re: [Qemu-devel] [PATCH] block: add a knob to disable multiwrite_merge

[Kevin Wolf]

Am 20.10.2014 um 15:22 hat Max Reitz geschrieben:
> On 20.10.2014 at 15:19, Peter Lieven wrote:
> >On 20.10.2014 15:15, Max Reitz wrote:
> >>On 20.10.2014 at 14:48, Peter Lieven wrote:
> >>>On 20.10.2014 14:19, Max Reitz wrote:
> >>>>On 2014-10-20 at 14:16, Peter Lieven wrote:
> >>>>>On 20.10.2014 13:51, Max Reitz wrote:
> >>>>>>On 2014-10-20 at 12:03, Peter Lieven wrote:
> >>>>>>[...]
> >>>>>
> >>>>>Can you further help here. I think my problem was that I
> >>>>>don't have access to the commandline options in
> >>>>>bdrv_open?!
> >>>>
> >>>>You do. It's the "options" QDict. :-)
> >>>
> >>>Maybe I just don't get it.
> >>>
> >>>If I specify
> >>>
> >>>qemu -drive if=virtio,file=image.qcow2,write-merging=off
> >>>
> >>>and check with
> >>>
> >>>qdict_get_try_bool(options, "write-merging", true);
> >>>
> >>>in bdrv_open() directly before bdrv_swap I always get true.
> >>
> >>Hm, judging from fprintf(stderr, "%s\n",
> >>qstring_get_str(qobject_to_json_pretty(QOBJECT(options))));,
> >>it's there for me (directly after qdict_del(options,
> >>"node-name"). The output is:
> >>
> >>Qemu wrote:
> >>>{
> >>>    "filename": "image.qcow2"
> >>>}
> >>>{
> >>>    "write-merging": "off"
> >>>}
> >>>qemu-system-x86_64: -drive
> >>>if=virtio,file=image.qcow2,write-merging=off: could not open
> >>>disk image image.qcow2: Block format 'qcow2' used by device
> >>>'virtio0' doesn't support the option 'write-merging'
> >>
> >>But as you can see, it's a string and not a bool. So the problem
> >>is that there are (at least) two parameter "types" in qemu: One
> >>is just giving a QDict, and the other are QemuOpts. QDicts are
> >>just the raw user input and the user can only input strings, so
> >>everything is just a string. As far as I know, typing everything
> >>correctly is done by converting the QDict to a QemuOpts object
> >>(as you can see in generally every block driver which supports
> >>some options (e.g. qcow2) and also in blockdev_init(), it's
> >>qemu_opts_absorb_qdict()).
> >>
> >>Sooo, right, I forgot that. Currently, there are no non-string
> >>non-block-driver-specific options for mid-tree BDS (in contrast
> >>to the root BDS, which are parsed in blockdev_init()), so you
> >>now have the honorable task of introducing such a QemuOptsList
> >>along with qemu_opts_absorb_qdict() and everything to
> >>bdrv_open_common(). *cough*
> >
> >I would appreciate if someone with better knowledge of this whole
> >stuff would start this. Or we postpone this know until all the
> >ongoing conversions are done.
> 
> I can try and create some barebone which your patches can then be
> based on. I probably don't have the knowledge either, but I'm daring
> enough to do it anyway. ;-)

Actually I have some patches somewhere [1] that introduce a QemuOpts for
bdrv_open_common(). I intended to use that for cache modes, but as I
explained in our KVM Forum presentation, it's not quite as easy as I
thought it would be and so the patch series isn't ready yet.

Anyway, having the QemuOpts there for driver-independent options is
probably the way to go. Feel free to remove the caching from my
patch and keep only the node-name part. Then it can be a preparatory
patch for your series where you simply add a new option to the list.

Kevin

[1] 
http://repo.or.cz/w/qemu/kevin.git/commitdiff/9c22aee04cf0bdf6a3858340bc6ff27d6805254f