From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38856) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VocvH-0007NE-T2 for qemu-devel@nongnu.org; Thu, 05 Dec 2013 12:41:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VocvB-0000tZ-TF for qemu-devel@nongnu.org; Thu, 05 Dec 2013 12:41:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:32142) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VocvB-0000sb-KQ for qemu-devel@nongnu.org; Thu, 05 Dec 2013 12:41:05 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id rB5Hf32h016437 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 5 Dec 2013 12:41:04 -0500 Message-ID: <52A0BAD8.90001@redhat.com> Date: Thu, 05 Dec 2013 18:41:44 +0100 From: Max Reitz MIME-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] [RFC] Using BlockdevRef in the block layer List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "qemu-devel@nongnu.org" Cc: Kevin Wolf , Stefan Hajnoczi Hi everyone, Quite recently, I sent a series to qemu-devel which allowed it to use=20 blkdebug and blkverify in some kind of a =93native=94 QMP mode; that is,=20 there was no need to parse the filename anymore, instead everything=20 could be passed through the blockdev options. The image filename could easily be specified through the filename=20 itself; however, the raw image for blkverify had to be specified through=20 some special option, which I just used =93x-raw=94 for. Kevin correctly=20 commented that this is a pretty ugly way to solve the issue and instead,=20 both the image to be verified and the raw image should be specified=20 through a BlockdevRef. When trying to implement this, I hit the problem that BlockdevRef allows=20 you to reference an existing block device; however, this seems currently=20 unimplemented. This is further hindered by the fact how this reference=20 is done: If you want to give a file to some driver such as qcow2 (or=20 blkdebug) which references an existing block device, the option dict=20 should look something like this: { 'driver': 'blkdebug', 'id': 'drive0-debug', 'file': 'drive0' } So, as you can see, the =93file=94 value now is simply a string (this is=20 what distinguishes a reference from a "real" file, in which case it=20 would be a dict). The problem is that blockdev_init() already uses this=20 case for just specifying a filename without any further options. My current solution is to ignore the =93file=94 value in case=20 blockdev_init() is called from qmp_blockdev_add(), but this doesn't=20 solve the general legacy issue. So, did I miss anything or is=20 referencing an existing block device really not supported so far and the=20 only meaning to the "file" option containing a pure string is specifying=20 a filename? Second, if specifying a reference to an existing device should really be=20 supported, bdrv_open() should ideally not call bdrv_file_open() anymore,=20 but a function bdrv_find_ref() instead which resolves a BlockdevRef=20 structure (for simplicity, it appears to be easier to use a QDict=20 equivalent to a BlockdevRef instead of the latter itself (since that=20 results in many effectively redundant conversions to and from those=20 representations)). However, bdrv_file_open() supports parsing protocol=20 filenames, which bdrv_find_ref() would not. As a result, it is probably=20 best to call bdrv_find_ref() from bdrv_file_open() instead and leave=20 bdrv_open() generally the way it is right now =96 yes, this is a question= .=20 ;-) (=93Do you agree?=94) Third, I planned to implement the blkdebug and blkverify QMP interface=20 by just making them BlockdevOptionsGenericFormat (with the addition of a=20 "test" BlockdevRef for blkverify). This will give them a =93file=94=20 automatically. However, this makes them =93drivers for the protocol level= =94=20 (or however this is properly called), i.e., they need to specify=20 bdrv_open() instead of bdrv_file_open() to work. But blkdebug and=20 blkverify are their own protocols with the current interface: Making=20 them require an underlying file will break the current interface with=20 the filename specifying the required options. To resolve this, I added=20 two new =93interfaces=94, blkdebug-qmp and blkverify-qmp, which reference= =20 the same functions as blkdebug and blkverify, respectively, however,=20 they offer bdrv_open() instead of bdrv_file_open(). These new block=20 drivers will thus not support the current interface, but they will be=20 properly supported through the QMP interface. In retrospect, I thought to have changed a lot more, but this is still=20 more than I actually thought would be required to adapt blkdebug and=20 blkverify to the =93standard=93 QMP interface in the first place, so I=20 considered it worth asking for comments before sending a rather large=20 patch series. ;-) Kind regards, Max