Re: [libvirt] [PATCH] esx: Support VLAN tags in virtual network port groups

On 08/18/2012 07:46 AM, Matthias Bolte wrote: > --- > src/esx/esx_network_driver.c | 38 +++++++++++++++++++++++++++++++++----- > 1 files changed, 33 insertions(+), 5 deletions(-) > > diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c > index 09d46d3..2f5f1ab 100644 > --- a/src/esx/esx_network_driver.c > +++ b/src/esx/esx_network_driver.c > @@ -489,7 +489,16 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml) > goto cleanup; > } > > - hostPortGroupSpec->vlanId->value = 0; > + if (def->portGroups[i].vlan.trunk) { > + hostPortGroupSpec->vlanId->value = 4095; > + } 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")); > + } else { > + hostPortGroupSpec->vlanId->value = 0; > + } So in vmware you can specify that a port is to be used for trunking, (and you do that by specifying a tag of 4095), but can't limit the list of tags to allow - is that correct? The parser (and the RNG) currently requires at least one <tag> element in any <vlan> element. It sounds like we need to change that. Kyle or Ansis - does it make sense for Open vSwitch to have a port specified with trunk=yes and no list of specific tags?

Aside from this, you are interpreting the <vlan> element in portgroups, but not the overlying <vlan> for the entire network (in practice this is what would be used if no portgroup was chosen, and no vlan info came from the domain's interface definition). If there isn't any practical way to implement this, you should check for it and log a CONFIG_UNSUPPORTED error if found. > > if (def->portGroups[i].bandwidth != NULL) { > if (esxBandwidthToShapingPolicy > @@ -695,6 +704,7 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags) > esxVI_String *hostPortGroupKey = NULL; > esxVI_String *networkName = NULL; > virNetworkDefPtr def; > + virPortGroupDefPtr portGroup = NULL; > > if (esxVI_EnsureSession(priv->primary) < 0) { > return NULL; > @@ -824,9 +834,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 +847,28 @@ 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 == 4095) { > + portGroup->vlan.trunk = true; > + } > + > + portGroup->vlan.nTags = 1; > + > + if (VIR_ALLOC_N(portGroup->vlan.tag, 1) < 0) { > + virReportOOMError(); > + goto cleanup; > + } > + > + *portGroup->vlan.tag = > + hostPortGroup->spec->vlanId->value; > + } > + > break; > } > }