| Message ID | 159280903824.485572.831378159272329707.stgit@bahia.lan (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ppc/pnv: Silence missing BMC warning with qtest | expand |
On 6/22/20 8:57 AM, Greg Kurz wrote: > The device introspect test in qtest emits some warnings with the > the pnv machine types during the "nodefaults" phase: > > TEST check-qtest-ppc64: tests/qtest/device-introspect-test > qemu-system-ppc64: warning: machine has no BMC device. Use '-device > ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define > one > qemu-system-ppc64: warning: machine has no BMC device. Use '-device > ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define > one > qemu-system-ppc64: warning: machine has no BMC device. Use '-device > ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define > one > > This is expected since the pnv machine doesn't create the internal > BMC simulator fallback when "-nodefaults" is passed on the command > line, but these warnings appear in ci logs and confuse people. > > Not having a BMC isn't recommended but it is still a supported > configuration, so a straightforward fix is to just silent this > warning when qtest is enabled. > > Fixes: 25f3170b0654 ("ppc/pnv: Create BMC devices only when defaults are enabled") > Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Signed-off-by: Greg Kurz <groug@kaod.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> It looks good but could you reproduce ? C. > --- > hw/ppc/pnv.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index 806a5d9a8d34..1622d29b4ba7 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -21,6 +21,7 @@ > #include "qemu-common.h" > #include "qemu/units.h" > #include "qapi/error.h" > +#include "sysemu/qtest.h" > #include "sysemu/sysemu.h" > #include "sysemu/numa.h" > #include "sysemu/reset.h" > @@ -587,9 +588,11 @@ static void pnv_reset(MachineState *machine) > bmc = pnv_bmc_find(&error_fatal); > if (!pnv->bmc) { > if (!bmc) { > - warn_report("machine has no BMC device. Use '-device " > - "ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' " > - "to define one"); > + if (!qtest_enabled()) { > + warn_report("machine has no BMC device. Use '-device " > + "ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' " > + "to define one"); > + } > } else { > pnv_bmc_set_pnor(bmc, pnv->pnor); > pnv->bmc = bmc; > >
On Mon, Jun 22, 2020 at 08:57:18AM +0200, Greg Kurz wrote: > The device introspect test in qtest emits some warnings with the > the pnv machine types during the "nodefaults" phase: > > TEST check-qtest-ppc64: tests/qtest/device-introspect-test > qemu-system-ppc64: warning: machine has no BMC device. Use '-device > ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define > one > qemu-system-ppc64: warning: machine has no BMC device. Use '-device > ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define > one > qemu-system-ppc64: warning: machine has no BMC device. Use '-device > ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define > one > > This is expected since the pnv machine doesn't create the internal > BMC simulator fallback when "-nodefaults" is passed on the command > line, but these warnings appear in ci logs and confuse people. > > Not having a BMC isn't recommended but it is still a supported > configuration, so a straightforward fix is to just silent this > warning when qtest is enabled. > > Fixes: 25f3170b0654 ("ppc/pnv: Create BMC devices only when defaults are enabled") > Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Signed-off-by: Greg Kurz <groug@kaod.org> Applied to ppc-for-5.1, thanks. > --- > hw/ppc/pnv.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index 806a5d9a8d34..1622d29b4ba7 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -21,6 +21,7 @@ > #include "qemu-common.h" > #include "qemu/units.h" > #include "qapi/error.h" > +#include "sysemu/qtest.h" > #include "sysemu/sysemu.h" > #include "sysemu/numa.h" > #include "sysemu/reset.h" > @@ -587,9 +588,11 @@ static void pnv_reset(MachineState *machine) > bmc = pnv_bmc_find(&error_fatal); > if (!pnv->bmc) { > if (!bmc) { > - warn_report("machine has no BMC device. Use '-device " > - "ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' " > - "to define one"); > + if (!qtest_enabled()) { > + warn_report("machine has no BMC device. Use '-device " > + "ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' " > + "to define one"); > + } > } else { > pnv_bmc_set_pnor(bmc, pnv->pnor); > pnv->bmc = bmc; > >
On Mon, 22 Jun 2020 09:13:46 +0200 Cédric Le Goater <clg@kaod.org> wrote: > On 6/22/20 8:57 AM, Greg Kurz wrote: > > The device introspect test in qtest emits some warnings with the > > the pnv machine types during the "nodefaults" phase: > > > > TEST check-qtest-ppc64: tests/qtest/device-introspect-test > > qemu-system-ppc64: warning: machine has no BMC device. Use '-device > > ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define > > one > > qemu-system-ppc64: warning: machine has no BMC device. Use '-device > > ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define > > one > > qemu-system-ppc64: warning: machine has no BMC device. Use '-device > > ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define > > one > > > > This is expected since the pnv machine doesn't create the internal > > BMC simulator fallback when "-nodefaults" is passed on the command > > line, but these warnings appear in ci logs and confuse people. > > > > Not having a BMC isn't recommended but it is still a supported > > configuration, so a straightforward fix is to just silent this > > warning when qtest is enabled. > > > > Fixes: 25f3170b0654 ("ppc/pnv: Create BMC devices only when defaults are enabled") > > Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > Signed-off-by: Greg Kurz <groug@kaod.org> > > Reviewed-by: Cédric Le Goater <clg@kaod.org> > > It looks good but could you reproduce ? > Yup, this test is only run in "slow" mode, eg: make check-qtest-ppc64 SPEED=slow > C. > > > --- > > hw/ppc/pnv.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > > index 806a5d9a8d34..1622d29b4ba7 100644 > > --- a/hw/ppc/pnv.c > > +++ b/hw/ppc/pnv.c > > @@ -21,6 +21,7 @@ > > #include "qemu-common.h" > > #include "qemu/units.h" > > #include "qapi/error.h" > > +#include "sysemu/qtest.h" > > #include "sysemu/sysemu.h" > > #include "sysemu/numa.h" > > #include "sysemu/reset.h" > > @@ -587,9 +588,11 @@ static void pnv_reset(MachineState *machine) > > bmc = pnv_bmc_find(&error_fatal); > > if (!pnv->bmc) { > > if (!bmc) { > > - warn_report("machine has no BMC device. Use '-device " > > - "ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' " > > - "to define one"); > > + if (!qtest_enabled()) { > > + warn_report("machine has no BMC device. Use '-device " > > + "ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' " > > + "to define one"); > > + } > > } else { > > pnv_bmc_set_pnor(bmc, pnv->pnor); > > pnv->bmc = bmc; > > > > >
On 6/22/20 9:53 AM, Greg Kurz wrote: > On Mon, 22 Jun 2020 09:13:46 +0200 > Cédric Le Goater <clg@kaod.org> wrote: > >> On 6/22/20 8:57 AM, Greg Kurz wrote: >>> The device introspect test in qtest emits some warnings with the >>> the pnv machine types during the "nodefaults" phase: >>> >>> TEST check-qtest-ppc64: tests/qtest/device-introspect-test >>> qemu-system-ppc64: warning: machine has no BMC device. Use '-device >>> ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define >>> one >>> qemu-system-ppc64: warning: machine has no BMC device. Use '-device >>> ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define >>> one >>> qemu-system-ppc64: warning: machine has no BMC device. Use '-device >>> ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define >>> one >>> >>> This is expected since the pnv machine doesn't create the internal >>> BMC simulator fallback when "-nodefaults" is passed on the command >>> line, but these warnings appear in ci logs and confuse people. >>> >>> Not having a BMC isn't recommended but it is still a supported >>> configuration, so a straightforward fix is to just silent this >>> warning when qtest is enabled. >>> >>> Fixes: 25f3170b0654 ("ppc/pnv: Create BMC devices only when defaults are enabled") >>> Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com> >>> Signed-off-by: Greg Kurz <groug@kaod.org> >> >> Reviewed-by: Cédric Le Goater <clg@kaod.org> >> >> It looks good but could you reproduce ? >> > > Yup, this test is only run in "slow" mode, eg: > > make check-qtest-ppc64 SPEED=slow Indeed: https://gitlab.com/qemu-project/qemu/-/jobs/603546723#L3337 See in .gitlab-ci.yml: build-disabled: ... - make -j"$JOBS" - make -j"$JOBS" check-qtest SPEED=slow Thomas, FYI this job is now timeouting most of the time. > > >> C. >> >>> --- >>> hw/ppc/pnv.c | 9 ++++++--- >>> 1 file changed, 6 insertions(+), 3 deletions(-) >>> >>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >>> index 806a5d9a8d34..1622d29b4ba7 100644 >>> --- a/hw/ppc/pnv.c >>> +++ b/hw/ppc/pnv.c >>> @@ -21,6 +21,7 @@ >>> #include "qemu-common.h" >>> #include "qemu/units.h" >>> #include "qapi/error.h" >>> +#include "sysemu/qtest.h" >>> #include "sysemu/sysemu.h" >>> #include "sysemu/numa.h" >>> #include "sysemu/reset.h" >>> @@ -587,9 +588,11 @@ static void pnv_reset(MachineState *machine) >>> bmc = pnv_bmc_find(&error_fatal); >>> if (!pnv->bmc) { >>> if (!bmc) { >>> - warn_report("machine has no BMC device. Use '-device " >>> - "ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' " >>> - "to define one"); >>> + if (!qtest_enabled()) { >>> + warn_report("machine has no BMC device. Use '-device " >>> + "ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' " >>> + "to define one"); >>> + } >>> } else { >>> pnv_bmc_set_pnor(bmc, pnv->pnor); >>> pnv->bmc = bmc; >>> >>> >> >
On 22/06/2020 10.24, Philippe Mathieu-Daudé wrote: > On 6/22/20 9:53 AM, Greg Kurz wrote: >> On Mon, 22 Jun 2020 09:13:46 +0200 >> Cédric Le Goater <clg@kaod.org> wrote: >> >>> On 6/22/20 8:57 AM, Greg Kurz wrote: >>>> The device introspect test in qtest emits some warnings with the >>>> the pnv machine types during the "nodefaults" phase: >>>> >>>> TEST check-qtest-ppc64: tests/qtest/device-introspect-test >>>> qemu-system-ppc64: warning: machine has no BMC device. Use '-device >>>> ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define >>>> one >>>> qemu-system-ppc64: warning: machine has no BMC device. Use '-device >>>> ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define >>>> one >>>> qemu-system-ppc64: warning: machine has no BMC device. Use '-device >>>> ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define >>>> one >>>> >>>> This is expected since the pnv machine doesn't create the internal >>>> BMC simulator fallback when "-nodefaults" is passed on the command >>>> line, but these warnings appear in ci logs and confuse people. >>>> >>>> Not having a BMC isn't recommended but it is still a supported >>>> configuration, so a straightforward fix is to just silent this >>>> warning when qtest is enabled. >>>> >>>> Fixes: 25f3170b0654 ("ppc/pnv: Create BMC devices only when defaults are enabled") >>>> Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com> >>>> Signed-off-by: Greg Kurz <groug@kaod.org> >>> >>> Reviewed-by: Cédric Le Goater <clg@kaod.org> >>> >>> It looks good but could you reproduce ? >>> >> >> Yup, this test is only run in "slow" mode, eg: >> >> make check-qtest-ppc64 SPEED=slow > > Indeed: > https://gitlab.com/qemu-project/qemu/-/jobs/603546723#L3337 > > See in .gitlab-ci.yml: > > build-disabled: > ... > - make -j"$JOBS" > - make -j"$JOBS" check-qtest SPEED=slow > > Thomas, FYI this job is now timeouting most of the time. Do you know why it got much slower? Have additional tests been added? Or is there a performance regressions somewhere? Thomas
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 806a5d9a8d34..1622d29b4ba7 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -21,6 +21,7 @@ #include "qemu-common.h" #include "qemu/units.h" #include "qapi/error.h" +#include "sysemu/qtest.h" #include "sysemu/sysemu.h" #include "sysemu/numa.h" #include "sysemu/reset.h" @@ -587,9 +588,11 @@ static void pnv_reset(MachineState *machine) bmc = pnv_bmc_find(&error_fatal); if (!pnv->bmc) { if (!bmc) { - warn_report("machine has no BMC device. Use '-device " - "ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' " - "to define one"); + if (!qtest_enabled()) { + warn_report("machine has no BMC device. Use '-device " + "ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' " + "to define one"); + } } else { pnv_bmc_set_pnor(bmc, pnv->pnor); pnv->bmc = bmc;
The device introspect test in qtest emits some warnings with the the pnv machine types during the "nodefaults" phase: TEST check-qtest-ppc64: tests/qtest/device-introspect-test qemu-system-ppc64: warning: machine has no BMC device. Use '-device ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define one qemu-system-ppc64: warning: machine has no BMC device. Use '-device ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define one qemu-system-ppc64: warning: machine has no BMC device. Use '-device ipmi-bmc-sim,id=bmc0 -device isa-ipmi-bt,bmc=bmc0,irq=10' to define one This is expected since the pnv machine doesn't create the internal BMC simulator fallback when "-nodefaults" is passed on the command line, but these warnings appear in ci logs and confuse people. Not having a BMC isn't recommended but it is still a supported configuration, so a straightforward fix is to just silent this warning when qtest is enabled. Fixes: 25f3170b0654 ("ppc/pnv: Create BMC devices only when defaults are enabled") Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Greg Kurz <groug@kaod.org> --- hw/ppc/pnv.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)