[PATCH v2] target/s390x: improve cpu compatibility check error message

[PATCH v2] target/s390x: improve cpu compatibility check error message

[Claudio Fontana]

some users were confused by this message showing under TCG:

Selected CPU generation is too new. Maximum supported model
in the configuration: 'xyz'

Clarify that the maximum can depend on the accel, and add a
hint to try a different one.

Also add a hint for features mismatch to suggest trying
different accel, QEMU and kernel versions.

Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
 target/s390x/cpu_models.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 1a1c096122..8ed3bb6a27 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -500,6 +500,16 @@ static void error_prepend_missing_feat(const char *name, void *opaque)
     error_prepend((Error **) opaque, "%s ", name);
 }
 
+static void check_compat_model_failed(Error **errp,
+                                      const S390CPUModel *max_model,
+                                      const char *msg)
+{
+    error_setg(errp, "%s. Maximum supported model in the current configuration: \'%s\'",
+               msg, max_model->def->name);
+    error_append_hint(errp, "Consider a different accelerator, try \"-accel help\"\n");
+    return;
+}
+
 static void check_compatibility(const S390CPUModel *max_model,
                                 const S390CPUModel *model, Error **errp)
 {
@@ -507,15 +517,11 @@ static void check_compatibility(const S390CPUModel *max_model,
     S390FeatBitmap missing;
 
     if (model->def->gen > max_model->def->gen) {
-        error_setg(errp, "Selected CPU generation is too new. Maximum "
-                   "supported model in the configuration: \'%s\'",
-                   max_model->def->name);
+        check_compat_model_failed(errp, max_model, "Selected CPU generation is too new");
         return;
     } else if (model->def->gen == max_model->def->gen &&
                model->def->ec_ga > max_model->def->ec_ga) {
-        error_setg(errp, "Selected CPU GA level is too new. Maximum "
-                   "supported model in the configuration: \'%s\'",
-                   max_model->def->name);
+        check_compat_model_failed(errp, max_model, "Selected CPU GA level is too new");
         return;
     }
 
@@ -537,7 +543,9 @@ static void check_compatibility(const S390CPUModel *max_model,
     error_setg(errp, " ");
     s390_feat_bitmap_to_ascii(missing, errp, error_prepend_missing_feat);
     error_prepend(errp, "Some features requested in the CPU model are not "
-                  "available in the configuration: ");
+                  "available in the current configuration: ");
+    error_append_hint(errp,
+                      "Consider a different accelerator, QEMU, or kernel version\n");
 }
 
 S390CPUModel *get_max_cpu_model(Error **errp)
-- 
2.26.2

Re: [PATCH v2] target/s390x: improve cpu compatibility check error message

[Philippe Mathieu-Daudé]

On 14/3/24 22:37, Claudio Fontana wrote:
> some users were confused by this message showing under TCG:
> 
> Selected CPU generation is too new. Maximum supported model
> in the configuration: 'xyz'

(Note for the maintainer queuing this patch, consider adding
  few extra spaces to indent the previously displayed output).

> Clarify that the maximum can depend on the accel, and add a
> hint to try a different one.
> 
> Also add a hint for features mismatch to suggest trying
> different accel, QEMU and kernel versions.
> 
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> ---
>   target/s390x/cpu_models.c | 22 +++++++++++++++-------
>   1 file changed, 15 insertions(+), 7 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v2] target/s390x: improve cpu compatibility check error message

[Nina Schoetterl-Glausch]

On Thu, 2024-03-14 at 22:37 +0100, Claudio Fontana wrote:
> some users were confused by this message showing under TCG:
> 
> Selected CPU generation is too new. Maximum supported model
> in the configuration: 'xyz'
> 
> Clarify that the maximum can depend on the accel, and add a
> hint to try a different one.
> 
> Also add a hint for features mismatch to suggest trying
> different accel, QEMU and kernel versions.
> 
> Signed-off-by: Claudio Fontana <cfontana@suse.de>

Reviewed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>

