Re: [Qemu-devel] [PATCHv2] parallel: Allow to disable CONFIG_PARALLEL

[Paolo Bonzini <pbonzini@redhat.com>]

On 11/05/2015 17:52, Markus Armbruster wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> On 11/05/2015 11:36, Miroslav Rezanina wrote:
>>>>> This patch will make "-parallel" a nop.  The right thing to do is to
>>>>> fail startup whenever -parallel is passed and CONFIG_PARALLEL is disabled.
>>>>>
>>> This was original behavior before 07dc788. Intention of this patch is to
>>> make qemu buildable with CONFIG_PARALLEL disabled.
>>
>> Understood, but in the meanwhile Markus wrote commit 4bc6a3e (parallel:
>> parallel_hds_isa_init() shouldn't fail, 2015-02-04), and you should
>> preserve the logic of that commit.
> 
> I have to admit didn't consider CONFIG_PARALLEL when I wrote the commit.
> 
>>>>> You can move parallel_hds_isa_init and parallel_init to
>>>>> hw/isa/isa-bus.c, or to a new file hw/isa/isa-devices.c.
>>>>>
>>> Moving functions will cause abort with "Unknown device" error.
>>
>> This is the right behavior that we want: exit QEMU, not go on silently
>> without the parallel port.
> 
> I agree silently ignoring command line options isn't nice, but it's
> unfortunately what QEMU has always done.
> 
> In particular, -parallel is silently ignored with the vast majority of
> machine types.  The few machine types that implement it silently ignore
> it only when they fail to create the device.

Right.  However, if I move a VM (that has a parallel port, which already
puts us in a kind of reductio as absurdum) from a QEMU that has parallel
ports to a QEMU that doesn't have them, _and the board does something
about -parallel_, I think there should be a failure.

This is because whoever compiled that QEMU is crippling a board, no
matter what their reasons are.

> I'm fine with changing -parallel to either create the device or fail.
> Seems outside the scope of this series, though.

Why?  Your patch is _already_ trying to "create the device or fail",
even if the failure mode isn't particularly clean.  The thing that can
be debated is whether to keep the abort or require a nicer check, and
I'm not requiring it.

Paolo

>> If you do not like the abort, you should revert commit 4bc6a3e, and make
>> parallel_hds_isa_init check for failure of parallel_init.  But for me
>> it's okay to just let it abort.