Re: [Qemu-devel] [PATCH] block: Remove "options" indirection from blockdev-add

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2376 bytes --]

On 10/07/2016 10:38 AM, Kevin Wolf wrote:
> Now that QAPI supports boxed types, we can have unions at the top level
> of a command, so let's put our real options directly there for
> blockdev-add instead of having a single "options" dict that contains the
> real arguments.
> 
> blockdev-add is still experimental and we already made substantial
> changes to the API recently, so we're free to make changes like this
> one, too.

I like it, and it needs to make 2.8.  Max was correct that there are
some doc changes missing, so looking forward to v2.

> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> 
> Yes, that's right. Ignoring the test cases, this is a one-liner in
> the schema without any C code changes. :-)

Yay! My qapi refactoring work is paying dividends.

The lack of C code changes is because we convert everything to QDict and
then manually parse it using QemuOpts, rather than sticking to the nicer
qapi structs.  Someday, it would be nice to use the actual qapi structs
all the way, but that doesn't have to be today.

> +++ b/qapi/block-core.json
> @@ -2321,7 +2321,7 @@
>  #
>  # Since: 1.7
>  ##
> -{ 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
> +{ 'command': 'blockdev-add', 'data': 'BlockdevOptions', 'boxed': true }

Eventually, when we rename x-blockdev-del, we'll want some sort of doc
note here that even though this command has existed since 1.7, it is
fundamentally different in 2.8 and should not be used in anger unless
blockdev-del is also defined.

> +++ b/tests/qemu-iotests/041
> @@ -194,10 +194,9 @@ class TestSingleBlockdev(TestSingleDrive):
>      def setUp(self):
>          TestSingleDrive.setUp(self)
>          qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, target_img)
> -        args = {'options':
> -                    {'driver': iotests.imgfmt,
> -                     'node-name': self.qmp_target,
> -                     'file': { 'filename': target_img, 'driver': 'file' } } }
> +        args = {'driver': iotests.imgfmt,
> +                'node-name': self.qmp_target,
> +                'file': { 'filename': target_img, 'driver': 'file' } }

Less nesting is indeed a bit easier to read :)

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]