[Qemu-devel] Re: RFC: blockdev_add & friends, brief rationale, QMP docs

[Kevin Wolf]

Am 04.06.2010 17:53, schrieb Markus Armbruster:
> Kevin Wolf <address@hidden> writes:
> 
>> Am 04.06.2010 16:16, schrieb Markus Armbruster:
>>> Our abstraction for formats is struct BlockDriver.
>>>
>>> Our abstraction for protocols is also struct BlockDriver.  Except for
>>> the special protocol "file", but that's detail.
>>
>> See above, file isn't really special.
> 
> I got confused by this code in bdrv_open_common():
> 
>     /* Open the image, either directly or using a protocol */
>     if (drv->bdrv_file_open) {
>         ret = drv->bdrv_file_open(bs, filename, open_flags);
>     } else {
>         ret = bdrv_file_open(&bs->file, filename, open_flags);
>         if (ret >= 0) {
>             ret = drv->bdrv_open(bs, open_flags);
>         }
>     }
> 
> When called by drive_init() via bdrv_open(), drv is the BlockDriver
> named by format=F.  If drv->bdrv_file_open, is drv a format or a
> protocol?  

A protocol. Formats use bdrv_open and get a BlockDriverState of the
underlying protocol as a parameter, whereas protocols use bdrv_file_open
and get the filename as a parameter.

> It's true for F in blkdebug, cow, http, https, ftp, ftps,
> tftp, nbd, file, host_device, host_floppy, host_cdrom, vvfat, so it's a
> protocol (except for cow, but Christoph tells me he's about to get that
> one off the list).  But what's the format then?

You're not supposed to use a protocol with format=F, even though
currently it works. Once cow is converted (it currently accesses the
file directly with POSIX functions instead of using the protocol) we can
make this an error so that it's not even possible to directly use a
protocol. Then the only way to get into the then branch is explicitly
calling bdrv_file_open.

To get the same behaviour as today with directly using a protocol,
you'll need to use raw as the format.

> The conditional's else seems clear: bdrv_file_open() finds the protocol
> in filename (probes the file if missing), and then opens with that
> protocol.
> 
>>> Examples:
>>>
>>> -drive file=foo.qcow2,format=qcow2
>>>
>>>  Format "qcow2", protocol "file" with argument filename "foo.img"
>>
>> Actually the protocol is guessed here. For this, not all protocols are
>> considered, it's only between file/host_device/host_cdrom/host_floppy
>> (these are the protocols implementing bdrv_probe_device, and file as the
>> default if no other protocol feels responsible)
> 
> Then we need a way to ask for this guessing, say (pseudo-)protocol
> "auto".

I agree.

>>> -drive file=nbd:unix:/tmp/my_socket,format=raw
>>>
>>>  Format "raw", protocol "nbd" with arguments domain "unix", filename
>>>  "/tmp/my_socket"
>>>
>>> -drive blkdebug:/tmp/blkdebug.cfg:fat:floppy:rw:/tmp/dir
>>>
>>>  Format not specified (system guesses one), protocol "blkdebug" with
>>>  argument filename "/tmp/blkdebug.cfg" stacked onto protocol "fat" with
>>>  arguments floppy true, dirname "/tmp/dir"
>>
>> These look right to me.
>>
>>>
>>> You see that -drive has a separate option for format, but has protocols
>>> encoded in option file, in their own mini-language.  Doesn't work for
>>> arbitrary filenames.  Besides, mini-languages to encode options in
>>> strings are quite inappropriate for QMP.
>>>
>>> So we need something cleaner for QMP.  Here's a sketch.  Instead of
>>>
>>> - "file": the disk image file to use (json-string, optional)
>>> - "format": disk format (json-string, optional)
>>>     - Possible values: "raw", "qcow2", ...
>>>
>>> have
>>>
>>> - "format": disk format (json-string, optional)
>>>     - Possible values: "raw", "qcow2", ...
>>> - "protocol": json-array of json-object
>>>   Each element object has a member "name"
>>>     - Possible values: "file", "nbd", ...
>>>   Additional members depend on the value of "name".
>>>   For "name" = "file":
>>>     - "file": file name (json-string)
>>>   For "name" = "nbd":
>>>     - "domain": address family (json-string, optional)
>>>         - Possible values: "inet" (default), "unix"
>>>     - "file": file name (json-string), only with "domain" = "unix"
>>>     - "host": host name (json-string), only with "domain" = "inet"
>>>     - "port": port (json-int), only with "domain" = "inet"
>>>   ...
>>>
>>> You get the idea.
>>>
>>> Comments?
>>
>> Makes sense.
>>
>> So blkdebug would define a field "protocol" (json-object) that it uses
>> to initialize the underlying protocol and we would get the stacking this
>> way?
> 
> No, my proposal represents the stack of protocols as json-array, not by
> nesting them.  I should have given examples.  Reusing my three examples
> from above:

Ah, sorry, I missed the part with the array. That's probably even better.

> 
> * Format "qcow2", protocol "auto" with argument filename "foo.img"
> 
>     "format": "qcow2",
>     "protocol": [{ "name": "auto", "file": "foo.qcow2" }],
> 
> * Format "raw", protocol "nbd" with arguments domain "unix", filename
>   "/tmp/my_socket"
> 
>     "format": "raw"
>     "protocol": [{ "name": "nbd", "domain": "unix", "file": "/tmp/my_socket" 
> }]
> 
> * Format not specified (system guesses one), protocol "blkdebug" with
>   argument filename "/tmp/blkdebug.cfg" stacked onto protocol "fat" with
>   arguments floppy true, dirname "/tmp/dir"
> 
>     "protocol": [{ "name": "blkdebug", "file": "/tmp/blkdebug.cfg" },
>                  { "name": "fat", "floppy": true, "dir": "/tmp/dir" }]
> 
> With nesting, we'd get something like this instead:
> 
> * Format "qcow2", protocol "auto" with argument filename "foo.img"
> 
>     "format": "qcow2",
>     "protocol": { "name": "auto", "file": "foo.qcow2" }
> 
> * Format "raw", protocol "nbd" with arguments domain "unix", filename
>   "/tmp/my_socket"
> 
>     "format": "raw"
>     "protocol": { "name": "nbd", "domain": "unix", "file": "/tmp/my_socket" }
> 
> * Format not specified (system guesses one), protocol "blkdebug" with
>   argument filename "/tmp/blkdebug.cfg" stacked onto protocol "fat" with
>   arguments floppy true, dirname "/tmp/dir"
> 
>     "protocol": { "name": "blkdebug", "file": "/tmp/blkdebug.cfg",
>         "protocol": { "name": "fat", "floppy": true, "dir": "/tmp/dir" }
>     }
> 
> Nesting has one small advantage, namely a protocol's property "stacks on
> top of another protocol" becomes explicit in syntax: it's true iff it
> has a "protocol" member.

Right. The array has a different nice advantage though: Stacked protocol
drivers like blkdebug currently need to call bdrv_file_open manually to
get their underlying protocol. With the array we can do it in generic
code like for formats.

Kevin