[Qemu-devel] [PATCH 0/3] scsi: eject fixed

[Qemu-devel] [PATCH 0/3] scsi: eject fixed

[Alexey Kardashevskiy]

The initial problem was if the user ejects a ISO/DVD mounted in Fedora19,
the block device went into weird state when QEMU's monitor reported that
it is locked, has medium but tray open (some debug output is below).

Please comment. Thanks!


[root@localhost ~]# mount /dev/cdrom /mnt
mount: /dev/sr0 is write-protected, mounting read-only
ISO 9660 Extensions: Microsoft Joliet Level 3
ISO 9660 Extensions: RRIP_1991A
[root@localhost ~]# QEMU 1.7.50 monitor - type 'help' for more information
(qemu) eject cd1
Device 'cd1' is locked
(qemu) info block
sd0: virtimg/fc19_16GB.qcow2 (qcow2)

cd1: virtimg/Fedora-19-ppc64-netinst.iso (raw)
    Removable device: locked, tray open
    (qemu) 

[root@localhost ~]# ls /mnt
sr 1:0:0:0: [sr0] Device not ready
sr 1:0:0:0: [sr0]  
Result: hostbyte=0x00 driverbyte=0x08
sr 1:0:0:0: [sr0]  
Sense Key : 0x2 [current] 
sr 1:0:0:0: [sr0]  
ASC=0x3a ASCQ=0x0
sr 1:0:0:0: [sr0] CDB: 
cdb[0]=0x28: 28 00 00 00 00 1c 00 00 01 00
end_request: I/O error, dev sr0, sector 112
[root@localhost ~]# 



Alexey Kardashevskiy (3):
  scsi-disk: close drive on START_STOP
  scsi-disk: check for meduim on ALLOW_MEDIUM_REMOVAL
  scsi debug: print command name in debug

 hw/scsi/scsi-bus.c    | 5 ++++-
 hw/scsi/scsi-disk.c   | 7 +++++--
 hw/scsi/spapr_vscsi.c | 5 +++--
 include/block/scsi.h  | 2 ++
 4 files changed, 14 insertions(+), 5 deletions(-)

-- 
1.8.4.rc4

[Qemu-devel] [PATCH 1/3] scsi-disk: close drive on START_STOP

[Alexey Kardashevskiy]

Normally the user is expected to eject DVD if it is not locked by
the guest. eject_device() makes few checks and calls bdrv_close()
if DVD is not in use.

However it is still possible to eject DVD even if it is in use.
For that, QEMU sets "eject requested" flag, the guest reads it, issues
ALLOW_MEDIUM_REMOVAL(enable=1) and START_STOP(start=0). But in this case,
bdrv_close() is not called anywhere so it remains "inserted" in QEMU's
terms.

This adds a block device removal on the SCSI device stop.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/scsi/scsi-disk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 74e6a14..08b6135 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1283,6 +1283,9 @@ static int scsi_disk_emulate_start_stop(SCSIDiskReq *r)
         if (s->tray_open != !start) {
             bdrv_eject(s->qdev.conf.bs, !start);
             s->tray_open = !start;
+            if (!start) {
+                bdrv_close(s->qdev.conf.bs);
+            }
         }
     }
     return 0;
-- 
1.8.4.rc4

Re: [Qemu-devel] [PATCH 1/3] scsi-disk: close drive on START_STOP

[Paolo Bonzini]

Il 04/12/2013 05:55, Alexey Kardashevskiy ha scritto:
> Normally the user is expected to eject DVD if it is not locked by
> the guest. eject_device() makes few checks and calls bdrv_close()
> if DVD is not in use.
> 
> However it is still possible to eject DVD even if it is in use.
> For that, QEMU sets "eject requested" flag, the guest reads it, issues
> ALLOW_MEDIUM_REMOVAL(enable=1) and START_STOP(start=0). But in this case,
> bdrv_close() is not called anywhere so it remains "inserted" in QEMU's
> terms.

This is expected behavior, and matches what IDE does.

Markus, can you confirm?

Paolo

Re: [Qemu-devel] [PATCH 1/3] scsi-disk: close drive on START_STOP

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 04/12/2013 05:55, Alexey Kardashevskiy ha scritto:
>> Normally the user is expected to eject DVD if it is not locked by
>> the guest. eject_device() makes few checks and calls bdrv_close()
>> if DVD is not in use.
>> 
>> However it is still possible to eject DVD even if it is in use.
>> For that, QEMU sets "eject requested" flag, the guest reads it, issues
>> ALLOW_MEDIUM_REMOVAL(enable=1) and START_STOP(start=0). But in this case,
>> bdrv_close() is not called anywhere so it remains "inserted" in QEMU's
>> terms.
>
> This is expected behavior, and matches what IDE does.
>
> Markus, can you confirm?

