On 7/20/20 6:32 PM, Daniel P. Berrangé wrote:
We first had a proposal to add a "check" attribute for MAC addresses https://www.redhat.com/archives/libvir-list/2020-July/msg00617.html For reasons I don't understand this was then replaced by a "type" attribute https://www.redhat.com/archives/libvir-list/2020-July/msg00656.html with the "type" attribute having the side-effect of changing the VMX checkMACAddress config. See the first patch for more detailed description of the flaws. The core problem with the original VMX code before either of these patches was that we have multiple distinct VMX config settings and they were all being overloaded into a single MAC address attribute in the XML. This overloading is inherantly loosing information so cannot be reliably round-trippped. The only way to solve this is to actually have explicit attributes for the config parameters from VMX and stop overloading things. IOW, we needed *both* the "check" and "type" attributes. That is what this series does. It also adds the missing VMX -> XML conversion step so we round-trip everything. There are still some pieces that are not perfect. - libvirt has type=static|generated, but VMX has type=static|generated|vpx. "vpx" is just a synonym for "generated", but using a different MAC addr range. We use the range to do the mapping, and can probablylive with that. - VMX has a a address offset field - I've not found any info on what this is needed for, but we hardcode it to 0 for XML -> VMX config, and ignore it entirely for VMX -> XML config. This means we won't roundtrip this setting. This needs fixing if anyone expects we'll see offset != 0 in the real world. Bastien Orivel (1): Add a check attribute on the mac address element Daniel P. Berrangé (2): vmx: fix logic handling mac address type vmx: support outputing the type attribute for MAC addresses docs/schemas/domaincommon.rng | 5 ++ src/conf/domain_conf.c | 14 ++++ src/conf/domain_conf.h | 1 + src/vmx/vmx.c | 77 ++++++++++++++----- .../network-interface-mac-check.xml | 29 +++++++ tests/genericxml2xmltest.c | 2 + .../vmx2xml-case-insensitive-1.xml | 2 +- .../vmx2xml-case-insensitive-2.xml | 2 +- .../vmx2xmldata/vmx2xml-esx-in-the-wild-1.xml | 2 +- .../vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml | 2 +- .../vmx2xmldata/vmx2xml-esx-in-the-wild-3.xml | 2 +- .../vmx2xmldata/vmx2xml-esx-in-the-wild-4.xml | 4 +- .../vmx2xmldata/vmx2xml-esx-in-the-wild-5.xml | 2 +- .../vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml | 2 +- .../vmx2xmldata/vmx2xml-esx-in-the-wild-7.xml | 2 +- .../vmx2xmldata/vmx2xml-esx-in-the-wild-8.xml | 20 ++--- .../vmx2xmldata/vmx2xml-esx-in-the-wild-9.xml | 2 +- .../vmx2xmldata/vmx2xml-ethernet-bridged.xml | 2 +- tests/vmx2xmldata/vmx2xml-ethernet-custom.xml | 2 +- tests/vmx2xmldata/vmx2xml-ethernet-e1000.xml | 2 +- .../vmx2xml-ethernet-generated.xml | 2 +- tests/vmx2xmldata/vmx2xml-ethernet-nat.xml | 2 +- tests/vmx2xmldata/vmx2xml-ethernet-other.xml | 2 +- tests/vmx2xmldata/vmx2xml-ethernet-static.xml | 2 +- .../vmx2xmldata/vmx2xml-ethernet-vmxnet2.xml | 2 +- tests/vmx2xmldata/vmx2xml-ethernet-vpx.xml | 2 +- .../vmx2xml-fusion-in-the-wild-1.xml | 4 +- .../vmx2xmldata/vmx2xml-gsx-in-the-wild-1.xml | 2 +- .../vmx2xmldata/vmx2xml-gsx-in-the-wild-2.xml | 2 +- .../vmx2xmldata/vmx2xml-gsx-in-the-wild-3.xml | 4 +- .../vmx2xmldata/vmx2xml-gsx-in-the-wild-4.xml | 2 +- .../vmx2xmldata/vmx2xml-ws-in-the-wild-1.xml | 2 +- .../vmx2xmldata/vmx2xml-ws-in-the-wild-2.xml | 2 +- .../xml2vmxdata/xml2vmx-ethernet-mac-type.vmx | 7 +- .../xml2vmxdata/xml2vmx-ethernet-mac-type.xml | 4 +- 35 files changed, 154 insertions(+), 63 deletions(-) create mode 100644 tests/genericxml2xmlindata/network-interface-mac-check.xml
Reviewed-by: Michal Privoznik <mprivozn@xxxxxxxxxx> Michal
