| Message ID | 20171026080041.8280-1-aik@ozlabs.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Alexey Kardashevskiy <aik@ozlabs.ru> writes: > Running "qemu-system-ppc64 -machine prep -device i82374" creates an ISA > bus with two i82374 DMA controllers - one is implicit from ppc_prep_init(), > the other one is from "-device i82374". QEMU asserts but it is not > immediately clear why. > > This adds an error message to explain the failure. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > > > Better phase suggestions are welcome. Thanks! > > > --- > hw/isa/isa-bus.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c > index 348e0eab9d..553707b18d 100644 > --- a/hw/isa/isa-bus.c > +++ b/hw/isa/isa-bus.c > @@ -107,7 +107,10 @@ void isa_connect_gpio_out(ISADevice *isadev, int gpioirq, int isairq) > void isa_bus_dma(ISABus *bus, IsaDma *dma8, IsaDma *dma16) > { > assert(bus && dma8 && dma16); > - assert(!bus->dma[0] && !bus->dma[1]); > + if (bus->dma[0] || bus->dma[1]) { > + error_setg(&error_fatal, > + "DMA is already set to ISA bus, duplicated DMA controller?"); > + } > bus->dma[0] = dma8; > bus->dma[1] = dma16; > } Quote include/qapi/error.h: * Please don't error_setg(&error_fatal, ...), use error_report() and * exit(), because that's more obvious.
On 26 October 2017 at 09:00, Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > Running "qemu-system-ppc64 -machine prep -device i82374" creates an ISA > bus with two i82374 DMA controllers - one is implicit from ppc_prep_init(), > the other one is from "-device i82374". QEMU asserts but it is not > immediately clear why. > > This adds an error message to explain the failure. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> Does it ever make sense for the user to try to create this device on the command line? If not, maybe we should set it to dc->user_creatable = false; ? If it can be usefully user-created, then rather than being a fatal error in this function the error should be passed back up so that we can fail the 'realize' method of the device. thanks -- PMM
On 10/26/2017 04:00 AM, Alexey Kardashevskiy wrote: > Running "qemu-system-ppc64 -machine prep -device i82374" creates an ISA > bus with two i82374 DMA controllers - one is implicit from ppc_prep_init(), > the other one is from "-device i82374". QEMU asserts but it is not > immediately clear why. > > This adds an error message to explain the failure. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > > > Better phase suggestions are welcome. Thanks! > > > --- > hw/isa/isa-bus.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c > index 348e0eab9d..553707b18d 100644 > --- a/hw/isa/isa-bus.c > +++ b/hw/isa/isa-bus.c > @@ -107,7 +107,10 @@ void isa_connect_gpio_out(ISADevice *isadev, int gpioirq, int isairq) > void isa_bus_dma(ISABus *bus, IsaDma *dma8, IsaDma *dma16) > { > assert(bus && dma8 && dma16); > - assert(!bus->dma[0] && !bus->dma[1]); > + if (bus->dma[0] || bus->dma[1]) { > + error_setg(&error_fatal, > + "DMA is already set to ISA bus, duplicated DMA controller?"); > + } > bus->dma[0] = dma8; > bus->dma[1] = dma16; > } > I suppose it's an improvement strictly, but really we're just naming a runtime assertion here. We should be avoiding the assertion -- and then how valuable is the error message? Is this something we anticipate can never be fixed? (I.e. exclusively the cause of asking for impossible configurations?)
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c index 348e0eab9d..553707b18d 100644 --- a/hw/isa/isa-bus.c +++ b/hw/isa/isa-bus.c @@ -107,7 +107,10 @@ void isa_connect_gpio_out(ISADevice *isadev, int gpioirq, int isairq) void isa_bus_dma(ISABus *bus, IsaDma *dma8, IsaDma *dma16) { assert(bus && dma8 && dma16); - assert(!bus->dma[0] && !bus->dma[1]); + if (bus->dma[0] || bus->dma[1]) { + error_setg(&error_fatal, + "DMA is already set to ISA bus, duplicated DMA controller?"); + } bus->dma[0] = dma8; bus->dma[1] = dma16; }
Running "qemu-system-ppc64 -machine prep -device i82374" creates an ISA bus with two i82374 DMA controllers - one is implicit from ppc_prep_init(), the other one is from "-device i82374". QEMU asserts but it is not immediately clear why. This adds an error message to explain the failure. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- Better phase suggestions are welcome. Thanks! --- hw/isa/isa-bus.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)