* [Qemu-devel] [PATCH 1/2] Generalize -machine command line option @ 2011-05-20 11:18 Jan Kiszka 2011-05-22 11:00 ` [Qemu-devel] [PATCH v2 " Jan Kiszka 0 siblings, 1 reply; 12+ messages in thread From: Jan Kiszka @ 2011-05-20 11:18 UTC (permalink / raw) To: Anthony Liguori, qemu-devel; +Cc: Anthony PERARD -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. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- qemu-config.c | 5 +++++ qemu-options.hx | 20 +++++++++++++++----- vl.c | 34 +++++++++++++++++++--------------- 3 files changed, 39 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..4560376 100644 --- a/vl.c +++ b/vl.c @@ -2144,20 +2144,9 @@ 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 != '?'); - } + olist = qemu_find_opts("machine"); + qemu_opts_reset(olist); + qemu_opts_parse(olist, optarg, 1); break; case QEMU_OPTION_cpu: /* hw initialization will check this */ @@ -2675,11 +2664,26 @@ 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); } + optarg = qemu_opt_get(opts, "type"); + machine = optarg ? find_machine(optarg) : NULL; + 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 || *optarg != '?'); + } break; case QEMU_OPTION_usb: usb_enabled = 1; ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 1/2] Generalize -machine command line option 2011-05-20 11:18 [Qemu-devel] [PATCH 1/2] Generalize -machine command line option Jan Kiszka @ 2011-05-22 11:00 ` Jan Kiszka 2011-05-24 16:06 ` Ian Campbell 0 siblings, 1 reply; 12+ messages in thread From: Jan Kiszka @ 2011-05-22 11:00 UTC (permalink / raw) To: Anthony Liguori, qemu-devel; +Cc: Anthony PERARD From: Jan Kiszka <jan.kiszka@siemens.com> -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 <anthony.perard@citrix.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- 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; -- 1.7.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] Generalize -machine command line option 2011-05-22 11:00 ` [Qemu-devel] [PATCH v2 " Jan Kiszka @ 2011-05-24 16:06 ` Ian Campbell 2011-05-24 16:18 ` Jan Kiszka 0 siblings, 1 reply; 12+ messages in thread From: Ian Campbell @ 2011-05-24 16:06 UTC (permalink / raw) 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 <jan.kiszka@siemens.com> > > -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 <anthony.perard@citrix.com> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> "-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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] Generalize -machine command line option 2011-05-24 16:06 ` Ian Campbell @ 2011-05-24 16:18 ` Jan Kiszka 2011-05-25 7:03 ` [Qemu-devel] [PATCH v3 " Jan Kiszka 2011-05-25 8:13 ` [Qemu-devel] [PATCH v2 " Ian Campbell 0 siblings, 2 replies; 12+ messages in thread From: Jan Kiszka @ 2011-05-24 16:18 UTC (permalink / raw) To: Ian Campbell; +Cc: Anthony PERARD, Anthony Liguori, qemu-devel On 2011-05-24 18:06, Ian Campbell wrote: > On Sun, 2011-05-22 at 13:00 +0200, Jan Kiszka wrote: >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> -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 <anthony.perard@citrix.com> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > "-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...) Well, first of all I think this revealed a Xen bug because it crashes when you try to run xenfv with an inappropriate accelerator, no? What is the result of -machine xenfv,accel=tcg or, without my patch, -M xenfv -machine accel=tcg? Then the question is what accel options are actually picked with -machine xenfv, or why the default options are maybe not considered. I don't have any Xen around, but I will check how far I can debug this - or actually try to understand what I coded if nothing helps. Thanks for testing! Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v3 1/2] Generalize -machine command line option 2011-05-24 16:18 ` Jan Kiszka @ 2011-05-25 7:03 ` Jan Kiszka 2011-05-25 8:17 ` Ian Campbell 2011-06-07 15:58 ` Jan Kiszka 2011-05-25 8:13 ` [Qemu-devel] [PATCH v2 " Ian Campbell 1 sibling, 2 replies; 12+ messages in thread From: Jan Kiszka @ 2011-05-25 7:03 UTC (permalink / raw) To: Anthony Liguori, qemu-devel; +Cc: Anthony PERARD, Ian Campbell From: Jan Kiszka <jan.kiszka@siemens.com> -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 <anthony.perard@citrix.com> CC: Ian Campbell <ijc@hellion.org.uk> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- Changes in v3: - fix regression of default machine options handling, -machine xenfv selects accel=xen again (I really hope we can clean up the defaults, make them more generally useful when switching to some QCFG.) 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 | 43 ++++++++++++++++++++++++++----------------- 3 files changed, 46 insertions(+), 22 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..1865a8d 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; @@ -2941,8 +2950,8 @@ int main(int argc, char **argv, char **envp) p = qemu_opt_get(QTAILQ_FIRST(&list->head), "accel"); } if (p == NULL) { - opts = qemu_opts_parse(qemu_find_opts("machine"), - machine->default_machine_opts, 0); + qemu_opts_reset(list); + opts = qemu_opts_parse(list, machine->default_machine_opts, 0); if (!opts) { fprintf(stderr, "parse error for machine %s: %s\n", machine->name, machine->default_machine_opts); -- 1.7.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] Generalize -machine command line option 2011-05-25 7:03 ` [Qemu-devel] [PATCH v3 " Jan Kiszka @ 2011-05-25 8:17 ` Ian Campbell 2011-06-07 15:58 ` Jan Kiszka 1 sibling, 0 replies; 12+ messages in thread From: Ian Campbell @ 2011-05-25 8:17 UTC (permalink / raw) To: Jan Kiszka; +Cc: Anthony PERARD, Anthony Liguori, qemu-devel On Wed, 2011-05-25 at 09:03 +0200, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > -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 <anthony.perard@citrix.com> > CC: Ian Campbell <ijc@hellion.org.uk> Thanks Jan, this one works for me. Tested-by: Ian Campbell <ian.campbell@citrix.com> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > > Changes in v3: > - fix regression of default machine options handling, -machine xenfv > selects accel=xen again > (I really hope we can clean up the defaults, make them more > generally useful when switching to some QCFG.) > > 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 | 43 ++++++++++++++++++++++++++----------------- > 3 files changed, 46 insertions(+), 22 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..1865a8d 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; > @@ -2941,8 +2950,8 @@ int main(int argc, char **argv, char **envp) > p = qemu_opt_get(QTAILQ_FIRST(&list->head), "accel"); > } > if (p == NULL) { > - opts = qemu_opts_parse(qemu_find_opts("machine"), > - machine->default_machine_opts, 0); > + qemu_opts_reset(list); > + opts = qemu_opts_parse(list, machine->default_machine_opts, 0); > if (!opts) { > fprintf(stderr, "parse error for machine %s: %s\n", > machine->name, machine->default_machine_opts); -- Ian Campbell Current Noise: Blind Melon - I Wonder When the going gets weird, the weird turn pro. -- Hunter S. Thompson ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/2] Generalize -machine command line option 2011-05-25 7:03 ` [Qemu-devel] [PATCH v3 " Jan Kiszka 2011-05-25 8:17 ` Ian Campbell @ 2011-06-07 15:58 ` Jan Kiszka 1 sibling, 0 replies; 12+ messages in thread From: Jan Kiszka @ 2011-06-07 15:58 UTC (permalink / raw) To: Anthony Liguori; +Cc: Anthony PERARD, qemu-devel, Ian Campbell On 2011-05-25 09:03, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > -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. Ping (and also for http://thread.gmane.org/gmane.comp.emulators.qemu/103348). Jan > > CC: Anthony PERARD <anthony.perard@citrix.com> > CC: Ian Campbell <ijc@hellion.org.uk> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > > Changes in v3: > - fix regression of default machine options handling, -machine xenfv > selects accel=xen again > (I really hope we can clean up the defaults, make them more > generally useful when switching to some QCFG.) > > 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 | 43 ++++++++++++++++++++++++++----------------- > 3 files changed, 46 insertions(+), 22 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..1865a8d 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; > @@ -2941,8 +2950,8 @@ int main(int argc, char **argv, char **envp) > p = qemu_opt_get(QTAILQ_FIRST(&list->head), "accel"); > } > if (p == NULL) { > - opts = qemu_opts_parse(qemu_find_opts("machine"), > - machine->default_machine_opts, 0); > + qemu_opts_reset(list); > + opts = qemu_opts_parse(list, machine->default_machine_opts, 0); > if (!opts) { > fprintf(stderr, "parse error for machine %s: %s\n", > machine->name, machine->default_machine_opts); -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] Generalize -machine command line option 2011-05-24 16:18 ` Jan Kiszka 2011-05-25 7:03 ` [Qemu-devel] [PATCH v3 " Jan Kiszka @ 2011-05-25 8:13 ` Ian Campbell 2011-05-25 8:23 ` Jan Kiszka 2011-05-25 10:54 ` Anthony PERARD 1 sibling, 2 replies; 12+ messages in thread From: Ian Campbell @ 2011-05-25 8:13 UTC (permalink / raw) To: Jan Kiszka; +Cc: Anthony PERARD, Anthony Liguori, qemu-devel On Tue, 2011-05-24 at 18:18 +0200, Jan Kiszka wrote: > On 2011-05-24 18:06, Ian Campbell wrote: > > On Sun, 2011-05-22 at 13:00 +0200, Jan Kiszka wrote: > >> From: Jan Kiszka <jan.kiszka@siemens.com> > >> > >> -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 <anthony.perard@citrix.com> > >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > > > "-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...) > > Well, first of all I think this revealed a Xen bug because it crashes > when you try to run xenfv with an inappropriate accelerator, no? What is > the result of -machine xenfv,accel=tcg or, without my patch, -M xenfv > -machine accel=tcg? Unsurprisingly it crashed... I'm not sure if this is a bug in the xen side of simply a case of providing enough rope. I didn't really follow the threads which resulted in the accel stuff all that closely so I'm not sure what the intention was, it seems to me that the accel option is invalid with certain machine types though. I suppose in theory -machine xenpv,accel=kvm might result in xenner or something, accel=xenpv,tcg I'm less sure about (perhaps xenner too?). For -machine xenfv I don't expect anything other than accel=xen makes much sense. > Then the question is what accel options are actually picked with > -machine xenfv, or why the default options are maybe not considered. I > don't have any Xen around, but I will check how far I can debug this - > or actually try to understand what I coded if nothing helps. I saw a follow up patch -- I'll give it a go. > > Thanks for testing! > Jan > -- Ian Campbell Current Noise: Blind Melon - Soak The Sin Anti-trust laws should be approached with exactly that attitude. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] Generalize -machine command line option 2011-05-25 8:13 ` [Qemu-devel] [PATCH v2 " Ian Campbell @ 2011-05-25 8:23 ` Jan Kiszka 2011-05-25 8:34 ` Ian Campbell 2011-05-25 10:54 ` Anthony PERARD 1 sibling, 1 reply; 12+ messages in thread From: Jan Kiszka @ 2011-05-25 8:23 UTC (permalink / raw) To: Ian Campbell; +Cc: Anthony PERARD, Anthony Liguori, qemu-devel On 2011-05-25 10:13, Ian Campbell wrote: > On Tue, 2011-05-24 at 18:18 +0200, Jan Kiszka wrote: >> On 2011-05-24 18:06, Ian Campbell wrote: >>> On Sun, 2011-05-22 at 13:00 +0200, Jan Kiszka wrote: >>>> From: Jan Kiszka <jan.kiszka@siemens.com> >>>> >>>> -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 <anthony.perard@citrix.com> >>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >>> >>> "-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...) >> >> Well, first of all I think this revealed a Xen bug because it crashes >> when you try to run xenfv with an inappropriate accelerator, no? What is >> the result of -machine xenfv,accel=tcg or, without my patch, -M xenfv >> -machine accel=tcg? > > Unsurprisingly it crashed... > > I'm not sure if this is a bug in the xen side of simply a case of > providing enough rope. I didn't really follow the threads which resulted > in the accel stuff all that closely so I'm not sure what the intention > was, it seems to me that the accel option is invalid with certain > machine types though. > > I suppose in theory -machine xenpv,accel=kvm might result in xenner or > something, accel=xenpv,tcg I'm less sure about (perhaps xenner too?). > For -machine xenfv I don't expect anything other than accel=xen makes > much sense. The point is that crashing is always a very poor way of reporting a misconfiguration to the user. I bet you are able to detect and report that more gracefully, e.g. during xenfv machine init. > >> Then the question is what accel options are actually picked with >> -machine xenfv, or why the default options are maybe not considered. I >> don't have any Xen around, but I will check how far I can debug this - >> or actually try to understand what I coded if nothing helps. > > I saw a follow up patch -- I'll give it a go. TIA, Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] Generalize -machine command line option 2011-05-25 8:23 ` Jan Kiszka @ 2011-05-25 8:34 ` Ian Campbell 0 siblings, 0 replies; 12+ messages in thread From: Ian Campbell @ 2011-05-25 8:34 UTC (permalink / raw) To: Jan Kiszka; +Cc: Anthony PERARD, Anthony Liguori, qemu-devel On Wed, 2011-05-25 at 10:23 +0200, Jan Kiszka wrote: > On 2011-05-25 10:13, Ian Campbell wrote: > > On Tue, 2011-05-24 at 18:18 +0200, Jan Kiszka wrote: > >> On 2011-05-24 18:06, Ian Campbell wrote: > >> Well, first of all I think this revealed a Xen bug because it crashes > >> when you try to run xenfv with an inappropriate accelerator, no? What is > >> the result of -machine xenfv,accel=tcg or, without my patch, -M xenfv > >> -machine accel=tcg? > > > > Unsurprisingly it crashed... > > > > I'm not sure if this is a bug in the xen side of simply a case of > > providing enough rope. I didn't really follow the threads which resulted > > in the accel stuff all that closely so I'm not sure what the intention > > was, it seems to me that the accel option is invalid with certain > > machine types though. > > > > I suppose in theory -machine xenpv,accel=kvm might result in xenner or > > something, accel=xenpv,tcg I'm less sure about (perhaps xenner too?). > > For -machine xenfv I don't expect anything other than accel=xen makes > > much sense. > > The point is that crashing is always a very poor way of reporting a > misconfiguration to the user. I bet you are able to detect and report > that more gracefully, e.g. during xenfv machine init. ACK. How about: 8<------------------------------------ >From b23cadda8da72da480fe7cc8e31ffccd449f4b47 Mon Sep 17 00:00:00 2001 From: Ian Campbell <ian.campbell@citrix.com> Date: Wed, 25 May 2011 09:32:07 +0100 Subject: [PATCH] Fail to initialise Xen HVM support gracefully if Xen acceleration not enabled. This is preferable to the existing segmentation fault due to use of uninitialised xen_xc context, this is normally inialisted in xen_init which is not called if xen acceleration is not enabled. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Cc: Jan Kiszka <jan.kiszka@siemens.com> Cc: Anthony PERARD <anthony.perard@citrix.com> --- xen-all.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/xen-all.c b/xen-all.c index 377aff7..c04bcbf 100644 --- a/xen-all.c +++ b/xen-all.c @@ -783,6 +783,12 @@ int xen_hvm_init(void) unsigned long ioreq_pfn; XenIOState *state; + if (!xen_xc) { + errno = ENOTSUP; + perror("xen: xen hvm requires accel=xen"); + return -errno; + } + state = qemu_mallocz(sizeof (XenIOState)); state->xce_handle = xen_xc_evtchn_open(NULL, 0); -- 1.7.2.5 -- Ian Campbell Current Noise: Blind Melon - No Rain Who are you? ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] Generalize -machine command line option 2011-05-25 8:13 ` [Qemu-devel] [PATCH v2 " Ian Campbell 2011-05-25 8:23 ` Jan Kiszka @ 2011-05-25 10:54 ` Anthony PERARD 2011-05-26 9:04 ` Ian Campbell 1 sibling, 1 reply; 12+ messages in thread From: Anthony PERARD @ 2011-05-25 10:54 UTC (permalink / raw) To: Jan Kiszka, Ian Campbell; +Cc: qemu-devel On Wed, 25 May 2011, Ian Campbell wrote: > On Tue, 2011-05-24 at 18:18 +0200, Jan Kiszka wrote: > > On 2011-05-24 18:06, Ian Campbell wrote: > > > On Sun, 2011-05-22 at 13:00 +0200, Jan Kiszka wrote: > > >> From: Jan Kiszka <jan.kiszka@siemens.com> > > >> > > >> -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 <anthony.perard@citrix.com> > > >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > > > > > "-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...) > > > > Well, first of all I think this revealed a Xen bug because it crashes > > when you try to run xenfv with an inappropriate accelerator, no? What is > > the result of -machine xenfv,accel=tcg or, without my patch, -M xenfv > > -machine accel=tcg? > > Unsurprisingly it crashed... > > I'm not sure if this is a bug in the xen side of simply a case of > providing enough rope. I didn't really follow the threads which resulted > in the accel stuff all that closely so I'm not sure what the intention > was, it seems to me that the accel option is invalid with certain > machine types though. This options was here to initialise the xen stuff early in the QEMU initilisation, and the purpose was to replace --enable-xen (and --enable-kvm) at the same occasion. But since most of the xen stuff have been put in xen_hvm_init, called by machine{xenfv}->init(), it's could make sense to call xen_init from xen_hvm_init, or just check if it have been called. > I suppose in theory -machine xenpv,accel=kvm might result in xenner or > something, accel=xenpv,tcg I'm less sure about (perhaps xenner too?). > For -machine xenfv I don't expect anything other than accel=xen makes > much sense. But a -machine pc,accel=xen could result in a xenfv machine. We already use the pc machine for xenfv with few difference. So a -machine pc,accel=xen:kvm could use the hypervisor present on the machine :) even if it's not realy useful in many case. -- Anthony PERARD ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] Generalize -machine command line option 2011-05-25 10:54 ` Anthony PERARD @ 2011-05-26 9:04 ` Ian Campbell 0 siblings, 0 replies; 12+ messages in thread From: Ian Campbell @ 2011-05-26 9:04 UTC (permalink / raw) To: Anthony PERARD; +Cc: Jan Kiszka, qemu-devel On Wed, 2011-05-25 at 11:54 +0100, Anthony PERARD wrote: > On Wed, 25 May 2011, Ian Campbell wrote: > > > On Tue, 2011-05-24 at 18:18 +0200, Jan Kiszka wrote: > > > On 2011-05-24 18:06, Ian Campbell wrote: > > > > On Sun, 2011-05-22 at 13:00 +0200, Jan Kiszka wrote: > > > >> From: Jan Kiszka <jan.kiszka@siemens.com> > > > >> > > > >> -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 <anthony.perard@citrix.com> > > > >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > > > > > > > "-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...) > > > > > > Well, first of all I think this revealed a Xen bug because it crashes > > > when you try to run xenfv with an inappropriate accelerator, no? What is > > > the result of -machine xenfv,accel=tcg or, without my patch, -M xenfv > > > -machine accel=tcg? > > > > Unsurprisingly it crashed... > > > > I'm not sure if this is a bug in the xen side of simply a case of > > providing enough rope. I didn't really follow the threads which resulted > > in the accel stuff all that closely so I'm not sure what the intention > > was, it seems to me that the accel option is invalid with certain > > machine types though. > > This options was here to initialise the xen stuff early in the QEMU > initilisation, and the purpose was to replace --enable-xen (and > --enable-kvm) at the same occasion. > > But since most of the xen stuff have been put in xen_hvm_init, called > by machine{xenfv}->init(), it's could make sense to call xen_init from > xen_hvm_init, or just check if it have been called. I posted a patch to do that check elsewhere in this thread, I did wonder about moving the call too but I'll leave that up to you. > > I suppose in theory -machine xenpv,accel=kvm might result in xenner or > > something, accel=xenpv,tcg I'm less sure about (perhaps xenner too?). > > For -machine xenfv I don't expect anything other than accel=xen makes > > much sense. > > But a -machine pc,accel=xen could result in a xenfv machine. We already > use the pc machine for xenfv with few difference. So a -machine > pc,accel=xen:kvm could use the hypervisor present on the machine :) > even if it's not realy useful in many case. That seems reasonable, I guess. Ian. -- Ian Campbell Current Noise: She Said Destroy - No Zen Tell me, O Octopus, I begs, Is those things arms, or is they legs? I marvel at thee, Octopus; If I were thou, I'd call me us. -- Ogden Nash ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2011-06-07 15:58 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-05-20 11:18 [Qemu-devel] [PATCH 1/2] Generalize -machine command line option Jan Kiszka 2011-05-22 11:00 ` [Qemu-devel] [PATCH v2 " Jan Kiszka 2011-05-24 16:06 ` Ian Campbell 2011-05-24 16:18 ` Jan Kiszka 2011-05-25 7:03 ` [Qemu-devel] [PATCH v3 " Jan Kiszka 2011-05-25 8:17 ` Ian Campbell 2011-06-07 15:58 ` Jan Kiszka 2011-05-25 8:13 ` [Qemu-devel] [PATCH v2 " Ian Campbell 2011-05-25 8:23 ` Jan Kiszka 2011-05-25 8:34 ` Ian Campbell 2011-05-25 10:54 ` Anthony PERARD 2011-05-26 9:04 ` Ian Campbell
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.