* [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' @ 2017-05-02 10:06 Thomas Huth 2017-05-02 10:21 ` Daniel P. Berrange 2017-05-02 10:32 ` Christian Borntraeger 0 siblings, 2 replies; 22+ messages in thread From: Thomas Huth @ 2017-05-02 10:06 UTC (permalink / raw) To: qemu-devel; +Cc: Paolo Bonzini, Vincent Palatin The '-enable-...' option do not make too much sense: They do not allow additional parameters, using '-accel xxx' is shorter than '-enable-xxx' and we're also inconsistent here, since there is no '-enable-xen' option available. So let's try to convince the users to use '-accel xxx' instead. Signed-off-by: Thomas Huth <thuth@redhat.com> --- qemu-options.hx | 5 +++-- vl.c | 4 ++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index c7b1d2d..eb33286 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -3367,7 +3367,8 @@ STEXI @item -enable-kvm @findex -enable-kvm Enable KVM full virtualization support. This option is only available -if KVM support is enabled when compiling. +if KVM support is enabled when compiling. Note: This option is deprecated, +please use @code{-accel kvm} instead. ETEXI DEF("enable-hax", 0, QEMU_OPTION_enable_hax, \ @@ -3378,7 +3379,7 @@ STEXI Enable HAX (Hardware-based Acceleration eXecution) support. This option is only available if HAX support is enabled when compiling. HAX is only applicable to MAC and Windows platform, and thus does not conflict with -KVM. +KVM. Note: This option is deprecated, please use @code{-accel hax} instead. ETEXI DEF("xen-domid", HAS_ARG, QEMU_OPTION_xen_domid, diff --git a/vl.c b/vl.c index d5e88fb..d5ec87e 100644 --- a/vl.c +++ b/vl.c @@ -3690,10 +3690,14 @@ int main(int argc, char **argv, char **envp) } break; case QEMU_OPTION_enable_kvm: + error_report("'-enable-kvm' is depreacted, please use " + "'-accel kvm' instead"); olist = qemu_find_opts("machine"); qemu_opts_parse_noisily(olist, "accel=kvm", false); break; case QEMU_OPTION_enable_hax: + error_report("'-enable-hax' is depreacted, please use " + "'-accel hax' instead"); olist = qemu_find_opts("machine"); qemu_opts_parse_noisily(olist, "accel=hax", false); break; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 10:06 [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' Thomas Huth @ 2017-05-02 10:21 ` Daniel P. Berrange 2017-05-02 10:29 ` Thomas Huth 2017-05-02 10:32 ` Christian Borntraeger 1 sibling, 1 reply; 22+ messages in thread From: Daniel P. Berrange @ 2017-05-02 10:21 UTC (permalink / raw) To: Thomas Huth; +Cc: qemu-devel, Paolo Bonzini, Vincent Palatin On Tue, May 02, 2017 at 12:06:40PM +0200, Thomas Huth wrote: > The '-enable-...' option do not make too much sense: They do not > allow additional parameters, using '-accel xxx' is shorter than > '-enable-xxx' and we're also inconsistent here, since there is > no '-enable-xen' option available. So let's try to convince the > users to use '-accel xxx' instead. What is our general approach to deciding which of our "legacy" parameters are merely syntactic sugar for other parameters, vs stuff we want to deprecate & eventually remove ? Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 10:21 ` Daniel P. Berrange @ 2017-05-02 10:29 ` Thomas Huth 0 siblings, 0 replies; 22+ messages in thread From: Thomas Huth @ 2017-05-02 10:29 UTC (permalink / raw) To: Daniel P. Berrange; +Cc: qemu-devel, Paolo Bonzini, Vincent Palatin On 02.05.2017 12:21, Daniel P. Berrange wrote: > On Tue, May 02, 2017 at 12:06:40PM +0200, Thomas Huth wrote: >> The '-enable-...' option do not make too much sense: They do not >> allow additional parameters, using '-accel xxx' is shorter than >> '-enable-xxx' and we're also inconsistent here, since there is >> no '-enable-xen' option available. So let's try to convince the >> users to use '-accel xxx' instead. > > What is our general approach to deciding which of our "legacy" parameters > are merely syntactic sugar for other parameters, vs stuff we want to > deprecate & eventually remove ? I'd say if the parameters can help to specify something in a short way, then it's nice-to-have syntactic sugar. Otherwise, it's just legacy cruft which should be removed one day. For the accelerator options, we've got now three (!) ways to specify them: 1) -machine accel=xxx (which is the way we use it internally, thus we should keep it) 2) -enable-xxx 3) -accel xxx -accel xxx makes sense as syntactic sugar since it helps to keep the command line short, but I fail to see the reason for the inconsistent and inflexible "-enable-xxx" here, so I'd like to suggest to mark it as deprecated so we could remove it one day (it's likely in use in the wild, so maybe remove it in v4.0.0 in a couple of years?). Thomas ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 10:06 [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' Thomas Huth 2017-05-02 10:21 ` Daniel P. Berrange @ 2017-05-02 10:32 ` Christian Borntraeger 2017-05-02 10:37 ` Thomas Huth 1 sibling, 1 reply; 22+ messages in thread From: Christian Borntraeger @ 2017-05-02 10:32 UTC (permalink / raw) To: Thomas Huth, qemu-devel; +Cc: Paolo Bonzini, Vincent Palatin On 05/02/2017 12:06 PM, Thomas Huth wrote: > The '-enable-...' option do not make too much sense: They do not > allow additional parameters, using '-accel xxx' is shorter than > '-enable-xxx' and we're also inconsistent here, since there is > no '-enable-xen' option available. So let's try to convince the > users to use '-accel xxx' instead. google has 36000 hits for "--enable-kvm" and 18000 hits for "--accel kvm" So I assume this will affect a lot of setups for only a very small benefit. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > qemu-options.hx | 5 +++-- > vl.c | 4 ++++ > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/qemu-options.hx b/qemu-options.hx > index c7b1d2d..eb33286 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -3367,7 +3367,8 @@ STEXI > @item -enable-kvm > @findex -enable-kvm > Enable KVM full virtualization support. This option is only available > -if KVM support is enabled when compiling. > +if KVM support is enabled when compiling. Note: This option is deprecated, > +please use @code{-accel kvm} instead. > ETEXI > > DEF("enable-hax", 0, QEMU_OPTION_enable_hax, \ > @@ -3378,7 +3379,7 @@ STEXI > Enable HAX (Hardware-based Acceleration eXecution) support. This option > is only available if HAX support is enabled when compiling. HAX is only > applicable to MAC and Windows platform, and thus does not conflict with > -KVM. > +KVM. Note: This option is deprecated, please use @code{-accel hax} instead. > ETEXI > > DEF("xen-domid", HAS_ARG, QEMU_OPTION_xen_domid, > diff --git a/vl.c b/vl.c > index d5e88fb..d5ec87e 100644 > --- a/vl.c > +++ b/vl.c > @@ -3690,10 +3690,14 @@ int main(int argc, char **argv, char **envp) > } > break; > case QEMU_OPTION_enable_kvm: > + error_report("'-enable-kvm' is depreacted, please use " > + "'-accel kvm' instead"); > olist = qemu_find_opts("machine"); > qemu_opts_parse_noisily(olist, "accel=kvm", false); > break; > case QEMU_OPTION_enable_hax: > + error_report("'-enable-hax' is depreacted, please use " > + "'-accel hax' instead"); > olist = qemu_find_opts("machine"); > qemu_opts_parse_noisily(olist, "accel=hax", false); > break; > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 10:32 ` Christian Borntraeger @ 2017-05-02 10:37 ` Thomas Huth 2017-05-02 10:48 ` Christian Borntraeger 2017-05-02 12:21 ` Christian Borntraeger 0 siblings, 2 replies; 22+ messages in thread From: Thomas Huth @ 2017-05-02 10:37 UTC (permalink / raw) To: Christian Borntraeger, qemu-devel; +Cc: Paolo Bonzini, Vincent Palatin On 02.05.2017 12:32, Christian Borntraeger wrote: > On 05/02/2017 12:06 PM, Thomas Huth wrote: >> The '-enable-...' option do not make too much sense: They do not >> allow additional parameters, using '-accel xxx' is shorter than >> '-enable-xxx' and we're also inconsistent here, since there is >> no '-enable-xen' option available. So let's try to convince the >> users to use '-accel xxx' instead. > > google has 36000 hits for "--enable-kvm" and 18000 hits for "--accel kvm" > So I assume this will affect a lot of setups for only a very small benefit. I'm aware of the fact that likely a lot of users are still using -enable-kvm, and I did not mean that we should remove it soon yet. But IMHO we should start now to inform the users that they should slowly switch to the better option "-accel" instead, so that we could maybe remove this "-enable-xxx" stuff sometime in the distant future (let's say QEMU v4.0?). Thomas ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 10:37 ` Thomas Huth @ 2017-05-02 10:48 ` Christian Borntraeger 2017-05-02 11:26 ` Thomas Huth 2017-05-02 13:22 ` Paolo Bonzini 2017-05-02 12:21 ` Christian Borntraeger 1 sibling, 2 replies; 22+ messages in thread From: Christian Borntraeger @ 2017-05-02 10:48 UTC (permalink / raw) To: Thomas Huth, qemu-devel; +Cc: Paolo Bonzini, Vincent Palatin On 05/02/2017 12:37 PM, Thomas Huth wrote: > On 02.05.2017 12:32, Christian Borntraeger wrote: >> On 05/02/2017 12:06 PM, Thomas Huth wrote: >>> The '-enable-...' option do not make too much sense: They do not >>> allow additional parameters, using '-accel xxx' is shorter than >>> '-enable-xxx' and we're also inconsistent here, since there is >>> no '-enable-xen' option available. So let's try to convince the >>> users to use '-accel xxx' instead. >> >> google has 36000 hits for "--enable-kvm" and 18000 hits for "--accel kvm" >> So I assume this will affect a lot of setups for only a very small benefit. > > I'm aware of the fact that likely a lot of users are still using > -enable-kvm, and I did not mean that we should remove it soon yet. But > IMHO we should start now to inform the users that they should slowly > switch to the better option "-accel" instead, so that we could maybe > remove this "-enable-xxx" stuff sometime in the distant future (let's > say QEMU v4.0?). I come from the Linux side, where "breaking a working setup" will result in an angry Linus. We certainly have not such strict rules here and we could base the decision on the question "how expensive is the maintenance of this option?". I think marking it as "legacy option" is fine, but I doubt that removing it will make qemu maintenance cheaper. So my preferred variant is - have it marked in the docs as "legacy" - no error_report as it might break some setups (since error_report might write to the monitor) - never remove the option unless it turns out to be a burden But its certainly not my call to make. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 10:48 ` Christian Borntraeger @ 2017-05-02 11:26 ` Thomas Huth 2017-05-02 11:59 ` Daniel P. Berrange 2017-05-02 13:22 ` Paolo Bonzini 1 sibling, 1 reply; 22+ messages in thread From: Thomas Huth @ 2017-05-02 11:26 UTC (permalink / raw) To: Christian Borntraeger, qemu-devel; +Cc: Paolo Bonzini, Vincent Palatin On 02.05.2017 12:48, Christian Borntraeger wrote: > On 05/02/2017 12:37 PM, Thomas Huth wrote: >> On 02.05.2017 12:32, Christian Borntraeger wrote: >>> On 05/02/2017 12:06 PM, Thomas Huth wrote: >>>> The '-enable-...' option do not make too much sense: They do not >>>> allow additional parameters, using '-accel xxx' is shorter than >>>> '-enable-xxx' and we're also inconsistent here, since there is >>>> no '-enable-xen' option available. So let's try to convince the >>>> users to use '-accel xxx' instead. >>> >>> google has 36000 hits for "--enable-kvm" and 18000 hits for "--accel kvm" >>> So I assume this will affect a lot of setups for only a very small benefit. >> >> I'm aware of the fact that likely a lot of users are still using >> -enable-kvm, and I did not mean that we should remove it soon yet. But >> IMHO we should start now to inform the users that they should slowly >> switch to the better option "-accel" instead, so that we could maybe >> remove this "-enable-xxx" stuff sometime in the distant future (let's >> say QEMU v4.0?). > > I come from the Linux side, where "breaking a working setup" will result in > an angry Linus. IMHO that's a good approach, but I think it should primarily applied for the interfaces that are designed as "API" to other software layers, i.e. things like QMP and the "-machine" parameter. "-enable-kvm" is in my eyes rather a "syntactic sugar" convenience option, so I'd not apply this rule to this option. > We certainly have not such strict rules here and we could > base the decision on the question "how expensive is the maintenance > of this option?". I think marking it as "legacy option" is fine, but I doubt > that removing it will make qemu maintenance cheaper. Likely not. Actually, I have another point of view in mind here: You have to consider that QEMU has a *lot* of options, and I think this is very confusing for the users, especially the new ones. If we always provide two or three ways to achieve a goal, especially in an inconsistent way like we do it here, we likely rather create frustration than joy for the normal users. Providing a clean, straightforward CLI interface one day could help to improve the user experience quite a bit. > So my preferred variant is > - have it marked in the docs as "legacy" > - no error_report as it might break some setups (since error_report might write > to the monitor) > - never remove the option unless it turns out to be a burden > > But its certainly not my call to make. Paolo, since you're the KVM / main loop maintainer, what's your opinion here? Thomas ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 11:26 ` Thomas Huth @ 2017-05-02 11:59 ` Daniel P. Berrange 2017-05-02 12:07 ` Thomas Huth 0 siblings, 1 reply; 22+ messages in thread From: Daniel P. Berrange @ 2017-05-02 11:59 UTC (permalink / raw) To: Thomas Huth Cc: Christian Borntraeger, qemu-devel, Paolo Bonzini, Vincent Palatin On Tue, May 02, 2017 at 01:26:17PM +0200, Thomas Huth wrote: > On 02.05.2017 12:48, Christian Borntraeger wrote: > > On 05/02/2017 12:37 PM, Thomas Huth wrote: > >> On 02.05.2017 12:32, Christian Borntraeger wrote: > >>> On 05/02/2017 12:06 PM, Thomas Huth wrote: > >>>> The '-enable-...' option do not make too much sense: They do not > >>>> allow additional parameters, using '-accel xxx' is shorter than > >>>> '-enable-xxx' and we're also inconsistent here, since there is > >>>> no '-enable-xen' option available. So let's try to convince the > >>>> users to use '-accel xxx' instead. > >>> > >>> google has 36000 hits for "--enable-kvm" and 18000 hits for "--accel kvm" > >>> So I assume this will affect a lot of setups for only a very small benefit. > >> > >> I'm aware of the fact that likely a lot of users are still using > >> -enable-kvm, and I did not mean that we should remove it soon yet. But > >> IMHO we should start now to inform the users that they should slowly > >> switch to the better option "-accel" instead, so that we could maybe > >> remove this "-enable-xxx" stuff sometime in the distant future (let's > >> say QEMU v4.0?). > > > > I come from the Linux side, where "breaking a working setup" will result in > > an angry Linus. > > IMHO that's a good approach, but I think it should primarily applied for > the interfaces that are designed as "API" to other software layers, i.e. > things like QMP and the "-machine" parameter. > "-enable-kvm" is in my eyes rather a "syntactic sugar" convenience > option, so I'd not apply this rule to this option. > > > We certainly have not such strict rules here and we could > > base the decision on the question "how expensive is the maintenance > > of this option?". I think marking it as "legacy option" is fine, but I doubt > > that removing it will make qemu maintenance cheaper. > > Likely not. Actually, I have another point of view in mind here: You > have to consider that QEMU has a *lot* of options, and I think this is > very confusing for the users, especially the new ones. If we always > provide two or three ways to achieve a goal, especially in an > inconsistent way like we do it here, we likely rather create frustration > than joy for the normal users. Providing a clean, straightforward CLI > interface one day could help to improve the user experience quite a bit. The issue is that we have mutually exclusive requirements here. For a straightforward, easy to understand CLI, things like "--enable-kvm" are much quicker to discover & understand than "-machine accel=kvm". The latter gives much more flexibility since it can set all the other opts, but most of those are rarely used by people who are invoking QEMU manually/directly. We need the things like -machine for libvirt and similar, but they are not end user friendly. Killing all the shortcuts like --enable-kvm would cut down the args we expose, but forcing users onto more complex syntax for args like -machine is not improving their lives in general if they don't need that extra flexibility. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 11:59 ` Daniel P. Berrange @ 2017-05-02 12:07 ` Thomas Huth 2017-05-02 12:14 ` Thomas Huth 2017-05-02 12:16 ` Daniel P. Berrange 0 siblings, 2 replies; 22+ messages in thread From: Thomas Huth @ 2017-05-02 12:07 UTC (permalink / raw) To: Daniel P. Berrange Cc: Christian Borntraeger, Vincent Palatin, qemu-devel, Paolo Bonzini On 02.05.2017 13:59, Daniel P. Berrange wrote: > On Tue, May 02, 2017 at 01:26:17PM +0200, Thomas Huth wrote: >> On 02.05.2017 12:48, Christian Borntraeger wrote: >>> On 05/02/2017 12:37 PM, Thomas Huth wrote: >>>> On 02.05.2017 12:32, Christian Borntraeger wrote: >>>>> On 05/02/2017 12:06 PM, Thomas Huth wrote: >>>>>> The '-enable-...' option do not make too much sense: They do not >>>>>> allow additional parameters, using '-accel xxx' is shorter than >>>>>> '-enable-xxx' and we're also inconsistent here, since there is >>>>>> no '-enable-xen' option available. So let's try to convince the >>>>>> users to use '-accel xxx' instead. >>>>> >>>>> google has 36000 hits for "--enable-kvm" and 18000 hits for "--accel kvm" >>>>> So I assume this will affect a lot of setups for only a very small benefit. >>>> >>>> I'm aware of the fact that likely a lot of users are still using >>>> -enable-kvm, and I did not mean that we should remove it soon yet. But >>>> IMHO we should start now to inform the users that they should slowly >>>> switch to the better option "-accel" instead, so that we could maybe >>>> remove this "-enable-xxx" stuff sometime in the distant future (let's >>>> say QEMU v4.0?). >>> >>> I come from the Linux side, where "breaking a working setup" will result in >>> an angry Linus. >> >> IMHO that's a good approach, but I think it should primarily applied for >> the interfaces that are designed as "API" to other software layers, i.e. >> things like QMP and the "-machine" parameter. >> "-enable-kvm" is in my eyes rather a "syntactic sugar" convenience >> option, so I'd not apply this rule to this option. >> >>> We certainly have not such strict rules here and we could >>> base the decision on the question "how expensive is the maintenance >>> of this option?". I think marking it as "legacy option" is fine, but I doubt >>> that removing it will make qemu maintenance cheaper. >> >> Likely not. Actually, I have another point of view in mind here: You >> have to consider that QEMU has a *lot* of options, and I think this is >> very confusing for the users, especially the new ones. If we always >> provide two or three ways to achieve a goal, especially in an >> inconsistent way like we do it here, we likely rather create frustration >> than joy for the normal users. Providing a clean, straightforward CLI >> interface one day could help to improve the user experience quite a bit. > > The issue is that we have mutually exclusive requirements here. For a > straightforward, easy to understand CLI, things like "--enable-kvm" are > much quicker to discover & understand than "-machine accel=kvm". The > latter gives much more flexibility since it can set all the other opts, > but most of those are rarely used by people who are invoking QEMU > manually/directly. We need the things like -machine for libvirt and > similar, but they are not end user friendly. Killing all the shortcuts > like --enable-kvm would cut down the args we expose, but forcing users > onto more complex syntax for args like -machine is not improving their > lives in general if they don't need that extra flexibility. Theoretically yes, but in this case we also have the "-accel kvm" option which is IMHO also straighforward and easy to understand, and even shorter than "-enable-kvm". If you look at my patch, I did *not* want to force the normal users to use "-machine accel=kvm" here, so I don't see the point of your argument here. Thomas ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 12:07 ` Thomas Huth @ 2017-05-02 12:14 ` Thomas Huth 2017-05-02 12:16 ` Daniel P. Berrange 1 sibling, 0 replies; 22+ messages in thread From: Thomas Huth @ 2017-05-02 12:14 UTC (permalink / raw) To: Daniel P. Berrange Cc: Christian Borntraeger, Vincent Palatin, qemu-devel, Paolo Bonzini On 02.05.2017 14:07, Thomas Huth wrote: > On 02.05.2017 13:59, Daniel P. Berrange wrote: >> On Tue, May 02, 2017 at 01:26:17PM +0200, Thomas Huth wrote: >>> On 02.05.2017 12:48, Christian Borntraeger wrote: >>>> On 05/02/2017 12:37 PM, Thomas Huth wrote: >>>>> On 02.05.2017 12:32, Christian Borntraeger wrote: >>>>>> On 05/02/2017 12:06 PM, Thomas Huth wrote: >>>>>>> The '-enable-...' option do not make too much sense: They do not >>>>>>> allow additional parameters, using '-accel xxx' is shorter than >>>>>>> '-enable-xxx' and we're also inconsistent here, since there is >>>>>>> no '-enable-xen' option available. So let's try to convince the >>>>>>> users to use '-accel xxx' instead. >>>>>> >>>>>> google has 36000 hits for "--enable-kvm" and 18000 hits for "--accel kvm" >>>>>> So I assume this will affect a lot of setups for only a very small benefit. >>>>> >>>>> I'm aware of the fact that likely a lot of users are still using >>>>> -enable-kvm, and I did not mean that we should remove it soon yet. But >>>>> IMHO we should start now to inform the users that they should slowly >>>>> switch to the better option "-accel" instead, so that we could maybe >>>>> remove this "-enable-xxx" stuff sometime in the distant future (let's >>>>> say QEMU v4.0?). >>>> >>>> I come from the Linux side, where "breaking a working setup" will result in >>>> an angry Linus. >>> >>> IMHO that's a good approach, but I think it should primarily applied for >>> the interfaces that are designed as "API" to other software layers, i.e. >>> things like QMP and the "-machine" parameter. >>> "-enable-kvm" is in my eyes rather a "syntactic sugar" convenience >>> option, so I'd not apply this rule to this option. >>> >>>> We certainly have not such strict rules here and we could >>>> base the decision on the question "how expensive is the maintenance >>>> of this option?". I think marking it as "legacy option" is fine, but I doubt >>>> that removing it will make qemu maintenance cheaper. >>> >>> Likely not. Actually, I have another point of view in mind here: You >>> have to consider that QEMU has a *lot* of options, and I think this is >>> very confusing for the users, especially the new ones. If we always >>> provide two or three ways to achieve a goal, especially in an >>> inconsistent way like we do it here, we likely rather create frustration >>> than joy for the normal users. Providing a clean, straightforward CLI >>> interface one day could help to improve the user experience quite a bit. >> >> The issue is that we have mutually exclusive requirements here. For a >> straightforward, easy to understand CLI, things like "--enable-kvm" are >> much quicker to discover & understand than "-machine accel=kvm". The >> latter gives much more flexibility since it can set all the other opts, >> but most of those are rarely used by people who are invoking QEMU >> manually/directly. We need the things like -machine for libvirt and >> similar, but they are not end user friendly. Killing all the shortcuts >> like --enable-kvm would cut down the args we expose, but forcing users >> onto more complex syntax for args like -machine is not improving their >> lives in general if they don't need that extra flexibility. > > Theoretically yes, but in this case we also have the "-accel kvm" option > which is IMHO also straighforward and easy to understand, and even > shorter than "-enable-kvm". If you look at my patch, I did *not* want to > force the normal users to use "-machine accel=kvm" here, so I don't see > the point of your argument here. Apart from that, we never use "-enable-xxx" for any other convenience option, e.g. we do not use "-enable-usb", but just "-usb". So for a real convenience option to enable KVM, the option should have been simply named "-kvm" instead. That "-enable-xxx" stuff is just bad and IMHO does not really fit into the QEMU world. Thomas ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 12:07 ` Thomas Huth 2017-05-02 12:14 ` Thomas Huth @ 2017-05-02 12:16 ` Daniel P. Berrange 2017-05-02 12:38 ` Daniel P. Berrange 1 sibling, 1 reply; 22+ messages in thread From: Daniel P. Berrange @ 2017-05-02 12:16 UTC (permalink / raw) To: Thomas Huth Cc: Christian Borntraeger, Vincent Palatin, qemu-devel, Paolo Bonzini On Tue, May 02, 2017 at 02:07:15PM +0200, Thomas Huth wrote: > On 02.05.2017 13:59, Daniel P. Berrange wrote: > > On Tue, May 02, 2017 at 01:26:17PM +0200, Thomas Huth wrote: > >> On 02.05.2017 12:48, Christian Borntraeger wrote: > >>> On 05/02/2017 12:37 PM, Thomas Huth wrote: > >>>> On 02.05.2017 12:32, Christian Borntraeger wrote: > >>>>> On 05/02/2017 12:06 PM, Thomas Huth wrote: > >>>>>> The '-enable-...' option do not make too much sense: They do not > >>>>>> allow additional parameters, using '-accel xxx' is shorter than > >>>>>> '-enable-xxx' and we're also inconsistent here, since there is > >>>>>> no '-enable-xen' option available. So let's try to convince the > >>>>>> users to use '-accel xxx' instead. > >>>>> > >>>>> google has 36000 hits for "--enable-kvm" and 18000 hits for "--accel kvm" > >>>>> So I assume this will affect a lot of setups for only a very small benefit. > >>>> > >>>> I'm aware of the fact that likely a lot of users are still using > >>>> -enable-kvm, and I did not mean that we should remove it soon yet. But > >>>> IMHO we should start now to inform the users that they should slowly > >>>> switch to the better option "-accel" instead, so that we could maybe > >>>> remove this "-enable-xxx" stuff sometime in the distant future (let's > >>>> say QEMU v4.0?). > >>> > >>> I come from the Linux side, where "breaking a working setup" will result in > >>> an angry Linus. > >> > >> IMHO that's a good approach, but I think it should primarily applied for > >> the interfaces that are designed as "API" to other software layers, i.e. > >> things like QMP and the "-machine" parameter. > >> "-enable-kvm" is in my eyes rather a "syntactic sugar" convenience > >> option, so I'd not apply this rule to this option. > >> > >>> We certainly have not such strict rules here and we could > >>> base the decision on the question "how expensive is the maintenance > >>> of this option?". I think marking it as "legacy option" is fine, but I doubt > >>> that removing it will make qemu maintenance cheaper. > >> > >> Likely not. Actually, I have another point of view in mind here: You > >> have to consider that QEMU has a *lot* of options, and I think this is > >> very confusing for the users, especially the new ones. If we always > >> provide two or three ways to achieve a goal, especially in an > >> inconsistent way like we do it here, we likely rather create frustration > >> than joy for the normal users. Providing a clean, straightforward CLI > >> interface one day could help to improve the user experience quite a bit. > > > > The issue is that we have mutually exclusive requirements here. For a > > straightforward, easy to understand CLI, things like "--enable-kvm" are > > much quicker to discover & understand than "-machine accel=kvm". The > > latter gives much more flexibility since it can set all the other opts, > > but most of those are rarely used by people who are invoking QEMU > > manually/directly. We need the things like -machine for libvirt and > > similar, but they are not end user friendly. Killing all the shortcuts > > like --enable-kvm would cut down the args we expose, but forcing users > > onto more complex syntax for args like -machine is not improving their > > lives in general if they don't need that extra flexibility. > > Theoretically yes, but in this case we also have the "-accel kvm" option > which is IMHO also straighforward and easy to understand, and even > shorter than "-enable-kvm". If you look at my patch, I did *not* want to > force the normal users to use "-machine accel=kvm" here, so I don't see > the point of your argument here. Just looking at the -help output though we have "-machine [type=]name[,prop[=value][,...]] property accel=accel1[:accel2[:...]] selects accelerator supported accelerators are kvm, xen, tcg (default: tcg)" and "-enable-kvm enable KVM full virtualization support" but the "-accel" option is not documented, so essentially doesn't exist, unless you read the man page to see if there are other secret options listed there. So from -help output, "-enable-kvm" looks like the best option to pick from a novice user POV. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 12:16 ` Daniel P. Berrange @ 2017-05-02 12:38 ` Daniel P. Berrange 2017-05-02 12:46 ` Thomas Huth 0 siblings, 1 reply; 22+ messages in thread From: Daniel P. Berrange @ 2017-05-02 12:38 UTC (permalink / raw) To: Thomas Huth Cc: Christian Borntraeger, Vincent Palatin, qemu-devel, Paolo Bonzini On Tue, May 02, 2017 at 01:16:33PM +0100, Daniel P. Berrange wrote: > On Tue, May 02, 2017 at 02:07:15PM +0200, Thomas Huth wrote: > > On 02.05.2017 13:59, Daniel P. Berrange wrote: > > > On Tue, May 02, 2017 at 01:26:17PM +0200, Thomas Huth wrote: > > >> On 02.05.2017 12:48, Christian Borntraeger wrote: > > >>> On 05/02/2017 12:37 PM, Thomas Huth wrote: > > >>>> On 02.05.2017 12:32, Christian Borntraeger wrote: > > >>>>> On 05/02/2017 12:06 PM, Thomas Huth wrote: > > >>>>>> The '-enable-...' option do not make too much sense: They do not > > >>>>>> allow additional parameters, using '-accel xxx' is shorter than > > >>>>>> '-enable-xxx' and we're also inconsistent here, since there is > > >>>>>> no '-enable-xen' option available. So let's try to convince the > > >>>>>> users to use '-accel xxx' instead. > > >>>>> > > >>>>> google has 36000 hits for "--enable-kvm" and 18000 hits for "--accel kvm" > > >>>>> So I assume this will affect a lot of setups for only a very small benefit. > > >>>> > > >>>> I'm aware of the fact that likely a lot of users are still using > > >>>> -enable-kvm, and I did not mean that we should remove it soon yet. But > > >>>> IMHO we should start now to inform the users that they should slowly > > >>>> switch to the better option "-accel" instead, so that we could maybe > > >>>> remove this "-enable-xxx" stuff sometime in the distant future (let's > > >>>> say QEMU v4.0?). > > >>> > > >>> I come from the Linux side, where "breaking a working setup" will result in > > >>> an angry Linus. > > >> > > >> IMHO that's a good approach, but I think it should primarily applied for > > >> the interfaces that are designed as "API" to other software layers, i.e. > > >> things like QMP and the "-machine" parameter. > > >> "-enable-kvm" is in my eyes rather a "syntactic sugar" convenience > > >> option, so I'd not apply this rule to this option. > > >> > > >>> We certainly have not such strict rules here and we could > > >>> base the decision on the question "how expensive is the maintenance > > >>> of this option?". I think marking it as "legacy option" is fine, but I doubt > > >>> that removing it will make qemu maintenance cheaper. > > >> > > >> Likely not. Actually, I have another point of view in mind here: You > > >> have to consider that QEMU has a *lot* of options, and I think this is > > >> very confusing for the users, especially the new ones. If we always > > >> provide two or three ways to achieve a goal, especially in an > > >> inconsistent way like we do it here, we likely rather create frustration > > >> than joy for the normal users. Providing a clean, straightforward CLI > > >> interface one day could help to improve the user experience quite a bit. > > > > > > The issue is that we have mutually exclusive requirements here. For a > > > straightforward, easy to understand CLI, things like "--enable-kvm" are > > > much quicker to discover & understand than "-machine accel=kvm". The > > > latter gives much more flexibility since it can set all the other opts, > > > but most of those are rarely used by people who are invoking QEMU > > > manually/directly. We need the things like -machine for libvirt and > > > similar, but they are not end user friendly. Killing all the shortcuts > > > like --enable-kvm would cut down the args we expose, but forcing users > > > onto more complex syntax for args like -machine is not improving their > > > lives in general if they don't need that extra flexibility. > > > > Theoretically yes, but in this case we also have the "-accel kvm" option > > which is IMHO also straighforward and easy to understand, and even > > shorter than "-enable-kvm". If you look at my patch, I did *not* want to > > force the normal users to use "-machine accel=kvm" here, so I don't see > > the point of your argument here. > > Just looking at the -help output though we have > > "-machine [type=]name[,prop[=value][,...]] > property accel=accel1[:accel2[:...]] selects accelerator > supported accelerators are kvm, xen, tcg (default: tcg)" > > and > > "-enable-kvm enable KVM full virtualization support" > > but the "-accel" option is not documented, so essentially doesn't exist, > unless you read the man page to see if there are other secret options > listed there. So from -help output, "-enable-kvm" looks like the best > option to pick from a novice user POV. Opps, I was looking at wrong QEMU binary - it is listed "-accel [accel=]accelerator[,thread=single|multi] select accelerator ('-accel help for list')" but it doesn't mention KVM, so I still think -enable-kvm is the more obvious right answer based on what we're telling users in help Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 12:38 ` Daniel P. Berrange @ 2017-05-02 12:46 ` Thomas Huth 0 siblings, 0 replies; 22+ messages in thread From: Thomas Huth @ 2017-05-02 12:46 UTC (permalink / raw) To: Daniel P. Berrange Cc: Christian Borntraeger, Vincent Palatin, qemu-devel, Paolo Bonzini On 02.05.2017 14:38, Daniel P. Berrange wrote: > On Tue, May 02, 2017 at 01:16:33PM +0100, Daniel P. Berrange wrote: >> On Tue, May 02, 2017 at 02:07:15PM +0200, Thomas Huth wrote: >>> On 02.05.2017 13:59, Daniel P. Berrange wrote: >>>> On Tue, May 02, 2017 at 01:26:17PM +0200, Thomas Huth wrote: >>>>> On 02.05.2017 12:48, Christian Borntraeger wrote: >>>>>> On 05/02/2017 12:37 PM, Thomas Huth wrote: >>>>>>> On 02.05.2017 12:32, Christian Borntraeger wrote: >>>>>>>> On 05/02/2017 12:06 PM, Thomas Huth wrote: >>>>>>>>> The '-enable-...' option do not make too much sense: They do not >>>>>>>>> allow additional parameters, using '-accel xxx' is shorter than >>>>>>>>> '-enable-xxx' and we're also inconsistent here, since there is >>>>>>>>> no '-enable-xen' option available. So let's try to convince the >>>>>>>>> users to use '-accel xxx' instead. >>>>>>>> >>>>>>>> google has 36000 hits for "--enable-kvm" and 18000 hits for "--accel kvm" >>>>>>>> So I assume this will affect a lot of setups for only a very small benefit. >>>>>>> >>>>>>> I'm aware of the fact that likely a lot of users are still using >>>>>>> -enable-kvm, and I did not mean that we should remove it soon yet. But >>>>>>> IMHO we should start now to inform the users that they should slowly >>>>>>> switch to the better option "-accel" instead, so that we could maybe >>>>>>> remove this "-enable-xxx" stuff sometime in the distant future (let's >>>>>>> say QEMU v4.0?). >>>>>> >>>>>> I come from the Linux side, where "breaking a working setup" will result in >>>>>> an angry Linus. >>>>> >>>>> IMHO that's a good approach, but I think it should primarily applied for >>>>> the interfaces that are designed as "API" to other software layers, i.e. >>>>> things like QMP and the "-machine" parameter. >>>>> "-enable-kvm" is in my eyes rather a "syntactic sugar" convenience >>>>> option, so I'd not apply this rule to this option. >>>>> >>>>>> We certainly have not such strict rules here and we could >>>>>> base the decision on the question "how expensive is the maintenance >>>>>> of this option?". I think marking it as "legacy option" is fine, but I doubt >>>>>> that removing it will make qemu maintenance cheaper. >>>>> >>>>> Likely not. Actually, I have another point of view in mind here: You >>>>> have to consider that QEMU has a *lot* of options, and I think this is >>>>> very confusing for the users, especially the new ones. If we always >>>>> provide two or three ways to achieve a goal, especially in an >>>>> inconsistent way like we do it here, we likely rather create frustration >>>>> than joy for the normal users. Providing a clean, straightforward CLI >>>>> interface one day could help to improve the user experience quite a bit. >>>> >>>> The issue is that we have mutually exclusive requirements here. For a >>>> straightforward, easy to understand CLI, things like "--enable-kvm" are >>>> much quicker to discover & understand than "-machine accel=kvm". The >>>> latter gives much more flexibility since it can set all the other opts, >>>> but most of those are rarely used by people who are invoking QEMU >>>> manually/directly. We need the things like -machine for libvirt and >>>> similar, but they are not end user friendly. Killing all the shortcuts >>>> like --enable-kvm would cut down the args we expose, but forcing users >>>> onto more complex syntax for args like -machine is not improving their >>>> lives in general if they don't need that extra flexibility. >>> >>> Theoretically yes, but in this case we also have the "-accel kvm" option >>> which is IMHO also straighforward and easy to understand, and even >>> shorter than "-enable-kvm". If you look at my patch, I did *not* want to >>> force the normal users to use "-machine accel=kvm" here, so I don't see >>> the point of your argument here. >> >> Just looking at the -help output though we have >> >> "-machine [type=]name[,prop[=value][,...]] >> property accel=accel1[:accel2[:...]] selects accelerator >> supported accelerators are kvm, xen, tcg (default: tcg)" >> >> and >> >> "-enable-kvm enable KVM full virtualization support" >> >> but the "-accel" option is not documented, so essentially doesn't exist, >> unless you read the man page to see if there are other secret options >> listed there. So from -help output, "-enable-kvm" looks like the best >> option to pick from a novice user POV. > > Opps, I was looking at wrong QEMU binary - it is listed > > "-accel [accel=]accelerator[,thread=single|multi] > select accelerator ('-accel help for list')" > > but it doesn't mention KVM, so I still think -enable-kvm is the more obvious > right answer based on what we're telling users in help Oh, right, that parameter has just been introduced with QEMU 2.9.0 ... so maybe it's really too early to spill out deprecation messages for "-enable-kvm", and we just should update the documentation instead (as Christian suggested)... Thomas ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 10:48 ` Christian Borntraeger 2017-05-02 11:26 ` Thomas Huth @ 2017-05-02 13:22 ` Paolo Bonzini 2017-05-02 15:01 ` Markus Armbruster 1 sibling, 1 reply; 22+ messages in thread From: Paolo Bonzini @ 2017-05-02 13:22 UTC (permalink / raw) To: Christian Borntraeger, Thomas Huth, qemu-devel; +Cc: Vincent Palatin On 02/05/2017 12:48, Christian Borntraeger wrote: >> I'm aware of the fact that likely a lot of users are still using >> -enable-kvm, and I did not mean that we should remove it soon yet. But >> IMHO we should start now to inform the users that they should slowly >> switch to the better option "-accel" instead, so that we could maybe >> remove this "-enable-xxx" stuff sometime in the distant future (let's >> say QEMU v4.0?). > > I come from the Linux side, where "breaking a working setup" will result in > an angry Linus. We certainly have not such strict rules here and we could > base the decision on the question "how expensive is the maintenance > of this option?". I think marking it as "legacy option" is fine, but I doubt > that removing it will make qemu maintenance cheaper. So my preferred variant > is Even for Linux the rules aren't 100% black or white. There have been cases where small breakage are introduced, for example 4.12 will break the BLKDISCARDZEROES ioctl. We shouldn't treat things are black or white here too. "-usbdevice" makes sense to deprecate because it brings together a complicated parsing mechanism, though perhaps we could keep it for simple devices such as keyboard, mouse, tablet where it's pretty surely in use in the wild. "-drive cyls=..." also makes sense to deprecate because it is a very obscure functionality. "-drive serial=..." would be nice to deprecate, but I think it's already less clear that it's not in use in the wild. Probably it's still on the side of wanting to eventually remove it. "-net" is more complicated still. It's almost surely in use in the wild, but then probably the users are fairly advanced and (with the provision that documentation should be updated) I guess we could eventually get there. But I really, really see no point in removing --enable-kvm. It costs perhaps 10 lines of code that has pretty much no ramifications elsewhere in the code. If anything I'd expect "-machine accel" to disappear before, if ever (in favor of multiple "-accel" options, e.g. "-machine accel=kvm:tcg" can become "-accel kvm -accel tcg". Thanks, Paolo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 13:22 ` Paolo Bonzini @ 2017-05-02 15:01 ` Markus Armbruster 2017-05-02 15:09 ` Paolo Bonzini 0 siblings, 1 reply; 22+ messages in thread From: Markus Armbruster @ 2017-05-02 15:01 UTC (permalink / raw) To: Paolo Bonzini Cc: Christian Borntraeger, Thomas Huth, qemu-devel, Vincent Palatin Paolo Bonzini <pbonzini@redhat.com> writes: > On 02/05/2017 12:48, Christian Borntraeger wrote: >>> I'm aware of the fact that likely a lot of users are still using >>> -enable-kvm, and I did not mean that we should remove it soon yet. But >>> IMHO we should start now to inform the users that they should slowly >>> switch to the better option "-accel" instead, so that we could maybe >>> remove this "-enable-xxx" stuff sometime in the distant future (let's >>> say QEMU v4.0?). >> >> I come from the Linux side, where "breaking a working setup" will result in >> an angry Linus. We certainly have not such strict rules here and we could >> base the decision on the question "how expensive is the maintenance >> of this option?". I think marking it as "legacy option" is fine, but I doubt >> that removing it will make qemu maintenance cheaper. So my preferred variant >> is > > Even for Linux the rules aren't 100% black or white. There have been > cases where small breakage are introduced, for example 4.12 will break > the BLKDISCARDZEROES ioctl. > > We shouldn't treat things are black or white here too. > > "-usbdevice" makes sense to deprecate because it brings together a > complicated parsing mechanism, though perhaps we could keep it for > simple devices such as keyboard, mouse, tablet where it's pretty surely > in use in the wild. > > "-drive cyls=..." also makes sense to deprecate because it is a very > obscure functionality. > > "-drive serial=..." would be nice to deprecate, but I think it's already > less clear that it's not in use in the wild. Probably it's still on the > side of wanting to eventually remove it. > > "-net" is more complicated still. It's almost surely in use in the > wild, but then probably the users are fairly advanced and (with the > provision that documentation should be updated) I guess we could > eventually get there. > > But I really, really see no point in removing --enable-kvm. It costs > perhaps 10 lines of code that has pretty much no ramifications elsewhere > in the code. If anything I'd expect "-machine accel" to disappear > before, if ever (in favor of multiple "-accel" options, e.g. "-machine > accel=kvm:tcg" can become "-accel kvm -accel tcg". I basically agree, except I wouldn't say "no point", but "doesn't buy us enough to justify inconveniencing users". What we've done before for options that have become unloved, but not harmful, is remove them just from documentation. Grep qemu-options.hx for "HXCOMM Deprecated". Let's do that for --enable-kvm and similarly weird sugared options. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 15:01 ` Markus Armbruster @ 2017-05-02 15:09 ` Paolo Bonzini 2017-05-02 15:53 ` Thomas Huth 0 siblings, 1 reply; 22+ messages in thread From: Paolo Bonzini @ 2017-05-02 15:09 UTC (permalink / raw) To: Markus Armbruster Cc: Christian Borntraeger, Thomas Huth, qemu-devel, Vincent Palatin On 02/05/2017 17:01, Markus Armbruster wrote: >> But I really, really see no point in removing --enable-kvm. It costs >> perhaps 10 lines of code that has pretty much no ramifications elsewhere >> in the code. If anything I'd expect "-machine accel" to disappear >> before, if ever (in favor of multiple "-accel" options, e.g. "-machine >> accel=kvm:tcg" can become "-accel kvm -accel tcg". > I basically agree, except I wouldn't say "no point", but "doesn't buy us > enough to justify inconveniencing users". > > What we've done before for options that have become unloved, but not > harmful, is remove them just from documentation. Grep qemu-options.hx > for "HXCOMM Deprecated". Let's do that for --enable-kvm and similarly > weird sugared options. Except that this provides no easily greppable way to find how to enable KVM, as things stand. So the first lesson should be "no deprecation without documentation" (any reference to 18th century political slogans is purely coincidential :)). Paolo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 15:09 ` Paolo Bonzini @ 2017-05-02 15:53 ` Thomas Huth 2017-05-02 16:08 ` Paolo Bonzini 0 siblings, 1 reply; 22+ messages in thread From: Thomas Huth @ 2017-05-02 15:53 UTC (permalink / raw) To: Paolo Bonzini, Markus Armbruster Cc: Christian Borntraeger, qemu-devel, Vincent Palatin On 02.05.2017 17:09, Paolo Bonzini wrote: > > > On 02/05/2017 17:01, Markus Armbruster wrote: >>> But I really, really see no point in removing --enable-kvm. It costs >>> perhaps 10 lines of code that has pretty much no ramifications elsewhere >>> in the code. If anything I'd expect "-machine accel" to disappear >>> before, if ever (in favor of multiple "-accel" options, e.g. "-machine >>> accel=kvm:tcg" can become "-accel kvm -accel tcg". >> I basically agree, except I wouldn't say "no point", but "doesn't buy us >> enough to justify inconveniencing users". >> >> What we've done before for options that have become unloved, but not >> harmful, is remove them just from documentation. Grep qemu-options.hx >> for "HXCOMM Deprecated". Let's do that for --enable-kvm and similarly >> weird sugared options. > > Except that this provides no easily greppable way to find how to enable > KVM, as things stand. Should we then improve the description of the -accel parameter? > So the first lesson should be "no deprecation > without documentation" (any reference to 18th century political slogans > is purely coincidential :)). Would you accept my patch if I'd remove the hunks for vl.c, i.e. a patch that just updates the documentation? Thomas PS: Thinking about all of this again, I think the thing that bugs me most is that we now have a brand new "-enable-hax" option, too, but no "-enable-xen" option ... that's just inconsistent. What do you think about removing the "-enable-hax" option again - there should only be very few users for this option so far, so the impact should be low. HAX users could simply use the "-accel hax" instead... ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 15:53 ` Thomas Huth @ 2017-05-02 16:08 ` Paolo Bonzini 2017-05-02 16:19 ` Thomas Huth 0 siblings, 1 reply; 22+ messages in thread From: Paolo Bonzini @ 2017-05-02 16:08 UTC (permalink / raw) To: Thomas Huth, Markus Armbruster Cc: Christian Borntraeger, qemu-devel, Vincent Palatin On 02/05/2017 17:53, Thomas Huth wrote: >>> >>> What we've done before for options that have become unloved, but not >>> harmful, is remove them just from documentation. Grep qemu-options.hx >>> for "HXCOMM Deprecated". Let's do that for --enable-kvm and similarly >>> weird sugared options. >> Except that this provides no easily greppable way to find how to enable >> KVM, as things stand. > Should we then improve the description of the -accel parameter? Yes, having a list of accelerators would be useful. For example: -accel tcg[,threads=on|off] .... -accel kvm -accel hax -accel qtest >> So the first lesson should be "no deprecation >> without documentation" (any reference to 18th century political slogans >> is purely coincidential :)). > Would you accept my patch if I'd remove the hunks for vl.c, i.e. a patch > that just updates the documentation? No, updating the documentation is pointless if you don't improve it at the same time. I don't like writing it so often, especially to someone who is a relatively experienced contributor, but the common word in all these reviews is "pointless". What is the *purpose* of this work? I think a valid purpose would be to improve documentation, for example. Another would be to consolidate non-QemuOpts command-line options. For this, some options that are entirely internal provide interesting grounds for improvement. For example, "-qtest" and "qtest-log" could be consolidated in "-qtest [chardev=]chardev,log=file". Paolo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 16:08 ` Paolo Bonzini @ 2017-05-02 16:19 ` Thomas Huth 2017-05-02 16:22 ` Paolo Bonzini 0 siblings, 1 reply; 22+ messages in thread From: Thomas Huth @ 2017-05-02 16:19 UTC (permalink / raw) To: Paolo Bonzini, Markus Armbruster Cc: Christian Borntraeger, qemu-devel, Vincent Palatin On 02.05.2017 18:08, Paolo Bonzini wrote: [...] >>> So the first lesson should be "no deprecation >>> without documentation" (any reference to 18th century political slogans >>> is purely coincidential :)). >> Would you accept my patch if I'd remove the hunks for vl.c, i.e. a patch >> that just updates the documentation? > > No, updating the documentation is pointless if you don't improve it at > the same time. > > I don't like writing it so often, especially to someone who is a > relatively experienced contributor, but the common word in all these > reviews is "pointless". What is the *purpose* of this work? Marking the -enable-xxx options in the documentation as deprecated would maybe at least prevent that we end up with more -enable-mynewcoolaccelerator options in the future, like we did with -enable-hax now. But ok, I got the message, and I'll shut up here now. Sorry for wasting your time. Thomas ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 16:19 ` Thomas Huth @ 2017-05-02 16:22 ` Paolo Bonzini 2017-05-03 8:07 ` Markus Armbruster 0 siblings, 1 reply; 22+ messages in thread From: Paolo Bonzini @ 2017-05-02 16:22 UTC (permalink / raw) To: Thomas Huth, Markus Armbruster Cc: Christian Borntraeger, qemu-devel, Vincent Palatin On 02/05/2017 18:19, Thomas Huth wrote: >> I don't like writing it so often, especially to someone who is a >> relatively experienced contributor, but the common word in all these >> reviews is "pointless". What is the *purpose* of this work? > > Marking the -enable-xxx options in the documentation as deprecated would > maybe at least prevent that we end up with more > -enable-mynewcoolaccelerator options in the future, like we did with > -enable-hax now. That assumes -enable-hax was a mistake. :) > But ok, I got the message, and I'll shut up here now. Sorry for wasting > your time. That absolutely wasn't the intent, sorry. The intent is to make your efforts more useful. Paolo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 16:22 ` Paolo Bonzini @ 2017-05-03 8:07 ` Markus Armbruster 0 siblings, 0 replies; 22+ messages in thread From: Markus Armbruster @ 2017-05-03 8:07 UTC (permalink / raw) To: Paolo Bonzini Cc: Thomas Huth, Christian Borntraeger, Vincent Palatin, qemu-devel Paolo Bonzini <pbonzini@redhat.com> writes: > On 02/05/2017 18:19, Thomas Huth wrote: >>> I don't like writing it so often, especially to someone who is a >>> relatively experienced contributor, but the common word in all these >>> reviews is "pointless". What is the *purpose* of this work? >> >> Marking the -enable-xxx options in the documentation as deprecated would >> maybe at least prevent that we end up with more >> -enable-mynewcoolaccelerator options in the future, like we did with >> -enable-hax now. > > That assumes -enable-hax was a mistake. :) It was, or else -accel was a mistake. >> But ok, I got the message, and I'll shut up here now. Sorry for wasting >> your time. > > That absolutely wasn't the intent, sorry. The intent is to make your > efforts more useful. Seconded. I hope your patches and our discussion lead us towards a saner, more consistent documented public interface. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' 2017-05-02 10:37 ` Thomas Huth 2017-05-02 10:48 ` Christian Borntraeger @ 2017-05-02 12:21 ` Christian Borntraeger 1 sibling, 0 replies; 22+ messages in thread From: Christian Borntraeger @ 2017-05-02 12:21 UTC (permalink / raw) To: Thomas Huth, qemu-devel; +Cc: Paolo Bonzini, Vincent Palatin On 05/02/2017 12:37 PM, Thomas Huth wrote: > On 02.05.2017 12:32, Christian Borntraeger wrote: >> On 05/02/2017 12:06 PM, Thomas Huth wrote: >>> The '-enable-...' option do not make too much sense: They do not >>> allow additional parameters, using '-accel xxx' is shorter than >>> '-enable-xxx' and we're also inconsistent here, since there is >>> no '-enable-xen' option available. So let's try to convince the >>> users to use '-accel xxx' instead. FWIW, -enable-kvm ia not shortcut to make things easier, it predates the accel parameter. Maybe that is the reason why this patch does not feel right to me. >> >> google has 36000 hits for "--enable-kvm" and 18000 hits for "--accel kvm" >> So I assume this will affect a lot of setups for only a very small benefit. > > I'm aware of the fact that likely a lot of users are still using > -enable-kvm, and I did not mean that we should remove it soon yet. But > IMHO we should start now to inform the users that they should slowly > switch to the better option "-accel" instead, so that we could maybe > remove this "-enable-xxx" stuff sometime in the distant future (let's > say QEMU v4.0?). > > Thomas > ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2017-05-03 8:07 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-05-02 10:06 [Qemu-devel] [PATCH] Deprecate '-enable-kvm' and '-enable-hax' in favour of '-accel' Thomas Huth 2017-05-02 10:21 ` Daniel P. Berrange 2017-05-02 10:29 ` Thomas Huth 2017-05-02 10:32 ` Christian Borntraeger 2017-05-02 10:37 ` Thomas Huth 2017-05-02 10:48 ` Christian Borntraeger 2017-05-02 11:26 ` Thomas Huth 2017-05-02 11:59 ` Daniel P. Berrange 2017-05-02 12:07 ` Thomas Huth 2017-05-02 12:14 ` Thomas Huth 2017-05-02 12:16 ` Daniel P. Berrange 2017-05-02 12:38 ` Daniel P. Berrange 2017-05-02 12:46 ` Thomas Huth 2017-05-02 13:22 ` Paolo Bonzini 2017-05-02 15:01 ` Markus Armbruster 2017-05-02 15:09 ` Paolo Bonzini 2017-05-02 15:53 ` Thomas Huth 2017-05-02 16:08 ` Paolo Bonzini 2017-05-02 16:19 ` Thomas Huth 2017-05-02 16:22 ` Paolo Bonzini 2017-05-03 8:07 ` Markus Armbruster 2017-05-02 12:21 ` Christian Borntraeger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).