| Message ID | 1490266548-22500-5-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 <armbru@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 <eblake@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.
Am 23.03.2017 um 11:55 hat Markus Armbruster geschrieben: > 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. Delete the wrapper, and > rename the enum. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
On 03/23/2017 01:10 PM, Eric Blake wrote: >> # @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" } ], Another question - why does auth-supported take an array? Can you really pass both 'none' and 'cephx' at the same time? Or can you only pass an array of at most one element, at which point why is it an array?
On 03/23/2017 03:59 PM, Eric Blake wrote: > On 03/23/2017 01:10 PM, Eric Blake wrote: > >>> # @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" } ], > > Another question - why does auth-supported take an array? Can you > really pass both 'none' and 'cephx' at the same time? Or can you only > pass an array of at most one element, at which point why is it an array? In fact, the more I look at this, the more I wonder if we really want 'auth' to be a flat union and move the 'password-secret' key to be part of the cephx authorization scheme, since 'password-secret' only makes sense with 'cephx', and not with 'none'. Jeff pointed out to me that libvirt is currently passing both at once; qemuBuildRBDSecinfoURI(): static int qemuBuildRBDSecinfoURI(virBufferPtr buf, qemuDomainSecretInfoPtr secinfo) { char *base64secret = NULL; if (!secinfo) { virBufferAddLit(buf, ":auth_supported=none"); return 0; } switch ((qemuDomainSecretInfoType) secinfo->type) { case VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN: if (!(base64secret = virStringEncodeBase64(secinfo->s.plain.secret, secinfo->s.plain.secretlen))) return -1; virBufferEscape(buf, '\\', ":", ":id=%s", secinfo->s.plain.username); virBufferEscape(buf, '\\', ":", ":key=%s:auth_supported=cephx\\;none", base64secret); But maybe what that _really_ means is that librados is letting us say "I'd really prefer cephx authorization, and here's my key; but if you can't honor that, I'm also okay with a fallback to no auth". That feels wrong to me. If you've gone to the bother of providing an encryption key, falling back to none should probably be an error. So my proposal is to have: { 'enum': 'RbdAuthSupport', 'data': [ 'cephx', 'none' ] } { 'struct': 'RbdAuthNone', 'data': { } } { 'struct': 'RbdAuthCephx', 'data': { 'password-secret': 'str' } } { 'union', 'RbdAuthMethod', 'base': { 'type': 'RbdAuthSupport' }, 'discriminator': 'type', 'data': { 'none': 'RbdAuthNone', 'cephx': 'RbdAuthCephx' } } } { 'struct': 'BlockdevOptionsRbd', 'data': { 'pool': 'str', 'image': 'str', '*conf': 'str', '*snapshot': 'str', '*user': 'str', '*server': ['InetSocketAddress'], '*auth': 'RbdAuthMethod' } } so that you pass at most one auth method, looking like: ..., "image": ..., "auth": { "type": "none" } or like: ..., "image": ..., "auth": { "type": "cephx", "password-secret": "..." } note that password-secret is NOT at the same level as image, so from the command line, supporting either old-style insecure 'key=' or new-style 'password-secret' at the top-level of the QemuOpts will have to have glue code that maps it to the right nested location within the QAPI type. If we like it, we'd have to get it implemented before 2.9 bakes in any other design. We'd still have to allow libvirt's use of ":key=...:auth_supported=cephx\\;none" on the command line, but from the QMP side, we would support exactly one auth method, and libvirt will be updated to match when it starts targetting blockdev-add instead of legacy command line. If librados really needs 'cephx;none' any time cephx authorization is requested, then even though the QMP only requests 'cephx', we can still map it to 'cephx;none' in qemu - but I'm hoping that setting auth_supported=cephx rather than auth_supported=cephx;none on the librados side gives us what we (and libvirt) really want in the first place. Jeff or Josh, if you could chime in, that would be helpful.
On 03/23/2017 04:43 PM, Eric Blake wrote: > We'd still have to allow libvirt's use of > ":key=...:auth_supported=cephx\\;none" on the command line, but from the > QMP side, we would support exactly one auth method, and libvirt will be > updated to match when it starts targetting blockdev-add instead of > legacy command line. > > If librados really needs 'cephx;none' any time cephx authorization is > requested, then even though the QMP only requests 'cephx', we can still > map it to 'cephx;none' in qemu - but I'm hoping that setting > auth_supported=cephx rather than auth_supported=cephx;none on the > librados side gives us what we (and libvirt) really want in the first place. Or, if it becomes something that libvirt wants to allow users to tune in their XML (right now, users don't get a choice, they get either 'none' or 'cephx;none'), a forward-compatible solution under my QMP proposal would be to have qemu add a third enum state, "none", "cephx", and "cephx-no-fallback". On the other hand, if supporting multiple methods at once makes sense (say librados adds a 'cephy' method, and that users could legitimately use both 'cephx;cephy' and 'cephy;cephx' with different behaviors), then keeping auth as an array of instances of a simple flat union scales nicer than having to add a combinatorial explosion of branches to the flat union to cover every useful combination of auth_supported methods. Maybe I'm overthinking it. > > Jeff or Josh, if you could chime in, that would be helpful. >
On Thu, Mar 23, 2017 at 04:56:30PM -0500, Eric Blake wrote: > On 03/23/2017 04:43 PM, Eric Blake wrote: > > > We'd still have to allow libvirt's use of > > ":key=...:auth_supported=cephx\\;none" on the command line, but from the > > QMP side, we would support exactly one auth method, and libvirt will be > > updated to match when it starts targetting blockdev-add instead of > > legacy command line. > > > > If librados really needs 'cephx;none' any time cephx authorization is > > requested, then even though the QMP only requests 'cephx', we can still > > map it to 'cephx;none' in qemu - but I'm hoping that setting > > auth_supported=cephx rather than auth_supported=cephx;none on the > > librados side gives us what we (and libvirt) really want in the first place. > > Or, if it becomes something that libvirt wants to allow users to tune in > their XML (right now, users don't get a choice, they get either 'none' > or 'cephx;none'), a forward-compatible solution under my QMP proposal > would be to have qemu add a third enum state, "none", "cephx", and > "cephx-no-fallback". > > On the other hand, if supporting multiple methods at once makes sense > (say librados adds a 'cephy' method, and that users could legitimately > use both 'cephx;cephy' and 'cephy;cephx' with different behaviors), then > keeping auth as an array of instances of a simple flat union scales > nicer than having to add a combinatorial explosion of branches to the > flat union to cover every useful combination of auth_supported methods. > Maybe I'm overthinking it. > I spoke over email with Jason Dillaman (cc'ed), and this is what he told me with regards to the authentication methods, and what cephx;none means: On Thu, Mar 23, 2017 at 06:04:30PM -0400, Jason Dillaman wrote: > It's saying that the client is willing to connect to a server that > uses cephx auth or a server that uses no auth. Looking at the code, > the "auth_supported" configuration key is actually deprecated and > "auth_client_required" should be used instead (which defaults to > 'cephx, none' already). Since it's been deprecated since v0.55 and if > you are cleaning things up, feel free to switch to the new one if > possible. So from what Jason is saying, it seems like the conclusion we reached over IRC is correct: it will attempt cephx but also fallback to no auth. Also, since the preferred auth option may be named different depending on ceph versions, I know think we probably should not tie the QAPI parameter to the name of the older deprecated option. I suggest that the 'auth_supported' parameter for QAPI be renamed to 'auth_method'. If you don't like the array and the flexibility it provides at the cost of complexity, I think a flat enum consisting of 3 values like you mentioned is reasonable. Since the QAPI does not need to map to the legacy commandline used by libvirt, I would suggest maybe naming them slightly different, though: any, none, strict For 2.9, they could each map to 'auth_supported' like so: any: "cephx;none" none: "none" strict: "cephx" For 2.10, we could try to discover if 'auth_client_required' is supported or not, and use either it or 'auth_supported' as appropriate (with the same mappings as above). The reason I like "any" and "strict", is it gives consistent meanings to options even if new auth methods are introduced. For a hypothetical "cephy" method example: any: "cephy;cephx;none" none: "none" strict: "cephy;cephx" -Jeff
Adding Daniel Berrange for QCryptoSecret expertise. Jeff Cody <jcody@redhat.com> writes: > On Thu, Mar 23, 2017 at 04:56:30PM -0500, Eric Blake wrote: >> On 03/23/2017 04:43 PM, Eric Blake wrote: >> >> > We'd still have to allow libvirt's use of >> > ":key=...:auth_supported=cephx\\;none" on the command line, but from the >> > QMP side, we would support exactly one auth method, and libvirt will be >> > updated to match when it starts targetting blockdev-add instead of >> > legacy command line. >> > >> > If librados really needs 'cephx;none' any time cephx authorization is >> > requested, then even though the QMP only requests 'cephx', we can still >> > map it to 'cephx;none' in qemu - but I'm hoping that setting >> > auth_supported=cephx rather than auth_supported=cephx;none on the >> > librados side gives us what we (and libvirt) really want in the first place. >> >> Or, if it becomes something that libvirt wants to allow users to tune in >> their XML (right now, users don't get a choice, they get either 'none' >> or 'cephx;none'), a forward-compatible solution under my QMP proposal >> would be to have qemu add a third enum state, "none", "cephx", and >> "cephx-no-fallback". >> >> On the other hand, if supporting multiple methods at once makes sense >> (say librados adds a 'cephy' method, and that users could legitimately >> use both 'cephx;cephy' and 'cephy;cephx' with different behaviors), then >> keeping auth as an array of instances of a simple flat union scales >> nicer than having to add a combinatorial explosion of branches to the >> flat union to cover every useful combination of auth_supported methods. >> Maybe I'm overthinking it. >> > > I spoke over email with Jason Dillaman (cc'ed), and this is what he told me > with regards to the authentication methods, and what cephx;none means: > > On Thu, Mar 23, 2017 at 06:04:30PM -0400, Jason Dillaman wrote: >> It's saying that the client is willing to connect to a server that >> uses cephx auth or a server that uses no auth. Looking at the code, >> the "auth_supported" configuration key is actually deprecated and >> "auth_client_required" should be used instead (which defaults to >> 'cephx, none' already). Since it's been deprecated since v0.55 and if >> you are cleaning things up, feel free to switch to the new one if >> possible. > > So from what Jason is saying, it seems like the conclusion we reached over > IRC is correct: it will attempt cephx but also fallback to no auth. So the client offers the server a list of authentication methods with credentials, and the server picks one it likes. Makes sense to me. Inluding "none" in the default less so, but that's of no concern to the QMP interface, so let's ignore it for now. > Also, since the preferred auth option may be named different depending on > ceph versions, I know think we probably should not tie the QAPI parameter to > the name of the older deprecated option. Yes. > I suggest that the 'auth_supported' parameter for QAPI be renamed to > 'auth_method'. If you don't like the array and the flexibility it provides > at the cost of complexity, I think a flat enum consisting of 3 values like > you mentioned is reasonable. Since the QAPI does not need to map to the > legacy commandline used by libvirt, I would suggest maybe naming them > slightly different, though: any, none, strict > > For 2.9, they could each map to 'auth_supported' like so: > > any: "cephx;none" > none: "none" > strict: "cephx" > > For 2.10, we could try to discover if 'auth_client_required' is supported or > not, and use either it or 'auth_supported' as appropriate (with the same > mappings as above). > > The reason I like "any" and "strict", is it gives consistent meanings to > options even if new auth methods are introduced. For a hypothetical "cephy" > method example: > > any: "cephy;cephx;none" > none: "none" > strict: "cephy;cephx" Two years later, an unfixable cryptograhic weakness in cephx is discovered. Some users really, really want to select only "cephy", but they can't. We react by pushing out a QEMU update adding method "stricter", which expands into "cephy". Libvirt then reacts and pushes out an update. And so forth, up the stack. Many moons later, users can actually select "cephy". Thanks, but no thanks. I think we should expose the current, recommended way to configure authentication, in a form that is suitable for QAPI/QMP, i.e. structured data as JSON objects, not strings you need to parse. If a future authentication method might need something else than a QCryptoSecret object, we need to take Eric's proposal and make this an array of unions, where the union tag is the method, and the variant members supply whatever data the method needs (none: nothing, cephx: a QCryptoSecret). Member @password-secret goes away. Else, we can make it an array of methods, and keep member @password-secret. We need to decide quickly to keep rbd fully supported in 2.9.
On Fri, Mar 24, 2017 at 08:05:49AM +0100, Markus Armbruster wrote: > Adding Daniel Berrange for QCryptoSecret expertise. > > Jeff Cody <jcody@redhat.com> writes: > > > On Thu, Mar 23, 2017 at 04:56:30PM -0500, Eric Blake wrote: > >> On 03/23/2017 04:43 PM, Eric Blake wrote: > >> > >> > We'd still have to allow libvirt's use of > >> > ":key=...:auth_supported=cephx\\;none" on the command line, but from the > >> > QMP side, we would support exactly one auth method, and libvirt will be > >> > updated to match when it starts targetting blockdev-add instead of > >> > legacy command line. > >> > > >> > If librados really needs 'cephx;none' any time cephx authorization is > >> > requested, then even though the QMP only requests 'cephx', we can still > >> > map it to 'cephx;none' in qemu - but I'm hoping that setting > >> > auth_supported=cephx rather than auth_supported=cephx;none on the > >> > librados side gives us what we (and libvirt) really want in the first place. > >> > >> Or, if it becomes something that libvirt wants to allow users to tune in > >> their XML (right now, users don't get a choice, they get either 'none' > >> or 'cephx;none'), a forward-compatible solution under my QMP proposal > >> would be to have qemu add a third enum state, "none", "cephx", and > >> "cephx-no-fallback". > >> > >> On the other hand, if supporting multiple methods at once makes sense > >> (say librados adds a 'cephy' method, and that users could legitimately > >> use both 'cephx;cephy' and 'cephy;cephx' with different behaviors), then > >> keeping auth as an array of instances of a simple flat union scales > >> nicer than having to add a combinatorial explosion of branches to the > >> flat union to cover every useful combination of auth_supported methods. > >> Maybe I'm overthinking it. > >> > > > > I spoke over email with Jason Dillaman (cc'ed), and this is what he told me > > with regards to the authentication methods, and what cephx;none means: > > > > On Thu, Mar 23, 2017 at 06:04:30PM -0400, Jason Dillaman wrote: > >> It's saying that the client is willing to connect to a server that > >> uses cephx auth or a server that uses no auth. Looking at the code, > >> the "auth_supported" configuration key is actually deprecated and > >> "auth_client_required" should be used instead (which defaults to > >> 'cephx, none' already). Since it's been deprecated since v0.55 and if > >> you are cleaning things up, feel free to switch to the new one if > >> possible. > > > > So from what Jason is saying, it seems like the conclusion we reached over > > IRC is correct: it will attempt cephx but also fallback to no auth. > > So the client offers the server a list of authentication methods with > credentials, and the server picks one it likes. Makes sense to me. > > Inluding "none" in the default less so, but that's of no concern to the > QMP interface, so let's ignore it for now. > > > Also, since the preferred auth option may be named different depending on > > ceph versions, I know think we probably should not tie the QAPI parameter to > > the name of the older deprecated option. > > Yes. > > > I suggest that the 'auth_supported' parameter for QAPI be renamed to > > 'auth_method'. If you don't like the array and the flexibility it provides > > at the cost of complexity, I think a flat enum consisting of 3 values like > > you mentioned is reasonable. Since the QAPI does not need to map to the > > legacy commandline used by libvirt, I would suggest maybe naming them > > slightly different, though: any, none, strict > > > > For 2.9, they could each map to 'auth_supported' like so: > > > > any: "cephx;none" > > none: "none" > > strict: "cephx" > > > > For 2.10, we could try to discover if 'auth_client_required' is supported or > > not, and use either it or 'auth_supported' as appropriate (with the same > > mappings as above). > > > > The reason I like "any" and "strict", is it gives consistent meanings to > > options even if new auth methods are introduced. For a hypothetical "cephy" > > method example: > > > > any: "cephy;cephx;none" > > none: "none" > > strict: "cephy;cephx" > > Two years later, an unfixable cryptograhic weakness in cephx is > discovered. Some users really, really want to select only "cephy", but > they can't. We react by pushing out a QEMU update adding method > "stricter", which expands into "cephy". Libvirt then reacts and pushes > out an update. And so forth, up the stack. Many moons later, users can > actually select "cephy". Thanks, but no thanks. > Good point; it could be dealt with, but only clumsily. > I think we should expose the current, recommended way to configure > authentication, in a form that is suitable for QAPI/QMP, i.e. structured > data as JSON objects, not strings you need to parse. > > If a future authentication method might need something else than a > QCryptoSecret object, we need to take Eric's proposal and make this an > array of unions, where the union tag is the method, and the variant > members supply whatever data the method needs (none: nothing, cephx: a > QCryptoSecret). Member @password-secret goes away. > I guess it is always technically a possibility that a new method would need something different. Different methods may of course change the assumptions about the requirement of a 'user' or not, etc.., as well. > Else, we can make it an array of methods, and keep member > @password-secret. > Do you mean keep it as it currently is (just renamed)? One new bit we should add in, if we stay with this method, is to throw an error if user/password-secret is provided but only "none" is selected as the auth method. The current method essentially pushes the decision of the correct combination of methods to use to libvirt, which feels like the right place to be making that decision anyway. > We need to decide quickly to keep rbd fully supported in 2.9. Agree. My preference is to leave it as an array of methods, so long as that is tenable to libvirt. -Jeff
On 03/24/2017 07:42 AM, Jeff Cody wrote: > Agree. My preference is to leave it as an array of methods, so long as that > is tenable to libvirt. The -drive syntax should remain unchanged (that's an absolute must for libvirt). But the QMP syntax being an array of methods sounds best to me, and I think password-secret should be part of the array. So my vote would be for: { "driver": "rbd", "image": "foo", "auth": [ { "type": "cephx", "password-secret": "sec0" }, { "type": "none" } ], "pool": "bar" } It makes mapping -drive arguments into QMP form a bit trickier, but the QMP form is then easily extensible if we add another auth method down the road.
On Fri, Mar 24, 2017 at 08:49:20AM -0500, Eric Blake wrote: > On 03/24/2017 07:42 AM, Jeff Cody wrote: > > > Agree. My preference is to leave it as an array of methods, so long as that > > is tenable to libvirt. > > The -drive syntax should remain unchanged (that's an absolute must for > libvirt). But the QMP syntax being an array of methods sounds best to > me, and I think password-secret should be part of the array. So my vote > would be for: > > { "driver": "rbd", "image": "foo", > "auth": [ { "type": "cephx", "password-secret": "sec0" }, > { "type": "none" } ], > "pool": "bar" } > > It makes mapping -drive arguments into QMP form a bit trickier, but the > QMP form is then easily extensible if we add another auth method down > the road. > In that case, what about adding user as well, and not just password-secret? Jeff
On 03/24/2017 09:10 AM, Jeff Cody wrote: > On Fri, Mar 24, 2017 at 08:49:20AM -0500, Eric Blake wrote: >> On 03/24/2017 07:42 AM, Jeff Cody wrote: >> >>> Agree. My preference is to leave it as an array of methods, so long as that >>> is tenable to libvirt. >> >> The -drive syntax should remain unchanged (that's an absolute must for >> libvirt). But the QMP syntax being an array of methods sounds best to >> me, and I think password-secret should be part of the array. So my vote >> would be for: >> >> { "driver": "rbd", "image": "foo", >> "auth": [ { "type": "cephx", "password-secret": "sec0" }, >> { "type": "none" } ], >> "pool": "bar" } >> >> It makes mapping -drive arguments into QMP form a bit trickier, but the >> QMP form is then easily extensible if we add another auth method down >> the road. >> > > In that case, what about adding user as well, and not just password-secret? Makes sense - anything that is associated solely with cephx should belong to the same flat-union branch for cephx, rather than at the top level.
Am 24.03.2017 um 14:49 hat Eric Blake geschrieben: > On 03/24/2017 07:42 AM, Jeff Cody wrote: > > > Agree. My preference is to leave it as an array of methods, so long as that > > is tenable to libvirt. > > The -drive syntax should remain unchanged (that's an absolute must for > libvirt). But the QMP syntax being an array of methods sounds best to > me, and I think password-secret should be part of the array. So my vote > would be for: > > { "driver": "rbd", "image": "foo", > "auth": [ { "type": "cephx", "password-secret": "sec0" }, > { "type": "none" } ], > "pool": "bar" } > > It makes mapping -drive arguments into QMP form a bit trickier, but the > QMP form is then easily extensible if we add another auth method down > the road. Not sure if anybody cares, but I came to the same conclusion, so I like this. Kevin
Eric Blake <eblake@redhat.com> writes: > On 03/24/2017 09:10 AM, Jeff Cody wrote: >> On Fri, Mar 24, 2017 at 08:49:20AM -0500, Eric Blake wrote: >>> On 03/24/2017 07:42 AM, Jeff Cody wrote: >>> >>>> Agree. My preference is to leave it as an array of methods, so long as that >>>> is tenable to libvirt. >>> >>> The -drive syntax should remain unchanged (that's an absolute must for >>> libvirt). But the QMP syntax being an array of methods sounds best to >>> me, and I think password-secret should be part of the array. So my vote >>> would be for: >>> >>> { "driver": "rbd", "image": "foo", >>> "auth": [ { "type": "cephx", "password-secret": "sec0" }, >>> { "type": "none" } ], >>> "pool": "bar" } >>> >>> It makes mapping -drive arguments into QMP form a bit trickier, but the >>> QMP form is then easily extensible if we add another auth method down >>> the road. >>> >> >> In that case, what about adding user as well, and not just password-secret? > > Makes sense - anything that is associated solely with cephx should > belong to the same flat-union branch for cephx, rather than at the top > level. I've thought about this some more and have come to the conclusion that this design is clumsy and overly complex. Moreover, I suspect our testing has been poor. Let me explain. = Overview = What we're trying to configure is authentication methods the client is to offer to the server. This is not a list, it's a set: the order doesn't matter, and multiple entries of the same type make no sense. We could reject multiple entries, or let the last one win silently, but this is just unnecessary complexity. Type "cephx" uses a user name and a key. Type "none" uses neither (it could theoretically use the user name, but I've been assured it doesn't). The user name defaults to "admin". The key defaults to the user name's entry in a keyring. There is a configurable list of key ring files, with a default. The default commonly includes /etc/ceph/client.keyring. = Librados configuration = Librados takes these configuration bits as follows: * The user name is to be passed to rados_create(). NULL means default to "admin". * The key may be passed to rados_conf_set() with key "key" (value is the key) or "keyfile" (value is name of the file containing the key). Documentation discourages use of "key". * The list of keyrings may passed to rados_conf_set() with key "keyring" and a value listing file names separated by ','. At least according to the documentation; the source looks like any non-empty sequence of [;,= \t]* serves as separator. * The offered authentication methods may be passed to rados_conf_set() with key "auth_supported" (deprecated) or "auth_client_required", and a value listing methods separated by "," (or maybe [;,= \t]*, see above). The methods are "cephx" and "none". Default is "cephx,none". = Command line -drive = With -drive file=rbd:pool/image[@snapshot][:key=value...], the key=value... part lets you specify arbitrary rados_conf_set() settings, except pseudo-key "id" is mapped to the user name instead, and pseudo-key "conf" names a rados configuration file. This overloading of rados_conf_set() keys and our own pseudo-keys isn't pretty, but it's a done deal. The full set of authentication configuration discussed above is available as keys "id", "key", "keyfile", "keyring", "auth_supported" and "auth_client_required". Also via "conf", of course. These -drive rbd:...:key=value settings are also available in -drive QemuOpts syntax -drive driver=rbd,key=value: * Pseudo-key "id" is "user" in QemuOpts. * Pseudo-key "conf" is "conf" in QemuOpts, too * Any other keys together become "keyvalue-pairs" in QemuOpts, with the same key=value:... syntax. Additionally, QemuOpts-only "password-secret" lets you set rados_conf_set() key "key" to a value obtained from the QCryptoSecret interface. Note that @password-secret is a misnomer: this is *not* a password, it's a *key*. Calling it a password is confusing, and makes it harder to mentally connect QMP and Ceph configuration. The settings in file=rbd:... override the ones in QemuOpts (that's how ->bdrv_parse_filename() works), which in turn override (I think) settings from a config file. Note that *any* key other than "id" and "conf" in file=rbd:... completely overrides *all* keys in "keyvalue-pairs". Except "password-secret" works the other way: it overrides "key" set in file=rbd:... or "keyvalue-pairs". As so often with syntactic sugar, once you actually explain how it works, you realize what a bloody mess it is. It's not quite clear whether "keyvalue-pairs" is really meant for the user, or just for internal use to implement file=rbd:... I posted a patch to hide it from the user. = QMP blockdev-add and command line -blockdev = The current QAPI/QMP schema lets you specify only a few settings directly: pseudo-keys "id" and "conf" (optional members @user and @conf), keys "key" and "auth_supported" (optional members @password-secret and @auth_supported). The only way to specify other rados_conf_set() settings such as "keyfile" and "keyring" is via "conf". Begs the question how the settings you can specify directly interact with the config file. I figure they override the config file. Let's review how this could be used. If you specify no optional members, you get the Ceph defaults described above. Good. If you want to override the default user, there's @user. Good. If you want to override the keyring, you have to fall back to @conf. Not ideal, but good enough for 2.9. I guess most users will be fine with the default keyrings. If you want to override the authentication methods, you have several options: * Use @conf and set auth_supported (deprecated) or auth_client_required in the config file * Use @auth_supported like this: "auth_supported": [ { "auth": METHOD}, ... ] Clumsy. If you want to override the key, you again have several options: * Use @conf and set keyfile or key in the config file * Use @password-secret to get it from the QCryptoSecret interface Did we actually test this? = What to do for 2.9 = I propose to * drop both "auth_supported" and "password-secret" from the QAPI schema * drop "password-secret" from QemuOpts * hide "keyvalue-pairs" in QemuOpts No existing usage is affected, since all these things are new in 2.9. Users who need something else than the default keyring can continue to configure alternate keys and keyrings with -drive, both directly with file=rbd:...:key=value and with a config file. With -blockdev / blockdev_add, they need to use the config file. We can consider improvements post 2.9.
On Mon, Mar 27, 2017 at 07:58:51AM +0200, Markus Armbruster wrote: > Eric Blake <eblake@redhat.com> writes: > > > On 03/24/2017 09:10 AM, Jeff Cody wrote: > >> On Fri, Mar 24, 2017 at 08:49:20AM -0500, Eric Blake wrote: > >>> On 03/24/2017 07:42 AM, Jeff Cody wrote: > >>> > >>>> Agree. My preference is to leave it as an array of methods, so long as that > >>>> is tenable to libvirt. > >>> > >>> The -drive syntax should remain unchanged (that's an absolute must for > >>> libvirt). But the QMP syntax being an array of methods sounds best to > >>> me, and I think password-secret should be part of the array. So my vote > >>> would be for: > >>> > >>> { "driver": "rbd", "image": "foo", > >>> "auth": [ { "type": "cephx", "password-secret": "sec0" }, > >>> { "type": "none" } ], > >>> "pool": "bar" } > >>> > >>> It makes mapping -drive arguments into QMP form a bit trickier, but the > >>> QMP form is then easily extensible if we add another auth method down > >>> the road. > >>> > >> > >> In that case, what about adding user as well, and not just password-secret? > > > > Makes sense - anything that is associated solely with cephx should > > belong to the same flat-union branch for cephx, rather than at the top > > level. > > I've thought about this some more and have come to the conclusion that > this design is clumsy and overly complex. Moreover, I suspect our > testing has been poor. Let me explain. > > = Overview = > > What we're trying to configure is authentication methods the client is > to offer to the server. > > This is not a list, it's a set: the order doesn't matter, and multiple > entries of the same type make no sense. We could reject multiple > entries, or let the last one win silently, but this is just unnecessary > complexity. > > Type "cephx" uses a user name and a key. > > Type "none" uses neither (it could theoretically use the user name, but > I've been assured it doesn't). > > The user name defaults to "admin". > > The key defaults to the user name's entry in a keyring. There is a > configurable list of key ring files, with a default. The default > commonly includes /etc/ceph/client.keyring. > I don't think 'client.keyring' is one of the defaults. I know /etc/ceph/keyring is, however. > = Librados configuration = > > Librados takes these configuration bits as follows: > > * The user name is to be passed to rados_create(). NULL means default > to "admin". > > * The key may be passed to rados_conf_set() with key "key" (value is the > key) or "keyfile" (value is name of the file containing the key). > Documentation discourages use of "key". > > * The list of keyrings may passed to rados_conf_set() with key > "keyring" and a value listing file names separated by ','. At least > according to the documentation; the source looks like any non-empty > sequence of [;,= \t]* serves as separator. > > * The offered authentication methods may be passed to rados_conf_set() > with key "auth_supported" (deprecated) or "auth_client_required", and > a value listing methods separated by "," (or maybe [;,= \t]*, see > above). The methods are "cephx" and "none". Default is "cephx,none". > > = Command line -drive = > > With -drive file=rbd:pool/image[@snapshot][:key=value...], the > key=value... part lets you specify arbitrary rados_conf_set() settings, > except pseudo-key "id" is mapped to the user name instead, and > pseudo-key "conf" names a rados configuration file. This overloading of > rados_conf_set() keys and our own pseudo-keys isn't pretty, but it's a > done deal. The full set of authentication configuration discussed above > is available as keys "id", "key", "keyfile", "keyring", "auth_supported" > and "auth_client_required". Also via "conf", of course. > > These -drive rbd:...:key=value settings are also available in -drive > QemuOpts syntax -drive driver=rbd,key=value: > > * Pseudo-key "id" is "user" in QemuOpts. > > * Pseudo-key "conf" is "conf" in QemuOpts, too > > * Any other keys together become "keyvalue-pairs" in QemuOpts, with > the same key=value:... syntax. > > Additionally, QemuOpts-only "password-secret" lets you set > rados_conf_set() key "key" to a value obtained from the QCryptoSecret > interface. > > Note that @password-secret is a misnomer: this is *not* a password, it's > a *key*. Calling it a password is confusing, and makes it harder to > mentally connect QMP and Ceph configuration. > Good point; @key-secret would be a better name > The settings in file=rbd:... override the ones in QemuOpts (that's how > ->bdrv_parse_filename() works), which in turn override (I think) > settings from a config file. Note that *any* key other than "id" and > "conf" in file=rbd:... completely overrides *all* keys in > "keyvalue-pairs". > > Except "password-secret" works the other way: it overrides "key" set in > file=rbd:... or "keyvalue-pairs". > > As so often with syntactic sugar, once you actually explain how it > works, you realize what a bloody mess it is. > > It's not quite clear whether "keyvalue-pairs" is really meant for the > user, or just for internal use to implement file=rbd:... I posted a > patch to hide it from the user. > > = QMP blockdev-add and command line -blockdev = > > The current QAPI/QMP schema lets you specify only a few settings > directly: pseudo-keys "id" and "conf" (optional members @user and > @conf), keys "key" and "auth_supported" (optional members > @password-secret and @auth_supported). The only way to specify other > rados_conf_set() settings such as "keyfile" and "keyring" is via "conf". > > Begs the question how the settings you can specify directly interact > with the config file. I figure they override the config file. > Yes, looking at the code rados_conf_set() will override both the config file, and environment variables. However, certain environment variables will override settings in the specified "conf" file. I think "CEPH_KEYRING" is the only one (corresponds to "keyring"), but I am not sure there are not others. This is probably a reason to provide an option for "keyring" via QAPI. > Let's review how this could be used. > > If you specify no optional members, you get the Ceph defaults described > above. Good. > > If you want to override the default user, there's @user. Good. > > If you want to override the keyring, you have to fall back to @conf. > Not ideal, but good enough for 2.9. I guess most users will be fine > with the default keyrings. > Unless the environment variable CEPH_KEYRING is set, at which point "conf" won't override it, unless I am missing something. However, we could either provide a QAPI interface to set the keyring, or keep a user/key option. > If you want to override the authentication methods, you have several > options: > > * Use @conf and set auth_supported (deprecated) or auth_client_required > in the config file > > * Use @auth_supported like this: > > "auth_supported": [ { "auth": METHOD}, ... ] > > Clumsy. > > If you want to override the key, you again have several options: > > * Use @conf and set keyfile or key in the config file > > * Use @password-secret to get it from the QCryptoSecret interface > > Did we actually test this? Yes, and it works. One caveat: the key obtained from the keyring or "ceph auth list" are base64 encoded, and qemu_rbd_set_auth() base64 encodes the key itself into base64. The rados_conf_set() value for "key" is assumed to be base64 encoded. This means the secret file needs to have the key not base64 encoded, with the current code. > > = What to do for 2.9 = > > I propose to > > * drop both "auth_supported" and "password-secret" from the QAPI schema > > * drop "password-secret" from QemuOpts > > * hide "keyvalue-pairs" in QemuOpts > > No existing usage is affected, since all these things are new in 2.9. > > Users who need something else than the default keyring can continue to > configure alternate keys and keyrings with -drive, both directly with > file=rbd:...:key=value and with a config file. With -blockdev / > blockdev_add, they need to use the config file. > > We can consider improvements post 2.9. I am fine with this; the only real functional limitation with dropping this for 2.9 is that default keyring cannot be overridden in certain scenarios (it is set via an environment variable, and 'conf' won't override it). -Jeff
Jeff Cody <jcody@redhat.com> writes: > On Mon, Mar 27, 2017 at 07:58:51AM +0200, Markus Armbruster wrote: >> Eric Blake <eblake@redhat.com> writes: >> >> > On 03/24/2017 09:10 AM, Jeff Cody wrote: >> >> On Fri, Mar 24, 2017 at 08:49:20AM -0500, Eric Blake wrote: >> >>> On 03/24/2017 07:42 AM, Jeff Cody wrote: >> >>> >> >>>> Agree. My preference is to leave it as an array of methods, so long as that >> >>>> is tenable to libvirt. >> >>> >> >>> The -drive syntax should remain unchanged (that's an absolute must for >> >>> libvirt). But the QMP syntax being an array of methods sounds best to >> >>> me, and I think password-secret should be part of the array. So my vote >> >>> would be for: >> >>> >> >>> { "driver": "rbd", "image": "foo", >> >>> "auth": [ { "type": "cephx", "password-secret": "sec0" }, >> >>> { "type": "none" } ], >> >>> "pool": "bar" } >> >>> >> >>> It makes mapping -drive arguments into QMP form a bit trickier, but the >> >>> QMP form is then easily extensible if we add another auth method down >> >>> the road. >> >>> >> >> >> >> In that case, what about adding user as well, and not just password-secret? >> > >> > Makes sense - anything that is associated solely with cephx should >> > belong to the same flat-union branch for cephx, rather than at the top >> > level. >> >> I've thought about this some more and have come to the conclusion that >> this design is clumsy and overly complex. Moreover, I suspect our >> testing has been poor. Let me explain. >> >> = Overview = >> >> What we're trying to configure is authentication methods the client is >> to offer to the server. >> >> This is not a list, it's a set: the order doesn't matter, and multiple >> entries of the same type make no sense. We could reject multiple >> entries, or let the last one win silently, but this is just unnecessary >> complexity. >> >> Type "cephx" uses a user name and a key. >> >> Type "none" uses neither (it could theoretically use the user name, but >> I've been assured it doesn't). >> >> The user name defaults to "admin". >> >> The key defaults to the user name's entry in a keyring. There is a >> configurable list of key ring files, with a default. The default >> commonly includes /etc/ceph/client.keyring. >> > > I don't think 'client.keyring' is one of the defaults. I know > /etc/ceph/keyring is, however. Double-checking... source suggests the default is actually /etc/ceph/$cluster.$name.keyring,/etc/ceph/$cluster.keyring,/etc/ceph/keyring,/etc/ceph/keyring.bin https://github.com/ceph/ceph/blob/master/src/common/config_opts.h#L172 >> = Librados configuration = >> >> Librados takes these configuration bits as follows: >> >> * The user name is to be passed to rados_create(). NULL means default >> to "admin". >> >> * The key may be passed to rados_conf_set() with key "key" (value is the >> key) or "keyfile" (value is name of the file containing the key). >> Documentation discourages use of "key". >> >> * The list of keyrings may passed to rados_conf_set() with key >> "keyring" and a value listing file names separated by ','. At least >> according to the documentation; the source looks like any non-empty >> sequence of [;,= \t]* serves as separator. >> >> * The offered authentication methods may be passed to rados_conf_set() >> with key "auth_supported" (deprecated) or "auth_client_required", and >> a value listing methods separated by "," (or maybe [;,= \t]*, see >> above). The methods are "cephx" and "none". Default is "cephx,none". >> >> = Command line -drive = >> >> With -drive file=rbd:pool/image[@snapshot][:key=value...], the >> key=value... part lets you specify arbitrary rados_conf_set() settings, >> except pseudo-key "id" is mapped to the user name instead, and >> pseudo-key "conf" names a rados configuration file. This overloading of >> rados_conf_set() keys and our own pseudo-keys isn't pretty, but it's a >> done deal. The full set of authentication configuration discussed above >> is available as keys "id", "key", "keyfile", "keyring", "auth_supported" >> and "auth_client_required". Also via "conf", of course. >> >> These -drive rbd:...:key=value settings are also available in -drive >> QemuOpts syntax -drive driver=rbd,key=value: >> >> * Pseudo-key "id" is "user" in QemuOpts. >> >> * Pseudo-key "conf" is "conf" in QemuOpts, too >> >> * Any other keys together become "keyvalue-pairs" in QemuOpts, with >> the same key=value:... syntax. >> >> Additionally, QemuOpts-only "password-secret" lets you set >> rados_conf_set() key "key" to a value obtained from the QCryptoSecret >> interface. >> >> Note that @password-secret is a misnomer: this is *not* a password, it's >> a *key*. Calling it a password is confusing, and makes it harder to >> mentally connect QMP and Ceph configuration. >> > > Good point; @key-secret would be a better name > > >> The settings in file=rbd:... override the ones in QemuOpts (that's how >> ->bdrv_parse_filename() works), which in turn override (I think) >> settings from a config file. Note that *any* key other than "id" and >> "conf" in file=rbd:... completely overrides *all* keys in >> "keyvalue-pairs". >> >> Except "password-secret" works the other way: it overrides "key" set in >> file=rbd:... or "keyvalue-pairs". >> >> As so often with syntactic sugar, once you actually explain how it >> works, you realize what a bloody mess it is. >> >> It's not quite clear whether "keyvalue-pairs" is really meant for the >> user, or just for internal use to implement file=rbd:... I posted a >> patch to hide it from the user. >> >> = QMP blockdev-add and command line -blockdev = >> >> The current QAPI/QMP schema lets you specify only a few settings >> directly: pseudo-keys "id" and "conf" (optional members @user and >> @conf), keys "key" and "auth_supported" (optional members >> @password-secret and @auth_supported). The only way to specify other >> rados_conf_set() settings such as "keyfile" and "keyring" is via "conf". >> >> Begs the question how the settings you can specify directly interact >> with the config file. I figure they override the config file. >> > > Yes, looking at the code rados_conf_set() will override both the config > file, and environment variables. > > > However, certain environment variables will override settings in the > specified "conf" file. I think "CEPH_KEYRING" is the only one (corresponds > to "keyring"), but I am not sure there are not others. > > This is probably a reason to provide an option for "keyring" via QAPI. I'm open to provide more configuration knobs via QAPI, but for 2.9, I propose to stick to the smallest interface that can possibly work. >> Let's review how this could be used. >> >> If you specify no optional members, you get the Ceph defaults described >> above. Good. >> >> If you want to override the default user, there's @user. Good. >> >> If you want to override the keyring, you have to fall back to @conf. >> Not ideal, but good enough for 2.9. I guess most users will be fine >> with the default keyrings. >> > > Unless the environment variable CEPH_KEYRING is set, at which point "conf" > won't override it, unless I am missing something. However, we could either > provide a QAPI interface to set the keyring, or keep a user/key option. For 2.9 QAPI/QMP, I'd say our story for authentication should be "stick to *Ceph* configuration; how you use it is up to you". I.e. set up suitable keyrings, and how exactly you tell Ceph to use non-default keyrings (config file, environment variable, whatever) is your problem. >> If you want to override the authentication methods, you have several >> options: >> >> * Use @conf and set auth_supported (deprecated) or auth_client_required >> in the config file >> >> * Use @auth_supported like this: >> >> "auth_supported": [ { "auth": METHOD}, ... ] >> >> Clumsy. >> >> If you want to override the key, you again have several options: >> >> * Use @conf and set keyfile or key in the config file >> >> * Use @password-secret to get it from the QCryptoSecret interface >> >> Did we actually test this? > > Yes, and it works. One caveat: the key obtained from the keyring or > "ceph auth list" are base64 encoded, and qemu_rbd_set_auth() base64 encodes > the key itself into base64. The rados_conf_set() value for "key" is assumed > to be base64 encoded. This means the secret file needs to have the key not > base64 encoded, with the current code. Good to know, thanks. Documentation or at least comments explaining this would be welcome. >> = What to do for 2.9 = >> >> I propose to >> >> * drop both "auth_supported" and "password-secret" from the QAPI schema >> >> * drop "password-secret" from QemuOpts >> >> * hide "keyvalue-pairs" in QemuOpts >> >> No existing usage is affected, since all these things are new in 2.9. >> >> Users who need something else than the default keyring can continue to >> configure alternate keys and keyrings with -drive, both directly with >> file=rbd:...:key=value and with a config file. With -blockdev / >> blockdev_add, they need to use the config file. >> >> We can consider improvements post 2.9. > > I am fine with this; the only real functional limitation with dropping this > for 2.9 is that default keyring cannot be overridden in certain scenarios > (it is set via an environment variable, and 'conf' won't override it). My PATCH RFC v3 implements this, let's get it reviewed and merged. Regarding the limitation: if you pass an environment variable to QEMU, then realize it gets in your way, consider not passing it :)
On Mon, Mar 27, 2017 at 07:58:51AM +0200, Markus Armbruster wrote: > = What to do for 2.9 = > > I propose to > > * drop both "auth_supported" and "password-secret" from the QAPI schema > > * drop "password-secret" from QemuOpts > > * hide "keyvalue-pairs" in QemuOpts > > No existing usage is affected, since all these things are new in 2.9. Maybe I'm mis-understanding what you're suggesting wrt QemuOpts, but 'password-secret' with RBD is not new in 2.9.0 It was added in 2.6.0 in this commit: commit 60390a2192e7b38aee18db6ce7fb740498709737 Author: Daniel P. Berrange <berrange@redhat.com> Date: Thu Jan 21 14:19:19 2016 +0000 rbd: add support for getting password from QCryptoSecret object Currently RBD passwords must be provided on the command line via $QEMU -drive file=rbd:pool/image:id=myname:\ key=QVFDVm41aE82SHpGQWhBQXEwTkN2OGp0SmNJY0UrSE9CbE1RMUE=:\ auth_supported=cephx This is insecure because the key is visible in the OS process listing. This adds support for an 'password-secret' parameter in the RBD parameters that can be used with the QCryptoSecret object to provide the password via a file: echo "QVFDVm41aE82SHpGQWhBQXEwTkN2OGp0SmNJY0UrSE9CbE1RMUE=" > poolkey.b64 $QEMU -object secret,id=secret0,file=poolkey.b64,format=base64 \ -drive driver=rbd,filename=rbd:pool/image:id=myname:\ auth_supported=cephx,password-secret=secret0 Reviewed-by: Josh Durgin <jdurgin@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Message-id: 1453385961-10718-2-git-send-email-berrange@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com> Regards, Daniel
On 04/03/2017 06:25 AM, Daniel P. Berrange wrote: > On Mon, Mar 27, 2017 at 07:58:51AM +0200, Markus Armbruster wrote: >> = What to do for 2.9 = >> >> I propose to >> >> * drop both "auth_supported" and "password-secret" from the QAPI schema >> >> * drop "password-secret" from QemuOpts >> >> * hide "keyvalue-pairs" in QemuOpts >> >> No existing usage is affected, since all these things are new in 2.9. > > Maybe I'm mis-understanding what you're suggesting wrt QemuOpts, but > 'password-secret' with RBD is not new in 2.9.0 We updated things in the respin of this series; the actual committed version (ending in commit d1c13688) did NOT kill password-secret from QemuOpts, but merely removed it from QMP (it was the QMP portion that was attempted to be new in 2.9).
diff --git a/block/rbd.c b/block/rbd.c index 8ba0f79..f460d71 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -566,7 +566,7 @@ static char *rbd_auth(QDict *options) int i; for (i = 0;; i++) { - sprintf(keybuf, "auth-supported.%d.auth", i); + sprintf(keybuf, "auth-supported.%d", i); val = qdict_get(options, keybuf); if (!val) { break; diff --git a/qapi/block-core.json b/qapi/block-core.json index 0f132fc..fe1e40f 100644 --- a/qapi/block-core.json +++ 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' ] } ## # @BlockdevOptionsRbd:
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. Delete the wrapper, and rename the enum. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- block/rbd.c | 2 +- qapi/block-core.json | 15 ++------------- 2 files changed, 3 insertions(+), 14 deletions(-)