From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35384) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVHYR-0005gH-M0 for qemu-devel@nongnu.org; Tue, 10 Mar 2015 06:38:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YVHYK-0002v7-Jw for qemu-devel@nongnu.org; Tue, 10 Mar 2015 06:38:27 -0400 Received: from mail-wi0-x230.google.com ([2a00:1450:400c:c05::230]:37437) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVHYK-0002uy-A2 for qemu-devel@nongnu.org; Tue, 10 Mar 2015 06:38:20 -0400 Received: by widem10 with SMTP id em10so14215656wid.2 for ; Tue, 10 Mar 2015 03:38:19 -0700 (PDT) Sender: Paolo Bonzini From: Paolo Bonzini Date: Tue, 10 Mar 2015 11:37:56 +0100 Message-Id: <1425983880-26906-8-git-send-email-pbonzini@redhat.com> In-Reply-To: <1425983880-26906-1-git-send-email-pbonzini@redhat.com> References: <1425983880-26906-1-git-send-email-pbonzini@redhat.com> Subject: [Qemu-devel] [PULL 07/11] scsi: Clean up duplicated error in legacy if=scsi code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Markus Armbruster From: Markus Armbruster Commit a818a4b changed scsi_bus_legacy_handle_cmdline() to report errors from scsi_bus_legacy_add_drive() with error_report() in addition to returning them. That's inappropriate. Two kinds of callers: 1. realize methods (devices "esp", "virtio-scsi-device" and "spapr-vscsi") The error object gets passed up the call chain until it gets reported again and freed. Example: $ qemu-system-arm -M virt -S -display none \ > -drive if=scsi,id=foo,bus=1,file=tmp.qcow2 \ > -device nec-usb-xhci -device usb-storage,drive=foo \ > -device virtio-scsi-pci qemu-system-arm: -drive if=scsi,id=foo,bus=1,file=tmp.qcow2: Property 'scsi-disk.drive' can't take value 'foo', it's in use qemu-system-arm: -drive if=scsi,id=foo,bus=1,file=tmp.qcow2: Setting drive property failed qemu-system-arm: -device virtio-scsi-pci: Setting drive property failed qemu-system-arm: -device virtio-scsi-pci: Device initialization failed qemu-system-arm: -device virtio-scsi-pci: Device 'virtio-scsi-pci' could not be initialized The second message in this error cascade comes from scsi_bus_legacy_handle_cmdline(). The error object then gets passed up to the qdev_init() called from virtio_scsi_pci_init_pci(), which reports it again. 2. init methods (devices "am53c974", "dc390", "lsi53c895a", "lsi53c810", "megasas", "megasas-gen2") init methods need to report their errors with qerror_report(). These don't. The inappropriate error_report() papers over the bug. error_report() isn't the same as qerror_report() in QMP context, but this can't actually happen: QMP can still only hot-plug, and callers call scsi_bus_legacy_handle_cmdline() only on cold-plug. Except for sysbus_esp_realize(), but that can't be hot-plugged at all, as far as I can tell. Fix the init methods and drop the inappropriate error_report() in scsi_bus_legacy_handle_cmdline(). Signed-off-by: Markus Armbruster Reviewed-by: Peter Crosthwaite Message-Id: <1425925048-15482-2-git-send-email-armbru@redhat.com> Signed-off-by: Paolo Bonzini --- hw/scsi/esp-pci.c | 2 ++ hw/scsi/lsi53c895a.c | 3 ++- hw/scsi/megasas.c | 2 ++ hw/scsi/scsi-bus.c | 1 - 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c index 00b7297..a75fcfa 100644 --- a/hw/scsi/esp-pci.c +++ b/hw/scsi/esp-pci.c @@ -28,6 +28,7 @@ #include "hw/scsi/esp.h" #include "trace.h" #include "qemu/log.h" +#include "qapi/qmp/qerror.h" #define TYPE_AM53C974_DEVICE "am53c974" @@ -369,6 +370,7 @@ static int esp_pci_scsi_init(PCIDevice *dev) if (!d->hotplugged) { scsi_bus_legacy_handle_cmdline(&s->bus, &err); if (err != NULL) { + qerror_report_err(err); error_free(err); return -1; } diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c index db7d4b8..4ffab03 100644 --- a/hw/scsi/lsi53c895a.c +++ b/hw/scsi/lsi53c895a.c @@ -19,7 +19,7 @@ #include "hw/pci/pci.h" #include "hw/scsi/scsi.h" #include "sysemu/dma.h" -#include "qemu/error-report.h" +#include "qapi/qmp/qerror.h" //#define DEBUG_LSI //#define DEBUG_LSI_REG @@ -2119,6 +2119,7 @@ static int lsi_scsi_init(PCIDevice *dev) if (!d->hotplugged) { scsi_bus_legacy_handle_cmdline(&s->bus, &err); if (err != NULL) { + qerror_report_err(err); error_free(err); return -1; } diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c index 4852237..69ffdbd 100644 --- a/hw/scsi/megasas.c +++ b/hw/scsi/megasas.c @@ -28,6 +28,7 @@ #include "hw/scsi/scsi.h" #include "block/scsi.h" #include "trace.h" +#include "qapi/qmp/qerror.h" #include "mfi.h" @@ -2409,6 +2410,7 @@ static int megasas_scsi_init(PCIDevice *dev) if (!d->hotplugged) { scsi_bus_legacy_handle_cmdline(&s->bus, &err); if (err != NULL) { + qerror_report_err(err); error_free(err); return -1; } diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index dca9576..f8de569 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -273,7 +273,6 @@ void scsi_bus_legacy_handle_cmdline(SCSIBus *bus, Error **errp) scsi_bus_legacy_add_drive(bus, blk_by_legacy_dinfo(dinfo), unit, false, -1, NULL, &err); if (err != NULL) { - error_report("%s", error_get_pretty(err)); error_propagate(errp, err); break; } -- 2.3.0