Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging

Block layer patches:

- Relax restrictions for blockdev-snapshot (allows libvirt to do live
  storage migration with blockdev-mirror)
- luks: Delete created files when block_crypto_co_create_opts_luks fails
- Fix memleaks in qmp_object_add

# gpg: Signature made Wed 11 Mar 2020 15:38:59 GMT
# gpg:                using RSA key 7F09B272C88F2FD6
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full]
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74  56FE 7F09 B272 C88F 2FD6

* remotes/kevin/tags/for-upstream:
  qemu-iotests: adding LUKS cleanup for non-UTF8 secret error
  crypto.c: cleanup created file when block_crypto_co_create_opts_luks fails
  block.c: adding bdrv_co_delete_file
  block: introducing 'bdrv_co_delete_file' interface
  tests/qemu-iotests: Fix socket_scm_helper build path
  qapi: Add '@allow-write-only-overlay' feature for 'blockdev-snapshot'
  iotests: Add iothread cases to 155
  block: Fix cross-AioContext blockdev-snapshot
  iotests: Test mirror with temporarily disabled target backing file
  iotests: Fix run_job() with use_log=False
  block: Relax restrictions for blockdev-snapshot
  block: Make bdrv_get_cumulative_perm() public
  qom-qmp-cmds: fix two memleaks in qmp_object_add

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

17 files changed, 262 insertions, 57 deletions

diff --git a/block.c b/block.c
index 957630b1c5..a2542c977b 100644
--- a/block.c
+++ b/block.c
@@ -668,6 +668,32 @@ int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp)
     }
 }
 
+int coroutine_fn bdrv_co_delete_file(BlockDriverState *bs, Error **errp)
+{
+    Error *local_err = NULL;
+    int ret;
+
+    assert(bs != NULL);
+
+    if (!bs->drv) {
+        error_setg(errp, "Block node '%s' is not opened", bs->filename);
+        return -ENOMEDIUM;
+    }
+
+    if (!bs->drv->bdrv_co_delete_file) {
+        error_setg(errp, "Driver '%s' does not support image deletion",
+                   bs->drv->format_name);
+        return -ENOTSUP;
+    }
+
+    ret = bs->drv->bdrv_co_delete_file(bs, &local_err);
+    if (ret < 0) {
+        error_propagate(errp, local_err);
+    }
+
+    return ret;
+}
+
 /**
  * Try to get @bs's logical and physical block size.
  * On success, store them in @bsz struct and return 0.
@@ -1872,8 +1898,6 @@ static int bdrv_child_check_perm(BdrvChild *c, BlockReopenQueue *q,
                                  bool *tighten_restrictions, Error **errp);
 static void bdrv_child_abort_perm_update(BdrvChild *c);
 static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared);
-static void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
-                                     uint64_t *shared_perm);
 
 typedef struct BlockReopenQueueEntry {
      bool prepared;
@@ -2097,8 +2121,8 @@ static void bdrv_set_perm(BlockDriverState *bs, uint64_t cumulative_perms,
     }
 }
 
-static void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
-                                     uint64_t *shared_perm)
+void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
+                              uint64_t *shared_perm)
 {
     BdrvChild *c;
     uint64_t cumulative_perms = 0;
@@ -4367,6 +4391,7 @@ void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
     bdrv_ref(from);
 
     assert(qemu_get_current_aio_context() == qemu_get_aio_context());
+    assert(bdrv_get_aio_context(from) == bdrv_get_aio_context(to));
     bdrv_drained_begin(from);
 
     /* Put all parents into @list and calculate their cumulative permissions */
diff --git a/block/crypto.c b/block/crypto.c
index 23e9c74d6f..4425ebeb47 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -30,6 +30,7 @@
 #include "qapi/error.h"
 #include "qemu/module.h"
 #include "qemu/option.h"
+#include "qemu/cutils.h"
 #include "crypto.h"
 
 typedef struct BlockCrypto BlockCrypto;
@@ -657,6 +658,23 @@ static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename,
 
     ret = 0;
 fail:
