Re: [libvirt] [PATCH 2/3] xenconfig: add support for openvswitch configuration
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 --- 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
Re: [libvirt] [PATCH 2/3] xenconfig: add support for openvswitch configuration
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 > --- > 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. > +} > + > + > static virDomainNetDefPtr > xenParseVif(char *entry, const char *vif_typename) > { > @@ -974,8 +1052,8 @@ xenParseVif(char *entry, const char *vif_typename) > net->type = VIR_DOMAIN_NET_TYPE_ETHERNET; > } > > -if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { > -if (bridge[0] && VIR_STRDUP(net->data.bridge.brname, bridge) < 0) > +if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE && bridge[0]) { > +if (xenParseVifBridge(net, bridge) < 0) > goto cleanup; > } > if (ip[0]) { Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] xenconfig: add support for openvswitch configuration
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 --- 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; +} + + static virDomainNetDefPtr xenParseVif(char *entry, const char *vif_typename) { @@ -974,8 +1052,8 @@ xenParseVif(char *entry, const char *vif_typename) net->type = VIR_DOMAIN_NET_TYPE_ETHERNET; } -if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { -if (bridge[0] && VIR_STRDUP(net->data.bridge.brname, bridge) < 0) +if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE && bridge[0]) { +if (xenParseVifBridge(net, bridge) < 0) goto cleanup; } if (ip[0]) { @@ -1249,14 +1327,41 @@ xenFormatNet(virConnectPtr conn, switch (net->type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: +{ +virNetDevVPortProfilePtr port_profile = virDomainNetGetActualVirtPortProfile(net); +virNetDevVlanPtr virt_vlan = virDomainNetGetActualVlan(net); +const char *script = net->script; +size_t i; + virBufferAsprintf(&buf, ",bridge=%s", net->data.bridge.brname); +if (port_profile && +port_profile->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) { +if (!script) +script = "vif-openvswitch"; +/* + * libxl_device_nic->bridge supports an extended format for + * specifying VLAN tags and trunks + * + * BRIDGE_NAME[.VLAN][:TRUNK:TRUNK] + */ +if (virt_vlan && virt_vlan->nTags > 0) { +if (virt_vlan->trunk) { +for (i = 0; i < virt_vlan->nTags; i++) +virBufferAsprintf(&buf, ":%d", virt_vlan->tag[i]); +} else { +virBuffer