Series comparison

-[Qemu-devel] [PULL 0/1] Block patches
+[Qemu-devel] [PULL 0/2] Block patches
-The following changes since commit 64175afc695c0672876fbbfc31b299c86d562cb4:
+The following changes since commit ee135aa0428fe5af2af7be04ff16d2b596a9330a:
-  arm_gicv3: Fix ICC_BPR1 reset value when EL3 not implemented (2017-06-07 17:21:44 +0100)
+  Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-3.1-20180821' into staging (2018-08-21 13:27:11 +0100)
-are available in the git repository at:
+are available in the Git repository at:
   git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request
-for you to fetch changes up to 56faeb9bb6872b3f926b3b3e0452a70beea10af2:
+for you to fetch changes up to 26bf474ba92c76e61bea51726e22da6dfd185296:
-  block/gluster.c: Handle qdict_array_entries() failure (2017-06-09 08:41:29 -0400)
+  block: iotest to catch abort on forced blockjob cancel (2018-08-21 15:20:37 -0400)
 ----------------------------------------------------------------
-Gluster patch
+Block job patches
 ----------------------------------------------------------------
-Peter Maydell (1):
+Jeff Cody (2):
-  block/gluster.c: Handle qdict_array_entries() failure
+  block: for jobs, do not clear user_paused until after the resume
   block: iotest to catch abort on forced blockjob cancel
- block/gluster.c | 3 +--
+ job.c                      |  2 +-
-file changed, 1 insertion(+), 2 deletions(-)
+ tests/qemu-iotests/229     | 95 ++++++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/229.out | 23 +++++++++
  tests/qemu-iotests/group   |  1 +
 files changed, 120 insertions(+), 1 deletion(-)
  create mode 100755 tests/qemu-iotests/229
  create mode 100644 tests/qemu-iotests/229.out
 --
-.9.3
+.17.1

