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, ) < 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, ) < 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
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, ) < 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, ) < 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, ) < 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, ) < 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(, ",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(, ":%d", virt_vlan->tag[i]);
+} else {
+virBufferAsprintf(,
