Re: [Qemu-devel] QMP interface for drive-add (or even blockdev-add)

[Eric Blake]

On 05/16/2013 02:24 AM, Kevin Wolf wrote:
> Another thing that we'll probably want from QAPI is some kind of
> inheritance, like this:
> 
> { "type": "BlockFileBase", data: { "driver": "str", "cache": "CacheEnum",
>   ... }
> 
> { "type": "BlockFileNBD", "extends": "BlockFileBase", "data": {
>     "port": "*int", "host": "str", ... }}

This sounds independently useful.

> 
>> In working all of this out upstream, we may decide to tweak the command
>> line syntax (after all, how do I know that file.driver controls which
>> arm of the union to take, and therefore which other file.FOO sub-options
>> are available?), so I'm glad we disabled sub-maps in qemu 1.5 until we
>> can match the QMP exposure of submaps.
> 
> Okay, let's take a step back here. The idea was more or less that you
> can specify each BlockDriverState by itself in the end, like this:
> 
> { "execute": "blockdev-add", "data": {
>     "id": "my_file", "driver": "file", "filename": "test.qcow2" }}
> 
> { "execute": "blockdev-add", "data": {
>     "id": "my_qcow2", "driver": "qcow2", "file": "my_file" }}

So, you are using "driver" as the key for which arm of the discriminated
union supplies the remaining arguments.  That would look more like:

{ 'type': 'BlockDriverFile', 'data': {
  'name': 'str' } }
{ 'type': 'BlockDriverQcow2', 'data': {
  'file': 'str' } }
{ 'type': 'BlockDriverNBD', 'data': {
  '*port': 'int', '*host': 'str' } }

{ 'union': 'BlockDriver', 'data': {
  'file':  'BlockDriverFile',
  'qcow2': 'BlockDriverQcow2',
  'nbd':   'BlockDriverNBD' } }

{ 'command': 'blockdev-add', 'data': {
  'id': 'str', 'driver': 'BlockDriver' }

and called like:

{ "execute": "blockdev-add", "arguments": {
  "id": "my_file",
  "driver": { "type": "file", "data": { "name": "test.qcow2" } } } }
{ "execute": "blockdev-add", "arguments": {
  "id": "my_qcow2",
  "driver": { "type": "qcow2", "data": { "file": "my_file" } } } }

Dynamic introspection on 'BlockDriver' would let us know what 'type'
fields we can supply, and the type field then determines what we can use
in the 'data' dictionary.

While QMP remains quite nested to make the use of 'union' easy to parse
and introspect, simplifying things into a flatter namespace for the
command line still makes sense:

-drive id=my_file,driver=file,driver.name=test.qcow2
-drive id=my_qcow2,driver=qcow2,driver.file=my_file

seems legible enough and still something that the command line parser
can explode back into the fully-structured QMP command.

> 
> But at least as an interface for human users this would become very
> tedious, so "file" became inlined and you specify only the qcow2 image
> and reference the options of the underlying raw-posix driver by using
> its file option:
> 
> {
>     "execute": "blockdev-add",
>     "data": {
>         "id": "my_qcow2",
>         "driver": "qcow2",
>         "file": {
>             "id": "my_file",
>             "driver": "file",
>             "filename": "test.qcow2"
>         }
>     }
> }

So we want nesting.  Still might be possible, by tweaking the types I
gave above:

{ 'type': 'BlockDriverFile', 'data': {
  'name': 'str' } }
{ 'union': 'BlockDriverQcow2', 'data': {
  'file': 'str',
  'block': 'BlockDriverNested' } }
{ 'type': 'BlockDriverNBD', 'data': {
  '*port': 'int', '*host': 'str' } }
{ 'type': 'BlockDriverNested', 'data': {
  'id': 'str', 'driver': 'BlockDriver' } }

{ 'union': 'BlockDriver', 'data': {
  'file':  'BlockDriverFile',
  'qcow2': 'BlockDriverQcow2',
  'nbd':   'BlockDriverNBD' } }

{ 'command': 'blockdev-add', 'data': {
  'id': 'str', 'driver': 'BlockDriver' }

and called like this in longhand:

{ "execute": "blockdev-add", "arguments": {
  "id": "my_file",
  "driver": { "type": "file", "data": { "name": "test.qcow2" } } } }
{ "execute": "blockdev-add", "arguments": {
  "id": "my_qcow2",
  "driver": { "type": "qcow2", "data": {
     "type": "file", "data": "my_file" } } } }

on the command line as:
-drive id=my_file,driver=file,driver.name=test.qcow2
-drive id=my_qcow2,driver=qcow2,driver.file=my_file

or this in shorthand:

{ "execute": "blockdev-add", "arguments": {
  "id": "my_qcow2",
  "driver": { "type": "qcow2", "data": { "type": "block", "data": {
     "id": "my_file", "driver": {
        "type": "file", "data": { "name": "test.qcow2" } } } } } } }

on the command line as:
-drive id=my_qcow2,driver=qcow2,driver.block.id=my_file,\
driver.block.driver=file, driver.block.name=test.qcow2



Hmm, looking at that QMP, it starts to nest rather deeply, I see a
couple of possibilities for less QMP syntax:

Right now, the qapi-schema.json file requires discriminated unions that
are a dictionary of exactly two members: a 'type' member that specifies
which arm of the union, and a 'data' member that specifies a JSON object
of all remaining data (and I don't even know if we can use 'str', or if
we have to use the 'String' wrapper, in the above attempt to make
BlockDriverQcow2.file be a string).  Your idea of inheritance means we
could design an inherited union - by inheriting from a base class, the
union gains all the dictionary members of the base class plus the
additional 'type' and 'data' member.  Something like:

{ 'type': BlockDriverBase', 'data': {
  'id': 'str' }

{ 'type': 'BlockDriverFile', 'data': {
  'name': 'str' } }
{ 'union': 'BlockDriverQcow2', 'data': {
  'file': 'str',
  'block': 'BlockDriver' } }
{ 'type': 'BlockDriverNBD', 'data': {
  '*port': 'int', '*host': 'str' } }

{ 'union': 'BlockDriver', 'extends': 'BlockDriverBase', 'data': {
  'file':  'BlockDriverFile',
  'qcow2': 'BlockDriverQcow2',
  'nbd':   'BlockDriverNBD' } }

{ 'command': 'blockdev-add', 'data': {
  'driver': 'BlockDriver' }

and called like:

{ "execute": "blockdev-add", "arguments": {
  "driver": { "id": "my_file", "type": "file",
              "data": { "name": "test.qcow2" } } } }
{ "execute": "blockdev-add", "arguments": {
  "driver": { "id": "my_qcow2", "type": "qcow2",
              "data": { "type": "file", "data": "my_file" } } } }

or in shorthand:

{ "execute": "blockdev-add", "arguments": {
  "driver": { "id": "my_qcow2", "type": "qcow2",
              "data": { "type": "block", "data": {
                        "id": "my_file", "type": "file",
                        "data": { "name": "test.qcow2" } } } } } }


Next, I've noticed that qapi-schema currently always specifies a
dictionary object for 'data' when defining a 'command', which requires
another level of dictionary nesting for using a union.  But since a
union is always a dictionary, what if we modify the generator to
directly allow a union type for 'data' rather than the usual object
notation?  Likewise, what if we tweak the QMP parser to so that a
metasyntax of 'name':'uniontype' allows a shorthand of 'foo':{...}
anywhere we currently have to use 'name':{'type':'foo','data':{...}}
longhand.  (That is, anywhere the parser expects a union, if the
dictionary is missing the 'type' and 'data' fields but has an unknown
'name'/'value' pair, then reconstruct a 'type' and 'data' field
accordingly).  Observe:

{ 'type': BlockDriverBase', 'data': {
  'id': 'str' }

{ 'type': 'BlockDriverFile', 'data': {
  'name': 'str' } }
{ 'union': 'BlockDriverQcow2', 'data': {
  'file': 'str',
  'block': 'BlockDriver' } }
{ 'type': 'BlockDriverNBD', 'data': {
  '*port': 'int', '*host': 'str' } }

{ 'union': 'BlockDriver', 'extends': 'BlockDriverBase', 'data': {
  'file':  'BlockDriverFile',
  'qcow2': 'BlockDriverQcow2',
  'nbd':   'BlockDriverNBD' } }

{ 'command': 'blockdev-add', 'data': 'BlockDriver' }

and called like:

{ "execute": "blockdev-add", "arguments": {
  "id": "my_file", "file": { "name": "test.qcow2" } } }
{ "execute": "blockdev-add", "arguments": {
  "id": "my_qcow2", "qcow2": { "file": "my_file" } } } }

or with nesting:

{ "execute": "blockdev-add", "arguments": {
  "id": "my_qcow2", "qcow2": { "block": {
     "id": "my_file", "file": { "name": "test.qcow2" } } } } }

and where the parser treats it identically with the longer:

{ "execute": "blockdev-add", "arguments": {
  "id": "my_qcow2", "type": "qcow2",
  "data": { "type": "block", "data": {
     "id": "my_file", "type": "file",
     "data": { "name": "test.qcow2" } } } } }


Still slightly more verbose than your proposal (I had to add a "file" or
"block" discriminator to control whether qcow2 referred to its
underlying block device by name or by inline nested struct), but seems
like it might be easier to code into current qapi.


> 
> So far I've neglected the option to refer to an existing
> BlockDriverState using its ID instead of specifying a new BDS inline.

I've covered that - see how I made BlockDriverQcow2 a union, which was
discriminated either by type=file data=name, or type=block data=nested
dictionary.

> I'm not even sure yet how we can make it an option to use either a string
> or an embedded struct, but we'll need both options.

My ideas above fit it into the existing framework by using nested unions.

> 
> In this example, you can also see that "file.driver" doesn't specify
> which arm of the union "my_qcow2" takes, but it's the type of "my_file"
> that it influences.
> 
> Anyway, I hope this makes it a bit clearer what my thoughts behind this
> system were and where I was planning to go with it.

And I hope my ideas also help in coming up with a usable design and/or
improvements to how we generate code based on qapi JSON callouts.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature