Re: [Qemu-devel] [PATCH v2 2/5] sockets: remove use of QemuOpts from socket_listen

[Eric Blake]

On 11/18/2015 03:08 AM, Daniel P. Berrange wrote:
> On Tue, Nov 17, 2015 at 03:22:04PM -0700, Eric Blake wrote:
>> On 11/17/2015 10:00 AM, Daniel P. Berrange wrote:
>>> The socket_listen method accepts a QAPI SocketAddress object
>>> which it then turns into QemuOpts before calling the
>>> inet_listen_opts/unix_listen_opts helper methods. By
>>> converting the latter to use QAPI SocketAddress directly,
>>> the QemuOpts conversion step can be eliminated
>>>
>>> This also fixes the problem where ipv4=off && ipv6=off
>>> would be treated the same as ipv4=on && ipv6=on
>>>

>>> + *  ipv4  ipv6   family
>>> + *   -     -       PF_UNSPEC

This says I have no preference, so pick the one that works.

>>> + *   -     f       PF_INET
>>> + *   -     t       PF_INET6
>>> + *   f     -       PF_INET6
>>> + *   f     f       <error>
>>> + *   f     t       PF_INET6
>>> + *   t     -       PF_INET
>>> + *   t     f       PF_INET
>>
>> These I understand,

Generally to mean "I specifically requested this" or "I specifically
don't want that", where there is no collision.

>>
>>> + *   t     t       PF_INET6
>>
>> but why is this one PF_INET6 instead of PF_UNSPEC?
> 
> If you use PF_UNSPEC, then the addresses we listen on will be automatically
> deteremined by results of the DNS lookup. ie if DNS only returns an IPv4
> address, it won't listen on IPv6.  When the user has said ipv6=on, then
> they expect to get an error if it was not possible to listen on IPv6. So
> we must use PF_INET6 here to ensure that error, otherwise ipv6=on & ipv4=on
> would be no different from ipv6=- & ipv4=-.

But if I'm specifically requesting that both families be used, either
that should be an error (we can't honor two families at once) or it
should be allowed (use the family that makes sense), not a synonym for
ipv6-only.

>>> @@ -219,13 +251,15 @@ listen:
>>>          freeaddrinfo(res);
>>>          return -1;
>>>      }
>>> -    qemu_opt_set(opts, "host", uaddr, &error_abort);
>>> -    qemu_opt_set_number(opts, "port", inet_getport(e) - port_offset,
>>> -                        &error_abort);
>>> -    qemu_opt_set_bool(opts, "ipv6", e->ai_family == PF_INET6,
>>> -                      &error_abort);
>>> -    qemu_opt_set_bool(opts, "ipv4", e->ai_family != PF_INET6,
>>> -                      &error_abort);
>>> +    if (update_addr) {
>>> +        g_free(saddr->host);
>>> +        saddr->host = g_strdup(uaddr);
>>> +        g_free(saddr->port);
>>> +        saddr->port = g_strdup_printf("%d",
>>> +                                      inet_getport(e) - port_offset);
>>> +        saddr->has_ipv6 = saddr->ipv6 = e->ai_family == PF_INET6;
>>> +        saddr->has_ipv4 = saddr->ipv4 = e->ai_family != PF_INET6;
>>
>> Should we handle PF_UNSPEC specifically, maybe by having the has_ipv6
>> assignment based on e->ai_family != PF_INET?
> 
> The returne e->ai_family from getaddrinfo will never have a value
> of PF_UNSPEC - that's an input only value. So we're OK to assume
> we'll have PF_INET6 and PF_INET only.  Well at least until someone
> invents IPv7 but I'll let my great grandchildren deal with that
> problem ;-)

Okay, but maybe worth a comment somewhere.  Or put another way, on
input, we can request one or both families, on output, we want to
guarantee which family was selected.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature