On Wed, Apr 24, 2013 at 07:25:17PM +0200, Andreas Färber wrote:
> Am 23.04.2013 10:29, schrieb Igor Mammedov:
> > hot_add_cpu hook should be overriden by target that implements
> > cpu hot-add via cpu-add QMP command.
> >
> > Make machine_args available to machine init code, it allows
> > to centralize cpu_model starage instead of adding target
> > specific globals to keep it.
> >
> > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> > ---
> > include/hw/boards.h | 3 +++
> > vl.c | 6 +++++-
> > 2 files changed, 8 insertions(+), 1 deletions(-)
> >
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 425bdc7..de8f92a 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -18,6 +18,8 @@ typedef struct QEMUMachineInitArgs {
> > const char *cpu_model;
> > } QEMUMachineInitArgs;
> >
> > +extern QEMUMachineInitArgs *machine_args;
> > +
> > typedef void QEMUMachineInitFunc(QEMUMachineInitArgs *args);
> >
> > typedef void QEMUMachineResetFunc(void);
> > @@ -43,6 +45,7 @@ typedef struct QEMUMachine {
> > GlobalProperty *compat_props;
> > struct QEMUMachine *next;
> > const char *hw_version;
> > + void (*hot_add_cpu)(const int64_t id, Error **errp);
> > } QEMUMachine;
> >
> > int qemu_register_machine(QEMUMachine *m);
> > diff --git a/vl.c b/vl.c
> > index 5612c33..6a612e6 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -179,6 +179,8 @@ int main(int argc, char **argv)
> > #define MAX_VIRTIO_CONSOLES 1
> > #define MAX_SCLP_CONSOLES 1
> >
> > +QEMUMachineInitArgs *machine_args;
> > +
> > static const char *data_dir[16];
> > static int data_dir_idx;
> > const char *bios_name = NULL;
> > @@ -4295,13 +4297,15 @@ int main(int argc, char **argv, char **envp)
> > .kernel_cmdline = kernel_cmdline,
> > .initrd_filename = initrd_filename,
> > .cpu_model = cpu_model };
> > + machine_args = &args;
>
> args is a stack variable, so making it global does not strike me as a
> good idea... (I did have a patch to split main() in two at one time)
>
> > + current_machine = machine;
> > +
> > machine->init(&args);
> >
> > cpu_synchronize_all_post_init();
> >
> > set_numa_modes();
> >
> > - current_machine = machine;
>
> Did you look up why it was placed here? Eduardo?
> I'm not strongly opinionated but I wonder whether we may want to leave
> it after machine->init?
It was always there since the first time I looked at that code. But I
believe it makes sense to set it earlier so initialization code can use
the machine object.
On the other hand, I would really like to reduce usage of global
variables during initialization, and instead pass a QEMUMachineInitArgs
argument to init functions that need it. But I understand that sometimes
we need to be pragmatic and a global variable is a simpler approach (if
considered just a temporary solution).
--
Eduardo