+    /*
+     * If an error occurred, delete 'filename'. Even if the file existed
+     * beforehand, it has been truncated and corrupted in the process.
+     */
+    if (ret && bs) {
+        Error *local_delete_err = NULL;
+        int r_del = bdrv_co_delete_file(bs, &local_delete_err);
+        /*
+         * ENOTSUP will happen if the block driver doesn't support
+         * the 'bdrv_co_delete_file' interface. This is a predictable
+         * scenario and shouldn't be reported back to the user.
+         */
+        if ((r_del < 0) && (r_del != -ENOTSUP)) {
+            error_report_err(local_delete_err);
+        }
+    }
+
     bdrv_unref(bs);
     qapi_free_QCryptoBlockCreateOptions(create_opts);
     qobject_unref(cryptoopts);
diff --git a/block/file-posix.c b/block/file-posix.c
index 0f77447a25..9bc3838b2a 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2445,6 +2445,28 @@ static int coroutine_fn raw_co_create_opts(const char *filename, QemuOpts *opts,
     return raw_co_create(&options, errp);
 }
 
+static int coroutine_fn raw_co_delete_file(BlockDriverState *bs,
+                                           Error **errp)
+{
+    struct stat st;
+    int ret;
+
+    if (!(stat(bs->filename, &st) == 0) || !S_ISREG(st.st_mode)) {
+        error_setg_errno(errp, ENOENT, "%s is not a regular file",
+                         bs->filename);
+        return -ENOENT;
+    }
+
+    ret = unlink(bs->filename);
+    if (ret < 0) {
+        ret = -errno;
+        error_setg_errno(errp, -ret, "Error when deleting file %s",
+                         bs->filename);
+    }
+
+    return ret;
+}
+
 /*
  * Find allocation range in @bs around offset @start.
  * May change underlying file descriptor's file offset.
@@ -3075,6 +3097,7 @@ BlockDriver bdrv_file = {
     .bdrv_co_block_status = raw_co_block_status,
     .bdrv_co_invalidate_cache = raw_co_invalidate_cache,
     .bdrv_co_pwrite_zeroes = raw_co_pwrite_zeroes,
+    .bdrv_co_delete_file = raw_co_delete_file,
 
     .bdrv_co_preadv         = raw_co_preadv,
     .bdrv_co_pwritev        = raw_co_pwritev,
diff --git a/blockdev.c b/blockdev.c
index 257cb37682..fa8630cb41 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1470,8 +1470,7 @@ static void external_snapshot_prepare(BlkActionState *common,
                              DO_UPCAST(ExternalSnapshotState, common, common);
     TransactionAction *action = common->action;
     AioContext *aio_context;
-    AioContext *old_context;
-    int ret;
+    uint64_t perm, shared;
 
     /* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar
      * purpose but a different set of parameters */
@@ -1586,16 +1585,17 @@ static void external_snapshot_prepare(BlkActionState *common,
         goto out;
     }
 
