On 08.07.2012, at 16:46, Li Zhang <zhlci...@gmail.com> wrote:
> On Sat, Jul 7, 2012 at 12:03 AM, Alexander Graf <ag...@suse.de> wrote:
>>
>> On 06.07.2012, at 17:58, Andreas Färber wrote:
>>
>>> Am 06.07.2012 15:43, schrieb Alexander Graf:
>>>>
>>>> On 02.07.2012, at 07:25, zhlci...@gmail.com wrote:
>>>>
>>>>> From: Li Zhang <zhlci...@linux.vnet.ibm.com>
>>>>>
>>>>> pSeries machine needs to enable USB to add a USB
>>>>> keyboard or USB mouse. -usb option won't be used in
>>>>> the future, and machine options are a better way to
>>>>> enable USB.
>>>>>
>>>>> So this patch is to add USB option to machine options
>>>>> (-machine type=pseries,usb=on/off) to enable/disable
>>>>> USB controller. And machines will get the machine option
>>>>> and create a USB controller if USB is on.
>>>>>
>>>>> By the way, USB is on by default on pSeries machine.
>>>>> So USB controller should be turned off explicitly through
>>>>> the command line for "-nodefault" case as the following:
>>>>> -machine type=pseries,usb=off.
>>>>>
>>>>> Signed-off-by: Li Zhang <zhlci...@linux.vnet.ibm.com>
>>>>> Reviewed-by: Andreas Färber <afaer...@suse.de>
>>>>> ---
>>>>> hw/spapr.c | 11 +++++++++++
>>>>> qemu-config.c | 4 ++++
>>>>> 2 files changed, 15 insertions(+)
>>>>>
>>>>> diff --git a/hw/spapr.c b/hw/spapr.c
>>>>> index 81c9343..973de1b 100644
>>>>> --- a/hw/spapr.c
>>>>> +++ b/hw/spapr.c
>>> [...]
>>>>> @@ -710,6 +712,15 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>>>>> spapr_vscsi_create(spapr->vio_bus);
>>>>> }
>>>>>
>>>>> + machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>>>>> + if (machine_opts) {
>>>>> + add_usb = qemu_opt_get_bool(machine_opts, "usb", true);
>>>>> + }
>>>>> +
>>>>> + if (add_usb) {
>>>>> + pci_create_simple(QLIST_FIRST(&spapr->phbs)->host_state.bus,
>>>>> + -1, "pci-ohci");
>>>>> + }
>>>>
>>>> Didn't I ask you to extract this out to generic code? I don't want to have
>>>> a "usb" machine opt that is only available for -M pseries.
>>>
>>> Isn't the mapping from "usb=on" to device-level actions
>>> machine-specific? We have ohci, uhci, ehci, xhci to choose from. And the
>>> bus to place it on is machine-specific, too.
>>>
>>> So did you rather mean adding usb= awareness to more machines? If we
>>> generalize usb=on to usb=none/ohci/... plus some usbbus= we get -device.
>>
>> I was thinking of replacing usb_enabled with
>>
>> static inline int usb_enabled(bool default) {
>> machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>> if (machine_opts) {
>> return qemu_opt_get_bool(machine_opts, "usb", default);
>> }
>> return default;
>> }
>>
>> Whereas -usb in vl.c would set machine.usb = on. Then all users of
>> usb_enabled would automatically be converted to the new syntax and we could
>> even use the same logic in spapr.c.
>>
>
> I see, you mean global varible usb_enabled is still used, and when
> -usb is passed to qemu command line,
> use_enabled will be assigned by the above function and set machine.usb=on.
>
> So all the places where usb_enabled is used still can work correctly.
I mean convert usb_enabled into a function that checks machine.usb :). The only
global state should live in machine opts.
Alex
