From 5ff5efb46c4526f111e14c2247609f1c56f0f8f3 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 12 Aug 2014 10:12:54 +0800 Subject: [PATCH 1/8] block: Pass errp in blkconf_geometry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows us to pass error information to caller. Reviewed-by: Andreas Färber Reviewed-by: Stefan Hajnoczi Signed-off-by: Fam Zheng Signed-off-by: Paolo Bonzini --- hw/block/block.c | 18 +++++++++--------- hw/block/virtio-blk.c | 7 +++---- hw/ide/qdev.c | 11 ++++++++--- hw/scsi/scsi-disk.c | 11 ++++++++--- include/hw/block/block.h | 6 ++++-- 5 files changed, 32 insertions(+), 21 deletions(-) diff --git a/hw/block/block.c b/hw/block/block.c index 33dd3f33b6..b6a6dc6baa 100644 --- a/hw/block/block.c +++ b/hw/block/block.c @@ -22,8 +22,9 @@ void blkconf_serial(BlockConf *conf, char **serial) } } -int blkconf_geometry(BlockConf *conf, int *ptrans, - unsigned cyls_max, unsigned heads_max, unsigned secs_max) +void blkconf_geometry(BlockConf *conf, int *ptrans, + unsigned cyls_max, unsigned heads_max, unsigned secs_max, + Error **errp) { DriveInfo *dinfo; @@ -46,17 +47,16 @@ int blkconf_geometry(BlockConf *conf, int *ptrans, } if (conf->cyls || conf->heads || conf->secs) { if (conf->cyls < 1 || conf->cyls > cyls_max) { - error_report("cyls must be between 1 and %u", cyls_max); - return -1; + error_setg(errp, "cyls must be between 1 and %u", cyls_max); + return; } if (conf->heads < 1 || conf->heads > heads_max) { - error_report("heads must be between 1 and %u", heads_max); - return -1; + error_setg(errp, "heads must be between 1 and %u", heads_max); + return; } if (conf->secs < 1 || conf->secs > secs_max) { - error_report("secs must be between 1 and %u", secs_max); - return -1; + error_setg(errp, "secs must be between 1 and %u", secs_max); + return; } } - return 0; } diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 302c39e2be..0b68a1781d 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -728,9 +728,7 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) VirtIODevice *vdev = VIRTIO_DEVICE(dev); VirtIOBlock *s = VIRTIO_BLK(dev); VirtIOBlkConf *blk = &(s->blk); -#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE Error *err = NULL; -#endif static int virtio_blk_id; if (!blk->conf.bs) { @@ -744,8 +742,9 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) blkconf_serial(&blk->conf, &blk->serial); s->original_wce = bdrv_enable_write_cache(blk->conf.bs); - if (blkconf_geometry(&blk->conf, NULL, 65535, 255, 255) < 0) { - error_setg(errp, "Error setting geometry"); + blkconf_geometry(&blk->conf, NULL, 65535, 255, 255, &err); + if (err) { + error_propagate(errp, err); return; } diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index 6e475e6970..b4a467116e 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -151,6 +151,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) { IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus); IDEState *s = bus->ifs + dev->unit; + Error *err = NULL; if (dev->conf.discard_granularity == -1) { dev->conf.discard_granularity = 512; @@ -161,9 +162,13 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) } blkconf_serial(&dev->conf, &dev->serial); - if (kind != IDE_CD - && blkconf_geometry(&dev->conf, &dev->chs_trans, 65536, 16, 255) < 0) { - return -1; + if (kind != IDE_CD) { + blkconf_geometry(&dev->conf, &dev->chs_trans, 65536, 16, 255, &err); + if (err) { + error_report("%s", error_get_pretty(err)); + error_free(err); + return -1; + } } if (ide_init_drive(s, dev->conf.bs, kind, diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index d55521dcf6..b7ebdd766a 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2237,6 +2237,7 @@ static void scsi_disk_unit_attention_reported(SCSIDevice *dev) static int scsi_initfn(SCSIDevice *dev) { SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev); + Error *err = NULL; if (!s->qdev.conf.bs) { error_report("drive property not set"); @@ -2250,9 +2251,13 @@ static int scsi_initfn(SCSIDevice *dev) } blkconf_serial(&s->qdev.conf, &s->serial); - if (dev->type == TYPE_DISK - && blkconf_geometry(&dev->conf, NULL, 65535, 255, 255) < 0) { - return -1; + if (dev->type == TYPE_DISK) { + blkconf_geometry(&dev->conf, NULL, 65535, 255, 255, &err); + if (err) { + error_report("%s", error_get_pretty(err)); + error_free(err); + return -1; + } } if (s->qdev.conf.discard_granularity == -1) { diff --git a/include/hw/block/block.h b/include/hw/block/block.h index 7c3d6c8178..3a0148848b 100644 --- a/include/hw/block/block.h +++ b/include/hw/block/block.h @@ -12,6 +12,7 @@ #define HW_BLOCK_COMMON_H #include "qemu-common.h" +#include "qapi/error.h" /* Configuration */ @@ -60,8 +61,9 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) /* Configuration helpers */ void blkconf_serial(BlockConf *conf, char **serial); -int blkconf_geometry(BlockConf *conf, int *trans, - unsigned cyls_max, unsigned heads_max, unsigned secs_max); +void blkconf_geometry(BlockConf *conf, int *trans, + unsigned cyls_max, unsigned heads_max, unsigned secs_max, + Error **errp); /* Hard disk geometry */ From a818a4b69d47ca3826dee36878074395aeac2083 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 12 Aug 2014 10:12:55 +0800 Subject: [PATCH 2/8] 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 scsi_bus_legacy_handle_cmdline, report the error when initializing the if=scsi devices, before returning it, because in the callee, error_report is changed to error_setg. And the callers don't have the right locations (e.g. "-drive if=scsi"). Reviewed-by: Stefan Hajnoczi Signed-off-by: Fam Zheng Signed-off-by: Paolo Bonzini --- hw/scsi/lsi53c895a.c | 1 + hw/scsi/scsi-bus.c | 70 +++++++++++++++++----------------- hw/scsi/scsi-disk.c | 78 +++++++++++++++++++------------------- hw/scsi/scsi-generic.c | 37 +++++++++--------- include/hw/scsi/scsi.h | 4 +- tests/qemu-iotests/051.out | 2 - 6 files changed, 95 insertions(+), 97 deletions(-) diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c index 513ea47e8a..d9b4c7ea3c 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 diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index 6f4462b483..954c6072bf 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -36,20 +36,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); } } @@ -143,24 +142,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) { @@ -172,8 +171,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) { @@ -182,43 +181,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 int scsi_qdev_exit(DeviceState *qdev) +static void scsi_qdev_unrealize(DeviceState *qdev, Error **errp) { SCSIDevice *dev = SCSI_DEVICE(qdev); if (dev->vmsentry) { qemu_del_vm_change_state_handler(dev->vmsentry); } - scsi_device_destroy(dev); - return 0; + scsi_device_unrealize(dev, errp); } /* handle legacy '-drive if=scsi,...' cmd line args */ @@ -273,6 +270,7 @@ void scsi_bus_legacy_handle_cmdline(SCSIBus *bus, Error **errp) scsi_bus_legacy_add_drive(bus, dinfo->bdrv, unit, false, -1, NULL, &err); if (err != NULL) { + error_report("%s", error_get_pretty(err)); error_propagate(errp, err); break; } @@ -1992,11 +1990,11 @@ static void scsi_device_class_init(ObjectClass *klass, void *data) { DeviceClass *k = DEVICE_CLASS(klass); set_bit(DEVICE_CATEGORY_STORAGE, k->categories); - k->bus_type = TYPE_SCSI_BUS; - k->init = scsi_qdev_init; - k->unplug = scsi_qdev_unplug; - k->exit = scsi_qdev_exit; - k->props = scsi_props; + k->bus_type = TYPE_SCSI_BUS; + k->realize = scsi_qdev_realize; + k->unplug = scsi_qdev_unplug; + k->unrealize = scsi_qdev_unrealize; + k->props = scsi_props; } static const TypeInfo scsi_device_type_info = { diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index b7ebdd766a..e34a54404b 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2151,7 +2151,7 @@ static void scsi_disk_reset(DeviceState *dev) s->tray_open = 0; } -static void scsi_destroy(SCSIDevice *dev) +static void scsi_unrealize(SCSIDevice *dev, Error **errp) { SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev); @@ -2234,29 +2234,28 @@ static void scsi_disk_unit_attention_reported(SCSIDevice *dev) } } -static int scsi_initfn(SCSIDevice *dev) +static void scsi_realize(SCSIDevice *dev, Error **errp) { SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev); Error *err = NULL; if (!s->qdev.conf.bs) { - error_report("drive property not set"); - return -1; + error_setg(errp, "drive property not set"); + return; } if (!(s->features & (1 << SCSI_DISK_F_REMOVABLE)) && !bdrv_is_inserted(s->qdev.conf.bs)) { - error_report("Device needs media, but drive is empty"); - return -1; + error_setg(errp, "Device needs media, but drive is empty"); + return; } blkconf_serial(&s->qdev.conf, &s->serial); if (dev->type == TYPE_DISK) { blkconf_geometry(&dev->conf, NULL, 65535, 255, 255, &err); if (err) { - error_report("%s", error_get_pretty(err)); - error_free(err); - return -1; + error_propagate(errp, err); + return; } } @@ -2273,8 +2272,8 @@ static int scsi_initfn(SCSIDevice *dev) } if (bdrv_is_sg(s->qdev.conf.bs)) { - error_report("unwanted /dev/sg*"); - return -1; + error_setg(errp, "unwanted /dev/sg*"); + return; } if ((s->features & (1 << SCSI_DISK_F_REMOVABLE)) && @@ -2287,10 +2286,9 @@ static int scsi_initfn(SCSIDevice *dev) bdrv_iostatus_enable(s->qdev.conf.bs); add_boot_device_path(s->qdev.conf.bootindex, &dev->qdev, NULL); - return 0; } -static int scsi_hd_initfn(SCSIDevice *dev) +static void scsi_hd_realize(SCSIDevice *dev, Error **errp) { SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev); s->qdev.blocksize = s->qdev.conf.logical_block_size; @@ -2298,10 +2296,10 @@ static int scsi_hd_initfn(SCSIDevice *dev) if (!s->product) { s->product = g_strdup("QEMU HARDDISK"); } - return scsi_initfn(&s->qdev); + scsi_realize(&s->qdev, errp); } -static int scsi_cd_initfn(SCSIDevice *dev) +static void scsi_cd_realize(SCSIDevice *dev, Error **errp) { SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev); s->qdev.blocksize = 2048; @@ -2310,22 +2308,26 @@ static int scsi_cd_initfn(SCSIDevice *dev) if (!s->product) { s->product = g_strdup("QEMU CD-ROM"); } - return scsi_initfn(&s->qdev); + scsi_realize(&s->qdev, errp); } -static int scsi_disk_initfn(SCSIDevice *dev) +static void scsi_disk_realize(SCSIDevice *dev, Error **errp) { DriveInfo *dinfo; + Error *local_err = NULL; if (!dev->conf.bs) { - return scsi_initfn(dev); /* ... and die there */ + scsi_realize(dev, &local_err); + assert(local_err); + error_propagate(errp, local_err); + return; } dinfo = drive_get_by_blockdev(dev->conf.bs); if (dinfo->media_cd) { - return scsi_cd_initfn(dev); + scsi_cd_realize(dev, errp); } else { - return scsi_hd_initfn(dev); + scsi_hd_realize(dev, errp); } } @@ -2457,35 +2459,35 @@ static int get_device_type(SCSIDiskState *s) return 0; } -static int scsi_block_initfn(SCSIDevice *dev) +static void scsi_block_realize(SCSIDevice *dev, Error **errp) { SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev); int sg_version; int rc; if (!s->qdev.conf.bs) { - error_report("drive property not set"); - return -1; + error_setg(errp, "drive property not set"); + return; } /* check we are using a driver managing SG_IO (version 3 and after) */ rc = bdrv_ioctl(s->qdev.conf.bs, SG_GET_VERSION_NUM, &sg_version); if (rc < 0) { - error_report("cannot get SG_IO version number: %s. " + error_setg(errp, "cannot get SG_IO version number: %s. " "Is this a SCSI device?", strerror(-rc)); - return -1; + return; } if (sg_version < 30000) { - error_report("scsi generic interface too old"); - return -1; + error_setg(errp, "scsi generic interface too old"); + return; } /* get device type from INQUIRY data */ rc = get_device_type(s); if (rc < 0) { - error_report("INQUIRY failed"); - return -1; + error_setg(errp, "INQUIRY failed"); + return; } /* Make a guess for the block size, we'll fix it when the guest sends. @@ -2503,7 +2505,7 @@ static int scsi_block_initfn(SCSIDevice *dev) */ s->features |= (1 << SCSI_DISK_F_NO_REMOVABLE_DEVOPS); - return scsi_initfn(&s->qdev); + scsi_realize(&s->qdev, errp); } static bool scsi_block_is_passthrough(SCSIDiskState *s, uint8_t *buf) @@ -2626,8 +2628,8 @@ static void scsi_hd_class_initfn(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); SCSIDeviceClass *sc = SCSI_DEVICE_CLASS(klass); - sc->init = scsi_hd_initfn; - sc->destroy = scsi_destroy; + sc->realize = scsi_hd_realize; + sc->unrealize = scsi_unrealize; sc->alloc_req = scsi_new_request; sc->unit_attention_reported = scsi_disk_unit_attention_reported; dc->fw_name = "disk"; @@ -2657,8 +2659,8 @@ static void scsi_cd_class_initfn(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); SCSIDeviceClass *sc = SCSI_DEVICE_CLASS(klass); - sc->init = scsi_cd_initfn; - sc->destroy = scsi_destroy; + sc->realize = scsi_cd_realize; + sc->unrealize = scsi_unrealize; sc->alloc_req = scsi_new_request; sc->unit_attention_reported = scsi_disk_unit_attention_reported; dc->fw_name = "disk"; @@ -2687,8 +2689,8 @@ static void scsi_block_class_initfn(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); SCSIDeviceClass *sc = SCSI_DEVICE_CLASS(klass); - sc->init = scsi_block_initfn; - sc->destroy = scsi_destroy; + sc->realize = scsi_block_realize; + sc->unrealize = scsi_unrealize; sc->alloc_req = scsi_block_new_request; sc->parse_cdb = scsi_block_parse_cdb; dc->fw_name = "disk"; @@ -2725,8 +2727,8 @@ static void scsi_disk_class_initfn(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); SCSIDeviceClass *sc = SCSI_DEVICE_CLASS(klass); - sc->init = scsi_disk_initfn; - sc->destroy = scsi_destroy; + sc->realize = scsi_disk_realize; + sc->unrealize = scsi_unrealize; sc->alloc_req = scsi_new_request; sc->unit_attention_reported = scsi_disk_unit_attention_reported; dc->fw_name = "disk"; diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c index 0b2ff90b90..3ebe4a6a81 100644 --- a/hw/scsi/scsi-generic.c +++ b/hw/scsi/scsi-generic.c @@ -386,49 +386,49 @@ static void scsi_generic_reset(DeviceState *dev) scsi_device_purge_requests(s, SENSE_CODE(RESET)); } -static void scsi_destroy(SCSIDevice *s) +static void scsi_unrealize(SCSIDevice *s, Error **errp) { scsi_device_purge_requests(s, SENSE_CODE(NO_SENSE)); blockdev_mark_auto_del(s->conf.bs); } -static int scsi_generic_initfn(SCSIDevice *s) +static void scsi_generic_realize(SCSIDevice *s, Error **errp) { int rc; int sg_version; struct sg_scsi_id scsiid; if (!s->conf.bs) { - error_report("drive property not set"); - return -1; + error_setg(errp, "drive property not set"); + return; } if (bdrv_get_on_error(s->conf.bs, 0) != BLOCKDEV_ON_ERROR_ENOSPC) { - error_report("Device doesn't support drive option werror"); - return -1; + error_setg(errp, "Device doesn't support drive option werror"); + return; } if (bdrv_get_on_error(s->conf.bs, 1) != BLOCKDEV_ON_ERROR_REPORT) { - error_report("Device doesn't support drive option rerror"); - return -1; + error_setg(errp, "Device doesn't support drive option rerror"); + return; } /* check we are using a driver managing SG_IO (version 3 and after */ rc = bdrv_ioctl(s->conf.bs, SG_GET_VERSION_NUM, &sg_version); if (rc < 0) { - error_report("cannot get SG_IO version number: %s. " - "Is this a SCSI device?", - strerror(-rc)); - return -1; + error_setg(errp, "cannot get SG_IO version number: %s. " + "Is this a SCSI device?", + strerror(-rc)); + return; } if (sg_version < 30000) { - error_report("scsi generic interface too old"); - return -1; + error_setg(errp, "scsi generic interface too old"); + return; } /* get LUN of the /dev/sg? */ if (bdrv_ioctl(s->conf.bs, SG_GET_SCSI_ID, &scsiid)) { - error_report("SG_GET_SCSI_ID ioctl failed"); - return -1; + error_setg(errp, "SG_GET_SCSI_ID ioctl failed"); + return; } /* define device state */ @@ -460,7 +460,6 @@ static int scsi_generic_initfn(SCSIDevice *s) } DPRINTF("block size %d\n", s->blocksize); - return 0; } const SCSIReqOps scsi_generic_req_ops = { @@ -501,8 +500,8 @@ static void scsi_generic_class_initfn(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); SCSIDeviceClass *sc = SCSI_DEVICE_CLASS(klass); - sc->init = scsi_generic_initfn; - sc->destroy = scsi_destroy; + sc->realize = scsi_generic_realize; + sc->unrealize = scsi_unrealize; sc->alloc_req = scsi_new_request; sc->parse_cdb = scsi_generic_parse_cdb; dc->fw_name = "disk"; diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h index a7a28e6bbd..2e3a8f987d 100644 --- a/include/hw/scsi/scsi.h +++ b/include/hw/scsi/scsi.h @@ -74,8 +74,8 @@ struct SCSIRequest { typedef struct SCSIDeviceClass { DeviceClass parent_class; - int (*init)(SCSIDevice *dev); - void (*destroy)(SCSIDevice *s); + void (*realize)(SCSIDevice *dev, Error **errp); + void (*unrealize)(SCSIDevice *dev, Error **errp); int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf, void *hba_private); SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun, diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index d7b0f503af..a3f28209c8 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -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 From 9db693f76441e2fc7e1b05dc454e7db4d3298dcb Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Fri, 22 Aug 2014 10:08:49 +0200 Subject: [PATCH 3/8] block/iscsi: fix memory corruption on iscsi resize bs->total_sectors is not yet updated at this point. resulting in memory corruption if the volume has grown and data is written to the newly availble areas. CC: qemu-stable@nongnu.org Signed-off-by: Peter Lieven Signed-off-by: Paolo Bonzini --- block/iscsi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/block/iscsi.c b/block/iscsi.c index 2c9cfc18eb..0bde13dc75 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -1512,7 +1512,8 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t offset) if (iscsilun->allocationmap != NULL) { g_free(iscsilun->allocationmap); iscsilun->allocationmap = - bitmap_new(DIV_ROUND_UP(bs->total_sectors, + bitmap_new(DIV_ROUND_UP(sector_lun2qemu(iscsilun->num_blocks, + iscsilun), iscsilun->cluster_sectors)); } From f93d2c15d603b811c877efbf2a47e418bf975f0c Mon Sep 17 00:00:00 2001 From: Gonglei Date: Fri, 22 Aug 2014 10:01:50 +0800 Subject: [PATCH 4/8] scsi-generic: remove superfluous DPRINTF avoid to break compiling variables lun and tag had been eliminated, break compiling when enable debug switch. Meanwhile traces provide the same information with this DPRINTF, so remove it. Signed-off-by: Gonglei Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-generic.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c index 3ebe4a6a81..20587b41c1 100644 --- a/hw/scsi/scsi-generic.c +++ b/hw/scsi/scsi-generic.c @@ -303,9 +303,6 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd) SCSIDevice *s = r->req.dev; int ret; - DPRINTF("Command: lun=%d tag=0x%x len %zd data=0x%02x", lun, tag, - r->req.cmd.xfer, cmd[0]); - #ifdef DEBUG_SCSI { int i; From c9f6552803d852d593dec9ef5bb39b7a61091963 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 26 Aug 2014 14:30:30 +0800 Subject: [PATCH 5/8] virtio-scsi: Report error if num_queues is 0 or too large No cmd vq surprises guest (Linux panics in virtscsi_probe), too many queues abort qemu (in the following virtio_add_queue). Signed-off-by: Fam Zheng Signed-off-by: Paolo Bonzini --- hw/scsi/virtio-scsi.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 2dd92552e0..86aba8851d 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -699,6 +699,12 @@ void virtio_scsi_common_realize(DeviceState *dev, Error **errp, virtio_init(vdev, "virtio-scsi", VIRTIO_ID_SCSI, sizeof(VirtIOSCSIConfig)); + if (s->conf.num_queues <= 0 || s->conf.num_queues > VIRTIO_PCI_QUEUE_MAX) { + error_setg(errp, "Invalid number of queues (= %" PRId32 "), " + "must be a positive integer less than %d.", + s->conf.num_queues, VIRTIO_PCI_QUEUE_MAX); + return; + } s->cmd_vqs = g_malloc0(s->conf.num_queues * sizeof(VirtQueue *)); s->sense_size = VIRTIO_SCSI_SENSE_SIZE; s->cdb_size = VIRTIO_SCSI_CDB_SIZE; From dc6c4fe8378b2e94bcf2ee2efa62ed0aba9aa5cc Mon Sep 17 00:00:00 2001 From: Peter Crosthwaite Date: Mon, 25 Aug 2014 20:09:13 -0700 Subject: [PATCH 6/8] xen-hvm: Constify string It's constant, and sourced from existing const strings. Avoid dodgy casts by converting to const. Signed-off-by: Peter Crosthwaite Reviewed-by: Stefan Weil Signed-off-by: Paolo Bonzini --- xen-hvm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xen-hvm.c b/xen-hvm.c index 91de2e230b..d763e86f24 100644 --- a/xen-hvm.c +++ b/xen-hvm.c @@ -71,7 +71,7 @@ static inline ioreq_t *xen_vcpu_ioreq(shared_iopage_t *shared_page, int vcpu) typedef struct XenPhysmap { hwaddr start_addr; ram_addr_t size; - char *name; + const char *name; hwaddr phys_offset; QLIST_ENTRY(XenPhysmap) list; @@ -330,7 +330,7 @@ go_physmap: physmap->start_addr = start_addr; physmap->size = size; - physmap->name = (char *)mr->name; + physmap->name = mr->name; physmap->phys_offset = phys_offset; QLIST_INSERT_HEAD(&state->physmap, physmap, list); From 3e1f50867b6872130cc19b7eadd93ab9ce268cdc Mon Sep 17 00:00:00 2001 From: Peter Crosthwaite Date: Mon, 25 Aug 2014 20:09:48 -0700 Subject: [PATCH 7/8] xen: hvm: Abstract away memory region name ref The mr->name field is removed. This slipped through compile testing. Fix. Reviewed-by: Stefan Weil Signed-off-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- xen-hvm.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/xen-hvm.c b/xen-hvm.c index d763e86f24..0d09940111 100644 --- a/xen-hvm.c +++ b/xen-hvm.c @@ -291,6 +291,7 @@ static int xen_add_to_physmap(XenIOState *state, hwaddr pfn, start_gpfn; hwaddr phys_offset = memory_region_get_ram_addr(mr); char path[80], value[17]; + const char *mr_name; if (get_physmapping(state, start_addr, size)) { return 0; @@ -326,11 +327,13 @@ go_physmap: } } + mr_name = memory_region_name(mr); + physmap = g_malloc(sizeof (XenPhysmap)); physmap->start_addr = start_addr; physmap->size = size; - physmap->name = mr->name; + physmap->name = mr_name; physmap->phys_offset = phys_offset; QLIST_INSERT_HEAD(&state->physmap, physmap, list); @@ -354,11 +357,11 @@ go_physmap: if (!xs_write(state->xenstore, 0, path, value, strlen(value))) { return -1; } - if (mr->name) { + if (mr_name) { snprintf(path, sizeof(path), "/local/domain/0/device-model/%d/physmap/%"PRIx64"/name", xen_domid, (uint64_t)phys_offset); - if (!xs_write(state->xenstore, 0, path, mr->name, strlen(mr->name))) { + if (!xs_write(state->xenstore, 0, path, mr_name, strlen(mr_name))) { return -1; } } From d1dd32af6f37e5bb8e6b2024d07fce74f510a668 Mon Sep 17 00:00:00 2001 From: Peter Crosthwaite Date: Mon, 25 Aug 2014 20:10:24 -0700 Subject: [PATCH 8/8] memory: Lazy init name from QOM name as needed To support name retrieval of MemoryRegions that were created dynamically (that is, not via memory_region_init and friends). We cache the name in MemoryRegion's state as object_get_canonical_path_component mallocs the returned value so it's not suitable for direct return to callers. Memory already frees the name field, so this will be garbage collected along with the MR object. Signed-off-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- memory.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/memory.c b/memory.c index 42317a216d..ef0be1c3aa 100644 --- a/memory.c +++ b/memory.c @@ -1309,6 +1309,10 @@ uint64_t memory_region_size(MemoryRegion *mr) const char *memory_region_name(const MemoryRegion *mr) { + if (!mr->name) { + ((MemoryRegion *)mr)->name = + object_get_canonical_path_component(OBJECT(mr)); + } return mr->name; }