On 07/28/2014 10:20 PM, Eric Blake wrote:
Fix things by saving state any time we modify live XML, while delaying XML disk modifications until after the event completes.
Except that I didn't, which means I caused a use-after-free situation which can crash libvirtd when doing blockcopy or active blockcommit:
+++ b/src/qemu/qemu_driver.c @@ -14911,38 +14911,43 @@ qemuDomainBlockPivot(virConnectPtr conn, virSecurityManagerSetDiskLabel(driver->securityManager, vm->def, disk) < 0)) goto cleanup;
- /* Attempt the pivot. */ + /* Attempt the pivot. Record the attempt now, to prevent duplicate + * attempts; but the actual disk change will be made when emitting + * the event.
What I missed was that a couple lines before, the code did 'disk->src = disk->mirror', which means that the qemu event handler...
@@ -1027,29 +1029,58 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
+ /* If we completed a block pull or commit, then update the XML + * to match. */ + if (status == VIR_DOMAIN_BLOCK_JOB_COMPLETED) { + bool has_mirror = !!disk->mirror; + + if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_PIVOT) { + /* XXX We want to revoke security labels and disk + * lease, as well as audit that revocation, before + * dropping the original source. But it gets tricky + * if both source and mirror share common backing + * files (we want to only revoke the non-shared + * portion of the chain); so for now, we leak the + * access to the original. */ + virStorageSourceFree(disk->src); + disk->src = disk->mirror;
is promptly calling virStorageSourceFree() on the same pointer that it is about to assign into disk->src, as well as leaking the original disk->src. Fix coming up. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org