Re: [Qemu-devel] [PATCH 0/6] simplify usb enabling logic and fix a Qemu crash

[Paolo Bonzini]

On 06/01/2015 14:29, Marcel Apfelbaum wrote:
> Patch e79d5a6 ("machine: remove qemu_machine_opts global list") removed option
> descriptions from the -machine QemuOptsList to avoid repeating MachineState's 
> QOM properties.
> 
> This resulted in a Qemu crash:
> $ qemu-system-x86_64 -usb
> qemu-system-x86_64: util/qemu-option.c:387: qemu_opt_get_bool_helper:
> Assertion `opt->desc && opt->desc->type == QEMU_OPT_BOOL' failed.
> Aborted (core dumped)
> 
> Fixed it by simplifying usb_enabled usage:
>  - removed the usb_enabled argument
>  - call directly to machine's QOM property.
>  - expose defaults_enabled to calling sites
> 
> As part of this series the semantics for some ppc machines to
> create the default usb controller were changed, but I still think
> is correct, please let me know if you see a problem there.
> 
> Based on the comments I will receive, I will continue to fix the options
> properties in the same way.
> 
> Marcel Apfelbaum (6):
>   hw/ppc: modified the condition for usb controllers to be created for
>     some ppc machines
>   hw/machine: added machine_usb wrapper
>   hw/usb: simplified usb_enabled
>   hw/ppc/mac_newworld: QOMified mac99 machines
>   hw/ppc/spapr: simplify usb controller creation logic
>   hw/ppc/mac_newworld: simplify usb controller creation logic
> 
>  hw/arm/nseries.c        |  2 +-
>  hw/arm/pxa2xx.c         |  4 ++--
>  hw/arm/realview.c       |  2 +-
>  hw/arm/versatilepb.c    |  2 +-
>  hw/core/machine.c       |  5 +++++
>  hw/i386/pc_piix.c       |  2 +-
>  hw/i386/pc_q35.c        |  2 +-
>  hw/ppc/mac_newworld.c   | 32 +++++++++++++++++++++-----------
>  hw/ppc/mac_oldworld.c   |  2 +-
>  hw/ppc/prep.c           |  2 +-
>  hw/ppc/spapr.c          |  3 ++-
>  include/hw/boards.h     |  2 ++
>  include/sysemu/sysemu.h |  3 ++-
>  vl.c                    | 16 ++++++++++------
>  14 files changed, 51 insertions(+), 28 deletions(-)
> 

I like the way you structured the series!

Reviewed-by: Paolo Bonzini <address@hidden>