[libvirt] [PATCH] qemu: hotplug: Report error if we hit tray status timeout

If we exceed the timeout waiting for the tray status to change, we don't report an error. Fix it --- I hit this trying to eject floppy media for a win10 VM with F24 qemu, but I didn't dig deeper to figure out _why_ it's timing out... src/qemu/qemu_hotplug.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 03e5309..e5f8a38 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -224,7 +224,13 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, goto error; while (disk->tray_status != VIR_DOMAIN_DISK_TRAY_OPEN) { - if (virDomainObjWaitUntil(vm, now + CHANGE_MEDIA_TIMEOUT) != 0) + int rc2 = virDomainObjWaitUntil(vm, now + CHANGE_MEDIA_TIMEOUT); + if (rc2 == 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("timed out waiting for " + "disk tray status update")); + } + if (rc2 != 0) goto error; } } while (rc < 0); -- 2.7.4

On Mon, May 02, 2016 at 19:09:35 -0400, Cole Robinson wrote:
If we exceed the timeout waiting for the tray status to change, we don't report an error. Fix it --- I hit this trying to eject floppy media for a win10 VM with F24 qemu, but I didn't dig deeper to figure out _why_ it's timing out...
Did you try it after: commit a34faf33011c5c0d7b47ee0849bf1e11635e17c5 Author: Peter Krempa <pkrempa@redhat.com> Date: Fri Apr 29 13:38:51 2016 +0200 qemu: process: Refresh ejectable media tray state on VM start Empty floppy drives start with tray in "open" state and libvirt did not refresh it after startup. The code that inserts media into the tray then waited until the tray was open before inserting the media and thus floppies could not be inserted. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1326660 If not, then that's the reason, and the fix below doesn't make much sense. Peter

On 05/03/2016 02:24 AM, Peter Krempa wrote:
On Mon, May 02, 2016 at 19:09:35 -0400, Cole Robinson wrote:
If we exceed the timeout waiting for the tray status to change, we don't report an error. Fix it --- I hit this trying to eject floppy media for a win10 VM with F24 qemu, but I didn't dig deeper to figure out _why_ it's timing out...
Did you try it after:
commit a34faf33011c5c0d7b47ee0849bf1e11635e17c5 Author: Peter Krempa <pkrempa@redhat.com> Date: Fri Apr 29 13:38:51 2016 +0200
qemu: process: Refresh ejectable media tray state on VM start
Empty floppy drives start with tray in "open" state and libvirt did not refresh it after startup. The code that inserts media into the tray then waited until the tray was open before inserting the media and thus floppies could not be inserted.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1326660
If not, then that's the reason, and the fix below doesn't make much sense.
I just double checked: with current libvirt.git + qemu 2.6.0, trying to eject floppy media via virt-manager will return 'An error occurred, but the cause is unknown'. After my patch it at least reports an error - Cole

On 05/03/2016 08:23 AM, Cole Robinson wrote:
On 05/03/2016 02:24 AM, Peter Krempa wrote:
On Mon, May 02, 2016 at 19:09:35 -0400, Cole Robinson wrote:
If we exceed the timeout waiting for the tray status to change, we don't report an error. Fix it --- I hit this trying to eject floppy media for a win10 VM with F24 qemu, but I didn't dig deeper to figure out _why_ it's timing out...
Did you try it after:
commit a34faf33011c5c0d7b47ee0849bf1e11635e17c5 Author: Peter Krempa <pkrempa@redhat.com> Date: Fri Apr 29 13:38:51 2016 +0200
qemu: process: Refresh ejectable media tray state on VM start
Empty floppy drives start with tray in "open" state and libvirt did not refresh it after startup. The code that inserts media into the tray then waited until the tray was open before inserting the media and thus floppies could not be inserted.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1326660
If not, then that's the reason, and the fix below doesn't make much sense.
I just double checked: with current libvirt.git + qemu 2.6.0, trying to eject floppy media via virt-manager will return 'An error occurred, but the cause is unknown'. After my patch it at least reports an error
To clarify a bit, the case I was testing is starting a VM with media already in the floppy drive, then trying to eject it, which doesn't exactly match the commit message so maybe there's a mismatch somewhere - Cole

