[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 28/55] block: Leave enforcing tray lock to devic
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 28/55] block: Leave enforcing tray lock to device models |
Date: |
Thu, 21 Jul 2011 17:16:13 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) |
Luiz Capitulino <address@hidden> writes:
> On Wed, 20 Jul 2011 18:24:02 +0200
> Markus Armbruster <address@hidden> wrote:
>
>> The device model knows best when to accept the guest's eject command.
>> No need to detour through the block layer.
>>
>> bdrv_eject() can't fail anymore. Make it void.
>
> But we're supposed to return an error to the user/client if we're unable
> to eject, aren't we?
Same story as for bdrv_set_locked() [PATCH 07/55]: the purpose is to
synchronize the physical tray with the virtual one. Errors would have
to be propagated up into device model init, post migration and guest
START STOP UNIT. What error to report to the guest? Hardware failure?
As with bdrv_set_locked(), I'm not removing any error handling. I don't
add any either. The series is long enough...
> (one more question below)
Where?
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>> block.c | 7 +------
>> block.h | 2 +-
>> hw/ide/atapi.c | 29 +++++++++--------------------
>> hw/scsi-disk.c | 3 +++
>> 4 files changed, 14 insertions(+), 27 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 6759066..70928af 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -2778,18 +2778,13 @@ int bdrv_media_changed(BlockDriverState *bs)
>> /**
>> * If eject_flag is TRUE, eject the media. Otherwise, close the tray
>> */
>> -int bdrv_eject(BlockDriverState *bs, int eject_flag)
>> +void bdrv_eject(BlockDriverState *bs, int eject_flag)
>> {
>> BlockDriver *drv = bs->drv;
>>
>> - if (eject_flag && bs->locked) {
>> - return -EBUSY;
>> - }
>> -
>> if (drv && drv->bdrv_eject) {
>> drv->bdrv_eject(bs, eject_flag);
>> }
>> - return 0;
>> }
>>
>> int bdrv_is_locked(BlockDriverState *bs)
>> diff --git a/block.h b/block.h
>> index 65a0115..7cc7919 100644
>> --- a/block.h
>> +++ b/block.h
>> @@ -209,7 +209,7 @@ int bdrv_is_inserted(BlockDriverState *bs);
>> int bdrv_media_changed(BlockDriverState *bs);
>> int bdrv_is_locked(BlockDriverState *bs);
>> void bdrv_set_locked(BlockDriverState *bs, int locked);
>> -int bdrv_eject(BlockDriverState *bs, int eject_flag);
>> +void bdrv_eject(BlockDriverState *bs, int eject_flag);
>> void bdrv_get_format(BlockDriverState *bs, char *buf, int buf_size);
>> BlockDriverState *bdrv_find(const char *name);
>> BlockDriverState *bdrv_next(BlockDriverState *bs);
>> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
>> index 237657f..6cb8f0e 100644
>> --- a/hw/ide/atapi.c
>> +++ b/hw/ide/atapi.c
>> @@ -894,33 +894,22 @@ static void cmd_seek(IDEState *s, uint8_t* buf)
>>
>> static void cmd_start_stop_unit(IDEState *s, uint8_t* buf)
>> {
>> - int sense, err = 0;
>> + int sense;
>> bool start = buf[4] & 1;
>> bool loej = buf[4] & 2;
>>
>> if (loej) {
>> - err = bdrv_eject(s->bs, !start);
>> - }
>> -
>> - switch (err) {
>> - case 0:
>> - ide_atapi_cmd_ok(s);
>> - break;
>> - case -EBUSY:
>> - sense = SENSE_NOT_READY;
>> - if (bdrv_is_inserted(s->bs)) {
>> - sense = SENSE_ILLEGAL_REQUEST;
>> + if (!start && s->tray_locked) {
>> + sense = bdrv_is_inserted(s->bs)
>> + ? SENSE_NOT_READY : SENSE_ILLEGAL_REQUEST;
>> + ide_atapi_cmd_error(s, sense, ASC_MEDIA_REMOVAL_PREVENTED);
>> + return;
>> }
>> - ide_atapi_cmd_error(s, sense, ASC_MEDIA_REMOVAL_PREVENTED);
>> - break;
>> - default:
>> - ide_atapi_cmd_error(s, SENSE_NOT_READY, ASC_MEDIUM_NOT_PRESENT);
>> - break;
>> - }
>> -
>> - if (loej && !err) {
>> + bdrv_eject(s->bs, !start);
>> s->tray_open = !start;
>> }
>> +
>> + ide_atapi_cmd_ok(s);
>> }
>>
>> static void cmd_mechanism_status(IDEState *s, uint8_t* buf)
>> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
>> index aac63b6..a4ed499 100644
>> --- a/hw/scsi-disk.c
>> +++ b/hw/scsi-disk.c
>> @@ -833,6 +833,9 @@ static void scsi_disk_emulate_start_stop(SCSIDiskReq *r)
>> bool loej = req->cmd.buf[4] & 2;
>>
>> if (s->drive_kind == SCSI_CD && loej) {
>> + if (!start && s->tray_locked) {
>> + return;
>> + }
>> bdrv_eject(s->bs, !start);
>> s->tray_open = !start;
>> }
- Re: [Qemu-devel] [PATCH 25/55] ide/atapi: Switch from BlockDriverState's locked to own tray_locked, (continued)
- [Qemu-devel] [PATCH 44/55] spitz tosa: Simplify "drive is suitable for microdrive" test, Markus Armbruster, 2011/07/20
- [Qemu-devel] [PATCH 43/55] savevm: Include writable devices with removable media, Markus Armbruster, 2011/07/20
- [Qemu-devel] [PATCH 29/55] block: Drop medium lock tracking, ask device models instead, Markus Armbruster, 2011/07/20
- [Qemu-devel] [PATCH 46/55] block: Drop BlockDriverState member removable, Markus Armbruster, 2011/07/20
- [Qemu-devel] [PATCH 22/55] block: Drop tray status tracking, no longer used, Markus Armbruster, 2011/07/20
- [Qemu-devel] [PATCH 28/55] block: Leave enforcing tray lock to device models, Markus Armbruster, 2011/07/20
- [Qemu-devel] [PATCH 47/55] block: Move BlockConf & friends from block_int.h to block.h, Markus Armbruster, 2011/07/20
- [Qemu-devel] [PATCH 48/55] hw: Trim superfluous #include "block_int.h", Markus Armbruster, 2011/07/20
- [Qemu-devel] [PATCH 53/55] nbd: Clean up use of block_int.h, Markus Armbruster, 2011/07/20
- [Qemu-devel] [PATCH 40/55] block: Leave tracking media change to device models, Markus Armbruster, 2011/07/20
- Re: [Qemu-devel] [PATCH 00/55] Block layer cleanup & fixes, Amit Shah, 2011/07/22