On 03/23/2017 05:55 AM, Markus Armbruster wrote: > BlockdevOptionsRbd member auth-supported is a list of struct > RbdAuthMethod, whose only member is enum RbdAuthSupport. There is no > reason for wrapping the enum in a struct.
Well, the previous patch removed the QemuOpts madness as a technical
reason that required the wrapper, leaving nothing else technically using
it at the moment. But there's still the design to think about...
> Delete the wrapper, and
> rename the enum.
This patch changes the QMP wire format. It MUST go in 2.9, otherwise,
we've baked in the insanity; that is, if we decide it is insanity [1]
>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
> ---
> block/rbd.c | 2 +-
> qapi/block-core.json | 15 ++-------------
> 2 files changed, 3 insertions(+), 14 deletions(-)
>
> +++ b/qapi/block-core.json
> @@ -2601,25 +2601,14 @@
>
>
> ##
> -# @RbdAuthSupport:
> -#
> -# An enumeration of RBD auth support
> -#
> -# Since: 2.9
> -##
> -{ 'enum': 'RbdAuthSupport',
> - 'data': [ 'cephx', 'none' ] }
> -
> -
> -##
> # @RbdAuthMethod:
> #
> # An enumeration of rados auth_supported types
> #
> # Since: 2.9
> ##
> -{ 'struct': 'RbdAuthMethod',
> - 'data': { 'auth': 'RbdAuthSupport' } }
> +{ 'enum': 'RbdAuthMethod',
> + 'data': [ 'cephx', 'none' ] }
Changes BlockdevOptionsRbd from:
..., "auth-supported": [ { "auth": "none" } ],
to the potentially much nicer:
..., "auth-supported": [ "none" ],
[1] But what if we want to make auth-supported be an array of flat
unions, where 'auth' is the discriminator that determines if additional
members are needed? In other words, the existing API would allow:
"auth-supported": [ { "auth": "cephx" },
{ "auth": "new", "password-id": "foo" } ]
for specifying a new auth type 'new' that comes with an associated
'password-id' field for looking up a corresponding 'secret' object used
for retrieving the associated password when using that type of
authorization. In that scenario, RbdAuthMethod would look more like
this pseudo-qapi:
{ 'union': 'RbdAuthMethod', 'base': { 'auth': 'RbdAuthSupport' },
'discriminator': 'auth',
'data': { 'none': {},
'cephx': {},
'new': { 'password-id': 'str' } } }
Assuming we are okay with not needing to extend the current one-member
RbdAuthMethod struct into a flat union, then this patch is fine, and you
can add:
Reviewed-by: Eric Blake <ebl...@redhat.com>
But if you think the flat union expansion path may ever prove useful,
then maybe we don't want this patch after all.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
