[libvirt] [PATCH] qemu: hotplug: Properly clean up drive backend if frontend hotplug fails
Commit 8125113c added code that should remove the disk backend if the fronted hotplug failed for any reason. The code had a bug though as it used the disk string for unplug rather than the backend alias. Fix the code by pre-creating an alias string and using it instead of the disk string. In cases where qemu does not support QEMU_CAPS_DEVICE, we ignore the unplug of the backend since we can't really create an alias in that case. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1262399 --- src/qemu/qemu_hotplug.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 63fafa6..c956e8c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -321,6 +321,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, qemuDomainObjPrivatePtr priv = vm->privateData; char *devstr = NULL; char *drivestr = NULL; + char *drivealias = NULL; bool releaseaddr = false; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *src = virDomainDiskGetSource(disk); @@ -365,6 +366,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps))) goto error; + if (virAsprintf(&drivealias, "%s%s", QEMU_DRIVE_HOST_PREFIX, + disk->info.alias) < 0) + goto error; + if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps))) goto error; } @@ -379,7 +384,8 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, ret = qemuMonitorAddDevice(priv->mon, devstr); if (ret < 0) { virErrorPtr orig_err = virSaveLastError(); - if (qemuMonitorDriveDel(priv->mon, drivestr) < 0) { + if (!drivealias || + qemuMonitorDriveDel(priv->mon, drivealias) < 0) { VIR_WARN("Unable to remove drive %s (%s) after failed " "qemuMonitorAddDevice", drivestr, devstr); @@ -415,6 +421,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, cleanup: VIR_FREE(devstr); VIR_FREE(drivestr); + VIR_FREE(drivealias); virObjectUnref(cfg); return ret; -- 2.4.5
On 11.09.2015 17:41, Peter Krempa wrote:
Commit 8125113c added code that should remove the disk backend if the fronted hotplug failed for any reason. The code had a bug though as it used the disk string for unplug rather than the backend alias. Fix the code by pre-creating an alias string and using it instead of the disk string. In cases where qemu does not support QEMU_CAPS_DEVICE, we ignore the unplug of the backend since we can't really create an alias in that case.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1262399 --- src/qemu/qemu_hotplug.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 63fafa6..c956e8c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -321,6 +321,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, qemuDomainObjPrivatePtr priv = vm->privateData; char *devstr = NULL; char *drivestr = NULL; + char *drivealias = NULL; bool releaseaddr = false; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *src = virDomainDiskGetSource(disk); @@ -365,6 +366,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps))) goto error;
+ if (virAsprintf(&drivealias, "%s%s", QEMU_DRIVE_HOST_PREFIX, + disk->info.alias) < 0) + goto error; +
Maybe we can use qemuDeviceDriveHostAlias() instead of virAsprintf?
if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps))) goto error; } @@ -379,7 +384,8 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, ret = qemuMonitorAddDevice(priv->mon, devstr); if (ret < 0) { virErrorPtr orig_err = virSaveLastError(); - if (qemuMonitorDriveDel(priv->mon, drivestr) < 0) { + if (!drivealias || + qemuMonitorDriveDel(priv->mon, drivealias) < 0) { VIR_WARN("Unable to remove drive %s (%s) after failed " "qemuMonitorAddDevice", drivestr, devstr); @@ -415,6 +421,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, cleanup: VIR_FREE(devstr); VIR_FREE(drivestr); + VIR_FREE(drivealias); virObjectUnref(cfg); return ret;
ACK Michal
On Mon, Sep 14, 2015 at 08:43:37 +0200, Michal Privoznik wrote:
On 11.09.2015 17:41, Peter Krempa wrote:
Commit 8125113c added code that should remove the disk backend if the fronted hotplug failed for any reason. The code had a bug though as it used the disk string for unplug rather than the backend alias. Fix the code by pre-creating an alias string and using it instead of the disk string. In cases where qemu does not support QEMU_CAPS_DEVICE, we ignore the unplug of the backend since we can't really create an alias in that case.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1262399 --- src/qemu/qemu_hotplug.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 63fafa6..c956e8c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -321,6 +321,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, qemuDomainObjPrivatePtr priv = vm->privateData; char *devstr = NULL; char *drivestr = NULL; + char *drivealias = NULL; bool releaseaddr = false; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *src = virDomainDiskGetSource(disk); @@ -365,6 +366,10 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->qemuCaps))) goto error;
+ if (virAsprintf(&drivealias, "%s%s", QEMU_DRIVE_HOST_PREFIX, + disk->info.alias) < 0) + goto error; +
Maybe we can use qemuDeviceDriveHostAlias() instead of virAsprintf?
Indeed. I couldn't remember that we have something like that when I was fixing the code. I'll fix it and push. Thanks Peter
participants (2)
-
Michal Privoznik -
Peter Krempa