diff mbox

[V2,4/4] hw/machine: qemu machine opts as properties to QemuMachineState

Message ID 538369BD.608@suse.de
State New
Headers show

Commit Message

Andreas Färber May 26, 2014, 4:20 p.m. UTC
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 <marcel.a@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():

         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

Comments

Marcel Apfelbaum May 27, 2014, 8:59 a.m. UTC | #1
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 <marcel.a@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():
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
>
diff mbox

Patch

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));