Re: [Qemu-devel] [PATCH 0/8] machine: query machine properties rather than qemu opts

[Christian Borntraeger <borntraeger@de.ibm.com>]

Am 04.02.2015 um 16:43 schrieb Marcel Apfelbaum:
> Commit e79d5a6 ("machine: remove qemu_machine_opts global list") removed option
> descriptions from the -machine QemuOptsList to avoid repeating MachineState's QOM properties.
> 
> This results in a Qemu crash if a non string option is queried using qemu opts.
> Fix this by querying machine properties through designated wrappers.
> 
> I hope I didn't miss anything.
> Comments are appreciated as always.
> 
> Thanks,
> Marcel
> 
> Marcel Apfelbaum (8):
>   machine: query iommu machine property rather than qemu opts
>   hw/machine: kernel-irqchip property support for allowed/required
>   machine: query kernel-irqchip machine property rather than qemu opts
>   kvm: add machine state to kvm_arch_init
>   machine: query kvm-shadow-mem machine property rather than qemu opts
>   machine: query phandle-start machine property rather than qemu opts
>   machine: query dump-guest-core machine property rather than qemu opts
>   machine: query mem-merge machine property rather than qemu opts

In general this seems to work.
I have a question, though:

What I like is a way to do some wrappers in the specific machines.
For example, we plan to add

static inline void s390_machine_initfn(Object *obj)
{
    object_property_add_bool(obj, "aes-key-wrap",
                             machine_get_aes_key_wrap,
                             machine_set_aes_key_wrap, NULL);
    object_property_set_description(obj, "aes-key-wrap",
            "enable/disable AES key wrapping using the CPACF wrapping key",
            NULL);
    object_property_add_bool(obj, "dea-key-wrap",
                             machine_get_dea_key_wrap,
                             machine_set_dea_key_wrap, NULL);
    object_property_set_description(obj, "dea-key-wrap",
            "enable/disable DEA key wrapping using the CPACF wrapping key",
            NULL);
}



Previously we used   
 if (qemu_opt_get_bool(opts, "aes-key-wrap", false)) 
if target-s390/kvm.c
to query that.

Now, these options are pretty specific to s390 and adding them to hw/core/machine.c
to create wrappers seems strange. So implementing them in hw/s390/s390-virtio-ccw.c
seems a much better place.
Would a function there that does  gets S390_CCW_MACHINE(current_machine)->aes_key_wrap 
considered ok, or do we need to pollute hw/core/machine.c with architecture specific
options?

Christian

PS: The same is somewhat true for qemu-options.hx. Having such specific machine option
in the global help offers room for improvement