Re: [Qemu-devel] [PATCH RFC V2 0/9] qemu-machine as a QOM object

[Marcel Apfelbaum <marcel.a@redhat.com>]

On Mon, 2014-03-03 at 13:56 +0100, Paolo Bonzini wrote:
> Il 03/03/2014 13:07, Marcel Apfelbaum ha scritto:
> >> Another early refactoring should be to pass &current_machine->init_args
> >> to machine->init, not the "args".
> > Problem is that this is a "private field", should I add a getter for it?
> 
> vl.c is already accessing it one line before, isn't it?
Yes it is :(

> 
> So vl.c is already looking at QemuMachineState internals.  I expected 
> this to be temporary.
Yes, it will be temporary.

> 
> >> Now here's a possible plan to get rid of QEMUMachineInitArgs:
> >>
> >> 1) Add three wrappers to QemuMachine for reading kernel_filename,
> >> kernel_cmdline, initrd_filename.  Unlike object_property_get_str, they
> >> can skip the strdup of the value.  This way you don't have to add the
> >> matching free to all uses of the fields.
> >
> > I have nothing against it, but this will break QEMU's unified way to
> > handle object properties, right?
> > Meaning I will have a field  of the object state(kernel_filename)
> > and I will add a method like machine_get_field(QemuMachineState) going
> > "around" QOM,,,
> 
> True.  On the other hand, I believe there is a purpose in using getters 
> and setters: using object_property_get/set all the time is more verbose 
> and especially less type-safe.  I prefer adding getters to naming 
> properties with #define.  Especially if you need getters but not setters.
I kind of don't like using the QOM getters/setters too (when I actually know the object type)...
Do we have in Qemu getters to properties declared with #define? I am
going to have a look on GObject too, thanks.
 
> 
> GObject uses a similar scheme where you have both regular 
> getters/setters and properties.  Static languages use the getters and 
> setters, while bindings for dynamic languages will always go through 
> properties.
> 
> >> 2) Similarly, add get/set functions (not properties, since these are not
> >> accessible via -machine) for ram_size, boot_order, cpu_model.
> >
> > I am sorry, here you lost me, what do you mean "accessible via -machine"?
> > Maybe that cannot be queried by QOM tree?
> 
> Not yet, at least.  I prefer to keep these fields out of QOM because 
> they cannot be set with -machine.  cpu_model is already accessible since 
> it's related to the class of the CPU devices.
Got it, thanks.

> 
> > Those getter/setters are not the same wrappers as above, going around "QOM"?
> 
> Actually they are the same thing.  But I think above you only need 
> getters.  Here you also need setters.
OK.

> 
> >> 3) Now you can have something like patch 8 in this series.
> > I also need patch 5 that deals with getting a string property with NULL value,
> 
> Ok.
> 
> > I see no reason why not, the main problem I see is the use of those wrappers
> > or setters/getters, I suspect that the usage will be:
> > 1. global QOM query to get the machine
> > 2. Use this wrappers(getters/setters) to do query/alter the machine.
> 
> I think setters should appear only in vl.c.
Conclusion:
1. It is OK to add getters for QemuMachineState fields in the corresponding
   header file. They can be used when the caller "knows" the object type.
2. In order to populate the fields, static setters that access the internal fields
   are also OK - as long as their scope is very small(e.g: vl.c) and not code wide.

Thanks for the detailed explanations!
Marcel
 
> 
> > Doesn't QOM have another way to do this? Or I am missing something, of course.
> 
> I don't know really.  My main requirement is not to change the -machine 
> interface.
> 
> Paolo