Confirmed.  See commit 4be9762.

Alexey, monitor commands eject does two things: it first opens the tray,
and if that works, it removes the medium.

If the tray is locked closed, it tells the device model that eject was
requested.  Works just like the physical eject button.

With -f, it then rips out the medium.  This is similar to opening the
tray with a unbent paperclip.  Let's ignore this case.

The scsi-cd device model tells the guest about the eject request.  A
well-behaved guest will then command the device to unlock and open the
tray.

The guest uses the same commands on behalf of its applications,
e.g. /usr/bin/eject.

Your patch changes behavior of "eject /dev/sr0 && eject -t /dev/sr0":
you no longer get the same medium back.  You normally do with real
hardware.

The somewhat unfortunate consequence is that monitor command eject can
only remove the medium when the tray is not locked.

Re: [Qemu-devel] [PATCH 1/3] scsi-disk: close drive on START_STOP

[Alexey Kardashevskiy]

On 12/04/2013 08:33 PM, Markus Armbruster wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> Il 04/12/2013 05:55, Alexey Kardashevskiy ha scritto:
>>> Normally the user is expected to eject DVD if it is not locked by
>>> the guest. eject_device() makes few checks and calls bdrv_close()
>>> if DVD is not in use.
>>>
>>> However it is still possible to eject DVD even if it is in use.
>>> For that, QEMU sets "eject requested" flag, the guest reads it, issues
>>> ALLOW_MEDIUM_REMOVAL(enable=1) and START_STOP(start=0). But in this case,
>>> bdrv_close() is not called anywhere so it remains "inserted" in QEMU's
>>> terms.
>>
>> This is expected behavior, and matches what IDE does.
>>
>> Markus, can you confirm?
> 
> Confirmed.  See commit 4be9762.
> 
> Alexey, monitor commands eject does two things: it first opens the tray,
> and if that works, it removes the medium.
> 
> If the tray is locked closed, it tells the device model that eject was
> requested.  Works just like the physical eject button.
> 
> With -f, it then rips out the medium.  This is similar to opening the
> tray with a unbent paperclip.  Let's ignore this case.
> 
> The scsi-cd device model tells the guest about the eject request.  A
> well-behaved guest will then command the device to unlock and open the
> tray.
> 
> The guest uses the same commands on behalf of its applications,
> e.g. /usr/bin/eject.
> 
> Your patch changes behavior of "eject /dev/sr0 && eject -t /dev/sr0":
> you no longer get the same medium back.  You normally do with real
> hardware.
> 
> The somewhat unfortunate consequence is that monitor command eject can
> only remove the medium when the tray is not locked.


Oh. Wow. Nice :-/

Ok. So. It is expected that the real system will close the tray back if it
was mounted, is not it?

Right now, after "eject" "info block" is like this:

cd1: virtimg/Fedora-19-ppc64-netinst.iso (raw)
    Removable device: locked, tray open

And the mountpoint does not work in the guest. The state above even
persists after "umount" in the guest. It only becomes correct again
(tray==closed) when I mount DVD again.

Is it all expected to work like this? Thanks.


-- 
Alexey

Re: [Qemu-devel] [PATCH 1/3] scsi-disk: close drive on START_STOP

[Markus Armbruster]

Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> On 12/04/2013 08:33 PM, Markus Armbruster wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> 
>>> Il 04/12/2013 05:55, Alexey Kardashevskiy ha scritto:
>>>> Normally the user is expected to eject DVD if it is not locked by
>>>> the guest. eject_device() makes few checks and calls bdrv_close()
>>>> if DVD is not in use.
>>>>
>>>> However it is still possible to eject DVD even if it is in use.
>>>> For that, QEMU sets "eject requested" flag, the guest reads it, issues
>>>> ALLOW_MEDIUM_REMOVAL(enable=1) and START_STOP(start=0). But in this case,
>>>> bdrv_close() is not called anywhere so it remains "inserted" in QEMU's
>>>> terms.
>>>
>>> This is expected behavior, and matches what IDE does.
>>>
>>> Markus, can you confirm?
>> 
>> Confirmed.  See commit 4be9762.
>> 
>> Alexey, monitor commands eject does two things: it first opens the tray,
>> and if that works, it removes the medium.
>> 
>> If the tray is locked closed, it tells the device model that eject was
>> requested.  Works just like the physical eject button.
>> 
>> With -f, it then rips out the medium.  This is similar to opening the
>> tray with a unbent paperclip.  Let's ignore this case.
>> 
>> The scsi-cd device model tells the guest about the eject request.  A
>> well-behaved guest will then command the device to unlock and open the
>> tray.
>> 
>> The guest uses the same commands on behalf of its applications,
>> e.g. /usr/bin/eject.
>> 
>> Your patch changes behavior of "eject /dev/sr0 && eject -t /dev/sr0":
>> you no longer get the same medium back.  You normally do with real
>> hardware.
>> 
>> The somewhat unfortunate consequence is that monitor command eject can
>> only remove the medium when the tray is not locked.
>
>
> Oh. Wow. Nice :-/
>
> Ok. So. It is expected that the real system will close the tray back if it
> was mounted, is not it?
>
> Right now, after "eject" "info block" is like this:
>
> cd1: virtimg/Fedora-19-ppc64-netinst.iso (raw)
>     Removable device: locked, tray open
>
> And the mountpoint does not work in the guest. The state above even
> persists after "umount" in the guest. It only becomes correct again
> (tray==closed) when I mount DVD again.
>
> Is it all expected to work like this? Thanks.

