The ordering semantics of +feature/-feature is tricky and not
obvious, and it requires a custom option parser.  Deprecate that
syntax so we can eventually remove the custom -cpu option parser
and plus_features/minus_features global variables in i386.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 docs/system/deprecated.rst | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index e20bfcb17a4..2c4b8d4b78b 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -127,6 +127,20 @@ Drives with interface types other than ``if=none`` are for onboard
 devices.  It is possible to use drives the board doesn't pick up with
 -device.  This usage is now deprecated.  Use ``if=none`` instead.
 
+``-cpu`` ``+feature`` and ``-feature`` syntax (since 6.0.0)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+The ``-cpu ...,+feature`` and ``-cpu ...,-feature`` syntax for
+enabling and disabling CPU features is deprecated.  The ``-cpu
+...,feature=on`` or ``-cpu ...,feature=off`` should be used
+instead.
+
+Note that the ordering semantics of ``-cpu ...,-feature,+feature``
+is different from ``-cpu ...,feature=off,feature=on``.  With the
+former, the feature got disabled because ``-feature`` had
+precedence, but with the latter the feature will be enabled
+because options are applied in the order they appear.
+
 
 QEMU Machine Protocol (QMP) commands
 ------------------------------------
-- 
2.28.0

On Wednesday, 2021-01-20 at 15:12:41 -05, Eduardo Habkost wrote:

> The ordering semantics of +feature/-feature is tricky and not
> obvious, and it requires a custom option parser.  Deprecate that
> syntax so we can eventually remove the custom -cpu option parser
> and plus_features/minus_features global variables in i386.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

With very minor wording comment...

Reviewed-by: David Edmondson <david.edmondson@oracle.com>

> ---
>  docs/system/deprecated.rst | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index e20bfcb17a4..2c4b8d4b78b 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -127,6 +127,20 @@ Drives with interface types other than ``if=none`` are for onboard
>  devices.  It is possible to use drives the board doesn't pick up with
>  -device.  This usage is now deprecated.  Use ``if=none`` instead.
>  
> +``-cpu`` ``+feature`` and ``-feature`` syntax (since 6.0.0)
> +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +The ``-cpu ...,+feature`` and ``-cpu ...,-feature`` syntax for
> +enabling and disabling CPU features is deprecated.  The ``-cpu
> +...,feature=on`` or ``-cpu ...,feature=off`` should be used
> +instead.
> +
> +Note that the ordering semantics of ``-cpu ...,-feature,+feature``
> +is different from ``-cpu ...,feature=off,feature=on``.  With the
> +former, the feature got disabled because ``-feature`` had

s/got/was/

> +precedence, but with the latter the feature will be enabled
> +because options are applied in the order they appear.
> +
>  
>  QEMU Machine Protocol (QMP) commands
>  ------------------------------------
> -- 
> 2.28.0

dme.
-- 
I got a girlfriend that's better than that.

On Wed, Jan 20, 2021 at 03:12:41PM -0500, Eduardo Habkost wrote:
> The ordering semantics of +feature/-feature is tricky and not
> obvious, and it requires a custom option parser.  Deprecate that
> syntax so we can eventually remove the custom -cpu option parser
> and plus_features/minus_features global variables in i386.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  docs/system/deprecated.rst | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

Ideally we would also print a warning on stderr when this deprecated
style is used.

> 
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index e20bfcb17a4..2c4b8d4b78b 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -127,6 +127,20 @@ Drives with interface types other than ``if=none`` are for onboard
>  devices.  It is possible to use drives the board doesn't pick up with
>  -device.  This usage is now deprecated.  Use ``if=none`` instead.
>  
> +``-cpu`` ``+feature`` and ``-feature`` syntax (since 6.0.0)
> +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +The ``-cpu ...,+feature`` and ``-cpu ...,-feature`` syntax for
> +enabling and disabling CPU features is deprecated.  The ``-cpu
> +...,feature=on`` or ``-cpu ...,feature=off`` should be used
> +instead.
> +
> +Note that the ordering semantics of ``-cpu ...,-feature,+feature``
> +is different from ``-cpu ...,feature=off,feature=on``.  With the
> +former, the feature got disabled because ``-feature`` had
> +precedence, but with the latter the feature will be enabled
> +because options are applied in the order they appear.
> +

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 :|

On 1/21/21 4:39 AM, Daniel P. Berrangé wrote:
> On Wed, Jan 20, 2021 at 03:12:41PM -0500, Eduardo Habkost wrote:
>> The ordering semantics of +feature/-feature is tricky and not
>> obvious, and it requires a custom option parser.  Deprecate that
>> syntax so we can eventually remove the custom -cpu option parser
>> and plus_features/minus_features global variables in i386.
>>
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> ---
>>   docs/system/deprecated.rst | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
> 
> Ideally we would also print a warning on stderr when this deprecated
> style is used.
> 

+1.

It might break some tests to do that, though, but if it's not too 
gruesome it's probably worth it.

On Wed, 20 Jan 2021 15:12:41 -0500
Eduardo Habkost <ehabkost@redhat.com> wrote:

> The ordering semantics of +feature/-feature is tricky and not
> obvious, and it requires a custom option parser.  Deprecate that
> syntax so we can eventually remove the custom -cpu option parser
> and plus_features/minus_features global variables in i386.
it affects spark as well

with that

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  docs/system/deprecated.rst | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index e20bfcb17a4..2c4b8d4b78b 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -127,6 +127,20 @@ Drives with interface types other than ``if=none`` are for onboard
>  devices.  It is possible to use drives the board doesn't pick up with
>  -device.  This usage is now deprecated.  Use ``if=none`` instead.
>  
> +``-cpu`` ``+feature`` and ``-feature`` syntax (since 6.0.0)
> +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +The ``-cpu ...,+feature`` and ``-cpu ...,-feature`` syntax for
> +enabling and disabling CPU features is deprecated.  The ``-cpu
> +...,feature=on`` or ``-cpu ...,feature=off`` should be used
> +instead.
> +
> +Note that the ordering semantics of ``-cpu ...,-feature,+feature``
> +is different from ``-cpu ...,feature=off,feature=on``.  With the
> +former, the feature got disabled because ``-feature`` had
> +precedence, but with the latter the feature will be enabled
> +because options are applied in the order they appear.
> +
>  
>  QEMU Machine Protocol (QMP) commands
>  ------------------------------------