[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 08/55] block: Make BlockDriver method bdrv_eject
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 08/55] block: Make BlockDriver method bdrv_eject() return void |
Date: |
Fri, 22 Jul 2011 17:04:56 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) |
Kevin Wolf <address@hidden> writes:
> Am 20.07.2011 18:23, schrieb Markus Armbruster:
>> Callees always return 0, except for FreeBSD's cdrom_eject(), which
>> returns -ENOTSUP when the device is in a terminally wedged state.
>>
>> The only caller is bdrv_eject(), and it maps -ENOTSUP to 0 since
>> commit 4be9762a.
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>
> What about failed ioctls? Currently we only print an error message but
> still return 0. Is this the right behaviour? Could callers make use of
> an error return here or would we end up like with bdrv_set_locked()
> where we can't really communicate the error to the guest?
Note that I'm not removing any error handling. I don't add any either.
The series is long enough...
bdrv_eject()'s purpose is similar to bdrv_set_locked()'s: synchronize
the physical tray to the virtual one. Unsurprisingly, our error
handling options are also similar.
All bdrv_eject() can do with errors from the BlockDriver method is pass
them on.
At this point in the series, bdrv_eject() still returns an error of its
own: -EBUSY when bs->locked. 28/55 moves that check into callers.
bdrv_eject()'s callers are device models: ide-cd and scsi-cd. Each one
calls it in three places (with all my patches applied):
* Device init: can return failure. Failure makes QEMU refuse to start
(-device cold plug), or hot plug fail (device_add).
* Device post migration: can return failure, makes migration fail.
Given the choice, I figure I'd pick an incorrect physical tray lock
over a failed migration.
* Guest PREVENT ALLOW MEDIUM REMOVAL: can send error reply to guest.
Does send the appropriate error when the virtual tray is locked.
What sense to send when the ioctl() fails?
How can the ioctl() fail? When the tray is locked, but for that to
happen some other process must have interfered with the lock (Bad
process! No treat for you! Go away!). We could synchronize the lock
again right before the eject, but that merely shrinks the race's
window.
I doubt adding the error handling is worth our while, but feel free to
send patches.
- [Qemu-devel] [PATCH 00/55] Block layer cleanup & fixes, Markus Armbruster, 2011/07/20
- [Qemu-devel] [PATCH 01/55] blockdev: Make eject fail for non-removable drives even with -f, Markus Armbruster, 2011/07/20
- [Qemu-devel] [PATCH 06/55] block/raw-win32: Drop disabled code for removable host devices, Markus Armbruster, 2011/07/20
- [Qemu-devel] [PATCH 16/55] ide/atapi: Track tray open/close state, Markus Armbruster, 2011/07/20
- [Qemu-devel] [PATCH 08/55] block: Make BlockDriver method bdrv_eject() return void, Markus Armbruster, 2011/07/20
- [Qemu-devel] [PATCH 09/55] block: Don't let locked flag prevent medium load, Markus Armbruster, 2011/07/20
- [Qemu-devel] [PATCH 21/55] block: Revert entanglement of bdrv_is_inserted() with tray status, Markus Armbruster, 2011/07/20
- [Qemu-devel] [PATCH 27/55] scsi-disk: Switch from BlockDriverState's locked to own tray_locked, Markus Armbruster, 2011/07/20
- [Qemu-devel] [PATCH 04/55] block: Generalize change_cb() to BlockDevOps, Markus Armbruster, 2011/07/20