-New patch
+[Qemu-devel] [PULL 1/2] block: for jobs, do not clear user_paused until after the resume
+The function job_cancel_async() will always cause an assert for blockjob
+user resume.  We set job->user_paused to false, and then call
+job->driver->user_resume().  In the case of blockjobs, this is the
+block_job_user_resume() function.
+In that function, we assert that job.user_paused is set to true.
+Unfortunately, right before calling this function, it has explicitly
+been set to false.
+The fix is pretty simple: set job->user_paused to false only after the
+job user_resume() function has been called.
+Reviewed-by: John Snow <jsnow@redhat.com>
+Reviewed-by: Eric Blake <eblake@redhat.com>
+Signed-off-by: Jeff Cody <jcody@redhat.com>
+Message-id: bb183b77d8f2dd6bd67b8da559a90ac1e74b2052.1534868459.git.jcody@redhat.com
+Signed-off-by: Jeff Cody <jcody@redhat.com>
+---
+ job.c | 2 +-
+file changed, 1 insertion(+), 1 deletion(-)
+diff --git a/job.c b/job.c
+index XXXXXXX..XXXXXXX 100644
+--- a/job.c
++++ b/job.c
+@@ -XXX,XX +XXX,XX @@ static void job_cancel_async(Job *job, bool force)
+ {
+     if (job->user_paused) {
+         /* Do not call job_enter here, the caller will handle it.  */
+-        job->user_paused = false;
+         if (job->driver->user_resume) {
+             job->driver->user_resume(job);
+         }
++        job->user_paused = false;
+         assert(job->pause_count > 0);
+         job->pause_count--;
+     }
+--
+.17.1

-[Qemu-devel] [PULL 1/1] block/gluster.c: Handle qdict_array_entries() failure
+[Qemu-devel] [PULL 2/2] block: iotest to catch abort on forced blockjob cancel
-From: Peter Maydell <peter.maydell@linaro.org>
+Signed-off-by: Jeff Cody <jcody@redhat.com>
+Reviewed-by: John Snow <jsnow@redhat.com>
-In qemu_gluster_parse_json(), the call to qdict_array_entries()
+Message-id: df317f617fbe5affcf699cb8560e7b0c2e028a64.1534868459.git.jcody@redhat.com
 could return a negative error code, which we were ignoring
 because we assigned the result to an unsigned variable.
 Fix this by using the 'int' type instead, which matches the
 return type of qdict_array_entries() and also the type
 we use for the loop enumeration variable 'i'.
 (Spotted by Coverity, CID 1360960.)
 Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
 Reviewed-by: Eric Blake <eblake@redhat.com>
 Reviewed-by: Jeff Cody <jcody@redhat.com>
 Message-id: 1496682098-1540-1-git-send-email-peter.maydell@linaro.org
 Signed-off-by: Jeff Cody <jcody@redhat.com>
 ---
- block/gluster.c | 3 +--
+ tests/qemu-iotests/229     | 95 ++++++++++++++++++++++++++++++++++++++
-file changed, 1 insertion(+), 2 deletions(-)
+ tests/qemu-iotests/229.out | 23 +++++++++
  tests/qemu-iotests/group   |  1 +
 files changed, 119 insertions(+)
  create mode 100755 tests/qemu-iotests/229
  create mode 100644 tests/qemu-iotests/229.out
-diff --git a/block/gluster.c b/block/gluster.c
+diff --git a/tests/qemu-iotests/229 b/tests/qemu-iotests/229
 new file mode 100755
 index XXXXXXX..XXXXXXX
 --- /dev/null
 +++ b/tests/qemu-iotests/229
@@ -XXX,XX +XXX,XX @@
 +#!/bin/bash
 +#
 +# Test for force canceling a running blockjob that is paused in
 +# an error state.
 +#
 +# Copyright (C) 2018 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/>.
 +#
 +
 +# creator
 +owner=jcody@redhat.com
 +
 +seq="$(basename $0)"
 +echo "QA output created by $seq"
 +
 +here="$PWD"
 +status=1    # failure is the default!
 +
 +_cleanup()
 +{
 +    _cleanup_qemu
 +    _cleanup_test_img
 +    rm -f "$TEST_IMG" "$DEST_IMG"
 +}
 +trap "_cleanup; exit \$status" 0 1 2 3 15
 +
 +# get standard environment, filters and checks
 +. ./common.rc
 +. ./common.filter
 +. ./common.qemu
 +
 +# Needs backing file and backing format support
 +_supported_fmt qcow2 qed
 +_supported_proto file
 +_supported_os Linux
 +
 +
 +DEST_IMG="$TEST_DIR/d.$IMGFMT"
 +TEST_IMG="$TEST_DIR/b.$IMGFMT"
 +
 +_make_test_img 2M
 +
 +# destination for mirror will be too small, causing error
 +TEST_IMG=$DEST_IMG _make_test_img 1M
 +
 +$QEMU_IO -c 'write 0 2M' "$TEST_IMG" | _filter_qemu_io
 +
 +_launch_qemu -drive id=testdisk,file="$TEST_IMG",format="$IMGFMT"
 +
 +_send_qemu_cmd $QEMU_HANDLE \
 +    "{'execute': 'qmp_capabilities'}" \
 +    'return'
 +
 +echo
 +echo '=== Starting drive-mirror, causing error & stop  ==='
 +echo
 +
 +_send_qemu_cmd $QEMU_HANDLE \
 +    "{'execute': 'drive-mirror',
 +                 'arguments': {'device': 'testdisk',
 +                               'mode':   'absolute-paths',
 +                               'format': '$IMGFMT',
 +                               'target': '$DEST_IMG',
 +                               'sync':   'full',
 +                               'mode':   'existing',
 +                               'on-source-error': 'stop',
 +                               'on-target-error': 'stop' }}"    \
 +     "JOB_STATUS_CHANGE.*pause"
 +
 +echo
 +echo '=== Force cancel job paused in error state  ==='
 +echo
 +
 +success_or_failure="y" _send_qemu_cmd $QEMU_HANDLE \
 +    "{'execute': 'block-job-cancel',
 +                 'arguments': { 'device': 'testdisk',
 +                                'force': true}}" \
 +     "BLOCK_JOB_CANCELLED" "Assertion"
 +
 +# success, all done
 +echo "*** done"
 +rm -f $seq.full
 +status=0
 diff --git a/tests/qemu-iotests/229.out b/tests/qemu-iotests/229.out
 new file mode 100644
 index XXXXXXX..XXXXXXX
 --- /dev/null
 +++ b/tests/qemu-iotests/229.out
@@ -XXX,XX +XXX,XX @@
 +QA output created by 229
 +Formatting 'TEST_DIR/b.IMGFMT', fmt=IMGFMT size=2097152
 +Formatting 'TEST_DIR/d.IMGFMT', fmt=IMGFMT size=1048576
 +wrote 2097152/2097152 bytes at offset 0
 +2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 +{"return": {}}
 +
 +=== Starting drive-mirror, causing error & stop  ===
 +
 +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "testdisk"}}
 +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "testdisk"}}
 +{"return": {}}
 +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "testdisk", "operation": "write", "action": "stop"}}
 +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "paused", "id": "testdisk"}}
 +
 +=== Force cancel job paused in error state  ===
 +
 +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "testdisk"}}
 +{"return": {}}
 +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "testdisk", "operation": "write", "action": "stop"}}
 +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "aborting", "id": "testdisk"}}
 +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "testdisk", "len": 2097152, "offset": 1048576, "speed": 0, "type": "mirror"}}
 +*** done
 diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
 index XXXXXXX..XXXXXXX 100644
---- a/block/gluster.c
+--- a/tests/qemu-iotests/group
-+++ b/block/gluster.c
++++ b/tests/qemu-iotests/group
-@@ -XXX,XX +XXX,XX @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
+@@ -XXX,XX +XXX,XX @@
-     Error *local_err = NULL;
+rw auto quick
-     char *str = NULL;
+auto quick
-     const char *ptr;
+auto quick
--    size_t num_servers;
++229 auto quick
 -    int i, type;
 +    int i, type, num_servers;
      /* create opts info from runtime_json_opts list */
      opts = qemu_opts_create(&runtime_json_opts, NULL, 0, &error_abort);
 --
-.9.3
+.17.1

From: Peter Maydell <peter.maydell@linaro.org>

In qemu_gluster_parse_json(), the call to qdict_array_entries()
could return a negative error code, which we were ignoring
because we assigned the result to an unsigned variable.
Fix this by using the 'int' type instead, which matches the
return type of qdict_array_entries() and also the type
we use for the loop enumeration variable 'i'.

