[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] apic: Make APIC ID limit error message clearer
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH] apic: Make APIC ID limit error message clearer |
Date: |
Tue, 4 Dec 2018 16:21:33 -0200 |
User-agent: |
Mutt/1.9.2 (2017-12-15) |
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 <address@hidden>
> > ---
> > 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