Re: [Qemu-devel] [RFC PATCH] block: Fix eject -f for locked devices

[Markus Armbruster]

Sorry for my slow reply.

Blue Swirl <address@hidden> writes:

> On Fri, Feb 18, 2011 at 5:16 PM, Markus Armbruster <address@hidden> wrote:
>> From 8cd4978c9be6ff2bcc414bb1c1b258b96b9a74c1 Mon Sep 17 00:00:00 2001
>> From: Markus Armbruster <address@hidden>
>> Date: Fri, 18 Feb 2011 15:54:02 +0100
>>
>> After forcefully ejecting media locked by the guest, you can't ever
>> again insert new media.
>>
>> Example:
>>
>>    (qemu) info block
>>    hda: type=hd removable=0 file=test.img ro=0 drv=raw encrypted=0
>>    cd: type=cdrom removable=1 locked=1 file=x.iso ro=0 drv=raw encrypted=0
>>    (qemu) eject cd
>>    Device 'cd' is locked
>>    (qemu) eject -f cd
>>    (qemu) info block
>>    hda: type=hd removable=0 file=test.img ro=0 drv=raw encrypted=0
>>    cd: type=cdrom removable=1 locked=1 [not inserted]
>>    (qemu) change cd x.iso
>>    Device 'cd' is locked
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>> I'm not entirely sure this is the appropriate fix, and that's why
>> there's RFC in the subject.
>>
>> Both IDE and SCSI devices expose their drive's BlockDriverState member
>> locked to the guest via mode sense.
>>
>> What does real hardware do when I force-eject media (typically by
>> rummaging in that little hole with a paperclip)?  Does it actively
>> notify the OS?  Does mode sense change?
>
> No idea, but IIRC the drive is still usable after that,

That's what I'd expect.  If the OS recovers from losing the media, the
drive should be fine.

>                                                         so locking the
> drive does not look correct.

I'm not sure I get you here.

>> A possible alternative fix is to make do_change_block() ignore
>> bdrv_is_locked() when inserting media into an empty drive.
>
> Then the meaning of locked would change, maybe eject_disabled would
> then describe the state better.

I like the current meaning of BlockDriverState member locked just fine,
it nicely mirrors the SCSI / ATAPI mode sense page bit.

I'm worried about unwanted side effects of my change.  For instance,
what about this code in do_snapshot_blkdev():

    flags = bs->open_flags;
    bdrv_close(bs);
    ret = bdrv_open(bs, filename, flags, drv);

I'm afraid my change would make it screw up bs->locked.  Jes?