* [Qemu-devel] [PATCH v2 1/4] scsi: Clean up duplicated error in legacy if=scsi code
2015-03-09 18:17 [Qemu-devel] [PATCH v2 0/4] scsi: Error reporting cleanups, realize() Markus Armbruster
@ 2015-03-09 18:17 ` Markus Armbruster
2015-03-09 18:17 ` [Qemu-devel] [PATCH v2 2/4] hw: Propagate errors through qdev_prop_set_drive() Markus Armbruster
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2015-03-09 18:17 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, peter.crosthwaite, hare
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>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH v2 2/4] hw: Propagate errors through qdev_prop_set_drive()
2015-03-09 18:17 [Qemu-devel] [PATCH v2 0/4] scsi: Error reporting cleanups, realize() Markus Armbruster
2015-03-09 18:17 ` [Qemu-devel] [PATCH v2 1/4] scsi: Clean up duplicated error in legacy if=scsi code Markus Armbruster
@ 2015-03-09 18:17 ` Markus Armbruster
2015-03-09 18:17 ` [Qemu-devel] [PATCH v2 3/4] scsi: Improve error reporting for invalid drive property Markus Armbruster
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2015-03-09 18:17 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, peter.crosthwaite, hare
Three kinds of callers:
1. On failure, report the error and abort
Passing &error_abort does the job. No functional change.
2. On failure, report the error and exit()
This is qdev_prop_set_drive_nofail(). Error reporting moves from
qdev_prop_set_drive() to its caller. Because hiding away the error
in the monitor right before exit() isn't helpful, replace
qerror_report_err() by error_report_err(). Shouldn't make a
difference, because qdev_prop_set_drive_nofail() should never be
used in QMP context.
3. On failure, report the error and recover
This is usb_msd_init() and scsi_bus_legacy_add_drive(). Error
reporting and freeing the error object moves from
qdev_prop_set_drive() to its callers.
Because usb_msd_init() can't run in QMP context, replace
qerror_report_err() by error_report_err() there.
No functional change.
scsi_bus_legacy_add_drive() calling qerror_report_err() is of
course inappropriate, but this commit merely makes it more obvious.
The next one will clean it up.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
hw/arm/vexpress.c | 6 +++---
hw/arm/virt.c | 6 +++---
hw/block/pflash_cfi01.c | 4 ++--
hw/block/pflash_cfi02.c | 4 ++--
hw/core/qdev-properties-system.c | 22 ++++++++++------------
hw/scsi/scsi-bus.c | 6 +++++-
hw/usb/dev-storage.c | 7 +++++--
include/hw/qdev-properties.h | 4 ++--
8 files changed, 32 insertions(+), 27 deletions(-)
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 5933454..8496c16 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -515,9 +515,9 @@ static pflash_t *ve_pflash_cfi01_register(hwaddr base, const char *name,
{
DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
- if (di && qdev_prop_set_drive(dev, "drive",
- blk_by_legacy_dinfo(di))) {
- abort();
+ if (di) {
+ qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(di),
+ &error_abort);
}
qdev_prop_set_uint32(dev, "num-blocks",
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 69f51ac..93b7605 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -522,9 +522,9 @@ static void create_one_flash(const char *name, hwaddr flashbase,
DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
const uint64_t sectorlength = 256 * 1024;
- if (dinfo && qdev_prop_set_drive(dev, "drive",
- blk_by_legacy_dinfo(dinfo))) {
- abort();
+ if (dinfo) {
+ qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
+ &error_abort);
}
qdev_prop_set_uint32(dev, "num-blocks", flashsize / sectorlength);
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 89d380e..d282695 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -969,8 +969,8 @@ pflash_t *pflash_cfi01_register(hwaddr base,
{
DeviceState *dev = qdev_create(NULL, TYPE_CFI_PFLASH01);
- if (blk && qdev_prop_set_drive(dev, "drive", blk)) {
- abort();
+ if (blk) {
+ qdev_prop_set_drive(dev, "drive", blk, &error_abort);
}
qdev_prop_set_uint32(dev, "num-blocks", nb_blocs);
qdev_prop_set_uint64(dev, "sector-length", sector_len);
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 389b4aa..074a005 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -773,8 +773,8 @@ pflash_t *pflash_cfi02_register(hwaddr base,
{
DeviceState *dev = qdev_create(NULL, TYPE_CFI_PFLASH02);
- if (blk && qdev_prop_set_drive(dev, "drive", blk)) {
- abort();
+ if (blk) {
+ qdev_prop_set_drive(dev, "drive", blk, &error_abort);
}
qdev_prop_set_uint32(dev, "num-blocks", nb_blocs);
qdev_prop_set_uint32(dev, "sector-length", sector_len);
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index a2e44bd..c413226 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -341,27 +341,25 @@ PropertyInfo qdev_prop_vlan = {
.set = set_vlan,
};
-int qdev_prop_set_drive(DeviceState *dev, const char *name,
- BlockBackend *value)
+void qdev_prop_set_drive(DeviceState *dev, const char *name,
+ BlockBackend *value, Error **errp)
{
- Error *err = NULL;
- object_property_set_str(OBJECT(dev),
- value ? blk_name(value) : "", name, &err);
- if (err) {
- qerror_report_err(err);
- error_free(err);
- return -1;
- }
- return 0;
+ object_property_set_str(OBJECT(dev), value ? blk_name(value) : "",
+ name, errp);
}
void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name,
BlockBackend *value)
{
- if (qdev_prop_set_drive(dev, name, value) < 0) {
+ Error *err = NULL;
+
+ qdev_prop_set_drive(dev, name, value, &err);
+ if (err) {
+ error_report_err(err);
exit(1);
}
}
+
void qdev_prop_set_chr(DeviceState *dev, const char *name,
CharDriverState *value)
{
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index f8de569..61c595f 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -7,6 +7,7 @@
#include "sysemu/blockdev.h"
#include "trace.h"
#include "sysemu/dma.h"
+#include "qapi/qmp/qerror.h"
static char *scsibus_get_dev_path(DeviceState *dev);
static char *scsibus_get_fw_dev_path(DeviceState *dev);
@@ -242,7 +243,10 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
if (serial && object_property_find(OBJECT(dev), "serial", NULL)) {
qdev_prop_set_string(dev, "serial", serial);
}
- if (qdev_prop_set_drive(dev, "drive", blk) < 0) {
+ qdev_prop_set_drive(dev, "drive", blk, &err);
+ if (err) {
+ qerror_report_err(err);
+ error_free(err);
error_setg(errp, "Setting drive property failed");
object_unparent(OBJECT(dev));
return NULL;
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 65d9aa6..3f5b32d 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -663,6 +663,7 @@ static void usb_msd_realize_bot(USBDevice *dev, Error **errp)
static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
{
static int nr=0;
+ Error *err = NULL;
char id[8];
QemuOpts *opts;
DriveInfo *dinfo;
@@ -706,8 +707,10 @@ static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
/* create guest device */
dev = usb_create(bus, "usb-storage");
- if (qdev_prop_set_drive(&dev->qdev, "drive",
- blk_by_legacy_dinfo(dinfo)) < 0) {
+ qdev_prop_set_drive(&dev->qdev, "drive", blk_by_legacy_dinfo(dinfo),
+ &err);
+ if (err) {
+ error_report_err(err);
object_unparent(OBJECT(dev));
return NULL;
}
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 57ee363..2d47c0c 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -168,8 +168,8 @@ void qdev_prop_set_uint64(DeviceState *dev, const char *name, uint64_t value);
void qdev_prop_set_string(DeviceState *dev, const char *name, const char *value);
void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState *value);
void qdev_prop_set_netdev(DeviceState *dev, const char *name, NetClientState *value);
-int qdev_prop_set_drive(DeviceState *dev, const char *name,
- BlockBackend *value) QEMU_WARN_UNUSED_RESULT;
+void qdev_prop_set_drive(DeviceState *dev, const char *name,
+ BlockBackend *value, Error **errp);
void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name,
BlockBackend *value);
void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value);
--
1.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH v2 3/4] scsi: Improve error reporting for invalid drive property
2015-03-09 18:17 [Qemu-devel] [PATCH v2 0/4] scsi: Error reporting cleanups, realize() Markus Armbruster
2015-03-09 18:17 ` [Qemu-devel] [PATCH v2 1/4] scsi: Clean up duplicated error in legacy if=scsi code Markus Armbruster
2015-03-09 18:17 ` [Qemu-devel] [PATCH v2 2/4] hw: Propagate errors through qdev_prop_set_drive() Markus Armbruster
@ 2015-03-09 18:17 ` Markus Armbruster
2015-03-09 18:17 ` [Qemu-devel] [PATCH v2 4/4] scsi: Convert remaining PCI HBAs to realize() Markus Armbruster
2015-03-09 19:22 ` [Qemu-devel] [PATCH v2 0/4] scsi: Error reporting cleanups, realize() Paolo Bonzini
4 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2015-03-09 18:17 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, peter.crosthwaite, hare
When setting "realized" fails, scsi_bus_legacy_add_drive() passes the
error to qerror_report_err(), then returns an unspecific "Setting
drive property failed" error, which is reported further up the call
chain.
Example:
$ qemu-system-x86_64 -nodefaults -S -display none \
> -drive if=scsi,id=foo,file=tmp.qcow2 -global isa-fdc.driveA=foo
qemu-system-x86_64: -drive if=scsi,id=foo,file=tmp.qcow2: Property 'scsi-disk.drive' can't take value 'foo', it's in use
qemu-system-x86_64: Setting drive property failed
qemu-system-x86_64: Initialization of device lsi53c895a failed: Device initialization failed
Clean up the obvious way: simply return the original error to the
caller. Gets rid of the second message in the above error cascade.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
hw/scsi/scsi-bus.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 61c595f..bd2c0e4 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -7,7 +7,6 @@
#include "sysemu/blockdev.h"
#include "trace.h"
#include "sysemu/dma.h"
-#include "qapi/qmp/qerror.h"
static char *scsibus_get_dev_path(DeviceState *dev);
static char *scsibus_get_fw_dev_path(DeviceState *dev);
@@ -245,9 +244,7 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
}
qdev_prop_set_drive(dev, "drive", blk, &err);
if (err) {
- qerror_report_err(err);
- error_free(err);
- error_setg(errp, "Setting drive property failed");
+ error_propagate(errp, err);
object_unparent(OBJECT(dev));
return NULL;
}
--
1.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH v2 4/4] scsi: Convert remaining PCI HBAs to realize()
2015-03-09 18:17 [Qemu-devel] [PATCH v2 0/4] scsi: Error reporting cleanups, realize() Markus Armbruster
` (2 preceding siblings ...)
2015-03-09 18:17 ` [Qemu-devel] [PATCH v2 3/4] scsi: Improve error reporting for invalid drive property Markus Armbruster
@ 2015-03-09 18:17 ` Markus Armbruster
2015-03-09 19:22 ` [Qemu-devel] [PATCH v2 0/4] scsi: Error reporting cleanups, realize() Paolo Bonzini
4 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2015-03-09 18:17 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, peter.crosthwaite, hare
These are "am53c974", "dc390", "lsi53c895a", "lsi53c810", "megasas",
"megasas-gen2".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
hw/scsi/esp-pci.c | 30 +++++++++++-------------------
hw/scsi/lsi53c895a.c | 14 +++-----------
hw/scsi/megasas.c | 14 +++-----------
3 files changed, 17 insertions(+), 41 deletions(-)
diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index a75fcfa..8d2242d 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -28,7 +28,6 @@
#include "hw/scsi/esp.h"
#include "trace.h"
#include "qemu/log.h"
-#include "qapi/qmp/qerror.h"
#define TYPE_AM53C974_DEVICE "am53c974"
@@ -343,13 +342,12 @@ static const struct SCSIBusInfo esp_pci_scsi_info = {
.cancel = esp_request_cancelled,
};
-static int esp_pci_scsi_init(PCIDevice *dev)
+static void esp_pci_scsi_realize(PCIDevice *dev, Error **errp)
{
PCIESPState *pci = PCI_ESP(dev);
DeviceState *d = DEVICE(dev);
ESPState *s = &pci->esp;
uint8_t *pci_conf;
- Error *err = NULL;
pci_conf = dev->config;
@@ -368,14 +366,8 @@ static int esp_pci_scsi_init(PCIDevice *dev)
scsi_bus_new(&s->bus, sizeof(s->bus), d, &esp_pci_scsi_info, NULL);
if (!d->hotplugged) {
- scsi_bus_legacy_handle_cmdline(&s->bus, &err);
- if (err != NULL) {
- qerror_report_err(err);
- error_free(err);
- return -1;
- }
+ scsi_bus_legacy_handle_cmdline(&s->bus, errp);
}
- return 0;
}
static void esp_pci_scsi_uninit(PCIDevice *d)
@@ -390,7 +382,7 @@ static void esp_pci_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
- k->init = esp_pci_scsi_init;
+ k->realize = esp_pci_scsi_realize;
k->exit = esp_pci_scsi_uninit;
k->vendor_id = PCI_VENDOR_ID_AMD;
k->device_id = PCI_DEVICE_ID_AMD_SCSI;
@@ -468,17 +460,19 @@ static void dc390_write_config(PCIDevice *dev,
}
}
-static int dc390_scsi_init(PCIDevice *dev)
+static void dc390_scsi_realize(PCIDevice *dev, Error **errp)
{
DC390State *pci = DC390(dev);
+ Error *err = NULL;
uint8_t *contents;
uint16_t chksum = 0;
- int i, ret;
+ int i;
/* init base class */
- ret = esp_pci_scsi_init(dev);
- if (ret < 0) {
- return ret;
+ esp_pci_scsi_realize(dev, &err);
+ if (err) {
+ error_propagate(errp, err);
+ return;
}
/* EEPROM */
@@ -505,8 +499,6 @@ static int dc390_scsi_init(PCIDevice *dev)
chksum = 0x1234 - chksum;
contents[EE_CHKSUM1] = chksum & 0xff;
contents[EE_CHKSUM2] = chksum >> 8;
-
- return 0;
}
static void dc390_class_init(ObjectClass *klass, void *data)
@@ -514,7 +506,7 @@ static void dc390_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
- k->init = dc390_scsi_init;
+ k->realize = dc390_scsi_realize;
k->config_read = dc390_read_config;
k->config_write = dc390_write_config;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 4ffab03..c5b0cc5 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -19,7 +19,6 @@
#include "hw/pci/pci.h"
#include "hw/scsi/scsi.h"
#include "sysemu/dma.h"
-#include "qapi/qmp/qerror.h"
//#define DEBUG_LSI
//#define DEBUG_LSI_REG
@@ -2089,12 +2088,11 @@ static const struct SCSIBusInfo lsi_scsi_info = {
.cancel = lsi_request_cancelled
};
-static int lsi_scsi_init(PCIDevice *dev)
+static void lsi_scsi_realize(PCIDevice *dev, Error **errp)
{
LSIState *s = LSI53C895A(dev);
DeviceState *d = DEVICE(dev);
uint8_t *pci_conf;
- Error *err = NULL;
pci_conf = dev->config;
@@ -2117,14 +2115,8 @@ static int lsi_scsi_init(PCIDevice *dev)
scsi_bus_new(&s->bus, sizeof(s->bus), d, &lsi_scsi_info, NULL);
if (!d->hotplugged) {
- scsi_bus_legacy_handle_cmdline(&s->bus, &err);
- if (err != NULL) {
- qerror_report_err(err);
- error_free(err);
- return -1;
- }
+ scsi_bus_legacy_handle_cmdline(&s->bus, errp);
}
- return 0;
}
static void lsi_class_init(ObjectClass *klass, void *data)
@@ -2132,7 +2124,7 @@ static void lsi_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
- k->init = lsi_scsi_init;
+ k->realize = lsi_scsi_realize;
k->vendor_id = PCI_VENDOR_ID_LSI_LOGIC;
k->device_id = PCI_DEVICE_ID_LSI_53C895A;
k->class_id = PCI_CLASS_STORAGE_SCSI;
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 69ffdbd..bf83b65 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -28,7 +28,6 @@
#include "hw/scsi/scsi.h"
#include "block/scsi.h"
#include "trace.h"
-#include "qapi/qmp/qerror.h"
#include "mfi.h"
@@ -2321,14 +2320,13 @@ static const struct SCSIBusInfo megasas_scsi_info = {
.cancel = megasas_command_cancel,
};
-static int megasas_scsi_init(PCIDevice *dev)
+static void megasas_scsi_realize(PCIDevice *dev, Error **errp)
{
DeviceState *d = DEVICE(dev);
MegasasState *s = MEGASAS(dev);
MegasasBaseClass *b = MEGASAS_DEVICE_GET_CLASS(s);
uint8_t *pci_conf;
int i, bar_type;
- Error *err = NULL;
pci_conf = dev->config;
@@ -2408,14 +2406,8 @@ static int megasas_scsi_init(PCIDevice *dev)
scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(dev),
&megasas_scsi_info, NULL);
if (!d->hotplugged) {
- scsi_bus_legacy_handle_cmdline(&s->bus, &err);
- if (err != NULL) {
- qerror_report_err(err);
- error_free(err);
- return -1;
- }
+ scsi_bus_legacy_handle_cmdline(&s->bus, errp);
}
- return 0;
}
static void
@@ -2509,7 +2501,7 @@ static void megasas_class_init(ObjectClass *oc, void *data)
MegasasBaseClass *e = MEGASAS_DEVICE_CLASS(oc);
const MegasasInfo *info = data;
- pc->init = megasas_scsi_init;
+ pc->realize = megasas_scsi_realize;
pc->exit = megasas_scsi_uninit;
pc->vendor_id = PCI_VENDOR_ID_LSI_LOGIC;
pc->device_id = info->device_id;
--
1.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH v2 0/4] scsi: Error reporting cleanups, realize()
2015-03-09 18:17 [Qemu-devel] [PATCH v2 0/4] scsi: Error reporting cleanups, realize() Markus Armbruster
` (3 preceding siblings ...)
2015-03-09 18:17 ` [Qemu-devel] [PATCH v2 4/4] scsi: Convert remaining PCI HBAs to realize() Markus Armbruster
@ 2015-03-09 19:22 ` Paolo Bonzini
4 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2015-03-09 19:22 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: peter.crosthwaite, hare
On 09/03/2015 19:17, Markus Armbruster wrote:
> Applies cleanly on master now, and passes make check with my "[PATCH
> RFC 1/1] qtest: Add generic PCI device test" applied on top.
>
> v2:
> - Rebase: PATCH 01 loses the spapr_vscsi.c hunk, because commit
> 28b07e7 converted it to realize(). Commit message updated. PATCH
> 03 commit message updated to current error messages. R-bys
> retained, hope that's okay.
>
> Markus Armbruster (4):
> scsi: Clean up duplicated error in legacy if=scsi code
> hw: Propagate errors through qdev_prop_set_drive()
> scsi: Improve error reporting for invalid drive property
> scsi: Convert remaining PCI HBAs to realize()
>
> hw/arm/vexpress.c | 6 +++---
> hw/arm/virt.c | 6 +++---
> hw/block/pflash_cfi01.c | 4 ++--
> hw/block/pflash_cfi02.c | 4 ++--
> hw/core/qdev-properties-system.c | 22 ++++++++++------------
> hw/scsi/esp-pci.c | 28 +++++++++++-----------------
> hw/scsi/lsi53c895a.c | 13 +++----------
> hw/scsi/megasas.c | 12 +++---------
> hw/scsi/scsi-bus.c | 6 +++---
> hw/usb/dev-storage.c | 7 +++++--
> include/hw/qdev-properties.h | 4 ++--
> 11 files changed, 47 insertions(+), 65 deletions(-)
>
Applied, thanks.
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread