Re: [PATCH-for-9.1] hw/i386/pc: Warn about unsatisfied vmport deps

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 15/8/24 20:22, Kamil Szczęk wrote:
> Hi Philippe and sorry for the delay!
> 
> On Wednesday, August 14th, 2024 at 16:02, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> 
>>
>>
>> Hi Kamil,
>>
>> On 14/8/24 13:10, Kamil Szczęk wrote:
>>
>>> Since commit 4ccd5fe22feb95137d325f422016a6473541fe9f ('pc: add option
>>> to disable PS/2 mouse/keyboard'), the vmport will not be created unless
>>> the i8042 PS/2 controller is enabled. To not confuse users, let's add a
>>> warning if vmport was explicitly requested, but the i8042 controller is
>>> disabled. This also changes the behavior of vmport=auto to take i8042
>>> controller availability into account.
>>>
>>> Signed-off-by: Kamil Szczęk kamil@szczek.dev
>>> ---
>>> hw/i386/pc.c | 4 ++++
>>> hw/i386/pc_piix.c | 3 ++-
>>> hw/i386/pc_q35.c | 2 +-
>>> qemu-options.hx | 4 ++--
>>> 4 files changed, 9 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>>> index c74931d577..5bd8dd0350 100644
>>> --- a/hw/i386/pc.c
>>> +++ b/hw/i386/pc.c
>>> @@ -1100,6 +1100,10 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl,
>>> }
>>>
>>> if (!create_i8042) {
>>> + if (!no_vmport) {
>>> + warn_report("vmport requires the i8042 controller to be enabled");
>>
>>
>> Should we fail instead?
> 
> I think failing would be preferrable over a warning, but I opted for the latter to maintain backward compatibility in this specific configuration.
> 
> But now that I think about it, this explicit configuration (vmport=on,i8042=off) is probably very rare in the real world, if it is exercised at all. So failing may not be as big of a breaking change as I first thought.
> 
> If you're fine with introducing this "breaking" change, then I'm down for it too. Let me know if I should post v2.

Better fail early on incoherent config options rather than keep
going with a broken machine IMHO, so I'd rather a v2.