[Qemu-devel] [PATCH v2 0/3] usb-msd: Add usb-storage, removable=on|off property

[Qemu-devel] [PATCH v2 0/3] usb-msd: Add usb-storage, removable=on|off property

[Stefan Hajnoczi]

Allow overriding the SCSI INQUIRY removable (RMB) bit for scsi-disk and usb-msd
devices.  In particular this addresses the problem that some usb-msd devices
have the bit set while other do not have it set.  Now the user can choose and
get desired guest behavior.

qemu -usb
     -drive if=none,file=test.img,cache=none,id=disk0
     -device usb-storage,drive=disk0,removable=on

The default is off.

v2:
 * Rewritten to override the bit at the scsi-disk level

 hw/pci-hotplug.c |    2 +-
 hw/scsi-bus.c    |    8 ++++++--
 hw/scsi-disk.c   |    3 +++
 hw/scsi.h        |    3 ++-
 hw/usb-msd.c     |    4 +++-
 5 files changed, 15 insertions(+), 5 deletions(-)

[Qemu-devel] [PATCH v2 1/3] scsi-disk: Allow overriding SCSI INQUIRY removable bit

[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>
---
 hw/scsi-disk.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

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(),
     },
 };
-- 
1.7.2.3

Re: [Qemu-devel] [PATCH v2 1/3] scsi-disk: Allow overriding SCSI INQUIRY removable bit

[Kevin Wolf]

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

Re: [Qemu-devel] [PATCH v2 1/3] scsi-disk: Allow overriding SCSI INQUIRY removable bit

[Stefan Hajnoczi]

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

Re: [Qemu-devel] [PATCH v2 1/3] scsi-disk: Allow overriding SCSI INQUIRY removable bit

[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.

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

Re: [Qemu-devel] [PATCH v2 1/3] scsi-disk: Allow overriding SCSI INQUIRY removable bit

[Kevin Wolf]

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

Re: [Qemu-devel] [PATCH v2 1/3] scsi-disk: Allow overriding SCSI INQUIRY removable bit

[Markus Armbruster]

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

[Qemu-devel] [PATCH v2 2/3] scsi: Allow SCSI devices to override the removable bit

[Stefan Hajnoczi]

Some SCSI devices may wish to override the removable bit.  Add support
for a qdev property on the SCSI device.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 hw/pci-hotplug.c |    2 +-
 hw/scsi-bus.c    |    8 ++++++--
 hw/scsi.h        |    3 ++-
 hw/usb-msd.c     |    2 +-
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 716133c..270a982 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -90,7 +90,7 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
      * specified).
      */
     dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
-    scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit);
+    scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit, false);
     if (!scsidev) {
         return -1;
     }
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 7febb86..ceeb4ec 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -87,7 +87,8 @@ void scsi_qdev_register(SCSIDeviceInfo *info)
 }
 
 /* handle legacy '-drive if=scsi,...' cmd line args */
-SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv, int unit)
+SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv,
+                                      int unit, bool removable)
 {
     const char *driver;
     DeviceState *dev;
@@ -95,6 +96,9 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv, int
     driver = bdrv_is_sg(bdrv) ? "scsi-generic" : "scsi-disk";
     dev = qdev_create(&bus->qbus, driver);
     qdev_prop_set_uint32(dev, "scsi-id", unit);
+    if (qdev_prop_exists(dev, "removable")) {
+        qdev_prop_set_bit(dev, "removable", removable);
+    }
     if (qdev_prop_set_drive(dev, "drive", bdrv) < 0) {
         qdev_free(dev);
         return NULL;
@@ -117,7 +121,7 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
             continue;
         }
         qemu_opts_loc_restore(dinfo->opts);
-        if (!scsi_bus_legacy_add_drive(bus, dinfo->bdrv, unit)) {
+        if (!scsi_bus_legacy_add_drive(bus, dinfo->bdrv, unit, false)) {
             res = -1;
             break;
         }
diff --git a/hw/scsi.h b/hw/scsi.h
index bf02adf..846fbba 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -94,7 +94,8 @@ static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
     return DO_UPCAST(SCSIBus, qbus, d->qdev.parent_bus);
 }
 
-SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv, int unit);
+SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockDriverState *bdrv,
+                                      int unit, bool removable);
 int scsi_bus_legacy_handle_cmdline(SCSIBus *bus);
 
 SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun);
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 0a95d8d..ee897b6 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -544,7 +544,7 @@ static int usb_msd_initfn(USBDevice *dev)
 
     s->dev.speed = USB_SPEED_FULL;
     scsi_bus_new(&s->bus, &s->dev.qdev, 0, 1, usb_msd_command_complete);
-    s->scsi_dev = scsi_bus_legacy_add_drive(&s->bus, bs, 0);
+    s->scsi_dev = scsi_bus_legacy_add_drive(&s->bus, bs, 0, false);
     if (!s->scsi_dev) {
         return -1;
     }
-- 
1.7.2.3

[Qemu-devel] [PATCH v2 3/3] usb-msd: Propagate removable bit to SCSI device

[Stefan Hajnoczi]

USB Mass Storage Devices sometimes have the RMB (removable) bit set in
the SCSI INQUIRY response.  Thumbdrives tend to have the bit set whereas
hard disks do not.

Operating systems differentiate between removable devices and fixed
devices.  Under Linux, the anaconda installer looks for removable
devices.  Under Windows, only fixed devices may have more than one
partition and AutoRun is also affected by the removable bit.

For these reasons, allow USB Mass Storage Devices to override the
removable bit:

qemu -usb
     -drive if=none,file=test.img,cache=none,id=disk0
     -device usb-storage,drive=disk0,removable=on

The default is off.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 hw/usb-msd.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index ee897b6..f593c46 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -50,6 +50,7 @@ typedef struct {
     SCSIBus bus;
     BlockConf conf;
     SCSIDevice *scsi_dev;
+    uint32_t removable;
     int result;
     /* For async completion.  */
     USBPacket *packet;
@@ -544,7 +545,7 @@ static int usb_msd_initfn(USBDevice *dev)
 
     s->dev.speed = USB_SPEED_FULL;
     scsi_bus_new(&s->bus, &s->dev.qdev, 0, 1, usb_msd_command_complete);
-    s->scsi_dev = scsi_bus_legacy_add_drive(&s->bus, bs, 0, false);
+    s->scsi_dev = scsi_bus_legacy_add_drive(&s->bus, bs, 0, !!s->removable);
     if (!s->scsi_dev) {
         return -1;
     }
@@ -634,6 +635,7 @@ static struct USBDeviceInfo msd_info = {
     .usbdevice_init = usb_msd_init,
     .qdev.props     = (Property[]) {
         DEFINE_BLOCK_PROPERTIES(MSDState, conf),
+        DEFINE_PROP_BIT("removable", MSDState, removable, 1, false),
         DEFINE_PROP_END_OF_LIST(),
     },
 };
-- 
1.7.2.3