Re: [PATCH] conf: Generate MAC address instead of keeping all zeroes

On 9/1/23 17:12, Martin Kletzander wrote: > When we parse <mac address="00:00:00:00:00:00"/> we keep that in memory > and pass it down to the hypervisor. However, that MAC address is not > strictly valid as it is not marked as locally administered (bit 0x02) > but it is not even globally unique. It is also used for loopback device > on Linux, for example. And QEMU sees such MAC address just as "not > specified" and generates a new one that libvirt does not even know > about. So to make the overall experience better we now generate it if > the supplied one is all clear. Please consider s/ / /g

> > Resolves: https://issues.redhat.com/browse/RHEL-974 > > Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; > --- > src/conf/domain_conf.c | 2 +- > src/util/virmacaddr.c | 5 +++++ > src/util/virmacaddr.h | 1 + > .../network-interface-mac-clear.xml | 21 +++++++++++++++++++ > .../network-interface-mac-clear.xml | 21 +++++++++++++++++++ > tests/genericxml2xmltest.c | 4 +++- > 6 files changed, 52 insertions(+), 2 deletions(-) > create mode 100644 tests/genericxml2xmlindata/network-interface-mac-clear.xml > create mode 100644 tests/genericxml2xmloutdata/network-interface-mac-clear.xml > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index bb4f1fdb948d..652bd09b21b8 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -9675,7 +9675,7 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt, > return NULL; > } > > - if (!macaddr) { > + if (!macaddr || virMacAddrIsAllClear(&def->mac)) { > virDomainNetGenerateMAC(xmlopt, &def->mac); > def->mac_generated = true; > } > diff --git a/src/util/virmacaddr.c b/src/util/virmacaddr.c > index 073f298b5b66..e06bb200fc68 100644 > --- a/src/util/virmacaddr.c > +++ b/src/util/virmacaddr.c > @@ -246,6 +246,11 @@ virMacAddrIsBroadcastRaw(const unsigned char s[VIR_MAC_BUFLEN]) > return memcmp(virMacAddrBroadcastAddrRaw, s, sizeof(*s)) == 0; > } > > +bool virMacAddrIsAllClear(const virMacAddr *addr) > +{ > + return !virMacAddrCmpRaw(addr, (const unsigned char[VIR_MAC_BUFLEN]){0}); > +} > + > void > virMacAddrFree(virMacAddr *addr) > { > diff --git a/src/util/virmacaddr.h b/src/util/virmacaddr.h > index f32b58805a61..7b9eb7443bd1 100644 > --- a/src/util/virmacaddr.h > +++ b/src/util/virmacaddr.h > @@ -58,6 +58,7 @@ int virMacAddrParseHex(const char* str, > bool virMacAddrIsUnicast(const virMacAddr *addr); > bool virMacAddrIsMulticast(const virMacAddr *addr); > bool virMacAddrIsBroadcastRaw(const unsigned char s[VIR_MAC_BUFLEN]); > +bool virMacAddrIsAllClear(const virMacAddr *addr); > void virMacAddrFree(virMacAddr *addr); > > G_DEFINE_AUTOPTR_CLEANUP_FUNC(virMacAddr, virMacAddrFree); Please expose the symbol in src/libvirt_private.syms too.

Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com&gt; Michal