Re: [Qemu-devel] [PATCH qom-next] scsi: Improve error propagation for scsi_bus_legacy_handle_cmdline()

[Andreas Färber]

Am 22.07.2013 11:16, schrieb Hu Tao:
> On Sun, Jul 21, 2013 at 12:30:54PM +0200, Andreas Färber wrote:
>> Let scsi_bus_legacy_add_drive() and scsi_bus_legacy_handle_cmdline()
>> return an Error**. Prepare qdev initfns for QOM realize error model.
>>
>> Signed-off-by: Andreas Färber <address@hidden>
>> ---
>>  hw/pci/pci-hotplug-old.c |  2 +-
>>  hw/scsi/esp-pci.c        |  7 ++++++-
>>  hw/scsi/esp.c            |  6 ++++--
>>  hw/scsi/lsi53c895a.c     |  7 ++++++-
>>  hw/scsi/megasas.c        |  7 ++++++-
>>  hw/scsi/scsi-bus.c       | 22 +++++++++++++++-------
>>  hw/scsi/spapr_vscsi.c    |  7 ++++++-
>>  hw/scsi/virtio-scsi.c    |  7 ++++++-
>>  hw/usb/dev-storage.c     |  4 +++-
>>  include/hw/scsi/scsi.h   |  4 ++--
>>  10 files changed, 55 insertions(+), 18 deletions(-)
>>
>> diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c
>> index 8077289..619fe47 100644
>> --- a/hw/pci/pci-hotplug-old.c
>> +++ b/hw/pci/pci-hotplug-old.c
>> @@ -127,7 +127,7 @@ static int scsi_hot_add(Monitor *mon, DeviceState 
>> *adapter,
>>      dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
>>      dinfo->bus = scsibus->busnr;
>>      scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo->bdrv, dinfo->unit,
>> -                                        false, -1, NULL);
>> +                                        false, -1, NULL, NULL);
>>      if (!scsidev) {
>>          return -1;
>>      }
>> diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
>> index 6cdfd53..2ac21d4 100644
>> --- a/hw/scsi/esp-pci.c
>> +++ b/hw/scsi/esp-pci.c
>> @@ -346,6 +346,7 @@ static int esp_pci_scsi_init(PCIDevice *dev)
>>      DeviceState *d = DEVICE(dev);
>>      ESPState *s = &pci->esp;
>>      uint8_t *pci_conf;
>> +    Error *err = NULL;
>>  
>>      pci_conf = dev->config;
>>  
>> @@ -364,7 +365,11 @@ static int esp_pci_scsi_init(PCIDevice *dev)
>>  
>>      scsi_bus_new(&s->bus, d, &esp_pci_scsi_info, NULL);
>>      if (!d->hotplugged) {
>> -        return scsi_bus_legacy_handle_cmdline(&s->bus);
>> +        scsi_bus_legacy_handle_cmdline(&s->bus, &err);
>> +        if (err != NULL) {
> 
>            if (error_is_set(&err))

I was specifically instructed not to use error_is_set() on local err
variables. It is intended for potentially-NULL errp only, which &err
will never be.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg