Series comparison

-[Qemu-devel] [PULL for-2.9 0/1] Block patches
+[Qemu-devel] [PULL for-v3.0 0/1] Block patches
-The following changes since commit 940a8ce075e3408742a4edcabfd6c2a15e2539eb:
+The following changes since commit ea6abffa8a08d832feb759d359d5b935e3087cf7:
-  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2017-03-20 16:34:26 +0000)
+  Update version for v3.0.0-rc1 release (2018-07-17 18:15:19 +0100)
-are available in the git repository at:
+are available in the Git repository at:
   git://github.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to ff5bbe56c6f9a74c2d77389a21d5d2368458c939:
+for you to fetch changes up to 6fccbb475bc6effc313ee9481726a1748b6dae57:
-  parallels: fix default options parsing (2017-03-21 10:02:36 +0000)
+  throttle-groups: fix hang when group member leaves (2018-07-19 13:08:26 +0100)
 ----------------------------------------------------------------
 Pull request
 This fix prevents hangs when a drive leaves a throttling group.
 ----------------------------------------------------------------
-----------------------------------------------------------------
+Stefan Hajnoczi (1):
   throttle-groups: fix hang when group member leaves
-Edgar Kaziahmedov (1):
+ block/throttle-groups.c | 4 ++++
-  parallels: fix default options parsing
+file changed, 4 insertions(+)
  block/parallels.c | 2 +-
 file changed, 1 insertion(+), 1 deletion(-)
 --
-.9.3
+.17.1

-[Qemu-devel] [PULL for-2.9 1/1] parallels: fix default options parsing
+[Qemu-devel] [PULL for-v3.0 1/1] throttle-groups: fix hang when group member leaves
-From: Edgar Kaziahmedov <edos@virtuozzo.mipt.ru>
+Throttle groups consist of members sharing one throttling state
 (including bps/iops limits).  Round-robin scheduling is used to ensure
 fairness.  If a group member already has a timer pending then other
 groups members do not schedule their own timers.  The next group member
 will have its turn when the existing timer expires.
-parallels block driver is completely broken since commit
+A hang may occur when a group member leaves while it had a timer
-    commit 75cdcd1553e74b5edc58aed23e3b2da8dabb1876
+scheduled.  Although the code carefully removes the group member from
-    Author: Markus Armbruster <armbru@redhat.com>
+the round-robin list, it does not schedule the next member.  Therefore
-    Date:   Tue Feb 21 21:14:08 2017 +0100
+remaining members continue to wait for the removed member's timer to
-    option: Fix checking of sizes for overflow and trailing crap
+expire.
 Right now even simple
     qemu-io -c "read 512 64k" 1.hds
 ends up with
     Unexpected error in parse_option_size() at util/qemu-option.c:188:
     Parameter 'prealloc-size' expects a non-negative number below 2^64
     Aborted (core dumped)
 The cure is simple - we should use 'M' as a suffix in default option value
 instead of 'MiB'.
-Signed-off-by: Edgar Kaziahmedov <edos@virtuozzo.mipt.ru>
+This patch schedules the next request if a timer is pending.
-Signed-off-by: Denis V. Lunev <den@openvz.org>
+Unfortunately the actual bug is a race condition that I've been unable
-Message-id: 1490002022-22653-1-git-send-email-den@openvz.org
+to capture in a test case.
-CC: Markus Armbruster <armbru@redhat.com>
-CC: Stefan Hajnoczi <stefanha@redhat.com>
+Sometimes drive2 hangs when drive1 is removed from the throttling group:
   $ qemu ... -drive if=none,id=drive1,cache=none,format=qcow2,file=data1.qcow2,iops=100,group=foo \
              -device virtio-blk-pci,id=virtio-blk-pci0,drive=drive1 \
              -drive if=none,id=drive2,cache=none,format=qcow2,file=data2.qcow2,iops=10,group=foo \
              -device virtio-blk-pci,id=virtio-blk-pci1,drive=drive2
   (guest-console1)# fio -filename /dev/vda 4k-seq-read.job
   (guest-console2)# fio -filename /dev/vdb 4k-seq-read.job
   (qmp) {"execute": "block_set_io_throttle", "arguments": {"device": "drive1","bps": 0,"bps_rd": 0,"bps_wr": 0,"iops": 0,"iops_rd": 0,"iops_wr": 0}}
 Reported-by: Nini Gu <ngu@redhat.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Message-id: 20180704145410.794-1-stefanha@redhat.com
 RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1535914
 Cc: Alberto Garcia <berto@igalia.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/parallels.c | 2 +-
