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