[libvirt] [PATCH] storage: escape ipv6 for ceph mon hosts to librados

[Yi Li]

> >Hosts for rbd are ceph monitor daemons. These have fixed IP addresses,
> >so they are often referenced by IP rather than hostname for
> >convenience, or to avoid relying on DNS. Using IPv4 addresses as the
> >host name works already, but IPv6 addresses require rbd-specific
>
> If you include the escaping in the XML, does it currently work?
>   <host name='[2205::192:168:205:141]' port='6789'/>

Yes, It works.
>
>
> >escaping because the colon is used as an option separator in the
> >string passed to librados.
> >
> >Escape these colons, and enclose the IPv6 address in square brackets
> >so it is distinguished from the port, which is currently mandatory.
> >
> >Signed-off-by: Yi Li <yili at winhong.com>
> >---
> > docs/schemas/storagepool.rng                  |  5 ++++-
> > src/storage/storage_backend_rbd.c             | 13 ++++++++++---
> > tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml  | 13 +++++++++++++
> > tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml | 16 ++++++++++++++++
> > tests/storagepoolxml2xmltest.c                |  1 +
> > 5 files changed, 44 insertions(+), 4 deletions(-)
> > create mode 100644 tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml
> > create mode 100644 tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml
> >
> >diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
> >index f8c968e..3056563 100644
> >--- a/src/storage/storage_backend_rbd.c
> >+++ b/src/storage/storage_backend_rbd.c
> >@@ -268,9 +268,16 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
> >                               source->hosts[i].name);
> >         } else if (source->hosts[i].name != NULL &&
> >             source->hosts[i].port) {
> >-            virBufferAsprintf(&mon_host, "%s:%d,",
> >-                              source->hosts[i].name,
> >-                              source->hosts[i].port);
> >+            /* assume host containing : is ipv6 */
> >+            if (strchr(source->hosts[i].name, ':')) {
>
> if (virSocketAddrNumericFamily(listenAddress) == AF_INET6)
>
> By using this helper function, we won't try to escape an address that is
> already escaped.
>
> Also, instead of copying the whole virBuffer call twice, it would be
> nicer to assign the format to a temporary variable like we do in qemuMigrationDstPrepare
>
> Jano
Good point. I'd sending  a v2.
>
>
> >+                virBufferAsprintf(&mon_host, "[%s]:%d,",
> >+                                  source->hosts[i].name,
> >+                                  source->hosts[i].port);
> >+            } else {
> >+                virBufferAsprintf(&mon_host, "%s:%d,",
> >+                                  source->hosts[i].name,
> >+                                  source->hosts[i].port);
> >+            }
> >         } else {
> >             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >                            _("received malformed monitor, check the XML definition"));


On Sun, Apr 28, 2019 at 1:10 PM winhong-yili <yili at winhong.com> wrote:
>
> >Hosts for rbd are ceph monitor daemons. These have fixed IP addresses,
> >so they are often referenced by IP rather than hostname for
> >convenience, or to avoid relying on DNS. Using IPv4 addresses as the
> >host name works already, but IPv6 addresses require rbd-specific
>
> If you include the escaping in the XML, does it currently work?
>   <host name='[2205::192:168:205:141]' port='6789'/>
>
> >escaping because the colon is used as an option separator in the
> >string passed to librados.
> >
> >Escape these colons, and enclose the IPv6 address in square brackets
> >so it is distinguished from the port, which is currently mandatory.
> >
> >Signed-off-by: Yi Li <yili at winhong.com>
> >---
> > docs/schemas/storagepool.rng                  |  5 ++++-
> > src/storage/storage_backend_rbd.c             | 13 ++++++++++---
> > tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml  | 13 +++++++++++++
> > tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml | 16 ++++++++++++++++
> > tests/storagepoolxml2xmltest.c                |  1 +
> > 5 files changed, 44 insertions(+), 4 deletions(-)
> > create mode 100644 tests/storagepoolxml2xmlin/pool-rbd-ipv6.xml
> > create mode 100644 tests/storagepoolxml2xmlout/pool-rbd-ipv6.xml
> >
> >diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
> >index f8c968e..3056563 100644
> >--- a/src/storage/storage_backend_rbd.c
> >+++ b/src/storage/storage_backend_rbd.c
> >@@ -268,9 +268,16 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
> >                               source->hosts[i].name);
> >         } else if (source->hosts[i].name != NULL &&
> >             source->hosts[i].port) {
> >-            virBufferAsprintf(&mon_host, "%s:%d,",
> >-                              source->hosts[i].name,
> >-                              source->hosts[i].port);
> >+            /* assume host containing : is ipv6 */
> >+            if (strchr(source->hosts[i].name, ':')) {
>
> if (virSocketAddrNumericFamily(listenAddress) == AF_INET6)
>
> By using this helper function, we won't try to escape an address that is
> already escaped.
>
> Also, instead of copying the whole virBuffer call twice, it would be
> nicer to assign the format to a temporary variable like we do in qemuMigrationDstPrepare
>
> Jano
>
> >+                virBufferAsprintf(&mon_host, "[%s]:%d,",
> >+                                  source->hosts[i].name,
> >+                                  source->hosts[i].port);
> >+            } else {
> >+                virBufferAsprintf(&mon_host, "%s:%d,",
> >+                                  source->hosts[i].name,
> >+                                  source->hosts[i].port);
> >+            }
> >         } else {
> >             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >                            _("received malformed monitor, check the XML definition"));