Commit 30896374 started to pass the full BlockConf from usb-storage to
scsi-disk, while previously only a few select properties would be
forwarded. This enables the user to set more properties, e.g. the block
size, that are actually taking effect.

However, now the calls to blkconf_apply_backend_options() and
blkconf_blocksizes() in usb_msd_storage_realize() that modify some of
these properties take effect, too, instead of being silently ignored.
This means at least that the block sizes get an unconditional default of
512 bytes before the configuration is passed to scsi-disk.

Before commit 30896374, the property wouldn't be set for scsi-disk and
therefore the device dependent defaults would apply - 512 for scsi-hd,
but 2048 for scsi-cd. The latter default has now become 512, too, which
makes at least Windows 11 installation fail when installing from
usb-storage.

Fix this by simply not calling these functions any more in usb-storage
and passing BlockConf on unmodified (except for the BlockBackend). The
same functions are called by the SCSI code anyway and it sets the right
defaults for the actual media type.

Fixes: 308963746169 ('scsi: Don't ignore most usb-storage properties')
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2260
Reported-by: Jonas Svensson
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
Considering this a candidate for 9.0 given that we're already having an
rc4, it's a regression from 8.2 and breaks installing Windows from USB

 hw/usb/dev-storage-classic.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/hw/usb/dev-storage-classic.c b/hw/usb/dev-storage-classic.c
index 50a3ad6285..6147387dc6 100644
--- a/hw/usb/dev-storage-classic.c
+++ b/hw/usb/dev-storage-classic.c
@@ -38,15 +38,6 @@ static void usb_msd_storage_realize(USBDevice *dev, Error **errp)
         return;
     }
 
-    if (!blkconf_blocksizes(&s->conf, errp)) {
-        return;
-    }
-
-    if (!blkconf_apply_backend_options(&s->conf, !blk_supports_write_perm(blk),
-                                       true, errp)) {
-        return;
-    }
-
     /*
      * Hack alert: this pretends to be a block device, but it's really
      * a SCSI bus that can serve only a single device, which it
-- 
2.44.0

On 12.04.24 16:42, Kevin Wolf wrote:
> Commit 30896374 started to pass the full BlockConf from usb-storage to
> scsi-disk, while previously only a few select properties would be
> forwarded. This enables the user to set more properties, e.g. the block
> size, that are actually taking effect.
>
> However, now the calls to blkconf_apply_backend_options() and
> blkconf_blocksizes() in usb_msd_storage_realize() that modify some of
> these properties take effect, too, instead of being silently ignored.
> This means at least that the block sizes get an unconditional default of
> 512 bytes before the configuration is passed to scsi-disk.
>
> Before commit 30896374, the property wouldn't be set for scsi-disk and
> therefore the device dependent defaults would apply - 512 for scsi-hd,
> but 2048 for scsi-cd. The latter default has now become 512, too, which
> makes at least Windows 11 installation fail when installing from
> usb-storage.
>
> Fix this by simply not calling these functions any more in usb-storage
> and passing BlockConf on unmodified (except for the BlockBackend). The
> same functions are called by the SCSI code anyway and it sets the right
> defaults for the actual media type.
>
> Fixes: 308963746169 ('scsi: Don't ignore most usb-storage properties')
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2260
> Reported-by: Jonas Svensson
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> Considering this a candidate for 9.0 given that we're already having an
> rc4, it's a regression from 8.2 and breaks installing Windows from USB
>
>   hw/usb/dev-storage-classic.c | 9 ---------
>   1 file changed, 9 deletions(-)

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>

On Tue, 16 Apr 2024 at 10:26, Hanna Czenczek <hreitz@redhat.com> wrote:
>
> On 12.04.24 16:42, Kevin Wolf wrote:
> > Commit 30896374 started to pass the full BlockConf from usb-storage to
> > scsi-disk, while previously only a few select properties would be
> > forwarded. This enables the user to set more properties, e.g. the block
> > size, that are actually taking effect.
> >
> > However, now the calls to blkconf_apply_backend_options() and
> > blkconf_blocksizes() in usb_msd_storage_realize() that modify some of
> > these properties take effect, too, instead of being silently ignored.
> > This means at least that the block sizes get an unconditional default of
> > 512 bytes before the configuration is passed to scsi-disk.
> >
> > Before commit 30896374, the property wouldn't be set for scsi-disk and
> > therefore the device dependent defaults would apply - 512 for scsi-hd,
> > but 2048 for scsi-cd. The latter default has now become 512, too, which
> > makes at least Windows 11 installation fail when installing from
> > usb-storage.
> >
> > Fix this by simply not calling these functions any more in usb-storage
> > and passing BlockConf on unmodified (except for the BlockBackend). The
> > same functions are called by the SCSI code anyway and it sets the right
> > defaults for the actual media type.
> >
> > Fixes: 308963746169 ('scsi: Don't ignore most usb-storage properties')
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2260
> > Reported-by: Jonas Svensson
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> > Considering this a candidate for 9.0 given that we're already having an
> > rc4, it's a regression from 8.2 and breaks installing Windows from USB
> >
> >   hw/usb/dev-storage-classic.c | 9 ---------
> >   1 file changed, 9 deletions(-)
>
> Reviewed-by: Hanna Czenczek <hreitz@redhat.com>

Thanks; I've now applied this to git directly (following
discussion with Kevin on IRC.)

-- PMM