[PATCH 0/3] target: RFC: display deprecation note for '-cpu help'

[PATCH 0/3] target: RFC: display deprecation note for '-cpu help'

[Daniel P. Berrangé]

When querying '-cpu help' there is no presentation of fact that a
CPU may be deprecated. The user just has to try it and see if they
get a depecation message at runtime.  The QMP command for querying
CPUs report a deprecation bool flag, but not the explanatory
reason.

The Icelake-Client CPU (removed in 6df39f5e583ca0f67bd934d1327f9ead2e3bd49c)
handled this by modifying the '.notes' section to add the word
'deprecated':

            {
                .version = 2,
                .note = "no TSX, deprecated",
                .alias = "Icelake-Client-noTSX",
                .props = (PropValue[]) {
                    { "hle", "off" },
                    { "rtm", "off" },
                    { /* end of list */ }
                },
            },

This relies on the person deprecating the CPU to remember to do this,
and is redundant when this info is already expressed in the
'.deprecation_note' field.

This short series suggests just modifying the '-cpu help'
formatter so that it displays the full deprecation message

eg

$ qemu-system-x86_64 -cpu help:
Available CPUs:
x86 486                   (alias configured by machine type) (deprecated: use at least 'Nehalem' / 'Opteron_G4', or 'host' / 'max')

I wonder if this is too verbose, and we should just do a
concise flag like approach, similar to QMP:

$ qemu-system-x86_64 -cpu help:
Available CPUs:
x86 486                   (alias configured by machine type) (deprecated)

leaving the full message to be displayed at runtime ? I'm slightly
inclined to the simpler more concise output.

This series touched x86_64, s390x, and aarch64 because that's all I
personally needed from a downstream POV, but any & all of the targets
would benefit from this. They have each implemneted the '-cpu help'
logic independantly though, and unifying that code is not entirely
straightforward.

Daniel P. Berrangé (3):
  target/i386: display deprecation note in '-cpu help'
  target/s390x: display deprecation note in '-cpu help'
  target/arm: display deprecation note in '-cpu help'

 target/arm/helper.c       | 10 +++++++++-
 target/i386/cpu.c         | 13 ++++++++++++-
 target/s390x/cpu_models.c | 28 +++++++++++++++++++++++-----
 3 files changed, 44 insertions(+), 7 deletions(-)

-- 
2.36.1

[PATCH 1/3] target/i386: display deprecation note in '-cpu help'

[Daniel P. Berrangé]

The deprecation notes are currently only displayed at runtime when the
user activates a CPU. The QMP query displays a simple flag for
deprecation, while '-cpu help' displays nothing unless the deprecation
info is duplicated into the 'notes' field.

This changes the code so that deprecation notes are explicitly shown
in '-cpu help', to assist the user in deciding what to use.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 target/i386/cpu.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 6a57ef13af..a2c5dcfc04 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4833,10 +4833,21 @@ static void x86_cpu_list_entry(gpointer data, gpointer user_data)
     if (!desc && cc->model && cc->model->note) {
         desc = g_strdup_printf("%s [%s]", model_id, cc->model->note);
     }
-    if (!desc) {
+    if (!desc && *model_id) {
         desc = g_strdup_printf("%s", model_id);
     }
 
+    if (cc->model && cc->model->cpudef->deprecation_note) {
+        g_autofree char *dep = g_strdup_printf(
+            "(deprecated: %s)", cc->model->cpudef->deprecation_note);
+        if (desc) {
+            g_autofree char *olddesc = desc;
+            desc = g_strdup_printf("%s %s", olddesc, dep);
+        } else {
+            desc = g_steal_pointer(&dep);
+        }
+    }
+
     qemu_printf("x86 %-20s  %s\n", name, desc);
 }
 
-- 
2.36.1

Re: [PATCH 1/3] target/i386: display deprecation note in '-cpu help'

[Cornelia Huck]

On Thu, Jul 14 2022, Daniel P. Berrangé <berrange@redhat.com> wrote:

> The deprecation notes are currently only displayed at runtime when the
> user activates a CPU. The QMP query displays a simple flag for
> deprecation, while '-cpu help' displays nothing unless the deprecation
> info is duplicated into the 'notes' field.
>
> This changes the code so that deprecation notes are explicitly shown
> in '-cpu help', to assist the user in deciding what to use.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  target/i386/cpu.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

[PATCH 2/3] target/s390x: display deprecation note in '-cpu help'

[Daniel P. Berrangé]

The deprecation notes are currently only displayed at runtime when the
user activates a CPU. The QMP query displays a simple flag for
deprecation, while '-cpu help' displays nothing unless the deprecation
info is duplicated into the 'notes' field.

This changes the code so that deprecation notes are explicitly shown
in '-cpu help', to assist the user in deciding what to use.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 target/s390x/cpu_models.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 1a562d2801..a04375896d 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -334,18 +334,36 @@ const S390CPUDef *s390_find_cpu_def(uint16_t type, uint8_t gen, uint8_t ec_ga,
 static void s390_print_cpu_model_list_entry(gpointer data, gpointer user_data)
 {
     const S390CPUClass *scc = S390_CPU_CLASS((ObjectClass *)data);
+    CPUClass *cc = CPU_CLASS(scc);
     char *name = g_strdup(object_class_get_name((ObjectClass *)data));
-    const char *details = "";
+    g_autoptr(GString) details = g_string_new("(");
 
     if (scc->is_static) {
-        details = "(static, migration-safe)";
-    } else if (scc->is_migration_safe) {
-        details = "(migration-safe)";
+        g_string_append(details, "static");
+    }
+    if (scc->is_migration_safe) {
+        if (details->len > 1) {
+            g_string_append(details, ", ");
+        }
+        g_string_append(details, "migration-safe");
+    }
+    if (cc->deprecation_note) {
+        if (details->len > 1) {
+            g_string_append(details, ", ");
+        }
+        g_string_append(details, "deprecated: ");
+        g_string_append(details, cc->deprecation_note);
+    }
+    if (details->len > 1) {
+        g_string_append(details, ")");
+    }
+    if (details->len == 1) {
+        g_string_truncate(details, 0);
     }
 
     /* strip off the -s390x-cpu */
     g_strrstr(name, "-" TYPE_S390_CPU)[0] = 0;
-    qemu_printf("s390 %-15s %-35s %s\n", name, scc->desc, details);
+    qemu_printf("s390 %-15s %-35s %s\n", name, scc->desc, details->str);
     g_free(name);
 }
 
-- 
2.36.1

Re: [PATCH 2/3] target/s390x: display deprecation note in '-cpu help'

[Cornelia Huck]

On Thu, Jul 14 2022, Daniel P. Berrangé <berrange@redhat.com> wrote:

> The deprecation notes are currently only displayed at runtime when the
> user activates a CPU. The QMP query displays a simple flag for
> deprecation, while '-cpu help' displays nothing unless the deprecation
> info is duplicated into the 'notes' field.
>
> This changes the code so that deprecation notes are explicitly shown
> in '-cpu help', to assist the user in deciding what to use.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  target/s390x/cpu_models.c | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

[PATCH 3/3] target/arm: display deprecation note in '-cpu help'

[Daniel P. Berrangé]

The deprecation notes are currently only displayed at runtime when the
user activates a CPU. The QMP query displays a simple flag for
deprecation, while '-cpu help' displays nothing unless the deprecation
info is duplicated into the 'notes' field.

This changes the code so that deprecation notes are explicitly shown
in '-cpu help', to assist the user in deciding what to use.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 target/arm/helper.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index cfcad97ce0..1a0988d8fc 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8222,12 +8222,20 @@ static gint arm_cpu_list_compare(gconstpointer a, gconstpointer b)
 static void arm_cpu_list_entry(gpointer data, gpointer user_data)
 {
     ObjectClass *oc = data;
+    CPUClass *cc = CPU_CLASS(oc);
     const char *typename;
     char *name;
+    g_autofree char *details = NULL;
+
+    if (cc->deprecation_note) {
+        details = g_strdup_printf(" (deprecated: %s)", cc->deprecation_note);
+    } else {
+        details = g_strdup("");
+    }
 
     typename = object_class_get_name(oc);
     name = g_strndup(typename, strlen(typename) - strlen("-" TYPE_ARM_CPU));
-    qemu_printf("  %s\n", name);
+    qemu_printf("  %s%s\n", name, details);
     g_free(name);
 }
 
-- 
2.36.1

Re: [PATCH 3/3] target/arm: display deprecation note in '-cpu help'

[Cornelia Huck]

On Thu, Jul 14 2022, Daniel P. Berrangé <berrange@redhat.com> wrote:

> The deprecation notes are currently only displayed at runtime when the
> user activates a CPU. The QMP query displays a simple flag for
> deprecation, while '-cpu help' displays nothing unless the deprecation
> info is duplicated into the 'notes' field.
>
> This changes the code so that deprecation notes are explicitly shown
> in '-cpu help', to assist the user in deciding what to use.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  target/arm/helper.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH 0/3] target: RFC: display deprecation note for '-cpu help'

[Cornelia Huck]

