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

[Andreas Färber]

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 <marce...@redhat.com>
> ---
>  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():

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.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg