Patchwork [v2,1/3] scsi-disk: Allow overriding SCSI INQUIRY removable bit

login
register
mail settings
Submitter Stefan Hajnoczi
Date Jan. 18, 2011, 10:10 a.m.
Message ID <1295345442-16218-2-git-send-email-stefanha@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/79279/
State New
Headers show

Comments

Stefan Hajnoczi - Jan. 18, 2011, 10:10 a.m.
Provide the "removable" qdev property bit to override the SCSI INQUIRY
removable (RMB) bit for non-CDROM devices.  This will be used by USB
Mass Storage Devices, which sometimes have this guest-visible bit set
and sometimes do not.  They therefore requires a means for user
configuration.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 hw/scsi-disk.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
Kevin Wolf - Jan. 18, 2011, 11:39 a.m.
Am 18.01.2011 11:10, schrieb Stefan Hajnoczi:
> Provide the "removable" qdev property bit to override the SCSI INQUIRY
> removable (RMB) bit for non-CDROM devices.  This will be used by USB
> Mass Storage Devices, which sometimes have this guest-visible bit set
> and sometimes do not.  They therefore requires a means for user
> configuration.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Should we print an error message when the user tries to make a CD-ROM
non-removable instead of silently ignoring the option?

Kevin
Stefan Hajnoczi - Jan. 18, 2011, 12:06 p.m.
On Tue, Jan 18, 2011 at 11:39 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 18.01.2011 11:10, schrieb Stefan Hajnoczi:
>> Provide the "removable" qdev property bit to override the SCSI INQUIRY
>> removable (RMB) bit for non-CDROM devices.  This will be used by USB
>> Mass Storage Devices, which sometimes have this guest-visible bit set
>> and sometimes do not.  They therefore requires a means for user
>> configuration.
>>
>> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>
> Should we print an error message when the user tries to make a CD-ROM
> non-removable instead of silently ignoring the option?

Good point.  I will add a check in scsi_disk_initfn() for v3.

Stefan
Stefan Hajnoczi - Jan. 21, 2011, 10:45 a.m.
On Tue, Jan 18, 2011 at 12:06 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Tue, Jan 18, 2011 at 11:39 AM, Kevin Wolf <kwolf@redhat.com> wrote:
>> Am 18.01.2011 11:10, schrieb Stefan Hajnoczi:
>>> Provide the "removable" qdev property bit to override the SCSI INQUIRY
>>> removable (RMB) bit for non-CDROM devices.  This will be used by USB
>>> Mass Storage Devices, which sometimes have this guest-visible bit set
>>> and sometimes do not.  They therefore requires a means for user
>>> configuration.
>>>
>>> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>>
>> Should we print an error message when the user tries to make a CD-ROM
>> non-removable instead of silently ignoring the option?
>
> Good point.  I will add a check in scsi_disk_initfn() for v3.

Actually this case is hard to check against.  The removable property
is a boolean that defaults to false.  We can't detect the difference
between default, cleared, or set.

I'm sending out a new version of the patch series that updates
docs/qdev-device-use.txt to describe how the removable property works.

Stefan
Kevin Wolf - Jan. 21, 2011, 11:09 a.m.
Am 21.01.2011 11:45, schrieb Stefan Hajnoczi:
> On Tue, Jan 18, 2011 at 12:06 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
>> On Tue, Jan 18, 2011 at 11:39 AM, Kevin Wolf <kwolf@redhat.com> wrote:
>>> Am 18.01.2011 11:10, schrieb Stefan Hajnoczi:
>>>> Provide the "removable" qdev property bit to override the SCSI INQUIRY
>>>> removable (RMB) bit for non-CDROM devices.  This will be used by USB
>>>> Mass Storage Devices, which sometimes have this guest-visible bit set
>>>> and sometimes do not.  They therefore requires a means for user
>>>> configuration.
>>>>
>>>> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>>>
>>> Should we print an error message when the user tries to make a CD-ROM
>>> non-removable instead of silently ignoring the option?
>>
>> Good point.  I will add a check in scsi_disk_initfn() for v3.
> 
> Actually this case is hard to check against.  The removable property
> is a boolean that defaults to false.  We can't detect the difference
> between default, cleared, or set.

Hm, I see... Maybe we should make scsi-disk and scsi-cdrom different
devices in the long run, then scsi-cdrom could default to true (or
rather not have the property at all).

> I'm sending out a new version of the patch series that updates
> docs/qdev-device-use.txt to describe how the removable property works.

Okay, let's just document how it works.

Kevin
Markus Armbruster - Jan. 21, 2011, 5:38 p.m.
Kevin Wolf <kwolf@redhat.com> writes:

> Am 21.01.2011 11:45, schrieb Stefan Hajnoczi:
>> On Tue, Jan 18, 2011 at 12:06 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
>>> On Tue, Jan 18, 2011 at 11:39 AM, Kevin Wolf <kwolf@redhat.com> wrote:
>>>> Am 18.01.2011 11:10, schrieb Stefan Hajnoczi:
>>>>> Provide the "removable" qdev property bit to override the SCSI INQUIRY
>>>>> removable (RMB) bit for non-CDROM devices.  This will be used by USB
>>>>> Mass Storage Devices, which sometimes have this guest-visible bit set
>>>>> and sometimes do not.  They therefore requires a means for user
>>>>> configuration.
>>>>>
>>>>> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>>>>
>>>> Should we print an error message when the user tries to make a CD-ROM
>>>> non-removable instead of silently ignoring the option?
>>>
>>> Good point.  I will add a check in scsi_disk_initfn() for v3.
>> 
>> Actually this case is hard to check against.  The removable property
>> is a boolean that defaults to false.  We can't detect the difference
>> between default, cleared, or set.
>
> Hm, I see... Maybe we should make scsi-disk and scsi-cdrom different
> devices in the long run,

Yes.  Need to dig out and rebase my patches for it.

>                          then scsi-cdrom could default to true (or
> rather not have the property at all).
>
>> I'm sending out a new version of the patch series that updates
>> docs/qdev-device-use.txt to describe how the removable property works.
>
> Okay, let's just document how it works.
>
> Kevin

Patch

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 6cb317c..db423ca 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -72,6 +72,7 @@  struct SCSIDiskState
     /* The qemu block layer uses a fixed 512 byte sector size.
        This is the number of 512 byte blocks in a single scsi sector.  */
     int cluster_size;
+    uint32_t removable;
     uint64_t max_lba;
     QEMUBH *bh;
     char *version;
@@ -552,6 +553,7 @@  static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
         memcpy(&outbuf[16], "QEMU CD-ROM     ", 16);
     } else {
         outbuf[0] = 0;
+        outbuf[1] = s->removable ? 0x80 : 0;
         memcpy(&outbuf[16], "QEMU HARDDISK   ", 16);
     }
     memcpy(&outbuf[8], "QEMU    ", 8);
@@ -1295,6 +1297,7 @@  static SCSIDeviceInfo scsi_disk_info = {
         DEFINE_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf),
         DEFINE_PROP_STRING("ver",  SCSIDiskState, version),
         DEFINE_PROP_STRING("serial",  SCSIDiskState, serial),
+        DEFINE_PROP_BIT("removable", SCSIDiskState, removable, 1, false),
         DEFINE_PROP_END_OF_LIST(),
     },
 };