scsi: Clean up duplicated error in legacy if=scsi code

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 <armbru@redhat.com>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Message-Id: <1425925048-15482-2-git-send-email-armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
Markus Armbruster 2015-03-09 19:17:25 +01:00 committed by Paolo Bonzini
parent cba7054928
commit fa61718183
4 changed files with 6 additions and 2 deletions

View File

@ -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;
}

View File

@ -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;
}

View File

@ -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;
}

View File

@ -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;
}