[Qemu-devel] [PATCH] apic: Make APIC ID limit error message clearer

[Qemu-devel] [PATCH] apic: Make APIC ID limit error message clearer

[Eduardo Habkost]

Remove the "apic initialization failed" prefix (it conveys no
useful information), replace "invalid" with "too large", and add
an error hint with two possible solutions for the problem.

Before:

  $ qemu-system-x86_64 -machine q35 -smp 256
  qemu-system-x86_64: apic initialization failed. APIC ID 255 is invalid

After:

  $ qemu-system-x86_64 -machine q35 -smp 256 -display none
  qemu-system-x86_64: APIC ID 255 is too large
  Possible solutions:
  * Lowering the number of VCPUs on the -smp option
  * Using accel=kvm,kernel-irqchip=on or accel=kvm,kernel-irqchip=split

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
I'm not sure this is the best way to provide usage hints to the
user.  Any suggestions?
---
 hw/intc/apic.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 97ffdd820f..f08006334d 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -886,8 +886,11 @@ static void apic_realize(DeviceState *dev, Error **errp)
     APICCommonState *s = APIC(dev);
 
     if (s->id >= MAX_APICS) {
-        error_setg(errp, "%s initialization failed. APIC ID %d is invalid",
-                   object_get_typename(OBJECT(dev)), s->id);
+        error_setg(errp, "APIC ID %d is too large", s->id);
+        error_append_hint(errp,
+            "Possible solutions:\n"
+            "* Lowering the number of VCPUs on the -smp option\n"
+            "* Using accel=kvm,kernel-irqchip=on or accel=kvm,kernel-irqchip=split\n");
         return;
     }
 
-- 
2.18.0.rc1.1.g3f1ff2140

Re: [Qemu-devel] [PATCH] apic: Make APIC ID limit error message clearer

[Wainer dos Santos Moschetta]

On 11/26/2018 08:56 PM, Eduardo Habkost wrote:
> Remove the "apic initialization failed" prefix (it conveys no
> useful information), replace "invalid" with "too large", and add
> an error hint with two possible solutions for the problem.
>
> Before:
>
>    $ qemu-system-x86_64 -machine q35 -smp 256
>    qemu-system-x86_64: apic initialization failed. APIC ID 255 is invalid
>
> After:
>
>    $ qemu-system-x86_64 -machine q35 -smp 256 -display none
>    qemu-system-x86_64: APIC ID 255 is too large

I would keep the problem "apic initialization failed" sentence. "APIC ID 
255 is too large" is just the cause.

>    Possible solutions:
>    * Lowering the number of VCPUs on the -smp option
>    * Using accel=kvm,kernel-irqchip=on or accel=kvm,kernel-irqchip=split
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> I'm not sure this is the best way to provide usage hints to the
> user.  Any suggestions?

As a noob, I can testify that this kind of suggestion is very useful 
although there seems to not have many on QEMU. On the other hand, I 
understand it can make qemu verbose and so annoy people. Thus, maybe 
those suggestions could be enabled/disabled via options (e.g. -show-hints)?

- Wainer

> ---
>   hw/intc/apic.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> index 97ffdd820f..f08006334d 100644
> --- a/hw/intc/apic.c
> +++ b/hw/intc/apic.c
> @@ -886,8 +886,11 @@ static void apic_realize(DeviceState *dev, Error **errp)
>       APICCommonState *s = APIC(dev);
>   
>       if (s->id >= MAX_APICS) {
> -        error_setg(errp, "%s initialization failed. APIC ID %d is invalid",
> -                   object_get_typename(OBJECT(dev)), s->id);
> +        error_setg(errp, "APIC ID %d is too large", s->id);
> +        error_append_hint(errp,
> +            "Possible solutions:\n"
> +            "* Lowering the number of VCPUs on the -smp option\n"
> +            "* Using accel=kvm,kernel-irqchip=on or accel=kvm,kernel-irqchip=split\n");
>           return;
>       }
>   

Re: [Qemu-devel] [PATCH] apic: Make APIC ID limit error message clearer

[Eduardo Habkost]

