| Message ID | 87wowgyqj8.fsf@dusky.pond.sub.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07.05.2018 07:37, Markus Armbruster wrote: > Paolo Bonzini <pbonzini@redhat.com> writes: > >> On 04/05/2018 19:01, Thomas Huth wrote: >>> The -no-kvm* options are a remainder of the ancient "qemu-kvm" >>> fork. They have never been officially documented in our qemu-doc, >>> they have been marked as deprecated in the sources since a very >>> long time, and we've marked them as deprecated in our qemu-doc >>> since QEMU v2.10. So far I haven't seen any complaints that we >>> should keep them, so it's time to remove these old parameters now. >>> >>> While I'm at it, this patch series also removes a left-over from >>> the "-tdf" option (which has been removed with QEMU v2.12) and >>> fixes a deficiency in the option parsing code that has been >>> revealed by the remainder of the "-tdf" option. >>> >>> Thomas Huth (5): >>> qemu-options: Remove remainders of the -tdf option >>> qemu-options: Bail out on unsupported options instead of silently >>> ignoring them >>> qemu-options: Remove deprecated -no-kvm-pit-reinjection >>> qemu-options: Remove deprecated -no-kvm-irqchip >>> qemu-options: Remove deprecated -no-kvm >> >> I'm not that sure anymore about -no-kvm. It can come in handy for >> distros (*cough* RHEL *cough) that only ship a qemu-kvm binary with >> default accelerator "-machine accel=kvm:tcg". >> >> Yeah, "-accel tcg" is only a few characters later, but it is a >> relatively recent addition. > > 2012 is "relatively recent" in the alternate RHEL universe, perhaps :) That was the "-machine accel=tcg", but Paolo was rather talking about "-accel tcg", which has been added last year. Yes, we've got three (!) ways of dealing with accelerators now: "-M accel=", "-accel" and "-enable-kvm + -no-kvm". We're very userfriendly, aren't we? ;-) Thomas
Thomas Huth <thuth@redhat.com> writes: > On 07.05.2018 07:37, Markus Armbruster wrote: >> Paolo Bonzini <pbonzini@redhat.com> writes: >> >>> On 04/05/2018 19:01, Thomas Huth wrote: >>>> The -no-kvm* options are a remainder of the ancient "qemu-kvm" >>>> fork. They have never been officially documented in our qemu-doc, >>>> they have been marked as deprecated in the sources since a very >>>> long time, and we've marked them as deprecated in our qemu-doc >>>> since QEMU v2.10. So far I haven't seen any complaints that we >>>> should keep them, so it's time to remove these old parameters now. >>>> >>>> While I'm at it, this patch series also removes a left-over from >>>> the "-tdf" option (which has been removed with QEMU v2.12) and >>>> fixes a deficiency in the option parsing code that has been >>>> revealed by the remainder of the "-tdf" option. >>>> >>>> Thomas Huth (5): >>>> qemu-options: Remove remainders of the -tdf option >>>> qemu-options: Bail out on unsupported options instead of silently >>>> ignoring them >>>> qemu-options: Remove deprecated -no-kvm-pit-reinjection >>>> qemu-options: Remove deprecated -no-kvm-irqchip >>>> qemu-options: Remove deprecated -no-kvm >>> >>> I'm not that sure anymore about -no-kvm. It can come in handy for >>> distros (*cough* RHEL *cough) that only ship a qemu-kvm binary with >>> default accelerator "-machine accel=kvm:tcg". >>> >>> Yeah, "-accel tcg" is only a few characters later, but it is a >>> relatively recent addition. >> >> 2012 is "relatively recent" in the alternate RHEL universe, perhaps :) > > That was the "-machine accel=tcg", but Paolo was rather talking about > "-accel tcg", which has been added last year. Yes, we've got three (!) > ways of dealing with accelerators now: "-M accel=", "-accel" and > "-enable-kvm + -no-kvm". We're very userfriendly, aren't we? ;-) Ugh. Apparently, vl.c still is a playground without adult supervision.
On 07/05/2018 07:37, Markus Armbruster wrote: >> I'm not that sure anymore about -no-kvm. It can come in handy for >> distros (*cough* RHEL *cough) that only ship a qemu-kvm binary with >> default accelerator "-machine accel=kvm:tcg". >> >> Yeah, "-accel tcg" is only a few characters later, but it is a >> relatively recent addition. > > 2012 is "relatively recent" in the alternate RHEL universe, perhaps :) Well, that's for "-machine accel=tcg" which is quite a mouthful. "-accel tcg" was added in 2.9, about a year ago (commit 8d4e9146b3, "tcg: add options for enabling MTTCG", 2017-02-24). Paolo > > Author: Jan Kiszka <jan.kiszka@siemens.com> > Date: Fri Oct 5 14:51:45 2012 -0300 > > Emulate qemu-kvms -no-kvm option > > Releases of qemu-kvm will be interrupted at qemu 1.3.0. > Users should switch to plain qemu releases. > To avoid breaking scenarios which are setup with command line > options specific to qemu-kvm, port these switches from qemu-kvm > to qemu.git. > > Port -no-kvm option. > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
On 07/05/2018 09:56, Markus Armbruster wrote: >>>> I'm not that sure anymore about -no-kvm. It can come in handy for >>>> distros (*cough* RHEL *cough) that only ship a qemu-kvm binary with >>>> default accelerator "-machine accel=kvm:tcg". >>>> >>>> Yeah, "-accel tcg" is only a few characters later, but it is a >>>> relatively recent addition. >>> >>> 2012 is "relatively recent" in the alternate RHEL universe, perhaps :) >> >> That was the "-machine accel=tcg", but Paolo was rather talking about >> "-accel tcg", which has been added last year. Yes, we've got three (!) >> ways of dealing with accelerators now: "-M accel=", "-accel" and >> "-enable-kvm + -no-kvm". We're very userfriendly, aren't we? ;-) > > Ugh. Apparently, vl.c still is a playground without adult supervision. I'm not sure I get it... "-accel" was added in order to provide accelerator options. If anything, "-machine accel" (added back in 2010) is the one that makes little sense these days. Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > On 07/05/2018 07:37, Markus Armbruster wrote: >>> I'm not that sure anymore about -no-kvm. It can come in handy for >>> distros (*cough* RHEL *cough) that only ship a qemu-kvm binary with >>> default accelerator "-machine accel=kvm:tcg". >>> >>> Yeah, "-accel tcg" is only a few characters later, but it is a >>> relatively recent addition. >> >> 2012 is "relatively recent" in the alternate RHEL universe, perhaps :) > > Well, that's for "-machine accel=tcg" which is quite a mouthful. "-M accel=tcg" is a whopping *two* characters more than "-accel tcg". > "-accel tcg" was added in 2.9, about a year ago (commit 8d4e9146b3, > "tcg: add options for enabling MTTCG", 2017-02-24). Yes, I confused the two.
Paolo Bonzini <pbonzini@redhat.com> writes: > On 07/05/2018 09:56, Markus Armbruster wrote: >>>>> I'm not that sure anymore about -no-kvm. It can come in handy for >>>>> distros (*cough* RHEL *cough) that only ship a qemu-kvm binary with >>>>> default accelerator "-machine accel=kvm:tcg". >>>>> >>>>> Yeah, "-accel tcg" is only a few characters later, but it is a >>>>> relatively recent addition. >>>> >>>> 2012 is "relatively recent" in the alternate RHEL universe, perhaps :) >>> >>> That was the "-machine accel=tcg", but Paolo was rather talking about >>> "-accel tcg", which has been added last year. Yes, we've got three (!) >>> ways of dealing with accelerators now: "-M accel=", "-accel" and >>> "-enable-kvm + -no-kvm". We're very userfriendly, aren't we? ;-) >> >> Ugh. Apparently, vl.c still is a playground without adult supervision. > > I'm not sure I get it... "-accel" was added in order to provide > accelerator options. If anything, "-machine accel" (added back in 2010) > is the one that makes little sense these days. Adding more and more ways to do the same stuff does not improve an interface. Interface design needs to be *opinionated*. If we decide -machine accel=tcg isn't a nice interface, by all means create a better one, but as replacement[*], not as addition. Furthermore: tcg: add options for enabling MTTCG We know there will be cases where MTTCG won't work until additional work is done in the front/back ends to support. It will however be useful to be able to turn it on. As a result MTTCG will default to off unless the combination is supported. However the user can turn it on for the sake of testing. Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com> [AJB: move to -accel tcg,thread=multi|single, defaults] Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <rth@twiddle.net> I'm sorry, but this us sub-par. Yes, the commit is also about "enabling MTTCG", but it also adds a new way to select accelerators, without ever spelling that out. It should've been split, and properly described. [*] Replacements generally involve deprecation and a grace period.
On 07/05/2018 13:56, Markus Armbruster wrote: > Adding more and more ways to do the same stuff does not improve an > interface. Interface design needs to be *opinionated*. If we decide > -machine accel=tcg isn't a nice interface, by all means create a better > one, but as replacement[*], not as addition. > > Furthermore: > > tcg: add options for enabling MTTCG > > We know there will be cases where MTTCG won't work until additional work > is done in the front/back ends to support. It will however be useful to > be able to turn it on. > > As a result MTTCG will default to off unless the combination is > supported. However the user can turn it on for the sake of testing. > > Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com> > [AJB: move to -accel tcg,thread=multi|single, defaults] > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Reviewed-by: Richard Henderson <rth@twiddle.net> > > I'm sorry, but this us sub-par. Yes, the commit is also about "enabling > MTTCG", but it also adds a new way to select accelerators, without ever > spelling that out. It should've been split, and properly described. Perhaps we can deprecate "-M accel" then, and also while we're at it move kernel_irqchip from -machine to "-accel kvm" where it belongs? Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > On 07/05/2018 13:56, Markus Armbruster wrote: >> Adding more and more ways to do the same stuff does not improve an >> interface. Interface design needs to be *opinionated*. If we decide >> -machine accel=tcg isn't a nice interface, by all means create a better >> one, but as replacement[*], not as addition. >> >> Furthermore: >> >> tcg: add options for enabling MTTCG >> >> We know there will be cases where MTTCG won't work until additional work >> is done in the front/back ends to support. It will however be useful to >> be able to turn it on. >> >> As a result MTTCG will default to off unless the combination is >> supported. However the user can turn it on for the sake of testing. >> >> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com> >> [AJB: move to -accel tcg,thread=multi|single, defaults] >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> Reviewed-by: Richard Henderson <rth@twiddle.net> >> >> I'm sorry, but this us sub-par. Yes, the commit is also about "enabling >> MTTCG", but it also adds a new way to select accelerators, without ever >> spelling that out. It should've been split, and properly described. > > Perhaps we can deprecate "-M accel" then, and also while we're at it > move kernel_irqchip from -machine to "-accel kvm" where it belongs? Sounds good to me.
On 07/05/2018 18:50, Markus Armbruster wrote: > Paolo Bonzini <pbonzini@redhat.com> writes: > >> On 07/05/2018 13:56, Markus Armbruster wrote: >>> Adding more and more ways to do the same stuff does not improve an >>> interface. Interface design needs to be *opinionated*. If we decide >>> -machine accel=tcg isn't a nice interface, by all means create a better >>> one, but as replacement[*], not as addition. >>> >>> Furthermore: >>> >>> tcg: add options for enabling MTTCG >>> >>> We know there will be cases where MTTCG won't work until additional work >>> is done in the front/back ends to support. It will however be useful to >>> be able to turn it on. >>> >>> As a result MTTCG will default to off unless the combination is >>> supported. However the user can turn it on for the sake of testing. >>> >>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com> >>> [AJB: move to -accel tcg,thread=multi|single, defaults] >>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>> Reviewed-by: Richard Henderson <rth@twiddle.net> >>> >>> I'm sorry, but this us sub-par. Yes, the commit is also about "enabling >>> MTTCG", but it also adds a new way to select accelerators, without ever >>> spelling that out. It should've been split, and properly described. >> >> Perhaps we can deprecate "-M accel" then, and also while we're at it >> move kernel_irqchip from -machine to "-accel kvm" where it belongs? > > Sounds good to me. Thomas, here's one for you! :) Paolo
On 07.05.2018 19:01, Paolo Bonzini wrote: > On 07/05/2018 18:50, Markus Armbruster wrote: >> Paolo Bonzini <pbonzini@redhat.com> writes: >> >>> On 07/05/2018 13:56, Markus Armbruster wrote: >>>> Adding more and more ways to do the same stuff does not improve an >>>> interface. Interface design needs to be *opinionated*. If we decide >>>> -machine accel=tcg isn't a nice interface, by all means create a better >>>> one, but as replacement[*], not as addition. >>>> >>>> Furthermore: >>>> >>>> tcg: add options for enabling MTTCG >>>> >>>> We know there will be cases where MTTCG won't work until additional work >>>> is done in the front/back ends to support. It will however be useful to >>>> be able to turn it on. >>>> >>>> As a result MTTCG will default to off unless the combination is >>>> supported. However the user can turn it on for the sake of testing. >>>> >>>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com> >>>> [AJB: move to -accel tcg,thread=multi|single, defaults] >>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>>> Reviewed-by: Richard Henderson <rth@twiddle.net> >>>> >>>> I'm sorry, but this us sub-par. Yes, the commit is also about "enabling >>>> MTTCG", but it also adds a new way to select accelerators, without ever >>>> spelling that out. It should've been split, and properly described. >>> >>> Perhaps we can deprecate "-M accel" then, and also while we're at it >>> move kernel_irqchip from -machine to "-accel kvm" where it belongs? >> >> Sounds good to me. > > Thomas, here's one for you! :) I like the idea of deprecating "-machine accel=xxx" - but that will also be a bigger change, since "-accel" is internally setting the accel option of the machine again... Also libvirt is still using "-machine accel=..." as far as I know, so we've got to make sure that this gets changed there, too... Thomas
diff --git a/qemu-options.hx b/qemu-options.hx index 628bd44591..fe8f15c541 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2888,6 +2888,9 @@ STEXI Enable FIPS 140-2 compliance mode. ETEXI +HXCOMM Deprecated by -machine accel=tcg property +DEF("no-kvm", HAS_ARG, QEMU_OPTION_no_kvm, "", QEMU_ARCH_I386) + HXCOMM Deprecated by kvm-pit driver properties DEF("no-kvm-pit-reinjection", HAS_ARG, QEMU_OPTION_no_kvm_pit_reinjection, "", QEMU_ARCH_I386) diff --git a/vl.c b/vl.c index cd7c0fbd52..b39f22ed35 100644 --- a/vl.c +++ b/vl.c @@ -3171,6 +3171,10 @@ int main(int argc, char **argv, char **envp) machine = machine_parse(optarg); } break; + case QEMU_OPTION_no_kvm: + olist = qemu_find_opts("machine"); + qemu_opts_parse(olist, "accel=tcg", 0); + break; case QEMU_OPTION_no_kvm_pit: { fprintf(stderr, "Warning: KVM PIT can no longer be disabled " "separately.\n");