From 2264750483107c45877d29813c497b4c87f64cb6 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 19 Feb 2015 17:05:46 +0100 Subject: [PATCH 01/15] scsi: give device a parent before setting properties This mimics what is done in qdev_device_add, and lets the device be freed in case something goes wrong. Otherwise, object_unparent returns immediately without freeing the device, which is on the other hand left in the parent bus's list of children. scsi_bus_legacy_handle_cmdline then returns an error, and the HBA is destroyed as well with object_unparent. But the lingering device that was not removed in scsi_bus_legacy_add_drive cannot be removed now either, and bus_unparent gets stuck in an infinite loop trying to empty the list of children. The right fix of course would be to assert in bus_add_child that the device already has a bus, and remove the "safety net" that adds the drive to the QOM tree in device_set_realized. I am not yet sure whether that would entail changing all callers to qdev_create (as well as isa_create and usb_create and the corresponding _try_create versions). Reported-by: Markus Armbruster Tested-by: Markus Armbruster Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-bus.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index db39ae0e23..dca9576828 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -221,11 +221,16 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk, const char *serial, Error **errp) { const char *driver; + char *name; DeviceState *dev; Error *err = NULL; driver = blk_is_sg(blk) ? "scsi-generic" : "scsi-disk"; dev = qdev_create(&bus->qbus, driver); + name = g_strdup_printf("legacy[%d]", unit); + object_property_add_child(OBJECT(bus), name, OBJECT(dev), NULL); + g_free(name); + qdev_prop_set_uint32(dev, "scsi-id", unit); if (bootindex >= 0) { object_property_set_int(OBJECT(dev), bootindex, "bootindex", From 2e5b887cfc69991eee27be6cc0938c70a360fe45 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Sun, 15 Feb 2015 11:06:30 +0800 Subject: [PATCH 02/15] block: Forbid bdrv_set_aio_context outside BQL Even if the caller has both the old and the new AioContext's, there can be a deadlock, due to the leading bdrv_drain_all. Suppose there are four io threads (A, B, A0, B0) with A and B owning a BDS for each (bs_a, bs_b); Now A wants to move bs_a to iothread A0, and B wants to move bs_b to B0, at the same time: iothread A iothread B -------------------------------------------------------------------------- aio_context_acquire(A0) /* OK */ aio_context_acquire(B0) /* OK */ bdrv_set_aio_context(bs_a, A0) bdrv_set_aio_context(bs_b, B0) -> bdrv_drain_all() -> bdrv_drain_all() -> acquire A /* OK */ -> acquire A /* blocked */ -> acquire B /* blocked */ -> acquire B ... ... Deadlock happens because A is waiting for B, and B is waiting for A. Signed-off-by: Fam Zheng Reviewed-by: Paolo Bonzini Message-Id: <1423969591-23646-2-git-send-email-famz@redhat.com> Signed-off-by: Paolo Bonzini --- include/block/block.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index 471d11dbbe..649c269ecd 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -547,8 +547,7 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs); * Changes the #AioContext used for fd handlers, timers, and BHs by this * BlockDriverState and all its children. * - * This function must be called from the old #AioContext or with a lock held so - * the old #AioContext is not executing. + * This function must be called with iothread lock held. */ void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context); From 0543055967e554b5be10c3f96c077bf26dcaf181 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Sun, 15 Feb 2015 11:06:31 +0800 Subject: [PATCH 03/15] virtio-scsi-dataplane: Call blk_set_aio_context within BQL It's not safe to call blk_set_aio_context from outside BQL because of the bdrv_drain_all there. Let's put it in the hotplug callback which will be called by qdev device realization for each scsi device attached to the bus. Signed-off-by: Fam Zheng Message-Id: <1423969591-23646-3-git-send-email-famz@redhat.com> Signed-off-by: Paolo Bonzini --- hw/scsi/virtio-scsi.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 9e2c718be7..8c437dd8a6 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -254,10 +254,8 @@ static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req) int target; int ret = 0; - if (s->dataplane_started && blk_get_aio_context(d->conf.blk) != s->ctx) { - aio_context_acquire(s->ctx); - blk_set_aio_context(d->conf.blk, s->ctx); - aio_context_release(s->ctx); + if (s->dataplane_started) { + assert(blk_get_aio_context(d->conf.blk) == s->ctx); } /* Here VIRTIO_SCSI_S_OK means "FUNCTION COMPLETE". */ req->resp.tmf.response = VIRTIO_SCSI_S_OK; @@ -540,10 +538,8 @@ bool virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req) virtio_scsi_complete_cmd_req(req); return false; } - if (s->dataplane_started && blk_get_aio_context(d->conf.blk) != s->ctx) { - aio_context_acquire(s->ctx); - blk_set_aio_context(d->conf.blk, s->ctx); - aio_context_release(s->ctx); + if (s->dataplane_started) { + assert(blk_get_aio_context(d->conf.blk) == s->ctx); } req->sreq = scsi_req_new(d, req->req.cmd.tag, virtio_scsi_get_lun(req->req.cmd.lun), @@ -767,6 +763,9 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev, return; } blk_op_block_all(sd->conf.blk, s->blocker); + aio_context_acquire(s->ctx); + blk_set_aio_context(sd->conf.blk, s->ctx); + aio_context_release(s->ctx); } if ((vdev->guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) { From 2ed1ebcf65edf6757d8904000889ce52cc0a9d1b Mon Sep 17 00:00:00 2001 From: Pavel Dovgalyuk Date: Fri, 27 Feb 2015 16:11:02 +0300 Subject: [PATCH 04/15] timer: replace time() with QEMU_CLOCK_HOST This patch replaces time() function calls with calls to qemu_clock_get_ns(QEMU_CLOCK_HOST). It makes such requests deterministic in record/replay mode of icount. Reviewed-by: Paolo Bonzini Signed-off-by: Pavel Dovgalyuk Message-Id: <20150227131102.11912.89850.stgit@PASHA-ISP> Signed-off-by: Paolo Bonzini --- vl.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/vl.c b/vl.c index e1ffd0a96a..9213f687f1 100644 --- a/vl.c +++ b/vl.c @@ -710,13 +710,17 @@ void vm_start(void) /***********************************************************/ /* real time host monotonic timer */ +static time_t qemu_time(void) +{ + return qemu_clock_get_ms(QEMU_CLOCK_HOST) / 1000; +} + /***********************************************************/ /* host time/date access */ void qemu_get_timedate(struct tm *tm, int offset) { - time_t ti; + time_t ti = qemu_time(); - time(&ti); ti += offset; if (rtc_date_offset == -1) { if (rtc_utc) @@ -744,7 +748,7 @@ int qemu_timedate_diff(struct tm *tm) else seconds = mktimegm(tm) + rtc_date_offset; - return seconds - time(NULL); + return seconds - qemu_time(); } static void configure_rtc_date_offset(const char *startdate, int legacy) @@ -782,7 +786,7 @@ static void configure_rtc_date_offset(const char *startdate, int legacy) "'2006-06-17T16:01:21' or '2006-06-17'\n"); exit(1); } - rtc_date_offset = time(NULL) - rtc_start_date; + rtc_date_offset = qemu_time() - rtc_start_date; } } From 57fe6a6e4a050f639ac3d2f7c32b4cd3bcde3978 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Fri, 27 Feb 2015 09:49:44 +0800 Subject: [PATCH 05/15] bootdevice: fix segment fault when booting guest with '-kernel' and '-initrd' Reproducer: $./qemu-system-x86_64 --enable-kvm -kernel /home/vmlinuz-2.6.32.12-0.7-default \ -initrd /home/initrd-2.6.32.12-0.7-default -append \ "root=/dev/ram rw console=ttyS0,115200" -dtb guest.dtb -vnc :10 --monitor stdio -smp 2 QEMU 2.2.50 monitor - type 'help' for more information (qemu) Segmentation fault (core dumped) Reported-by: Edivaldo de Araujo Pereira Signed-off-by: Gonglei Message-Id: <1425001784-6752-1-git-send-email-arei.gonglei@huawei.com> Signed-off-by: Paolo Bonzini --- bootdevice.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/bootdevice.c b/bootdevice.c index c3a010c094..eacd8c88c2 100644 --- a/bootdevice.c +++ b/bootdevice.c @@ -221,10 +221,15 @@ char *get_boot_devices_list(size_t *size, bool ignore_suffixes) } if (!ignore_suffixes) { - d = qdev_get_own_fw_dev_path_from_handler(i->dev->parent_bus, i->dev); - if (d) { - assert(!i->suffix); - suffix = d; + if (i->dev) { + d = qdev_get_own_fw_dev_path_from_handler(i->dev->parent_bus, + i->dev); + if (d) { + assert(!i->suffix); + suffix = d; + } else { + suffix = g_strdup(i->suffix); + } } else { suffix = g_strdup(i->suffix); } From 4681867544f04f752bd98f39c3055493c80ea316 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Fri, 27 Feb 2015 17:04:35 +1100 Subject: [PATCH 06/15] Add specific config options for PCI-E bridges The i82801b11, ioh3420 and xio3130 PCI Express devices are currently included in the build unconditionally. While they could theoretically appear on any target platform with PCI-E, they're pretty unlikely to appear on platforms that aren't Intel derived. Therefore, to avoid presenting unlikely-to-be-relevant devices to the user, add config options to enable these components, and enable them by default only on x86 and arm platforms. (Note that this patch does include these for aarch64, via its inclusion of arm-softmmu.mak). Signed-off-by: David Gibson Reviewed-by: Peter Crosthwaite Message-Id: <1425017077-18487-2-git-send-email-david@gibson.dropbear.id.au> Signed-off-by: Paolo Bonzini --- default-configs/arm-softmmu.mak | 4 ++++ default-configs/i386-softmmu.mak | 3 +++ default-configs/x86_64-softmmu.mak | 3 +++ hw/pci-bridge/Makefile.objs | 5 +++-- 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak index b00c2e150e..6ee9b43cfd 100644 --- a/default-configs/arm-softmmu.mak +++ b/default-configs/arm-softmmu.mak @@ -91,3 +91,7 @@ CONFIG_INTEGRATOR_DEBUG=y CONFIG_ALLWINNER_A10_PIT=y CONFIG_ALLWINNER_A10_PIC=y CONFIG_ALLWINNER_A10=y + +CONFIG_XIO3130=y +CONFIG_IOH3420=y +CONFIG_I82801B11=y diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak index bd99af9942..0b8ce4bbf8 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -43,3 +43,6 @@ CONFIG_IOAPIC=y CONFIG_ICC_BUS=y CONFIG_PVPANIC=y CONFIG_MEM_HOTPLUG=y +CONFIG_XIO3130=y +CONFIG_IOH3420=y +CONFIG_I82801B11=y diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak index e7c273417b..6add04a2f6 100644 --- a/default-configs/x86_64-softmmu.mak +++ b/default-configs/x86_64-softmmu.mak @@ -43,3 +43,6 @@ CONFIG_IOAPIC=y CONFIG_ICC_BUS=y CONFIG_PVPANIC=y CONFIG_MEM_HOTPLUG=y +CONFIG_XIO3130=y +CONFIG_IOH3420=y +CONFIG_I82801B11=y diff --git a/hw/pci-bridge/Makefile.objs b/hw/pci-bridge/Makefile.objs index 968b3694ab..96c596eb31 100644 --- a/hw/pci-bridge/Makefile.objs +++ b/hw/pci-bridge/Makefile.objs @@ -1,5 +1,6 @@ common-obj-y += pci_bridge_dev.o -common-obj-y += ioh3420.o xio3130_upstream.o xio3130_downstream.o -common-obj-y += i82801b11.o +common-obj-$(CONFIG_XIO3130) += xio3130_upstream.o xio3130_downstream.o +common-obj-$(CONFIG_IOH3420) += ioh3420.o +common-obj-$(CONFIG_I82801B11) += i82801b11.o # NewWorld PowerMac common-obj-$(CONFIG_DEC_PCI) += dec.o From 2296594257d11a253d12f0219eef3f1a0201c2d9 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Fri, 27 Feb 2015 17:04:36 +1100 Subject: [PATCH 07/15] Create specific config option for "platform-bus" Currently the "platform-bus" device is included for all softmmu builds. This bridge is intended for use on any platforms that require dynamic creation of sysbus devices. However, at present it is used only for the PPC E500 target, with plans for the ARM "virt" target in the immediate future. To avoid a not-very-useful entry appearing in "qemu -device ?" output on other targets, this patch makes a specific config option for platform-bus and enables it (for now) only on ppc configurations which include E500 and on ARM (which always includes the "virt" target). Signed-off-by: David Gibson Message-Id: <1425017077-18487-3-git-send-email-david@gibson.dropbear.id.au> Signed-off-by: Paolo Bonzini --- default-configs/arm-softmmu.mak | 1 + default-configs/ppc-softmmu.mak | 1 + default-configs/ppc64-softmmu.mak | 1 + hw/core/Makefile.objs | 2 +- 4 files changed, 4 insertions(+), 1 deletion(-) diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak index 6ee9b43cfd..149ae1b595 100644 --- a/default-configs/arm-softmmu.mak +++ b/default-configs/arm-softmmu.mak @@ -34,6 +34,7 @@ CONFIG_PFLASH_CFI02=y CONFIG_MICRODRIVE=y CONFIG_USB_MUSB=y CONFIG_USB_EHCI_SYSBUS=y +CONFIG_PLATFORM_BUS=y CONFIG_ARM11MPCORE=y CONFIG_A9MPCORE=y diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak index aebfab92eb..4b60e699ff 100644 --- a/default-configs/ppc-softmmu.mak +++ b/default-configs/ppc-softmmu.mak @@ -43,6 +43,7 @@ CONFIG_PREP=y CONFIG_MAC=y CONFIG_E500=y CONFIG_OPENPIC_KVM=$(and $(CONFIG_E500),$(CONFIG_KVM)) +CONFIG_PLATFORM_BUS=y CONFIG_ETSEC=y CONFIG_LIBDECNUMBER=y # For PReP diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak index f195a8721a..de71e41b00 100644 --- a/default-configs/ppc64-softmmu.mak +++ b/default-configs/ppc64-softmmu.mak @@ -44,6 +44,7 @@ CONFIG_PREP=y CONFIG_MAC=y CONFIG_E500=y CONFIG_OPENPIC_KVM=$(and $(CONFIG_E500),$(CONFIG_KVM)) +CONFIG_PLATFORM_BUS=y CONFIG_ETSEC=y CONFIG_LIBDECNUMBER=y # For pSeries diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs index 9dce1bc53c..abb3560bea 100644 --- a/hw/core/Makefile.objs +++ b/hw/core/Makefile.objs @@ -14,4 +14,4 @@ common-obj-$(CONFIG_SOFTMMU) += machine.o common-obj-$(CONFIG_SOFTMMU) += null-machine.o common-obj-$(CONFIG_SOFTMMU) += loader.o common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o -common-obj-$(CONFIG_SOFTMMU) += platform-bus.o +common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o From 8af738b3eecf69a795c6ff78121edbb81ab03684 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Fri, 27 Feb 2015 17:04:37 +1100 Subject: [PATCH 08/15] Give ivshmem its own config option Currently the ivshmem device is built whenever both PCI and KVM support are included. This patch gives it its own config option to allow easier customization of whether to include it. It's enabled by default in the same circumstances as now - when both PCI and KVM are available. Signed-off-by: David Gibson Reviewed-by: Peter Crosthwaite Message-Id: <1425017077-18487-4-git-send-email-david@gibson.dropbear.id.au> Signed-off-by: Paolo Bonzini --- default-configs/pci.mak | 1 + hw/misc/Makefile.objs | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/default-configs/pci.mak b/default-configs/pci.mak index bea6b01553..58a2c0ace1 100644 --- a/default-configs/pci.mak +++ b/default-configs/pci.mak @@ -35,3 +35,4 @@ CONFIG_SDHCI=y CONFIG_EDU=y CONFIG_VGA=y CONFIG_VGA_PCI=y +CONFIG_IVSHMEM=$(CONFIG_KVM) diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs index 029a56f279..6c6e29681a 100644 --- a/hw/misc/Makefile.objs +++ b/hw/misc/Makefile.objs @@ -19,9 +19,7 @@ common-obj-$(CONFIG_PUV3) += puv3_pm.o common-obj-$(CONFIG_MACIO) += macio/ -ifeq ($(CONFIG_PCI), y) -obj-$(CONFIG_KVM) += ivshmem.o -endif +obj-$(CONFIG_IVSHMEM) += ivshmem.o obj-$(CONFIG_REALVIEW) += arm_sysctl.o obj-$(CONFIG_NSERIES) += cbus.o From 43ae8fb10c5f6ca78f242624c1f446e0050a9d43 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 25 Feb 2015 12:40:08 +0800 Subject: [PATCH 09/15] iscsi: Handle write protected case in reopen Save the write protected flag and check before reopen. Signed-off-by: Fam Zheng Message-Id: <1424839208-5195-1-git-send-email-famz@redhat.com> [Fixed typo in the name of the new field. - Paolo] Signed-off-by: Paolo Bonzini --- block/iscsi.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index 12ddbfb095..1fa855acdd 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -65,6 +65,7 @@ typedef struct IscsiLun { unsigned long *allocationmap; int cluster_sectors; bool use_16_for_rw; + bool write_protected; } IscsiLun; typedef struct IscsiTask { @@ -1268,10 +1269,6 @@ out: /* * We support iscsi url's on the form * iscsi://[%@][:]// - * - * Note: flags are currently not used by iscsi_open. If this function - * is changed such that flags are used, please examine iscsi_reopen_prepare() - * to see if needs to be changed as well. */ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) @@ -1385,9 +1382,10 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, scsi_free_scsi_task(task); task = NULL; + iscsilun->write_protected = iscsi_is_write_protected(iscsilun); /* Check the write protect flag of the LUN if we want to write */ if (iscsilun->type == TYPE_DISK && (flags & BDRV_O_RDWR) && - iscsi_is_write_protected(iscsilun)) { + iscsilun->write_protected) { error_setg(errp, "Cannot open a write protected LUN as read-write"); ret = -EACCES; goto out; @@ -1541,13 +1539,17 @@ static void iscsi_refresh_limits(BlockDriverState *bs, Error **errp) sector_limits_lun2qemu(iscsilun->bl.opt_xfer_len, iscsilun); } -/* Since iscsi_open() ignores bdrv_flags, there is nothing to do here in - * prepare. Note that this will not re-establish a connection with an iSCSI - * target - it is effectively a NOP. */ +/* Note that this will not re-establish a connection with an iSCSI target - it + * is effectively a NOP. */ static int iscsi_reopen_prepare(BDRVReopenState *state, BlockReopenQueue *queue, Error **errp) { - /* NOP */ + IscsiLun *iscsilun = state->bs->opaque; + + if (state->flags & BDRV_O_RDWR && iscsilun->write_protected) { + error_setg(errp, "Cannot open a write protected LUN as read-write"); + return -EACCES; + } return 0; } From 23cab7b7a9cb365b15eee953a0dd6cdb9198ae6c Mon Sep 17 00:00:00 2001 From: Vasily Efimov Date: Wed, 18 Feb 2015 15:59:37 +0300 Subject: [PATCH 10/15] Makefile: fix up parallel building under MSYS+MinGW This patch enables parallel building of QEMU in MSYS+MinGW environment. Currently an attempt to build QEMU in parallel fails on generation of version.lo (and version.o too). The cause of the failure is that when listing prerequisites "Makefile" references "config-host.h" by absolute path in some rules and by relative path in others. Make cannot figure out that these references points to the same file which leads to the race: the generation of "version.*" which requires "$(BUILD_DIR)/config-host.h" is launched in parallel with the generation of "config-host.h" needed by other "Makefile" targets. This patch removes "$(BUILD_DIR)/" prefix from corresponding prerequisite of "version.*". There is no other prerequisites "$(BUILD_DIR)/config-host.h" found. Also note that not every version of MSYS is able to build QEMU in parallel, see: "http://sourceforge.net/p/mingw/bugs/1950/". The suggested version is 1.0.17. Signed-off-by: Vasily Efimov Message-Id: <1424264377-5992-1-git-send-email-real@ispras.ru> Signed-off-by: Paolo Bonzini --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 6817c6f715..b0d8c07cec 100644 --- a/Makefile +++ b/Makefile @@ -197,9 +197,9 @@ ALL_SUBDIRS=$(TARGET_DIRS) $(patsubst %,pc-bios/%, $(ROMS)) recurse-all: $(SUBDIR_RULES) $(ROMSUBDIR_RULES) -$(BUILD_DIR)/version.o: $(SRC_PATH)/version.rc $(BUILD_DIR)/config-host.h | $(BUILD_DIR)/version.lo +$(BUILD_DIR)/version.o: $(SRC_PATH)/version.rc config-host.h | $(BUILD_DIR)/version.lo $(call quiet-command,$(WINDRES) -I$(BUILD_DIR) -o $@ $<," RC version.o") -$(BUILD_DIR)/version.lo: $(SRC_PATH)/version.rc $(BUILD_DIR)/config-host.h +$(BUILD_DIR)/version.lo: $(SRC_PATH)/version.rc config-host.h $(call quiet-command,$(WINDRES) -I$(BUILD_DIR) -o $@ $<," RC version.lo") Makefile: $(version-obj-y) $(version-lobj-y) From 12ccfec9684679fc1945b5b5020487b2cb17dc06 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Thu, 19 Feb 2015 08:48:46 +0100 Subject: [PATCH 11/15] Makefile: don't silence mak file test with V=1 V=1 should show what's going on, it's not nice to silence things unconditionally. Signed-off-by: Michael S. Tsirkin Message-Id: <1424332114-13440-1-git-send-email-mst@redhat.com> Signed-off-by: Paolo Bonzini --- Makefile | 6 +++--- scripts/make_device_config.sh | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index b0d8c07cec..d92d4cd573 100644 --- a/Makefile +++ b/Makefile @@ -109,8 +109,8 @@ endif -include $(SUBDIR_DEVICES_MAK_DEP) %/config-devices.mak: default-configs/%.mak - $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh $@ $<, " GEN $@") - @if test -f $@; then \ + $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/make_device_config.sh $@.tmp $<, " GEN $@.tmp") + $(call quiet-command, if test -f $@; then \ if cmp -s $@.old $@; then \ mv $@.tmp $@; \ cp -p $@ $@.old; \ @@ -126,7 +126,7 @@ endif else \ mv $@.tmp $@; \ cp -p $@ $@.old; \ - fi + fi, " GEN $@"); defconfig: rm -f config-all-devices.mak $(SUBDIR_DEVICES_MAK) diff --git a/scripts/make_device_config.sh b/scripts/make_device_config.sh index 7242707819..7958086132 100644 --- a/scripts/make_device_config.sh +++ b/scripts/make_device_config.sh @@ -2,7 +2,7 @@ # Construct a target device config file from a default, pulling in any # files from include directives. -dest=$1.tmp +dest=$1 dep=`dirname $1`-`basename $1`.d src=$2 src_dir=`dirname $src` From a9ad5e1efcbbdf4b34bd7679613605efdb41bd04 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Thu, 19 Feb 2015 08:48:52 +0100 Subject: [PATCH 12/15] Makefile.target: binary depends on config-devices relink binary whenever config-devices.mak changes: this makes sense as we are adding/removing devices, so binary has to be relinked to be up to date. Signed-off-by: Michael S. Tsirkin Message-Id: <1424332114-13440-2-git-send-email-mst@redhat.com> Signed-off-by: Paolo Bonzini --- Makefile.target | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile.target b/Makefile.target index 58c6ae1d69..2262d89354 100644 --- a/Makefile.target +++ b/Makefile.target @@ -175,9 +175,11 @@ all-obj-y += $(common-obj-y) all-obj-y += $(target-obj-y) all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y) +$(QEMU_PROG_BUILD): config-devices.mak + # build either PROG or PROGW $(QEMU_PROG_BUILD): $(all-obj-y) ../libqemuutil.a ../libqemustub.a - $(call LINK,$^) + $(call LINK, $(filter-out %.mak, $^)) gdbstub-xml.c: $(TARGET_XML_FILES) $(SRC_PATH)/scripts/feature_to_c.sh $(call quiet-command,rm -f $@ && $(SHELL) $(SRC_PATH)/scripts/feature_to_c.sh $@ $(TARGET_XML_FILES)," GEN $(TARGET_DIR)$@") From f6758f7d6b8b483eb1d061937ed06da830c1ecb8 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 27 Feb 2015 12:11:53 -0500 Subject: [PATCH 13/15] virtio-scsi: Allocate op blocker reason before blocking s->blocker is really only used in hw/scsi/virtio-scsi.c; the only places where it is used in hw/scsi/virtio-scsi-dataplane.c is when it is allocated and when it is freed. That does not make a whole lot of sense (and is actually wrong because this leads to s->blocker potentially being NULL when blk_op_block_all() is called in virtio-scsi.c), so move the allocation and destruction of s->blocker to the device realization and unrealization in virtio-scsi.c, respectively. Case in point: $ echo -e 'eject drv\nquit' | \ x86_64-softmmu/qemu-system-x86_64 \ -monitor stdio -machine accel=qtest -display none \ -object iothread,id=thr -device virtio-scsi-pci,iothread=thr \ -drive if=none,file=test.qcow2,format=qcow2,id=drv \ -device scsi-cd,drive=drv Without this patch: (qemu) eject drv [1] 10102 done 10103 segmentation fault (core dumped) With this patch: (qemu) eject drv Device 'drv' is busy: block device is in use by data plane (qemu) quit Signed-off-by: Max Reitz Message-Id: <1425057113-26940-1-git-send-email-mreitz@redhat.com> Signed-off-by: Paolo Bonzini --- hw/scsi/virtio-scsi-dataplane.c | 4 ---- hw/scsi/virtio-scsi.c | 4 ++++ 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c index 418d73b1b4..3f40ff090f 100644 --- a/hw/scsi/virtio-scsi-dataplane.c +++ b/hw/scsi/virtio-scsi-dataplane.c @@ -211,8 +211,6 @@ void virtio_scsi_dataplane_start(VirtIOSCSI *s) s->dataplane_starting = true; - assert(!s->blocker); - error_setg(&s->blocker, "block device is in use by data plane"); /* Set up guest notifier (irq) */ rc = k->set_guest_notifiers(qbus->parent, vs->conf.num_queues + 2, true); if (rc != 0) { @@ -279,8 +277,6 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s) if (!s->dataplane_started || s->dataplane_stopping) { return; } - error_free(s->blocker); - s->blocker = NULL; s->dataplane_stopping = true; assert(s->ctx == iothread_get_aio_context(vs->conf.iothread)); diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 8c437dd8a6..4db3b23ea3 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -903,6 +903,8 @@ static void virtio_scsi_device_realize(DeviceState *dev, Error **errp) virtio_scsi_save, virtio_scsi_load, s); s->migration_state_notifier.notify = virtio_scsi_migration_state_changed; add_migration_state_change_notifier(&s->migration_state_notifier); + + error_setg(&s->blocker, "block device is in use by data plane"); } static void virtio_scsi_instance_init(Object *obj) @@ -928,6 +930,8 @@ static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp) { VirtIOSCSI *s = VIRTIO_SCSI(dev); + error_free(s->blocker); + unregister_savevm(dev, "virtio-scsi", s); remove_migration_state_change_notifier(&s->migration_state_notifier); From 6b49809c597331803ea941eadda813e5bb4e8fe2 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 27 Feb 2015 19:58:23 +0100 Subject: [PATCH 14/15] cpus: fix deadlock and segfault in qemu_mutex_lock_iothread When two threads (other than the low-priority TCG VCPU thread) are competing for the iothread lock, a deadlock can happen. This is because iothread_requesting_mutex is set to false by the first thread that gets the mutex, and then the VCPU thread might never yield from the execution loop. If iothread_requesting_mutex is changed from a bool to a counter, the deadlock is fixed. However, there is another bug in qemu_mutex_lock_iothread that can be triggered by the new call_rcu thread. The bug happens if qemu_mutex_lock_iothread is called before the CPUs are created. In that case, first_cpu is NULL and the caller segfaults in qemu_mutex_lock_iothread. To fix this, just do not do the kick if first_cpu is NULL. Reported-by: Leon Alrae Reported-by: Andreas Gustafsson Tested-by: Leon Alrae Signed-off-by: Paolo Bonzini --- cpus.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cpus.c b/cpus.c index 1cd9867893..83c078e286 100644 --- a/cpus.c +++ b/cpus.c @@ -778,7 +778,7 @@ static void qemu_tcg_init_cpu_signals(void) static QemuMutex qemu_global_mutex; static QemuCond qemu_io_proceeded_cond; -static bool iothread_requesting_mutex; +static unsigned iothread_requesting_mutex; static QemuThread io_thread; @@ -1115,15 +1115,15 @@ bool qemu_in_vcpu_thread(void) void qemu_mutex_lock_iothread(void) { - if (!tcg_enabled()) { + if (!tcg_enabled() || !first_cpu) { qemu_mutex_lock(&qemu_global_mutex); } else { - iothread_requesting_mutex = true; + atomic_inc(&iothread_requesting_mutex); if (qemu_mutex_trylock(&qemu_global_mutex)) { qemu_cpu_kick_thread(first_cpu); qemu_mutex_lock(&qemu_global_mutex); } - iothread_requesting_mutex = false; + atomic_dec(&iothread_requesting_mutex); qemu_cond_broadcast(&qemu_io_proceeded_cond); } } From 21618b3e55ad2c6fede0bffcaea466091811ce59 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 27 Feb 2015 20:01:03 +0100 Subject: [PATCH 15/15] cpus: be more paranoid in avoiding deadlocks For good measure, ensure that the following sequence: thread 1 calls qemu_mutex_lock_iothread thread 2 calls qemu_mutex_lock_iothread VCPU thread are created VCPU thread enters execution loop results in the VCPU threads letting the other two threads run and obeying iothread_requesting_mutex even if the VCPUs are not halted. To do this, check iothread_requesting_mutex before execution starts. Tested-by: Leon Alrae Signed-off-by: Paolo Bonzini --- cpus.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cpus.c b/cpus.c index 83c078e286..0fac143355 100644 --- a/cpus.c +++ b/cpus.c @@ -1025,6 +1025,9 @@ static void *qemu_tcg_cpu_thread_fn(void *arg) } } + /* process any pending work */ + exit_request = 1; + while (1) { tcg_exec_all(); @@ -1115,10 +1118,11 @@ bool qemu_in_vcpu_thread(void) void qemu_mutex_lock_iothread(void) { + atomic_inc(&iothread_requesting_mutex); if (!tcg_enabled() || !first_cpu) { qemu_mutex_lock(&qemu_global_mutex); + atomic_dec(&iothread_requesting_mutex); } else { - atomic_inc(&iothread_requesting_mutex); if (qemu_mutex_trylock(&qemu_global_mutex)) { qemu_cpu_kick_thread(first_cpu); qemu_mutex_lock(&qemu_global_mutex);