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

[Daniel P. Berrange]

On Thu, Jul 25, 2013 at 07:37:22PM +0800, Osier Yang wrote:
> 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.

I rather prefer to see separate APIs for this job as I described. Sure
you could have an optional macaddr parameter, but I think it is nicer
to just have clear APIs for the "list many" vs "get one" tasks.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|