3:44 a.m.
---
v2: Use network level VLAN config if there is no portgroup specific VLAN
config given.
v3: Add ESX_VLAN_TRUNK define for magic number 4095
src/esx/esx_network_driver.c | 70 +++++++++++++++++++++++++++++++++++++++---
1 files changed, 65 insertions(+), 5 deletions(-)
diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index 09d46d3..21eabbe 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -45,6 +45,9 @@
*/
verify(MD5_DIGEST_SIZE == VIR_UUID_BUFLEN);
+/* ESX utilizes the VLAN ID of 4095 to mean that this port is in trunk mode */
+#define ESX_VLAN_TRUNK 4095
+
static virDrvOpenStatus
@@ -489,7 +492,42 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml)
goto cleanup;
}
- hostPortGroupSpec->vlanId->value = 0;
+ if (def->portGroups[i].vlan.trunk) {
+ /* FIXME: Change this once tag-less trunk-mode is supported */
+ if (def->portGroups[i].vlan.nTags != 1 ||
+ *def->portGroups[i].vlan.tag != ESX_VLAN_TRUNK) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("VLAN tag has to be %d in trunk mode"),
+ ESX_VLAN_TRUNK);
+ goto cleanup;
+ }
+
+ hostPortGroupSpec->vlanId->value = ESX_VLAN_TRUNK;
+ } else if (def->portGroups[i].vlan.nTags == 1) {
+ hostPortGroupSpec->vlanId->value = *def->portGroups[i].vlan.tag;
+ } else if (def->portGroups[i].vlan.nTags > 1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Can apply one VLAN tag per port group only"));
+ goto cleanup;
+ } else if (def->vlan.trunk) {
+ /* FIXME: Change this once tag-less trunk-mode is supported */
+ if (def->vlan.nTags != 1 || *def->vlan.tag != ESX_VLAN_TRUNK) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("VLAN tag has to be %d in trunk mode"),
+ ESX_VLAN_TRUNK);
+ goto cleanup;
+ }
+
+ hostPortGroupSpec->vlanId->value = ESX_VLAN_TRUNK;
+ } else if (def->vlan.nTags == 1) {
+ hostPortGroupSpec->vlanId->value = *def->vlan.tag;
+ } else if (def->vlan.nTags > 1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Can apply one VLAN tag per port group only"));
+ goto cleanup;
+ } else {
+ hostPortGroupSpec->vlanId->value = 0;
+ }
if (def->portGroups[i].bandwidth != NULL) {
if (esxBandwidthToShapingPolicy
@@ -519,6 +557,8 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml)
network = virGetNetwork(conn, hostVirtualSwitch->name, md5);
cleanup:
+ /* FIXME: need to remove virtual switch if adding port groups failed */
+
virNetworkDefFree(def);
esxVI_HostVirtualSwitch_Free(&hostVirtualSwitch);
esxVI_HostPortGroup_Free(&hostPortGroupList);
@@ -695,6 +735,7 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags)
esxVI_String *hostPortGroupKey = NULL;
esxVI_String *networkName = NULL;
virNetworkDefPtr def;
+ virPortGroupDefPtr portGroup;
if (esxVI_EnsureSession(priv->primary) < 0) {
return NULL;
@@ -824,9 +865,12 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags)
for (networkName = networkNameList; networkName != NULL;
networkName = networkName->_next) {
if (STREQ(networkName->value,
hostPortGroup->spec->name)) {
- def->portGroups[def->nPortGroups].name =
strdup(networkName->value);
+ portGroup = &def->portGroups[def->nPortGroups];
+ ++def->nPortGroups;
+
+ portGroup->name = strdup(networkName->value);
- if (def->portGroups[def->nPortGroups].name == NULL) {
+ if (portGroup->name == NULL) {
virReportOOMError();
goto cleanup;
}
@@ -834,13 +878,29 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags)
if (hostPortGroup->spec->policy != NULL) {
if (esxShapingPolicyToBandwidth
(hostPortGroup->spec->policy->shapingPolicy,
-
&def->portGroups[def->nPortGroups].bandwidth) < 0) {
+ &portGroup->bandwidth) < 0) {
++def->nPortGroups;
goto cleanup;
}
}
- ++def->nPortGroups;
+ if (hostPortGroup->spec->vlanId->value > 0) {
+ if (hostPortGroup->spec->vlanId->value ==
ESX_VLAN_TRUNK) {
+ portGroup->vlan.trunk = true;
+ }
+
+ /* FIXME: Remove this once tag-less trunk-mode is
supported */
+ portGroup->vlan.nTags = 1;
+
+ if (VIR_ALLOC_N(portGroup->vlan.tag, 1) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ *portGroup->vlan.tag =
+ hostPortGroup->spec->vlanId->value;
+ }
+
break;
}
}
--
1.7.4.1
2:24 p.m.
2012/9/9 Matthias Bolte <matthias.bolte(a)googlemail.com>:
---
v2: Use network level VLAN config if there is no portgroup specific VLAN
config given.
v3: Add ESX_VLAN_TRUNK define for magic number 4095
src/esx/esx_network_driver.c | 70 +++++++++++++++++++++++++++++++++++++++---
1 files changed, 65 insertions(+), 5 deletions(-)
*push* for a final review.
--
Matthias Bolte
http://photron.blogspot.com
2:50 p.m.
On 09/09/2012 04:44 AM, Matthias Bolte wrote:
---
v2: Use network level VLAN config if there is no portgroup specific VLAN
config given.
v3: Add ESX_VLAN_TRUNK define for magic number 4095
src/esx/esx_network_driver.c | 70 +++++++++++++++++++++++++++++++++++++++---
1 files changed, 65 insertions(+), 5 deletions(-)
diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index 09d46d3..21eabbe 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -45,6 +45,9 @@
*/
verify(MD5_DIGEST_SIZE == VIR_UUID_BUFLEN);
+/* ESX utilizes the VLAN ID of 4095 to mean that this port is in trunk mode */
+#define ESX_VLAN_TRUNK 4095
+
static virDrvOpenStatus
@@ -489,7 +492,42 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml)
goto cleanup;
}
- hostPortGroupSpec->vlanId->value = 0;
+ if (def->portGroups[i].vlan.trunk) {
+ /* FIXME: Change this once tag-less trunk-mode is supported */
I've lost the context (if there was any) of this comment. Is this
something that libvirt needs to support? Or ESX?
+ if (def->portGroups[i].vlan.nTags != 1 ||
+ *def->portGroups[i].vlan.tag != ESX_VLAN_TRUNK) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("VLAN tag has to be %d in trunk mode"),
+ ESX_VLAN_TRUNK);
This seems a bit odd. If ESX requires setting the vlan tag to 4095 to
indicate trunk mode, why not just have trunk='yes' imply a tag id of
4095? While not exactly the same as trunk mode for Open vSwitch, it
seems closer at least. (hmm, of course I think currently the parser
requires at least one tag id, even for trunk='yes'. Maybe that should be
relaxed in the parser, and enforced in each driver as necessary? Is that
what the comment above is referring to? If so, I would say "go for it" :-))
+ goto cleanup;
+ }
+
+ hostPortGroupSpec->vlanId->value = ESX_VLAN_TRUNK;
+ } else if (def->portGroups[i].vlan.nTags == 1) {
+ hostPortGroupSpec->vlanId->value = *def->portGroups[i].vlan.tag;
+ } else if (def->portGroups[i].vlan.nTags > 1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Can apply one VLAN tag per port group only"));
+ goto cleanup;
+ } else if (def->vlan.trunk) {
+ /* FIXME: Change this once tag-less trunk-mode is supported */
+ if (def->vlan.nTags != 1 || *def->vlan.tag !=
ESX_VLAN_TRUNK) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("VLAN tag has to be %d in trunk mode"),
+ ESX_VLAN_TRUNK);
+ goto cleanup;
+ }
+
+ hostPortGroupSpec->vlanId->value = ESX_VLAN_TRUNK;
+ } else if (def->vlan.nTags == 1) {
+ hostPortGroupSpec->vlanId->value = *def->vlan.tag;
+ } else if (def->vlan.nTags > 1) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Can apply one VLAN tag per port group only"));
+ goto cleanup;
+ } else {
+ hostPortGroupSpec->vlanId->value = 0;
+ }
if (def->portGroups[i].bandwidth != NULL) {
if (esxBandwidthToShapingPolicy
@@ -519,6 +557,8 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml)
network = virGetNetwork(conn, hostVirtualSwitch->name, md5);
cleanup:
+ /* FIXME: need to remove virtual switch if adding port groups failed */
+
virNetworkDefFree(def);
esxVI_HostVirtualSwitch_Free(&hostVirtualSwitch);
esxVI_HostPortGroup_Free(&hostPortGroupList);
@@ -695,6 +735,7 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags)
esxVI_String *hostPortGroupKey = NULL;
esxVI_String *networkName = NULL;
virNetworkDefPtr def;
+ virPortGroupDefPtr portGroup;
if (esxVI_EnsureSession(priv->primary) < 0) {
return NULL;
@@ -824,9 +865,12 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags)
for (networkName = networkNameList; networkName != NULL;
networkName = networkName->_next) {
if (STREQ(networkName->value,
hostPortGroup->spec->name)) {
- def->portGroups[def->nPortGroups].name =
strdup(networkName->value);
+ portGroup = &def->portGroups[def->nPortGroups];
+ ++def->nPortGroups;
+
+ portGroup->name = strdup(networkName->value);
- if (def->portGroups[def->nPortGroups].name == NULL) {
+ if (portGroup->name == NULL) {
virReportOOMError();
goto cleanup;
}
@@ -834,13 +878,29 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags)
if (hostPortGroup->spec->policy != NULL) {
if (esxShapingPolicyToBandwidth
(hostPortGroup->spec->policy->shapingPolicy,
-
&def->portGroups[def->nPortGroups].bandwidth) < 0) {
+ &portGroup->bandwidth) < 0) {
++def->nPortGroups;
goto cleanup;
}
}
- ++def->nPortGroups;
+ if (hostPortGroup->spec->vlanId->value > 0) {
+ if (hostPortGroup->spec->vlanId->value ==
ESX_VLAN_TRUNK) {
+ portGroup->vlan.trunk = true;
+ }
+
+ /* FIXME: Remove this once tag-less trunk-mode is
supported */
+ portGroup->vlan.nTags = 1;
+
+ if (VIR_ALLOC_N(portGroup->vlan.tag, 1) < 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ *portGroup->vlan.tag =
+ hostPortGroup->spec->vlanId->value;
+ }
+
break;
}
}
It all looks fine to me (although I can't check it myself), but I think
if there's a simple solution to automatically setting the tag id sent to
ESX to "ESX_VLAN_TRUNK" just by setting "trunk='yes'", better
to do that
now than later, saving the backward compatibility headaches (and it's
okay with me if the parser is changed to allow 0 tags when trunk='yes',
as long as openvswitch's (and hostdev's) use of vlan are updated to
disallow 0 tags even for trunk='yes' (if they don't already - I haven't
checked).
If I'm misunderstanding the issue and this can't be easily done, then
ACK as-is.
12:13 p.m.
2012/9/30 Laine Stump <laine(a)laine.org>:
On 09/09/2012 04:44 AM, Matthias Bolte wrote:
> ---
>
> v2: Use network level VLAN config if there is no portgroup specific VLAN
> config given.
>
> v3: Add ESX_VLAN_TRUNK define for magic number 4095
>
> src/esx/esx_network_driver.c | 70 +++++++++++++++++++++++++++++++++++++++---
> 1 files changed, 65 insertions(+), 5 deletions(-)
>
> diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
> index 09d46d3..21eabbe 100644
> --- a/src/esx/esx_network_driver.c
> +++ b/src/esx/esx_network_driver.c
> @@ -45,6 +45,9 @@
> */
> verify(MD5_DIGEST_SIZE == VIR_UUID_BUFLEN);
>
> +/* ESX utilizes the VLAN ID of 4095 to mean that this port is in trunk mode */
> +#define ESX_VLAN_TRUNK 4095
> +
>
>
> static virDrvOpenStatus
> @@ -489,7 +492,42 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml)
> goto cleanup;
> }
>
> - hostPortGroupSpec->vlanId->value = 0;
> + if (def->portGroups[i].vlan.trunk) {
> + /* FIXME: Change this once tag-less trunk-mode is supported */
I've lost the context (if there was any) of this comment. Is this
something that libvirt needs to support? Or ESX?
libvirt.
> + if (def->portGroups[i].vlan.nTags != 1 ||
> + *def->portGroups[i].vlan.tag != ESX_VLAN_TRUNK) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("VLAN tag has to be %d in trunk mode"),
> + ESX_VLAN_TRUNK);
This seems a bit odd. If ESX requires setting the vlan tag to 4095 to
indicate trunk mode, why not just have trunk='yes' imply a tag id of
4095? While not exactly the same as trunk mode for Open vSwitch, it
seems closer at least. (hmm, of course I think currently the parser
requires at least one tag id, even for trunk='yes'. Maybe that should be
relaxed in the parser, and enforced in each driver as necessary? Is that
what the comment above is referring to? If so, I would say "go for it" :-))
That's the point. Currently libvirt requires at least one tag per
VLAN. An here's the patch that relaxes that:
https://www.redhat.com/archives/libvir-list/2012-October/msg00214.html
And here's v2 of this patch:
https://www.redhat.com/archives/libvir-list/2012-October/msg00215.html
--
Matthias Bolte
http://photron.blogspot.com
4464
days inactive
4492
days old
3 comments
2 participants
participants (2)
-
Laine Stump -
Matthias Bolte