Re: [Qemu-devel] [PATCH 2/2] scsi-bus: Convert DeviceClass init to realize

2014-08-11 Thread Fam Zheng 
On Thu, 08/07 16:25, Kevin Wolf wrote:
 Am 05.08.2014 um 11:11 hat Fam Zheng geschrieben:
  Replace init/destroy with realize/unrealize in SCSIDeviceClass,
  which has errp as a parameter. So all the implementations now uses
  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 the callee's error_report is
  changed to error_segs.
  
  Signed-off-by: Fam Zheng f...@redhat.com
  ---
   hw/scsi/lsi53c895a.c   |  2 ++
   hw/scsi/scsi-bus.c | 64 ++---
   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, 96 insertions(+), 96 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;
   }
 
 Wouldn't qerror_report_err() be more useful? Or is already a QMP error
 emitted in a different place in the callchain?

This code block is specifically for command line, see above if
(!d-hotplugged) condition check and scsi_bus_legacy_handle_cmdline(s-bus,
err); call. So I think error_report is good enough.

 
 The same question is true for the added error_report() calls in patch 1.

Two error_report() added in patch 1: the first is consistent with the other
error_report() in the context function; the second is replaced by error_setg in
this patch when errp is added.

 
  @@ -169,43 +168,40 @@ 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) {
  +scsi_device_realize(dev, local_err);
  +if (local_err) {
   dev-vmsentry = 
  qemu_add_vm_change_state_handler(scsi_dma_restart_cb,
dev);
  +error_propagate(errp, local_err);
   }
 
 Maybe I'm misunderstanding something, but it looks to me as if the
 handler was previously installed in case of success, and now it's only
 installed on failure?

Good catch! Will fix.

Thanks,
Fam

Re: [Qemu-devel] [PATCH 2/2] scsi-bus: Convert DeviceClass init to realize

2014-08-07 Thread Kevin Wolf 
Am 05.08.2014 um 11:11 hat Fam Zheng geschrieben:
 Replace init/destroy with realize/unrealize in SCSIDeviceClass,
 which has errp as a parameter. So all the implementations now uses
 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 the callee's error_report is
 changed to error_segs.
 
 Signed-off-by: Fam Zheng f...@redhat.com
 ---
  hw/scsi/lsi53c895a.c   |  2 ++
  hw/scsi/scsi-bus.c | 64 ++---
  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, 96 insertions(+), 96 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;
  }

Wouldn't qerror_report_err() be more useful? Or is already a QMP error
emitted in a different place in the callchain?

The same question is true for the added error_report() calls in patch 1.

 @@ -169,43 +168,40 @@ 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) {
 +scsi_device_realize(dev, local_err);
 +if (local_err) {
  dev-vmsentry = qemu_add_vm_change_state_handler(scsi_dma_restart_cb,
   dev);
 +error_propagate(errp, local_err);
  }

Maybe I'm misunderstanding something, but it looks to me as if the
handler was previously installed in case of success, and now it's only
installed on failure?

  
  if (bus-info-hotplug) {
  bus-info-hotplug(bus, dev);
  }
 -
 -err:
 -return rc;
  }

 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

The old error message was certainly more useful. Not sure if there's a
good way to retain it, though.

Kevin

[Qemu-devel] [PATCH 2/2] scsi-bus: Convert DeviceClass init to realize

2014-08-05 Thread Fam Zheng 
Replace init/destroy with realize/unrealize in SCSIDeviceClass,
which has errp as a parameter. So all the implementations now uses
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 the callee's error_report is
changed to error_segs.

Signed-off-by: Fam Zheng f...@redhat.com
---
 hw/scsi/lsi53c895a.c   |  2 ++
 hw/scsi/scsi-bus.c | 64 ++---
 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, 96 insertions(+), 96 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..85d1287 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,40 @@ 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) {
+scsi_device_realize(dev, local_err);
+if (local_err) {
 dev-vmsentry = qemu_add_vm_change_state_handler(scsi_dma_restart_cb,
  dev);
+error_propagate(errp, local_err);
 }
 
 if (bus-info-hotplug) {
 bus-info-hotplug(bus, dev);
 }
-
-err:
-return rc;
 }
 
-static int scsi_qdev_exit(DeviceState *qdev)
+static void scsi_qdev_unrealize(DeviceState *qdev, Error **errp)
 {
 SCSIDevice *dev = SCSI_DEVICE(qdev);

Re: [Qemu-devel] [PATCH 2/2] scsi-bus: Convert DeviceClass init to realize

2014-08-05 Thread Fam Zheng 
On Tue, 08/05 17:11, Fam Zheng wrote:
 Replace init/destroy with realize/unrealize in SCSIDeviceClass,
 which has errp as a parameter. So all the implementations now uses

s/uses/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 the callee's error_report is
 changed to error_segs.

s/error_segs/error_setg/

Fam