[libvirt PATCH 0/3] vmx: fix handling of VMX MAC address options

Daniel P. Berrangé posted 3 patches 3 years, 8 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200720163217.1796949-1-berrange@redhat.com
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
[libvirt PATCH 0/3] vmx: fix handling of VMX MAC address options
Posted by Daniel P. Berrangé 3 years, 8 months ago
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

-- 
2.24.1

Re: [libvirt PATCH 0/3] vmx: fix handling of VMX MAC address options
Posted by Daniel P. Berrangé 3 years, 8 months ago
ping, I think we need  to get this series merged before the release
freeze, or revert the original broken vmx canges.

On Mon, Jul 20, 2020 at 05:32:14PM +0100, 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
> 
> -- 
> 2.24.1
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [libvirt PATCH 0/3] vmx: fix handling of VMX MAC address options
Posted by Michal Privoznik 3 years, 8 months ago
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@redhat.com>

Michal