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

[Daniel P. Berrange]

On Tue, Feb 28, 2017 at 07:34:52AM -0500, Jeff Cody wrote:
> On Tue, Feb 28, 2017 at 10:28:49AM +0000, Daniel P. Berrange wrote:
> > On Tue, Feb 28, 2017 at 10:16:51AM +0000, Daniel P. Berrange wrote:
> > > On Mon, Feb 27, 2017 at 10:57:44PM -0500, Jeff Cody wrote:
> > > > On Mon, Feb 27, 2017 at 04:47:54PM -0600, Eric Blake wrote:
> > > > > On 02/27/2017 12:58 PM, Jeff Cody wrote:
> > > > > > @@ -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.
> > > 
> > > Hmm, that is very odd to me, as that means parsing is ambiguous. The
> > > ceph 'mon_host' option allows the port to be omitted, so given a
> > > string  2001:242:24:23   there's no way of knowing whether it is
> > > a IPv6 addr 2001:242:24:23, with no port, or an addr 2001:242:24 with
> > > port of 23
> > 
> > Looking at the source code to ceph it appears that if you omit the
> > use of [], then it will treat the entire string as being the address
> > without port. It does look to support use of [], so we should use
> > that IIUC.
> 
> For the sake of clarity, when you say we should use [] for ipv6, you mean
> QEMU (rather than doing it in libvirt)?

Yes, in QAPI JSON, it should be the bare IPv6 address in the server
host field. When QEMU conbines the host + port to form the mon_host
string, it must add [] if it sees the host from QAPI is a raw IPv6
address

> > https://blog.widodh.nl/2014/11/ceph-with-a-cluster-and-public-network-on-ipv6/
> > 
> > See also entity_addr_t::parse in $CEPH/src/msg/msg_types.cc which is
> > what I think parses the mon addr. NB, I've not tested this yet, so
> > if you have a live ceph cluster with ipv6, it'd be good to verify that
> > using [] is correct.
> >
> 
> Do you think this needs to be in place before either A) we pull the series
> for softfreeze, or B) 2.10?  I.e., is this something we can fix up later?
> It is OK if not, but I have a flight leaving soon and can't work on the
> series anymore -- so depending on this and how v3 review goes, we may just
> need to slip the series to 2.10.

The fix doesn't affect QAPI protocol spec, so as long as it is fixed
before 2.9 release it'd be ok.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|