Can't reproduce, but can reproduce something similar.  Freshly booted
guest running RHEL-7 alpha, with the CD mounted:

    (qemu) info block cd

    cd: r7.iso (raw, read-only)
        Removable device: locked, tray closed

Looks good.  Try to eject:

    (qemu) eject cd
    Device 'cd' is locked

Looks good.  This should have signalled the guest "user wants to eject".
The guest should either ignore it, or unmount, unlock and eject.
Apparently, it does that:

    (qemu) info block cd

    cd: r7.iso (raw, read-only)
        Removable device: locked, tray closed
    (qemu) eject cd
    Device 'cd' is locked
    (qemu) info block cd

    cd: r7.iso (raw, read-only)
        Removable device: locked, tray closed
    (qemu) info block cd

    cd: r7.iso (raw, read-only)
        Removable device: not locked, tray open

Except it forgets to unmount!  dmesg has "VFS: busy inodes on changed
media or resized disk sr0".

Need somebody to find out how exactly this fails, and whether it's a
guest bug or a QEMU bug.

Re: [Qemu-devel] [PATCH 1/3] scsi-disk: close drive on START_STOP

[Alexey Kardashevskiy]

On 12/05/2013 12:12 AM, Markus Armbruster wrote:
> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> 
>> On 12/04/2013 08:33 PM, Markus Armbruster wrote:
>>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>>
>>>> Il 04/12/2013 05:55, Alexey Kardashevskiy ha scritto:
>>>>> Normally the user is expected to eject DVD if it is not locked by
>>>>> the guest. eject_device() makes few checks and calls bdrv_close()
>>>>> if DVD is not in use.
>>>>>
>>>>> However it is still possible to eject DVD even if it is in use.
>>>>> For that, QEMU sets "eject requested" flag, the guest reads it, issues
>>>>> ALLOW_MEDIUM_REMOVAL(enable=1) and START_STOP(start=0). But in this case,
>>>>> bdrv_close() is not called anywhere so it remains "inserted" in QEMU's
>>>>> terms.
>>>>
>>>> This is expected behavior, and matches what IDE does.
>>>>
>>>> Markus, can you confirm?
>>>
>>> Confirmed.  See commit 4be9762.
>>>
>>> Alexey, monitor commands eject does two things: it first opens the tray,
>>> and if that works, it removes the medium.
>>>
>>> If the tray is locked closed, it tells the device model that eject was
>>> requested.  Works just like the physical eject button.
>>>
>>> With -f, it then rips out the medium.  This is similar to opening the
>>> tray with a unbent paperclip.  Let's ignore this case.
>>>
>>> The scsi-cd device model tells the guest about the eject request.  A
>>> well-behaved guest will then command the device to unlock and open the
>>> tray.
>>>
>>> The guest uses the same commands on behalf of its applications,
>>> e.g. /usr/bin/eject.
>>>
>>> Your patch changes behavior of "eject /dev/sr0 && eject -t /dev/sr0":
>>> you no longer get the same medium back.  You normally do with real
>>> hardware.
>>>
>>> The somewhat unfortunate consequence is that monitor command eject can
>>> only remove the medium when the tray is not locked.
>>
>>
>> Oh. Wow. Nice :-/
>>
>> Ok. So. It is expected that the real system will close the tray back if it
>> was mounted, is not it?
>>
>> Right now, after "eject" "info block" is like this:
>>
>> cd1: virtimg/Fedora-19-ppc64-netinst.iso (raw)
>>     Removable device: locked, tray open
>>
>> And the mountpoint does not work in the guest. The state above even
>> persists after "umount" in the guest. It only becomes correct again
>> (tray==closed) when I mount DVD again.
>>
>> Is it all expected to work like this? Thanks.
> 
> Can't reproduce, but can reproduce something similar.  Freshly booted
> guest running RHEL-7 alpha, with the CD mounted:
> 
>     (qemu) info block cd
> 
>     cd: r7.iso (raw, read-only)
>         Removable device: locked, tray closed
> 
> Looks good.  Try to eject:
> 
>     (qemu) eject cd
>     Device 'cd' is locked
> 
> Looks good.  This should have signalled the guest "user wants to eject".
> The guest should either ignore it, or unmount, unlock and eject.
> Apparently, it does that:
> 
>     (qemu) info block cd
> 
>     cd: r7.iso (raw, read-only)
>         Removable device: locked, tray closed
>     (qemu) eject cd
>     Device 'cd' is locked
>     (qemu) info block cd
> 
>     cd: r7.iso (raw, read-only)
>         Removable device: locked, tray closed
>     (qemu) info block cd
> 
>     cd: r7.iso (raw, read-only)
>         Removable device: not locked, tray open
> 
> Except it forgets to unmount!  dmesg has "VFS: busy inodes on changed
> media or resized disk sr0".
> 
> Need somebody to find out how exactly this fails, and whether it's a
> guest bug or a QEMU bug.


The guest unlocks DVD (by sending ALLOW PERMIT MEDIUM REMOVAL) and stops
DVD (by sending START_STOP). Is there any other message missing which would
do real physical eject?

What does it have to do with unmount (which is purely the guest software
state)?




-- 
Alexey

Re: [Qemu-devel] [PATCH 1/3] scsi-disk: close drive on START_STOP

[Markus Armbruster]

Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> On 12/05/2013 12:12 AM, Markus Armbruster wrote:
>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>> 
>>> On 12/04/2013 08:33 PM, Markus Armbruster wrote:
>>>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>>>
>>>>> Il 04/12/2013 05:55, Alexey Kardashevskiy ha scritto:
>>>>>> Normally the user is expected to eject DVD if it is not locked by
>>>>>> the guest. eject_device() makes few checks and calls bdrv_close()
>>>>>> if DVD is not in use.
>>>>>>
>>>>>> However it is still possible to eject DVD even if it is in use.
>>>>>> For that, QEMU sets "eject requested" flag, the guest reads it, issues
>>>>>> ALLOW_MEDIUM_REMOVAL(enable=1) and START_STOP(start=0). But in this case,
>>>>>> bdrv_close() is not called anywhere so it remains "inserted" in QEMU's
>>>>>> terms.
>>>>>
>>>>> This is expected behavior, and matches what IDE does.
>>>>>
>>>>> Markus, can you confirm?
>>>>
>>>> Confirmed.  See commit 4be9762.
>>>>
>>>> Alexey, monitor commands eject does two things: it first opens the tray,
>>>> and if that works, it removes the medium.
>>>>
>>>> If the tray is locked closed, it tells the device model that eject was
>>>> requested.  Works just like the physical eject button.
>>>>
>>>> With -f, it then rips out the medium.  This is similar to opening the
>>>> tray with a unbent paperclip.  Let's ignore this case.
>>>>
>>>> The scsi-cd device model tells the guest about the eject request.  A
>>>> well-behaved guest will then command the device to unlock and open the
>>>> tray.
>>>>
>>>> The guest uses the same commands on behalf of its applications,
>>>> e.g. /usr/bin/eject.
>>>>
>>>> Your patch changes behavior of "eject /dev/sr0 && eject -t /dev/sr0":
>>>> you no longer get the same medium back.  You normally do with real
>>>> hardware.
>>>>
>>>> The somewhat unfortunate consequence is that monitor command eject can
>>>> only remove the medium when the tray is not locked.
>>>
>>>
>>> Oh. Wow. Nice :-/
>>>
>>> Ok. So. It is expected that the real system will close the tray back if it
>>> was mounted, is not it?
>>>
>>> Right now, after "eject" "info block" is like this:
>>>
>>> cd1: virtimg/Fedora-19-ppc64-netinst.iso (raw)
>>>     Removable device: locked, tray open
>>>
>>> And the mountpoint does not work in the guest. The state above even
>>> persists after "umount" in the guest. It only becomes correct again
>>> (tray==closed) when I mount DVD again.
>>>
>>> Is it all expected to work like this? Thanks.
>> 
>> Can't reproduce, but can reproduce something similar.  Freshly booted
>> guest running RHEL-7 alpha, with the CD mounted:
>> 
>>     (qemu) info block cd
>> 
>>     cd: r7.iso (raw, read-only)
>>         Removable device: locked, tray closed
>> 
>> Looks good.  Try to eject:
>> 
>>     (qemu) eject cd
>>     Device 'cd' is locked
>> 
>> Looks good.  This should have signalled the guest "user wants to eject".
>> The guest should either ignore it, or unmount, unlock and eject.
>> Apparently, it does that:
>> 
>>     (qemu) info block cd
>> 
>>     cd: r7.iso (raw, read-only)
>>         Removable device: locked, tray closed
>>     (qemu) eject cd
>>     Device 'cd' is locked
>>     (qemu) info block cd
>> 
>>     cd: r7.iso (raw, read-only)
>>         Removable device: locked, tray closed
>>     (qemu) info block cd
>> 
>>     cd: r7.iso (raw, read-only)
>>         Removable device: not locked, tray open
>> 
>> Except it forgets to unmount!  dmesg has "VFS: busy inodes on changed
>> media or resized disk sr0".
>> 
>> Need somebody to find out how exactly this fails, and whether it's a
>> guest bug or a QEMU bug.
>
>
> The guest unlocks DVD (by sending ALLOW PERMIT MEDIUM REMOVAL) and stops
> DVD (by sending START_STOP). Is there any other message missing which would
> do real physical eject?

START_STOP has a "load/eject" flag that causes load with start and eject
with stop.

> What does it have to do with unmount (which is purely the guest software
> state)?

Not sure I understand you here.

A guest that voluntarily ejects a medium while keeping it mounted gets
what it asked for: breakage.

Re: [Qemu-devel] [PATCH 1/3] scsi-disk: close drive on START_STOP

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Paolo Bonzini <pbonzini@redhat.com> writes:
>
>> Il 04/12/2013 05:55, Alexey Kardashevskiy ha scritto:
>>> Normally the user is expected to eject DVD if it is not locked by
>>> the guest. eject_device() makes few checks and calls bdrv_close()
>>> if DVD is not in use.
>>> 
>>> However it is still possible to eject DVD even if it is in use.
>>> For that, QEMU sets "eject requested" flag, the guest reads it, issues
>>> ALLOW_MEDIUM_REMOVAL(enable=1) and START_STOP(start=0). But in this case,
>>> bdrv_close() is not called anywhere so it remains "inserted" in QEMU's
>>> terms.
>>
>> This is expected behavior, and matches what IDE does.
>>
>> Markus, can you confirm?
>
> Confirmed.  See commit 4be9762.
>
> Alexey, monitor commands eject does two things: it first opens the tray,
> and if that works, it removes the medium.
>
> If the tray is locked closed, it tells the device model that eject was
> requested.  Works just like the physical eject button.
>
> With -f, it then rips out the medium.  This is similar to opening the
> tray with a unbent paperclip.  Let's ignore this case.
>
> The scsi-cd device model tells the guest about the eject request.  A
> well-behaved guest will then command the device to unlock and open the
> tray.
>
> The guest uses the same commands on behalf of its applications,
> e.g. /usr/bin/eject.
>
> Your patch changes behavior of "eject /dev/sr0 && eject -t /dev/sr0":
> you no longer get the same medium back.  You normally do with real
> hardware.

Alexey asked me for details on IRC.

    $ qemu -nodefaults -monitor stdio -S -machine accel=kvm -m 512 -display vnc=:0 -device cirrus-vga -drive if=none,id=disk,file=test.qcow2 -device ide-hd,drive=disk,bus=ide.0 -drive if=none,id=cd,file=f16.iso -device ide-cd,drive=cd,bus=ide.1
    QEMU 1.7.50 monitor - type 'help' for more information
    (qemu) info block cd

    cd: f16.iso (raw)
        Removable device: not locked, tray closed

Boot the guest (Fedora 16, no X)

    (qemu) c

The guest locked the tray:

    (qemu) info block cd

    cd: f16.iso (raw)
        Removable device: locked, tray closed

In the guest, log in as root on the console, and run

    # eject /dev/sr0

Makes the guest open the tray:

    (qemu) info block cd

    cd: f16.iso (raw)
        Removable device: locked, tray open

In the guest, run

    # eject -t /dev/sr0

Makes the guest close the tray:

    (qemu) info block cd

    cd: f16.iso (raw)
        Removable device: locked, tray closed

Verify the guest can access the medium:

    # mount -r /dev/sr0 /mnt

> The somewhat unfortunate consequence is that monitor command eject can
> only remove the medium when the tray is not locked.

Re: [Qemu-devel] [PATCH 1/3] scsi-disk: close drive on START_STOP

[Alexey Kardashevskiy]

On 12/05/2013 11:29 PM, Markus Armbruster wrote:
> Markus Armbruster <armbru@redhat.com> writes:
> 
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>
>>> Il 04/12/2013 05:55, Alexey Kardashevskiy ha scritto:
>>>> Normally the user is expected to eject DVD if it is not locked by
>>>> the guest. eject_device() makes few checks and calls bdrv_close()
>>>> if DVD is not in use.
>>>>
>>>> However it is still possible to eject DVD even if it is in use.
>>>> For that, QEMU sets "eject requested" flag, the guest reads it, issues
>>>> ALLOW_MEDIUM_REMOVAL(enable=1) and START_STOP(start=0). But in this case,
>>>> bdrv_close() is not called anywhere so it remains "inserted" in QEMU's
>>>> terms.
>>>
>>> This is expected behavior, and matches what IDE does.
>>>
>>> Markus, can you confirm?
>>
>> Confirmed.  See commit 4be9762.
>>
>> Alexey, monitor commands eject does two things: it first opens the tray,
>> and if that works, it removes the medium.
>>
>> If the tray is locked closed, it tells the device model that eject was
>> requested.  Works just like the physical eject button.
>>
>> With -f, it then rips out the medium.  This is similar to opening the
>> tray with a unbent paperclip.  Let's ignore this case.
>>
>> The scsi-cd device model tells the guest about the eject request.  A
>> well-behaved guest will then command the device to unlock and open the
>> tray.
>>
>> The guest uses the same commands on behalf of its applications,
>> e.g. /usr/bin/eject.
>>
>> Your patch changes behavior of "eject /dev/sr0 && eject -t /dev/sr0":
>> you no longer get the same medium back.  You normally do with real
>> hardware.
> 
> Alexey asked me for details on IRC.
> 
>     $ qemu -nodefaults -monitor stdio -S -machine accel=kvm -m 512 -display vnc=:0 -device cirrus-vga -drive if=none,id=disk,file=test.qcow2 -device ide-hd,drive=disk,bus=ide.0 -drive if=none,id=cd,file=f16.iso -device ide-cd,drive=cd,bus=ide.1
>     QEMU 1.7.50 monitor - type 'help' for more information
>     (qemu) info block cd
> 
>     cd: f16.iso (raw)
>         Removable device: not locked, tray closed
> 
> Boot the guest (Fedora 16, no X)
> 
>     (qemu) c
> 
> The guest locked the tray:
> 
>     (qemu) info block cd
> 
>     cd: f16.iso (raw)
>         Removable device: locked, tray closed
> 
> In the guest, log in as root on the console, and run
> 
>     # eject /dev/sr0
> 
> Makes the guest open the tray:
> 
>     (qemu) info block cd
> 
>     cd: f16.iso (raw)
>         Removable device: locked, tray open
> 
> In the guest, run
> 
>     # eject -t /dev/sr0
> 
> Makes the guest close the tray:
> 
>     (qemu) info block cd
> 
>     cd: f16.iso (raw)
>         Removable device: locked, tray closed
> 
> Verify the guest can access the medium:
> 
>     # mount -r /dev/sr0 /mnt
> 
>> The somewhat unfortunate consequence is that monitor command eject can
>> only remove the medium when the tray is not locked.

Thanks!

Just out of curiosity. A lot (in fact, all around me) dvd drives do not
support trayclose as they are in laptops or servers (which use the same
laptop models). I cannot even verify how this "eject -t" exactly works - no
hardware around me. And even if I could find it, I could easily take the
disc off the tray in that short period of time between tray is open and
tray is closed but we still absolutely want "eject" + "eject -t" to work as
you described.

Why exactly? :) Only because change of behavior is bad? Just asking. Thanks.



-- 
Alexey

Re: [Qemu-devel] [PATCH 1/3] scsi-disk: close drive on START_STOP

[Paolo Bonzini]

Il 05/12/2013 13:42, Alexey Kardashevskiy ha scritto:
> Thanks!
> 
> Just out of curiosity. A lot (in fact, all around me) dvd drives do not
> support trayclose as they are in laptops or servers (which use the same
> laptop models). I cannot even verify how this "eject -t" exactly works - no
> hardware around me. And even if I could find it, I could easily take the
> disc off the tray in that short period of time between tray is open and
> tray is closed but we still absolutely want "eject" + "eject -t" to work as
> you described.

Taking the disc off the tray is equivalent to going to the monitor and
doing "eject -f cd".

I.e. programmatic actions leave the disc in (at least if you do not
consider laptops and servers; but the guest can detect whether the tray
can be auto-closed, and we tell it that it can).  Out-of-band user
actions force the disc out.

> Why exactly? :) Only because change of behavior is bad? Just asking. Thanks.

The only practical use of CDs is installation, and it took a looooong
time to get it to work with all sorts of guests.

Paolo

Re: [Qemu-devel] [PATCH 1/3] scsi-disk: close drive on START_STOP

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 05/12/2013 13:42, Alexey Kardashevskiy ha scritto:
>> Thanks!
>> 
>> Just out of curiosity. A lot (in fact, all around me) dvd drives do not
>> support trayclose as they are in laptops or servers (which use the same
>> laptop models). I cannot even verify how this "eject -t" exactly works - no
>> hardware around me. And even if I could find it, I could easily take the
>> disc off the tray in that short period of time between tray is open and
>> tray is closed but we still absolutely want "eject" + "eject -t" to work as
>> you described.
>
> Taking the disc off the tray is equivalent to going to the monitor and
> doing "eject -f cd".

Actually, monitor command "eject -f" is like poking the little button
hidden in that little hole with an unbent paperclip or something, which
makes the drive eject whether the OS likes it or not (and tends to make
the OS rather upset), then removing the medium.

Monitor command "eject" without -f is like pushing the "normal" button,
and if that ejects, and the medium wasn't locked, removing the medium.
Yes, that's not exactly elegant semantics.

/usr/bin/eject is something else entirely: it sends commands to the
device.  Such commands can make the drive eject and load, but they
cannot cause the medium removal without further user action.  Exactly
the same on real and virtual systems.

> I.e. programmatic actions leave the disc in (at least if you do not
> consider laptops and servers; but the guest can detect whether the tray
> can be auto-closed, and we tell it that it can).  Out-of-band user
> actions force the disc out.
>
>> Why exactly? :) Only because change of behavior is bad? Just asking. Thanks.
>
> The only practical use of CDs is installation, and it took a looooong
> time to get it to work with all sorts of guests.

Indeed.

[Qemu-devel] [PATCH 2/3] scsi-disk: check for meduim on ALLOW_MEDIUM_REMOVAL

[Alexey Kardashevskiy]

This prevents the guest from preventing DVD medium removal when
there is no medium.

Without this, if the user has ejected a DVD, it is possible to
have a block device with an open tray, no media but locked.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/scsi/scsi-disk.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 08b6135..22b6859 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1622,7 +1622,6 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
     case RELEASE:
     case RELEASE_10:
     case START_STOP:
-    case ALLOW_MEDIUM_REMOVAL:
     case GET_CONFIGURATION:
     case GET_EVENT_STATUS_NOTIFICATION:
     case MECHANISM_STATUS:
-- 
1.8.4.rc4

Re: [Qemu-devel] [PATCH 2/3] scsi-disk: check for meduim on ALLOW_MEDIUM_REMOVAL

[Paolo Bonzini]

Il 04/12/2013 05:55, Alexey Kardashevskiy ha scritto:
> This prevents the guest from preventing DVD medium removal when
> there is no medium.
> 
> Without this, if the user has ejected a DVD, it is possible to
> have a block device with an open tray, no media but locked.

I need to check this against a real disk.  Thanks!

Paolo

Re: [Qemu-devel] [PATCH 2/3] scsi-disk: check for meduim on ALLOW_MEDIUM_REMOVAL

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 04/12/2013 05:55, Alexey Kardashevskiy ha scritto:
>> This prevents the guest from preventing DVD medium removal when
>> there is no medium.
>> 
>> Without this, if the user has ejected a DVD, it is possible to
>> have a block device with an open tray, no media but locked.
>
> I need to check this against a real disk.  Thanks!

If I remember correctly, I did, and it works.  You can still close the
open tray in this state.

Re: [Qemu-devel] [PATCH 2/3] scsi-disk: check for meduim on ALLOW_MEDIUM_REMOVAL

[Alexey Kardashevskiy]

On 12/04/2013 08:35 PM, Markus Armbruster wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> Il 04/12/2013 05:55, Alexey Kardashevskiy ha scritto:
>>> This prevents the guest from preventing DVD medium removal when
>>> there is no medium.
>>>
>>> Without this, if the user has ejected a DVD, it is possible to
>>> have a block device with an open tray, no media but locked.
>>
>> I need to check this against a real disk.  Thanks!
> 
> If I remember correctly, I did, and it works.  You can still close the
> open tray in this state.


I thought it is about NOT letting close the open tray. So the hardware can
be programmed to ignore the hardware "eject" button when the tray is open?


-- 
Alexey

Re: [Qemu-devel] [PATCH 2/3] scsi-disk: check for meduim on ALLOW_MEDIUM_REMOVAL

[Markus Armbruster]

Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> On 12/04/2013 08:35 PM, Markus Armbruster wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> 
>>> Il 04/12/2013 05:55, Alexey Kardashevskiy ha scritto:
>>>> This prevents the guest from preventing DVD medium removal when
>>>> there is no medium.
>>>>
>>>> Without this, if the user has ejected a DVD, it is possible to
>>>> have a block device with an open tray, no media but locked.
>>>
>>> I need to check this against a real disk.  Thanks!
>> 
>> If I remember correctly, I did, and it works.  You can still close the
>> open tray in this state.
>
>
> I thought it is about NOT letting close the open tray. So the hardware can
> be programmed to ignore the hardware "eject" button when the tray is open?

>From memory: you can lock the medium whether the tray is open or not.
The lock applies to the button operating the tray.  It prevents medium
eject.  It never prevents closing an empty tray.  I don't remember
whether opening an empty tray is still permitted.

Re: [Qemu-devel] [PATCH 2/3] scsi-disk: check for meduim on ALLOW_MEDIUM_REMOVAL

[Paolo Bonzini]

Il 04/12/2013 05:55, Alexey Kardashevskiy ha scritto:
> This prevents the guest from preventing DVD medium removal when
> there is no medium.
> 
> Without this, if the user has ejected a DVD, it is possible to
> have a block device with an open tray, no media but locked.

This state is weird but should not be a problem.  The tray can
still be closed when it is open and locked:

        if (!start && !s->tray_open && s->tray_locked) {
            scsi_check_condition(r,
                                 bdrv_is_inserted(s->qdev.conf.bs)
                                 ? SENSE_CODE(ILLEGAL_REQ_REMOVAL_PREVENTED)
                                 : SENSE_CODE(NOT_READY_REMOVAL_PREVENTED));
            return -1;
        }

Note that start && s->tray_open && s->tray_locked executes the
command normally, closing the tray and reporting that as a QMP
event.

Paolo

> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  hw/scsi/scsi-disk.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 08b6135..22b6859 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -1622,7 +1622,6 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
>      case RELEASE:
>      case RELEASE_10:
>      case START_STOP:
> -    case ALLOW_MEDIUM_REMOVAL:
>      case GET_CONFIGURATION:
>      case GET_EVENT_STATUS_NOTIFICATION:
>      case MECHANISM_STATUS:
> 

[Qemu-devel] [PATCH 3/3] scsi debug: print command name in debug

[Alexey Kardashevskiy]

This makes scsi_command_name() public.

This makes use of scsi_command_name() in debug output for scsi-disk and
spapr-vscsi host bus adapter.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/scsi/scsi-bus.c    | 5 ++++-
 hw/scsi/scsi-disk.c   | 3 ++-
 hw/scsi/spapr_vscsi.c | 5 +++--
 include/block/scsi.h  | 2 ++
 4 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index ea916d1..5bf9544 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1399,7 +1399,7 @@ int scsi_build_sense(uint8_t *in_buf, int in_len,
     }
 }
 
-static const char *scsi_command_name(uint8_t cmd)
+const char *scsi_command_name(uint8_t cmd)
 {
     static const char *names[] = {
         [ TEST_UNIT_READY          ] = "TEST_UNIT_READY",
@@ -1515,6 +1515,9 @@ static const char *scsi_command_name(uint8_t cmd)
         [ SET_READ_AHEAD           ] = "SET_READ_AHEAD",
         [ ALLOW_OVERWRITE          ] = "ALLOW_OVERWRITE",
         [ MECHANISM_STATUS         ] = "MECHANISM_STATUS",
+
+        [ GET_EVENT_STATUS_NOTIFICATION ] = "GET_EVENT_STATUS_NOTIFICATION",
+        [ READ_DISC_INFORMATION    ] = "READ_DISC_INFORMATION",
     };
 
     if (cmd >= ARRAY_SIZE(names) || names[cmd] == NULL)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 22b6859..d6cea68 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1865,7 +1865,8 @@ static int32_t scsi_disk_emulate_command(SCSIRequest *req, uint8_t *buf)
                                         scsi_aio_complete, r);
         return 0;
     default:
-        DPRINTF("Unknown SCSI command (%2.2x)\n", buf[0]);
+        DPRINTF("Unknown SCSI command (%2.2x=%s)\n", buf[0],
+                scsi_command_name(buf[0]));
         scsi_check_condition(r, SENSE_CODE(INVALID_OPCODE));
         return 0;
     }
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index c0c46d7..d5d18f9 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -741,8 +741,9 @@ static int vscsi_queue_cmd(VSCSIState *s, vscsi_req *req)
     req->sreq = scsi_req_new(sdev, req->qtag, lun, srp->cmd.cdb, req);
     n = scsi_req_enqueue(req->sreq);
 
-    DPRINTF("VSCSI: Queued command tag 0x%x CMD 0x%x LUN %d ret: %d\n",
-            req->qtag, srp->cmd.cdb[0], lun, n);
+    DPRINTF("VSCSI: Queued command tag 0x%x CMD 0x%x=%s LUN %d ret: %d\n",
+            req->qtag, srp->cmd.cdb[0], scsi_command_name(srp->cmd.cdb[0]),
+            lun, n);
 
     if (n) {
         /* Transfer direction must be set before preprocessing the
diff --git a/include/block/scsi.h b/include/block/scsi.h
index 9ab045b..edde960 100644
--- a/include/block/scsi.h
+++ b/include/block/scsi.h
@@ -143,6 +143,8 @@
 #define READ_CD               0xbe
 #define SEND_DVD_STRUCTURE    0xbf
 
+const char *scsi_command_name(uint8_t cmd);
+
 /*
  * SERVICE ACTION IN subcodes
  */
-- 
1.8.4.rc4