On 09/17/2012 02:04 PM, Daniel P. Berrange wrote:
> On Mon, Sep 17, 2012 at 05:48:45AM -0400, Laine Stump wrote:
>> This patchset implements a new API function called virNetworkUpdate
>> which enables updating certain parts of a libvirt network's definition
>> without the need to destroy/re-start the network. This is especially
>> useful, for example, to add/remove hosts from the dhcp static hosts
>> table, or change portgroup settings.
>>
>> This was previously discussed in this thread:
>>
>> https://www.redhat.com/archives/libvir-list/2012-August/msg01535.html
>>
>> continuing here in September:
>>
>> https://www.redhat.com/archives/libvir-list/2012-September/msg00328.html
>>
>> with the final form here:
>>
>> https://www.redhat.com/archives/libvir-list/2012-September/msg00465.html
>>
>> In short, the single function has a "section" specifier which tells
>> the part of the network definition to be updated, a "parentIndex" that
>> gives the index of the *parent* element containing this section (when
>> there are multiples - in particular in the case of the <ip> element),
>> and a fully formed XML element which will be added as-is in the case
>> of VIR_NETWORK_UPDATE_ADD_* (after checking for a duplicate), used to
>> search for the specific element to delete in case of
>> VIR_NETWORK_UPDATE_DELETE, and used both to find the existing element
>> and replace its current contents in the case of VIR_UPDATE_EXISTING
>> (this implies that you can't change the change the attribute used for
>> indexing, e.g. the name of a portgroup, or mac address of a dhcp host
>> entry).
>>
>> An example of use: to add a dhcp host entry to network "net", you would do
>> this:
>>
>> virNetworkUpdate(net, VIR_NETWORK_SECTION_IP_DHCP_HOST, -1,
>> "<host mac='00:11:22:33:44:55' ip='192.168.122.5'/>",
>> VIR_NETWORK_UPDATE_AFFECT_LIVE
>> | VIR_NETWORK_UPDATE_AFFECT_CONFIG
>> | VIR_NETWORK_UPDATE_ADD_LAST);
>>
>> To delete that same entry:
>>
>> virNetworkUpdate(net, VIR_NETWORK_SECTION_IP_DHCP_HOST, -1,
>> "<host mac='00:11:22:33:44:55'/>",
>> VIR_NETWORK_UPDATE_AFFECT_LIVE
>> | VIR_NETWORK_UPDATE_AFFECT_CONFIG
>> | VIR_NETWORK_UPDATE_DELETE);
>>
>> If you wanted to force any of these to affect the dhcp host list in
>> the 3rd <ip> element of the network, you would replace "-1" with "2".
>>
>> Another example: to modify the portgroup named "engineering" (e.g. to
>> increase the inbound average bandwidth from 1000 to 2000):
>>
>> virNetworkUpdate(net, VIR_NETWORK_SECTION_PORTGROUP, -1,
>> "<portgroup name='engineering' default='yes'>"
>> " <virtualport type='802.1Qbh'>"
>> " <parameters profileid='test'/>"
>> " </virtualport>"
>> " <bandwidth>"
>> " <inbound average='2000' peak='5000' burst='5120'/>"
>> " <outbound average='1000' peak='5000'
>> burst='5120'/>"
>> " </bandwidth>"
>> "</portgroup>",
>> VIR_NETWORK_UPDATE_EXISTING | VIR_NETWORK_UPDATE_LIVE
>> | VIR_NETWORK_UPDATE_CONFIG)
>>
>> (note that parentIndex is irrelevant for PORTGROUP, since they are in
>> the toplevel of <network>, so there aren't multiple instances of
>> parents. In such cases, the caller *must* set parentIndex to -1 or 0 -
>> any other value indicates that they don't understand the purpose/usage
>> of parentIndex, so it must result in an error. Also note that the
>> above function would fail if it couldn't find an existing portgroup
>> with name='engineering' (i.e. it wouldn't automatically add a new one).)
> I've been trying to think about how this might all map into the
> LibvirtGObject/LibvirtGConfig APIs, and the thing I'm struggling
> with is the parentIndex parameter.
>
> First of all, in the GConfig API I won't be exposing the virNetworkUpdate
> API as it is. To be typesafe, there will be separate APIs for each possible
> operation. eg
>
> gvir_network_add_portgroup
> gvir_network_remove_portgroup
> gvir_network_update_portgroup
>
>
> Consider how the <network> schema will eventually map into objects,
>
> <network> == GVirConfigNetwork
> <name>test1</name>
> <uuid>2d39a0ba-ac4b-6097-114c-50f8bccc277c</uuid>
> <forward mode='bridge'/>
> <bridge name='virbr5' stp='on' delay='0' />
> <mac address='52:54:00:38:81:4D'/>
> <domain name='example.com'/>
> <forward mode='private'/>
> <interface dev="eth20"/> == GVirConfigNetworkInterface
> <interface dev="eth21"/> == GVirConfigNetworkInterface
> <interface dev="eth22"/> == GVirConfigNetworkInterface
> <interface dev="eth23"/> == GVirConfigNetworkInterface
> <interface dev="eth24"/> == GVirConfigNetworkInterface
> </forward>
> <portgroup name='engineering' default='yes'> == GVirConfigNetworkPortGroup
> <virtualport type='802.1Qbh'>
> <parameters profileid='test'/>
> </virtualport>
> <bandwidth>
> <inbound average='1000' peak='5000' burst='5120'/>
> <outbound average='1000' peak='5000' burst='5120'/>
> </bandwidth>
> </portgroup>
> <portgroup name='sales'> == GVirConfigNetworkPortGroup
> <virtualport type='802.1Qbh'>
> <parameters profileid='salestest'/>
> </virtualport>
> <bandwidth>
> <inbound average='500' peak='2000' burst='2560'/>
> <outbound average='128' peak='256' burst='256'/>
> </bandwidth>
> </portgroup>
> <dns> == GVirConfigNetworkDNS
> <txt name="example" value="example value" /> == GVirConfigNetworkDNSEntry
> <srv service='name' protocol='tcp' domain='test-domain-name' target='.'
> port='1024' priority='10' weight='10'/> == GVirConfigNetworkDNSEntry
> <host ip='192.168.122.2'> == GVirConfigNetworkDNSEntry
> <hostname>myhost</hostname>
> <hostname>myhostalias</hostname>
> </host>
> </dns>
> <ip address='10.24.75.1' netmask='255.255.255.0'> ==
> GVirConfigNetworkAddress
> <dhcp> == GVirConfigNetworkDHCP
> <range start='10.24.75.128' end='10.24.75.254' />
> <host mac='52:54:3e:77:e2:ed' name='X.example.com' ip='10.24.75.10' />
> == GVirConfigNetworkDHCPHost
> <host mac='52:54:3e:77:e2:ef' name='Y.example.com' ip='10.24.75.11' />
> <host mac='52:54:34:77:e2:f0' name='Z.example.com' ip='10.24.75.12' />
> <host mac='52:54:3e:77:e2:f1' name='A.example.com' ip='10.24.75.13' />
> </dhcp>
> </ip>
> <ip address='192.168.4.1' netmask='255.255.255.0'/> ==
> GVirConfigNetworkAddress
> </network>
>
>
> So for example we get the config object using
>
> GVirNetwork *net = gvir_connection_get_network_by_name("default");
> GVirConfigNetwork *conf = gvir_network_get_config(net);
>
>
> Now we want to remove all portgroups. This is easy enough - I'd have
> an API like
>
> GList *groups = gvir_config_network_get_portgroups(conf);
>
> while (groups) {
> GVirConfigNetworkPortgroup *group = groups->data;
>
> gvir_network_remove_portgroup(net, group);
>
> data = data->next;
> }
>
>
> As you say, the concept of parentIndex doesn't make sense for portgroups,
> so I'll just ignore it in the API I expose.
>
>
> Likewise adding/removing <interface> from <forward>, we just ignore the
> parentIndex. Modify doesn't make sense for this part of the schema
> since there are no attributes to change, beyond the interface name.
>
> DNS entries are reasonably easy to deal with add/remove, since again
> parentIndex is irrelevant, there only being one <dns> block.
>
> I'm a little fuzzy on whether a modify action is practical for DNS
> entries, since the thing you'd want to change is probably also the
> thing the API would want to use as the unique identifier. The only
> way around this I see is to pass in both the original and new XML
> for the DNS entry being modified. The original XML is used to lookup
> which entry is being mdified, and then replace with the new XML.
> Perhaps this doesn't matter, and add+remove is sufficient for DNS.
>
>
> The updating of DHCP entries is the interesting / hard one that causes
> the fun with parentIndex.
>
> It is possible to come up with a mapping to GObject,
>
> GList *addrs = gvir_config_network_get_addresses(conf);
What if the private data for each address in this list had an index
stored in it by gvir_config_network_get_addresses()...
> int idx = 0;
>
> while (addrs) {
> GVirConfigNetworkAddress *addr = addrs->data;
>
> GVirConfigNetworkDHCP *dhcp =
> gvir_config_network_address_get_dhcp(addr);
...and the private data for dhcp had the same index set (by
gvir_config_network_address_get_dhcp() grabbing it from the addr's
private data)...
> GList *hosts = gvir_config_network_dhcp_get_hosts(dhcp);
...and finally, the private data of each host would have an idx that is
initialized from the dhcp's private data during
gvir_config_network_dhcp_get_hosts().
> while (hosts) {
> GVirConfigNeworkDHCPHost *host = hosts->data;
>
> gvir_network_remove_dhcp_host(net, idx, host);
So now when you get to here, gvir_network_remove_dhcp_host only needs
(net, host), because host->idx (which is private) is already set.
> }
>
> idx++;
> data = data->next;
> }
>
> What I don't like is that the user has to maintain this 'idx' counter
> value. It doesn't hurt in this example, but consider if you were just
> passed a single "GVirConfigNetworkAddress" object, and wanted to add a
> host entry to it. You have no idea what the parentIndex this corresponds
> to.
I think storing the ip index in the private data and passing it down the
hierarchy would work (until someone inserted a new <ip> element
somewhere in the middle :-/)
> This isn't fatal, but it is slightly unpleasant. I don't have any
> better idea though, so I guess we'll just go with what you designed.
Sigh. To paraphrase Hannibal Smith "I *hate* it when a plan doesn't come
together!"
--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list