On Thu, Jul 14 2022, Daniel P. Berrangé <berrange@redhat.com> wrote:

> When querying '-cpu help' there is no presentation of fact that a
> CPU may be deprecated. The user just has to try it and see if they
> get a depecation message at runtime.  The QMP command for querying
> CPUs report a deprecation bool flag, but not the explanatory
> reason.
>
> The Icelake-Client CPU (removed in 6df39f5e583ca0f67bd934d1327f9ead2e3bd49c)
> handled this by modifying the '.notes' section to add the word
> 'deprecated':
>
>             {
>                 .version = 2,
>                 .note = "no TSX, deprecated",
>                 .alias = "Icelake-Client-noTSX",
>                 .props = (PropValue[]) {
>                     { "hle", "off" },
>                     { "rtm", "off" },
>                     { /* end of list */ }
>                 },
>             },
>
> This relies on the person deprecating the CPU to remember to do this,
> and is redundant when this info is already expressed in the
> '.deprecation_note' field.
>
> This short series suggests just modifying the '-cpu help'
> formatter so that it displays the full deprecation message
>
> eg
>
> $ qemu-system-x86_64 -cpu help:
> Available CPUs:
> x86 486                   (alias configured by machine type) (deprecated: use at least 'Nehalem' / 'Opteron_G4', or 'host' / 'max')
>
> I wonder if this is too verbose, and we should just do a
> concise flag like approach, similar to QMP:
>
> $ qemu-system-x86_64 -cpu help:
> Available CPUs:
> x86 486                   (alias configured by machine type) (deprecated)
>
> leaving the full message to be displayed at runtime ? I'm slightly
> inclined to the simpler more concise output.

The good thing about the longer output is that the user gets the full
information right from the start, and does not need to dig around and
figure out why it is deprecated, and what to use instead. That said, if
we have very verbose deprecation notes, the output may get a bit
cluttered. I think I slightly prefer the verbose output.

>
> This series touched x86_64, s390x, and aarch64 because that's all I
> personally needed from a downstream POV, but any & all of the targets
> would benefit from this. They have each implemneted the '-cpu help'
> logic independantly though, and unifying that code is not entirely
> straightforward.

It seems that any arch that does not use a very simple output has chosen
a different format...

Re: [PATCH 0/3] target: RFC: display deprecation note for '-cpu help'

[Thomas Huth]

On 14/07/2022 17.07, Daniel P. Berrangé wrote:
> When querying '-cpu help' there is no presentation of fact that a
> CPU may be deprecated. The user just has to try it and see if they
> get a depecation message at runtime.  The QMP command for querying
> CPUs report a deprecation bool flag, but not the explanatory
> reason.
> 
> The Icelake-Client CPU (removed in 6df39f5e583ca0f67bd934d1327f9ead2e3bd49c)
> handled this by modifying the '.notes' section to add the word
> 'deprecated':
> 
>              {
>                  .version = 2,
>                  .note = "no TSX, deprecated",
>                  .alias = "Icelake-Client-noTSX",
>                  .props = (PropValue[]) {
>                      { "hle", "off" },
>                      { "rtm", "off" },
>                      { /* end of list */ }
>                  },
>              },
> 
> This relies on the person deprecating the CPU to remember to do this,
> and is redundant when this info is already expressed in the
> '.deprecation_note' field.
> 
> This short series suggests just modifying the '-cpu help'
> formatter so that it displays the full deprecation message
> 
> eg
> 
> $ qemu-system-x86_64 -cpu help:
> Available CPUs:
> x86 486                   (alias configured by machine type) (deprecated: use at least 'Nehalem' / 'Opteron_G4', or 'host' / 'max')
> 
> I wonder if this is too verbose, and we should just do a
> concise flag like approach, similar to QMP:
> 
> $ qemu-system-x86_64 -cpu help:
> Available CPUs:
> x86 486                   (alias configured by machine type) (deprecated)
> 
> leaving the full message to be displayed at runtime ? I'm slightly
> inclined to the simpler more concise output.

I'd prefer to keep it short here and just write "deprecated" without the 
reason. Otherwise this will overflow the lines and break the readability of 
the output. And it's also what we're also doing for "-machine", e.g.:

$ ./qemu-system-ppc64 -M help | grep deprecate
taihu                taihu (deprecated)
$ ./qemu-system-ppc64 -M taihu
qemu-system-ppc64: warning: Machine type 'taihu' is deprecated: incomplete, 
use 'ref405ep' instead

  Thomas

Re: [PATCH 0/3] target: RFC: display deprecation note for '-cpu help'

[Cornelia Huck]

On Mon, Jul 18 2022, Thomas Huth <thuth@redhat.com> wrote:

