From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:33960) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QOu8N-00009q-Vp for qemu-devel@nongnu.org; Tue, 24 May 2011 12:07:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QOu8M-0006Qo-HK for qemu-devel@nongnu.org; Tue, 24 May 2011 12:07:03 -0400 Received: from aaar.vm.bytemark.co.uk ([80.68.92.230]:53089) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QOu8M-0006Pr-7U for qemu-devel@nongnu.org; Tue, 24 May 2011 12:07:02 -0400 From: Ian Campbell In-Reply-To: <4DD8ECB0.9050803@web.de> References: <4DD64E1F.8020603@siemens.com> <4DD8ECB0.9050803@web.de> Content-Type: text/plain; charset="UTF-8" Date: Tue, 24 May 2011 17:06:40 +0100 Message-ID: <1306253200.20576.186.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 1/2] Generalize -machine command line option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Anthony PERARD , Anthony Liguori , qemu-devel On Sun, 2011-05-22 at 13:00 +0200, Jan Kiszka wrote: > From: Jan Kiszka > > -machine somehow suggests that it selects the machine, but it doesn't. > Fix that before this command is set in stone. > > Actually, -machine should supersede -M and allow to introduce arbitrary > per-machine options to the command line. That will change the internal > realization again, but we will be able to keep the user interface > stable. > > CC: Anthony PERARD > Signed-off-by: Jan Kiszka "-machine xenfv" doesn't work for me with this patch, it gives: Program received signal SIGSEGV, Segmentation fault. hypercall_buffer_cache_lock (xch=0x0) at xc_hcall_buf.c:36 36 xc_hcall_buf.c: No such file or directory. in xc_hcall_buf.c (gdb) bt #0 hypercall_buffer_cache_lock (xch=0x0) at xc_hcall_buf.c:36 #1 0xb7d53f1d in hypercall_buffer_cache_alloc (xch=0x0, b=0xbffff77c, nr_pages=1) at xc_hcall_buf.c:52 #2 xc__hypercall_buffer_alloc_pages (xch=0x0, b=0xbffff77c, nr_pages=1) at xc_hcall_buf.c:128 #3 0xb7d54028 in xc__hypercall_buffer_alloc (xch=0x0, b=0xbffff77c, size=16) at xc_hcall_buf.c:162 #4 0xb7d42719 in xc_get_hvm_param (handle=0x0, dom=248, param=5, value=0xbffff810) at xc_domain.c:1078 #5 0x08252777 in xen_hvm_init () at /home/ianc/devel/qemu.git/xen-all.c:803 #6 0x082921e9 in pc_xen_hvm_init (ram_size=536870912, boot_device=0xbffffabb "cda", kernel_filename=0x0, kernel_cmdline=0x82d648f "", initrd_filename=0x0, cpu_model=0x0) at /home/ianc/devel/qemu.git/hw/pc_piix.c:246 #7 0x081e3f0e in main (argc=26, argv=0xbffffba4, envp=0xbffffc10) at /home/ianc/devel/qemu.git/vl.c:3162 I suspect this is because xen_init() (which sets xch) is never called, perhaps because it causes the accelerator to always be tcg? If I use "-machine xenfv,accel=xen" then it works as expected, -M continues to work too AFAICT. It would be nice to retain the behaviour of defaulting to accel=xen for machine=xenfv. (to be honest I can't quite see where this behaviour came from, nor what about this patch changes it...) > --- > > Changes in v2: > - fix regression of -M my factoring out machine_parse and using it for > both old and new command. > > qemu-config.c | 5 +++++ > qemu-options.hx | 20 +++++++++++++++----- > vl.c | 39 ++++++++++++++++++++++++--------------- > 3 files changed, 44 insertions(+), 20 deletions(-) > > diff --git a/qemu-config.c b/qemu-config.c > index 5d7ffa2..01751b4 100644 > --- a/qemu-config.c > +++ b/qemu-config.c > @@ -452,9 +452,14 @@ QemuOptsList qemu_option_rom_opts = { > > static QemuOptsList qemu_machine_opts = { > .name = "machine", > + .implied_opt_name = "type", > .head = QTAILQ_HEAD_INITIALIZER(qemu_machine_opts.head), > .desc = { > { > + .name = "type", > + .type = QEMU_OPT_STRING, > + .help = "emulated machine" > + }, { > .name = "accel", > .type = QEMU_OPT_STRING, > .help = "accelerator list", > diff --git a/qemu-options.hx b/qemu-options.hx > index 82e085a..0dbc028 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -2033,13 +2033,23 @@ if KVM support is enabled when compiling. > ETEXI > > DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ > - "-machine accel=accel1[:accel2] use an accelerator (kvm,xen,tcg), default is tcg\n", QEMU_ARCH_ALL) > + "-machine [type=]name[,prop[=value][,...]]\n" > + " selects emulated machine (-machine ? for list)\n" > + " property accel=accel1[:accel2[:...]] selects accelerator\n" > + " supported accelerators are kvm, xen, tcg (default: tcg)\n", > + QEMU_ARCH_ALL) > STEXI > -@item -machine accel=@var{accels} > +@item -machine [type=]@var{name}[,prop=@var{value}[,...]] > @findex -machine > -This is use to enable an accelerator, in kvm,xen,tcg. > -By default, it use only tcg. If there a more than one accelerator > -specified, the next one is used if the first don't work. > +Select the emulated machine by @var{name}. Use @code{-machine ?} to list > +available machines. Supported machine properties are: > +@table @option > +@item accel=@var{accels1}[:@var{accels2}[:...]] > +This is used to enable an accelerator. Depending on the target architecture, > +kvm, xen, or tcg can be available. By default, tcg is used. If there is more > +than one accelerator specified, the next one is used if the previous one fails > +to initialize. > +@end table > ETEXI > > DEF("xen-domid", HAS_ARG, QEMU_OPTION_xen_domid, > diff --git a/vl.c b/vl.c > index b362871..a346c1e 100644 > --- a/vl.c > +++ b/vl.c > @@ -1891,6 +1891,27 @@ static int debugcon_parse(const char *devname) > return 0; > } > > +static QEMUMachine *machine_parse(const char *name) > +{ > + QEMUMachine *m, *machine = NULL; > + > + if (name) { > + machine = find_machine(name); > + } > + if (machine) { > + return machine; > + } > + printf("Supported machines are:\n"); > + for (m = first_machine; m != NULL; m = m->next) { > + if (m->alias) { > + printf("%-10s %s (alias of %s)\n", m->alias, m->desc, m->name); > + } > + printf("%-10s %s%s\n", m->name, m->desc, > + m->is_default ? " (default)" : ""); > + } > + exit(!name || *name != '?'); > +} > + > static int tcg_init(void) > { > return 0; > @@ -2144,20 +2165,7 @@ int main(int argc, char **argv, char **envp) > } > switch(popt->index) { > case QEMU_OPTION_M: > - machine = find_machine(optarg); > - if (!machine) { > - QEMUMachine *m; > - printf("Supported machines are:\n"); > - for(m = first_machine; m != NULL; m = m->next) { > - if (m->alias) > - printf("%-10s %s (alias of %s)\n", > - m->alias, m->desc, m->name); > - printf("%-10s %s%s\n", > - m->name, m->desc, > - m->is_default ? " (default)" : ""); > - } > - exit(*optarg != '?'); > - } > + machine = machine_parse(optarg); > break; > case QEMU_OPTION_cpu: > /* hw initialization will check this */ > @@ -2675,11 +2683,12 @@ int main(int argc, char **argv, char **envp) > case QEMU_OPTION_machine: > olist = qemu_find_opts("machine"); > qemu_opts_reset(olist); > - opts = qemu_opts_parse(olist, optarg, 0); > + opts = qemu_opts_parse(olist, optarg, 1); > if (!opts) { > fprintf(stderr, "parse error: %s\n", optarg); > exit(1); > } > + machine = machine_parse(qemu_opt_get(opts, "type")); > break; > case QEMU_OPTION_usb: > usb_enabled = 1; -- Ian Campbell "When people are least sure, they are often most dogmatic." -- John Kenneth Galbraith