[Qemu-devel] [PATCH v2 1/4] scsi: Clean up duplicated error in legacy if=scsi code

[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 <address@hidden>
Reviewed-by: Peter Crosthwaite <address@hidden>
---
 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;
         }
-- 
1.9.3