> On 14/07/2022 17.07, Daniel P. Berrangé wrote:
>> $ qemu-system-x86_64 -cpu help:
>> Available CPUs:
>> x86 486                   (alias configured by machine type) (deprecated: use at least 'Nehalem' / 'Opteron_G4', or 'host' / 'max')
>> 
>> I wonder if this is too verbose, and we should just do a
>> concise flag like approach, similar to QMP:
>> 
>> $ qemu-system-x86_64 -cpu help:
>> Available CPUs:
>> x86 486                   (alias configured by machine type) (deprecated)
>> 
>> leaving the full message to be displayed at runtime ? I'm slightly
>> inclined to the simpler more concise output.
>
> I'd prefer to keep it short here and just write "deprecated" without the 
> reason. Otherwise this will overflow the lines and break the readability of 
> the output. And it's also what we're also doing for "-machine", e.g.:
>
> $ ./qemu-system-ppc64 -M help | grep deprecate
> taihu                taihu (deprecated)
> $ ./qemu-system-ppc64 -M taihu
> qemu-system-ppc64: warning: Machine type 'taihu' is deprecated: incomplete, 
> use 'ref405ep' instead

Ok, following what -machine does is certainly a good point.

Is it easy enough the figure out the deprecation note? I think you
either have to actually start something with the deprecated entity, or
use qmp (which is not that straightforward)?

Re: [PATCH 0/3] target: RFC: display deprecation note for '-cpu help'

[Daniel P. Berrangé]

On Mon, Jul 18, 2022 at 11:37:35AM +0200, Cornelia Huck wrote:
> On Mon, Jul 18 2022, Thomas Huth <thuth@redhat.com> wrote:
> 
> > On 14/07/2022 17.07, Daniel P. Berrangé wrote:
> >> $ qemu-system-x86_64 -cpu help:
> >> Available CPUs:
> >> x86 486                   (alias configured by machine type) (deprecated: use at least 'Nehalem' / 'Opteron_G4', or 'host' / 'max')
> >> 
> >> I wonder if this is too verbose, and we should just do a
> >> concise flag like approach, similar to QMP:
> >> 
> >> $ qemu-system-x86_64 -cpu help:
> >> Available CPUs:
> >> x86 486                   (alias configured by machine type) (deprecated)
> >> 
> >> leaving the full message to be displayed at runtime ? I'm slightly
> >> inclined to the simpler more concise output.
> >
> > I'd prefer to keep it short here and just write "deprecated" without the 
> > reason. Otherwise this will overflow the lines and break the readability of 
> > the output. And it's also what we're also doing for "-machine", e.g.:
> >
> > $ ./qemu-system-ppc64 -M help | grep deprecate
> > taihu                taihu (deprecated)
> > $ ./qemu-system-ppc64 -M taihu
> > qemu-system-ppc64: warning: Machine type 'taihu' is deprecated: incomplete, 
> > use 'ref405ep' instead
> 
> Ok, following what -machine does is certainly a good point.

Yes, I should have thought to check what -machine does, it makese
sense to be consistent.

> Is it easy enough the figure out the deprecation note? I think you
> either have to actually start something with the deprecated entity, or
> use qmp (which is not that straightforward)?

QMP doesn't tell you the note, just a boolean deprecation flag. It is
only printed on startup only right now.

In the context of libvirt what happens is that libvirt can report that
something is deprecated (based on the QMP response). If you go ahead
and use it anyway, you'll get the deprecation message in the logfile
for the VM, and the VM gets marked tainted by libvirt, which serves
as a guide to look in the logfile.

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

Re: [PATCH 0/3] target: RFC: display deprecation note for '-cpu help'

[Cornelia Huck]

On Mon, Jul 18 2022, Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Mon, Jul 18, 2022 at 11:37:35AM +0200, Cornelia Huck wrote:
>> Is it easy enough the figure out the deprecation note? I think you
>> either have to actually start something with the deprecated entity, or
>> use qmp (which is not that straightforward)?
>
> QMP doesn't tell you the note, just a boolean deprecation flag. It is
> only printed on startup only right now.
>
> In the context of libvirt what happens is that libvirt can report that
> something is deprecated (based on the QMP response). If you go ahead
> and use it anyway, you'll get the deprecation message in the logfile
> for the VM, and the VM gets marked tainted by libvirt, which serves
> as a guide to look in the logfile.

Hm... so, a user who notes via -help that 'foo' is deprecated does not
really have a good way to figure out what they should use instead, other
than actually trying to use 'foo'? Is that a use case worth spending
some effort on, or do we consider it more of a niche case?