[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 0/8] machine: query machine properties rather th

From: Marcel Apfelbaum
Subject: Re: [Qemu-devel] [PATCH 0/8] machine: query machine properties rather than qemu opts
Date: Wed, 04 Feb 2015 23:35:05 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0

On 02/04/2015 09:45 PM, Christian Borntraeger wrote:
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.


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_set_aes_key_wrap, NULL);
     object_property_set_description(obj, "aes-key-wrap",
             "enable/disable AES key wrapping using the CPACF wrapping key",
     object_property_add_bool(obj, "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",
OK, You add 2 QOM properties to TYPE_S390_CCW_MACHINE instances.
Of course your setters/getters need to save the property values into an actual 
so you will need a S390State (derived from MachineState)

typedef struct {
  MachineState parent;

  bool   aes_key_wrap;

} S390State

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 
to create wrappers seems strange.
Completely agree. This is the reason we wanted options(properties) per machine 

 So implementing them in hw/s390/s390-virtio-ccw.c
seems a much better place.

Would a function there that does  gets 
considered ok, or do we need to pollute hw/core/machine.c with architecture 
We surely don't add this to hw/core/machine.c because is specific to 

Let's say you want to use this property in kvm_arch_init of target-s390x/kvm.c.
 - After this series you already have an instance of MachineState initialized 
with your new properties.
 - My assumption is that TYPE_S390_CCW_MACHINE is the only s390 machine or the 
base type of all s390 machines.
You have three options here:
 1. Use QOM infrastucture:
    bool aes_key_wrap = object_property_get_bool(OBJECT(machine), 
"aes-key-wrap", NULL);
 2. Add a wrapper somewhere in  include/hw/s390x/
    that gets MachineState, cast it into S390State and return the field value.
 3. Directly downcast MachineState to S390State and get the value.
* All of the above use my assumption that all s390 machines derive from this 


PS: The same is somewhat true for qemu-options.hx. Having such specific machine 
in the global help offers room for improvement
Can you please elaborate? I didn't fully understand what exactly are you 
referring to.

Hope I helped,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]