Re: [Qemu-block] [Qemu-devel] [PATCH v3 5/5] block/rbd: add support for 'mon_host', 'auth_supported' via QAPI

[Jeff Cody]

On Tue, Feb 28, 2017 at 03:34:10PM +0100, Markus Armbruster wrote:
> Starting with just the QAPI schema.
> 
> Jeff Cody <address@hidden> writes:
> 
> > This adds support for three additional options that may be specified
> > by QAPI in blockdev-add:
> >
> >     server: host, port
> >     auth method: either 'cephx' or 'none'
> >
> > The "server" and "auth-supported" QAPI parameters are arrays.  To conform
> > with the rados API, the array items are join as a single string with a ';'
> > character as a delimiter when setting the configuration values.
> >
> > Signed-off-by: Jeff Cody <address@hidden>
> > ---
> [...]
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index f152953..5f74f92 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -2666,6 +2666,28 @@
> >              '*header-digest': 'IscsiHeaderDigest',
> >              '*timeout': 'int' } }
> >  
> > +
> > +##
> > +# @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' } }
> > +
> 
> Any particular reason for wrapping the enum in a struct?  Do you
> envisage adding members to the struct?
>

I am going to admit, mainly it was my frustration with trying to deal with a
qapi array of just enums in a QDict, and structs was more straightforward.
What is the best way to parse an array of enums inside a QDict?  Do you need
to extract the subqdict via qdict_extract_subqdict() still?

> >  ##
> >  # @BlockdevOptionsRbd:
> >  #
> > @@ -2681,6 +2703,11 @@
> >  #
> >  # @user:               #optional Ceph id name.
> >  #
> > +# @server:             #optional Monitor host address and port.  This maps
> > +#                      to the "mon_host" Ceph option.
> 
> Suggest something like "Monitor addresses", for consistency with how we
> document *SocketAddress members elsewhere, and plural to hint at it
> being a list, not just one.
> 

OK, thanks.

> > +#
> > +# @auth-supported:     #optional Authentication supported.
> > +#
> >  # @password-secret:    #optional The ID of a QCryptoSecret object providing
> >  #                      the password for the login.
> >  #
> > @@ -2692,6 +2719,8 @@
> >              '*conf': 'str',
> >              '*snapshot': 'str',
> >              '*user': 'str',
> > +            '*server': ['InetSocketAddress'],
> > +            '*auth-supported': ['RbdAuthMethod'],
> >              '*password-secret': 'str' } }
> >  
> >  ##