[libvirt] [PATCH] qemu: escape ipv6 for rbd network disk hosts

[Josh Durgin]

On 01/23/2013 08:20 PM, Osier Yang wrote:
> On 2013年01月24日 10:15, Josh Durgin 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
>> escaping because the colon is used as an option separator in the
>> string passed to qemu.
>>
>> Escape these colons, and enclose the IPv6 address in square brackets
>> if a port is specified.
>
> Actually the IPv6 address is always enclosed in the code.

Indeed.

>>
>> Signed-off-by: Josh Durgin<josh.durgin at inktank.com>
>> ---
>>   docs/schemas/domaincommon.rng                      |    5 ++-
>>   src/qemu/qemu_command.c                            |   34
>> +++++++++++++++----
>>   tests/qemuargv2xmltest.c                           |    1 +
>>   .../qemuxml2argv-disk-drive-network-rbd-ipv6.args  |    9 +++++
>>   .../qemuxml2argv-disk-drive-network-rbd-ipv6.xml   |   36
>> ++++++++++++++++++++
>>   tests/qemuxml2argvtest.c                           |    2 +
>>   6 files changed, 79 insertions(+), 8 deletions(-)
>>   create mode 100644
>> tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args
>>   create mode 100644
>> tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.xml
>>
>> diff --git a/docs/schemas/domaincommon.rng
>> b/docs/schemas/domaincommon.rng
>> index 7f3320e..273e54c 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -1099,7 +1099,10 @@
>>                             </attribute>
>>                           </optional>
>>                           <attribute name="name">
>> -<ref name="dnsName"/>
>> +<choice>
>> +<ref name="dnsName"/>
>> +<ref name="ipAddr"/>
>> +</choice>
>>                           </attribute>
>>                           <attribute name="port">
>>                             <ref name="unsignedInt"/>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 02fe015..dfc042b 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -34,6 +34,7 @@
>>   #include "virerror.h"
>>   #include "virutil.h"
>>   #include "virfile.h"
>> +#include "virstring.h"
>>   #include "viruuid.h"
>>   #include "c-ctype.h"
>>   #include "domain_nwfilter.h"
>> @@ -1937,13 +1938,16 @@ qemuBuildRBDString(virConnectPtr conn,
>>               if (i) {
>>                   virBufferAddLit(opt, "\\;");
>>               }
>> -            if (disk->hosts[i].port) {
>> -                virBufferAsprintf(opt, "%s\\:%s",
>> -                                  disk->hosts[i].name,
>> -                                  disk->hosts[i].port);
>> +
>> +            /* assume host containing : is ipv6 */
>> +            if (strchr(disk->hosts[i].name, ':')) {
>> +                virBufferEscape(opt, '\\', ":", "[%s]",
>> disk->hosts[i].name);
>>               } else {
>>                   virBufferAsprintf(opt, "%s", disk->hosts[i].name);
>>               }
>> +            if (disk->hosts[i].port) {
>> +                virBufferAsprintf(opt, "\\:%s", disk->hosts[i].port);
>> +            }
>>           }
>>       }
>>
>> @@ -1961,15 +1965,26 @@ error:
>>   static int qemuAddRBDHost(virDomainDiskDefPtr disk, char *hostport)
>>   {
>>       char *port;
>> +    size_t skip;
>> +    char **parts;
>>
>>       disk->nhosts++;
>>       if (VIR_REALLOC_N(disk->hosts, disk->nhosts)<  0)
>>           goto no_memory;
>>
>> -    port = strstr(hostport, "\\:");
>> +    if (strchr(hostport, ']')) {
>> +        /* ipv6, strip brackets */10000 / 384 =
>> +        hostport += 1;
>> +        port = strstr(hostport, "]\\:");
>
> This can be simplified as (no need to get the same address
> twice):
>
>         if ((port = strchr(hostport, ']'))) {
>             hostport += 1;
>             skip = 3;
>         } else {
>             ...
>         }
>
> Others looks pretty neat. ACK.

Good point, I'd forgotten that the port is mandatory when a name is
specified. Sending a v2.

Thanks!
Josh