Re: [Qemu-block] [RFC][BROKEN] rbd: Allow configuration of authentication scheme

2018-04-18 Thread Kevin Wolf
Am 18.04.2018 um 16:16 hat Eric Blake geschrieben:
> On 04/18/2018 08:50 AM, Kevin Wolf wrote:
> 
> >>> @@ -3195,6 +3215,8 @@
> >>>  '*conf': 'str',
> >>>  '*snapshot': 'str',
> >>>  '*user': 'str',
> >>> +'*auth-none': 'bool',
> >>> +'*auth-cephx': 'RbdAuthCephx',
> >>>  '*server': ['InetSocketAddressBase'] } }
> >>
> >> Would it be better to have this be a flat union with 'auth' with enum
> >> values 'none', 'cephx', 'both' as a discriminator that determines which
> >> additional fields can be present?  Or does that require that we first
> >> fix the QAPI generator to allow nesting a flat union within another flat
> >> union (probably doable, just no one has needed it before now)?  Is it
> >> also time to improve the QAPI generator to allow a default value to the
> >> discriminator field, rather than requiring the field to be present?
> > 
> > Both options can be enabled at the same time, so that the client
> > connects to a server no matter whether it does 'cephx' authentication or
> > only 'none. This is even the default for rbd driver (in the existing
> > command line interface, but I think we need to stay compatible with it).
> > With a union you would have to explicitly choose one or the other, but
> > could never accept both.
> > 
> > The other option we were considering was a list of authentication
> > options, which would be easier to implement, but isn't really an
> > accurate representation of what we really accept. There is no way we
> > could meaningfully implement something like this:
> > 
> > 'auth': [ { 'type': 'cephx', 'key-secret': 'foo' },
> >   { 'type': 'cephx', 'key-secret': 'bar' } ]
> > 
> > Because Ceph only allows us to enable the 'cephx' authentication method
> > and to set a single key for it.
> 
> How does it look as a choice between:
> 
> {'enum':'CephxAuth', 'data': ['none', 'cephx', 'both' ]}
> 
> where both 'cephx' and 'both' support the optional 'key-secret'
> parameter, but 'none' does not?

Doesn't really look extensible for the case that Ceph adds a third mode.
At least I don't think we want to have an enum and associated union
branches for all possible combinations?

Kevin


signature.asc
Description: PGP signature


Re: [Qemu-block] [RFC][BROKEN] rbd: Allow configuration of authentication scheme

2018-04-18 Thread Eric Blake
On 04/18/2018 08:50 AM, Kevin Wolf wrote:

>>> @@ -3195,6 +3215,8 @@
>>>  '*conf': 'str',
>>>  '*snapshot': 'str',
>>>  '*user': 'str',
>>> +'*auth-none': 'bool',
>>> +'*auth-cephx': 'RbdAuthCephx',
>>>  '*server': ['InetSocketAddressBase'] } }
>>
>> Would it be better to have this be a flat union with 'auth' with enum
>> values 'none', 'cephx', 'both' as a discriminator that determines which
>> additional fields can be present?  Or does that require that we first
>> fix the QAPI generator to allow nesting a flat union within another flat
>> union (probably doable, just no one has needed it before now)?  Is it
>> also time to improve the QAPI generator to allow a default value to the
>> discriminator field, rather than requiring the field to be present?
> 
> Both options can be enabled at the same time, so that the client
> connects to a server no matter whether it does 'cephx' authentication or
> only 'none. This is even the default for rbd driver (in the existing
> command line interface, but I think we need to stay compatible with it).
> With a union you would have to explicitly choose one or the other, but
> could never accept both.
> 
> The other option we were considering was a list of authentication
> options, which would be easier to implement, but isn't really an
> accurate representation of what we really accept. There is no way we
> could meaningfully implement something like this:
> 
> 'auth': [ { 'type': 'cephx', 'key-secret': 'foo' },
>   { 'type': 'cephx', 'key-secret': 'bar' } ]
> 
> Because Ceph only allows us to enable the 'cephx' authentication method
> and to set a single key for it.

