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

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(a)suse.com&gt; > --- > 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).

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?