On 12/6/18 12:44 AM, Michal Privoznik wrote:
On 11/16/18 11:26 PM, Jim Fehlig wrote:
Add support for converting openvswitch interface configuration
to/from libvirt domXML and xl.cfg(5). The xl config syntax for
virtual interfaces is described in detail in the
xl-network-configuration(5) man page. The Xen Networking wiki
also contains information and examples for using openvswitch
in xl.cfg config format
https://wiki.xenproject.org/wiki/Xen_Networking#Open_vSwitch
Tests are added to check conversions of openvswitch tagged and
trunked VLAN configuration.
Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
---
src/xenconfig/xen_common.c | 113 +++++++++++++++++-
.../test-fullvirt-ovswitch-tagged.cfg | 25 ++++
.../test-fullvirt-ovswitch-tagged.xml | 50 ++++++++
.../test-fullvirt-ovswitch-trunked.cfg | 25 ++++
.../test-fullvirt-ovswitch-trunked.xml | 51 ++++++++
tests/xlconfigtest.c | 2 +
6 files changed, 262 insertions(+), 4 deletions(-)
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
index 0a9958711f..5390b933e0 100644
--- a/src/xenconfig/xen_common.c
+++ b/src/xenconfig/xen_common.c
@@ -856,6 +856,84 @@ xenParseCharDev(virConfPtr conf, virDomainDefPtr def, const char *nativeFormat)
}
+static int
+xenParseVifBridge(virDomainNetDefPtr net, char *bridge)
+{
+ char *vlanstr;
+ unsigned int tag;
+
+ /* 'bridge' string contains a bridge name and single vlan tag */
+ vlanstr = strchr(bridge, '.');
+ if (vlanstr) {
+ if (VIR_STRNDUP(net->data.bridge.brname, bridge, vlanstr - bridge) < 0)
+ return -1;
+
+ vlanstr++;
+ if (virStrToLong_ui(vlanstr, NULL, 10, &tag) < 0)
+ return -1;
+
+ if (VIR_ALLOC_N(net->vlan.tag, 1) < 0)
+ return -1;
+
+ net->vlan.tag[0] = tag;
+ net->vlan.nTags = 1;
+
+ if (VIR_ALLOC(net->virtPortProfile) < 0)
+ return -1;
+
+ net->virtPortProfile->virtPortType = VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH;
+ return 0;
+ }
+
+ /* 'bridge' string contains a bridge name and one or more vlan trunks */
+ vlanstr = strchr(bridge, ':');
+ if (vlanstr) {
+ size_t i;
+ size_t nvlans = 0;
+ char **vlanstr_list = virStringSplit(bridge, ":", 0);
+
+ if (!vlanstr_list)
+ return -1;
+
+ if (VIR_STRDUP(net->data.bridge.brname, vlanstr_list[0]) < 0) {
+ virStringListFree(vlanstr_list);
+ return -1;
+ }
+
+ for (i = 1; vlanstr_list[i]; i++)
+ nvlans++;
+
+ if (VIR_ALLOC_N(net->vlan.tag, nvlans) < 0) {
+ virStringListFree(vlanstr_list);
+ return -1;
+ }
+
+ for (i = 1; i <= nvlans; i++) {
+ if (virStrToLong_ui(vlanstr_list[i], NULL, 10, &tag) < 0) {
+ virStringListFree(vlanstr_list);
+ return -1;
+ }
+ net->vlan.tag[i - 1] = tag;
+ }
+ net->vlan.nTags = nvlans;
+ net->vlan.trunk = true;
+ virStringListFree(vlanstr_list);
+
+ if (VIR_ALLOC(net->virtPortProfile) < 0)
+ return -1;
+
+ net->virtPortProfile->virtPortType = VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH;
+ return 0;
+ }
+
+ /* 'bridge' string only contains the bridge name */
+ if (VIR_STRDUP(net->data.bridge.brname, bridge) < 0)
+ return -1;
+
+ return 0;
Not a show stopper, I just wonder if we should perhaps use:
if ((vlanstr = strchr(bridge, '.'))) {
...
} else if ((vlanstr = strchr(bridge, ':'))) {
...
} else {
...
}
return 0;
To me that looks a bit cleaner, but I'll leave it up to you. Don't want
to be picky.
It definitely looks cleaner. I'll squash in the below diff before pushing.
Thanks a lot for reviewing the patches!
Regards,
Jim
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
index 83eb28cdc9..60c8d7edc8 100644
--- a/src/xenconfig/xen_common.c
+++ b/src/xenconfig/xen_common.c
@@ -862,9 +862,8 @@ xenParseVifBridge(virDomainNetDefPtr net, char *bridge)
char *vlanstr;
unsigned int tag;
- /* 'bridge' string contains a bridge name and single vlan tag */
- vlanstr = strchr(bridge, '.');
- if (vlanstr) {
+ if ((vlanstr = strchr(bridge, '.'))) {
+ /* 'bridge' string contains a bridge name and single vlan tag */
if (VIR_STRNDUP(net->data.bridge.brname, bridge, vlanstr - bridge) < 0)
return -1;
@@ -883,11 +882,8 @@ xenParseVifBridge(virDomainNetDefPtr net, char *bridge)
net->virtPortProfile->virtPortType = VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH;
return 0;
- }
-
- /* 'bridge' string contains a bridge name and one or more vlan trunks */
- vlanstr = strchr(bridge, ':');
- if (vlanstr) {
+ } else if ((vlanstr = strchr(bridge, ':'))) {
+ /* 'bridge' string contains a bridge name and one or more vlan trunks */
size_t i;
size_t nvlans = 0;
char **vlanstr_list = virStringSplit(bridge, ":", 0);
@@ -924,12 +920,12 @@ xenParseVifBridge(virDomainNetDefPtr net, char *bridge)
net->virtPortProfile->virtPortType = VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH;
return 0;
+ } else {
+ /* 'bridge' string only contains the bridge name */
+ if (VIR_STRDUP(net->data.bridge.brname, bridge) < 0)
+ return -1;
}
- /* 'bridge' string only contains the bridge name */
- if (VIR_STRDUP(net->data.bridge.brname, bridge) < 0)
- return -1;
-
return 0;
}
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
