[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V2 4/4] hw/machine: qemu machine opts as propert
From: |
Marcel Apfelbaum |
Subject: |
Re: [Qemu-devel] [PATCH V2 4/4] hw/machine: qemu machine opts as properties to QemuMachineState |
Date: |
Tue, 27 May 2014 11:59:40 +0300 |
On Mon, 2014-05-26 at 18:20 +0200, Andreas Färber wrote:
> Am 26.05.2014 14:40, schrieb Marcel Apfelbaum:
> > Make machine's QemuOpts QOM properties of machine. The properties
> > are automatically filled in. This opens the possiblity to create
> > opts per machine rather than global.
> >
> > Signed-off-by: Marcel Apfelbaum <address@hidden>
> > ---
> > hw/core/machine.c | 256
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > include/hw/boards.h | 6 +-
> > vl.c | 10 +-
> > 3 files changed, 266 insertions(+), 6 deletions(-)
>
> I've updated the commit message and inserted some white lines after
> variable block, and on top the following name cleanup to match your
> machine_initfn():
Thanks!
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 0989c60..cbba679 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -276,7 +276,7 @@ static void machine_initfn(Object *obj)
> machine_get_firmware, machine_set_firmware,
> NULL);
> }
>
> -static void qemu_machine_finalize(Object *obj)
> +static void machine_finalize(Object *obj)
> {
> MachineState *ms = MACHINE(obj);
>
> @@ -297,7 +297,7 @@ static const TypeInfo machine_info = {
> .class_size = sizeof(MachineClass),
> .instance_size = sizeof(MachineState),
> .instance_init = machine_initfn,
> - .instance_finalize = qemu_machine_finalize,
> + .instance_finalize = machine_finalize,
> };
>
> static void machine_register_types(void)
>
> Further there's a line too long:
>
> diff --git a/vl.c b/vl.c
> index 676df6e..8267679 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4216,7 +4216,8 @@ int main(int argc, char **argv, char **envp)
> }
>
> machine_opts = qemu_get_machine_opts();
> - if (qemu_opt_foreach(machine_opts, object_set_property,
> current_machine, 1) < 0) {
> + if (qemu_opt_foreach(machine_opts, object_set_property,
> current_machine,
> + 1) < 0) {
> object_unref(OBJECT(current_machine));
> exit(1);
> }
>
> which makes me whether you are intentionally doing < 0 here while the
> surrounding code is doing != 0? Minor nit only, of course.
Well, I kind of took this from "-object" way to fill in the properties :).
I am OK other-way, but we should change them both. The same for line length.
Thanks again,
Marcel
>
> Regards,
> Andreas
>
- [Qemu-devel] [PATCH V2 0/4] machine: QemuOpts per machine, Marcel Apfelbaum, 2014/05/26
- [Qemu-devel] [PATCH V2 1/4] qapi: output visitor crashes qemu if it encounters a NULL value, Marcel Apfelbaum, 2014/05/26
- [Qemu-devel] [PATCH V2 2/4] tests: check empty qmp output visitor, Marcel Apfelbaum, 2014/05/26
- [Qemu-devel] [PATCH V2 3/4] vl.c: do not set 'type' property in obj_set_property, Marcel Apfelbaum, 2014/05/26
- [Qemu-devel] [PATCH V2 4/4] hw/machine: qemu machine opts as properties to QemuMachineState, Marcel Apfelbaum, 2014/05/26
- Re: [Qemu-devel] [PATCH V2 0/4] machine: QemuOpts per machine, Michael S. Tsirkin, 2014/05/26