Re: [Qemu-devel] [PATCH for-2.3] powerpc: fix -machine usb=no for newworld and pseries machines

[Paolo Bonzini]

On 23/03/2015 23:00, Marcel Apfelbaum wrote:
>>>> I know it comes to solve a bug, but we talked about it in another mail
>>>> thread and this change in semantics was approved.

I forgot to reply to this---my understanding is that it was okay for the
sake of your patch series, but it would be fixed before 2.3.

>>>> Let me explain *why* I don't like it.
>>>> 1. We add an "usb_disabled" field to a base class (actually object)
>>>>     of all the machines and the only place it is interesting is
>>>>     for 2 machines on ppc.
>>
>> So we do for kernel_irqchip_requested/allowed.  Both approaches could be
>> replaced by a tri-state on/off/auto.
> Personally I prefer this one, but out of the scope of this patch.

Yes, that was my rationale as well.

>>>> 2. Even for these 2 machines, the scenario of defaults=on and usb=off
>>>>     is not practical.
>>
>> Why?  For example you could add a virtio-input device instead of a USB
>> keyboard and mouse.
> You got me there :)
> From what I understood for those boards there is no need for this
> combination but I don't know them enough (OK.. at all).

Well, you can always find a reason.  USB is a good default, but it
doesn't have to be the only one.  You might even be okay with USB, but
prefer a different host controller.

> Bottom line, of course I don't have anything against fixing this bug,
> my problem was only with the way we add those fields (usb_disabled),
> maybe a three state QOM property (and variable behind it) is a
> solution, but not for now of course.

I think the QOM property should not be tristate, only the variable.
Another possibility is backing "xyz" with a bool xyz, but adding a
bool xyz_set.

Then irqchip_required = irqchip_set && irqchip, and irqchip_allowed =
!irqchip_set || irqchip.

Paolo

> I also didn't like the required/allowed fields and I added them anyway...
> 
> Thanks,
> Marcel
> 
> 
>>
>> Paolo
>>
> 
> 
>