Re: [libvirt] [PATCHv2 04/14] Fix vmdef usage after domain crash in monitor on device detach

On 01/07/2015 10:42 AM, Ján Tomko wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1161024 > > Skip audit and removing the device from live def because > it has already been cleaned up. > --- > src/qemu/qemu_hotplug.c | 59 ++++++++++++++++++++++++++++++------------------- > 1 file changed, 36 insertions(+), 23 deletions(-) > Similar Audit concerns w/ 3/14... > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > index ce05f80..c480dcd 100644 > --- a/src/qemu/qemu_hotplug.c > +++ b/src/qemu/qemu_hotplug.c > @@ -2986,19 +2986,22 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver, > qemuDomainObjEnterMonitor(driver, vm); > if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { > if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { > - qemuDomainObjExitMonitor(driver, vm); > + if (qemuDomainObjExitMonitor(driver, vm) < 0) > + goto cleanup; > virDomainAuditDisk(vm, detach->src, NULL, "detach", false); > goto cleanup; > } > } else { > if (qemuMonitorRemovePCIDevice(priv->mon, > &detach->info.addr.pci) < 0) { > - qemuDomainObjExitMonitor(driver, vm); > + if (qemuDomainObjExitMonitor(driver, vm) < 0) > + goto cleanup; > virDomainAuditDisk(vm, detach->src, NULL, "detach", false); > goto cleanup; > } > } > - qemuDomainObjExitMonitor(driver, vm); > + if (qemuDomainObjExitMonitor(driver, vm) < 0) > + goto cleanup; > > rc = qemuDomainWaitForDeviceRemoval(vm); > if (rc == 0 || rc == 1) > @@ -3038,11 +3041,13 @@ qemuDomainDetachDiskDevice(virQEMUDriverPtr driver, > > qemuDomainObjEnterMonitor(driver, vm); > if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) { > - qemuDomainObjExitMonitor(driver, vm); > + if (qemuDomainObjExitMonitor(driver, vm) < 0) > + goto cleanup; > virDomainAuditDisk(vm, detach->src, NULL, "detach", false); > goto cleanup; > } > - qemuDomainObjExitMonitor(driver, vm); > + if (qemuDomainObjExitMonitor(driver, vm) < 0) > + goto cleanup; > > rc = qemuDomainWaitForDeviceRemoval(vm); > if (rc == 0 || rc == 1) > @@ -3219,17 +3224,20 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver, > qemuDomainObjEnterMonitor(driver, vm); > if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) { > if (qemuMonitorDelDevice(priv->mon, detach->info.alias)) { > - qemuDomainObjExitMonitor(driver, vm); > + if (qemuDomainObjExitMonitor(driver, vm) < 0) > + goto cleanup; > goto cleanup; > } > } else { > if (qemuMonitorRemovePCIDevice(priv->mon, > &detach->info.addr.pci) < 0) { > - qemuDomainObjExitMonitor(driver, vm); > + if (qemuDomainObjExitMonitor(driver, vm) < 0) > + goto cleanup; > goto cleanup; > } > } > - qemuDomainObjExitMonitor(driver, vm); > + if (qemuDomainObjExitMonitor(driver, vm) < 0) > + goto cleanup; > > rc = qemuDomainWaitForDeviceRemoval(vm); > if (rc == 0 || rc == 1) > @@ -3274,7 +3282,8 @@ qemuDomainDetachHostPCIDevice(virQEMUDriverPtr driver, > } else { > ret = qemuMonitorRemovePCIDevice(priv->mon, &detach->info->addr.pci); > } > - qemuDomainObjExitMonitor(driver, vm); > + if (qemuDomainObjExitMonitor(driver, vm) < 0) > + ret = -1; > > return ret; Or just ret qemuDomainObjExitMonitor(driver, vm);

> @@ -3374,7 +3381,8 @@ qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver, > } > > if (ret < 0) { > - virDomainAuditHostdev(vm, detach, "detach", false); > + if (virDomainObjIsActive(vm)) > + virDomainAuditHostdev(vm, detach, "detach", false); Hmm.... well this makes my comments in 3/14 appear to be unnecessary, since there's an explicit check for active vm... Although the Vcpu failure will still call virDomainAuditVcpu