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

[Akihiko Odaki <akihiko.odaki@daynix.com>]

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/devel@lists.libvirt.org/thread/SVXUWW2426H73XBXJXDUMTDJM3YL37QU/

Regards,
Akihiko Odaki

> 
> Paolo
> 
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@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,
>