isa-fdc: assert replaced by proper error exit
PATCH 1/1 isa-fdc: assert replaced by proper error exit
When not available, isa-fdc falls into assert instead of proper error exit. This patch fixes this behavior.
--- hw/block/fdc.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
hw/block/fdc.c +5 -1
@@ -2699,11 +2699,15 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp) fdctrl->dma_chann = isa->dma;if (fdctrl->dma_chann !=-1 ) { fdctrl->dma =isa_get_dma (isa_bus_from_device (isadev), isa->dma); -assert (fdctrl->dma); +if (!fdctrl->dma) { +error_setg (errp,"isa-fdc not supported" ); + goto error; + } }qdev_set_legacy_instance_id (dev, isa->iobase,2 );fdctrl_realize_common (dev, fdctrl, &err); +error:if (err !=NULL ) {error_propagate (errp, err);return ;
Signature · 1 line
2.13.5
Quoted 19 lines · Eduardo Otubo
When not available, isa-fdc falls into assert instead of proper error exit. This patch fixes this behavior.
Signed-off-by Eduardo Otubo <[email protected]>--- hw/block/fdc.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)hw/block/fdc.c +2
-1@@ -2699,11 +2699,15 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp) fdctrl->dma_chann = isa->dma;if (fdctrl->dma_chann !=-1 ) { fdctrl->dma =isa_get_dma (isa_bus_from_device (isadev), isa->dma); -assert (fdctrl->dma); +if (!fdctrl->dma) { +error_setg (errp,"isa-fdc not supported" );
hw/block/fdc.c static void isabus_fdc_realize(DeviceState *dev, Error **errp) · line 2713
Quoted 10 lines
hw/block/fdc.c +3
-0+ goto error; + } }qdev_set_legacy_instance_id (dev, isa->iobase,2 );fdctrl_realize_common (dev, fdctrl, &err); +error:if (err !=NULL ) {error_propagate (errp, err);return ;
Maybe add the reproducer to the commit message:
qemu-system-ppc64 -S -machine powernv -device isa-fdc
Quoted 2 lines · Eduardo Otubo
When not available, isa-fdc falls into assert instead of proper error exit. This patch fixes this behavior.
When what exactly isn't available?
Quoted 17 lines
Signed-off-by Eduardo Otubo <[email protected]>--- hw/block/fdc.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)hw/block/fdc.c +2
-1@@ -2699,11 +2699,15 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp) fdctrl->dma_chann = isa->dma;if (fdctrl->dma_chann !=-1 ) { fdctrl->dma =isa_get_dma (isa_bus_from_device (isadev), isa->dma); -assert (fdctrl->dma); +if (!fdctrl->dma) { +error_setg (errp,"isa-fdc not supported" );
hw/block/fdc.c static void isabus_fdc_realize(DeviceState *dev, Error **errp) · line 2713
Quoted 10 lines
hw/block/fdc.c +3
-0+ goto error; + } }qdev_set_legacy_instance_id (dev, isa->iobase,2 );fdctrl_realize_common (dev, fdctrl, &err); +error:if (err !=NULL ) {error_propagate (errp, err);return ;
The actual patch makes me suspect it's "when ISA DMA isn't available".
Quoted 20 lines · Eduardo Otubo
When not available, isa-fdc falls into assert instead of proper error exit. This patch fixes this behavior.
Signed-off-by Eduardo Otubo <[email protected]>--- hw/block/fdc.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)hw/block/fdc.c +2
-1@@ -2699,11 +2699,15 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp) fdctrl->dma_chann = isa->dma;if (fdctrl->dma_chann !=-1 ) { fdctrl->dma =isa_get_dma (isa_bus_from_device (isadev), isa->dma); -assert (fdctrl->dma); +if (!fdctrl->dma) { +error_setg (errp,"isa-fdc not supported" );
hw/block/fdc.c static void isabus_fdc_realize(DeviceState *dev, Error **errp) · line 2713
Quoted 10 lines
hw/block/fdc.c +3
-0+ goto error; + } }qdev_set_legacy_instance_id (dev, isa->iobase,2 );fdctrl_realize_common (dev, fdctrl, &err); +error:if (err !=NULL ) {error_propagate (errp, err);return ;
I was going to ask if we needn't take care of undoing the portio or IRQ registration, but it turns out that there is not one single caller of portio_list_destroy in all of QEMU...
Paolo admitted as much in e3fb0ade83420a86464ee50c71f2daf5641cab10 when he updated the destroy function :)
It looks like the way we assign IRQs to ISA devices is kind of simple and not trivial to undo correctly, so instead of barking up that tree, why don't we hoist the isa_get_dma call up and fail before we try to initialize portio or IRQs?