Re: [Qemu-devel] [PATCH V2 4/4] hw/machine: qemu machine opts as properties to QemuMachineState

[Marcel Apfelbaum]

On Fri, 2014-05-30 at 16:25 -0300, Eduardo Habkost wrote:
> On Mon, May 26, 2014 at 03:40:58PM +0300, Marcel Apfelbaum wrote:
> [...]
> > +static void machine_initfn(Object *obj)
> > +{
> > +    object_property_add_str(obj, "accel",
> > +                            machine_get_accel, machine_set_accel, NULL);
> > +    object_property_add_bool(obj, "kernel_irqchip",
> > +                             machine_get_kernel_irqchip,
> > +                             machine_set_kernel_irqchip,
> > +                             NULL);
> 
> In the case of kernel_irqchip, the information contained in MachineState
> is not a superset of the information contained on
> qemu_get_machine_opts().
> 
> See hw/ppc/{e500,spapr}.c. They use kernel_irqchip like this:
> 
>     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) {
>         dev = ppce500_init_mpic_kvm(params, irqs);
>     }
> 
>     if (irqchip_required && !dev) {
>         fprintf(stderr, "%s: irqchip requested but unavailable\n",
>                 __func__);
>         abort();
>     }
> 
> This means kernel_irqchip have three possible states: "disabled", "required",
> and "allowed".
I already had a patch adding "property_is_set" to QMP, but was not accepted
as there is no way yet to "unset" a property. (I can point you to the series)

> 
> This means that MachineState.kernel_irqchip is not usable by current
> code that uses the kernel_irqchip option. I suppose we plan to address
> this on MachineState, too, to not get stuck with a global
> qemu_get_machine_opts() forever?
I completely agree with you and I already had a patch tackling it,
based on "property_is_set", but was no accepted yet, obviously.
I was instructed to set the default value in the machine init function
and some way (I don't remember now) to emulate  required/allowed.

I do plan to get back to this.

Thanks,
Marcel
>