On 05/19/14 16:07, Pavel Hrdina wrote:
The commit 84c59ffa improved the way we change ejectable media. If for any reason the first "eject" didn't open the tray we should return with error.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_hotplug.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 125a2db..47ec779 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -125,6 +125,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, /* If ret == -1, EjectMedia already set an error message */ virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Unable to eject media")); + ret = -1;
Although this is technically correct, the control flow in this function seems a little bit weird. The eject media error should be checked before the polling loop and removed from here. Then also the ret=0 asignment a few lines below isn't useful.
} goto audit; }
I'd go with the following patch: diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 125a2db..76e289b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -106,6 +106,9 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, ret = qemuMonitorEjectMedia(priv->mon, driveAlias, force); qemuDomainObjExitMonitor(driver, vm); + if (ret < 0) + goto audit; + virObjectRef(vm); /* we don't want to report errors from media tray_open polling */ while (retries) { @@ -121,14 +124,11 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, virObjectUnref(vm); if (retries <= 0) { - if (ret == 0) { - /* If ret == -1, EjectMedia already set an error message */ - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("Unable to eject media")); - } + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Unable to eject media")); + ret = -1; goto audit; } - ret = 0; src = virDomainDiskGetSource(disk); if (src) { ACK if you go with the above. Peter