[libvirt] [PATCH 0/2] Simplify MAC address generation on update

https://bugzilla.redhat.com/show_bug.cgi?id=1156367 Ján Tomko (2): Silently ignore MAC in NetworkLoadConfig Generate a MAC when loading a config instead of package update libvirt.spec.in | 42 ------------------------------------------ src/conf/network_conf.c | 8 ++++++++ 2 files changed, 8 insertions(+), 42 deletions(-) -- 2.0.4

Libvirt's RPMs have been adding it to networks which don't support it. https://bugzilla.redhat.com/show_bug.cgi?id=1156367 --- src/conf/network_conf.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 3bad07d..f36be63 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3160,6 +3160,10 @@ virNetworkObjPtr virNetworkLoadConfig(virNetworkObjListPtr nets, */ if (virNetworkSetBridgeName(nets, def, 0)) goto error; + } else { + /* Throw away MAC address for other forward types, + * which could have been generated by older libvirt RPMs */ + def->mac_specified = false; } if (!(net = virNetworkAssignDef(nets, def, false))) -- 2.0.4

Partially reverts commit 5754dbd. The code in the specfile adds a MAC address to every <bridge>, even for <forward mode='bridge'> for which we don't support changing MAC addresses. Remove it completely. For new networks, we have been adding MAC addresses on definition/creation since the commit mentioned above. For existing networks (pre-0.9.0), the MAC is added by the previous commit. https://bugzilla.redhat.com/show_bug.cgi?id=1156367 --- libvirt.spec.in | 42 ------------------------------------------ src/conf/network_conf.c | 4 ++++ 2 files changed, 4 insertions(+), 42 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 6fcaa3e..43b3899 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1606,48 +1606,6 @@ exit 0 %post daemon - %if %{with_network} -# All newly defined networks will have a mac address for the bridge -# auto-generated, but networks already existing at the time of upgrade -# will not. We need to go through all the network configs, look for -# those that don't have a mac address, and add one. - -network_files=$( (cd %{_localstatedir}/lib/libvirt/network && \ - grep -L "mac address" *.xml; \ - cd %{_sysconfdir}/libvirt/qemu/networks && \ - grep -L "mac address" *.xml) 2>/dev/null \ - | sort -u) - -for file in $network_files -do - # each file exists in either the config or state directory (or both) and - # does not have a mac address specified in either. We add the same mac - # address to both files (or just one, if the other isn't there) - - mac4=`printf '%X' $(($RANDOM % 256))` - mac5=`printf '%X' $(($RANDOM % 256))` - mac6=`printf '%X' $(($RANDOM % 256))` - for dir in %{_localstatedir}/lib/libvirt/network \ - %{_sysconfdir}/libvirt/qemu/networks - do - if test -f $dir/$file - then - sed -i.orig -e \ - "s|\(<bridge.*$\)|\0\n <mac address='52:54:00:$mac4:$mac5:$mac6'/>|" \ - $dir/$file - if test $? != 0 - then - echo "failed to add <mac address='52:54:00:$mac4:$mac5:$mac6'/>" \ - "to $dir/$file" - mv -f $dir/$file.orig $dir/$file - else - rm -f $dir/$file.orig - fi - fi - done -done - %endif - %if %{with_systemd} %if %{with_systemd_macros} %systemd_post virtlockd.socket libvirtd.service libvirtd.socket diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index f36be63..4c16bb4 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3155,6 +3155,10 @@ virNetworkObjPtr virNetworkLoadConfig(virNetworkObjListPtr nets, def->forward.type == VIR_NETWORK_FORWARD_NAT || def->forward.type == VIR_NETWORK_FORWARD_ROUTE) { + if (!def->mac_specified) { + virNetworkSetBridgeMacAddr(def); + virNetworkSaveConfig(configDir, def); + } /* Generate a bridge if none is specified, but don't check for collisions * if a bridge is hardcoded, so the network is at least defined. */ -- 2.0.4

On 11.11.2014 10:42, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1156367
Ján Tomko (2): Silently ignore MAC in NetworkLoadConfig Generate a MAC when loading a config instead of package update
libvirt.spec.in | 42 ------------------------------------------ src/conf/network_conf.c | 8 ++++++++ 2 files changed, 8 insertions(+), 42 deletions(-)
Okay. I think this may work. I mean, the first time that new libvirtd is started, the network mac is generated and written into the XML file. Cool. And all those applications that parse MAC from network XML file directly - they are broken already. ACK series Michal

On 11/13/2014 03:40 PM, Michal Privoznik wrote:
On 11.11.2014 10:42, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1156367
Ján Tomko (2): Silently ignore MAC in NetworkLoadConfig Generate a MAC when loading a config instead of package update
libvirt.spec.in | 42 ------------------------------------------ src/conf/network_conf.c | 8 ++++++++ 2 files changed, 8 insertions(+), 42 deletions(-)
Okay. I think this may work. I mean, the first time that new libvirtd is started, the network mac is generated and written into the XML file. Cool. And all those applications that parse MAC from network XML file directly - they are broken already.
ACK series
Thanks, pushed now. Jan
Michal
participants (2)
-
Ján Tomko
-
Michal Privoznik