Re: [Qemu-devel] [PATCH v3 2/2] scsi-bus: Convert DeviceClass init to realize
On Tue, Aug 12, 2014 at 10:04:11AM +0800, Fam Zheng wrote: On Mon, 08/11 15:32, Stefan Hajnoczi wrote: On Mon, Aug 11, 2014 at 04:45:18PM +0800, Fam Zheng wrote: diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index d7b0f50..f6d9dc1 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -122,7 +122,7 @@ QEMU_PROG: -drive if=virtio: Device 'virtio-blk-pci' could not be initialized Testing: -drive if=scsi QEMU X.Y.Z monitor - type 'help' for more information -(qemu) QEMU_PROG: -drive if=scsi: Device needs media, but drive is empty +(qemu) QEMU_PROG: Device needs media, but drive is empty QEMU_PROG: Device initialization failed. QEMU_PROG: Initialization of device lsi53c895a failed Have you checked where error_report()'s location was set to -drive if=scsi? Maybe something similar can be implemented. Yes. In scsi_bus_legacy_handle_cmdline (callee of lsi, location is maintained until function return. Since its callers only use err for failure detection, but not for reporting (the specific information) to user, let's call error_report in scsi_bus_legacy_handle_cmdline. Another option is: void error_prepend(Error **dst_errp, Error *local_err, const char *fmt, ...); Examples: error_prepend(errp, NULL, -drive if=scsi) - NULL error_prepend(errp, initialization failed, -drive if=scsi) - -drive if=scsi: initialization failed So basically like error_propagate() except it also adds a prefix to the error message. pgpFVh79ov1sg.pgp Description: PGP signature
[Qemu-devel] [PATCH v3 2/2] scsi-bus: Convert DeviceClass init to realize
Replace init/destroy with realize/unrealize in SCSIDeviceClass,
which has errp as a parameter. So all the implementations now use
error_setg instead of error_report for reporting error.
Also in lsi53c895a, report the error when initializing the if=scsi
devices, before dropping it, because in the callee, error_report is
changed to error_setg.
Signed-off-by: Fam Zheng f...@redhat.com
---
hw/scsi/lsi53c895a.c | 2 ++
hw/scsi/scsi-bus.c | 69
hw/scsi/scsi-disk.c| 78 --
hw/scsi/scsi-generic.c | 37 +++---
include/hw/scsi/scsi.h | 7 +++--
tests/qemu-iotests/051.out | 4 +--
6 files changed, 99 insertions(+), 98 deletions(-)
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 786d848..dbc98a0 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -19,6 +19,7 @@
#include hw/pci/pci.h
#include hw/scsi/scsi.h
#include sysemu/dma.h
+#include qemu/error-report.h
//#define DEBUG_LSI
//#define DEBUG_LSI_REG
@@ -2121,6 +2122,7 @@ static int lsi_scsi_init(PCIDevice *dev)
if (!d-hotplugged) {
scsi_bus_legacy_handle_cmdline(s-bus, err);
if (err != NULL) {
+error_report(%s, error_get_pretty(err));
error_free(err);
return -1;
}
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 4341754..da1276e 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -37,20 +37,19 @@ static const TypeInfo scsi_bus_info = {
};
static int next_scsi_bus;
-static int scsi_device_init(SCSIDevice *s)
+static void scsi_device_realize(SCSIDevice *s, Error **errp)
{
SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(s);
-if (sc-init) {
-return sc-init(s);
+if (sc-realize) {
+sc-realize(s, errp);
}
-return 0;
}
-static void scsi_device_destroy(SCSIDevice *s)
+static void scsi_device_unrealize(SCSIDevice *s, Error **errp)
{
SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(s);
-if (sc-destroy) {
-sc-destroy(s);
+if (sc-unrealize) {
+sc-unrealize(s, errp);
}
}
@@ -130,24 +129,24 @@ static void scsi_dma_restart_cb(void *opaque, int
running, RunState state)
}
}
-static int scsi_qdev_init(DeviceState *qdev)
+static void scsi_qdev_realize(DeviceState *qdev, Error **errp)
{
SCSIDevice *dev = SCSI_DEVICE(qdev);
SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev-qdev.parent_bus);
SCSIDevice *d;
-int rc = -1;
+Error *local_err = NULL;
if (dev-channel bus-info-max_channel) {
-error_report(bad scsi channel id: %d, dev-channel);
-goto err;
+error_setg(errp, bad scsi channel id: %d, dev-channel);
+return;
}
if (dev-id != -1 dev-id bus-info-max_target) {
-error_report(bad scsi device id: %d, dev-id);
-goto err;
+error_setg(errp, bad scsi device id: %d, dev-id);
+return;
}
if (dev-lun != -1 dev-lun bus-info-max_lun) {
-error_report(bad scsi device lun: %d, dev-lun);
-goto err;
+error_setg(errp, bad scsi device lun: %d, dev-lun);
+return;
}
if (dev-id == -1) {
@@ -159,8 +158,8 @@ static int scsi_qdev_init(DeviceState *qdev)
d = scsi_device_find(bus, dev-channel, ++id, dev-lun);
} while (d d-lun == dev-lun id bus-info-max_target);
if (d d-lun == dev-lun) {
-error_report(no free target);
-goto err;
+error_setg(errp, no free target);
+return;
}
dev-id = id;
} else if (dev-lun == -1) {
@@ -169,43 +168,41 @@ static int scsi_qdev_init(DeviceState *qdev)
d = scsi_device_find(bus, dev-channel, dev-id, ++lun);
} while (d d-lun == lun lun bus-info-max_lun);
if (d d-lun == lun) {
-error_report(no free lun);
-goto err;
+error_setg(errp, no free lun);
+return;
}
dev-lun = lun;
} else {
d = scsi_device_find(bus, dev-channel, dev-id, dev-lun);
assert(d);
if (d-lun == dev-lun dev != d) {
-error_report(lun already used by '%s', d-qdev.id);
-goto err;
+error_setg(errp, lun already used by '%s', d-qdev.id);
+return;
}
}
QTAILQ_INIT(dev-requests);
-rc = scsi_device_init(dev);
-if (rc == 0) {
-dev-vmsentry = qemu_add_vm_change_state_handler(scsi_dma_restart_cb,
- dev);
+scsi_device_realize(dev, local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
}
+dev-vmsentry = qemu_add_vm_change_state_handler(scsi_dma_restart_cb,
+ dev);
if (bus-info-hotplug) {
bus-info-hotplug(bus, dev);
}
-
-err:
-return rc;
}
-static
Re: [Qemu-devel] [PATCH v3 2/2] scsi-bus: Convert DeviceClass init to realize
On Mon, Aug 11, 2014 at 04:45:18PM +0800, Fam Zheng wrote: diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index d7b0f50..f6d9dc1 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -122,7 +122,7 @@ QEMU_PROG: -drive if=virtio: Device 'virtio-blk-pci' could not be initialized Testing: -drive if=scsi QEMU X.Y.Z monitor - type 'help' for more information -(qemu) QEMU_PROG: -drive if=scsi: Device needs media, but drive is empty +(qemu) QEMU_PROG: Device needs media, but drive is empty QEMU_PROG: Device initialization failed. QEMU_PROG: Initialization of device lsi53c895a failed Have you checked where error_report()'s location was set to -drive if=scsi? Maybe something similar can be implemented. @@ -149,13 +149,11 @@ QEMU_PROG: -device ide-hd,drive=disk: Device 'ide-hd' could not be initialized Testing: -drive if=none,id=disk -device lsi53c895a -device scsi-disk,drive=disk QEMU X.Y.Z monitor - type 'help' for more information (qemu) QEMU_PROG: -device scsi-disk,drive=disk: Device needs media, but drive is empty -QEMU_PROG: -device scsi-disk,drive=disk: Device initialization failed. QEMU_PROG: -device scsi-disk,drive=disk: Device 'scsi-disk' could not be initialized Testing: -drive if=none,id=disk -device lsi53c895a -device scsi-hd,drive=disk QEMU X.Y.Z monitor - type 'help' for more information (qemu) QEMU_PROG: -device scsi-hd,drive=disk: Device needs media, but drive is empty -QEMU_PROG: -device scsi-hd,drive=disk: Device initialization failed. QEMU_PROG: -device scsi-hd,drive=disk: Device 'scsi-hd' could not be initialized Not a big loss, I found the generic Device initialization failed followed by a specific error message pointless. After this patch we only get the specific error message. I'm happy with that. pgpielmYpMHu8.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v3 2/2] scsi-bus: Convert DeviceClass init to realize
On Mon, 08/11 15:32, Stefan Hajnoczi wrote: On Mon, Aug 11, 2014 at 04:45:18PM +0800, Fam Zheng wrote: diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index d7b0f50..f6d9dc1 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -122,7 +122,7 @@ QEMU_PROG: -drive if=virtio: Device 'virtio-blk-pci' could not be initialized Testing: -drive if=scsi QEMU X.Y.Z monitor - type 'help' for more information -(qemu) QEMU_PROG: -drive if=scsi: Device needs media, but drive is empty +(qemu) QEMU_PROG: Device needs media, but drive is empty QEMU_PROG: Device initialization failed. QEMU_PROG: Initialization of device lsi53c895a failed Have you checked where error_report()'s location was set to -drive if=scsi? Maybe something similar can be implemented. Yes. In scsi_bus_legacy_handle_cmdline (callee of lsi, location is maintained until function return. Since its callers only use err for failure detection, but not for reporting (the specific information) to user, let's call error_report in scsi_bus_legacy_handle_cmdline. Thanks, Fam
