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@suse.com> --- 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; }