Re: [PATCHv2 1/4] util: introduce 2 new helpers for get interface IPv6 address

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 03/09/2015 03:35 AM, lhuang wrote:
> 

<...snip...>

>> I think once you have an address you just return the first one found and
>> document it that way avoiding this path completely. You could also note
>> that if you want a specific address use the type='address'
>>
> 
> Hmm, yes, I agree it is not a good idea to report error when we get
> multiple addresses in this function (In some case, caller just want one
> ip address and not care about which one we chose), so remove the check
> and error output here is reasonable (maybe we can use a DEBUG or WARNING
> here complain about this and set ret to 0).
> 
> However this check and error output is a result from last review, i
> think maybe we can move them to a right place (should in another patch).
> Because we use listen type='network' for migration in the most case, and
> if a host interface has multiple address than we always chose the first
> one, this will make things worse in some case. An example:
> 
> In host A, we have a happy vm which use listen type='network' and use a
> spice client to connect to it (listen address is
> "fe80::7ae3:b5ff:fec5:189e%enp1s0" and address is get from an interface
> via virNetDevGetIfaddrsAddress() ), than we migrate this vm to a another
> host B, we found a network have the same name and use a host interface
> in host B, we use virNetDevGetIfaddrsAddress() get the first ipv6
> address and we get "2001:db8:ca2:2::1/64", unfortunately this address is
> not a "good" address (the good one is the second one :-P ) and spice
> connection will be broken, and the user will say : "why libvirt chose a
> wrong address and broke my connection :-(".
> 
> NB: the comment from Laine in this mail:
> https://www.redhat.com/archives/libvir-list/2015-February/msg01080.html
> 

Right and as Laine points out:

"... Because it's impossible to do it right, I say we shouldn't even
try; it would be worse for it to work halfway."

Doing it "right" thus requires proper configuration rather than
"premonition" by libvirt.  I think a follow up patch could describe the
migration conundrum where if the plan is to migrate the vm, then the
target node will have to have a "valid" IPv{4|6} address as the first
address; otherwise, the connection will be broken.

Migration is a tricky beast. I recall doing something for a former
project regarding this kind of issue, but I cannot remember the details.
I do remember though our logic filtered out a few address types before
returning what "should" be a usable address. I also recall some sort of
nasty ARP test, but the details were specific to the HP-UX world I used
to live in.



<...snip...>

>>> +int virNetDevGetIPAddress(const char *ifname,
>>> +                          bool want_ipv6,
>>> +                          virSocketAddrPtr addr)
>> same
>>
>>> +{
>>> +    int ret;
>>> +
>>> +    memset(addr, 0, sizeof(*addr));
>>> +    addr->data.stor.ss_family = AF_UNSPEC;
>>> +
>>> +    ret = virNetDevGetIfaddrsAddress(ifname, want_ipv6, addr);
>>> +    if (ret != -2)
>>> +        return ret;
>>> +
>>> +    if (!want_ipv6)
>>> +        return virNetDevGetIPv4Address(ifname, addr);
>> Here if we have virNetDevGetIPv4Address() return -2, then we can take
>> our message above and place it right here while also adjusting the "!
>> SIOCGIFADDR" to just return -2.
>>
>>> +
>>> +    return -1;
>>> +}
>> NOTE: This does not return -2 in any
> 
> should be return -2 (and this only happen when want_ipv6 is true and the
> ret is -2)


Hmm... if we return -2, then the caller's caller spits out

"network-based listen not possible, network driver not present"

rather than our message... Which is less than helpful.

I still have to process your follow-up email with a patch in it, but my
guess is I'm going to repost the patches I have so that we're on the
same page.

John

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]