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

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" :-))