From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44043) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ciYvC-0007fB-H1 for qemu-devel@nongnu.org; Mon, 27 Feb 2017 22:57:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ciYvB-00050l-Ay for qemu-devel@nongnu.org; Mon, 27 Feb 2017 22:57:54 -0500 Date: Mon, 27 Feb 2017 22:57:44 -0500 From: Jeff Cody Message-ID: <20170228035744.GO25637@localhost.localdomain> References: <12a16b28300f871052a49897ec7875a082fc63e3.1488220970.git.jcody@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v2 5/5] block/rbd: add support for 'mon_host', 'auth_supported' via QAPI List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, armbru@redhat.com, berrange@redhat.com On Mon, Feb 27, 2017 at 04:47:54PM -0600, Eric Blake wrote: > On 02/27/2017 12:58 PM, Jeff Cody wrote: > > This adds support for two additional options that may be specified > > by QAPI in blockdev-add: > > > > mon_host: servername and port > > auth_supported: either 'cephx' or 'none' > > Please spell new options with '-' > > > > > Signed-off-by: Jeff Cody > > --- > > block/rbd.c | 39 +++++++++++++++++++++++++++++++++++++++ > > qapi/block-core.json | 8 ++++++++ > > 2 files changed, 47 insertions(+) > > > > diff --git a/block/rbd.c b/block/rbd.c > > index e04a5e1..51e971e 100644 > > --- a/block/rbd.c > > +++ b/block/rbd.c > > @@ -394,6 +394,18 @@ static QemuOptsList runtime_opts = { > > .name = "keyvalue-pairs", > > .type = QEMU_OPT_STRING, > > }, > > + { > > + .name = "server.host", > > + .type = QEMU_OPT_STRING, > > + }, > > + { > > + .name = "server.port", > > + .type = QEMU_OPT_STRING, > > + }, > > See Dan's comment about supporting more than one server via an array in > QAPI. > > > + { > > + .name = "auth_supported", > > Should be auth-supported in QAPI. > > > > @@ -604,6 +620,29 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags, > > goto failed_shutdown; > > } > > > > + /* if mon_host was specified */ > > + if (host) { > > + const char *hostname = host; > > + char *mon_host = NULL; > > + > > + if (port) { > > + mon_host = g_strdup_printf("%s:%s", host, port); > > Does Ceph care about IPv6 (in which case you may need [host]:port when > host itself includes ':')? > Some quick sanity testing seems to show that it does not need [] for ipv6 addresses, which is nice. > > + hostname = mon_host; > > + } > > + r = rados_conf_set(s->cluster, "mon_host", hostname); > > + g_free(mon_host); > > + if (r < 0) { > > + goto failed_shutdown; > > + } > > + } > > + > > + if (auth_supported) { > > + r = rados_conf_set(s->cluster, "auth_supported", auth_supported); > > Translating QAPI auth-supported to rados auth_supported is fine. > > > > +++ b/qapi/block-core.json > > @@ -2680,6 +2680,12 @@ > > # > > # @user: #optional Ceph id name. > > # > > +# @server: #optional Monitor host address and port. This maps > > +# to the "mon_host" Ceph option. > > +# > > +# @auth_supported: #optional Authentication supported. > > +# Either "cephx" or"none". > > Missing a space. > > If you're going to support only a finite set of strings, this should be > a QAPI enum type, not 'str'. > > > +# > > # @password-secret: #optional The ID of a QCryptoSecret object providing > > # the password for the login. > > # > > @@ -2691,6 +2697,8 @@ > > '*conf': 'str', > > '*snapshot': 'str', > > '*user': 'str', > > + '*server': 'InetSocketAddress', > > + '*auth_supported': 'str', > > '*password-secret': 'str' } } > > > > ## > > > > Looks like we'll need a v3 to tweak this one. > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >