From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51546) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VMzWL-0002sk-LQ for qemu-devel@nongnu.org; Fri, 20 Sep 2013 08:09:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VMzWF-0006Ac-Kv for qemu-devel@nongnu.org; Fri, 20 Sep 2013 08:09:13 -0400 Received: from nodalink.pck.nerim.net ([62.212.105.220]:37366 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VMzWF-00069N-0l for qemu-devel@nongnu.org; Fri, 20 Sep 2013 08:09:07 -0400 Date: Fri, 20 Sep 2013 14:08:50 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20130920120849.GA2872@irqsave.net> References: <20130917124413.GA822@stefanha-thinkpad.str.redhat.com> <20130918150527.GA5025@irqsave.net> <20130919082653.GC22814@stefanha-thinkpad.redhat.com> <20130919085750.GA3631@dhcp-200-207.str.redhat.com> <20130919125536.GA12498@irqsave.net> <20130919132146.GB3631@dhcp-200-207.str.redhat.com> <20130919133818.GB12498@irqsave.net> <20130919142354.GC3631@dhcp-200-207.str.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20130919142354.GC3631@dhcp-200-207.str.redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] Merging the quorum block driver List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: =?iso-8859-1?Q?Beno=EEt?= Canet , Stefan Hajnoczi , qemu-devel@nongnu.org, Stefan Hajnoczi Le Thursday 19 Sep 2013 =E0 16:23:54 (+0200), Kevin Wolf a =E9crit : > Am 19.09.2013 um 15:38 hat Beno=EEt Canet geschrieben: > > Le Thursday 19 Sep 2013 =E0 15:21:46 (+0200), Kevin Wolf a =E9crit : > > > Am 19.09.2013 um 14:55 hat Beno=EEt Canet geschrieben: > > > > Le Thursday 19 Sep 2013 =E0 10:57:50 (+0200), Kevin Wolf a =E9cri= t : > > > > > Am 19.09.2013 um 10:26 hat Stefan Hajnoczi geschrieben: > > > > > > On Wed, Sep 18, 2013 at 05:05:27PM +0200, Beno=EEt Canet wrot= e: > > > > > > > Le Tuesday 17 Sep 2013 =E0 14:44:13 (+0200), Stefan Hajnocz= i a =E9crit : > > > > > > > > Hi Benoit, > > > > > > > > Kevin and I had a chance to chat face-to-face and we disc= ussed what > > > > > > > > concrete changes are necessary to merge quorum (without s= olving all the > > > > > > > > other block layers problems at once). > > > > > > > > > > > > > > > > I think quorum could be merged relatively quickly (and wi= thout massive > > > > > > > > BlockFilter investments) by changing the following: > > > > > > > > > > > > > > > > 1. Defining children on the command-line > > > > > > > > > > > > > > > > Existing "filter" drivers use the protocol filename to em= bed their > > > > > > > > children, for example the blkverify driver. This is a bi= g hack because > > > > > > > > we have no proper syntax or escaping for the embedded dri= ve definitions > > > > > > > > in the file=3D option. > > > > > > > > > > > > > > > > This was one of the main arguments against merging quorum= . Now that > > > > > > > > Kevin has implemented driver-specific open options (see > > > > > > > > block/qcow2.c:qcow2_runtime_opts), it is possible for the= quorum driver > > > > > > > > to open children specific on the command-line: > > > > > > > > > > > > > > > > -drive if=3Dnone,file.driver=3Dquorum,format=3Draw,\ > > > > > > > > file.children0.file=3D/nfs1/test.qcow2,\ > > > > > > > > file.children1.file=3D/nfs2/test.qcow2,\ > > > > > > > > file.children2.file=3D/nfs3/test.qcow2 > > > > > > > > > > By the way, I think he concrete syntax might have to be a bit d= ifferent > > > > > so it can be mapped to QAPI for blockdev-add. I think we'll wan= t to use > > > > > a JSON list for the children; a mapping to QDict and to the com= mand line > > > > > is yet to be implemented I guess. > > > > > > > > Hi, > > > > > > > > Just to make sure I'll code the right thing; is the following cor= rect ? > > > > > > > > -drive if=3Dnone,file.driver=3Dquorum,format=3Draw,\ > > > > file.children=3D[/nfs1/test.qcow2, /nfs2/test.qcow2, /nfs3= /test.qcow2], \ > > > > file.vote_threshold=3D2 > > > > > > I was thinking more along the lines of: > > > > > > -drive if=3Dnone,file.driver=3Dquorum,format=3Draw,\ > > > file.children[0].file.filename=3D/nfs1/test.qcow2, \ > > > file.children[1].file.filename=3D/nfs2/test.qcow2, \ > > > file.children[1].file.cache.direct=3Don, \ > > > file.children[2].file.driver=3Dnbd, \ > > > file.children[2].file.host=3Dlocalhost, \ > > > file.vote_threshold=3D2 > > > > > > The exact syntax doesn't matter all that much as long as it can be > > > mapped to QAPI, but as you can see this allows specifying detailed > > > options even for the children. I think this is a requirement. > > > > So does it mean that on top of your blockdev branch qemu_opts_to_qdic= t should do > > a little parsing of opt->name to regroup options of the same children= together ? > > > > e.g: converting the incoming qoption into the qdict equivalent of: > > dict =3D { "driver": "quorum", > > "children[0]": "file.filename=3D/nfs1/test.qcow2", > > "children[1]": "file.filename=3D/nfs2/test.qcow2,file.cache.= direct=3Don", > > "children[2]": "file.driver=3Dnbd,file.host=3Dlocalhost", > > "vote_threshold": 2 } > > No, the qdict is supposed to be completely flat. I think there are two > parts to it: > > 1. Extend qdict_flatten() so that it converts QLists into an array > representation. The resulting qdict should look pretty much like the > command line: > > dict =3D { "driver": "quorum", > "children[0].file.filename": "/nfs1/test.qcow2", > "children[1].file.filename": "/nfs2/test.qcow2", > "children[1].file.cache.direct": "on", > "children[2].file.driver": "nbd", > "children[2].file.host": "localhost", > "vote_threshold": 2 } > > 2. The option parsing code in quorum must find a way to walk this > flattened children array. Basically it will end up checking if > the keys start with a "children[%d]", and if so, use > extract_subqdict() to get the full array element back. I made this. The block driver complains that no filename is provided at startup. --- } else if (!drv->bdrv_parse_filename && !filename) { qerror_report(ERROR_CLASS_GENERIC_ERROR, "The '%s' block driver requires a file name", drv->format_name); ret =3D -EINVAL; goto fail; } ---- How can I write a patch to fix this ? Best regards Beno=EEt > > Kevin >