[libvirt] [PATCH 0/2] Invoke drive_del on failute to attach disks

There is a drive_del in QEMU so we should use it. Guido Günther (2): qemu: attempt to delete disk when SCSI attach failed qemu: attempt to delete disk when USB mass storage attach failed src/qemu/qemu_hotplug.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) -- 2.8.1

We have a qemuMonitorDriveDel now so use it --- src/qemu/qemu_hotplug.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e8a30d5..6232a0e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -678,11 +678,12 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, return ret; exit_monitor: - /* XXX should call 'drive_del' on error but this does not exist yet */ - if (driveAdded) - VIR_WARN("qemuMonitorAddDevice failed on %s (%s)", drivestr, devstr); - orig_err = virSaveLastError(); + if (driveAdded && qemuMonitorDriveDel(priv->mon, drivestr) < 0) { + VIR_WARN("Unable to remove drive %s (%s) after failed " + "qemuMonitorAddDevice", + drivestr, devstr); + } if (encobjAdded) ignore_value(qemuMonitorDelObject(priv->mon, encinfo->s.aes.alias)); if (orig_err) { -- 2.8.1

On Fri, Jul 22, 2016 at 12:08:45 +0200, Guido Günther wrote:
We have a qemuMonitorDriveDel now so use it --- src/qemu/qemu_hotplug.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e8a30d5..6232a0e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -678,11 +678,12 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, return ret;
exit_monitor: - /* XXX should call 'drive_del' on error but this does not exist yet */ - if (driveAdded) - VIR_WARN("qemuMonitorAddDevice failed on %s (%s)", drivestr, devstr); - orig_err = virSaveLastError(); + if (driveAdded && qemuMonitorDriveDel(priv->mon, drivestr) < 0) {
This won't work. You need to delete the drive by alias. See commit 64c6695f1ad72f0a99faace5deb1caf7effa2275

On Fri, Jul 22, 2016 at 01:42:44PM +0200, Peter Krempa wrote:
On Fri, Jul 22, 2016 at 12:08:45 +0200, Guido Günther wrote:
We have a qemuMonitorDriveDel now so use it --- src/qemu/qemu_hotplug.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e8a30d5..6232a0e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -678,11 +678,12 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, return ret;
exit_monitor: - /* XXX should call 'drive_del' on error but this does not exist yet */ - if (driveAdded) - VIR_WARN("qemuMonitorAddDevice failed on %s (%s)", drivestr, devstr); - orig_err = virSaveLastError(); + if (driveAdded && qemuMonitorDriveDel(priv->mon, drivestr) < 0) {
This won't work. You need to delete the drive by alias.
See commit 64c6695f1ad72f0a99faace5deb1caf7effa2275
Thanks! John's patch from the "Various hotplug cleanup" series handles this with alias already so I'll not update this one. Cheers, -- Guido

We have a qemuMonitorDriveDel now so use it --- src/qemu/qemu_hotplug.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6232a0e..87c208b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -707,6 +707,7 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver, virDomainDiskDefPtr disk) { qemuDomainObjPrivatePtr priv = vm->privateData; + virErrorPtr orig_err; int ret = -1; char *drivestr = NULL; char *devstr = NULL; @@ -745,10 +746,16 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver, if (ret == 0) { ret = qemuMonitorAddDevice(priv->mon, devstr); if (ret < 0) { - VIR_WARN("qemuMonitorAddDevice failed on %s (%s)", - drivestr, devstr); - /* XXX should call 'drive_del' on error but this does not - exist yet */ + orig_err = virSaveLastError(); + if (qemuMonitorDriveDel(priv->mon, drivestr) < 0) { + VIR_WARN("Unable to remove drive %s (%s) after failed " + "qemuMonitorAddDevice", + drivestr, devstr); + } + if (orig_err) { + virSetError(orig_err); + virFreeError(orig_err); + } } } if (qemuDomainObjExitMonitor(driver, vm) < 0) { -- 2.8.1

On 07/22/2016 06:08 AM, Guido Günther wrote:
There is a drive_del in QEMU so we should use it.
Guido Günther (2): qemu: attempt to delete disk when SCSI attach failed qemu: attempt to delete disk when USB mass storage attach failed
src/qemu/qemu_hotplug.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-)
I already have some patches on list to handle SCSI and USB case, see: http://www.redhat.com/archives/libvir-list/2016-July/msg00730.html Although after Jan's USB address series, they may not git am -3 cleanly... John
participants (3)
-
Guido Günther
-
John Ferlan
-
Peter Krempa