rpms/qemu-kvm
7
0
Fork 0
Code Issues Pull Requests Releases Activity

* Tue Jan 29 2019 Danilo Cesar Lemes de Paula <ddepaula@redhat.com> - 3.1.0-10.el8

Browse Source 
- kvm-throttle-groups-fix-restart-coroutine-iothread-race.patch [bz#1655947]
- kvm-iotests-add-238-for-throttling-tgm-unregister-iothre.patch [bz#1655947]
- Resolves: bz#1655947
  (qemu-kvm core dumped after unplug the device which was set io throttling parameters)
This commit is contained in:
Danilo C. L. de Paula 2019-01-29 14:28:22 +00:00
parent 18677cc3dc
commit d9dd3f8dd3
3 changed files with 253 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

122
kvm-iotests-add-238-for-throttling-tgm-unregister-iothre.patch Normal file
View File

@ -0,0 +1,122 @@
From 91ae068923b70fc62c8504f7c77e42829b4c2e18 Mon Sep 17 00:00:00 2001
From: Stefan Hajnoczi <stefanha@redhat.com>
Date: Tue, 29 Jan 2019 07:02:50 +0000
Subject: [PATCH 2/2] iotests: add 238 for throttling tgm unregister iothread
segfault
RH-Author: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: <20190129070250.22709-3-stefanha@redhat.com>
Patchwork-id: 84138
O-Subject: [RHEL-8.0/AV qemu-kvm PATCH 2/2] iotests: add 238 for throttling tgm unregister iothread segfault
Bugzilla: 1655947
RH-Acked-by: Stefano Garzarella <sgarzare@redhat.com>
RH-Acked-by: Thomas Huth <thuth@redhat.com>
RH-Acked-by: Laurent Vivier <lvivier@redhat.com>
Hot-unplug a scsi-hd using an iothread. The previous patch fixes a
segfault in this scenario.
This patch adds a regression test.
Suggested-by: Alberto Garcia <berto@igalia.com>
Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Message-id: 20190114133257.30299-3-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
(cherry picked from commit 202277f43d544779b7a63123a51c54c3a16b74ad)
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
Conflicts:
tests/qemu-iotests/group
Context conflict because downstream doesn't have 236. This patch adds
238 and doesn't depend on 236.
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
---
tests/qemu-iotests/238 | 47 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/238.out | 6 ++++++
tests/qemu-iotests/group | 1 +
3 files changed, 54 insertions(+)
create mode 100755 tests/qemu-iotests/238
create mode 100644 tests/qemu-iotests/238.out
diff --git a/tests/qemu-iotests/238 b/tests/qemu-iotests/238
new file mode 100755
index 0000000..f81ee11
--- /dev/null
+++ b/tests/qemu-iotests/238
@@ -0,0 +1,47 @@
+#!/usr/bin/env python
+#
+# Regression test for throttle group member unregister segfault with iothread
+#
+# Copyright (c) 2019 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+import sys
+import os
+import iotests
+from iotests import log
+
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'scripts'))
+
+from qemu import QEMUMachine
+
+if iotests.qemu_default_machine == 's390-ccw-virtio':
+ virtio_scsi_device = 'virtio-scsi-ccw'
+else:
+ virtio_scsi_device = 'virtio-scsi-pci'
+
+vm = QEMUMachine(iotests.qemu_prog)
+vm.add_args('-machine', 'accel=kvm')
+vm.launch()
+
+log(vm.qmp('blockdev-add', node_name='hd0', driver='null-co'))
+log(vm.qmp('object-add', qom_type='iothread', id='iothread0'))
+log(vm.qmp('device_add', id='scsi0', driver=virtio_scsi_device, iothread='iothread0'))
+log(vm.qmp('device_add', id='scsi-hd0', driver='scsi-hd', drive='hd0'))
+log(vm.qmp('block_set_io_throttle', id='scsi-hd0', bps=0, bps_rd=0, bps_wr=0,
+ iops=1000, iops_rd=0, iops_wr=0, conv_keys=False))
+log(vm.qmp('device_del', id='scsi-hd0'))
+
+vm.shutdown()
diff --git a/tests/qemu-iotests/238.out b/tests/qemu-iotests/238.out
new file mode 100644
index 0000000..4de840b
--- /dev/null
+++ b/tests/qemu-iotests/238.out
@@ -0,0 +1,6 @@
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 05996ae..268fefa 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -233,3 +233,4 @@
233 auto quick
234 auto quick migration
235 auto quick
+238 auto quick
--
1.8.3.1

