Message ID | 1345667067-24298-1-git-send-email-aliguori@us.ibm.com |
---|---|
State | New |
Headers | show |
On 08/22/2012 02:24 PM, Anthony Liguori wrote: > This allows any QEMU binary to be executed with: > > $QEMU_BINARY -qmp stdio Don't you mean: $QEMU_BINARY -M none -qmp stdio I agree with including this in 1.2, as otherwise your new query-target and other commands are incomplete (that is, this is a 'bug fix' of rounding out a feature already promised at hard freeze, and not a new feature on its own). > +static QEMUMachine machine_none = { > + .name = "none", > + .desc = "empty machine", > + .init = machine_none_init, > + .max_cpus = 0, > +}; I guess libvirt just blindly tries '-S -M none'; if it works, we must be talking to new enough qemu (and all the other QMP commands that we want to probe are then immediately available); if it doesn't work, then we must be talking to older qemu and can fall back to -help scraping (since older versions won't be further modifying their -help output now that they are released). I like the idea, although I'm not familiar enough with this part of the code to know if my review counts for anything: Reviewed-by: Eric Blake <eblake@redhat.com>
On 22 August 2012 21:24, Anthony Liguori <aliguori@us.ibm.com> wrote: > This allows any QEMU binary to be executed with: > > $QEMU_BINARY -qmp stdio ...presumably you mean -qmp stdio -M none ? > > Without errors from missing options that are required by various boards. This > also provides a mode that we can use in the future to construct machines > entirely through QMP commands. How about documenting this machine (and its purpose) somewhere? > Cc: Daniel Berrange <berrange@redhat.com> > Cc: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > --- > hw/Makefile.objs | 2 ++ > hw/null-machine.c | 40 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 42 insertions(+), 0 deletions(-) > create mode 100644 hw/null-machine.c > > diff --git a/hw/Makefile.objs b/hw/Makefile.objs > index 7f57ed5..6dfebd2 100644 > --- a/hw/Makefile.objs > +++ b/hw/Makefile.objs > @@ -134,6 +134,8 @@ hw-obj-$(CONFIG_DP8393X) += dp8393x.o > hw-obj-$(CONFIG_DS1225Y) += ds1225y.o > hw-obj-$(CONFIG_MIPSNET) += mipsnet.o > > +hw-obj-y += null-machine.o > + > # Sound > sound-obj-y = > sound-obj-$(CONFIG_SB16) += sb16.o > diff --git a/hw/null-machine.c b/hw/null-machine.c > new file mode 100644 > index 0000000..69910d3 > --- /dev/null > +++ b/hw/null-machine.c > @@ -0,0 +1,40 @@ > +/* > + * Empty machine > + * > + * Copyright IBM, Corp. 2012 > + * > + * Authors: > + * Anthony Liguori <aliguori@us.ibm.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + * > + */ > + > +#include "qemu-common.h" > +#include "hw/hw.h" > +#include "hw/boards.h" > + > +static void machine_none_init(ram_addr_t ram_size, > + const char *boot_device, > + const char *kernel_filename, > + const char *kernel_cmdline, > + const char *initrd_filename, > + const char *cpu_model) > +{ > +} > + > +static QEMUMachine machine_none = { > + .name = "none", > + .desc = "empty machine", > + .init = machine_none_init, > + .max_cpus = 0, > +}; > + > +static void register_machines(void) > +{ > + qemu_register_machine(&machine_none); > +} > + > +machine_init(register_machines); We seem to be about evenly split about whether machine_init() should have a trailing semicolon (it doesn't need one but it doesn't hurt either...) -- PMM
Eric Blake <eblake@redhat.com> writes: > On 08/22/2012 02:24 PM, Anthony Liguori wrote: >> This allows any QEMU binary to be executed with: >> >> $QEMU_BINARY -qmp stdio > > Don't you mean: > > $QEMU_BINARY -M none -qmp stdio I did, thanks. > > I agree with including this in 1.2, as otherwise your new query-target > and other commands are incomplete (that is, this is a 'bug fix' of > rounding out a feature already promised at hard freeze, and not a new > feature on its own). > >> +static QEMUMachine machine_none = { >> + .name = "none", >> + .desc = "empty machine", >> + .init = machine_none_init, >> + .max_cpus = 0, >> +}; > > I guess libvirt just blindly tries '-S -M none'; if it works, we must be > talking to new enough qemu (and all the other QMP commands that we want > to probe are then immediately available); Correct. '-M none' will fail with a non-zero exit status in all old versions of QEMU. -S isn't really needed FWIW but it certainly doesn't hurt. There aren't any VCPUs created with -M none so strictly speaking, -S doesn't do anything. No ram is allocated with -M none either which is another nice touch (less resource usage). > if it doesn't work, then we > must be talking to older qemu and can fall back to -help scraping (since > older versions won't be further modifying their -help output now that > they are released). Correct. Regards, Anthony Liguori > I like the idea, although I'm not familiar enough > with this part of the code to know if my review counts for anything: > > Reviewed-by: Eric Blake <eblake@redhat.com> > > -- > Eric Blake eblake@redhat.com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org
Peter Maydell <peter.maydell@linaro.org> writes: > On 22 August 2012 21:24, Anthony Liguori <aliguori@us.ibm.com> wrote: >> This allows any QEMU binary to be executed with: >> >> $QEMU_BINARY -qmp stdio > > ...presumably you mean -qmp stdio -M none ? > >> >> Without errors from missing options that are required by various boards. This >> also provides a mode that we can use in the future to construct machines >> entirely through QMP commands. > > How about documenting this machine (and its purpose) somewhere? Okay, but where? I don't know an obvious place. >> Cc: Daniel Berrange <berrange@redhat.com> >> Cc: Markus Armbruster <armbru@redhat.com> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> >> --- >> hw/Makefile.objs | 2 ++ >> hw/null-machine.c | 40 ++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 42 insertions(+), 0 deletions(-) >> create mode 100644 hw/null-machine.c >> >> diff --git a/hw/Makefile.objs b/hw/Makefile.objs >> index 7f57ed5..6dfebd2 100644 >> --- a/hw/Makefile.objs >> +++ b/hw/Makefile.objs >> @@ -134,6 +134,8 @@ hw-obj-$(CONFIG_DP8393X) += dp8393x.o >> hw-obj-$(CONFIG_DS1225Y) += ds1225y.o >> hw-obj-$(CONFIG_MIPSNET) += mipsnet.o >> >> +hw-obj-y += null-machine.o >> + >> # Sound >> sound-obj-y = >> sound-obj-$(CONFIG_SB16) += sb16.o >> diff --git a/hw/null-machine.c b/hw/null-machine.c >> new file mode 100644 >> index 0000000..69910d3 >> --- /dev/null >> +++ b/hw/null-machine.c >> @@ -0,0 +1,40 @@ >> +/* >> + * Empty machine >> + * >> + * Copyright IBM, Corp. 2012 >> + * >> + * Authors: >> + * Anthony Liguori <aliguori@us.ibm.com> >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + * >> + */ >> + >> +#include "qemu-common.h" >> +#include "hw/hw.h" >> +#include "hw/boards.h" >> + >> +static void machine_none_init(ram_addr_t ram_size, >> + const char *boot_device, >> + const char *kernel_filename, >> + const char *kernel_cmdline, >> + const char *initrd_filename, >> + const char *cpu_model) >> +{ >> +} >> + >> +static QEMUMachine machine_none = { >> + .name = "none", >> + .desc = "empty machine", >> + .init = machine_none_init, >> + .max_cpus = 0, >> +}; >> + >> +static void register_machines(void) >> +{ >> + qemu_register_machine(&machine_none); >> +} >> + >> +machine_init(register_machines); > > We seem to be about evenly split about whether machine_init() > should have a trailing semicolon (it doesn't need one > but it doesn't hurt either...) It's obviously superior to use a semicolon... C is completely consistent synactically about the usage of semicolons afterall :-) Regards, Anthony Liguori > > -- PMM
On 22 August 2012 23:42, Anthony Liguori <aliguori@us.ibm.com> wrote: > Peter Maydell <peter.maydell@linaro.org> writes: >> How about documenting this machine (and its purpose) somewhere? > > Okay, but where? I don't know an obvious place. If we hadn't got ourselves into this mess where we can't change -help output, the obvious place would be in the qemu-options.hx documentation of the -machine option. Is it possible to mark up qemu-options.hx for "put this text in the HTML docs but not the -help output" ? -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On 22 August 2012 23:42, Anthony Liguori <aliguori@us.ibm.com> wrote: >> Peter Maydell <peter.maydell@linaro.org> writes: >>> How about documenting this machine (and its purpose) somewhere? >> >> Okay, but where? I don't know an obvious place. > > If we hadn't got ourselves into this mess where we can't change > -help output, the obvious place would be in the qemu-options.hx > documentation of the -machine option. > > Is it possible to mark up qemu-options.hx for "put this text > in the HTML docs but not the -help output" ? Since 1.3 is just a couple weeks away from being open, let's just wait until then and change the help output. Regards, Anthony Liguori > > -- PMM
Am 23.08.2012 00:42, schrieb Anthony Liguori: > Peter Maydell <peter.maydell@linaro.org> writes: > >> On 22 August 2012 21:24, Anthony Liguori <aliguori@us.ibm.com> wrote: >>> This allows any QEMU binary to be executed with: >>> >>> $QEMU_BINARY -qmp stdio >> >> ...presumably you mean -qmp stdio -M none ? >> >>> >>> Without errors from missing options that are required by various boards. This >>> also provides a mode that we can use in the future to construct machines >>> entirely through QMP commands. >> >> How about documenting this machine (and its purpose) somewhere? > > Okay, but where? I don't know an obvious place. > >>> Cc: Daniel Berrange <berrange@redhat.com> >>> Cc: Markus Armbruster <armbru@redhat.com> >>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> >>> --- >>> hw/Makefile.objs | 2 ++ >>> hw/null-machine.c | 40 ++++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 42 insertions(+), 0 deletions(-) >>> create mode 100644 hw/null-machine.c >>> >>> diff --git a/hw/Makefile.objs b/hw/Makefile.objs >>> index 7f57ed5..6dfebd2 100644 >>> --- a/hw/Makefile.objs >>> +++ b/hw/Makefile.objs >>> @@ -134,6 +134,8 @@ hw-obj-$(CONFIG_DP8393X) += dp8393x.o >>> hw-obj-$(CONFIG_DS1225Y) += ds1225y.o >>> hw-obj-$(CONFIG_MIPSNET) += mipsnet.o >>> >>> +hw-obj-y += null-machine.o >>> + >>> # Sound >>> sound-obj-y = >>> sound-obj-$(CONFIG_SB16) += sb16.o >>> diff --git a/hw/null-machine.c b/hw/null-machine.c >>> new file mode 100644 >>> index 0000000..69910d3 >>> --- /dev/null >>> +++ b/hw/null-machine.c >>> @@ -0,0 +1,40 @@ >>> +/* >>> + * Empty machine >>> + * >>> + * Copyright IBM, Corp. 2012 >>> + * >>> + * Authors: >>> + * Anthony Liguori <aliguori@us.ibm.com> >>> + * >>> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >>> + * See the COPYING file in the top-level directory. >>> + * >>> + */ >>> + >>> +#include "qemu-common.h" >>> +#include "hw/hw.h" >>> +#include "hw/boards.h" >>> + >>> +static void machine_none_init(ram_addr_t ram_size, >>> + const char *boot_device, >>> + const char *kernel_filename, >>> + const char *kernel_cmdline, >>> + const char *initrd_filename, >>> + const char *cpu_model) >>> +{ >>> +} >>> + >>> +static QEMUMachine machine_none = { >>> + .name = "none", >>> + .desc = "empty machine", >>> + .init = machine_none_init, >>> + .max_cpus = 0, >>> +}; >>> + >>> +static void register_machines(void) >>> +{ >>> + qemu_register_machine(&machine_none); >>> +} >>> + >>> +machine_init(register_machines); >> >> We seem to be about evenly split about whether machine_init() >> should have a trailing semicolon (it doesn't need one >> but it doesn't hurt either...) > > It's obviously superior to use a semicolon... C is completely > consistent synactically about the usage of semicolons afterall :-) I disagree. This macro does not turn into a statement but a full function definition. Neither our Coding Style nor any other that I know does void foo(bar baz) { }; with a trailing semicolon. For type_init() I cleaned this up, and I attribute the existing semicolons to people's ignorance of what these macros actually do. You might remember that Eduardo and I wanted to QOM'ify machines so that machine_init() would get replaced with type_init() but you stopped us at the time, leaving it untouched. So there's no strict need to go through and change existing users, but I believe in setting good examples. Regards, Andreas
Andreas Färber <afaerber@suse.de> writes: > Am 23.08.2012 00:42, schrieb Anthony Liguori: >> Peter Maydell <peter.maydell@linaro.org> writes: >> >>> On 22 August 2012 21:24, Anthony Liguori <aliguori@us.ibm.com> wrote: >>> >>> We seem to be about evenly split about whether machine_init() >>> should have a trailing semicolon (it doesn't need one >>> but it doesn't hurt either...) >> >> It's obviously superior to use a semicolon... C is completely >> consistent synactically about the usage of semicolons afterall :-) > > I disagree. This macro does not turn into a statement but a full > function definition. Neither our Coding Style nor any other that I know > does I was only responding to be funny. This is not something worth anyone arguing about. We've got *much* more important things to sort through. Regards, Anthony Liguori > > void foo(bar baz) > { > }; > > with a trailing semicolon. For type_init() I cleaned this up, and I > attribute the existing semicolons to people's ignorance of what these > macros actually do. > > You might remember that Eduardo and I wanted to QOM'ify machines so that > machine_init() would get replaced with type_init() but you stopped us at > the time, leaving it untouched. So there's no strict need to go through > and change existing users, but I believe in setting good examples. > > 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
diff --git a/hw/Makefile.objs b/hw/Makefile.objs index 7f57ed5..6dfebd2 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -134,6 +134,8 @@ hw-obj-$(CONFIG_DP8393X) += dp8393x.o hw-obj-$(CONFIG_DS1225Y) += ds1225y.o hw-obj-$(CONFIG_MIPSNET) += mipsnet.o +hw-obj-y += null-machine.o + # Sound sound-obj-y = sound-obj-$(CONFIG_SB16) += sb16.o diff --git a/hw/null-machine.c b/hw/null-machine.c new file mode 100644 index 0000000..69910d3 --- /dev/null +++ b/hw/null-machine.c @@ -0,0 +1,40 @@ +/* + * Empty machine + * + * Copyright IBM, Corp. 2012 + * + * Authors: + * Anthony Liguori <aliguori@us.ibm.com> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + */ + +#include "qemu-common.h" +#include "hw/hw.h" +#include "hw/boards.h" + +static void machine_none_init(ram_addr_t ram_size, + const char *boot_device, + const char *kernel_filename, + const char *kernel_cmdline, + const char *initrd_filename, + const char *cpu_model) +{ +} + +static QEMUMachine machine_none = { + .name = "none", + .desc = "empty machine", + .init = machine_none_init, + .max_cpus = 0, +}; + +static void register_machines(void) +{ + qemu_register_machine(&machine_none); +} + +machine_init(register_machines); +
This allows any QEMU binary to be executed with: $QEMU_BINARY -qmp stdio Without errors from missing options that are required by various boards. This also provides a mode that we can use in the future to construct machines entirely through QMP commands. Cc: Daniel Berrange <berrange@redhat.com> Cc: Markus Armbruster <armbru@redhat.com> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> --- hw/Makefile.objs | 2 ++ hw/null-machine.c | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 0 deletions(-) create mode 100644 hw/null-machine.c