On Sun, Jul 8, 2012 at 10:45 PM, Alexander Graf <ag...@suse.de> wrote:
>
>
> On 08.07.2012, at 16:21, Li Zhang <zhlci...@gmail.com> wrote:
>
>> On Fri, Jul 6, 2012 at 9:43 PM, Alexander Graf <ag...@suse.de> wrote:
>>>
>>> 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>
>>>
>>> Shouldn't you also convert -usb over to the new syntax, so that -usb
>>> basically is the same as -machine x,usb=on?
>>>
>> Assumely, -usb and -machine x, usb=on shouldn't be used at the same time.
>> When using -usb, and -machine x, usb=off, it's a little like to
>> disable the default USB of machine,
>> and use another specific USB with -usb option.
>>
>> That's what my understanding. :)
>
> It's not about passing them in at the same time, but about having a single
> semantic instance where "Do we expose USB?" gets stored. That makes the code
> easier.
>
> Also if you change things the way I suggested, every platform will be able to
> use -usb and -machine usb=x. So things become a lot more logical from the
> command line perspective as well, since a management app can always pass in
> -machine usb=x and have the same semantics, regardless of the machine we're
> running on.
>
Got it. I will change it.
>
> Alex
>
>>
>>>
>>> Alex
>>>
>>>> ---
>>>> 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
>>>> @@ -575,6 +575,8 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>>>> long load_limit, rtas_limit, fw_size;
>>>> long pteg_shift = 17;
>>>> char *filename;
>>>> + QemuOpts *machine_opts;
>>>> + bool add_usb = true;
>>>>
>>>> spapr = g_malloc0(sizeof(*spapr));
>>>> QLIST_INIT(&spapr->phbs);
>>>> @@ -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.
>>>
>> OK. I will do that. :)
>>>
>>> Alex
>>>
>>
>>
>>
>> --
>>
>> Best Regards
>> -Li
--
Best Regards
-Li
