From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56729) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XgD3Q-0001Ru-ES for qemu-devel@nongnu.org; Mon, 20 Oct 2014 09:31:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XgD3K-00056n-Jl for qemu-devel@nongnu.org; Mon, 20 Oct 2014 09:31:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:11466) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XgD3H-00055F-MC for qemu-devel@nongnu.org; Mon, 20 Oct 2014 09:31:14 -0400 Date: Mon, 20 Oct 2014 15:31:00 +0200 From: Kevin Wolf Message-ID: <20141020133100.GJ3585@noname.redhat.com> References: <5444D28F.4010005@kamp.de> <5444D57C.6080409@redhat.com> <5444DDF4.40800@kamp.de> <5444F730.4000003@redhat.com> <5444FD07.6030702@kamp.de> <5444FDBE.9060501@redhat.com> <54450491.7060506@kamp.de> <54450AD7.8000901@redhat.com> <54450BED.6000101@kamp.de> <54450C9A.8040807@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <54450C9A.8040807@redhat.com> Subject: Re: [Qemu-devel] [PATCH] block: add a knob to disable multiwrite_merge List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: famz@redhat.com, Peter Lieven , qemu-devel@nongnu.org, stefanha@redhat.com, armbru@redhat.com 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