Stefan Hajnoczi <stefa...@linux.vnet.ibm.com> writes:
> Some SCSI devices may wish to override the removable bit. Add support
> for a qdev property on the SCSI device.
I find this description a bit misleading. The qdev property is added in
1/4. Here, you merely extend scsi_bus_legacy_add_drive() to provide
access to it. Its primary users (-drive if=scsi & friends) don't use
that access. But there's another user: usb_msd_initfn()[*], and 3/4
will make that one use the access.
I guess I'd squash 2+3 together, but that's strictly a matter of taste.
>
> Signed-off-by: Stefan Hajnoczi <stefa...@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")) {
Isn't this just a funky way to check for "scsi-disk"?
Removable gets silently ignored for -device usb-storage with a scsi
generic drive. Worth nothing in 4/4.
> + 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;
> }
[...]
[*] I hate that usb_msd_initfn() uses scsi_bus_legacy_add_drive().
Actually, I hate pretty much everything about usb-msd.c. But it's not
your fault.
