Re: [Qemu-devel] [PATCH V2 4/4] hw/machine: qemu machine opts as properties to QemuMachineState

[Marcel Apfelbaum]

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
>