Re: [PATCH v9 14/15] hw/i386: Introduce the microvm machine type

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 1/7/20 2:35 PM, Paolo Bonzini wrote:
> On 24/12/19 01:09, Philippe Mathieu-Daudé wrote:
>> We have CONFIG_SERIAL_ISA declared in "config-devices.h" so we could
>> also use:
>>
>> -- >8 --
>> --- a/hw/i386/microvm.c
>> +++ b/hw/i386/microvm.c
>> @@ -153,9 +153,11 @@ static void
>> microvm_devices_init(MicrovmMachineState *mms)
>>           microvm_set_rtc(mms, rtc_state);
>>       }
>>
>> +#ifdef CONFIG_SERIAL_ISA
>>       if (mms->isa_serial) {
>>           serial_hds_isa_init(isa_bus, 0, 1);
>>       }
>> +#endif
>>
>>       if (bios_name == NULL) {
>>           bios_name = MICROVM_BIOS_FILENAME;
>> ---
>>
>> The binary links too, and the difference with the other hunk is now the
>> device is not listed in 'qemu-system-x86_64 -M microvm -device help'.
>> However I guess understand we prefer to avoid that kind of ifdef'ry.
> 
> The ifdef'ery is okay in board code, but in this case I think it would
> be even better to have a stub for serial_hds_isa_init that gives the
> right error (via Error*, and then you can call it with &error_fatal
> etc.).  So for now I included your patch that changes it to "select"
> from the other series.

Clever! I'll wait Marc-André SerialState cleanup gets in and will do as 
suggested.

Thanks,

Phil.