On 03/10/2015 07:44 AM, John Ferlan wrote:
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.
yes, migration part need more patch and more intelligence :)
<...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.
right, i forgot this
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.
Thanks a lot for your help !
John
Luyao
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list