On 9/25/19 6:57 PM, Laine Stump wrote:
A virDomainNetDef object in a domain's nets array might contain a
virDomainHostdevDef, and when this is the case, the domain's hostdevs
array will also have a pointer to this embedded hostdev (this is done
so that internal functions that need to perform some operation on all
hostdevs won't leave out the type='hostdev' network interfaces).
When a network device was updated with virDomainUpdateDeviceFlags(),
we were replacing the entry in the nets array (and free'ing the
original) but forgetting about the pointer in the hostdevs array
(which would then point to the now-free'd hostdev contained in the old
net object.) This often resulted in a libvirtd crash.
The solution is to add a function, virDomainNetUpdate(), called by
qemuDomainUpdateDeviceConfig(), that updates the hostdevs array
appropriately along with the nets array.
Resolves: https://bugzilla.redhat.com/1558934
Signed-off-by: Laine Stump
---
(I actually think that it was a bad idea to have this "reach over"
pointer in the hostdevs array (I can say that, since it was my idea
:-), and want to eliminate it in favor of using an iterator function
for all operations that need to do something to all hostdevs, but this
bug exists on old, maintained branches, and I wouldn't want to require
backporting anything that disruptive to a stable branch.)
Also, I was unable to reproduce the crashes described in the bug
report using current upstream code, but could witness (by adding a
breakpoint in gdb) the stale pointer when running without this patch,
and the correct pointer when running with the patch)
Agreed, refactoring the whole approach would require a significant
amount of work and it's not worth it only to fix a crasher.
src/conf/domain_conf.c | 41
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 6 --
4 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0753904472..5f63c4f51e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8786,8 +8786,10 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
false) < 0)
return -1;
-virDomainNetDefFree(vmdef->nets[pos]);
-vmdef->nets[pos] = net;
+if (virDomainNetUpdate(vmdef, pos, net))
+return -1;
+
+virDomainNetDefFree(oldDev.data.net);
dev->data.net = NULL;
break;
The same code pattern occurrs in lxcDomainUpdateDeviceConfig() so you
may want to fix it the same way as you're doing here. With that,
Reviewed-by: Michal Privoznik
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