How does it look as a choice between:

{'enum':'CephxAuth', 'data': ['none', 'cephx', 'both' ]}

where both 'cephx' and 'both' support the optional 'key-secret'
parameter, but 'none' does not?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-block] [RFC][BROKEN] rbd: Allow configuration of authentication scheme

2018-04-18 Thread Kevin Wolf
Am 18.04.2018 um 15:26 hat Eric Blake geschrieben:
> On 04/05/2018 12:06 PM, Kevin Wolf wrote:
> > The legacy command line syntax supports a "password-secret" option that
> > allows to pass an authentication key to Ceph. This was not supported in
> > QMP so far.
> > 
> > This patch introduces authentication options in the QAPI schema, makes
> > them do the corresponding rados_conf_set() calls and adds compatibility
> > code that translates the old "password-secret" option both for opening
> > and creating images to the new set of options.
> > 
> > Note that the old option didn't allow to explicitly specify the set of
> > allowed authentication schemes. The compatibility code assumes that if
> > "password-secret" is given, only the cephx scheme is allowed. If it's
> > missing, both none and cephx are allowed because the configuration file
> > could still provide a key.
> > 
> > Signed-off-by: Kevin Wolf 
> > ---
> 
> > Any thoughts on the proposed QAPI schema or the two implementation
> > problems are welcome.
> > 
> 
> >  qapi/block-core.json |  22 +++
> >  block/rbd.c  | 102 
> > ++-
> >  2 files changed, 99 insertions(+), 25 deletions(-)
> > 
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index c50517bff3..d5ce588add 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -3170,6 +3170,19 @@
> >  
> >  
> >  ##
> > +# @RbdAuthCephx:
> > +#
> > +# @key-secret: ID of a QCryptoSecret object providing a key for 
> > cephx
> > +#  authentication. If not specified, a key from the
> > +#  specified configuration file, or the system default
> > +#  configuration is used, if present.
> > +#
> > +# Since: 2.13
> > +##
> > +{ 'struct': 'RbdAuthCephx',
> > +  'data': { '*key-secret': 'str' } }
> > +
> > +##
> >  # @BlockdevOptionsRbd:
> >  #
> >  # @pool:   Ceph pool name.
> > @@ -3184,6 +3197,13 @@
> >  #
> >  # @user:   Ceph id name.
> >  #
> > +# @auth-none:  true if connecting to a server without 
> > authentication
> > +#  should be allowed (default: false; since 2.13)
> > +#
> > +# @auth-cephx: Configuration for cephx authentication if 
> > specified. If
> > +#  not specified, cephx authentication is not allowed.
> > +#  (since 2.13)
> > +#
> >  # @server: Monitor host address and port.  This maps
> >  #  to the "mon_host" Ceph option.
> >  #
> > @@ -3195,6 +3215,8 @@
> >  '*conf': 'str',
> >  '*snapshot': 'str',
> >  '*user': 'str',
> > +'*auth-none': 'bool',
> > +'*auth-cephx': 'RbdAuthCephx',
> >  '*server': ['InetSocketAddressBase'] } }
> 
> Would it be better to have this be a flat union with 'auth' with enum
> values 'none', 'cephx', 'both' as a discriminator that determines which
> additional fields can be present?  Or does that require that we first
> fix the QAPI generator to allow nesting a flat union within another flat
> union (probably doable, just no one has needed it before now)?  Is it
> also time to improve the QAPI generator to allow a default value to the
> discriminator field, rather than requiring the field to be present?

Both options can be enabled at the same time, so that the client
connects to a server no matter whether it does 'cephx' authentication or
only 'none. This is even the default for rbd driver (in the existing
command line interface, but I think we need to stay compatible with it).
With a union you would have to explicitly choose one or the other, but
could never accept both.

