From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39515) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vl2Sg-0005Wc-HP for qemu-devel@nongnu.org; Mon, 25 Nov 2013 15:08:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vl2Sa-0005jp-Cv for qemu-devel@nongnu.org; Mon, 25 Nov 2013 15:08:50 -0500 Received: from mx1.redhat.com ([209.132.183.28]:4300) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vl2Sa-0005j0-4h for qemu-devel@nongnu.org; Mon, 25 Nov 2013 15:08:44 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rAPK8hx4003391 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 25 Nov 2013 15:08:43 -0500 Message-ID: <5293AE65.505@redhat.com> Date: Mon, 25 Nov 2013 21:09:09 +0100 From: Max Reitz MIME-Version: 1.0 References: <1385136658-16347-1-git-send-email-mreitz@redhat.com> <1385136658-16347-8-git-send-email-mreitz@redhat.com> <20131125144011.GJ3009@dhcp-200-207.str.redhat.com> In-Reply-To: <20131125144011.GJ3009@dhcp-200-207.str.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 7/7] blkverify: Don't require protocol filename List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Fam Zheng , qemu-devel@nongnu.org, Stefan Hajnoczi On 25.11.2013 15:40, Kevin Wolf wrote: > Am 22.11.2013 um 17:10 hat Max Reitz geschrieben: >> If the filename is not prefixed by "blkverify:" in >> blkverify_parse_filename(), the blkverify driver was not selected >> through that protocol prefix, but by an explicit command line option >> (like file.driver=3Dblkverify). Contrary to the current reaction, this= is >> not really a problem; the whole filename just has to be stored (in the >> x-image option) and the user has to manually specify the x-raw option. >> >> Signed-off-by: Max Reitz >> --- >> block/blkverify.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/block/blkverify.c b/block/blkverify.c >> index 3c63528..bdbdd68 100644 >> --- a/block/blkverify.c >> +++ b/block/blkverify.c >> @@ -78,7 +78,9 @@ static void blkverify_parse_filename(const char *fil= ename, QDict *options, >> =20 >> /* Parse the blkverify: prefix */ >> if (!strstart(filename, "blkverify:", &filename)) { >> - error_setg(errp, "File name string must start with 'blkverify= :'"); >> + /* There was no prefix; therefore, all options have to be alr= eady >> + present in the QDict (except for the filename) */ >> + qdict_put(options, "x-image", qstring_from_str(filename)); >> return; >> } > We don't want users to specify x-raw options, that's why it starts with > "x-" in the first place. So I'm not sure if this patch is a useful > intermediate step to make. Well, it's the last patch of the series, so we could just leave it out=20 for the time being. ;-) > What we want to allow in the end is something like this: > > { "execute": "blockdev-add", "options": { > { "driver": "blkverify", > "image": { > "driver": "qcow2", > "file": ... }, > "raw:" { > "driver": "raw", > "file": ... } } } > > Where "image" and "raw" are both of the BlockdevRef union type in QAPI, > i.e. there could also be a string that references an existing block > device. > > We'll probably want a function that takes a BlockdevRef and returns a > BlockDriverState; either by bdrv_open() on a new one, or by bdrv_ref() > on an existing one. Hm, yes, that seems far better. > Fam already has some code to achieve this in his BlockOp blockers > series, though not yet in a reusable way. I guess this series is the > good reason to actually request something reusable and then make use of > it here. > > I guess you two just need to coordinate who's going to implement it (Fa= m > by default, I'd assume?) Hm, are you referring to patch 5/7? I don't see right now how we could=20 use that code for BlockdevRef=85 I'd agree we could probably use such cod= e=20 (using BlockdevRef) there instead, but not really the other way round.=20 So, I think I could try to implement that function myself first. Max