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