The other option we were considering was a list of authentication
options, which would be easier to implement, but isn't really an
accurate representation of what we really accept. There is no way we
could meaningfully implement something like this:

'auth': [ { 'type': 'cephx', 'key-secret': 'foo' },
  { 'type': 'cephx', 'key-secret': 'bar' } ]

Because Ceph only allows us to enable the 'cephx' authentication method
and to set a single key for it.

Kevin


signature.asc
Description: PGP signature


Re: [Qemu-block] [RFC][BROKEN] rbd: Allow configuration of authentication scheme

2018-04-18 Thread Kevin Wolf
Am 18.04.2018 um 15:21 hat Eric Blake geschrieben:
> On 04/06/2018 03:04 AM, Kevin Wolf wrote:
> > Am 05.04.2018 um 19:06 hat Kevin Wolf geschrieben:
> >> The legacy command line syntax supports a "password-secret" option that
> >> allows to pass an authentication key to Ceph. This was not supported in
> >> QMP so far.
> >>
> >> This patch introduces authentication options in the QAPI schema, makes
> >> them do the corresponding rados_conf_set() calls and adds compatibility
> >> code that translates the old "password-secret" option both for opening
> >> and creating images to the new set of options.
> >>
> >> Note that the old option didn't allow to explicitly specify the set of
> >> allowed authentication schemes. The compatibility code assumes that if
> >> "password-secret" is given, only the cephx scheme is allowed. If it's
> >> missing, both none and cephx are allowed because the configuration file
> >> could still provide a key.
> > 
> > There is another problem here that suggests that maybe this is not the
> > right QAPI schema after all: The defaults needed for command line
> > compatibility and those promised in the QAPI schema are conflicting.
> > 
> > The required command line behaviour is as described above:
> > 
> > * password-secret given: only cephx
> > * no options given: cephx, none
> > 
> > The desired QMP default behaviour is:
> > 
> > * auth-cephx given: allow cephx
> > * auth-none given: allow none
> > * both given: allow both
> > * no options given: error
> > 
> > In .bdrv_open() there is no way to distinguish the "no options given" of
> > the command line from that of QMP. The current implementation allows
> > everything if no options are given, i.e. it keeps existing command lines
> > working, but it doesn't correctly implement the behaviour described in
> > the QAPI schema.
> 
> Can we use a QAPI alternate with an explicit JSON null for the command
> line 'no options given' case?

The fundamental problem is that we can only have a single default value
for both command line and QMP. I don't think an alternate would change
anything there.

Both the commands line and the QMP 'no options given' cases would end up
being represented by a missing key in the options QDict. null would only
be used if explicitly given in blockdev-add (I don't think you can
specify null on the command line at all).

> > 
> > I don't think changing the description of the QAPI schema would be a
> > good idea, it would be a rather surprising interface.
> > 
> >> Signed-off-by: Kevin Wolf 
> >> ---
> >>
> >> This doesn't actually work correctly yet because the way that options
> >> are passed through the block layer (QAPI -> QemuOpts -> QDict). Before
> >> we fix or hack around this, let's make sure this is the schema that we
> >> want.
> 
> Can we skip the intermediate QemuOpts?  If we can go straight from
> command line to QDict using just QAPI, won't that give us what we really
> need?

In fact, I think that's what -blockdev already does. The one that I had
in mind is -drive, which adds the additional QemuOpts step, but we don't
have to support -drive with a new syntax.

However, the problem is with the representation in the QDict, so
skipping QemuOpts doesn't help.

