From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39088) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VkxKm-0003wM-VG for qemu-devel@nongnu.org; Mon, 25 Nov 2013 09:40:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VkxKg-0002vt-1y for qemu-devel@nongnu.org; Mon, 25 Nov 2013 09:40:20 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48147) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VkxKf-0002vn-OI for qemu-devel@nongnu.org; Mon, 25 Nov 2013 09:40:13 -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 rAPEeCwm016564 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 25 Nov 2013 09:40:13 -0500 Date: Mon, 25 Nov 2013 15:40:11 +0100 From: Kevin Wolf Message-ID: <20131125144011.GJ3009@dhcp-200-207.str.redhat.com> References: <1385136658-16347-1-git-send-email-mreitz@redhat.com> <1385136658-16347-8-git-send-email-mreitz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1385136658-16347-8-git-send-email-mreitz@redhat.com> 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: Max Reitz Cc: Fam Zheng , qemu-devel@nongnu.org, Stefan Hajnoczi 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=blkverify). 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 *filename, QDict *options, > > /* 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 already > + 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. 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. 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 (Fam by default, I'd assume?) Kevin