On Mon, Dec 03, 2018 at 07:07:10PM -0200, Wainer dos Santos Moschetta wrote:
> 
> On 11/26/2018 08:56 PM, Eduardo Habkost wrote:
> > Remove the "apic initialization failed" prefix (it conveys no
> > useful information), replace "invalid" with "too large", and add
> > an error hint with two possible solutions for the problem.
> > 
> > Before:
> > 
> >    $ qemu-system-x86_64 -machine q35 -smp 256
> >    qemu-system-x86_64: apic initialization failed. APIC ID 255 is invalid
> > 
> > After:
> > 
> >    $ qemu-system-x86_64 -machine q35 -smp 256 -display none
> >    qemu-system-x86_64: APIC ID 255 is too large
> 
> I would keep the problem "apic initialization failed" sentence. "APIC ID 255
> is too large" is just the cause.

I'm not sure I agree.  "APIC initialization failed" doesn't
convey any useful information to the user, does it?


> 
> >    Possible solutions:
> >    * Lowering the number of VCPUs on the -smp option
> >    * Using accel=kvm,kernel-irqchip=on or accel=kvm,kernel-irqchip=split
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > I'm not sure this is the best way to provide usage hints to the
> > user.  Any suggestions?
> 
> As a noob, I can testify that this kind of suggestion is very useful
> although there seems to not have many on QEMU. On the other hand, I
> understand it can make qemu verbose and so annoy people. Thus, maybe those
> suggestions could be enabled/disabled via options (e.g. -show-hints)?

I can't imagine who would be annoyed by them.


> 
> - Wainer
> 
> > ---
> >   hw/intc/apic.c | 7 +++++--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> > index 97ffdd820f..f08006334d 100644
> > --- a/hw/intc/apic.c
> > +++ b/hw/intc/apic.c
> > @@ -886,8 +886,11 @@ static void apic_realize(DeviceState *dev, Error **errp)
> >       APICCommonState *s = APIC(dev);
> >       if (s->id >= MAX_APICS) {
> > -        error_setg(errp, "%s initialization failed. APIC ID %d is invalid",
> > -                   object_get_typename(OBJECT(dev)), s->id);
> > +        error_setg(errp, "APIC ID %d is too large", s->id);
> > +        error_append_hint(errp,
> > +            "Possible solutions:\n"
> > +            "* Lowering the number of VCPUs on the -smp option\n"
> > +            "* Using accel=kvm,kernel-irqchip=on or accel=kvm,kernel-irqchip=split\n");
> >           return;
> >       }
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH] apic: Make APIC ID limit error message clearer

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Mon, Dec 03, 2018 at 07:07:10PM -0200, Wainer dos Santos Moschetta wrote:
>> 
>> On 11/26/2018 08:56 PM, Eduardo Habkost wrote:
>> > Remove the "apic initialization failed" prefix (it conveys no
>> > useful information), replace "invalid" with "too large", and add
>> > an error hint with two possible solutions for the problem.
>> > 
>> > Before:
>> > 
>> >    $ qemu-system-x86_64 -machine q35 -smp 256
>> >    qemu-system-x86_64: apic initialization failed. APIC ID 255 is invalid
>> > 
>> > After:
>> > 
>> >    $ qemu-system-x86_64 -machine q35 -smp 256 -display none
>> >    qemu-system-x86_64: APIC ID 255 is too large
>> 
>> I would keep the problem "apic initialization failed" sentence. "APIC ID 255
>> is too large" is just the cause.
>
> I'm not sure I agree.  "APIC initialization failed" doesn't
> convey any useful information to the user, does it?

Concur.

>> >    Possible solutions:
>> >    * Lowering the number of VCPUs on the -smp option
>> >    * Using accel=kvm,kernel-irqchip=on or accel=kvm,kernel-irqchip=split
>> > 
>> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> > ---
>> > I'm not sure this is the best way to provide usage hints to the
>> > user.  Any suggestions?
>> 
>> As a noob, I can testify that this kind of suggestion is very useful
>> although there seems to not have many on QEMU. On the other hand, I
>> understand it can make qemu verbose and so annoy people. Thus, maybe those
>> suggestions could be enabled/disabled via options (e.g. -show-hints)?
>
> I can't imagine who would be annoyed by them.

A few lines of hints are fine.  Longer than that and the error becomes
hard to spot among the hints.