5:30 a.m.
Add support for <interface type='network'> in the libxl driver.
Signed-off-by: Jim Fehlig <jfehlig(a)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;
+
+ if (VIR_STRDUP(x_nic->bridge, brname) < 0)
+ goto cleanup_net;
+
+ error = false;
+
+ cleanup_net:
+ VIR_FREE(brname);
+ virNetworkFree(network);
+ virObjectUnref(conn);
+ if (error)
+ return -1;
+ break;
+ }
+ case VIR_DOMAIN_NET_TYPE_USER:
+ case VIR_DOMAIN_NET_TYPE_SERVER:
+ case VIR_DOMAIN_NET_TYPE_CLIENT:
+ case VIR_DOMAIN_NET_TYPE_MCAST:
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ case VIR_DOMAIN_NET_TYPE_DIRECT:
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ case VIR_DOMAIN_NET_TYPE_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("libxenlight does not support network device type %s"),
virDomainNetTypeToString(l_nic->type));
@@ -919,7 +956,7 @@ libxlMakeNic(virDomainDefPtr def,
}
static int
-libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config)
+libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config)
{
virDomainNetDefPtr *l_nics = def->nets;
size_t nnics = def->nnets;
--
1.8.0.1
1:20 p.m.
Jim Fehlig wrote:
Add support for <interface type='network'> in the libxl
driver.
Signed-off-by: Jim Fehlig <jfehlig(a)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;
+
+ if (VIR_STRDUP(x_nic->bridge, brname) < 0)
+ goto cleanup_net;
+
+ error = false;
+
+ cleanup_net:
+ VIR_FREE(brname);
+ virNetworkFree(network);
While testing this patch a bit more, noticed that freeing the
virNetworkPtr here swallowed any previous errors. E.g. when specifying
a non-existent network, I got the following error
error: invalid network pointer in virNetworkFree
With the below squashed in, I get the more reasonable
error: Network not found: no network with matching name 'foobar'
Regards,
Jim
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 6efcea6..195bacc 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -910,31 +910,37 @@ libxlMakeNic(virDomainDefPtr def,
break;
case VIR_DOMAIN_NET_TYPE_NETWORK:
{
- bool error = true;
+ bool fail = false;
char *brname = NULL;
- virNetworkPtr network = NULL;
+ virNetworkPtr network;
virConnectPtr conn;
+ virErrorPtr errobj;
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;
-
- if (VIR_STRDUP(x_nic->bridge, brname) < 0)
- goto cleanup_net;
+ virNetworkLookupByName(conn,
l_nic->data.network.name))) {
+ virObjectUnref(conn);
+ return -1;
+ }
- error = false;
+ if ((brname = virNetworkGetBridgeName(network))) {
+ if (VIR_STRDUP(x_nic->bridge, brname) < 0)
+ fail = true;
+ } else {
+ fail = true;
+ }
- cleanup_net:
VIR_FREE(brname);
+
+ /* Preserve any previous failure */
+ errobj = virSaveLastError();
virNetworkFree(network);
+ virSetError(errobj);
+ virFreeError(errobj);
virObjectUnref(conn);
- if (error)
+ if (fail)
return -1;
break;
}
7:10 p.m.
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>
---
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?
+
+ if (VIR_STRDUP(x_nic->bridge, brname) < 0)
+ goto cleanup_net;
+
+ error = false;
+
+ cleanup_net:
+ VIR_FREE(brname);
+ virNetworkFree(network);
+ virObjectUnref(conn);
+ if (error)
+ return -1;
+ break;
+ }
+ case VIR_DOMAIN_NET_TYPE_USER:
+ case VIR_DOMAIN_NET_TYPE_SERVER:
+ case VIR_DOMAIN_NET_TYPE_CLIENT:
+ case VIR_DOMAIN_NET_TYPE_MCAST:
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ case VIR_DOMAIN_NET_TYPE_DIRECT:
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ case VIR_DOMAIN_NET_TYPE_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("libxenlight does not support network device type %s"),
virDomainNetTypeToString(l_nic->type));
@@ -919,7 +956,7 @@ libxlMakeNic(virDomainDefPtr def,
}
static int
-libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config)
+libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config)
{
virDomainNetDefPtr *l_nics = def->nets;
size_t nnics = def->nnets;
5:38 a.m.
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(a)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
3799
days inactive
3803
days old
3 comments
2 participants
participants (2)
-
Jim Fehlig -
Laine Stump