(Spotted by Coverity, CID 1360960.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Message-id: 1496682098-1540-1-git-send-email-peter.maydell@linaro.org
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/gluster.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index XXXXXXX..XXXXXXX 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -XXX,XX +XXX,XX @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
     Error *local_err = NULL;
     char *str = NULL;
     const char *ptr;
-    size_t num_servers;
-    int i, type;
+    int i, type, num_servers;
 
     /* create opts info from runtime_json_opts list */
     opts = qemu_opts_create(&runtime_json_opts, NULL, 0, &error_abort);
-- 
2.9.3

The following changes since commit ee135aa0428fe5af2af7be04ff16d2b596a9330a:

Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-3.1-20180821' into staging (2018-08-21 13:27:11 +0100)

are available in the Git repository at:

git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request

for you to fetch changes up to 26bf474ba92c76e61bea51726e22da6dfd185296:

block: iotest to catch abort on forced blockjob cancel (2018-08-21 15:20:37 -0400)

----------------------------------------------------------------
Block job patches
----------------------------------------------------------------

Jeff Cody (2):
  block: for jobs, do not clear user_paused until after the resume
  block: iotest to catch abort on forced blockjob cancel

job.c                      |  2 +-
 tests/qemu-iotests/229     | 95 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/229.out | 23 +++++++++
 tests/qemu-iotests/group   |  1 +
 4 files changed, 120 insertions(+), 1 deletion(-)
 create mode 100755 tests/qemu-iotests/229
 create mode 100644 tests/qemu-iotests/229.out

-- 
2.17.1

The function job_cancel_async() will always cause an assert for blockjob
user resume.  We set job->user_paused to false, and then call
job->driver->user_resume().  In the case of blockjobs, this is the
block_job_user_resume() function.

In that function, we assert that job.user_paused is set to true.
Unfortunately, right before calling this function, it has explicitly
been set to false.

The fix is pretty simple: set job->user_paused to false only after the
job user_resume() function has been called.

Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
Message-id: bb183b77d8f2dd6bd67b8da559a90ac1e74b2052.1534868459.git.jcody@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 job.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/job.c b/job.c
index XXXXXXX..XXXXXXX 100644
--- a/job.c
+++ b/job.c
@@ -XXX,XX +XXX,XX @@ static void job_cancel_async(Job *job, bool force)
 {
     if (job->user_paused) {
         /* Do not call job_enter here, the caller will handle it.  */
-        job->user_paused = false;
         if (job->driver->user_resume) {
             job->driver->user_resume(job);
         }
+        job->user_paused = false;
         assert(job->pause_count > 0);
         job->pause_count--;
     }
-- 
2.17.1

Signed-off-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: df317f617fbe5affcf699cb8560e7b0c2e028a64.1534868459.git.jcody@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 tests/qemu-iotests/229     | 95 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/229.out | 23 +++++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 119 insertions(+)
 create mode 100755 tests/qemu-iotests/229
 create mode 100644 tests/qemu-iotests/229.out

diff --git a/tests/qemu-iotests/229 b/tests/qemu-iotests/229
new file mode 100755
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/229
@@ -XXX,XX +XXX,XX @@
+#!/bin/bash
+#
+# Test for force canceling a running blockjob that is paused in
+# an error state.
+#
+# Copyright (C) 2018 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/>.
+#
+
+# creator
+owner=jcody@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+status=1	# failure is the default!
+
+_cleanup()
+{
+    _cleanup_qemu
+    _cleanup_test_img
+    rm -f "$TEST_IMG" "$DEST_IMG"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+# Needs backing file and backing format support
+_supported_fmt qcow2 qed
+_supported_proto file
+_supported_os Linux
+
+
+DEST_IMG="$TEST_DIR/d.$IMGFMT"
+TEST_IMG="$TEST_DIR/b.$IMGFMT"
+
+_make_test_img 2M
+
+# destination for mirror will be too small, causing error
+TEST_IMG=$DEST_IMG _make_test_img 1M
+
+$QEMU_IO -c 'write 0 2M' "$TEST_IMG" | _filter_qemu_io
+
+_launch_qemu -drive id=testdisk,file="$TEST_IMG",format="$IMGFMT"
+
+_send_qemu_cmd $QEMU_HANDLE \
+    "{'execute': 'qmp_capabilities'}" \
+    'return'
+
+echo
+echo '=== Starting drive-mirror, causing error & stop  ==='
+echo
+
+_send_qemu_cmd $QEMU_HANDLE \
+    "{'execute': 'drive-mirror',
+                 'arguments': {'device': 'testdisk',
+                               'mode':   'absolute-paths',
+                               'format': '$IMGFMT',
+                               'target': '$DEST_IMG',
+                               'sync':   'full',
+                               'mode':   'existing',
+                               'on-source-error': 'stop',
+                               'on-target-error': 'stop' }}"    \
+     "JOB_STATUS_CHANGE.*pause"
+
+echo
+echo '=== Force cancel job paused in error state  ==='
+echo
+
+success_or_failure="y" _send_qemu_cmd $QEMU_HANDLE \
+    "{'execute': 'block-job-cancel',
+                 'arguments': { 'device': 'testdisk',
+                                'force': true}}" \
+     "BLOCK_JOB_CANCELLED" "Assertion"
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/229.out b/tests/qemu-iotests/229.out
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/229.out
@@ -XXX,XX +XXX,XX @@
+QA output created by 229
+Formatting 'TEST_DIR/b.IMGFMT', fmt=IMGFMT size=2097152
+Formatting 'TEST_DIR/d.IMGFMT', fmt=IMGFMT size=1048576
+wrote 2097152/2097152 bytes at offset 0
+2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": {}}
+
+=== Starting drive-mirror, causing error & stop  ===
+
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "testdisk"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "testdisk"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "testdisk", "operation": "write", "action": "stop"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "paused", "id": "testdisk"}}
+
+=== Force cancel job paused in error state  ===
+
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "testdisk"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "testdisk", "operation": "write", "action": "stop"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "aborting", "id": "testdisk"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "testdisk", "len": 2097152, "offset": 1048576, "speed": 0, "type": "mirror"}}
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -XXX,XX +XXX,XX @@
 225 rw auto quick
 226 auto quick
 227 auto quick
+229 auto quick
-- 
2.17.1