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

Re: [libvirt] [PATCH 00/10] new virNetworkUpdate API



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!"


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