| Summary: | bootindex causes qemu exit for conflict on hotplug, doesn't support removal | ||
|---|---|---|---|
| Product: | Red Hat Enterprise Linux 6 | Reporter: | juzhang <juzhang> |
| Component: | qemu-kvm | Assignee: | Gleb Natapov <gleb> |
| Status: | CLOSED NOTABUG | QA Contact: | Virtualization Bugs <virt-bugs> |
| Severity: | low | Docs Contact: | |
| Priority: | medium | ||
| Version: | 6.1 | CC: | alex.williamson, ddutile, knoel, michen, mkenneth, tburke, virt-maint |
| Target Milestone: | rc | ||
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2011-02-17 19:32:06 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Bug Depends On: | |||
| Bug Blocks: | 580954 | ||
|
Description
juzhang
2011-02-16 06:18:28 UTC
The above error has nothing to do with device assignment in particular, it will happen with any device that supports bootindex. The new bootindex option does not work well with hotplug. add_boot_device_path() should not call exit, it should return an error if there's a conflict. There also appears to be no remove function, so if a device is specified with a bootindex and later removed, what happens? Assigning back to Gleb. Don't add two devices with same same bootindex. This does not suppose to work neither from command line nor from qemu monitor. Closing as NOTABUG. Alex is correct that we probably need to add del_boot_device_path(), but hot-unplug will not cause crash without it too, so this is not high priority. (In reply to comment #4) > Don't add two devices with same same bootindex. This does not suppose to work > neither from command line nor from qemu monitor. Closing as NOTABUG. Alex is > correct that we probably need to add del_boot_device_path(), but hot-unplug > will not cause crash without it too, so this is not high priority. Gleb, should we at least ignore the bootindex for hotplugged device, maybe printing a warning? I'm not a fan of leaving holes that are so easy to trigger and result in qemu calling exit() while the guest is running. (In reply to comment #5) > (In reply to comment #4) > > Don't add two devices with same same bootindex. This does not suppose to work > > neither from command line nor from qemu monitor. Closing as NOTABUG. Alex is > > correct that we probably need to add del_boot_device_path(), but hot-unplug > > will not cause crash without it too, so this is not high priority. > > Gleb, should we at least ignore the bootindex for hotplugged device, maybe > printing a warning? I'm not a fan of leaving holes that are so easy to trigger > and result in qemu calling exit() while the guest is running. Why should we ignore bootindex for hotplugged device? User may want to hotplug a device and boot from in on the next boot for instance. About exit() I am not sure what can be done here. This is not exit that can be triggered by a guest and qemu usually exits on wrong command line (user may not notice the warning if we will print one). Also printing a warning is not appropriate response on monitor command failure since the one who issues the command may not even have access to stderr. I can change the code to not treat two devices with the same bootindex as an error, but then user may unintentionally specify two devices with the same bootindex and be surprised by a result. (In reply to comment #7) > Why should we ignore bootindex for hotplugged device? User may want to hotplug > a device and boot from in on the next boot for instance. About exit() I am not > sure what can be done here. This is not exit that can be triggered by a guest > and qemu usually exits on wrong command line (user may not notice the warning > if we will print one). Also printing a warning is not appropriate response on > monitor command failure since the one who issues the command may not even have > access to stderr. I can change the code to not treat two devices with the same > bootindex as an error, but then user may unintentionally specify two devices > with the same bootindex and be surprised by a result. Gleb, it sounds like you're validating why this should not be closed. If we want to allow users to hotplug devices and boot from it on the next boot, then we need to make it such that a trivial typo on the hotplug doesn't cause the VM to exit. It seems like a fairly simple matter to have add_boot_device_path() return an error and make the device initfn fail when this occurs. If an initfn fails before the VM is running, I think we automatically abort. I actually started fixing this, but decided to re-assign to you after I saw there was no del_boot_device_path(). (In reply to comment #9) > (In reply to comment #7) > > Why should we ignore bootindex for hotplugged device? User may want to hotplug > > a device and boot from in on the next boot for instance. About exit() I am not > > sure what can be done here. This is not exit that can be triggered by a guest > > and qemu usually exits on wrong command line (user may not notice the warning > > if we will print one). Also printing a warning is not appropriate response on > > monitor command failure since the one who issues the command may not even have > > access to stderr. I can change the code to not treat two devices with the same > > bootindex as an error, but then user may unintentionally specify two devices > > with the same bootindex and be surprised by a result. > > Gleb, it sounds like you're validating why this should not be closed. If we > want to allow users to hotplug devices and boot from it on the next boot, then > we need to make it such that a trivial typo on the hotplug doesn't cause the VM > to exit. It seems like a fairly simple matter to have add_boot_device_path() > return an error and make the device initfn fail when this occurs. If an initfn > fails before the VM is running, I think we automatically abort. I actually > started fixing this, but decided to re-assign to you after I saw there was no > del_boot_device_path(). Are you saying that if initfn fails during machine creation qemu aborts, but if it fails during hot plug it doesn't? If yes then we may change the behaviour for 6.2. We may make add_boot_device_path() to return token or an error. Token will be used for del_boot_device_path(). (In reply to comment #11) > Are you saying that if initfn fails during machine creation qemu aborts, but if > it fails during hot plug it doesn't? If yes then we may change the behaviour > for 6.2. We may make add_boot_device_path() to return token or an error. Token > will be used for del_boot_device_path(). Yes, exactly. See how device_init_func is called versus do_device_add. |