On 03.05.2016 01:09, Cole Robinson wrote:
If we exceed the timeout waiting for the tray status to change, we don't report an error. Fix it --- I hit this trying to eject floppy media for a win10 VM with F24 qemu, but I didn't dig deeper to figure out _why_ it's timing out...
src/qemu/qemu_hotplug.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 03e5309..e5f8a38 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -224,7 +224,13 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, goto error;
while (disk->tray_status != VIR_DOMAIN_DISK_TRAY_OPEN) { - if (virDomainObjWaitUntil(vm, now + CHANGE_MEDIA_TIMEOUT) != 0) + int rc2 = virDomainObjWaitUntil(vm, now + CHANGE_MEDIA_TIMEOUT); + if (rc2 == 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("timed out waiting for " + "disk tray status update")); + } + if (rc2 != 0) goto error; } } while (rc < 0);
Huh, funny; I've just got to the same problem producing nearly the same patch. I'd rename rc2 to wait_rc and test it for begin greater than 0 instead of being exactly one. I know it's the same right now, but I think it's more solid. And for the 'lost' event - I guess it's a qemu bug. While I was debugging, I ran 'query-block' monitor command and saw tray closed. So I've ran 'virsh update-device' which opens the tray and it timed out, just like in your case. So I ran 'query-block' again just to find that tray is now being reported open. So there clearly has been a state translation without qemu sending us corresponding event thus I'd call it a qemu bug. ACK Michal

On 05/17/2016 04:39 AM, Michal Privoznik wrote:
On 03.05.2016 01:09, Cole Robinson wrote:
If we exceed the timeout waiting for the tray status to change, we don't report an error. Fix it --- I hit this trying to eject floppy media for a win10 VM with F24 qemu, but I didn't dig deeper to figure out _why_ it's timing out...
src/qemu/qemu_hotplug.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 03e5309..e5f8a38 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -224,7 +224,13 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, goto error;
while (disk->tray_status != VIR_DOMAIN_DISK_TRAY_OPEN) { - if (virDomainObjWaitUntil(vm, now + CHANGE_MEDIA_TIMEOUT) != 0) + int rc2 = virDomainObjWaitUntil(vm, now + CHANGE_MEDIA_TIMEOUT); + if (rc2 == 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("timed out waiting for " + "disk tray status update")); + } + if (rc2 != 0) goto error; } } while (rc < 0);
Huh, funny; I've just got to the same problem producing nearly the same patch. I'd rename rc2 to wait_rc and test it for begin greater than 0 instead of being exactly one. I know it's the same right now, but I think it's more solid.
Thanks, I pushed with those changes
And for the 'lost' event - I guess it's a qemu bug. While I was debugging, I ran 'query-block' monitor command and saw tray closed. So I've ran 'virsh update-device' which opens the tray and it timed out, just like in your case. So I ran 'query-block' again just to find that tray is now being reported open. So there clearly has been a state translation without qemu sending us corresponding event thus I'd call it a qemu bug.
Seems to be a qemu regression between v2.5.0 and v2.6.0, I'm bisecting now Thanks, Cole

On 05/17/2016 08:10 AM, Cole Robinson wrote:
On 05/17/2016 04:39 AM, Michal Privoznik wrote:
On 03.05.2016 01:09, Cole Robinson wrote:
If we exceed the timeout waiting for the tray status to change, we don't report an error. Fix it --- I hit this trying to eject floppy media for a win10 VM with F24 qemu, but I didn't dig deeper to figure out _why_ it's timing out...
src/qemu/qemu_hotplug.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 03e5309..e5f8a38 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -224,7 +224,13 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, goto error;
while (disk->tray_status != VIR_DOMAIN_DISK_TRAY_OPEN) { - if (virDomainObjWaitUntil(vm, now + CHANGE_MEDIA_TIMEOUT) != 0) + int rc2 = virDomainObjWaitUntil(vm, now + CHANGE_MEDIA_TIMEOUT); + if (rc2 == 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("timed out waiting for " + "disk tray status update")); + } + if (rc2 != 0) goto error; } } while (rc < 0);
Huh, funny; I've just got to the same problem producing nearly the same patch. I'd rename rc2 to wait_rc and test it for begin greater than 0 instead of being exactly one. I know it's the same right now, but I think it's more solid.
Thanks, I pushed with those changes
And for the 'lost' event - I guess it's a qemu bug. While I was debugging, I ran 'query-block' monitor command and saw tray closed. So I've ran 'virsh update-device' which opens the tray and it timed out, just like in your case. So I ran 'query-block' again just to find that tray is now being reported open. So there clearly has been a state translation without qemu sending us corresponding event thus I'd call it a qemu bug.
Seems to be a qemu regression between v2.5.0 and v2.6.0, I'm bisecting now
Well I guess that explains it: commit abb3e55b5b718d6392441f56ba0729a62105ac56 Author: Max Reitz <mreitz@redhat.com> Date: Fri Jan 29 20:49:12 2016 +0100 Revert "hw/block/fdc: Implement tray status" The cover letter here has an explanation: http://lists.nongnu.org/archive/html/qemu-devel/2016-01/msg04471.html - Cole