120
kvm-throttle-groups-fix-restart-coroutine-iothread-race.patch Normal file
View File

@ -0,0 +1,120 @@
From 02287430957782ffb1db0d7d17693a73925ea02f Mon Sep 17 00:00:00 2001
From: Stefan Hajnoczi <stefanha@redhat.com>
Date: Tue, 29 Jan 2019 07:02:49 +0000
Subject: [PATCH 1/2] throttle-groups: fix restart coroutine iothread race
RH-Author: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: <20190129070250.22709-2-stefanha@redhat.com>
Patchwork-id: 84139
O-Subject: [RHEL-8.0/AV qemu-kvm PATCH 1/2] throttle-groups: fix restart coroutine iothread race
Bugzilla: 1655947
RH-Acked-by: Stefano Garzarella <sgarzare@redhat.com>
RH-Acked-by: Thomas Huth <thuth@redhat.com>
RH-Acked-by: Laurent Vivier <lvivier@redhat.com>
The following QMP command leads to a crash when iothreads are used:
{ 'execute': 'device_del', 'arguments': {'id': 'data'} }
The backtrace involves the queue restart coroutine where
tgm->throttle_state is a NULL pointer because
throttle_group_unregister_tgm() has already been called:
(gdb) bt full
#0 0x00005585a7a3b378 in qemu_mutex_lock_impl (mutex=0xffffffffffffffd0, file=0x5585a7bb3d54 "block/throttle-groups.c", line=412) at util/qemu-thread-posix.c:64
err = <optimized out>
__PRETTY_FUNCTION__ = "qemu_mutex_lock_impl"
__func__ = "qemu_mutex_lock_impl"
#1 0x00005585a79be074 in throttle_group_restart_queue_entry (opaque=0x5585a9de4eb0) at block/throttle-groups.c:412
_f = <optimized out>
data = 0x5585a9de4eb0
tgm = 0x5585a9079440
ts = 0x0
tg = 0xffffffffffffff98
is_write = false
empty_queue = 255
This coroutine should not execute in the iothread after the throttle
group member has been unregistered!
The root cause is that the device_del code path schedules the restart
coroutine in the iothread while holding the AioContext lock. Therefore
the iothread cannot execute the coroutine until after device_del
releases the lock - by this time it's too late.
This patch adds a reference count to ThrottleGroupMember so we can
synchronously wait for restart coroutines to complete. Once they are
done it is safe to unregister the ThrottleGroupMember.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Message-id: 20190114133257.30299-2-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
(cherry picked from commit bc19a0a6e4505390f99d3c593ebaf11b7962cc59)
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
---
block/throttle-groups.c | 9 +++++++++
include/block/throttle-groups.h | 5 +++++
2 files changed, 14 insertions(+)
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 5d8213a..a5a2037 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -415,6 +415,9 @@ static void coroutine_fn throttle_group_restart_queue_entry(void *opaque)
}
g_free(data);
+
+ atomic_dec(&tgm->restart_pending);
+ aio_wait_kick();
}
static void throttle_group_restart_queue(ThrottleGroupMember *tgm, bool is_write)
@@ -430,6 +433,8 @@ static void throttle_group_restart_queue(ThrottleGroupMember *tgm, bool is_write
* be no timer pending on this tgm at this point */
assert(!timer_pending(tgm->throttle_timers.timers[is_write]));
+ atomic_inc(&tgm->restart_pending);
+
co = qemu_coroutine_create(throttle_group_restart_queue_entry, rd);
aio_co_enter(tgm->aio_context, co);
}
@@ -538,6 +543,7 @@ void throttle_group_register_tgm(ThrottleGroupMember *tgm,
tgm->throttle_state = ts;
tgm->aio_context = ctx;
+ atomic_set(&tgm->restart_pending, 0);
qemu_mutex_lock(&tg->lock);
/* If the ThrottleGroup is new set this ThrottleGroupMember as the token */
@@ -584,6 +590,9 @@ void throttle_group_unregister_tgm(ThrottleGroupMember *tgm)
return;
}
+ /* Wait for throttle_group_restart_queue_entry() coroutines to finish */
+ AIO_WAIT_WHILE(tgm->aio_context, atomic_read(&tgm->restart_pending) > 0);
+
qemu_mutex_lock(&tg->lock);
for (i = 0; i < 2; i++) {
assert(tgm->pending_reqs[i] == 0);
diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
index e2fd051..712a8e6 100644
--- a/include/block/throttle-groups.h
+++ b/include/block/throttle-groups.h
@@ -43,6 +43,11 @@ typedef struct ThrottleGroupMember {
*/
unsigned int io_limits_disabled;
+ /* Number of pending throttle_group_restart_queue_entry() coroutines.
+ * Accessed with atomic operations.
+ */
+ unsigned int restart_pending;
+
/* The following fields are protected by the ThrottleGroup lock.
* See the ThrottleGroup documentation for details.
* throttle_state tells us if I/O limits are configured. */
--
1.8.3.1

12
qemu-kvm.spec
View File

@ -68,7 +68,7 @@ Obsoletes: %1-rhev
Summary: QEMU is a machine emulator and virtualizer Summary: QEMU is a machine emulator and virtualizer
Name: qemu-kvm Name: qemu-kvm
Version: 3.1.0 Version: 3.1.0
Release: 9%{?dist} Release: 10%{?dist}
# Epoch because we pushed a qemu-1.0 package. AIUI this can't ever be dropped # Epoch because we pushed a qemu-1.0 package. AIUI this can't ever be dropped
Epoch: 15 Epoch: 15
License: GPLv2 and GPLv2+ and CC-BY License: GPLv2 and GPLv2+ and CC-BY
@ -168,6 +168,10 @@ Patch41: kvm-migration-rdma-unregister-fd-handler.patch
Patch42: kvm-s390x-tod-Properly-stop-the-KVM-TOD-while-the-guest-.patch Patch42: kvm-s390x-tod-Properly-stop-the-KVM-TOD-while-the-guest-.patch
# For bz#1659127 - Stress guest and stop it, then do live migration, guest hit call trace on destination end # For bz#1659127 - Stress guest and stop it, then do live migration, guest hit call trace on destination end
Patch43: kvm-hw-s390x-Fix-bad-mask-in-time2tod.patch Patch43: kvm-hw-s390x-Fix-bad-mask-in-time2tod.patch
# For bz#1655947 - qemu-kvm core dumped after unplug the device which was set io throttling parameters
Patch44: kvm-throttle-groups-fix-restart-coroutine-iothread-race.patch
# For bz#1655947 - qemu-kvm core dumped after unplug the device which was set io throttling parameters
Patch45: kvm-iotests-add-238-for-throttling-tgm-unregister-iothre.patch
BuildRequires: zlib-devel BuildRequires: zlib-devel
BuildRequires: glib2-devel BuildRequires: glib2-devel
@ -1015,6 +1019,12 @@ useradd -r -u 107 -g qemu -G kvm -d / -s /sbin/nologin \
%changelog %changelog
* Tue Jan 29 2019 Danilo Cesar Lemes de Paula <ddepaula@redhat.com> - 3.1.0-10.el8
- kvm-throttle-groups-fix-restart-coroutine-iothread-race.patch [bz#1655947]
- kvm-iotests-add-238-for-throttling-tgm-unregister-iothre.patch [bz#1655947]
- Resolves: bz#1655947
(qemu-kvm core dumped after unplug the device which was set io throttling parameters)
* Tue Jan 29 2019 Danilo Cesar Lemes de Paula <ddepaula@redhat.com> - 3.1.0-9.el8 * Tue Jan 29 2019 Danilo Cesar Lemes de Paula <ddepaula@redhat.com> - 3.1.0-9.el8
- kvm-migration-rdma-unregister-fd-handler.patch [bz#1666601] - kvm-migration-rdma-unregister-fd-handler.patch [bz#1666601]
- kvm-s390x-tod-Properly-stop-the-KVM-TOD-while-the-guest-.patch [bz#1659127] - kvm-s390x-tod-Properly-stop-the-KVM-TOD-while-the-guest-.patch [bz#1659127]