On 05/27/14 16:53, Jiri Denemark wrote:
In general, we should only remove a backend after seeing DEVICE_DELETED event for a corresponding frontend. This doesn't make any difference for disks attached using -drive or drive_add since QEMU automatically removes their frontends but it's still better to make our code
s/frontends/backends/ ?
consistent. And it may start making difference in case we switch to attaching disks using -blockdev.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_hotplug.c | 37 +++++++++++++------------------------ 1 file changed, 13 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index fd1f002..43c52bf 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2472,10 +2472,23 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, virObjectEventPtr event; size_t i; const char *src = virDomainDiskGetSource(disk); + qemuDomainObjPrivatePtr priv = vm->privateData; + char *drivestr;
VIR_DEBUG("Removing disk %s from domain %p %s", disk->info.alias, vm, vm->def->name);
+ /* build the actual drive id string as the disk->info.alias doesn't + * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */ + if (virAsprintf(&drivestr, "%s%s", + QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0) + return;
Should this be treated the same way for propagating errors as in the previous patch? I'm not going to enforce this here as the allocaion is unlikely to fail.
+ + qemuDomainObjEnterMonitor(driver, vm); + qemuMonitorDriveDel(priv->mon, drivestr); + qemuDomainObjExitMonitor(driver, vm); + VIR_FREE(drivestr); + virDomainAuditDisk(vm, src, NULL, "detach", true);
event = virDomainEventDeviceRemovedNewFromObj(vm, disk->info.alias);
ACK if you go with this patch as-is, v2 if you upgrade the error reporting. Peter