| Message ID | 1493718721-29774-1-git-send-email-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 02/05/2017 11:52, Thomas Huth wrote: > Since 'hax' is a possible accelerator nowadays, too, the '-accel' > option should support it and we should mention it in the documentation, > too. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > qemu-options.hx | 8 ++++---- > vl.c | 24 ++++++++++-------------- > 2 files changed, 14 insertions(+), 18 deletions(-) > > diff --git a/qemu-options.hx b/qemu-options.hx > index 787b9c3..c7b1d2d 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -31,7 +31,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ > "-machine [type=]name[,prop[=value][,...]]\n" > " selects emulated machine ('-machine help' for list)\n" > " property accel=accel1[:accel2[:...]] selects accelerator\n" > - " supported accelerators are kvm, xen, tcg (default: tcg)\n" > + " supported accelerators are kvm, xen, hax or tcg (default: tcg)\n" > " kernel_irqchip=on|off|split controls accelerated irqchip support (default=off)\n" > " vmport=on|off|auto controls emulation of vmport (default: auto)\n" > " kvm_shadow_mem=size of KVM shadow MMU in bytes\n" > @@ -52,9 +52,9 @@ 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. > +kvm, xen, hax 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. > @item kernel_irqchip=on|off > Controls in-kernel irqchip support for the chosen accelerator when available. > @item gfx_passthru=on|off > diff --git a/vl.c b/vl.c > index f46e070..d5e88fb 100644 > --- a/vl.c > +++ b/vl.c > @@ -3725,26 +3725,22 @@ int main(int argc, char **argv, char **envp) > qdev_prop_register_global(&kvm_pit_lost_tick_policy); > break; > } > - case QEMU_OPTION_accel: > + case QEMU_OPTION_accel: { > + char *mstr; > + > accel_opts = qemu_opts_parse_noisily(qemu_find_opts("accel"), > optarg, true); > optarg = qemu_opt_get(accel_opts, "accel"); > - > - olist = qemu_find_opts("machine"); > - if (strcmp("kvm", optarg) == 0) { > - qemu_opts_parse_noisily(olist, "accel=kvm", false); > - } else if (strcmp("xen", optarg) == 0) { > - qemu_opts_parse_noisily(olist, "accel=xen", false); > - } else if (strcmp("tcg", optarg) == 0) { > - qemu_opts_parse_noisily(olist, "accel=tcg", false); > - } else { > - if (!is_help_option(optarg)) { > - error_printf("Unknown accelerator: %s", optarg); > - } > - error_printf("Supported accelerators: kvm, xen, tcg\n"); > + if (is_help_option(optarg)) { > + error_printf("Possible accelerators: kvm, xen, hax, tcg\n"); > exit(1); > } > + mstr = g_strdup_printf("accel=%s", optarg); > + olist = qemu_find_opts("machine"); > + qemu_opts_parse_noisily(olist, mstr, false); Thanks for the report. I think this has bad side effects when one specifies an invalid accelerator such as -accel tcg,,usb=on. Eduardo has more plans for -accel, I think. Paolo > + g_free(mstr); > break; > + } > case QEMU_OPTION_usb: > olist = qemu_find_opts("machine"); > qemu_opts_parse_noisily(olist, "usb=on", false); >
On Tue, May 02, 2017 at 02:58:35PM +0200, Paolo Bonzini wrote: > > > On 02/05/2017 11:52, Thomas Huth wrote: > > Since 'hax' is a possible accelerator nowadays, too, the '-accel' > > option should support it and we should mention it in the documentation, > > too. > > > > Signed-off-by: Thomas Huth <thuth@redhat.com> > > --- > > qemu-options.hx | 8 ++++---- > > vl.c | 24 ++++++++++-------------- > > 2 files changed, 14 insertions(+), 18 deletions(-) > > > > diff --git a/qemu-options.hx b/qemu-options.hx > > index 787b9c3..c7b1d2d 100644 > > --- a/qemu-options.hx > > +++ b/qemu-options.hx > > @@ -31,7 +31,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ > > "-machine [type=]name[,prop[=value][,...]]\n" > > " selects emulated machine ('-machine help' for list)\n" > > " property accel=accel1[:accel2[:...]] selects accelerator\n" > > - " supported accelerators are kvm, xen, tcg (default: tcg)\n" > > + " supported accelerators are kvm, xen, hax or tcg (default: tcg)\n" > > " kernel_irqchip=on|off|split controls accelerated irqchip support (default=off)\n" > > " vmport=on|off|auto controls emulation of vmport (default: auto)\n" > > " kvm_shadow_mem=size of KVM shadow MMU in bytes\n" > > @@ -52,9 +52,9 @@ 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. > > +kvm, xen, hax 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. > > @item kernel_irqchip=on|off > > Controls in-kernel irqchip support for the chosen accelerator when available. > > @item gfx_passthru=on|off > > diff --git a/vl.c b/vl.c > > index f46e070..d5e88fb 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -3725,26 +3725,22 @@ int main(int argc, char **argv, char **envp) > > qdev_prop_register_global(&kvm_pit_lost_tick_policy); > > break; > > } > > - case QEMU_OPTION_accel: > > + case QEMU_OPTION_accel: { > > + char *mstr; > > + > > accel_opts = qemu_opts_parse_noisily(qemu_find_opts("accel"), > > optarg, true); > > optarg = qemu_opt_get(accel_opts, "accel"); > > - > > - olist = qemu_find_opts("machine"); > > - if (strcmp("kvm", optarg) == 0) { > > - qemu_opts_parse_noisily(olist, "accel=kvm", false); > > - } else if (strcmp("xen", optarg) == 0) { > > - qemu_opts_parse_noisily(olist, "accel=xen", false); > > - } else if (strcmp("tcg", optarg) == 0) { > > - qemu_opts_parse_noisily(olist, "accel=tcg", false); > > - } else { Getting rid of this hardcoded list is welcome. > > - if (!is_help_option(optarg)) { > > - error_printf("Unknown accelerator: %s", optarg); > > - } > > - error_printf("Supported accelerators: kvm, xen, tcg\n"); > > + if (is_help_option(optarg)) { > > + error_printf("Possible accelerators: kvm, xen, hax, tcg\n"); This should be replaced by object_class_get_list()- or object_class_for_each()-based loop, so we don't need to maintain the list manually. But that's for a follow-up patch. > > exit(1); > > } > > + mstr = g_strdup_printf("accel=%s", optarg); > > + olist = qemu_find_opts("machine"); > > + qemu_opts_parse_noisily(olist, mstr, false); > > Thanks for the report. I think this has bad side effects when one > specifies an invalid accelerator such as -accel tcg,,usb=on. Eduardo > has more plans for -accel, I think. We can use qemu_opt_set() here to avoid this problem. I wasn't even aware that we had a new -accel option. Thanks for CCing me. I believe -accel should be mapped to "-object ...-accel" eventually, but that's for the future. > > Paolo > > > + g_free(mstr); > > break; > > + } > > case QEMU_OPTION_usb: > > olist = qemu_find_opts("machine"); > > qemu_opts_parse_noisily(olist, "usb=on", false); > >
diff --git a/qemu-options.hx b/qemu-options.hx index 787b9c3..c7b1d2d 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -31,7 +31,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ "-machine [type=]name[,prop[=value][,...]]\n" " selects emulated machine ('-machine help' for list)\n" " property accel=accel1[:accel2[:...]] selects accelerator\n" - " supported accelerators are kvm, xen, tcg (default: tcg)\n" + " supported accelerators are kvm, xen, hax or tcg (default: tcg)\n" " kernel_irqchip=on|off|split controls accelerated irqchip support (default=off)\n" " vmport=on|off|auto controls emulation of vmport (default: auto)\n" " kvm_shadow_mem=size of KVM shadow MMU in bytes\n" @@ -52,9 +52,9 @@ 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. +kvm, xen, hax 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. @item kernel_irqchip=on|off Controls in-kernel irqchip support for the chosen accelerator when available. @item gfx_passthru=on|off diff --git a/vl.c b/vl.c index f46e070..d5e88fb 100644 --- a/vl.c +++ b/vl.c @@ -3725,26 +3725,22 @@ int main(int argc, char **argv, char **envp) qdev_prop_register_global(&kvm_pit_lost_tick_policy); break; } - case QEMU_OPTION_accel: + case QEMU_OPTION_accel: { + char *mstr; + accel_opts = qemu_opts_parse_noisily(qemu_find_opts("accel"), optarg, true); optarg = qemu_opt_get(accel_opts, "accel"); - - olist = qemu_find_opts("machine"); - if (strcmp("kvm", optarg) == 0) { - qemu_opts_parse_noisily(olist, "accel=kvm", false); - } else if (strcmp("xen", optarg) == 0) { - qemu_opts_parse_noisily(olist, "accel=xen", false); - } else if (strcmp("tcg", optarg) == 0) { - qemu_opts_parse_noisily(olist, "accel=tcg", false); - } else { - if (!is_help_option(optarg)) { - error_printf("Unknown accelerator: %s", optarg); - } - error_printf("Supported accelerators: kvm, xen, tcg\n"); + if (is_help_option(optarg)) { + error_printf("Possible accelerators: kvm, xen, hax, tcg\n"); exit(1); } + mstr = g_strdup_printf("accel=%s", optarg); + olist = qemu_find_opts("machine"); + qemu_opts_parse_noisily(olist, mstr, false); + g_free(mstr); break; + } case QEMU_OPTION_usb: olist = qemu_find_opts("machine"); qemu_opts_parse_noisily(olist, "usb=on", false);
Since 'hax' is a possible accelerator nowadays, too, the '-accel' option should support it and we should mention it in the documentation, too. Signed-off-by: Thomas Huth <thuth@redhat.com> --- qemu-options.hx | 8 ++++---- vl.c | 24 ++++++++++-------------- 2 files changed, 14 insertions(+), 18 deletions(-)