From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35510) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XgDVG-0001Yn-Ug for qemu-devel@nongnu.org; Mon, 20 Oct 2014 10:00:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XgDVB-0008IY-Rv for qemu-devel@nongnu.org; Mon, 20 Oct 2014 10:00:06 -0400 Received: from mx1.redhat.com ([209.132.183.28]:32522) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XgDVB-0008IT-L9 for qemu-devel@nongnu.org; Mon, 20 Oct 2014 10:00:01 -0400 Message-ID: <54451559.2020804@redhat.com> Date: Mon, 20 Oct 2014 15:59:53 +0200 From: Max Reitz MIME-Version: 1.0 References: <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> <20141020133100.GJ3585@noname.redhat.com> <54451273.8040007@kamp.de> <20141020135556.GK3585@noname.redhat.com> <54451524.5070709@kamp.de> In-Reply-To: <54451524.5070709@kamp.de> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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: Peter Lieven , Kevin Wolf Cc: famz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, armbru@redhat.com On 20.10.2014 at 15:59, Peter Lieven wrote: > On 20.10.2014 15:55, Kevin Wolf wrote: >> Am 20.10.2014 um 15:47 hat Peter Lieven geschrieben: >>> On 20.10.2014 15:31, Kevin Wolf wrote: >>>> 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 >>> Thank you. >>> >>> Would it be legit to recycle qemu_common_drive_opts from blockdev.c >>> for this? >> No, I don't think so. That one should in theory be only for BlockBackend >> options. For the short term, it still mixes BB and BDS options, but BDS >> options should be moved out step by step. In any case, it is only used >> for the top level. >> >> Any option that is parsed with qemu_opts_absorb_qdict() in >> bdrv_open_common() must also be handled there. If you don't ensure that >> and extract all the options that blockdev_init() knows without actually >> handling them, it can happen that invalid options are silently ignored >> (e.g. backing.werror should error out, but would be accepted). >> >> And please coordinate with Max, if both of you write a patch, that's >> wasted time. > > Max, if you don't have started I would use Kevins patch as basis? No, I haven't. Feel free to. Max