> ---
>  target/s390x/cpu_models.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 1a1c096122..8ed3bb6a27 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -500,6 +500,16 @@ static void error_prepend_missing_feat(const char *name, void *opaque)
>      error_prepend((Error **) opaque, "%s ", name);
>  }
>  
> +static void check_compat_model_failed(Error **errp,
> +                                      const S390CPUModel *max_model,
> +                                      const char *msg)
> +{
> +    error_setg(errp, "%s. Maximum supported model in the current configuration: \'%s\'",
> +               msg, max_model->def->name);
> +    error_append_hint(errp, "Consider a different accelerator, try \"-accel help\"\n");
> +    return;
> +}
> +
>  static void check_compatibility(const S390CPUModel *max_model,
>                                  const S390CPUModel *model, Error **errp)
>  {
> @@ -507,15 +517,11 @@ static void check_compatibility(const S390CPUModel *max_model,
>      S390FeatBitmap missing;
>  
>      if (model->def->gen > max_model->def->gen) {
> -        error_setg(errp, "Selected CPU generation is too new. Maximum "
> -                   "supported model in the configuration: \'%s\'",
> -                   max_model->def->name);
> +        check_compat_model_failed(errp, max_model, "Selected CPU generation is too new");
>          return;
>      } else if (model->def->gen == max_model->def->gen &&
>                 model->def->ec_ga > max_model->def->ec_ga) {
> -        error_setg(errp, "Selected CPU GA level is too new. Maximum "
> -                   "supported model in the configuration: \'%s\'",
> -                   max_model->def->name);
> +        check_compat_model_failed(errp, max_model, "Selected CPU GA level is too new");
>          return;
>      }
>  
> @@ -537,7 +543,9 @@ static void check_compatibility(const S390CPUModel *max_model,
>      error_setg(errp, " ");
>      s390_feat_bitmap_to_ascii(missing, errp, error_prepend_missing_feat);
>      error_prepend(errp, "Some features requested in the CPU model are not "
> -                  "available in the configuration: ");
> +                  "available in the current configuration: ");
> +    error_append_hint(errp,
> +                      "Consider a different accelerator, QEMU, or kernel version\n");

If I'm reading the regex right, this ^, a string literal on a separate line, is excluded
from the line length check.

>  }
>  
>  S390CPUModel *get_max_cpu_model(Error **errp)

Re: [PATCH v2] target/s390x: improve cpu compatibility check error message

[Claudio Fontana]

On 3/15/24 17:59, Nina Schoetterl-Glausch wrote:
> On Thu, 2024-03-14 at 22:37 +0100, Claudio Fontana wrote:
>> some users were confused by this message showing under TCG:
>>
>> Selected CPU generation is too new. Maximum supported model
>> in the configuration: 'xyz'
>>
>> Clarify that the maximum can depend on the accel, and add a
>> hint to try a different one.
>>
>> Also add a hint for features mismatch to suggest trying
>> different accel, QEMU and kernel versions.
>>
>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> 
> Reviewed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> 
>> ---
>>  target/s390x/cpu_models.c | 22 +++++++++++++++-------
>>  1 file changed, 15 insertions(+), 7 deletions(-)
>>
>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>> index 1a1c096122..8ed3bb6a27 100644
>> --- a/target/s390x/cpu_models.c
>> +++ b/target/s390x/cpu_models.c
>> @@ -500,6 +500,16 @@ static void error_prepend_missing_feat(const char *name, void *opaque)
>>      error_prepend((Error **) opaque, "%s ", name);
>>  }
>>  
>> +static void check_compat_model_failed(Error **errp,
>> +                                      const S390CPUModel *max_model,
>> +                                      const char *msg)
>> +{
>> +    error_setg(errp, "%s. Maximum supported model in the current configuration: \'%s\'",
>> +               msg, max_model->def->name);
>> +    error_append_hint(errp, "Consider a different accelerator, try \"-accel help\"\n");
>> +    return;
>> +}
>> +
>>  static void check_compatibility(const S390CPUModel *max_model,
>>                                  const S390CPUModel *model, Error **errp)
>>  {
>> @@ -507,15 +517,11 @@ static void check_compatibility(const S390CPUModel *max_model,
>>      S390FeatBitmap missing;
>>  
>>      if (model->def->gen > max_model->def->gen) {
>> -        error_setg(errp, "Selected CPU generation is too new. Maximum "
>> -                   "supported model in the configuration: \'%s\'",
>> -                   max_model->def->name);
>> +        check_compat_model_failed(errp, max_model, "Selected CPU generation is too new");
>>          return;
>>      } else if (model->def->gen == max_model->def->gen &&
>>                 model->def->ec_ga > max_model->def->ec_ga) {
>> -        error_setg(errp, "Selected CPU GA level is too new. Maximum "
>> -                   "supported model in the configuration: \'%s\'",
>> -                   max_model->def->name);
>> +        check_compat_model_failed(errp, max_model, "Selected CPU GA level is too new");
>>          return;
>>      }
>>  
>> @@ -537,7 +543,9 @@ static void check_compatibility(const S390CPUModel *max_model,
>>      error_setg(errp, " ");
>>      s390_feat_bitmap_to_ascii(missing, errp, error_prepend_missing_feat);
>>      error_prepend(errp, "Some features requested in the CPU model are not "
>> -                  "available in the configuration: ");
>> +                  "available in the current configuration: ");
>> +    error_append_hint(errp,
>> +                      "Consider a different accelerator, QEMU, or kernel version\n");
> 
> If I'm reading the regex right, this ^, a string literal on a separate line, is excluded
> from the line length check.

Thanks, it's very good to know!

Claudio
> 
>>  }
>>  
>>  S390CPUModel *get_max_cpu_model(Error **errp)
>