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

Re: [libvirt] [Fwd: first cut public API for physical host interface configuration]



On Wed, Mar 25, 2009 at 02:16:48PM -0400, Laine Stump wrote:
> (Oops, I originally sent this with the wrong return address, which 
> probably caused it to be silently swallowed...)
> 
> To get started integrating libnetcf support into libvirt, the attached 
> libvirt.h diff has a first attempt at the public API that will hook up 
> to libnetcf on the libvirtd side. I started out with the virNetwork* 
> API, and modified/removed as seemed appropriate.
> 
> A few points worth mentioning:
> 
> virNetwork has "defined" and "active" interfaces, but so far 
> virInterface just has interfaces, as the active/inactive status is 
> really controlled by 1) the "onboot" property in the XML, and 2) whether 
> or not virInterfaceStart() has been called yet.

When 'virNetwork' talks about "defined" vs "active" interfaces 
there are a few things to be aware of:

 - There is a concept of a persistent network. This is a network
   which has a config file associated with it. 
 - There is a concept of a transient network. This is a network
   which does not have any config file associated wit hit.

 - A persistent network is first defined (virNetworkDefine). This
   creates the config file. 
 - A persistent network can be started (virNetworkCreate). This
   is now an active + defined network
 - A transient network can be started (virNetworkCreateXML). This
   is just an active network
 - When 'virNetworkDestroy' is called on a active network it is
   stopped. If it is transient, all trace of it now goes away.
   If it was persistent, you can still query the defined config

I think that does map reasonably well onto physical interfaces
too. eg, you can define an interface by creating the ifcfg-eth0
file (or equivalent).  A defined inteface can be made active
by calling 'ifup'.

You can also have transient interfaces for which there is no
ifcfg-eth0 file. eg, if someone does 'brctl addbr foo' you
now have a transient interface which is active, but has no
config. 

> libnetcf works with netcf structs (one per library instance) and 
> netcf_if structs (one per interface, multiple per library instance. As 
> far as I can see right now, the netcf struct will not need to be 
> referenced directly from the client side, but the netcf_if for each 
> interface is needed, and I guess a cookie representing it will be sent 
> to the client side and stored in the _virInterface. Is that what the 
> UUID in _virNetwork is used for? (I know, I know - "read the code!", 
> it's just quicker to (and avoids misreading on my part) to ask)

All use of netcf data structures would be internal to the specific
libvirt driver implementation, eg the src/interface_netcf_driver.c
virInterfacePtr struct should contain whatever unique identifiers 
are used in the public API. Typically name + UUID is sufficient
for most of our APIs. So given a virInterffacePtr you should be
able to lookup a corresponding netcf data structure based on either
UUID or name - whichever works best. 


> +/*
> + * Lookup interface by name
> + */
> +virInterfacePtr         virInterfaceLookupByName  (virConnectPtr conn,
> +                                                   const char *name);
> +                                                   const char *uuid);

There shouldn't be a uuid parameter here - instead have a 2nd method call

 +virInterfacePtr         virInterfaceLookupByUUID  (virConnectPtr conn,
 +                                                   const unsigned char *uuid);

Also by convention, we always have an equivalent lookup which takes a UUID
string, instead of the raw binary UUID format:

 +virInterfacePtr         virInterfaceLookupByUUIDString (virConnectPtr conn,
 +                                                        const char *uuid);


> +
> +/*
> + * Define interface (or modify existing interface configuration)
> + */
> +virInterfacePtr         virInterfaceDefineXML     (virConnectPtr conn,
> +                                                   const char *xmlDesc);
> +
> +/*
> + * Delete interface
> + */
> +int                     virInterfaceUndefine      (virInterfacePtr interface);
> +
> +/*
> + * Activate interface (ie call "ifup")
> + */
> +int                     virInterfaceStart         (virInterfacePtr interface);
> +
> +/*
> + * De-activate interface (call "ifdown")
> + */
> +int                     virInterfaceStop          (virInterfacePtr interface);

I'd prefer it if these were called

   virInterfaceCreate(virInterfacePtr)      (for ifup)
   virInterfaceDestory(virInterafacePtr)     (for ifdown)

just so we follow the naming conventions used in the rest of the public APIs.
It makes life easier on users / developers, so know that the same nouns are
used for the same operations  on each object.

To support transient networks, there's also be a use case for an API
taking an XML file & calling ifup on the definition immediately
without writing out an ifcfg-XXX file. eg

    virInterfaceCreateXML(virConnectPtr con,
                          const char *xmlDesc)

> +/*
> + * Interface information
> + */
> +const char*             virInterfaceGetName       (virInterfacePtr interface);

You'd also want equivalent for UUID query
5B
  int                     virInterfaceGetUUID        (virInterfacePtr interface,
                                                      unsigned char *uuid);
  int                     virInterfaceGetUUIDString  (virInterfacePtr interface,
                                                      char *buf);

>  
> +char *                  virInterfaceGetXMLDesc    (virInterfacePtr interface,
> +                                                   int flags);

In summary, this all looks pretty sensible & as I'd imagined it would
turn out...

For extra bonus points, you'll wnat to consider adding a method to query
ther network interfaces I/O stats, eg very similar for virDomainInterfaceStats

     int virInterfaceStats (virInterfacePtr interface,
                            virInterfaceStatsPtr stats,
                            size_t size);

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


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