Re: [libvirt] RFC: Introduce API to return configuration/state paths of the network driver

On 25/07/13 19:21, Osier Yang wrote: >On 25/07/13 19:13, Daniel P. Berrange wrote: >>On Thu, Jul 25, 2013 at 07:01:05PM +0800, Osier Yang wrote: >>>On 25/07/13 18:53, Daniel P. Berrange wrote: >>>>On Thu, Jul 25, 2013 at 06:43:00PM +0800, Osier Yang wrote: >>>>>On 25/07/13 17:35, Daniel P. Berrange wrote: >>>>>>On Thu, Jul 25, 2013 at 02:02:36PM +0530, Nehal J. Wani wrote: >>>>>>>Currently, there is no API which returns >>>>>>>configuration/state paths of the >>>>>>>network driver. >>>>>>>Although it is a private implementation of the network >>>>>>>driver, I don't see >>>>>>>any harm in >>>>>>>making the locations public because although the >>>>>>>locations might change, >>>>>>>there will always >>>>>>>be a location for these files. There is a need for >>>>>>>this API to implement >>>>>>>method 2 of the >>>>>>>"API to query ip addresses of a given domain", refer: >>>>>>>http://www.mail-archive.com/libvir-list@redhat.com/msg79793.html >>>>>>>. It is >>>>>>>required to parse >>>>>>>the leases file generated by dnsmasq. So, this API >>>>>>>will be used by the qemu >>>>>>>driver, but it >>>>>>>can also be made public, so that, if a user wants to know get some >>>>>>>information from a >>>>>>>configuration file, he can get the location from >>>>>>>libvirt and analyze it on >>>>>>>his own. Right now, >>>>>>>there is an alternate way to get the info: by using >>>>>>>networkDnsmasqLeaseFileNameDefault, >>>>>>>defined in /src/network/bridge_driver.c Since this >>>>>>>function is static, it >>>>>>>is part of the private >>>>>>>implementation and not visible outside. To make it >>>>>>>public, the following >>>>>>>hack is possible: >>>>>>NACK, >>>>>> >>>>>>As I explained on IRC, the hypervisor drivers have no >>>>>>business accessing >>>>>>the dnsmasq lease files from the bridge driver. This is >>>>>>considered to be >>>>>>a private implementation detail. >>>>>> >>>>>>At a conceptual level, what you're after here is a list >>>>>>of all the IP, >>>>>>mac address mappings of the virtual network. This >>>>>>information is useful >>>>>>even outside the context of the hypervisor driver method >>>>>>you're working >>>>>>on. So we should create formal APIs for exposing this, >>>>>>something like: >>>>>> >>>>>> virNetworkGetDHCPLeases(virNetworkPtr network, >>>>>> virNetworkDHCPLeasePtr *leases, >>>>>> unsigned int nleases); >>>>>i'm wondering if it should be more than just the lease >>>>>file path, e.g. >>>>>also the $net.conf, $net-radvd.conf, etc, though they are useless >>>>>now, but may be useful in future, i.e. to have a more general api >>>>>than this one. and in that case, it should return an array of typed >>>>>parameter instead. >>>>We've already discussed this in the context of the virDomain API for >>>>getting IP addresses & decided that virTypedParameter was >>>>not appropriate >>>>there & we'd use a struct. The same arguments apply here IMHO. >>>> >>>the api to get the ip addresses is more complicate than this, and we >>>finally chose the struct is because of the multiple level information >>>is hard to constuct with typed parameter, but for this api, >>>it's different, >>>as it just needs to return the file paths. >>No, file paths will absolutely never be exposed outside of the bridge >>driver. The API I suggest above are about exposing the IP address + MAC >>address of current leases. ie the actual data the user needs, *not* the >>file path containing the data which is a private impl detail. >> >oh, i see, agreed with the idea then. > for the api interface: int virNetworkGetDHCPLeases(virNetworkPtr network, unsigned char *macaddr, virNetworkDHCPLeasePtr *leases, unsigned int nleases); i think this is better. which returns all of the leases if no mac is specified. otherwise just returns the lease of the network matches the mac.