qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCHv2 1/1] Add usb option in machine options to enab


From: Li Zhang
Subject: Re: [Qemu-devel] [PATCHv2 1/1] Add usb option in machine options to enable/disable usb
Date: Mon, 18 Jun 2012 16:24:43 +0800

On Mon, Jun 18, 2012 at 3:54 PM, Peter Crosthwaite
<address@hidden> wrote:
>>
>> 3. Can't have USB: fail if the user tries to enable it.
>>
>> Code sketch:
>>
>>    /* init USB devices */
>>    if (!machine->has_usb) {
>>        if (usb_enabled)
>>            [report error; should point to the offending options]
>>            exit(1);
>>        }
>>    } else {
>>        if (machine->has_usb > 0) {
>>            usb_enabled = 1;
>>        }
>>        if (usb_enabled) {
>>            if (foreach_device_config(DEV_USB, usb_parse) < 0)
>>                exit(1);
>>        }
>>    }
>>
>>
>>>> Anyway, I don't see why we need to update opts.  Who's using the updated
>>>> opts?
>>>>
>>> psereis will use this opts.
>>> usb kbd and mouse will be needed  with vga enabled.
>>
>> Do they use the updated QemuOpts *opts?  I'd expect them to use usb_on,
>> or whatever flag variable governs USB (now: usb_enabled).
>>
>
> I think whats going on here is Li is trying to do the right thing by
> using QEMU opts for this new machine functionality, however, its
> getting tangled with all this global state replication of -usb. Isnt
> there predecessor work here in getting rid of usb_enabled first? To
I won't introduce global state any more in the latest version.
It just gets the usb_on from machine options.
It won't use usb_enabled.

> that end, I think what is being proposed here is two (somewhat
> independent) patches. One patch for changing usb to QEMU_OPTS that
> primarily does this:
>
> diff --git a/sysemu.h b/sysemu.h
> index bc2c788..9f5ce2c 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -117,7 +117,6 @@ extern const char *keyboard_layout;
>  extern int win2k_install_hack;
>  extern int alt_grab;
>  extern int ctrl_grab;
> -extern int usb_enabled;
>  extern int smp_cpus;
>  extern int max_cpus;
>  extern int cursor_hide;
> [6]   Done                    gedit ./sysemu.h
>
> And the second patch which is the pseries machine model stuff.
>
> Which way round probably doesnt matter right? You could make your
Because there are some machines using usb_enabled.
So I'd rather to left it as global state and add another usb option in
machine options.
Then the machine can get usb option from machine options to enable usb.
So the latest patch won't introduce the global state.
I will send out the latest version later.
> machine model use the extern int usb_enabled initially then move it
> across to machine opts along with the rest of the usb subsystem. Or
> you could fix USB first (globally) then build on top of it. But I
> think that this patch as is, is going to do is introduce is a
> duplicate -usb implementation which is a little messy (even if it is
> only an intermediary state).

> Regards,
> Peter
>
>> [...]
>>



-- 

Best Regards
-Li



reply via email to

[Prev in Thread] Current Thread [Next in Thread]