+ block/throttle-groups.c | 4 ++++
-file changed, 1 insertion(+), 1 deletion(-)
+file changed, 4 insertions(+)
-diff --git a/block/parallels.c b/block/parallels.c
+diff --git a/block/throttle-groups.c b/block/throttle-groups.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/parallels.c
+--- a/block/throttle-groups.c
-+++ b/block/parallels.c
++++ b/block/throttle-groups.c
-@@ -XXX,XX +XXX,XX @@ static QemuOptsList parallels_runtime_opts = {
+@@ -XXX,XX +XXX,XX @@ void throttle_group_unregister_tgm(ThrottleGroupMember *tgm)
-             .name = PARALLELS_OPT_PREALLOC_SIZE,
-             .type = QEMU_OPT_SIZE,
+     qemu_mutex_lock(&tg->lock);
-             .help = "Preallocation size on image expansion",
+     for (i = 0; i < 2; i++) {
--            .def_value_str = "128MiB",
++        if (timer_pending(tgm->throttle_timers.timers[i])) {
-+            .def_value_str = "128M",
++            tg->any_timer_armed[i] = false;
-         },
++            schedule_next_request(tgm, i);
-         {
++        }
-             .name = PARALLELS_OPT_PREALLOC_MODE,
+         if (tg->tokens[i] == tgm) {
              token = throttle_group_next_tgm(tgm);
              /* Take care of the case where this is the last tgm in the group */
 --
-.9.3
+.17.1

From: Edgar Kaziahmedov <edos@virtuozzo.mipt.ru>

parallels block driver is completely broken since commit
    commit 75cdcd1553e74b5edc58aed23e3b2da8dabb1876
    Author: Markus Armbruster <armbru@redhat.com>
    Date:   Tue Feb 21 21:14:08 2017 +0100
    option: Fix checking of sizes for overflow and trailing crap
Right now even simple
    qemu-io -c "read 512 64k" 1.hds
ends up with
    Unexpected error in parse_option_size() at util/qemu-option.c:188:
    Parameter 'prealloc-size' expects a non-negative number below 2^64
    Aborted (core dumped)
The cure is simple - we should use 'M' as a suffix in default option value
instead of 'MiB'.

Signed-off-by: Edgar Kaziahmedov <edos@virtuozzo.mipt.ru>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Message-id: 1490002022-22653-1-git-send-email-den@openvz.org
CC: Markus Armbruster <armbru@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/parallels.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/parallels.c b/block/parallels.c
index XXXXXXX..XXXXXXX 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -XXX,XX +XXX,XX @@ static QemuOptsList parallels_runtime_opts = {
             .name = PARALLELS_OPT_PREALLOC_SIZE,
             .type = QEMU_OPT_SIZE,
             .help = "Preallocation size on image expansion",
-            .def_value_str = "128MiB",
+            .def_value_str = "128M",
         },
         {
             .name = PARALLELS_OPT_PREALLOC_MODE,
-- 
2.9.3

Throttle groups consist of members sharing one throttling state
(including bps/iops limits).  Round-robin scheduling is used to ensure
fairness.  If a group member already has a timer pending then other
groups members do not schedule their own timers.  The next group member
will have its turn when the existing timer expires.

A hang may occur when a group member leaves while it had a timer
scheduled.  Although the code carefully removes the group member from
the round-robin list, it does not schedule the next member.  Therefore
remaining members continue to wait for the removed member's timer to
expire.

This patch schedules the next request if a timer is pending.
Unfortunately the actual bug is a race condition that I've been unable
to capture in a test case.

Sometimes drive2 hangs when drive1 is removed from the throttling group:

$ qemu ... -drive if=none,id=drive1,cache=none,format=qcow2,file=data1.qcow2,iops=100,group=foo \
             -device virtio-blk-pci,id=virtio-blk-pci0,drive=drive1 \
             -drive if=none,id=drive2,cache=none,format=qcow2,file=data2.qcow2,iops=10,group=foo \
             -device virtio-blk-pci,id=virtio-blk-pci1,drive=drive2
  (guest-console1)# fio -filename /dev/vda 4k-seq-read.job
  (guest-console2)# fio -filename /dev/vdb 4k-seq-read.job
  (qmp) {"execute": "block_set_io_throttle", "arguments": {"device": "drive1","bps": 0,"bps_rd": 0,"bps_wr": 0,"iops": 0,"iops_rd": 0,"iops_wr": 0}}

Reported-by: Nini Gu <ngu@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20180704145410.794-1-stefanha@redhat.com
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1535914
Cc: Alberto Garcia <berto@igalia.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/throttle-groups.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index XXXXXXX..XXXXXXX 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -XXX,XX +XXX,XX @@ void throttle_group_unregister_tgm(ThrottleGroupMember *tgm)
 
     qemu_mutex_lock(&tg->lock);
     for (i = 0; i < 2; i++) {
+        if (timer_pending(tgm->throttle_timers.timers[i])) {
+            tg->any_timer_armed[i] = false;
+            schedule_next_request(tgm, i);
+        }
         if (tg->tokens[i] == tgm) {
             token = throttle_group_next_tgm(tgm);
             /* Take care of the case where this is the last tgm in the group */
-- 
2.17.1