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

[Max Reitz <mreitz@redhat.com>]

On 05.12.2013 19:35, Max Reitz wrote:
> On 05.12.2013 18:41, Max Reitz wrote:
>> […]
>>
>> 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?”)
>
> I noticed only just now that the current design does not seem to allow 
> nesting of files (i.e., driver=blkdebug-qmp, file.driver=qcow2, 
> file.file.driver=file). Perhaps I do have to call bdrv_find_ref() in 
> bdrv_open() and only resort to bdrv_file_open() if a filename that 
> must be parsed was given...?

Okay, I removed bdrv_find_ref() in favor of letting bdrv_file_open() 
handle everything. If a reference to an existing block device is given 
(as an additional parameter, since the QDict cannot handle this because 
of the fact that a blockdev reference will replace the QDict instead of 
being part of it), it is used. Otherwise, if the block driver found does 
not support bdrv_file_open(), it will recursively call bdrv_open() 
instead of bdrv_open_common(). Concept-wise, this is perhaps a pretty 
ugly solution; but code-wise, it looks pretty well, especially 
considering that this does not change the current code much in terms of 
LoC changed (and, more importantly, it just works; at least apparently, 
so far).

On the other hand, as far as I know, the current “file” concept is not 
determined to stay anyway, is it? At some point, we will have to 
implement something to actually chain multiple block drivers. Allowing 
this through “file” is as far as the current concept goes and there is 
no way of getting around recursion.

The only question to me here is whether it is a good idea to allow calls 
to bdrv_file_open() on block drivers which don't actually support that 
function; but I don't know how to solve this problem in any other way, 
and I personally don't want to put too much effort into fixing the 
current model if we want to have a new one as soon as possible anyway.

Max