[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.3] powerpc: fix -machine usb=no for newwor
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH for-2.3] powerpc: fix -machine usb=no for newworld and pseries machines |
Date: |
Tue, 24 Mar 2015 08:47:30 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 |
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
>>
>
>
>