[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC V2 0/9] qemu-machine as a QOM object
From: |
Marcel Apfelbaum |
Subject: |
Re: [Qemu-devel] [PATCH RFC V2 0/9] qemu-machine as a QOM object |
Date: |
Mon, 03 Mar 2014 14:07:26 +0200 |
On Mon, 2014-03-03 at 11:50 +0100, Paolo Bonzini wrote:
> Il 02/03/2014 14:07, Marcel Apfelbaum ha scritto:
> > Most of the "Cc" list is due to patch 8: (Should I send each patch to a
> > different list?)
> > machine-opts: replace qemu_opt_get by QOM QemuMachine queries.
> >
> > Status:
> > - machine_opts are mapped into QemuMachineState's properties,
> > which can be queried as regular QOM properties.
> > - Subclassing QemuMachineClass allows to add a command line
> > option specific to a machine type, error mechanism should
> > work if this option is used on another machine. (Not tested, on the
> > todo list.)
> > - Next big step would be to completely remove the qemu machines
> > initialization
> > and replace it by regular QOM type registration.
> Having seen all the series, I think we can plan the actual code as follows.
Paolo, I really appreciate you are taking the time to go over this!
>
> Patches 1-3 are fine (except for the missing object_property_add_child
> in patch 3). Patch 4 is also fine, but it should refer to the embedded
> QEMUMachineInitArgs.
I will, thanks.
>
> The .machine field from QEMUMachineArgs can be removed very early. It
> can be replaced with the class of current_machine.
Sure.
>
> Another early refactoring should be to pass ¤t_machine->init_args
> to machine->init, not the "args".
Problem is that this is a "private field", should I add a getter for it?
>
> 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,,,
>
> 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?
Those getter/setters are not the same wrappers as above, going around "QOM"?
>
> 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,
>
> 4) Thanks to the previous change, there should be no usage of
> QEMUMachineInitArgs anymore. Change the machine init function to take a
> "QemuMachineState *current_machine".
This will touch a lot of files... but needs to be done.
>
> 5) Remove the current_machine global. There will be few users of
> current_machine, and they can look at /machine instead, as mentioned in
> the review of patch 3.
Agreed
>
> Does it look feasible? The bulk changes should all be fairly mechanical.
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.
Doesn't QOM have another way to do this? Or I am missing something, of course.
Thanks again for the review!
Marcel
>
> Paolo
>
- [Qemu-devel] [PATCH RFC V2 2/9] vl: use qemu machine QOM class instead of global machines list, (continued)
Re: [Qemu-devel] [PATCH RFC V2 2/9] vl: use qemu machine QOM class instead of global machines list, Andreas Färber, 2014/03/03
[Qemu-devel] [PATCH RFC V2 5/9] qapi: output visitor crashes qemu if it encounters a NULL value, Marcel Apfelbaum, 2014/03/02
Re: [Qemu-devel] [PATCH RFC V2 0/9] qemu-machine as a QOM object, Paolo Bonzini, 2014/03/03
- Re: [Qemu-devel] [PATCH RFC V2 0/9] qemu-machine as a QOM object,
Marcel Apfelbaum <=