[libvirt] [PATCH] conf: move 'generated' member from virMacAddr to virDomainNetDef

Michal Privoznik mprivozn at redhat.com
Mon Feb 19 06:23:23 UTC 2018


On 02/16/2018 09:04 PM, Laine Stump wrote:
> Commit 7e62c4cd26d (first appearing in libvirt-3.9.0 as a resolution
> to rhbz #1343919) added a "generated" attribute to virMacAddr that was
> set whenever a mac address was auto-generated by libvirt. This
> knowledge was used in a single place - when trying to match a NetDef
> from the domain to delete with user-provided XML. Since the XML parser
> always auto-generates a MAC address for NetDefs when none is provided,
> it was previously impossible to make a search where the MAC address
> wasn't significant, but the addition of the "generated" attribute made
> it possible for the search function to ignore auto-generated MACs.
> 
> This implementation had a problem though - it was adding a field to a
> "low level" struct - virMacAddr - which is used in other places with
> the assumption that it contains exactly a 6 byte MAC address and
> nothing else. In particular, virNWFilterSnoopEthHdr uses virMacAddr as
> part of the definition of an ethernet packet header, whose layout must
> of course match an actual ethernet packet. Adding the extra bools into
> virNWFilterSnoopEthHdr caused the nwfilter driver's "IP discovery via
> DHCP packet snooping" functionality to mysteriously stop working.
> 
> In order to fix that behavior, and prevent potential future similar
> odd behavior, this patch moves the "generated" member out of
> virMacAddr (so that it is again really just a MAC address) and into
> virDomainNetDef, and sets it only when virDomainNetGenerateMAC() is
> called from virDomainNetDefParseXML() (which is the only time we care
> about it).
> 
> Resolves: https://bugzilla.redhat.com/1529338
> 
> (It should also be applied to any maintenance branch that applies
> commit 7e62c4cd26 and friends to resolve
> https://bugzilla.redhat.com/1343919)
> 
> Signed-off-by: Laine Stump <laine at laine.org>
> ---
>  src/conf/domain_conf.c    | 3 ++-
>  src/conf/domain_conf.h    | 1 +
>  src/util/virmacaddr.c     | 5 -----
>  src/util/virmacaddr.h     | 2 --
>  tests/bhyveargv2xmlmock.c | 1 -
>  5 files changed, 3 insertions(+), 9 deletions(-)

What I am missing here is a comment to _virMacAddr struct saying that
the structure cannot change because of NWFilter code.

ACK with that changed.

Michal




More information about the libvir-list mailing list