Re: [libvirt] RFC: take two on live update of network configuration

On Tue, Aug 21, 2012 at 11:26:08PM -0400, Laine Stump wrote: [...] > =========== > OPTION 2) have a separate API for each different type of element we may want to > change, e.g.: > > > virNetworkUpdateForwardInterface(virNetworkPtr network, > const char *xml, > unsigned int flags); > virNetworkUpdatePortgroup(virNetworkPtr network, > const char *xml, > unsigned int flags); > virNetworkUpdateIpDhcpHost(virNetworkPtr network, > const char *xml, > unsigned int flags); > virNetworkUpdateDnsEntry(virNetworkPtr network, > const char *xml, > unsigned int flags); > > /* The name of this one may confuse... */ > virNetworkUpdateDomain(virNetworkPtr network, > const char *xml, > unsigned int flags); > virNetworkUpdateBridge(virNetworkPtr network, > const char *xml, > unsigned int flags); > virNetworkUpdateIpDnsHost(virNetworkPtr network, > const char *xml, > unsigned int flags); > etc. (etc. etc.) > > "flags" would include: > > /* force add if object with matching key doesn't exist */ > VIR_NETWORK_UPDATE_ADD > /* delete existing object(s) that matches the xml */ > VIR_NETWORK_UPDATE_DELETE > /* take effect immediately */ > VIR_NETWORK_UPDATE_LIVE > /* persistent change */ > VIR_NETWORK_UPDATE_CONFIG > > > This method could be problematic, e.g., for something like > virNetworkUpdateIpDhcpHost() - although currently only a single <ip> > element can have a <dhcp> entries, in the future we could allow any/all > of them to have dhcp entries. Another downside is that we would need to > add an new function for each new element we decide we want to be able to > update. that's an awful lot of potential APIs something more generic is needed > =========== > OPTION 3) Again, a single API, but with an added "nodespec" arg which would > be used to describe the parent node you of the node you want added/updated to: > > int virNetworkUpdate(virNetworkPtr network, > const char *nodeSpec, > const char *xml, > unsigned int flags); > > For example, if you wanted to add a new <host> entry to <ip > address='10.24.75.1' ...>'s dhcp element, you would do this: > > /* nodespec obviously uses an XPath expression */ > virNetworkUpdate("/ip[address='10.24.75.1']/dhcp", > /* full text of <host> element to add */ > "<host mac='00:16:3e:77:e2:ed' " > "name='X.example.com' ip='10.24.75.10'/>" > VIR_NETWORK_UPDATE_ADD | VIR_NETWORK_UPDATE_LIVE > | VIR_NETWORK_UPDATE_CONFIG); > > Or, to change the contents of the exiting portgroup "engineering" you > would use: > > virNetworkUpdate("/", > /* full text of portgroup */, > "<portgroup name='engineering'> ..... </portgroup>" > VIR_NETWORK_UPDATE_LIVE|VIR_NETWORK_UPDATE_CONFIG); > > To delete a static dhcp host entry, you would use this: > > virNetworkUpdate("/ip[address='10.24.75.1']/dhcp", shouldn't that be //ip[address='10.24.75.1']/dhcp or your really expect to have ip as the root ?

> /* just enough to match existing entry */ > "<host mac='00:16:3e:77:e2:ed'/>", > VIR_NETWORK_UPDATE_DELETE | VIR_NETWORK_UPDATE_LIVE > | VIR_NETWORK_UPDATE_CONFIG); > > or if you preferred: > > virNetworkUpdate("/ip[address='10.24.75.1']/dhcp", > /* again, just enough to match */ > "<host ip='10.24.75.10'/>", > VIR_NETWORK_UPDATE_DELETE |VIR_NETWORK_UPDATE_LIVE > | VIR_NETWORK_UPDATE_CONFIG); > > To remove an interface from the interface pool: > > virNetworkUpdate("/forward", > "<interface dev='eth20'/>", > VIR_NETWORK_UPDATE_DELETE | VIR_NETWORK_UPDATE_LIVE > | VIR_NETWORK_UPDATE_CONFIG) > > (adding an interface would be identical, except the flag). > > (An alternate implementation would be to have nodeSpec specify the node > that is being updated/removed rather than its parent. This may make more > logical sense, (although not for ADD) and would even make the rest of > the code simpler for update/remove (we wouldn't have to do a manual > search within the node). > > The positive side of this is that the one API function allows you to modify > *anything* (even stuff not yet invented, so it's future-proof). The negative > side is that the code that implements it would need to go to quite a bit of > extra trouble to determine what has been changed (basically, it would > > a) generate XML for the current state of the network, > b) parse that into an xmlNode, > c) perform the operation on xmlNode using nodeset to figure out where, > and adding in the new xml (or removing any nodes matching it), > d) convert modified xmlNode back to xml text, > e) parse the xml text into a virNetworkDef, > f) compare it to the original virNetworkDef to determine what to do. One problem I can see with 3) is that it would work for now but if we were to add namespaces to the XML then the API would not allow to address those elements/attributes //foo in XPath cannot address an element foo with a namespace to do this you need to do //prefix:foo and provide separately the mapping between prefix and the namespace URI So that will work as long as we don't add namespace to the XML

Another problem is making sure we have the addressing complete, for example suppose we have < The other problem is that people who already have a beef with XML usage will see red when they see the need to learn XPath O:-) , but that could be alleviated in a large part with a good set of examples ! Also the problem with Update APIs to XML is that the next step is people will want to add one part (not replace) and then you need to define where to add, before/after ...

XML editing APIs are *hard*, I'm actually a bit worried to go there something like 2/ while potentially leading to more entry points probably makes thing easier for the user.