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: | |||
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 |
Description of problem: Update-device fail to update floppy with unknown error Version-Release number of selected component (if applicable): libvirt-1.3.3-1.el7.x86_64 qemu-kvm-rhev-2.5.0-4.el7.x86_64 How reproducible: 100% Steps to Reproduce: 1. define and start with floppy device withour source # virsh dumpxml r72 |grep floppy -A 9 <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. update floppy device # cat floppy.xml <disk type='file' device='floppy'> <driver name='qemu' type='raw'/> <source file='/mnt/at-dt/change-media/a.iso'/> <target dev='fda' bus='fdc'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> 3. update device # virsh update-device r72 floppy.xml error: Failed to update device from floppy.xml error: An error occurred, but the cause is unknown Actual results: As step 3, fail update with unknown error. Expected results: update successfully. Additional info: