Re: [Qemu-devel] [Qemu-discuss] TCP options ipv4 and ipv6 have no effect

[Paolo Bonzini <pbonzini@redhat.com>]

On 05/10/2015 20:03, Sair, Umair wrote:
>> The first if handles the "default to N" case, the second handles "default to Y", the (absent) else case handles "default to PF_UNSPEC".
> 
> Can you please elaborate it. Also I am not understanding the reason for inverting the values of addr->has_ipv* in second if condition.

If a value is absent, you can either default it to Y (then the value is
!addr->has_xyz || addr->xyz) or default it to N (then the value is
addr->has_xyz && addr->xyz).

The desired logic is:

- with one absent value and one "on", the absent value should be "off"

- with one absent value and one "off", the absent value should be "on"

- with two absent values, the absent values can be both left absent

The patch works like this:

>>  static void inet_addr_to_opts(QemuOpts *opts, const InetSocketAddress *addr)  {
>> -    bool ipv4 = addr->ipv4 || !addr->has_ipv4;
>> -    bool ipv6 = addr->ipv6 || !addr->has_ipv6;
>> +    bool ipv4 = addr->has_ipv4 && addr->ipv4;
>> +    bool ipv6 = addr->has_ipv6 && addr->ipv6;
>>  
>> -    if (!ipv4 || !ipv6) {
>> +    if (ipv4 || ipv6) {
>>          qemu_opt_set_bool(opts, "ipv4", ipv4, &error_abort);
>>          qemu_opt_set_bool(opts, "ipv6", ipv6, &error_abort);

This handles the case where the absent value should be "off": at least
one value is present _and_ true.

>> +    } else if (addr->has_ipv4 || addr->has_ipv6) {
>> +        qemu_opt_set_bool(opts, "ipv4", !addr->has_ipv4, &error_abort);
>> +        qemu_opt_set_bool(opts, "ipv6", !addr->has_ipv6, &error_abort);

This handles the case where the absent value should be "on".  We know
that whichever the present value is, it is false; therefore
!addr->has_xyz || addr->xyz simplifies to just !addr->has_xyz.

> I believe that the fix for the issue under discussion will be
> committed to qemu repo very soon,

Did you test the patch, and did it work for you?  If so, it is customary
to reply with a line like "Tested by: Sair, Umair <Umair_Sair@mentor.com>".

> so I'll like to add one more thing
> which requires to be fixed along with it. In 'tcp_chr_accept'
> function of qemu-char.c, the data type of saddr should be
> sockaddr_in6 so that it works with both IPv6 and IPv4 on Windows
> (works for linux without it because of accept4 and works with this
> solution as well!).

Can you send a patch for it?

Thanks!

Paolo