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

Re: [libvirt] RFC: Implement virDomainGetIPAddress()



2011/7/15 Michal Novotny <minovotn redhat com>:
> On 07/15/2011 10:28 AM, Matthias Bolte wrote:
>> 2011/7/15 Michal Novotny <minovotn redhat com>:
>>> On 07/14/2011 05:42 PM, Matthias Bolte wrote:
>>>> 2011/7/14 Michal Novotny <minovotn redhat com>:
>>>>> Hi guys,
>>>>> some time ago I've been investigating the options to get the guest's IP
>>>>> address information without having to connect to guest's VNC window or
>>>>> console. It was for one project I've been working on and I found that
>>>>> the solution lies in the procfs, precisely in the /proc/{PID}/net/arp...
>>>>>
>>>>> The format is as follows:
>>>>>
>>>>> $ cat /proc/{PID}/net/arp
>>>>> IP address       HW type     Flags       HW address            Mask
>>>>> Device
>>>>> 192.168.122.36   0x1         0x2         52:54:00:35:76:e6     *
>>>>> virbr0
>>>>>
>>>>> where the HW address matches the MAC address associated to the guest's
>>>>> NIC. Implementing such an API shouldn't be a big problem however I know
>>>>> that there's some option to run libvirt on Windows machines. It should
>>>>> be just the client so it shouldn't really matter however I'd like to ask
>>>>> you whether it's really not an issue.
>>>> Windows or not is irrelevant here as the IP address lookup cannot be
>>>> implemented in a general way/place, but will have to be implemented by
>>>> the hypervisor/network drivers.
>>>>
>>>>> The function should return a string of the guest's IP address as read
>>>>> from the procfs or return a NULL value if there's no IP address
>>>>> associated with the guest.
>>>>>
>>>>> If the multiple NICs are being used by the guest then the function
>>>>> should return either the IP address matching the MAC address passed to
>>>>> the function or the first IP address if omitted.
>>>>>
>>>>> The prototype should be:
>>>>>
>>>>> char *virDomainGetIPAddress(virDomainPtr domain, char *devmac);
>>>> First of all you're missing the unsigned int flags parameter.
>>>>
>>>> Also did you consider that the MAC to IP(v4|v6) mapping isn't
>>>> necessarily a 1:1 mapping, but the signature of your function requires
>>>> this?
>>> Well, for this I considered the IPv4 address only.
>>>
>>>> I took a look at this and wonder what's the difference between
>>>> /proc/{PID}/net/arp and /proc/net/arp. Also as it's ARP it'll only
>>>> work for IPv4.
>>> Honestly I was unable to see it in /proc/net/arp. I saw just some ARP
>>> records but not related to the qemu-kvm process I tried on my Fedora-14
>>> box but I was able to see those records in /proc/{PID}/net/arp and
>>> therefore I was looking to the /proc/{PID}/net/arp and why I mentioned
>>> this instead.
>>>
>>> Considering the fact this will be working just for IPv4 there should be
>>> some prototype like:
>>>
>>> char *virDomainGetIPAddress(virDomainPtr domain, char *devmac, unsigned int flags);
>> Even if you're going to restrict it to IPv4 for now this signature
>> won't do, as you can have multiple IPv4 addresses assigned to the same
>> MAC address. How do you want to return multiple IP addresses? As a
>> comma separated list as string and the caller has to parse it?
>>
> Wait a minute there please. The MAC address should be unique in the
> system so user should be using just one IP address per one MAC address.
> Since MAC address should be unique in the system then the IP address
> assigned to this MAC address should be just one, shouldn't it ?

You can easily add multiple IP address to an interface (and therefore
to the same MAC address)

ip addr add 192.168.0.17 dev eth0
ip addr add 192.168.0.42 dev eth0

> Nevertheless I think you know more about networking options than I do so

Well, I know that there is no strict 1:1 mapping between MAC and IP
addresses and I want that this fact is consider in the discussion here
and we don't add a new API that turns out to be too simple/restricted
in the end. Whether we really want/need to cover this case is a
different question.

> when I consider the scenario you wrote me about I don't like the idea of
> leaving the parsing to the caller and for the scenario of multiple IP
> addresses in the return value I recommend a new prototype:
>
> char **virDomainGetIPAddress(virDomainPtr domain, char *devmac, unsigned int *count, unsigned int flags);
>
>
> where count will be the output parameter with the number of elements in
> the return value. It should be used like:
>
> virDomainPtr domain = ...;
> char *macaddr = "11:22:33:44:55:66";
> char **ips = NULL;
> int count = -1;
>
> ips = virDomainGetIPAddress(domain, macaddr, &count, 0);
>
> for (i = 0; i < count; i++) {
>  ... ips[i] ...
>  ... free(ips[i]) ...
> }
>
>
> The return value allocation should be done by the function itself and
> the called should free the result (ips in this case).

That's a possible way to deal with this, yes.

And now lets spin this a bit further and consider IPv6 addresses.

ipv4s = virDomainGetIPAddress(domain, macaddr, &count, VIR_DOMAIN_ADDRESS_IPV4);
ipv6s = virDomainGetIPAddress(domain, macaddr, &count, VIR_DOMAIN_ADDRESS_IPV6);

This are the simple cases where the caller explicitly requests only
one version and knows the version of the returned IP addresses. But
what about this

ips = virDomainGetIPAddress(domain, macaddr, &count,
VIR_DOMAIN_ADDRESS_IPV4 | VIR_DOMAIN_ADDRESS_IPV6);

Now the caller needs to detect the version from the IPs string
representation. Maybe string representation isn't the best approach
here.

Anyway, I'm probably already overengineering this here :)

> Why did you mention the comma-separated list of IP addresses?

Because I wanted to make a bad example.

> I think
> the solution with comma-separated list is not clean at all and the
> solution I recommend now is much cleaner, don't you think?

Yes.

-- 
Matthias Bolte
http://photron.blogspot.com


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