> >>
> >> The two known problems are:
> >>
> >> 1. QDict *options in qemu_rbd_open() may contain options either in their
> >>proper QObject types (if passed from blockdev-add) or as strings
> >>(-drive). Both forms can be mixed in the same options QDict.
> >>
> >>rbd uses the keyval visitor to convert the options into a QAPI
> >>object. This means that it requires all options to be strings. This
> >>patch, however, introduces a bool property, so the visitor breaks
> >>when it gets its input from blockdev-add.
> >>
> >>Options to hack around the problem:
> >>
> >>a. Do an extra conversion step or two and go through QemuOpts like
> >>   some other block drivers. When I offered something like this to
> >>   Markus a while ago in a similar case, he rejected the idea.
> >>
> >>b. Introduce a qdict_stringify_entries() that just does what its name
> >>   says. It would be called before the running keyval visitor so that
> >>   only strings will be present in the QDict.
> >>
> >>c. Do a local one-off hack that checks if the entry with the key
> >>   "auth-none" is a QBool, and if so, overwrite it with a string. The
> >>   problem will reappear with the next non-string option.
> >>
> >>(d. Get rid of the QDict detour and work only with QAPI objects
> >>everywhere. Support rbd authentication only in QEMU 4.0.)
> 
> Oh, even one step further than just avoiding QemuOpts, but using REAL
> types everywhere in the block layer!  It might be a nice cleanup, but it
> would probably take a lot of effort in the meantime to get to that point?

Yes, I would like to get there, but it's definitely a long term goal
(that's why I wrote "QEMU 4.0"). It definitely means getting rid of any
options that

Re: [Qemu-block] [RFC][BROKEN] rbd: Allow configuration of authentication scheme

2018-04-18 Thread Eric Blake
On 04/05/2018 12:06 PM, Kevin Wolf wrote:
> The legacy command line syntax supports a "password-secret" option that
> allows to pass an authentication key to Ceph. This was not supported in
> QMP so far.
> 
> This patch introduces authentication options in the QAPI schema, makes
> them do the corresponding rados_conf_set() calls and adds compatibility
> code that translates the old "password-secret" option both for opening
> and creating images to the new set of options.
> 
> Note that the old option didn't allow to explicitly specify the set of
> allowed authentication schemes. The compatibility code assumes that if
> "password-secret" is given, only the cephx scheme is allowed. If it's
> missing, both none and cephx are allowed because the configuration file
> could still provide a key.
> 
> Signed-off-by: Kevin Wolf 
> ---

> Any thoughts on the proposed QAPI schema or the two implementation
> problems are welcome.
> 

>  qapi/block-core.json |  22 +++
>  block/rbd.c  | 102 
> ++-
>  2 files changed, 99 insertions(+), 25 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index c50517bff3..d5ce588add 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -3170,6 +3170,19 @@
>  
>  
>  ##
> +# @RbdAuthCephx:
> +#
> +# @key-secret: ID of a QCryptoSecret object providing a key for cephx
> +#  authentication. If not specified, a key from the
> +#  specified configuration file, or the system default
> +#  configuration is used, if present.
> +#
> +# Since: 2.13
> +##
> +{ 'struct': 'RbdAuthCephx',
> +  'data': { '*key-secret': 'str' } }
> +
> +##
>  # @BlockdevOptionsRbd:
>  #
>  # @pool:   Ceph pool name.
> @@ -3184,6 +3197,13 @@
>  #
>  # @user:   Ceph id name.
>  #
> +# @auth-none:  true if connecting to a server without authentication
> +#  should be allowed (default: false; since 2.13)
> +#
> +# @auth-cephx: Configuration for cephx authentication if specified. 
> If
> +#  not specified, cephx authentication is not allowed.
> +#  (since 2.13)
> +#
>  # @server: Monitor host address and port.  This maps
>  #  to the "mon_host" Ceph option.
>  #
> @@ -3195,6 +3215,8 @@
>  '*conf': 'str',
>  '*snapshot': 'str',
>  '*user': 'str',
> +'*auth-none': 'bool',
> +'*auth-cephx': 'RbdAuthCephx',
>  '*server': ['InetSocketAddressBase'] } }

Would it be better to have this be a flat union with 'auth' with enum
values 'none', 'cephx', 'both' as a discriminator that determines which
additional fields can be present?  Or does that require that we first
fix the QAPI generator to allow nesting a flat union within another flat
union (probably doable, just no one has needed it before now)?  Is it
also time to improve the QAPI generator to allow a default value to the
discriminator field, rather than requiring the field to be present?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-block] [RFC][BROKEN] rbd: Allow configuration of authentication scheme

