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

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(a)laine.org&gt; > --- > 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.