Re: [Qemu-devel] [PATCH 3/8] machine: query kernel-irqchip machine property rather than qemu opts

[Marcel Apfelbaum]

On 02/04/2015 05:43 PM, Marcel Apfelbaum wrote:
> Fixes a QEMU crash when passing kernel_irqchip parameter in command line.
Please amend commit message:

Running
    x86_64-softmmu/qemu-system-x86_64 -machine pc,kernel_irqchip=on -enable-kvm
leads to crash:
    qemu-system-x86_64: qemu/util/qemu-option.c:387: qemu_opt_get_bool_helper: Assertion 
`opt->desc && opt->desc->type == QEMU_OPT_BOOL' failed.
    Aborted (core dumped)

This happens because the commit e79d5a6 ("machine: remove qemu_machine_opts global 
list")
removed the global option descriptions and moved them to MachineState's QOM 
properties.

Fix this by querying machine properties through designated wrappers.

Signed-off-by: Marcel Apfelbaum <address@hidden>
> Signed-off-by: Marcel Apfelbaum <address@hidden>
> ---
>   hw/ppc/e500.c  | 16 +++++-----------
>   hw/ppc/spapr.c | 16 ++++++----------
>   kvm-all.c      |  6 +++---
>   3 files changed, 14 insertions(+), 24 deletions(-)
>
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 7e17d18..641cab9 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -731,8 +731,8 @@ static DeviceState *ppce500_init_mpic_kvm(PPCE500Params 
> *params,
>       return dev;
>   }
>
> -static qemu_irq *ppce500_init_mpic(PPCE500Params *params, MemoryRegion *ccsr,
> -                                   qemu_irq **irqs)
> +static qemu_irq *ppce500_init_mpic(MachineState *machine, PPCE500Params 
> *params,
> +                                   MemoryRegion *ccsr, qemu_irq **irqs)
>   {
>       qemu_irq *mpic;
>       DeviceState *dev = NULL;
> @@ -742,17 +742,11 @@ static qemu_irq *ppce500_init_mpic(PPCE500Params *params, 
> MemoryRegion *ccsr,
>       mpic = g_new0(qemu_irq, 256);
>
>       if (kvm_enabled()) {
> -        QemuOpts *machine_opts = qemu_get_machine_opts();
> -        bool irqchip_allowed = qemu_opt_get_bool(machine_opts,
> -                                                "kernel_irqchip", true);
> -        bool irqchip_required = qemu_opt_get_bool(machine_opts,
> -                                                  "kernel_irqchip", false);
> -
> -        if (irqchip_allowed) {
> +        if (machine_kernel_irqchip_allowed(machine)) {
>               dev = ppce500_init_mpic_kvm(params, irqs);
>           }
>
> -        if (irqchip_required && !dev) {
> +        if (machine_kernel_irqchip_required(machine) && !dev) {
>               fprintf(stderr, "%s: irqchip requested but unavailable\n",
>                       __func__);
>               abort();
> @@ -876,7 +870,7 @@ void ppce500_init(MachineState *machine, PPCE500Params 
> *params)
>       memory_region_add_subregion(address_space_mem, params->ccsrbar_base,
>                                   ccsr_addr_space);
>
> -    mpic = ppce500_init_mpic(params, ccsr_addr_space, irqs);
> +    mpic = ppce500_init_mpic(machine, params, ccsr_addr_space, irqs);
>
>       /* Serial */
>       if (serial_hds[0]) {
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index b560459..f4b6adb 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -123,21 +123,16 @@ static XICSState *try_create_xics(const char *type, int 
> nr_servers,
>       return XICS_COMMON(dev);
>   }
>
> -static XICSState *xics_system_init(int nr_servers, int nr_irqs)
> +static XICSState *xics_system_init(MachineState *machine,
> +                                   int nr_servers, int nr_irqs)
>   {
>       XICSState *icp = NULL;
> -
>       if (kvm_enabled()) {
> -        QemuOpts *machine_opts = qemu_get_machine_opts();
> -        bool irqchip_allowed = qemu_opt_get_bool(machine_opts,
> -                                                "kernel_irqchip", true);
> -        bool irqchip_required = qemu_opt_get_bool(machine_opts,
> -                                                  "kernel_irqchip", false);
> -        if (irqchip_allowed) {
> +        if (machine_kernel_irqchip_allowed(machine)) {
>               icp = try_create_xics(TYPE_KVM_XICS, nr_servers, nr_irqs);
>           }
>
> -        if (irqchip_required && !icp) {
> +        if (machine_kernel_irqchip_required(machine) && !icp) {
>               perror("Failed to create in-kernel XICS\n");
>               abort();
>           }
> @@ -1420,7 +1415,8 @@ static void ppc_spapr_init(MachineState *machine)
>       }
>
>       /* Set up Interrupt Controller before we create the VCPUs */
> -    spapr->icp = xics_system_init(smp_cpus * kvmppc_smt_threads() / 
> smp_threads,
> +    spapr->icp = xics_system_init(machine,
> +                                  smp_cpus * kvmppc_smt_threads() / 
> smp_threads,
>                                     XICS_IRQS);
>
>       /* init CPUs */
> diff --git a/kvm-all.c b/kvm-all.c
> index 2f21a4e..cdb90c5 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1360,11 +1360,11 @@ int kvm_irqchip_remove_irqfd_notifier(KVMState *s, 
> EventNotifier *n, int virq)
>              false);
>   }
>
> -static int kvm_irqchip_create(KVMState *s)
> +static int kvm_irqchip_create(MachineState *machine, KVMState *s)
>   {
>       int ret;
>
> -    if (!qemu_opt_get_bool(qemu_get_machine_opts(), "kernel_irqchip", true) ||
> +    if (!machine_kernel_irqchip_allowed(machine) ||
>           (!kvm_check_extension(s, KVM_CAP_IRQCHIP) &&
>            (kvm_vm_enable_cap(s, KVM_CAP_S390_IRQCHIP, 0) < 0))) {
>           return 0;
> @@ -1603,7 +1603,7 @@ static int kvm_init(MachineState *ms)
>           goto err;
>       }
>
> -    ret = kvm_irqchip_create(s);
> +    ret = kvm_irqchip_create(ms, s);
>       if (ret < 0) {
>           goto err;
>       }