2018-04-18 Thread Eric Blake
On 04/06/2018 03:04 AM, Kevin Wolf wrote:
> Am 05.04.2018 um 19:06 hat Kevin Wolf geschrieben:
>> The legacy command line syntax supports a "password-secret" option that
>> allows to pass an authentication key to Ceph. This was not supported in
>> QMP so far.
>>
>> This patch introduces authentication options in the QAPI schema, makes
>> them do the corresponding rados_conf_set() calls and adds compatibility
>> code that translates the old "password-secret" option both for opening
>> and creating images to the new set of options.
>>
>> Note that the old option didn't allow to explicitly specify the set of
>> allowed authentication schemes. The compatibility code assumes that if
>> "password-secret" is given, only the cephx scheme is allowed. If it's
>> missing, both none and cephx are allowed because the configuration file
>> could still provide a key.
> 
> There is another problem here that suggests that maybe this is not the
> right QAPI schema after all: The defaults needed for command line
> compatibility and those promised in the QAPI schema are conflicting.
> 
> The required command line behaviour is as described above:
> 
> * password-secret given: only cephx
> * no options given: cephx, none
> 
> The desired QMP default behaviour is:
> 
> * auth-cephx given: allow cephx
> * auth-none given: allow none
> * both given: allow both
> * no options given: error
> 
> In .bdrv_open() there is no way to distinguish the "no options given" of
> the command line from that of QMP. The current implementation allows
> everything if no options are given, i.e. it keeps existing command lines
> working, but it doesn't correctly implement the behaviour described in
> the QAPI schema.

Can we use a QAPI alternate with an explicit JSON null for the command
line 'no options given' case?

> 
> I don't think changing the description of the QAPI schema would be a
> good idea, it would be a rather surprising interface.
> 
>> Signed-off-by: Kevin Wolf 
>> ---
>>
>> This doesn't actually work correctly yet because the way that options
>> are passed through the block layer (QAPI -> QemuOpts -> QDict). Before
>> we fix or hack around this, let's make sure this is the schema that we
>> want.

Can we skip the intermediate QemuOpts?  If we can go straight from
command line to QDict using just QAPI, won't that give us what we really
need?

>>
>> The two known problems are:
>>
>> 1. QDict *options in qemu_rbd_open() may contain options either in their
>>proper QObject types (if passed from blockdev-add) or as strings
>>(-drive). Both forms can be mixed in the same options QDict.
>>
>>rbd uses the keyval visitor to convert the options into a QAPI
>>object. This means that it requires all options to be strings. This
>>patch, however, introduces a bool property, so the visitor breaks
>>when it gets its input from blockdev-add.
>>
>>Options to hack around the problem:
>>
>>a. Do an extra conversion step or two and go through QemuOpts like
>>   some other block drivers. When I offered something like this to
>>   Markus a while ago in a similar case, he rejected the idea.
>>
>>b. Introduce a qdict_stringify_entries() that just does what its name
>>   says. It would be called before the running keyval visitor so that
>>   only strings will be present in the QDict.
>>
>>c. Do a local one-off hack that checks if the entry with the key
>>   "auth-none" is a QBool, and if so, overwrite it with a string. The
>>   problem will reappear with the next non-string option.
>>
>>(d. Get rid of the QDict detour and work only with QAPI objects
>>everywhere. Support rbd authentication only in QEMU 4.0.)

Oh, even one step further than just avoiding QemuOpts, but using REAL
types everywhere in the block layer!  It might be a nice cleanup, but it
would probably take a lot of effort in the meantime to get to that point?

