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