Re: [PATCH] usb-storage: Allow manually adding SCSI device

[Akihiko Odaki]

On 2025/03/03 21:17, Paolo Bonzini wrote:
On 3/3/25 11:28, Akihiko Odaki wrote:
usb-storage automatically adds a SCSI device, but it limits
configurability of the added SCSI device and causes usability
problems as observed in:
https://gitlab.com/libvirt/libvirt/-/issues/368
Allow manually adding SCSI device when the drive option is not
specified.

I might be misunderstanding what you're doing, but can't you do that 
already with usb-bot?

I wtithdraw this patch.

I wrote patches for libvirt to fix its issue mentioned in the patch 
message according to your suggestion to use usb-bot and it works nicely:
https://lists.libvirt.org/archives/list/de...@lists.libvirt.org/thread/SVXUWW2426H73XBXJXDUMTDJM3YL37QU/

Regards,
Akihiko Odaki


Paolo

Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com>
---
  hw/usb/dev-storage-classic.c | 33 ++++++++++++++++-----------------
  1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/hw/usb/dev-storage-classic.c b/hw/usb/dev-storage-classic.c
index 
56ef39da2e634d1639a07ac4636cdaa000989f5f..33e5a7cfc8bdf3f92b18014e885771aee6d32f5e 100644
--- a/hw/usb/dev-storage-classic.c
+++ b/hw/usb/dev-storage-classic.c
@@ -33,10 +33,9 @@ static void usb_msd_storage_realize(USBDevice *dev, 
Error **errp)
      BlockBackend *blk = s->conf.blk;
      SCSIDevice *scsi_dev;
-    if (!blk) {
-        error_setg(errp, "drive property not set");
-        return;
-    }
+    usb_desc_create_serial(dev);
+    scsi_bus_init(&s->bus, sizeof(s->bus), DEVICE(dev),
+                 &usb_msd_scsi_info_storage);
      /*
       * Hack alert: this pretends to be a block device, but it's really
@@ -48,23 +47,23 @@ static void usb_msd_storage_realize(USBDevice 
*dev, Error **errp)
       *
       * The hack is probably a bad idea.
       */
-    blk_ref(blk);
-    blk_detach_dev(blk, DEVICE(s));
-    s->conf.blk = NULL;
+    if (blk) {
+        blk_ref(blk);
+        blk_detach_dev(blk, DEVICE(s));
+        s->conf.blk = NULL;
+
+        scsi_dev = scsi_bus_legacy_add_drive(&s->bus, blk, 0, !!s- 
>removable,
+                                             &s->conf, dev->serial, 
errp);
+        blk_unref(blk);
+        if (!scsi_dev) {
+            return;
+        }
+        s->scsi_dev = scsi_dev;
+    }
-    usb_desc_create_serial(dev);
      usb_desc_init(dev);
      dev->flags |= (1 << USB_DEV_FLAG_IS_SCSI_STORAGE);
-    scsi_bus_init(&s->bus, sizeof(s->bus), DEVICE(dev),
-                 &usb_msd_scsi_info_storage);
-    scsi_dev = scsi_bus_legacy_add_drive(&s->bus, blk, 0, !!s- 
>removable,
-                                         &s->conf, dev->serial, errp);
-    blk_unref(blk);
-    if (!scsi_dev) {
-        return;
-    }
      usb_msd_handle_reset(dev);
-    s->scsi_dev = scsi_dev;
  }
  static const Property msd_properties[] = {

---
base-commit: b69801dd6b1eb4d107f7c2f643adf0a4e3ec9124
change-id: 20250301-usb-5dde4bcb1467

Best regards,