On 17.05.2016 14:19, Cole Robinson wrote:
On 05/17/2016 08:10 AM, Cole Robinson wrote:
On 05/17/2016 04:39 AM, Michal Privoznik wrote:
On 03.05.2016 01:09, Cole Robinson wrote:
If we exceed the timeout waiting for the tray status to change, we don't report an error. Fix it --- I hit this trying to eject floppy media for a win10 VM with F24 qemu, but I didn't dig deeper to figure out _why_ it's timing out...
src/qemu/qemu_hotplug.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 03e5309..e5f8a38 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -224,7 +224,13 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, goto error;
while (disk->tray_status != VIR_DOMAIN_DISK_TRAY_OPEN) { - if (virDomainObjWaitUntil(vm, now + CHANGE_MEDIA_TIMEOUT) != 0) + int rc2 = virDomainObjWaitUntil(vm, now + CHANGE_MEDIA_TIMEOUT); + if (rc2 == 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("timed out waiting for " + "disk tray status update")); + } + if (rc2 != 0) goto error; } } while (rc < 0);
Huh, funny; I've just got to the same problem producing nearly the same patch. I'd rename rc2 to wait_rc and test it for begin greater than 0 instead of being exactly one. I know it's the same right now, but I think it's more solid.
Thanks, I pushed with those changes
And for the 'lost' event - I guess it's a qemu bug. While I was debugging, I ran 'query-block' monitor command and saw tray closed. So I've ran 'virsh update-device' which opens the tray and it timed out, just like in your case. So I ran 'query-block' again just to find that tray is now being reported open. So there clearly has been a state translation without qemu sending us corresponding event thus I'd call it a qemu bug.
Seems to be a qemu regression between v2.5.0 and v2.6.0, I'm bisecting now
Well I guess that explains it:
commit abb3e55b5b718d6392441f56ba0729a62105ac56 Author: Max Reitz <mreitz@redhat.com> Date: Fri Jan 29 20:49:12 2016 +0100
Revert "hw/block/fdc: Implement tray status"
The cover letter here has an explanation: http://lists.nongnu.org/archive/html/qemu-devel/2016-01/msg04471.html
That's rather unpleasant. How is libvirt supposed to know when device tray has moved then? Michal

On Tue, May 17, 2016 at 15:09:51 +0200, Michal Privoznik wrote:
On 17.05.2016 14:19, Cole Robinson wrote:
On 05/17/2016 08:10 AM, Cole Robinson wrote:
On 05/17/2016 04:39 AM, Michal Privoznik wrote:
On 03.05.2016 01:09, Cole Robinson wrote:
[...]
Well I guess that explains it:
commit abb3e55b5b718d6392441f56ba0729a62105ac56 Author: Max Reitz <mreitz@redhat.com> Date: Fri Jan 29 20:49:12 2016 +0100
Revert "hw/block/fdc: Implement tray status"
The cover letter here has an explanation: http://lists.nongnu.org/archive/html/qemu-devel/2016-01/msg04471.html
That's rather unpleasant. How is libvirt supposed to know when device tray has moved then?
A floppy drive doesn't have the tray in reality. So as with a real drive the medium will always eject if you "push the button". [1] The guest can't reject it. I'll tweak the code appropriately since it might not work with floppies correctly at this point. Peter [1] I've seen floppy drives (or were it ls120) with software eject functionality ...
participants (3)
-
Cole Robinson
-
Michal Privoznik
-
Peter Krempa