Re: [Qemu-devel] [PATCH 3/4] net: split hostname and service by last colon

[Kevin Wolf <kwolf@redhat.com>]

Am 05.03.2012 09:59, schrieb Amos Kong:
> ----- Original Message -----
>> Am 02.03.2012 20:54, schrieb Laine Stump:
>>> On 03/02/2012 05:35 AM, Kevin Wolf wrote:
>>>> Am 02.03.2012 10:58, schrieb Amos Kong:
>>>>> On 02/03/12 11:38, Amos Kong wrote:
>>>>>>>> --- a/net.c
>>>>>>>> +++ b/net.c
>>>>>>>> @@ -84,7 +84,7 @@ static int get_str_sep(char *buf, int
>>>>>>>> buf_size,
>>>>>>>> const char **pp, int sep)
>>>>>>>>       const char *p, *p1;
>>>>>>>>       int len;
>>>>>>>>       p = *pp;
>>>>>>>> -    p1 = strchr(p, sep);
>>>>>>>> +    p1 = strrchr(p, sep);
>>>>>>>>       if (!p1)
>>>>>>>>           return -1;
>>>>>>>>       len = p1 - p;
>>>>>>> And what if the port isn't specified? I think you would
>>>>>>> erroneously
>>>>>>> interpret the last part of the IP address as port.
>>>>> Hi Kevin, port must be specified in '-incoming' parameters and
>>>>> migrate
>>>>> monitor cmd.
>>>>>
>>>>>   qemu-kvm ... -incoming tcp:$host:$port
>>>>>   (qemu) migrate -d tcp:$host:$port
>>>>>
>>>>>
>>>>> If use boot up guest by wrong cmdline, qemu will report an error
>>>>> msg.
>>>>>
>>>>> # ./x86_64-softmmu/qemu-system-x86_64 --enable-kvm -boot n
>>>>> -incoming
>>>>> tcp:2312::8272 -monitor stdio
>>>>> qemu-system-x86_64: qemu: getaddrinfo: Name or service not known
>>>>> tcp_server_start: Invalid argument
>>>>> Migration failed. Exit code tcp:2312::8272(-22), exiting.
>>>> Which is because 2312: isn't a valid IP address, right? But what
>>>> if you
>>>> have something like 2312::1234:8272? If you misinterpret the 8272
>>>> as a
>>>> port number, the remaining address is still a valid IPv6 address.
>>>
>>> This is made irrelevant by PATCH 4/4, which allows for the IP
>>> address to
>>> be placed inside brackets:
>>>
>>>    [2312::8272]:port
>>>
>>> (at least it's irrelevant if your documentation *requires* brackets
>>> for
>>> all numeric ipv6-address:port pairs, which is strongly recommended
>>> by
>>> RFC 5952). It really is impossible to disambiguate the meaning of
>>> the
>>> final ":nnnn" unless you require these brackets (or 1) require full
>>> specification of all potential colons in the IPv6 address or
>>> require
>>> that the port *always* be specified, neither of which seem
>>> acceptable to
>>> me).
>>
>> Here you're actually explaining why it's not irrelevant. You don't
>> want
>> to enforce port numbers, so 2312::1234:8272 must be interpreted as an
>> IPv6 address without a port. This code however would take 8727 as the
>> port and 2312::1234 as the IPv6 address, which is not what you
>> expected
>> (even after brackets are allowed - they don't make a difference
>> because
>> the example doesn't use brackets).
> 
> In the migration context, host/port are all necessary, so it's right to parse "8272" to a port.
> However, for IPv6 brackets must be mandatory if you require a port.

Makes sense.

> BTW, the DNS delay issue existed in the past (gethostbyname()), it should be fixed by another patchset.
> I will post my V2 (without fix of DNS delay) later.

Yes, I agree.

Kevin