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(a)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