[libvirt] [PATCH] libxl: support interface type=network

[Jim Fehlig]

Laine Stump wrote:
> On 06/07/2014 12:30 AM, Jim Fehlig wrote:
>   
>> Add support for <interface type='network'> in the libxl driver.
>>
>> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
>> ---
>>  src/libxl/libxl_conf.c | 41 +++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 39 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
>> index cec37d6..6efcea6 100644
>> --- a/src/libxl/libxl_conf.c
>> +++ b/src/libxl/libxl_conf.c
>> @@ -908,7 +908,44 @@ libxlMakeNic(virDomainDefPtr def,
>>              if (VIR_STRDUP(x_nic->script, l_nic->script) < 0)
>>                  return -1;
>>              break;
>> -        default:
>> +        case VIR_DOMAIN_NET_TYPE_NETWORK:
>> +        {
>> +            bool error = true;
>> +            char *brname = NULL;
>> +            virNetworkPtr network = NULL;
>> +            virConnectPtr conn;
>> +
>> +            if (!(conn = virConnectOpen("xen:///system")))
>> +                return -1;
>> +
>> +            if (!(network =
>> +                  virNetworkLookupByName(conn, l_nic->data.network.name)))
>> +                goto cleanup_net;
>> +
>> +            if (!(brname = virNetworkGetBridgeName(network)))
>> +                goto cleanup_net;
>>     
>
> This only accounts for the traditional libvirt-managed networks (the
> ones with <forward mode='nat|route'> or no <forward> at all, which use a
> transient bridge device created by libvirt). As long as you're adding
> this support, you may as well add it in a way that you can take
> advantage of the libvirt networks which are just thinly veiled coveres
> over existing bridges, e.g.:
>
>   http://www.libvirt.org/formatnetwork.html#examplesBridge
>
> That can be done by following the example of qemunetworkIfaceConnect().
> In short, instead of looking at l_nic->type, you look at
> virDomainNetGetActualType(l_nic), and do the above code only if *that*
> is VIR_DOMAIN_NET_TYPE_NETWORK. Otherwise, if actualType is
> VIR_DOMAIN_NET_TYPE_BRIDGE, you will want to VIR_STRDUP(x_nic->bridge,
> virDomainNetGetActualBridgeName(l_nic)) (note that this will end up
> accounting for both the case of an <interface type='bridge'> *AND* an
> <interface type='network'> where the network is a wrapper over a
> system-created bridge device).
>   

Thanks for the pointer!  I've adjusted the patch as you've suggested.

> BTW, I notice that because you added the new case at the end, none of
> these network-based connections will use the script from the definition
> as is done with bridge-based connections (yet no error will be logged if
> one exists). Was this intentional?
>   

Yes.  We discussed supporting <script> in the libxl driver a while back
and agreed to only support it for interface types 'ethernet' and 'bridge'

http://www.redhat.com/archives/libvir-list/2013-April/msg00775.html

I've sent a V2 with a second patch that enforces that

http://www.redhat.com/archives/libvir-list/2014-June/msg00460.html

And while writing this, realized I should have just sent patch 2
separately instead of creating the series.

Regards,
Jim