Bug 1326660
Summary: | Update-device fail to update floppy with an unknown error | ||
---|---|---|---|
Product: | Red Hat Enterprise Linux 7 | Reporter: | Pei Zhang <pzhang> |
Component: | libvirt | Assignee: | Peter Krempa <pkrempa> |
Status: | CLOSED ERRATA | QA Contact: | Virtualization Bugs <virt-bugs> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | 7.3 | CC: | crobinso, dyuan, hhan, hreitz, lmen, mzhan, rbalakri, xuzhang |
Target Milestone: | rc | ||
Target Release: | --- | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Whiteboard: | |||
Fixed In Version: | libvirt-1.3.5-1.el7 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2016-11-03 18:42:07 UTC | Type: | Bug |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Pei Zhang
2016-04-13 10:05:53 UTC
commit a34faf33011c5c0d7b47ee0849bf1e11635e17c5 Author: Peter Krempa <pkrempa> 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. Note this doesn't work again with qemu 2.6.0. I bisected to: commit abb3e55b5b718d6392441f56ba0729a62105ac56 Author: Max Reitz <mreitz> 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 So libvirt is waiting for a TRAY event that qemu is never going to send Apart from waiting for a TRAY_MOVED event that will never occur, there is the issue of Peter's commit making use of the "tray_open" status of the device. However, the change Cole presented above did not only make those events disappear but also the "tray_open" field in the query-block response for floppy disk drives. Instead, you could check query-block's "inserted" field. Its existence is equivalent to the presence of a medium in the drive. I have not checked it, but I think that this should even be true for previous versions of qemu. The only problem is that for devices with an actual tray it is possible for no medium to be inserted but the tray to be closed. I'm not sure whether this was possible before qemu 2.5, though. Maybe the presence of the "tray_open" field can help a bit. libvirt claims that "'tray_open' is presented iff medium is ejected" which is not true: It is always presented if a device supports removable media, and as far as I can see this has been so since its introduction (commit e4def80b). Starting from 2.6 this changed and now it is only presented iff a device actually has a tray. In 2.5, floppy disk drives basically had a tray and therefore apparently can be handled just like any other tray device. So I guess libvirt could make a distinction based on the presence of the "tray_open" field: If it is present, treat the device just like it has always been. If it is not (but it supports removable media as indicated by the "removable" field), then treat it as a tray-less slot device (i.e. what floppy disk and SD drives are) that will not send TRAY_MOVED events nor have a "tray_open" status. Max (In reply to Max Reitz from comment #4) > Apart from waiting for a TRAY_MOVED event that will never occur, there is > the issue of Peter's commit making use of the "tray_open" status of the > device. However, the change Cole presented above did not only make those My code just refreshed the state of the tray since for empty floppies it was open and for cdroms it was closed by default. The real bug is still in the logic that is handling the re-issue of the legacy 'eject' command. > events disappear but also the "tray_open" field in the query-block response > for floppy disk drives. Instead, you could check query-block's "inserted" > field. Its existence is equivalent to the presence of a medium in the drive. In libvirt we store the info in two places. One is that the medium is empty and one is that the tray is open. In case when the tray is open we consider that it's empty. With the possibility to retract the cdrom via the guest this might not be entirely a good model. > I have not checked it, but I think that this should even be true for > previous versions of qemu. The only problem is that for devices with an > actual tray it is possible for no medium to be inserted but the tray to be > closed. I'm not sure whether this was possible before qemu 2.5, though. As we are using just the legacy 'eject' command this should not be a problem though. I'll need to check if we remove the source if we get the TRAY_MOVED event or not. That might be still tricky. > > Maybe the presence of the "tray_open" field can help a bit. libvirt claims > that "'tray_open' is presented iff medium is ejected" which is not true: It > is always presented if a device supports removable media, and as far as I > can see this has been so since its introduction (commit e4def80b). Starting > from 2.6 this changed and now it is only presented iff a device actually has > a tray. Perhaps time to use 3-state logic rather than 2-state. > > In 2.5, floppy disk drives basically had a tray and therefore apparently can > be handled just like any other tray device. So I guess libvirt could make a > distinction based on the presence of the "tray_open" field: If it is > present, treat the device just like it has always been. If it is not (but it > supports removable media as indicated by the "removable" field), then treat > it as a tray-less slot device (i.e. what floppy disk and SD drives are) that > will not send TRAY_MOVED events nor have a "tray_open" status. More fun. SD-drives are not handled as removable in libvirt :) Following commits should fix it properly including the possible presence of the tray: commit 72a7ff6b507bcf389cc493ac0ba07d32de266d6e Author: Peter Krempa <pkrempa> Date: Thu May 19 15:30:12 2016 +0200 qemu: hotplug: wait for the tray to eject only for drives with a tray Use the detected tray presence flag to trigger the tray waiting code only if the given storage device in qemu reports to have a tray. This is necessary as the floppy device lost it's tray as of qemu commit: commit abb3e55b5b718d6392441f56ba0729a62105ac56 Author: Max Reitz <mreitz> Date: Fri Jan 29 20:49:12 2016 +0100 Revert "hw/block/fdc: Implement tray status" commit 2e75da42e41af0cd48ca6f75d0606b40a366cc54 Author: Peter Krempa <pkrempa> Date: Mon May 23 16:32:06 2016 +0200 qemu: hotplug: Fix error reported when cdrom tray is locked Commit 1fad65d49aae364576bd91352a001249510f8d4e used a really big hammer and overwrote the error message that might be reported by qemu if the tray is locked. Fix it by reporting the error only if no error is currently set. Error after commit mentioned above: error: internal error: timed out waiting for disk tray status update New error: error: internal error: unable to execute QEMU command 'eject': Tray of device 'drive-ide0-0-0' is not open commit 0aa19f35e0f3c1712f2569986d4d0a93b488c35c Author: Peter Krempa <pkrempa> Date: Mon May 23 14:50:17 2016 +0200 qemu: hotplug: Extract code for waiting for tray eject The code grew rather convoluted. Extract it to a separate function. commit 894dc85fd1ebcd63d8c897b355c550e68a5f432d Author: Peter Krempa <pkrempa> Date: Thu May 19 15:29:02 2016 +0200 qemu: process: Fix and improve disk data extraction Extract information for all disks and update tray state and source only for removable drives. Additionally store whether a drive is removable and whether it has a tray. commit d9bee413ade28e1e43ef222c7aaaa3c6d6fda0f1 Author: Peter Krempa <pkrempa> Date: Mon May 23 14:00:35 2016 +0200 qemu: Move and rename qemuDomainCheckEjectableMedia to qemuProcessRefreshDisks Move it to a more sane place since it's refreshing data about disks. commit f1690dc3d7934bf70f4fbc84d55bf210276c6f27 Author: Peter Krempa <pkrempa> Date: Thu May 19 14:57:41 2016 +0200 qemu: Extract more information about qemu drives Extract whether a given drive has a tray and whether there is no image inserted. Negative logic for the image insertion is chosen so that the flag is set only if we are certain of the fact. commit 5f963d89b1220460fadb1bf6fc347d26b311c1b2 Author: Peter Krempa <pkrempa> Date: Fri May 20 07:21:04 2016 +0200 qemu: Move struct qemuDomainDiskInfo to qemu_domain.h verify the bug version: libvirt-1.3.5-1.el7.x86_64 qemu-kvm-rhev-2.6.0-7.el7.x86_64 kernel-3.10.0-445.el7 steps: 1. define and start with floppy device without source <disk type='file' device='floppy'> <driver name='qemu' type='raw'/> <backingStore/> <target dev='fda' bus='fdc'/> <alias name='fdc0-0-0'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> 2. prepare xml file [root@localhost ~]# cat floppy.xml <disk type='file' device='floppy'> <driver name='qemu' type='raw'/> <source file='/var/lib/libvirt/images/temp1.iso'/> <target dev='fda' bus='fdc'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> 3. update device [root@localhost ~]# virsh update-device bios floppy.xml Device updated successfully [root@localhost ~]# virsh domblklist bios Target Source ------------------------------------------------ sda /var/lib/libvirt/images/admin.qcow2 fda /var/lib/libvirt/images/temp1.iso Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://rhn.redhat.com/errata/RHSA-2016-2577.html |