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

[Markus Armbruster <armbru@redhat.com>]

Jeff Cody <jcody@redhat.com> writes:

> On Tue, Feb 28, 2017 at 03:34:10PM +0100, Markus Armbruster wrote:
>> Starting with just the QAPI schema.
>> 
>> Jeff Cody <jcody@redhat.com> 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 <jcody@redhat.com>
>> > ---
>> [...]
>> > 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?

I admit I'm not so familiar with the block layer's QObject mangling, and
the helper functions we've grown to support that.

I *am* familiar with QAPI visitors (I better be, I maintain the
subsystem), and they are my preferred tool to go from QObject to C data
structures and back.

The "to C" direction uses a QObject input visitor.  Roughly like this:

    Error *err = NULL;
    Visitor *v;
    QapiType *qapi;

    v = qobject_input_visitor_new(qobj);
    visit_type_QapiType(v, NULL, &qapi, &err);
    visit_free(v);

You now either got an Error object in @err, or a QapiType object in
@qapi.

To free the QapiType:

    qapi_free_QapiType(qapi);

The QapiType for ['RbdAuthSupport'] (i.e. list of enum RbdAuthSupport) is
called RbdAuthSupportList, and should look like this:

    struct RbdAuthSupportList {
        RbdAuthSupportList *next;
        RbdAuthSupport value;
    };

The next thing I have to admit is that I fail to see where you're
dealing "with a qapi array of just enums in a QDict".  Can you help?

>> >  ##
>> >  # @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' } }
>> >  
>> >  ##