[Qemu-devel] [RFC] Using BlockdevRef in the block layer

[Max Reitz <mreitz@redhat.com>]

Hi everyone,

Quite recently, I sent a series to qemu-devel which allowed it to use 
blkdebug and blkverify in some kind of a “native” QMP mode; that is, 
there was no need to parse the filename anymore, instead everything 
could be passed through the blockdev options.

The image filename could easily be specified through the filename 
itself; however, the raw image for blkverify had to be specified through 
some special option, which I just used “x-raw” for. Kevin correctly 
commented that this is a pretty ugly way to solve the issue and instead, 
both the image to be verified and the raw image should be specified 
through a BlockdevRef.

When trying to implement this, I hit the problem that BlockdevRef allows 
you to reference an existing block device; however, this seems currently 
unimplemented. This is further hindered by the fact how this reference 
is done: If you want to give a file to some driver such as qcow2 (or 
blkdebug) which references an existing block device, the option dict 
should look something like this:

{
'driver': 'blkdebug',
'id': 'drive0-debug',
'file': 'drive0'
}

So, as you can see, the “file” value now is simply a string (this is 
what distinguishes a reference from a "real" file, in which case it 
would be a dict). The problem is that blockdev_init() already uses this 
case for just specifying a filename without any further options.

My current solution is to ignore the “file” value in case 
blockdev_init() is called from qmp_blockdev_add(), but this doesn't 
solve the general legacy issue. So, did I miss anything or is 
referencing an existing block device really not supported so far and the 
only meaning to the "file" option containing a pure string is specifying 
a filename?

Second, if specifying a reference to an existing device should really be 
supported, bdrv_open() should ideally not call bdrv_file_open() anymore, 
but a function bdrv_find_ref() instead which resolves a BlockdevRef 
structure (for simplicity, it appears to be easier to use a QDict 
equivalent to a BlockdevRef instead of the latter itself (since that 
results in many effectively redundant conversions to and from those 
representations)). However, bdrv_file_open() supports parsing protocol 
filenames, which bdrv_find_ref() would not. As a result, it is probably 
best to call bdrv_find_ref() from bdrv_file_open() instead and leave 
bdrv_open() generally the way it is right now – yes, this is a question. 
;-) (“Do you agree?”)

Third, I planned to implement the blkdebug and blkverify QMP interface 
by just making them BlockdevOptionsGenericFormat (with the addition of a 
"test" BlockdevRef for blkverify). This will give them a “file” 
automatically. However, this makes them “drivers for the protocol level” 
(or however this is properly called), i.e., they need to specify 
bdrv_open() instead of bdrv_file_open() to work. But blkdebug and 
blkverify are their own protocols with the current interface: Making 
them require an underlying file will break the current interface with 
the filename specifying the required options. To resolve this, I added 
two new “interfaces”, blkdebug-qmp and blkverify-qmp, which reference 
the same functions as blkdebug and blkverify, respectively, however, 
they offer bdrv_open() instead of bdrv_file_open(). These new block 
drivers will thus not support the current interface, but they will be 
properly supported through the QMP interface.

In retrospect, I thought to have changed a lot more, but this is still 
more than I actually thought would be required to adapt blkdebug and 
blkverify to the “standard“ QMP interface in the first place, so I 
considered it worth asking for comments before sending a rather large 
patch series. ;-)


Kind regards,

Max