On Mon, Mar 27, 2017 at 03:26:32PM +0200, Markus Armbruster wrote:
> This reverts half of commit 0a55679. We're having second thoughts on
> the QAPI schema (and thus the external interface), and haven't reached
> consensus, yet. Issues include:
>
> * The implementation uses deprecated rados_conf_set() key
> "auth_supported". No biggie.
>
> * The implementation makes -drive silently ignore invalid parameters
> "auth" and "auth-supported.*.X" where X isn't "auth". Fixable (in
> fact I'm going to fix similar bugs around parameter server), so
> again no biggie.
>
> * BlockdevOptionsRbd member @password-secret applies only to
> authentication method cephx. Should it be a variant member of
> RbdAuthMethod?
>
> * BlockdevOptionsRbd member @user could apply to both methods cephx
> and none, but I'm not sure it's actually used with none. If it
> isn't, should it be a variant member of RbdAuthMethod?
>
> * The client offers a *set* of authentication methods, not a list.
> Should the methods be optional members of BlockdevOptionsRbd instead
> of members of list @auth-supported? The latter begs the question
> what multiple entries for the same method mean. Trivial question
> now that RbdAuthMethod contains nothing but @type, but less so when
> RbdAuthMethod acquires other members, such the ones discussed above.
>
> * How BlockdevOptionsRbd member @auth-supported interacts with
> settings from a configuration file specified with @conf is
> undocumented. I suspect it's untested, too.
>
> Let's avoid painting ourselves into a corner now, and revert the
> feature for 2.9.
>
> Note that users can still configure authentication methods with a
> configuration file. They probably do that anyway if they use Ceph
> outside QEMU as well.
>
> qemu_rbd_array_opts()'s parameter @type now must be RBD_MON_HOST,
> which is silly. This will be cleaned up shortly.
>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
I think this move makes sense; it allows blockdev-add to still be supported
for rbd, but does not lock us into a perhaps unwieldy API.
Reviewed-by: Jeff Cody <jc...@redhat.com>
> ---
> block/rbd.c | 31 +++----------------------------
> qapi/block-core.json | 24 ------------------------
> 2 files changed, 3 insertions(+), 52 deletions(-)
>
> diff --git a/block/rbd.c b/block/rbd.c
> index cf0bab0..103ce44 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -320,8 +320,7 @@ static QemuOptsList runtime_opts = {
> .help = "Rados id name",
> },
> /*
> - * server.* and auth-supported.* extracted manually, see
> - * qemu_rbd_array_opts()
> + * server.* extracted manually, see qemu_rbd_array_opts()
> */
> {
> .name = "password-secret",
> @@ -356,11 +355,6 @@ static QemuOptsList runtime_opts = {
> .name = "port",
> .type = QEMU_OPT_STRING,
> },
> - {
> - .name = "auth",
> - .type = QEMU_OPT_STRING,
> - .help = "Supported authentication method, either cephx or none",
> - },
> { /* end of list */ }
> },
> };
> @@ -512,7 +506,6 @@ static void qemu_rbd_complete_aio(RADOSCB *rcb)
> }
>
> #define RBD_MON_HOST 0
> -#define RBD_AUTH_SUPPORTED 1
>
> static char *qemu_rbd_array_opts(QDict *options, const char *prefix, int
> type,
> Error **errp)
> @@ -527,7 +520,7 @@ static char *qemu_rbd_array_opts(QDict *options, const
> char *prefix, int type,
> Error *local_err = NULL;
> int i;
>
> - assert(type == RBD_MON_HOST || type == RBD_AUTH_SUPPORTED);
> + assert(type == RBD_MON_HOST);
>
> num_entries = qdict_array_entries(options, prefix);
>
> @@ -573,10 +566,9 @@ static char *qemu_rbd_array_opts(QDict *options, const
> char *prefix, int type,
> value = strbuf;
> }
> } else {
> - value = qemu_opt_get(opts, "auth");
> + abort();
> }
>
> -
> /* each iteration in the for loop will build upon the string, and if
> * rados_str is NULL then it is our first pass */
> if (rados_str) {
> @@ -608,7 +600,6 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
> *options, int flags,
> QemuOpts *opts;
> Error *local_err = NULL;
> char *mon_host = NULL;
> - char *auth_supported = NULL;
> int r;
>
> opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
> @@ -619,14 +610,6 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
> *options, int flags,
> return -EINVAL;
> }
>
> - auth_supported = qemu_rbd_array_opts(options, "auth-supported.",
> - RBD_AUTH_SUPPORTED, &local_err);
> - if (local_err) {
> - error_propagate(errp, local_err);
> - r = -EINVAL;
> - goto failed_opts;
> - }
> -
> mon_host = qemu_rbd_array_opts(options, "server.",
> RBD_MON_HOST, &local_err);
> if (local_err) {
> @@ -678,13 +661,6 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
> *options, int flags,
> }
> }
>
> - if (auth_supported) {
> - r = rados_conf_set(s->cluster, "auth_supported", auth_supported);
> - if (r < 0) {
> - goto failed_shutdown;
> - }
> - }
> -
> if (qemu_rbd_set_auth(s->cluster, secretid, errp) < 0) {
> r = -EIO;
> goto failed_shutdown;
> @@ -735,7 +711,6 @@ failed_shutdown:
> failed_opts:
> qemu_opts_del(opts);
> g_free(mon_host);
> - g_free(auth_supported);
> return r;
> }
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 5d2efe4..6a7ca0b 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2601,27 +2601,6 @@
>
>
> ##
> -# @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' } }
> -
> -##
> # @BlockdevOptionsRbd:
> #
> # @pool: Ceph pool name.
> @@ -2639,8 +2618,6 @@
> # @server: Monitor host address and port. This maps
> # to the "mon_host" Ceph option.
> #
> -# @auth-supported: Authentication supported.
> -#
> # @password-secret: The ID of a QCryptoSecret object providing
> # the password for the login.
> #
> @@ -2653,7 +2630,6 @@
> '*snapshot': 'str',
> '*user': 'str',
> '*server': ['InetSocketAddressBase'],
> - '*auth-supported': ['RbdAuthMethod'],
> '*password-secret': 'str' } }
>
> ##
> --
> 2.7.4
>
