| Message ID | 9f0a7e92429f3050ab2fb30ae31f959181d06c36.1462230544.git.crobinso@redhat.com |
|---|---|
| State | Accepted |
| Commit | 1fad65d49aae364576bd91352a001249510f8d4e |
| Headers | show |
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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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);