Re: [Qemu-devel] [PATCH 1/4] machine: conversion of QEMUMachineInitArgs to MachineState

[Marcel Apfelbaum <marcel.a@redhat.com>]

On Thu, 2014-05-15 at 17:04 +0200, Markus Armbruster wrote:
> Marcel Apfelbaum <marcel.a@redhat.com> writes:
> 
> > Total removal of QEMUMachineInitArgs struct. QEMUMachineInitArgs's fields
> > are copied into MachineState. Removed duplicated fields from MachineState.
> >
> > All the other changes are only mechanical refactoring, no semantic changes.
> >
> > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> > ---
> >    - I am perfectly aware that patches touching a lot of files
> >      are not desirable, but this one is a very simple replacement
> >      patch:
> >        QEMUMachineInitArgs -> MachineState
> >        args -> ms
> >    - This is the simplest way to get rid of QEMUMachineInitArgs fast.
> 
> Snipping all patch hunks that only replace QEMUMachineInitArgs *args by
> MachineState *ms leaves just the hunks quoted below.  Did I miss any?
Not that I am aware of.
Thanks for going over the patch!
Marcel

> 
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 28f0047..eba0574 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -7,17 +7,10 @@
> >  #include "hw/qdev.h"
> >  #include "qom/object.h"
> >  
> > -typedef struct QEMUMachineInitArgs {
> > -    const MachineClass *machine;
> > -    ram_addr_t ram_size;
> > -    const char *boot_order;
> > -    const char *kernel_filename;
> > -    const char *kernel_cmdline;
> > -    const char *initrd_filename;
> > -    const char *cpu_model;
> > -} QEMUMachineInitArgs;
> >  
> > -typedef void QEMUMachineInitFunc(QEMUMachineInitArgs *args);
> > +typedef struct MachineState MachineState;
> > +
> > +typedef void QEMUMachineInitFunc(MachineState *ms);
> >  
> >  typedef void QEMUMachineResetFunc(void);
> >  
> > @@ -61,8 +54,6 @@ int qemu_register_machine(QEMUMachine *m);
> >  #define MACHINE_CLASS(klass) \
> >      OBJECT_CLASS_CHECK(MachineClass, (klass), TYPE_MACHINE)
> >  
> > -typedef struct MachineState MachineState;
> > -
> >  MachineClass *find_default_machine(void);
> >  extern MachineState *current_machine;
> >  
> > @@ -79,7 +70,7 @@ struct MachineClass {
> >      const char *alias;
> >      const char *desc;
> >  
> > -    void (*init)(QEMUMachineInitArgs *args);
> > +    void (*init)(MachineState *state);
> >      void (*reset)(void);
> >      void (*hot_add_cpu)(const int64_t id, Error **errp);
> >      int (*kvm_type)(const char *arg);
> > @@ -111,9 +102,6 @@ struct MachineState {
> >      char *accel;
> >      bool kernel_irqchip;
> >      int kvm_shadow_mem;
> > -    char *kernel;
> > -    char *initrd;
> > -    char *append;
> >      char *dtb;
> >      char *dumpdtb;
> >      int phandle_start;
> > @@ -123,7 +111,13 @@ struct MachineState {
> >      bool usb;
> >      char *firmware;
> >  
> > -    QEMUMachineInitArgs init_args;
> > +    const MachineClass *machine;
> > +    ram_addr_t ram_size;
> > +    const char *boot_order;
> > +    const char *kernel_filename;
> > +    const char *kernel_cmdline;
> > +    const char *initrd_filename;
> > +    const char *cpu_model;
> >  };
> >  
> >  #endif
> > diff --git a/vl.c b/vl.c
> > index c4505dc..58673bd 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -4422,16 +4422,15 @@ int main(int argc, char **argv, char **envp)
> >  
> >      qdev_machine_init();
> >  
> > -    current_machine->init_args = (QEMUMachineInitArgs) {
> > -        .machine = machine_class,
> > -        .ram_size = ram_size,
> > -        .boot_order = boot_order,
> > -        .kernel_filename = kernel_filename,
> > -        .kernel_cmdline = kernel_cmdline,
> > -        .initrd_filename = initrd_filename,
> > -        .cpu_model = cpu_model };
> > -
> > -    machine_class->init(&current_machine->init_args);
> > +    current_machine->machine = machine_class;
> > +    current_machine->ram_size = ram_size;
> > +    current_machine->boot_order = boot_order;
> > +    current_machine->kernel_filename = kernel_filename;
> > +    current_machine->kernel_cmdline = kernel_cmdline;
> > +    current_machine->initrd_filename = initrd_filename;
> > +    current_machine->cpu_model = cpu_model;
> > +
> > +    machine_class->init(current_machine);
> >  
> >      audio_init();
> 
> This can't lose any implicit zero initialization, because
> current_machine has been created by object_new(), which zeroes the whole
> struct.  Good.