>>
>> 2. The proposed schema allows 'auth-cephx': {} as a valid option with
>>the meaning that the cephx authentication scheme is enabled, but no
>>key is given (e.g. it is taken from the config file).
>>
>>However, an empty dict cannot currently be represented by flattened
>>QDicts. We need to find a way to enable this. I think this will be
>>externally visible because it directly translates into the dotted
>>syntax of -blockdev, so we may want to be careful.

Can we just require users to give -blockdev with the JSON format (rather
than dotted format) when they need to use that particular feature on the
command line?

>>
>> Any thoughts on the proposed QAPI schema or the two implementation
>> problems are welcome.
> 
> Kevin
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-block] [RFC][BROKEN] rbd: Allow configuration of authentication scheme

2018-04-18 Thread Kevin Wolf
Am 06.04.2018 um 10:04 hat Kevin Wolf geschrieben:
> Am 05.04.2018 um 19:06 hat Kevin Wolf geschrieben:
> > The legacy command line syntax supports a "password-secret" option that
> > allows to pass an authentication key to Ceph. This was not supported in
> > QMP so far.
> > 
> > This patch introduces authentication options in the QAPI schema, makes
> > them do the corresponding rados_conf_set() calls and adds compatibility
> > code that translates the old "password-secret" option both for opening
> > and creating images to the new set of options.
> > 
> > Note that the old option didn't allow to explicitly specify the set of
> > allowed authentication schemes. The compatibility code assumes that if
> > "password-secret" is given, only the cephx scheme is allowed. If it's
> > missing, both none and cephx are allowed because the configuration file
> > could still provide a key.
> 
> There is another problem here that suggests that maybe this is not the
> right QAPI schema after all: The defaults needed for command line
> compatibility and those promised in the QAPI schema are conflicting.
> 
> The required command line behaviour is as described above:
> 
> * password-secret given: only cephx
> * no options given: cephx, none
> 
> The desired QMP default behaviour is:
> 
> * auth-cephx given: allow cephx
> * auth-none given: allow none
> * both given: allow both
> * no options given: error
> 
> In .bdrv_open() there is no way to distinguish the "no options given" of
> the command line from that of QMP. The current implementation allows
> everything if no options are given, i.e. it keeps existing command lines
> working, but it doesn't correctly implement the behaviour described in
> the QAPI schema.
> 
> I don't think changing the description of the QAPI schema would be a
> good idea, it would be a rather surprising interface.
> 
> > Signed-off-by: Kevin Wolf 
> > ---
> > 
> > This doesn't actually work correctly yet because the way that options
> > are passed through the block layer (QAPI -> QemuOpts -> QDict). Before
> > we fix or hack around this, let's make sure this is the schema that we
> > want.
> > 
> > The two known problems are:
> > 
> > 1. QDict *options in qemu_rbd_open() may contain options either in their
> >proper QObject types (if passed from blockdev-add) or as strings
> >(-drive). Both forms can be mixed in the same options QDict.
> > 
> >rbd uses the keyval visitor to convert the options into a QAPI
> >object. This means that it requires all options to be strings. This
> >patch, however, introduces a bool property, so the visitor breaks
> >when it gets its input from blockdev-add.
> > 
> >Options to hack around the problem:
> > 
> >a. Do an extra conversion step or two and go through QemuOpts like
> >   some other block drivers. When I offered something like this to
> >   Markus a while ago in a similar case, he rejected the idea.
> > 
> >b. Introduce a qdict_stringify_entries() that just does what its name
> >   says. It would be called before the running keyval visitor so that
> >   only strings will be present in the QDict.
> > 
> >c. Do a local one-off hack that checks if the entry with the key
> >   "auth-none" is a QBool, and if so, overwrite it with a string. The
> >   problem will reappear with the next non-string option.
> > 
> >(d. Get rid of the QDict detour and work only with QAPI objects
> >everywhere. Support rbd authentication only in QEMU 4.0.)
> > 
> > 2. The proposed schema allows 'auth-cephx': {} as a valid option with
> >the meaning that the cephx authentication scheme is enabled, but no
> >key is given (e.g. it is taken from the config file).
> > 
> >However, an empty dict cannot currently be represented by flattened
> >QDicts. We need to find a way to enable this. I think this will be
> >externally visible because it directly translates into the dotted
> >syntax of -blockdev, so we may want to be careful.
> > 
> > Any thoughts on the proposed QAPI schema or the two implementation
> > problems are welcome.

Ping?

If nobody has an opinion, we might as well just revert the revert and
bring the legacy interface 1:1 to QMP.

Kevin



Re: [Qemu-block] [RFC][BROKEN] rbd: Allow configuration of authentication scheme

2018-04-06 Thread Kevin Wolf
Am 05.04.2018 um 19:06 hat Kevin Wolf geschrieben:
> The legacy command line syntax supports a "password-secret" option that
> allows to pass an authentication key to Ceph. This was not supported in
> QMP so far.
> 
> This patch introduces authentication options in the QAPI schema, makes
> them do the corresponding rados_conf_set() calls and adds compatibility
> code that translates the old "password-secret" option both for opening
> and creating images to the new set of options.
> 
> Note that the old option didn't allow to explicitly specify the set of
> allowed authentication schemes. The compatibility code assumes that if
> "password-secret" is given, only the cephx scheme is allowed. If it's
> missing, both none and cephx are allowed because the configuration file
> could still provide a key.

There is another problem here that suggests that maybe this is not the
right QAPI schema after all: The defaults needed for command line
compatibility and those promised in the QAPI schema are conflicting.

The required command line behaviour is as described above:

* password-secret given: only cephx
* no options given: cephx, none

The desired QMP default behaviour is:

* auth-cephx given: allow cephx
* auth-none given: allow none
* both given: allow both
* no options given: error

In .bdrv_open() there is no way to distinguish the "no options given" of
the command line from that of QMP. The current implementation allows
everything if no options are given, i.e. it keeps existing command lines
working, but it doesn't correctly implement the behaviour described in
the QAPI schema.

I don't think changing the description of the QAPI schema would be a
good idea, it would be a rather surprising interface.

> Signed-off-by: Kevin Wolf 
> ---
> 
> This doesn't actually work correctly yet because the way that options
> are passed through the block layer (QAPI -> QemuOpts -> QDict). Before
> we fix or hack around this, let's make sure this is the schema that we
> want.
> 
> The two known problems are:
> 
> 1. QDict *options in qemu_rbd_open() may contain options either in their
>proper QObject types (if passed from blockdev-add) or as strings
>(-drive). Both forms can be mixed in the same options QDict.
> 
>rbd uses the keyval visitor to convert the options into a QAPI
>object. This means that it requires all options to be strings. This
>patch, however, introduces a bool property, so the visitor breaks
>when it gets its input from blockdev-add.
> 
>Options to hack around the problem:
> 
>a. Do an extra conversion step or two and go through QemuOpts like
>   some other block drivers. When I offered something like this to
>   Markus a while ago in a similar case, he rejected the idea.
> 
>b. Introduce a qdict_stringify_entries() that just does what its name
>   says. It would be called before the running keyval visitor so that
>   only strings will be present in the QDict.
> 
>c. Do a local one-off hack that checks if the entry with the key
>   "auth-none" is a QBool, and if so, overwrite it with a string. The
>   problem will reappear with the next non-string option.
> 
>(d. Get rid of the QDict detour and work only with QAPI objects
>everywhere. Support rbd authentication only in QEMU 4.0.)
> 
> 2. The proposed schema allows 'auth-cephx': {} as a valid option with
>the meaning that the cephx authentication scheme is enabled, but no
>key is given (e.g. it is taken from the config file).
> 
>However, an empty dict cannot currently be represented by flattened
>QDicts. We need to find a way to enable this. I think this will be
>externally visible because it directly translates into the dotted
>syntax of -blockdev, so we may want to be careful.
> 
> Any thoughts on the proposed QAPI schema or the two implementation
> problems are welcome.

Kevin