-    if (bdrv_has_blk(state->new_bs)) {
+    /*
+     * Allow attaching a backing file to an overlay that's already in use only
+     * if the parents don't assume that they are already seeing a valid image.
+     * (Specifically, allow it as a mirror target, which is write-only access.)
+     */
+    bdrv_get_cumulative_perm(state->new_bs, &perm, &shared);
+    if (perm & BLK_PERM_CONSISTENT_READ) {
         error_setg(errp, "The overlay is already in use");
         goto out;
     }
 
-    if (bdrv_op_is_blocked(state->new_bs, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT,
-                           errp)) {
-        goto out;
-    }
-
     if (state->new_bs->backing != NULL) {
         error_setg(errp, "The overlay already has a backing image");
         goto out;
@@ -1606,20 +1606,6 @@ static void external_snapshot_prepare(BlkActionState *common,
         goto out;
     }
 
-    /* Honor bdrv_try_set_aio_context() context acquisition requirements. */
-    old_context = bdrv_get_aio_context(state->new_bs);
-    aio_context_release(aio_context);
-    aio_context_acquire(old_context);
-
-    ret = bdrv_try_set_aio_context(state->new_bs, aio_context, errp);
-
-    aio_context_release(old_context);
-    aio_context_acquire(aio_context);
-
-    if (ret < 0) {
-        goto out;
-    }
-
     /* This removes our old bs and adds the new bs. This is an operation that
      * can fail, so we need to do it in .prepare; undoing it for abort is
      * always possible. */
diff --git a/include/block/block.h b/include/block/block.h
index cd6b5b95aa..e569a4d747 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -363,6 +363,7 @@ bool bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState *base,
 int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base,
                               Error **errp);
 void bdrv_unfreeze_backing_chain(BlockDriverState *bs, BlockDriverState *base);
+int coroutine_fn bdrv_co_delete_file(BlockDriverState *bs, Error **errp);
 
 
 typedef struct BdrvCheckResult {
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 3f70a98b2d..ae9c4da4d0 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -314,6 +314,10 @@ struct BlockDriver {
      */
     int coroutine_fn (*bdrv_co_flush)(BlockDriverState *bs);
 
+    /* Delete a created file. */
+    int coroutine_fn (*bdrv_co_delete_file)(BlockDriverState *bs,
+                                            Error **errp);
+
     /*
      * Flushes all data that was already written to the OS all the way down to
      * the disk (for example file-posix.c calls fsync()).
@@ -1224,6 +1228,9 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
                                   void *opaque, Error **errp);
 void bdrv_root_unref_child(BdrvChild *child);
 
+void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
+                              uint64_t *shared_perm);
+
 /**
  * Sets a BdrvChild's permissions.  Avoid if the parent is a BDS; use
  * bdrv_child_refresh_perms() instead and make the parent's
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9758fc48d2..91586fb1fb 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1472,6 +1472,12 @@
 #
 # For the arguments, see the documentation of BlockdevSnapshot.
 #
+# Features:
+# @allow-write-only-overlay: If present, the check whether this operation is safe
+#                            was relaxed so that it can be used to change
+#                            backing file of a destination of a blockdev-mirror.
+#                            (since 5.0)
+#
 # Since: 2.5
 #
 # Example:
@@ -1492,7 +1498,8 @@
 #
 ##
 { 'command': 'blockdev-snapshot',
-  'data': 'BlockdevSnapshot' }
+  'data': 'BlockdevSnapshot',
+  'features': [ 'allow-write-only-overlay' ] }
 
 ##
 # @change-backing-file:
diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index 49db926fcc..435193b036 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -247,26 +247,22 @@ void qmp_object_add(QDict *qdict, QObject **ret_data, Error **errp)
     QDict *pdict;
     Visitor *v;
     Object *obj;
-    const char *type;
-    const char *id;
+    g_autofree char *type = NULL;
+    g_autofree char *id = NULL;
 
-    type = qdict_get_try_str(qdict, "qom-type");
+    type = g_strdup(qdict_get_try_str(qdict, "qom-type"));
     if (!type) {
         error_setg(errp, QERR_MISSING_PARAMETER, "qom-type");
         return;
-    } else {
-        type = g_strdup(type);
-        qdict_del(qdict, "qom-type");
     }
+    qdict_del(qdict, "qom-type");
 
-    id = qdict_get_try_str(qdict, "id");
+    id = g_strdup(qdict_get_try_str(qdict, "id"));
     if (!id) {
         error_setg(errp, QERR_MISSING_PARAMETER, "id");
         return;
-    } else {
-        id = g_strdup(id);
-        qdict_del(qdict, "id");
     }
+    qdict_del(qdict, "id");
 
     props = qdict_get(qdict, "props");
     if (props) {
diff --git a/tests/Makefile.include b/tests/Makefile.include
index edcbd475aa..67e8fcddda 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -589,6 +589,7 @@ include $(SRC_PATH)/tests/qtest/Makefile.include
 tests/test-qga$(EXESUF): qemu-ga$(EXESUF)
 tests/test-qga$(EXESUF): tests/test-qga.o $(qtest-obj-y)
 tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o $(test-util-obj-y) libvhost-user.a
+tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
 
 SPEED = quick
 
diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out
index d94ad22f70..fd11aae678 100644
--- a/tests/qemu-iotests/085.out
+++ b/tests/qemu-iotests/085.out
@@ -82,7 +82,7 @@ Formatting 'TEST_DIR/12-snapshot-v0.IMGFMT', fmt=IMGFMT size=134217728 backing_f
 === Invalid command - cannot create a snapshot using a file BDS ===
 
 { 'execute': 'blockdev-snapshot', 'arguments': { 'node':'virtio0', 'overlay':'file_12' } }
-{"error": {"class": "GenericError", "desc": "The overlay does not support backing images"}}
+{"error": {"class": "GenericError", "desc": "The overlay is already in use"}}
 
 === Invalid command - snapshot node used as active layer ===
 
@@ -96,7 +96,7 @@ Formatting 'TEST_DIR/12-snapshot-v0.IMGFMT', fmt=IMGFMT size=134217728 backing_f
 === Invalid command - snapshot node used as backing hd ===
 
 { 'execute': 'blockdev-snapshot', 'arguments': { 'node': 'virtio0', 'overlay':'snap_11' } }
-{"error": {"class": "GenericError", "desc": "Node 'snap_11' is busy: node is used as backing hd of 'snap_12'"}}
+{"error": {"class": "GenericError", "desc": "The overlay is already in use"}}
 
 === Invalid command - snapshot node has a backing image ===
 
diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155
index f237868710..571bce9de4 100755
--- a/tests/qemu-iotests/155
+++ b/tests/qemu-iotests/155
@@ -45,10 +45,18 @@ target_img = os.path.join(iotests.test_dir, 'target.' + iotests.imgfmt)
 #                      image during runtime, only makes sense if
 #                      target_blockdev_backing is not None
 #                      (None: same as target_backing)
+# target_open_with_backing: If True, the target image is added with its backing
+#                           chain opened right away. If False, blockdev-add
+#                           opens it without a backing file and job completion
+#                           is supposed to open the backing chain.
+# use_iothread: If True, an iothread is configured for the virtio-blk device
+#               that uses the image being mirrored
 
 class BaseClass(iotests.QMPTestCase):
     target_blockdev_backing = None
     target_real_backing = None
+    target_open_with_backing = True
+    use_iothread = False
 
     def setUp(self):
         qemu_img('create', '-f', iotests.imgfmt, back0_img, '1440K')
@@ -64,7 +72,16 @@ class BaseClass(iotests.QMPTestCase):
                     'file': {'driver': 'file',
                              'filename': source_img}}
         self.vm.add_blockdev(self.vm.qmp_to_opts(blockdev))
-        self.vm.add_device('virtio-blk,id=qdev0,drive=source')
+
+        if self.use_iothread:
+            self.vm.add_object('iothread,id=iothread0')
+            iothread = ",iothread=iothread0"
+        else:
+            iothread = ""
+
+        self.vm.add_device('virtio-scsi%s' % iothread)
+        self.vm.add_device('scsi-hd,id=qdev0,drive=source')
+
         self.vm.launch()
 
         self.assertIntactSourceBackingChain()
@@ -80,9 +97,13 @@ class BaseClass(iotests.QMPTestCase):
                 options = { 'node-name': 'target',
                             'driver': iotests.imgfmt,
                             'file': { 'driver': 'file',
+                                      'node-name': 'target-file',
                                       'filename': target_img } }
-                if self.target_blockdev_backing:
-                    options['backing'] = self.target_blockdev_backing
+
+                if not self.target_open_with_backing:
+                        options['backing'] = None
+                elif self.target_blockdev_backing:
+                        options['backing'] = self.target_blockdev_backing
 
                 result = self.vm.qmp('blockdev-add', **options)
                 self.assert_qmp(result, 'return', {})
@@ -147,10 +168,14 @@ class BaseClass(iotests.QMPTestCase):
 # cmd: Mirroring command to execute, either drive-mirror or blockdev-mirror
 
 class MirrorBaseClass(BaseClass):
+    def openBacking(self):
+        pass
+
     def runMirror(self, sync):
         if self.cmd == 'blockdev-mirror':
             result = self.vm.qmp(self.cmd, job_id='mirror-job', device='source',
-                                 sync=sync, target='target')
+                                 sync=sync, target='target',
+                                 auto_finalize=False)
         else:
             if self.existing:
                 mode = 'existing'
@@ -159,33 +184,31 @@ class MirrorBaseClass(BaseClass):
             result = self.vm.qmp(self.cmd, job_id='mirror-job', device='source',
                                  sync=sync, target=target_img,
                                  format=iotests.imgfmt, mode=mode,
-                                 node_name='target')
+                                 node_name='target', auto_finalize=False)
 
         self.assert_qmp(result, 'return', {})
 
-        self.complete_and_wait('mirror-job')
+        self.vm.run_job('mirror-job', use_log=False, auto_finalize=False,
+                        pre_finalize=self.openBacking, auto_dismiss=True)
 
     def testFull(self):
         self.runMirror('full')
 
-        node = self.findBlockNode('target',
-                                  '/machine/peripheral/qdev0/virtio-backend')
+        node = self.findBlockNode('target', 'qdev0')
         self.assertCorrectBackingImage(node, None)
         self.assertIntactSourceBackingChain()
 
     def testTop(self):
         self.runMirror('top')
 
-        node = self.findBlockNode('target',
-                                  '/machine/peripheral/qdev0/virtio-backend')
+        node = self.findBlockNode('target', 'qdev0')
         self.assertCorrectBackingImage(node, back2_img)
         self.assertIntactSourceBackingChain()
 
     def testNone(self):
         self.runMirror('none')
 
-        node = self.findBlockNode('target',
-                                  '/machine/peripheral/qdev0/virtio-backend')
+        node = self.findBlockNode('target', 'qdev0')
         self.assertCorrectBackingImage(node, source_img)
         self.assertIntactSourceBackingChain()
 
@@ -221,6 +244,44 @@ class TestBlockdevMirrorForcedBacking(MirrorBaseClass):
     target_blockdev_backing = { 'driver': 'null-co' }
     target_real_backing = 'null-co://'
 
+# Attach the backing chain only during completion, with blockdev-reopen
+class TestBlockdevMirrorReopen(MirrorBaseClass):
+    cmd = 'blockdev-mirror'
+    existing = True
+    target_backing = 'null-co://'
+    target_open_with_backing = False
+
+    def openBacking(self):
+        if not self.target_open_with_backing:
+            result = self.vm.qmp('blockdev-add', node_name="backing",
+                                 driver="null-co")
+            self.assert_qmp(result, 'return', {})
+            result = self.vm.qmp('x-blockdev-reopen', node_name="target",
+                                 driver=iotests.imgfmt, file="target-file",
+                                 backing="backing")
+            self.assert_qmp(result, 'return', {})
+
+class TestBlockdevMirrorReopenIothread(TestBlockdevMirrorReopen):
+    use_iothread = True
+
+# Attach the backing chain only during completion, with blockdev-snapshot
+class TestBlockdevMirrorSnapshot(MirrorBaseClass):
+    cmd = 'blockdev-mirror'
+    existing = True
+    target_backing = 'null-co://'
+    target_open_with_backing = False
+
+    def openBacking(self):
+        if not self.target_open_with_backing:
+            result = self.vm.qmp('blockdev-add', node_name="backing",
+                                 driver="null-co")
+            self.assert_qmp(result, 'return', {})
+            result = self.vm.qmp('blockdev-snapshot', node="backing",
+                                 overlay="target")
+            self.assert_qmp(result, 'return', {})
+
+class TestBlockdevMirrorSnapshotIothread(TestBlockdevMirrorSnapshot):
+    use_iothread = True
 
 class TestCommit(BaseClass):
     existing = False
@@ -237,8 +298,7 @@ class TestCommit(BaseClass):
 
         self.vm.event_wait('BLOCK_JOB_COMPLETED')
 
-        node = self.findBlockNode(None,
-                                  '/machine/peripheral/qdev0/virtio-backend')
+        node = self.findBlockNode(None, 'qdev0')
         self.assert_qmp(node, 'image' + '/backing-image' * 0 + '/filename',
                         back1_img)
         self.assert_qmp(node, 'image' + '/backing-image' * 1 + '/filename',
diff --git a/tests/qemu-iotests/155.out b/tests/qemu-iotests/155.out
index 4176bb9402..ed714d5263 100644
--- a/tests/qemu-iotests/155.out
+++ b/tests/qemu-iotests/155.out
@@ -1,5 +1,5 @@
-...................
+...............................
 ----------------------------------------------------------------------
-Ran 19 tests
+Ran 31 tests
 
 OK
diff --git a/tests/qemu-iotests/282 b/tests/qemu-iotests/282
new file mode 100755
index 0000000000..081eb12080
--- /dev/null
+++ b/tests/qemu-iotests/282
@@ -0,0 +1,67 @@
+#!/usr/bin/env bash
+#
+# Test qemu-img file cleanup for LUKS when using a non-UTF8 secret
+#
+# Copyright (C) 2020, IBM Corporation.
+#
+# 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/>.
+#
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1	# failure is the default!
+TEST_IMAGE_FILE='vol.img'
+
+_cleanup()
+{
+  _cleanup_test_img
+  rm non_utf8_secret
+  rm -f $TEST_IMAGE_FILE
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt luks
+_supported_proto generic
+_unsupported_proto vxhs
+
+echo "== Create non-UTF8 secret =="
+echo -n -e '\x3a\x3c\x3b\xff' > non_utf8_secret
+SECRET="secret,id=sec0,file=non_utf8_secret"
+
+echo "== Throws an error because of invalid UTF-8 secret =="
+$QEMU_IMG create -f $IMGFMT --object $SECRET -o "key-secret=sec0" $TEST_IMAGE_FILE 4M
+
+echo "== Image file should not exist after the error =="
+if test -f "$TEST_IMAGE_FILE"; then
+    exit 1
+fi
+
+echo "== Create a stub image file and run qemu-img again =="
+touch $TEST_IMAGE_FILE
+$QEMU_IMG create -f $IMGFMT --object $SECRET -o "key-secret=sec0" $TEST_IMAGE_FILE 4M
+
+echo "== Pre-existing image file should also be deleted after the error =="
+if test -f "$TEST_IMAGE_FILE"; then
+    exit 1
+fi
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/282.out b/tests/qemu-iotests/282.out
new file mode 100644
index 0000000000..5d079dabce
--- /dev/null
+++ b/tests/qemu-iotests/282.out
@@ -0,0 +1,11 @@
+QA output created by 282
+== Create non-UTF8 secret ==
+== Throws an error because of invalid UTF-8 secret ==
+qemu-img: vol.img: Data from secret sec0 is not valid UTF-8
+Formatting 'vol.img', fmt=luks size=4194304 key-secret=sec0
+== Image file should not exist after the error ==
+== Create a stub image file and run qemu-img again ==
+qemu-img: vol.img: Data from secret sec0 is not valid UTF-8
+Formatting 'vol.img', fmt=luks size=4194304 key-secret=sec0
+== Pre-existing image file should also be deleted after the error ==
+ *** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 559edc139a..ec2b2302e5 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -290,6 +290,7 @@
 279 rw backing quick
 280 rw migration quick
 281 rw quick
+282 rw img quick
 283 auto quick
 284 rw
 286 rw quick
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 8815052eb5..23043baa26 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -624,7 +624,10 @@ class VM(qtest.QEMUQtestMachine):
                         if use_log:
                             log('Job failed: %s' % (j['error']))
             elif status == 'ready':
-                self.qmp_log('job-complete', id=job)
+                if use_log:
+                    self.qmp_log('job-complete', id=job)
+                else:
+                    self.qmp('job-complete', id=job)
             elif status == 'pending' and not auto_finalize:
                 if pre_finalize:
                     pre_finalize()
diff --git a/tests/qtest/Makefile.include b/tests/qtest/Makefile.include
index 5115f7897d..10a28de8a3 100644
--- a/tests/qtest/Makefile.include
+++ b/tests/qtest/Makefile.include
@@ -288,7 +288,6 @@ tests/qtest/usb-hcd-ehci-test$(EXESUF): tests/qtest/usb-hcd-ehci-test.o $(libqos
 tests/qtest/usb-hcd-xhci-test$(EXESUF): tests/qtest/usb-hcd-xhci-test.o $(libqos-usb-obj-y)
 tests/qtest/cpu-plug-test$(EXESUF): tests/qtest/cpu-plug-test.o
 tests/qtest/migration-test$(EXESUF): tests/qtest/migration-test.o tests/qtest/migration-helpers.o
-tests/qtest/qemu-iotests/qtest/socket_scm_helper$(EXESUF): tests/qtest/qemu-iotests/qtest/socket_scm_helper.o
 tests/qtest/test-netfilter$(EXESUF): tests/qtest/test-netfilter.o $(qtest-obj-y)
 tests/qtest/test-filter-mirror$(EXESUF): tests/qtest/test-filter-mirror.o $(qtest-obj-y)
 tests/qtest/test-filter-redirector$(EXESUF): tests/qtest/test-filter-redirector.o $(qtest-obj-y)