[PATCH 0/3] hw/scsi: Cleanup around scsi_bus_legacy_handle_cmdline()

Philippe Mathieu-Daudé posted 3 patches 1 day ago
include/hw/pci/pci.h  | 2 --
hw/arm/realview.c     | 3 +--
hw/arm/versatilepb.c  | 3 +--
hw/hppa/machine.c     | 3 +--
hw/m68k/next-cube.c   | 2 --
hw/m68k/q800.c        | 2 --
hw/mips/jazz.c        | 2 --
hw/ppc/prep.c         | 1 -
hw/scsi/esp.c         | 1 +
hw/scsi/lsi53c895a.c  | 8 +-------
hw/scsi/spapr_vscsi.c | 4 ++--
hw/sparc/sun4m.c      | 1 -
12 files changed, 7 insertions(+), 25 deletions(-)
[PATCH 0/3] hw/scsi: Cleanup around scsi_bus_legacy_handle_cmdline()
Posted by Philippe Mathieu-Daudé 1 day ago
When a device model requires legacy command line handling,
call scsi_bus_legacy_handle_cmdline() in its realize handler
instead of having each user call it.

This applies to:
 - spapr_vscsi
 - lsi53c810 / lsi53c895a
 - sysbus_esp

Note, scsi_bus_legacy_handle_cmdline() prototype could be
made private to hw/scsi/ to restrict its use to scsi device
implementations.

Philippe Mathieu-Daudé (3):
  hw/scsi/spapr_vscsi: Call scsi_bus_legacy_handle_cmdline() in REALIZE
  hw/scsi/lsi53c895a: Call scsi_bus_legacy_handle_cmdline() once
  hw/scsi/esp: Call scsi_bus_legacy_handle_cmdline() once

 include/hw/pci/pci.h  | 2 --
 hw/arm/realview.c     | 3 +--
 hw/arm/versatilepb.c  | 3 +--
 hw/hppa/machine.c     | 3 +--
 hw/m68k/next-cube.c   | 2 --
 hw/m68k/q800.c        | 2 --
 hw/mips/jazz.c        | 2 --
 hw/ppc/prep.c         | 1 -
 hw/scsi/esp.c         | 1 +
 hw/scsi/lsi53c895a.c  | 8 +-------
 hw/scsi/spapr_vscsi.c | 4 ++--
 hw/sparc/sun4m.c      | 1 -
 12 files changed, 7 insertions(+), 25 deletions(-)

-- 
2.45.2


Re: [PATCH 0/3] hw/scsi: Cleanup around scsi_bus_legacy_handle_cmdline()
Posted by Thomas Huth 23 hours ago
On 22/11/2024 12.19, Philippe Mathieu-Daudé wrote:
> When a device model requires legacy command line handling,
> call scsi_bus_legacy_handle_cmdline() in its realize handler
> instead of having each user call it.
> 
> This applies to:
>   - spapr_vscsi
>   - lsi53c810 / lsi53c895a
>   - sysbus_esp
> 
> Note, scsi_bus_legacy_handle_cmdline() prototype could be
> made private to hw/scsi/ to restrict its use to scsi device
> implementations.

Not sure whether this is the right way to go ... shouldn't the handling of 
the legacy command line be rather part of the machine than being part of the 
SCSI controller device? Imagine for example a machine that has multiple, 
different SCSI controllers - I think you'd rather want to let the machine 
decide where the legacy devices should be grabbed from than having the SCSI 
controller devices fight for them...?

  Thomas


Re: [PATCH 0/3] hw/scsi: Cleanup around scsi_bus_legacy_handle_cmdline()
Posted by Paolo Bonzini 23 hours ago
On 11/22/24 13:37, Thomas Huth wrote:
> On 22/11/2024 12.19, Philippe Mathieu-Daudé wrote:
>> When a device model requires legacy command line handling,
>> call scsi_bus_legacy_handle_cmdline() in its realize handler
>> instead of having each user call it.
>>
>> This applies to:
>>   - spapr_vscsi
>>   - lsi53c810 / lsi53c895a
>>   - sysbus_esp
>>
>> Note, scsi_bus_legacy_handle_cmdline() prototype could be
>> made private to hw/scsi/ to restrict its use to scsi device
>> implementations.
> 
> Not sure whether this is the right way to go ... shouldn't the handling 
> of the legacy command line be rather part of the machine than being part 
> of the SCSI controller device? Imagine for example a machine that has 
> multiple, different SCSI controllers - I think you'd rather want to let 
> the machine decide where the legacy devices should be grabbed from than 
> having the SCSI controller devices fight for them...?

I agree that it should be done in the machines generally:

1) if the machine creates a SCSI controller, it should then call
scsi_bus_legacy_handle_cmdline().  This is the case for esp and for
spapr-vscsi (so spapr_vscsi_create() could be inlined in its caller).

2) lsi53c* is the odd one out because it was "the" way to add "-drive
if=scsi" to the PC machine.

For case (2) it's okay to call it in the realize function, I guess.
However, let's only process -drive if=scsi for devices added on the
command-line.

The LSI HBA should not call  scsi_bus_legacy_handle_cmdline() if
dev->hotplugged.  I think we can do it without a deprecation period,
and in fact assert that !phase_check(PHASE_MACHINE_READY) in
scsi_bus_legacy_handle_cmdline().

Paolo