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

[Peter Crosthwaite]

>
> 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
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
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

> [...]
>