Re: [PATCH v2 01/10] hw/i386: Select CONFIG_PARALLEL for PC machines

[Thomas Huth]

On 08/02/2023 20.26, Fabiano Rosas wrote:
> Currently the isa-parallel driver is always added by the PC machines
> regardless of the presence of the actual code in the build, which can
> lead to a crash:
>
> qemu-system-i386: unknown type 'isa-parallel'
> Aborted (core dumped)
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>   hw/i386/Kconfig | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> index 1bf47b0b0b..d3c340e053 100644
> --- a/hw/i386/Kconfig
> +++ b/hw/i386/Kconfig
> @@ -20,7 +20,6 @@ config PC
>       imply PCI_IPMI_BT
>       imply IPMI_SSIF
>       imply ISA_DEBUG
> -    imply PARALLEL
>       imply PCI_DEVICES
>       imply PVPANIC_ISA
>       imply QXL
> @@ -46,6 +45,7 @@ config PC
>       select ACPI_VMGENID
>       select VIRTIO_PMEM_SUPPORTED
>       select VIRTIO_MEM_SUPPORTED
> +    select PARALLEL
>   
>   config PC_PCI
>       bool

Phew ... Ack from a plain upstream point of view. From a Red Hat downstream 
point of view, this will cause another downstream-only patch for us, since 
the binaries (and the machine types) that we have in RHEL have the 
"isa-parallel" device not compiled in on purpose.

So I started wondering now whether we could tackle all this a little bit 
different, in a more flexible way ... something similar like you did in your 
parallel port patch in v1 / something similar to what Peter suggested in his 
option (2) here:

https://lore.kernel.org/qemu-devel/CAFEAcA9VkFU_bh=aBAOoXCUCeSm1xuR+H+uerd468=vVuDrJEg@mail.gmail.com/ 


For display devices, we already have default_display in MachineClass, so 
instead of always selecting the VGA device in your "Select VGA_PCI in 
Kconfig" patch, we could check that in vl.c and set default_vga = 0 if it is 
not available.

For network devices, there is already default_nic_model in PCMachineClass 
... we could move that to the generic MachineClass and use it in vl.c according.

Then no_parallel, no_serial, no_floppy, no_cdrom, no_sdcard in the 
MachineClass could be replaced by a "char *default_XYZ", too, so that we 
rather have a device name here to indicate the availability of a default 
device than a boolean flag. If the pointer is not set ==> no default device. 
If the pointer is set and the device is available ==> use the default 
device. If the pointer is set and the device is not compiled in ==> emit a 
warning, but continue as if the pointer was not set.

What do you think?

 Thomas