Re: [PATCH v2 4/8] hw/boards: Remove all invalid uses of auto_create_sdcard=true

[BALATON Zoltan]

On Thu, 6 Feb 2025, Philippe Mathieu-Daudé wrote:
> On 6/2/25 13:56, BALATON Zoltan wrote:
> > On Thu, 6 Feb 2025, Philippe Mathieu-Daudé wrote:
> > > On 5/2/25 08:03, Markus Armbruster wrote:
> > > > Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> > > >
> > > > > MachineClass::auto_create_sdcard is only useful to automatically
> > > > > create a SD card, attach a IF_SD block drive to it and plug the
> > > > > card onto a SD bus. Only the ARM and RISCV targets use such
> > > > > feature:
> > > > >
> > > > >   $ git grep -wl IF_SD hw | cut -d/ -f-2 | sort -u
> > > > >   hw/arm
> > > > >   hw/riscv
> > > > >   $
> > > > >
> > > > > Remove all other uses.
> > > > >
> > > > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > >
> > > > Impact?
> > > >
> > > > As far as I can tell, this stops creation of the if=sd default drive
> > > > these machines don't actually use.  Correct?
> > >
> > > Yes, since these machines don't expose a SD-bus, the drive can
> > > not be attached and always triggers the same error:
> > >
> > > $ qemu-system-hppa -sd /bin/sh
> > > qemu-system-hppa: -sd /bin/sh: machine type does not support 
> > > if=sd,bus=0,unit=0
> >
> > And how is this error improved after this series? This seems to be a 
> > meaningful error already.
> >
> > In any case, if you plan to merge this, could you please cut down on adding 
> > nonsense defaults to every machine that are then removed again? Maybe only 
> > confine it to hw/arm where most of the machines are, for the rest you can 
> > point to an earlier version of the series in the cover letter to show how 
> > did you end up with the list of the machines to help review but the final 
> > version merged in master maybe does not have to have all the temporary 
> > changes to avoid excessive churn. Or was that the plan already?
>
> No, I plan to merge v4 as is. If you disagree, feel free to post a v5;

If nobody else minds the churn this causes I'm not opposed to it but I 
don't see the value of commiting things that the next patch removes. It's 
OK to have this in a series for review to show the process but for final 
only doing this in the hw/arm branch where it matters (or just squash the 
in-between temporary patches to only have the real change) in the final 
version would make less churn on master.

> I won't object to it, I simply spent a huge amount of time to remove
> this dumb field, and there are still 6 other fields to do:

I understand you don't want to spend much more time on this but squashing 
patches is also not much work.

Regards,
BALATON Zoltan

>    unsigned int no_serial:1,
>        no_parallel:1,
>        no_floppy:1,
>        no_cdrom:1,
>        no_sdcard:1,
>        pci_allow_0_address:1,
>        legacy_fw_cfg_order:1;
>
> Regards,
>
> Phil.