Block layer patches

- file-posix: Fix aio=threads performance regression after enablign FUA
 - QMP query-block/query-named-block-nodes: Include child references
 - Graph locking cleanups and fixes around making drain GRAPH_UNLOCKED
 - qemu-img: Overhaul option handling and --help
 - iotests: add test for changing the 'drive' property via 'qom-set'
 -----BEGIN PGP SIGNATURE-----
 
 iQJFBAABCgAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAmh2pZkRHGt3b2xmQHJl
 ZGhhdC5jb20ACgkQfwmycsiPL9aloBAAkbT2DpdcPb5v6Jc02bDTjBBi//R03cTy
 0jjU3zvEEjukeA8d7lsQnvD2YwQgvIoOgH/MeNjdYizYh6sLIofTKukbKWMWiBpt
 ygs67IrlsmEqrb+i/xeLdRA1o7jzpJCutU/cQeWV/fUur9ovhjnIJvwiw2Z3uhBR
 QajjPAZcGALwNMauweMhqTX7U1+EpAe/OdtAfc2UgyJIFCyLN9onuQleZ1lCbxSt
 PCAUT/M8zcId2Tcb9Bw3p0mzDNG2AI2FYqGIKNoaWwFfK/SgS8NCUvgpIWGghoxs
 bMbmKMqJpZamsbO7bmEEGjj1Vs14vYVMbqys6N2Gux74RXCBGDleGAR3HNvV+3jR
 98AuoTOWZxb3Sfu0e+9xNE/+kWcJ0vmsy3sxkpZ6hkPz6fmrrJJYy8Kv2tcCAOCi
 qIJ4hwNx052f1tnyxvARHj+Hj1Q4PSeQl/MAISVeQNAQXoinxzCP/hGLF3PkdpgD
 6m/xwQ+qMnnblgn4s2ICPXOJAaWLTeB6Y6F34MG+Wgi/7sfKGwxDgRSLMnlNICsm
 PpbSlRy3n7tBTUq4gF3kbknxKeEPGUGw3sakX8fc0DJshs6nz/nKL4Ftwgiuuo3F
 HWR8icj1giifohJOF0KJEa1Q2H9jR6hYwcNpjd9d/OEz1q/3HtuYAiEM3CUygVad
 2cyZBHjNWLE=
 =A4ZH
 -----END PGP SIGNATURE-----

Merge tag 'for-upstream' of https://repo.or.cz/qemu/kevin into staging

Block layer patches

- file-posix: Fix aio=threads performance regression after enablign FUA
- QMP query-block/query-named-block-nodes: Include child references
- Graph locking cleanups and fixes around making drain GRAPH_UNLOCKED
- qemu-img: Overhaul option handling and --help
- iotests: add test for changing the 'drive' property via 'qom-set'

# -----BEGIN PGP SIGNATURE-----
#
# iQJFBAABCgAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAmh2pZkRHGt3b2xmQHJl
# ZGhhdC5jb20ACgkQfwmycsiPL9aloBAAkbT2DpdcPb5v6Jc02bDTjBBi//R03cTy
# 0jjU3zvEEjukeA8d7lsQnvD2YwQgvIoOgH/MeNjdYizYh6sLIofTKukbKWMWiBpt
# ygs67IrlsmEqrb+i/xeLdRA1o7jzpJCutU/cQeWV/fUur9ovhjnIJvwiw2Z3uhBR
# QajjPAZcGALwNMauweMhqTX7U1+EpAe/OdtAfc2UgyJIFCyLN9onuQleZ1lCbxSt
# PCAUT/M8zcId2Tcb9Bw3p0mzDNG2AI2FYqGIKNoaWwFfK/SgS8NCUvgpIWGghoxs
# bMbmKMqJpZamsbO7bmEEGjj1Vs14vYVMbqys6N2Gux74RXCBGDleGAR3HNvV+3jR
# 98AuoTOWZxb3Sfu0e+9xNE/+kWcJ0vmsy3sxkpZ6hkPz6fmrrJJYy8Kv2tcCAOCi
# qIJ4hwNx052f1tnyxvARHj+Hj1Q4PSeQl/MAISVeQNAQXoinxzCP/hGLF3PkdpgD
# 6m/xwQ+qMnnblgn4s2ICPXOJAaWLTeB6Y6F34MG+Wgi/7sfKGwxDgRSLMnlNICsm
# PpbSlRy3n7tBTUq4gF3kbknxKeEPGUGw3sakX8fc0DJshs6nz/nKL4Ftwgiuuo3F
# HWR8icj1giifohJOF0KJEa1Q2H9jR6hYwcNpjd9d/OEz1q/3HtuYAiEM3CUygVad
# 2cyZBHjNWLE=
# =A4ZH
# -----END PGP SIGNATURE-----
# gpg: Signature made Tue 15 Jul 2025 15:01:45 EDT
# gpg:                using RSA key DC3DEB159A9AF95D3D7456FE7F09B272C88F2FD6
# gpg:                issuer "kwolf@redhat.com"
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full]
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74  56FE 7F09 B272 C88F 2FD6

* tag 'for-upstream' of https://repo.or.cz/qemu/kevin: (57 commits)
  qemu-img: extend cvtnum() and use it in more places
  qemu-img: implement short --help, remove global help() function
  qemu-img: measure: refresh options/--help
  qemu-img: dd: refresh options/--help
  qemu-img: bitmap: refresh options/--help
  qemu-img: bench: refresh options/--help
  qemu-img: amend: refresh options/--help
  qemu-img: resize: refresh options/--help
  qemu-img: resize: do not always eat last argument
  qemu-img: rebase: refresh options/--help (short option change)
  qemu-img: snapshot: refresh options/--help
  qemu-img: snapshot: make -l (list) the default, simplify option handling
  qemu-img: snapshot: allow specifying -f fmt
  qemu-img: map: refresh options/--help
  qemu-img: info: refresh options/--help
  qemu-img: convert: refresh options/--help (short option change)
  qemu-img: compare: refresh options/--help
  qemu-img: compare: use helper function for --object
  qemu-img: commit: refresh options/--help
  qemu-img: simplify --repair error message
  ...

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

block.c

@@ -431,7 +431,7 @@ BlockDriverState *bdrv_new(void)
     bs->block_status_cache = g_new0(BdrvBlockStatusCache, 1);
 
     for (i = 0; i < bdrv_drain_all_count; i++) {
-        bdrv_drained_begin(bs);
+        bdrv_do_drained_begin_quiesce(bs, NULL);
     }
 
     QTAILQ_INSERT_TAIL(&all_bdrv_states, bs, bs_list);
@@ -1721,14 +1721,12 @@ bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv, const char *node_name,
 open_failed:
     bs->drv = NULL;
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     if (bs->file != NULL) {
         bdrv_unref_child(bs, bs->file);
         assert(!bs->file);
     }
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     g_free(bs->opaque);
     bs->opaque = NULL;
@@ -3572,8 +3570,7 @@ out:
  *
  * All block nodes must be drained.
  */
-int bdrv_set_backing_hd_drained(BlockDriverState *bs,
+int bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
-                                BlockDriverState *backing_hd,
                         Error **errp)
 {
     int ret;
@@ -3596,21 +3593,6 @@ out:
     return ret;
 }
 
-int bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
-                        Error **errp)
-{
-    int ret;
-    GLOBAL_STATE_CODE();
-
-    bdrv_drain_all_begin();
-    bdrv_graph_wrlock();
-    ret = bdrv_set_backing_hd_drained(bs, backing_hd, errp);
-    bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
-
-    return ret;
-}
-
 /*
  * Opens the backing file for a BlockDriverState if not yet open
  *
@@ -3636,7 +3618,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
     Error *local_err = NULL;
 
     GLOBAL_STATE_CODE();
-    GRAPH_RDLOCK_GUARD_MAINLOOP();
+
+    bdrv_graph_rdlock_main_loop();
 
     if (bs->backing != NULL) {
         goto free_exit;
@@ -3717,7 +3700,11 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
 
     /* Hook up the backing file link; drop our reference, bs owns the
      * backing_hd reference now */
+    bdrv_graph_rdunlock_main_loop();
+    bdrv_graph_wrlock_drained();
     ret = bdrv_set_backing_hd(bs, backing_hd, errp);
+    bdrv_graph_wrunlock();
+    bdrv_graph_rdlock_main_loop();
     bdrv_unref(backing_hd);
 
     if (ret < 0) {
@@ -3729,6 +3716,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
 free_exit:
     g_free(backing_filename);
     qobject_unref(tmp_parent_options);
+    bdrv_graph_rdunlock_main_loop();
     return ret;
 }
 
@@ -3778,13 +3766,12 @@ done:
     return bs;
 }
 
-static BdrvChild *bdrv_open_child_common(const char *filename,
+static BdrvChild * GRAPH_UNLOCKED
-                                         QDict *options, const char *bdref_key,
+bdrv_open_child_common(const char *filename, QDict *options,
-                                         BlockDriverState *parent,
+                       const char *bdref_key, BlockDriverState *parent,
                        const BdrvChildClass *child_class,
-                                         BdrvChildRole child_role,
+                       BdrvChildRole child_role, bool allow_none,
-                                         bool allow_none, bool parse_filename,
+                       bool parse_filename, Error **errp)
-                                         Error **errp)
 {
     BlockDriverState *bs;
     BdrvChild *child;
@@ -3797,12 +3784,10 @@ static BdrvChild *bdrv_open_child_common(const char *filename,
         return NULL;
     }
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     child = bdrv_attach_child(parent, bs, bdref_key, child_class, child_role,
                               errp);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     return child;
 }
@@ -5160,7 +5145,7 @@ static void GRAPH_UNLOCKED bdrv_reopen_abort(BDRVReopenState *reopen_state)
 }
 
 
-static void bdrv_close(BlockDriverState *bs)
+static void GRAPH_UNLOCKED bdrv_close(BlockDriverState *bs)
 {
     BdrvAioNotifier *ban, *ban_next;
     BdrvChild *child, *next;
@@ -5180,8 +5165,7 @@ static void bdrv_close(BlockDriverState *bs)
         bs->drv = NULL;
     }
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     QLIST_FOREACH_SAFE(child, &bs->children, next, next) {
         bdrv_unref_child(bs, child);
     }
@@ -5189,7 +5173,6 @@ static void bdrv_close(BlockDriverState *bs)
     assert(!bs->backing);
     assert(!bs->file);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     g_free(bs->opaque);
     bs->opaque = NULL;
@@ -5515,8 +5498,7 @@ int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
     assert(!bs_new->backing);
     bdrv_graph_rdunlock_main_loop();
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
 
     child = bdrv_attach_child_noperm(bs_new, bs_top, "backing",
                                      &child_of_bds, bdrv_backing_role(bs_new),
@@ -5537,7 +5519,6 @@ out:
 
     bdrv_refresh_limits(bs_top, NULL, NULL);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     return ret;
 }
@@ -7076,31 +7057,25 @@ bdrv_inactivate_recurse(BlockDriverState *bs, bool top_level)
     return 0;
 }
 
+/* All block nodes must be drained. */
 int bdrv_inactivate(BlockDriverState *bs, Error **errp)
 {
     int ret;
 
     GLOBAL_STATE_CODE();
 
-    bdrv_drain_all_begin();
-    bdrv_graph_rdlock_main_loop();
-
     if (bdrv_has_bds_parent(bs, true)) {
         error_setg(errp, "Node has active parent node");
-        ret = -EPERM;
+        return -EPERM;
-        goto out;
     }
 
     ret = bdrv_inactivate_recurse(bs, true);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Failed to inactivate node");
-        goto out;
+        return ret;
     }
 
-out:
+    return 0;
-    bdrv_graph_rdunlock_main_loop();
-    bdrv_drain_all_end();
-    return ret;
 }
 
 int bdrv_inactivate_all(void)

block/backup.c

@@ -498,12 +498,10 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
     block_copy_set_speed(bcs, speed);
 
     /* Required permissions are taken by copy-before-write filter target */
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     block_job_add_bdrv(&job->common, "target", target, 0, BLK_PERM_ALL,
                        &error_abort);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     return &job->common;
 

block/blklogwrites.c

@@ -281,11 +281,9 @@ static int blk_log_writes_open(BlockDriverState *bs, QDict *options, int flags,
     ret = 0;
 fail_log:
     if (ret < 0) {
-        bdrv_drain_all_begin();
+        bdrv_graph_wrlock_drained();
-        bdrv_graph_wrlock();
         bdrv_unref_child(bs, s->log_file);
         bdrv_graph_wrunlock();
-        bdrv_drain_all_end();
         s->log_file = NULL;
         qemu_mutex_destroy(&s->mutex);
     }
@@ -298,12 +296,10 @@ static void blk_log_writes_close(BlockDriverState *bs)
 {
     BDRVBlkLogWritesState *s = bs->opaque;
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     bdrv_unref_child(bs, s->log_file);
     s->log_file = NULL;
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
     qemu_mutex_destroy(&s->mutex);
 }
 

block/blkverify.c

@@ -151,12 +151,10 @@ static void blkverify_close(BlockDriverState *bs)
 {
     BDRVBlkverifyState *s = bs->opaque;
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     bdrv_unref_child(bs, s->test_file);
     s->test_file = NULL;
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 }
 
 static int64_t coroutine_fn GRAPH_RDLOCK

block/block-backend.c

@@ -889,11 +889,9 @@ void blk_remove_bs(BlockBackend *blk)
     root = blk->root;
     blk->root = NULL;
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     bdrv_root_unref_child(root);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 }
 
 /*
@@ -906,8 +904,7 @@ int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp)
 
     GLOBAL_STATE_CODE();
     bdrv_ref(bs);
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
 
     if ((bs->open_flags & BDRV_O_INACTIVE) && blk_can_inactivate(blk)) {
         blk->disable_perm = true;
@@ -922,7 +919,6 @@ int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp)
                                        BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
                                        perm, shared_perm, blk, errp);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
     if (blk->root == NULL) {
         return -EPERM;
     }

block/commit.c

@@ -68,7 +68,7 @@ static int commit_prepare(Job *job)
                                   s->backing_mask_protocol);
 }
 
-static void commit_abort(Job *job)
+static void GRAPH_UNLOCKED commit_abort(Job *job)
 {
     CommitBlockJob *s = container_of(job, CommitBlockJob, common.job);
     BlockDriverState *top_bs = blk_bs(s->top);
@@ -392,8 +392,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
      * this is the responsibility of the interface (i.e. whoever calls
      * commit_start()).
      */
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     s->base_overlay = bdrv_find_overlay(top, base);
     assert(s->base_overlay);
 
@@ -425,21 +424,18 @@ void commit_start(const char *job_id, BlockDriverState *bs,
                                  iter_shared_perms, errp);
         if (ret < 0) {
             bdrv_graph_wrunlock();
-            bdrv_drain_all_end();
             goto fail;
         }
     }
 
     if (bdrv_freeze_backing_chain(commit_top_bs, base, errp) < 0) {
         bdrv_graph_wrunlock();
-        bdrv_drain_all_end();
         goto fail;
     }
     s->chain_frozen = true;
 
     ret = block_job_add_bdrv(&s->common, "base", base, 0, BLK_PERM_ALL, errp);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     if (ret < 0) {
         goto fail;
@@ -518,28 +514,32 @@ int bdrv_commit(BlockDriverState *bs)
     Error *local_err = NULL;
 
     GLOBAL_STATE_CODE();
-    GRAPH_RDLOCK_GUARD_MAINLOOP();
 
     if (!drv)
         return -ENOMEDIUM;
 
+    bdrv_graph_rdlock_main_loop();
+
     backing_file_bs = bdrv_cow_bs(bs);
 
     if (!backing_file_bs) {
-        return -ENOTSUP;
+        ret = -ENOTSUP;
+        goto out;
     }
 
     if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT_SOURCE, NULL) ||
         bdrv_op_is_blocked(backing_file_bs, BLOCK_OP_TYPE_COMMIT_TARGET, NULL))
     {
-        return -EBUSY;
+        ret = -EBUSY;
+        goto out;
     }
 
     ro = bdrv_is_read_only(backing_file_bs);
 
     if (ro) {
         if (bdrv_reopen_set_read_only(backing_file_bs, false, NULL)) {
-            return -EACCES;
+            ret = -EACCES;
+            goto out;
         }
     }
 
@@ -563,8 +563,14 @@ int bdrv_commit(BlockDriverState *bs)
         goto ro_cleanup;
     }
 
+    bdrv_graph_rdunlock_main_loop();
+
+    bdrv_graph_wrlock_drained();
     bdrv_set_backing_hd(commit_top_bs, backing_file_bs, &error_abort);
     bdrv_set_backing_hd(bs, commit_top_bs, &error_abort);
+    bdrv_graph_wrunlock();
+
+    bdrv_graph_rdlock_main_loop();
 
     ret = blk_insert_bs(backing, backing_file_bs, &local_err);
     if (ret < 0) {
@@ -639,9 +645,14 @@ int bdrv_commit(BlockDriverState *bs)
     ret = 0;
 ro_cleanup:
     blk_unref(backing);
+
+    bdrv_graph_rdunlock_main_loop();
+    bdrv_graph_wrlock_drained();
     if (bdrv_cow_bs(bs) != backing_file_bs) {
         bdrv_set_backing_hd(bs, backing_file_bs, &error_abort);
     }
+    bdrv_graph_wrunlock();
+    bdrv_graph_rdlock_main_loop();
     bdrv_unref(commit_top_bs);
     blk_unref(src);
 
@@ -650,5 +661,8 @@ ro_cleanup:
         bdrv_reopen_set_read_only(backing_file_bs, true, NULL);
     }
 
+out:
+    bdrv_graph_rdunlock_main_loop();
+
     return ret;
 }

block/file-posix.c

@@ -2564,9 +2564,9 @@ static inline bool raw_check_linux_aio(BDRVRawState *s)
 }
 #endif
 
-static int coroutine_fn raw_co_prw(BlockDriverState *bs, int64_t *offset_ptr,
+static int coroutine_fn GRAPH_RDLOCK
-                                   uint64_t bytes, QEMUIOVector *qiov, int type,
+raw_co_prw(BlockDriverState *bs, int64_t *offset_ptr, uint64_t bytes,
-                                   int flags)
+           QEMUIOVector *qiov, int type, int flags)
 {
     BDRVRawState *s = bs->opaque;
     RawPosixAIOData acb;
@@ -2625,7 +2625,7 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, int64_t *offset_ptr,
     ret = raw_thread_pool_submit(handle_aiocb_rw, &acb);
     if (ret == 0 && (flags & BDRV_REQ_FUA)) {
         /* TODO Use pwritev2() instead if it's available */
-        ret = raw_co_flush_to_disk(bs);
+        ret = bdrv_co_flush(bs);
     }
     goto out; /* Avoid the compiler err of unused label */
 
@@ -2660,16 +2660,16 @@ out:
     return ret;
 }
 
-static int coroutine_fn raw_co_preadv(BlockDriverState *bs, int64_t offset,
+static int coroutine_fn GRAPH_RDLOCK
-                                      int64_t bytes, QEMUIOVector *qiov,
+raw_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
-                                      BdrvRequestFlags flags)
+              QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     return raw_co_prw(bs, &offset, bytes, qiov, QEMU_AIO_READ, flags);
 }
 
-static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, int64_t offset,
+static int coroutine_fn GRAPH_RDLOCK
-                                       int64_t bytes, QEMUIOVector *qiov,
+raw_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
-                                       BdrvRequestFlags flags)
+               QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     return raw_co_prw(bs, &offset, bytes, qiov, QEMU_AIO_WRITE, flags);
 }
@@ -3606,7 +3606,8 @@ static int coroutine_fn raw_co_zone_mgmt(BlockDriverState *bs, BlockZoneOp op,
 #endif
 
 #if defined(CONFIG_BLKZONED)
-static int coroutine_fn raw_co_zone_append(BlockDriverState *bs,
+static int coroutine_fn GRAPH_RDLOCK
+raw_co_zone_append(BlockDriverState *bs,
                    int64_t *offset,
                    QEMUIOVector *qiov,
                    BdrvRequestFlags flags) {

block/graph-lock.c

@@ -33,6 +33,17 @@ static QemuMutex aio_context_list_lock;
 /* Written and read with atomic operations. */
 static int has_writer;
 
+/*
+ * Many write-locked sections are also drained sections. There is a convenience
+ * wrapper bdrv_graph_wrlock_drained() which begins a drained section before
+ * acquiring the lock. This variable here is used so bdrv_graph_wrunlock() knows
+ * if it also needs to end such a drained section. It needs to be a counter,
+ * because the aio_poll() call in bdrv_graph_wrlock() might re-enter
+ * bdrv_graph_wrlock_drained(). And note that aio_bh_poll() in
+ * bdrv_graph_wrunlock() might also re-enter a write-locked section.
+ */
+static int wrlock_quiesced_counter;
+
 /*
  * A reader coroutine could move from an AioContext to another.
  * If this happens, there is no problem from the point of view of
@@ -112,8 +123,14 @@ void no_coroutine_fn bdrv_graph_wrlock(void)
     assert(!qatomic_read(&has_writer));
     assert(!qemu_in_coroutine());
 
-    /* Make sure that constantly arriving new I/O doesn't cause starvation */
+    bool need_drain = wrlock_quiesced_counter == 0;
+
+    if (need_drain) {
+        /*
+         * Make sure that constantly arriving new I/O doesn't cause starvation
+         */
         bdrv_drain_all_begin_nopoll();
+    }
 
     /*
      * reader_count == 0: this means writer will read has_reader as 1
@@ -139,7 +156,18 @@ void no_coroutine_fn bdrv_graph_wrlock(void)
         smp_mb();
     } while (reader_count() >= 1);
 
+    if (need_drain) {
         bdrv_drain_all_end();
+    }
+}
+
+void no_coroutine_fn bdrv_graph_wrlock_drained(void)
+{
+    GLOBAL_STATE_CODE();
+
+    bdrv_drain_all_begin();
+    wrlock_quiesced_counter++;
+    bdrv_graph_wrlock();
 }
 
 void no_coroutine_fn bdrv_graph_wrunlock(void)
@@ -168,6 +196,12 @@ void no_coroutine_fn bdrv_graph_wrunlock(void)
      * progress.
      */
     aio_bh_poll(qemu_get_aio_context());
+
+    if (wrlock_quiesced_counter > 0) {
+        bdrv_drain_all_end();
+        wrlock_quiesced_counter--;
+    }
+
 }
 
 void coroutine_fn bdrv_graph_co_rdlock(void)

block/io.c

@@ -361,7 +361,7 @@ static void bdrv_do_drained_begin(BlockDriverState *bs, BdrvChild *parent,
     GLOBAL_STATE_CODE();
 
     /* Stop things in parent-to-child order */
-    if (qatomic_fetch_inc(&bs->quiesce_counter) == 0) {
+    if (bs->quiesce_counter++ == 0) {
         GRAPH_RDLOCK_GUARD_MAINLOOP();
         bdrv_parent_drained_begin(bs, parent);
         if (bs->drv && bs->drv->bdrv_drain_begin) {
@@ -401,8 +401,6 @@ bdrv_drained_begin(BlockDriverState *bs)
  */
 static void bdrv_do_drained_end(BlockDriverState *bs, BdrvChild *parent)
 {
-    int old_quiesce_counter;
-
     IO_OR_GS_CODE();
 
     if (qemu_in_coroutine()) {
@@ -415,8 +413,7 @@ static void bdrv_do_drained_end(BlockDriverState *bs, BdrvChild *parent)
     assert(bs->quiesce_counter > 0);
 
     /* Re-enable things in child-to-parent order */
-    old_quiesce_counter = qatomic_fetch_dec(&bs->quiesce_counter);
+    if (--bs->quiesce_counter == 0) {
-    if (old_quiesce_counter == 1) {
         GRAPH_RDLOCK_GUARD_MAINLOOP();
         if (bs->drv && bs->drv->bdrv_drain_end) {
             bs->drv->bdrv_drain_end(bs);

block/mirror.c

@@ -761,10 +761,14 @@ static int mirror_exit_common(Job *job)
     bdrv_graph_rdlock_main_loop();
     bdrv_child_refresh_perms(mirror_top_bs, mirror_top_bs->backing,
                              &error_abort);
+    bdrv_graph_rdunlock_main_loop();
 
     if (!abort && s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) {
         BlockDriverState *backing;
-        BlockDriverState *unfiltered_target = bdrv_skip_filters(target_bs);
+        BlockDriverState *unfiltered_target;
+
+        bdrv_graph_wrlock_drained();
+        unfiltered_target = bdrv_skip_filters(target_bs);
 
         backing = s->sync_mode == MIRROR_SYNC_MODE_NONE ? src : s->base;
         if (bdrv_cow_bs(unfiltered_target) != backing) {
@@ -775,16 +779,18 @@ static int mirror_exit_common(Job *job)
                 ret = -EPERM;
             }
         }
+        bdrv_graph_wrunlock();
     } else if (!abort && s->backing_mode == MIRROR_OPEN_BACKING_CHAIN) {
+        bdrv_graph_rdlock_main_loop();
         assert(!bdrv_backing_chain_next(target_bs));
         ret = bdrv_open_backing_file(bdrv_skip_filters(target_bs), NULL,
                                      "backing", &local_err);
+        bdrv_graph_rdunlock_main_loop();
         if (ret < 0) {
             error_report_err(local_err);
             local_err = NULL;
         }
     }
-    bdrv_graph_rdunlock_main_loop();
 
     if (s->should_complete && !abort) {
         BlockDriverState *to_replace = s->to_replace ?: src;
@@ -2014,15 +2020,13 @@ static BlockJob *mirror_start_job(
      */
     bdrv_disable_dirty_bitmap(s->dirty_bitmap);
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     ret = block_job_add_bdrv(&s->common, "source", bs, 0,
                              BLK_PERM_WRITE_UNCHANGED | BLK_PERM_WRITE |
                              BLK_PERM_CONSISTENT_READ,
                              errp);
     if (ret < 0) {
         bdrv_graph_wrunlock();
-        bdrv_drain_all_end();
         goto fail;
     }
 
@@ -2068,19 +2072,16 @@ static BlockJob *mirror_start_job(
                                      iter_shared_perms, errp);
             if (ret < 0) {
                 bdrv_graph_wrunlock();
-                bdrv_drain_all_end();
                 goto fail;
             }
         }
 
         if (bdrv_freeze_backing_chain(mirror_top_bs, target, errp) < 0) {
             bdrv_graph_wrunlock();
-            bdrv_drain_all_end();
             goto fail;
         }
     }
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     QTAILQ_INIT(&s->ops_in_flight);
 

block/monitor/block-hmp-cmds.c

@@ -144,7 +144,7 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
     Error *local_err = NULL;
 
     GLOBAL_STATE_CODE();
-    GRAPH_RDLOCK_GUARD_MAINLOOP();
+    bdrv_graph_rdlock_main_loop();
 
     bs = bdrv_find_node(id);
     if (bs) {
@@ -152,29 +152,31 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
         if (local_err) {
             error_report_err(local_err);
         }
-        return;
+        goto unlock;
     }
 
     blk = blk_by_name(id);
     if (!blk) {
         error_report("Device '%s' not found", id);
-        return;
+        goto unlock;
     }
 
     if (!blk_legacy_dinfo(blk)) {
         error_report("Deleting device added with blockdev-add"
                      " is not supported");
-        return;
+        goto unlock;
     }
 
     bs = blk_bs(blk);
     if (bs) {
         if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, &local_err)) {
             error_report_err(local_err);
-            return;
+            goto unlock;
         }
 
+        bdrv_graph_rdunlock_main_loop();
         blk_remove_bs(blk);
+        bdrv_graph_rdlock_main_loop();
     }
 
     /* Make the BlockBackend and the attached BlockDriverState anonymous */
@@ -191,6 +193,9 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
     } else {
         blk_unref(blk);
     }
+
+unlock:
+    bdrv_graph_rdunlock_main_loop();
 }
 
 void hmp_commit(Monitor *mon, const QDict *qdict)

block/qapi.c

@@ -51,6 +51,8 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
     ImageInfo *backing_info;
     BlockDriverState *backing;
     BlockDeviceInfo *info;
+    BlockdevChildList **children_list_tail;
+    BdrvChild *child;
 
     if (!bs->drv) {
         error_setg(errp, "Block device %s is ejected", bs->node_name);
@@ -73,8 +75,14 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
         .no_flush       = !!(bs->open_flags & BDRV_O_NO_FLUSH),
     };
 
-    if (bs->node_name[0]) {
     info->node_name = g_strdup(bs->node_name);
+
+    children_list_tail = &info->children;
+    QLIST_FOREACH(child, &bs->children, next) {
+        BlockdevChild *child_ref = g_new0(BlockdevChild, 1);
+        child_ref->child = g_strdup(child->name);
+        child_ref->node_name = g_strdup(child->bs->node_name);
+        QAPI_LIST_APPEND(children_list_tail, child_ref);
     }
 
     backing = bdrv_cow_bs(bs);

block/qcow2.c

@@ -2823,11 +2823,9 @@ qcow2_do_close(BlockDriverState *bs, bool close_data_file)
     if (close_data_file && has_data_file(bs)) {
         GLOBAL_STATE_CODE();
         bdrv_graph_rdunlock_main_loop();
-        bdrv_drain_all_begin();
+        bdrv_graph_wrlock_drained();
-        bdrv_graph_wrlock();
         bdrv_unref_child(bs, s->data_file);
         bdrv_graph_wrunlock();
-        bdrv_drain_all_end();
         s->data_file = NULL;
         bdrv_graph_rdlock_main_loop();
     }

block/quorum.c

@@ -1037,8 +1037,7 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
 
 close_exit:
     /* cleanup on error */
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     for (i = 0; i < s->num_children; i++) {
         if (!opened[i]) {
             continue;
@@ -1046,7 +1045,6 @@ close_exit:
         bdrv_unref_child(bs, s->children[i]);
     }
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
     g_free(s->children);
     g_free(opened);
 exit:
@@ -1059,13 +1057,11 @@ static void quorum_close(BlockDriverState *bs)
     BDRVQuorumState *s = bs->opaque;
     int i;
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     for (i = 0; i < s->num_children; i++) {
         bdrv_unref_child(bs, s->children[i]);
     }
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     g_free(s->children);
 }

block/replication.c

@@ -364,14 +364,15 @@ static void reopen_backing_file(BlockDriverState *bs, bool writable,
     BlockReopenQueue *reopen_queue = NULL;
 
     GLOBAL_STATE_CODE();
-    GRAPH_RDLOCK_GUARD_MAINLOOP();
 
+    bdrv_graph_rdlock_main_loop();
     /*
      * s->hidden_disk and s->secondary_disk may not be set yet, as they will
      * only be set after the children are writable.
      */
     hidden_disk = bs->file->bs->backing;
     secondary_disk = hidden_disk->bs->backing;
+    bdrv_graph_rdunlock_main_loop();
 
     if (writable) {
         s->orig_hidden_read_only = bdrv_is_read_only(hidden_disk->bs);
@@ -540,8 +541,7 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
             return;
         }
 
-        bdrv_drain_all_begin();
+        bdrv_graph_wrlock_drained();
-        bdrv_graph_wrlock();
 
         bdrv_ref(hidden_disk->bs);
         s->hidden_disk = bdrv_attach_child(bs, hidden_disk->bs, "hidden disk",
@@ -550,7 +550,6 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
         if (local_err) {
             error_propagate(errp, local_err);
             bdrv_graph_wrunlock();
-            bdrv_drain_all_end();
             return;
         }
 
@@ -561,7 +560,6 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
         if (local_err) {
             error_propagate(errp, local_err);
             bdrv_graph_wrunlock();
-            bdrv_drain_all_end();
             return;
         }
 
@@ -574,14 +572,12 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
             !check_top_bs(top_bs, bs)) {
             error_setg(errp, "No top_bs or it is invalid");
             bdrv_graph_wrunlock();
-            bdrv_drain_all_end();
             reopen_backing_file(bs, false, NULL);
             return;
         }
         bdrv_op_block_all(top_bs, s->blocker);
 
         bdrv_graph_wrunlock();
-        bdrv_drain_all_end();
 
         s->backup_job = backup_job_create(
                                 NULL, s->secondary_disk->bs, s->hidden_disk->bs,
@@ -656,14 +652,12 @@ static void replication_done(void *opaque, int ret)
     if (ret == 0) {
         s->stage = BLOCK_REPLICATION_DONE;
 
-        bdrv_drain_all_begin();
+        bdrv_graph_wrlock_drained();
-        bdrv_graph_wrlock();
         bdrv_unref_child(bs, s->secondary_disk);
         s->secondary_disk = NULL;
         bdrv_unref_child(bs, s->hidden_disk);
         s->hidden_disk = NULL;
         bdrv_graph_wrunlock();
-        bdrv_drain_all_end();
 
         s->error = 0;
     } else {

block/snapshot.c

@@ -291,11 +291,9 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
         }
 
         /* .bdrv_open() will re-attach it */
-        bdrv_drain_all_begin();
+        bdrv_graph_wrlock_drained();
-        bdrv_graph_wrlock();
         bdrv_unref_child(bs, fallback);
         bdrv_graph_wrunlock();
-        bdrv_drain_all_end();
 
         ret = bdrv_snapshot_goto(fallback_bs, snapshot_id, errp);
         memset(bs->opaque, 0, drv->instance_size);

block/stream.c

@@ -51,7 +51,7 @@ static int coroutine_fn stream_populate(BlockBackend *blk,
     return blk_co_preadv(blk, offset, bytes, NULL, BDRV_REQ_PREFETCH);
 }
 
-static int stream_prepare(Job *job)
+static int GRAPH_UNLOCKED stream_prepare(Job *job)
 {
     StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
     BlockDriverState *unfiltered_bs;
@@ -73,12 +73,11 @@ static int stream_prepare(Job *job)
     s->cor_filter_bs = NULL;
 
     /*
-     * bdrv_set_backing_hd() requires that the unfiltered_bs and the COW child
+     * bdrv_set_backing_hd() requires that all block nodes are drained. Drain
-     * of unfiltered_bs is drained. Drain already here and use
+     * already here, because the polling during drained_begin() might change the
-     * bdrv_set_backing_hd_drained() instead because the polling during
+     * graph, and if we do this only later, we may end up working with the wrong
-     * drained_begin() might change the graph, and if we do this only later, we
+     * base node (or it might even have gone away by the time we want to use
-     * may end up working with the wrong base node (or it might even have gone
+     * it).
-     * away by the time we want to use it).
      */
     if (unfiltered_bs_cow) {
         bdrv_ref(unfiltered_bs_cow);
@@ -105,7 +104,7 @@ static int stream_prepare(Job *job)
         }
 
         bdrv_graph_wrlock();
-        bdrv_set_backing_hd_drained(unfiltered_bs, base, &local_err);
+        bdrv_set_backing_hd(unfiltered_bs, base, &local_err);
         bdrv_graph_wrunlock();
 
         /*
@@ -371,12 +370,10 @@ void stream_start(const char *job_id, BlockDriverState *bs,
      * already have our own plans. Also don't allow resize as the image size is
      * queried only at the job start and then cached.
      */
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     if (block_job_add_bdrv(&s->common, "active node", bs, 0,
                            basic_flags | BLK_PERM_WRITE, errp)) {
         bdrv_graph_wrunlock();
-        bdrv_drain_all_end();
         goto fail;
     }
 
@@ -397,12 +394,10 @@ void stream_start(const char *job_id, BlockDriverState *bs,
                                  basic_flags, errp);
         if (ret < 0) {
             bdrv_graph_wrunlock();
-            bdrv_drain_all_end();
             goto fail;
         }
     }
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     s->base_overlay = base_overlay;
     s->above_base = above_base;

block/vmdk.c

@@ -271,8 +271,7 @@ static void vmdk_free_extents(BlockDriverState *bs)
     BDRVVmdkState *s = bs->opaque;
     VmdkExtent *e;
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     for (i = 0; i < s->num_extents; i++) {
         e = &s->extents[i];
         g_free(e->l1_table);
@@ -284,7 +283,6 @@ static void vmdk_free_extents(BlockDriverState *bs)
         }
     }
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     g_free(s->extents);
 }
@@ -1231,9 +1229,11 @@ vmdk_parse_extents(const char *desc, BlockDriverState *bs, QDict *options,
             extent_role |= BDRV_CHILD_METADATA;
         }
 
+        bdrv_graph_rdunlock_main_loop();
         extent_file = bdrv_open_child(extent_path, options, extent_opt_prefix,
                                       bs, &child_of_bds, extent_role, false,
                                       &local_err);
+        bdrv_graph_rdlock_main_loop();
         g_free(extent_path);
         if (!extent_file) {
             error_propagate(errp, local_err);
@@ -1249,11 +1249,9 @@ vmdk_parse_extents(const char *desc, BlockDriverState *bs, QDict *options,
                             0, 0, 0, 0, 0, &extent, errp);
             if (ret < 0) {
                 bdrv_graph_rdunlock_main_loop();
-                bdrv_drain_all_begin();
+                bdrv_graph_wrlock_drained();
-                bdrv_graph_wrlock();
                 bdrv_unref_child(bs, extent_file);
                 bdrv_graph_wrunlock();
-                bdrv_drain_all_end();
                 bdrv_graph_rdlock_main_loop();
                 goto out;
             }
@@ -1270,11 +1268,9 @@ vmdk_parse_extents(const char *desc, BlockDriverState *bs, QDict *options,
             g_free(buf);
             if (ret) {
                 bdrv_graph_rdunlock_main_loop();
-                bdrv_drain_all_begin();
+                bdrv_graph_wrlock_drained();
-                bdrv_graph_wrlock();
                 bdrv_unref_child(bs, extent_file);
                 bdrv_graph_wrunlock();
-                bdrv_drain_all_end();
                 bdrv_graph_rdlock_main_loop();
                 goto out;
             }
@@ -1283,11 +1279,9 @@ vmdk_parse_extents(const char *desc, BlockDriverState *bs, QDict *options,
             ret = vmdk_open_se_sparse(bs, extent_file, bs->open_flags, errp);
             if (ret) {
                 bdrv_graph_rdunlock_main_loop();
-                bdrv_drain_all_begin();
+                bdrv_graph_wrlock_drained();
-                bdrv_graph_wrlock();
                 bdrv_unref_child(bs, extent_file);
                 bdrv_graph_wrunlock();
-                bdrv_drain_all_end();
                 bdrv_graph_rdlock_main_loop();
                 goto out;
             }
@@ -1295,11 +1289,9 @@ vmdk_parse_extents(const char *desc, BlockDriverState *bs, QDict *options,
         } else {
             error_setg(errp, "Unsupported extent type '%s'", type);
             bdrv_graph_rdunlock_main_loop();
-            bdrv_drain_all_begin();
+            bdrv_graph_wrlock_drained();
-            bdrv_graph_wrlock();
             bdrv_unref_child(bs, extent_file);
             bdrv_graph_wrunlock();
-            bdrv_drain_all_end();
             bdrv_graph_rdlock_main_loop();
             ret = -ENOTSUP;
             goto out;
@@ -1362,13 +1354,13 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
     BDRVVmdkState *s = bs->opaque;
     uint32_t magic;
 
-    GRAPH_RDLOCK_GUARD_MAINLOOP();
-
     ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
     if (ret < 0) {
         return ret;
     }
 
+    GRAPH_RDLOCK_GUARD_MAINLOOP();
+
     buf = vmdk_read_desc(bs->file, 0, errp);
     if (!buf) {
         return -EINVAL;

blockdev.c

@@ -1421,7 +1421,7 @@ static void external_snapshot_action(TransactionAction *action,
     bdrv_graph_rdunlock_main_loop();
     /* Paired with .clean() */
     bdrv_drained_begin(state->old_bs);
-    GRAPH_RDLOCK_GUARD_MAINLOOP();
+    bdrv_graph_rdlock_main_loop();
 
     /* Make sure the associated bs did not change with the drain. */
     check_bs = bdrv_lookup_bs(device, node_name, errp);
@@ -1430,18 +1430,18 @@ static void external_snapshot_action(TransactionAction *action,
             error_setg(errp, "Block node of device '%s' unexpectedly changed",
                        device);
         } /* else errp is already set */
-        return;
+        goto unlock;
     }
 
     if (!bdrv_is_inserted(state->old_bs)) {
         error_setg(errp, "Device '%s' has no medium",
                    bdrv_get_device_or_node_name(state->old_bs));
-        return;
+        goto unlock;
     }
 
     if (bdrv_op_is_blocked(state->old_bs,
                            BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, errp)) {
-        return;
+        goto unlock;
     }
 
     if (!bdrv_is_read_only(state->old_bs)) {
@@ -1449,7 +1449,7 @@ static void external_snapshot_action(TransactionAction *action,
         if (ret < 0) {
             error_setg_errno(errp, -ret, "Write to node '%s' failed",
                              bdrv_get_device_or_node_name(state->old_bs));
-            return;
+            goto unlock;
         }
     }
 
@@ -1461,13 +1461,13 @@ static void external_snapshot_action(TransactionAction *action,
 
         if (node_name && !snapshot_node_name) {
             error_setg(errp, "New overlay node-name missing");
-            return;
+            goto unlock;
         }
 
         if (snapshot_node_name &&
             bdrv_lookup_bs(snapshot_node_name, snapshot_node_name, NULL)) {
             error_setg(errp, "New overlay node-name already in use");
-            return;
+            goto unlock;
         }
 
         flags = state->old_bs->open_flags;
@@ -1480,7 +1480,7 @@ static void external_snapshot_action(TransactionAction *action,
             int64_t size = bdrv_getlength(state->old_bs);
             if (size < 0) {
                 error_setg_errno(errp, -size, "bdrv_getlength failed");
-                return;
+                goto unlock;
             }
             bdrv_refresh_filename(state->old_bs);
 
@@ -1491,7 +1491,7 @@ static void external_snapshot_action(TransactionAction *action,
 
             if (local_err) {
                 error_propagate(errp, local_err);
-                return;
+                goto unlock;
             }
         }
 
@@ -1507,7 +1507,7 @@ static void external_snapshot_action(TransactionAction *action,
 
     /* We will manually add the backing_hd field to the bs later */
     if (!state->new_bs) {
-        return;
+        goto unlock;
     }
 
     /*
@@ -1518,22 +1518,22 @@ static void external_snapshot_action(TransactionAction *action,
     bdrv_get_cumulative_perm(state->new_bs, &perm, &shared);
     if (perm & BLK_PERM_CONSISTENT_READ) {
         error_setg(errp, "The overlay is already in use");
-        return;
+        goto unlock;
     }
 
     if (state->new_bs->drv->is_filter) {
         error_setg(errp, "Filters cannot be used as overlays");
-        return;
+        goto unlock;
     }
 
     if (bdrv_cow_child(state->new_bs)) {
         error_setg(errp, "The overlay already has a backing image");
-        return;
+        goto unlock;
     }
 
     if (!state->new_bs->drv->supports_backing) {
         error_setg(errp, "The overlay does not support backing images");
-        return;
+        goto unlock;
     }
 
     /*
@@ -1546,17 +1546,23 @@ static void external_snapshot_action(TransactionAction *action,
      * to keep this working.
      */
     if (bdrv_is_inactive(state->old_bs) && !bdrv_is_inactive(state->new_bs)) {
+        bdrv_graph_rdunlock_main_loop();
+        bdrv_drain_all_begin();
+        bdrv_graph_rdlock_main_loop();
         ret = bdrv_inactivate(state->new_bs, errp);
+        bdrv_drain_all_end();
         if (ret < 0) {
-            return;
+            goto unlock;
         }
     }
 
     ret = bdrv_append(state->new_bs, state->old_bs, errp);
     if (ret < 0) {
-        return;
+        goto unlock;
     }
     state->overlay_appended = true;
+unlock:
+    bdrv_graph_rdunlock_main_loop();
 }
 
 static void external_snapshot_commit(void *opaque)
@@ -1580,10 +1586,18 @@ static void external_snapshot_abort(void *opaque)
             AioContext *tmp_context;
             int ret;
 
+            bdrv_graph_wrlock_drained();
+
             aio_context = bdrv_get_aio_context(state->old_bs);
 
-            bdrv_ref(state->old_bs);   /* we can't let bdrv_set_backind_hd()
+            /*
-                                          close state->old_bs; we need it */
+             * Note that state->old_bs would not disappear during the
+             * write-locked section, because the unref from
+             * bdrv_set_backing_hd() only happens at the end of the write-locked
+             * section. However, just be explicit about keeping a reference and
+             * don't rely on that implicit detail.
+             */
+            bdrv_ref(state->old_bs);
             bdrv_set_backing_hd(state->new_bs, NULL, &error_abort);
 
             /*
@@ -1593,16 +1607,14 @@ static void external_snapshot_abort(void *opaque)
              */
             tmp_context = bdrv_get_aio_context(state->old_bs);
             if (aio_context != tmp_context) {
-                ret = bdrv_try_change_aio_context(state->old_bs,
+                ret = bdrv_try_change_aio_context_locked(state->old_bs,
-                                                  aio_context, NULL, NULL);
+                                                         aio_context, NULL,
+                                                         NULL);
                 assert(ret == 0);
             }
 
-            bdrv_drained_begin(state->new_bs);
-            bdrv_graph_wrlock();
             bdrv_replace_node(state->new_bs, state->old_bs, &error_abort);
             bdrv_graph_wrunlock();
-            bdrv_drained_end(state->new_bs);
 
             bdrv_unref(state->old_bs); /* bdrv_replace_node() ref'ed old_bs */
         }
@@ -1770,7 +1782,10 @@ static void drive_backup_action(DriveBackup *backup,
     }
 
     if (set_backing_hd) {
-        if (bdrv_set_backing_hd(target_bs, source, errp) < 0) {
+        bdrv_graph_wrlock_drained();
+        ret = bdrv_set_backing_hd(target_bs, source, errp);
+        bdrv_graph_wrunlock();
+        if (ret < 0) {
             goto unref;
         }
     }
@@ -3511,10 +3526,10 @@ void qmp_blockdev_del(const char *node_name, Error **errp)
 
 void qmp_blockdev_set_active(const char *node_name, bool active, Error **errp)
 {
+    BlockDriverState *bs;
     int ret;
 
     GLOBAL_STATE_CODE();
-    GRAPH_RDLOCK_GUARD_MAINLOOP();
 
     if (!node_name) {
         if (active) {
@@ -3525,19 +3540,30 @@ void qmp_blockdev_set_active(const char *node_name, bool active, Error **errp)
                 error_setg_errno(errp, -ret, "Failed to inactivate all nodes");
             }
         }
-    } else {
-        BlockDriverState *bs = bdrv_find_node(node_name);
-        if (!bs) {
-            error_setg(errp, "Failed to find node with node-name='%s'",
-                       node_name);
         return;
     }
 
+    if (!active) {
+        bdrv_drain_all_begin();
+    }
+    bdrv_graph_rdlock_main_loop();
+
+    bs = bdrv_find_node(node_name);
+    if (!bs) {
+        error_setg(errp, "Failed to find node with node-name='%s'",
+                   node_name);
+        goto unlock;
+    }
     if (active) {
         bdrv_activate(bs, errp);
     } else {
         bdrv_inactivate(bs, errp);
     }
+
+unlock:
+    bdrv_graph_rdunlock_main_loop();
+    if (!active) {
+        bdrv_drain_all_end();
     }
 }
 
@@ -3561,8 +3587,7 @@ void qmp_x_blockdev_change(const char *parent, const char *child,
     BlockDriverState *parent_bs, *new_bs = NULL;
     BdrvChild *p_child;
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
 
     parent_bs = bdrv_lookup_bs(parent, parent, errp);
     if (!parent_bs) {
@@ -3599,7 +3624,6 @@ void qmp_x_blockdev_change(const char *parent, const char *child,
 
 out:
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 }
 
 BlockJobInfoList *qmp_query_block_jobs(Error **errp)

blockjob.c

@@ -198,8 +198,7 @@ void block_job_remove_all_bdrv(BlockJob *job)
      * one to make sure that such a concurrent access does not attempt
      * to process an already freed BdrvChild.
      */
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     while (job->nodes) {
         GSList *l = job->nodes;
         BdrvChild *c = l->data;
@@ -212,7 +211,6 @@ void block_job_remove_all_bdrv(BlockJob *job)
         g_slist_free_1(l);
     }
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 }
 
 bool block_job_has_bdrv(BlockJob *job, BlockDriverState *bs)
@@ -498,8 +496,7 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
     int ret;
     GLOBAL_STATE_CODE();
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
 
     if (job_id == NULL && !(flags & JOB_INTERNAL)) {
         job_id = bdrv_get_device_name(bs);
@@ -509,7 +506,6 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
                      flags, cb, opaque, errp);
     if (job == NULL) {
         bdrv_graph_wrunlock();
-        bdrv_drain_all_end();
         return NULL;
     }
 
@@ -548,12 +544,10 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
     }
 
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
     return job;
 
 fail:
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
     job_early_fail(&job->job);
     return NULL;
 }

docs/tools/qemu-img.rst

@@ -256,7 +256,7 @@ Parameters to snapshot subcommand:
 
 .. option:: -l
 
-  Lists all snapshots in the given image
+  Lists all snapshots in the given image (default action)
 
 Command description:
 
@@ -419,7 +419,7 @@ Command description:
   4
     Error on reading data
 
-.. option:: convert [--object OBJECTDEF] [--image-opts] [--target-image-opts] [--target-is-zero] [--bitmaps [--skip-broken-bitmaps]] [-U] [-C] [-c] [-p] [-q] [-n] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-O OUTPUT_FMT] [-B BACKING_FILE [-F BACKING_FMT]] [-o OPTIONS] [-l SNAPSHOT_PARAM] [-S SPARSE_SIZE] [-r RATE_LIMIT] [-m NUM_COROUTINES] [-W] FILENAME [FILENAME2 [...]] OUTPUT_FILENAME
+.. option:: convert [--object OBJECTDEF] [--image-opts] [--target-image-opts] [--target-is-zero] [--bitmaps [--skip-broken-bitmaps]] [-U] [-C] [-c] [-p] [-q] [-n] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-O OUTPUT_FMT] [-b BACKING_FILE [-F BACKING_FMT]] [-o OPTIONS] [-l SNAPSHOT_PARAM] [-S SPARSE_SIZE] [-r RATE_LIMIT] [-m NUM_COROUTINES] [-W] FILENAME [FILENAME2 [...]] OUTPUT_FILENAME
 
   Convert the disk image *FILENAME* or a snapshot *SNAPSHOT_PARAM*
   to disk image *OUTPUT_FILENAME* using format *OUTPUT_FMT*. It can
@@ -467,11 +467,11 @@ Command description:
   ``--skip-broken-bitmaps`` is also specified to copy only the
   consistent bitmaps.
 
-.. option:: create [--object OBJECTDEF] [-q] [-f FMT] [-b BACKING_FILE [-F BACKING_FMT]] [-u] [-o OPTIONS] FILENAME [SIZE]
+.. option:: create [-f FMT] [-o FMT_OPTS] [-b BACKING_FILE [-B BACKING_FMT]] [-u] [-q] [--object OBJDEF] FILE [SIZE]
 
-  Create the new disk image *FILENAME* of size *SIZE* and format
+  Create the new disk image *FILE* of size *SIZE* and format
-  *FMT*. Depending on the file format, you can add one or more *OPTIONS*
+  *FMT*. Depending on the file format, you can add one or more *FMT_OPTS*
-  that enable additional features of this format.
+  options that enable additional features of this format.
 
   If the option *BACKING_FILE* is specified, then the image will record
   only the differences from *BACKING_FILE*. No size needs to be specified in
@@ -479,7 +479,7 @@ Command description:
   ``commit`` monitor command (or ``qemu-img commit``).
 
   If a relative path name is given, the backing file is looked up relative to
-  the directory containing *FILENAME*.
+  the directory containing *FILE*.
 
   Note that a given backing file will be opened to check that it is valid. Use
   the ``-u`` option to enable unsafe backing file mode, which means that the
@@ -663,11 +663,11 @@ Command description:
   bitmap support, or 0 if bitmaps are supported but there is nothing
   to copy.
 
-.. option:: snapshot [--object OBJECTDEF] [--image-opts] [-U] [-q] [-l | -a SNAPSHOT | -c SNAPSHOT | -d SNAPSHOT] FILENAME
+.. option:: snapshot [--object OBJECTDEF] [-f FMT | --image-opts] [-U] [-q] [-l | -a SNAPSHOT | -c SNAPSHOT | -d SNAPSHOT] FILENAME
 
   List, apply, create or delete snapshots in image *FILENAME*.
 
-.. option:: rebase [--object OBJECTDEF] [--image-opts] [-U] [-q] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-p] [-u] [-c] -b BACKING_FILE [-F BACKING_FMT] FILENAME
+.. option:: rebase [--object OBJECTDEF] [--image-opts] [-U] [-q] [-f FMT] [-t CACHE] [-T SRC_CACHE] [-p] [-u] [-c] -b BACKING_FILE [-B BACKING_FMT] FILENAME
 
   Changes the backing file of an image. Only the formats ``qcow2`` and
   ``qed`` support changing the backing file.

include/block/block-global-state.h

@@ -74,13 +74,14 @@ int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
 int GRAPH_WRLOCK
 bdrv_replace_node(BlockDriverState *from, BlockDriverState *to, Error **errp);
 
-int bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs,
+int GRAPH_UNLOCKED
+bdrv_replace_child_bs(BdrvChild *child, BlockDriverState *new_bs, Error **errp);
+BlockDriverState * GRAPH_UNLOCKED
+bdrv_insert_node(BlockDriverState *bs, QDict *node_options, int flags,
                  Error **errp);
-BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *node_options,
-                                   int flags, Error **errp);
 int bdrv_drop_filter(BlockDriverState *bs, Error **errp);
 
-BdrvChild * no_coroutine_fn
+BdrvChild * no_coroutine_fn GRAPH_UNLOCKED
 bdrv_open_child(const char *filename, QDict *options, const char *bdref_key,
                 BlockDriverState *parent, const BdrvChildClass *child_class,
                 BdrvChildRole child_role, bool allow_none, Error **errp);
@@ -90,9 +91,10 @@ bdrv_co_open_child(const char *filename, QDict *options, const char *bdref_key,
                 BlockDriverState *parent, const BdrvChildClass *child_class,
                 BdrvChildRole child_role, bool allow_none, Error **errp);
 
-int bdrv_open_file_child(const char *filename,
+int GRAPH_UNLOCKED
-                         QDict *options, const char *bdref_key,
+bdrv_open_file_child(const char *filename, QDict *options,
-                         BlockDriverState *parent, Error **errp);
+                     const char *bdref_key, BlockDriverState *parent,
+                     Error **errp);
 
 BlockDriverState * no_coroutine_fn
 bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp);
@@ -100,10 +102,8 @@ bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp);
 BlockDriverState * coroutine_fn no_co_wrapper
 bdrv_co_open_blockdev_ref(BlockdevRef *ref, Error **errp);
 
-int bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
-                        Error **errp);
 int GRAPH_WRLOCK
-bdrv_set_backing_hd_drained(BlockDriverState *bs, BlockDriverState *backing_hd,
+bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
                     Error **errp);
 
 int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
@@ -123,11 +123,12 @@ BlockDriverState *bdrv_new_open_driver_opts(BlockDriver *drv,
                                             Error **errp);
 BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, const char *node_name,
                                        int flags, Error **errp);
-BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
+BlockReopenQueue * GRAPH_UNLOCKED
-                                    BlockDriverState *bs, QDict *options,
+bdrv_reopen_queue(BlockReopenQueue *bs_queue, BlockDriverState *bs,
-                                    bool keep_old_opts);
+                  QDict *options, bool keep_old_opts);
 void bdrv_reopen_queue_free(BlockReopenQueue *bs_queue);
-int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp);
+int GRAPH_UNLOCKED
+bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp);
 int bdrv_reopen(BlockDriverState *bs, QDict *opts, bool keep_old_opts,
                 Error **errp);
 int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only,
@@ -143,7 +144,8 @@ int bdrv_commit(BlockDriverState *bs);
 int GRAPH_RDLOCK bdrv_make_empty(BdrvChild *c, Error **errp);
 
 void bdrv_register(BlockDriver *bdrv);
-int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
+int GRAPH_UNLOCKED
+bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base,
                        const char *backing_file_str,
                        bool backing_mask_protocol);
 
@@ -184,14 +186,14 @@ bdrv_activate(BlockDriverState *bs, Error **errp);
 int coroutine_fn no_co_wrapper_bdrv_rdlock
 bdrv_co_activate(BlockDriverState *bs, Error **errp);
 
-int no_coroutine_fn
+int no_coroutine_fn GRAPH_RDLOCK
 bdrv_inactivate(BlockDriverState *bs, Error **errp);
 
 void bdrv_activate_all(Error **errp);
-int bdrv_inactivate_all(void);
+int GRAPH_UNLOCKED bdrv_inactivate_all(void);
 
 int bdrv_flush_all(void);
-void bdrv_close_all(void);
+void GRAPH_UNLOCKED bdrv_close_all(void);
 void GRAPH_UNLOCKED bdrv_drain_all_begin(void);
 void bdrv_drain_all_begin_nopoll(void);
 void bdrv_drain_all_end(void);

include/block/block_int-common.h

@@ -248,7 +248,7 @@ struct BlockDriver {
     int GRAPH_UNLOCKED_PTR (*bdrv_open)(
         BlockDriverState *bs, QDict *options, int flags, Error **errp);
 
-    void (*bdrv_close)(BlockDriverState *bs);
+    void GRAPH_UNLOCKED_PTR (*bdrv_close)(BlockDriverState *bs);
 
     int coroutine_fn GRAPH_UNLOCKED_PTR (*bdrv_co_create)(
         BlockdevCreateOptions *opts, Error **errp);
@@ -1253,7 +1253,7 @@ struct BlockDriverState {
     /* do we need to tell the quest if we have a volatile write cache? */
     int enable_write_cache;
 
-    /* Accessed with atomic ops.  */
+    /* Accessed only in the main thread. */
     int quiesce_counter;
 
     unsigned int write_gen;               /* Current data generation */

include/block/blockjob.h

@@ -151,7 +151,7 @@ block_job_add_bdrv(BlockJob *job, const char *name, BlockDriverState *bs,
  * Remove all BlockDriverStates from the list of nodes that are involved in the
  * job. This removes the blockers added with block_job_add_bdrv().
  */
-void block_job_remove_all_bdrv(BlockJob *job);
+void GRAPH_UNLOCKED block_job_remove_all_bdrv(BlockJob *job);
 
 /**
  * block_job_has_bdrv:

include/block/graph-lock.h

@@ -112,10 +112,21 @@ void unregister_aiocontext(AioContext *ctx);
 void no_coroutine_fn TSA_ACQUIRE(graph_lock) TSA_NO_TSA
 bdrv_graph_wrlock(void);
 
+/*
+ * bdrv_graph_wrlock_drained:
+ * Similar to bdrv_graph_wrlock, but will begin a drained section before
+ * locking.
+ */
+void no_coroutine_fn TSA_ACQUIRE(graph_lock) TSA_NO_TSA
+bdrv_graph_wrlock_drained(void);
+
 /*
  * bdrv_graph_wrunlock:
  * Write finished, reset global has_writer to 0 and restart
  * all readers that are waiting.
+ *
+ * Also ends the drained section if bdrv_graph_wrlock_drained() was used to lock
+ * the graph.
  */
 void no_coroutine_fn TSA_RELEASE(graph_lock) TSA_NO_TSA
 bdrv_graph_wrunlock(void);

include/block/snapshot.h

@@ -90,8 +90,8 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
 
 bool bdrv_all_can_snapshot(bool has_devices, strList *devices,
                            Error **errp);
-int bdrv_all_delete_snapshot(const char *name,
+int GRAPH_UNLOCKED
-                             bool has_devices, strList *devices,
+bdrv_all_delete_snapshot(const char *name, bool has_devices, strList *devices,
                          Error **errp);
 int bdrv_all_goto_snapshot(const char *name,
                            bool has_devices, strList *devices,

include/qemu/job.h

@@ -263,7 +263,7 @@ struct JobDriver {
      * This callback will not be invoked if the job has already failed.
      * If it fails, abort and then clean will be called.
      */
-    int (*prepare)(Job *job);
+    int GRAPH_UNLOCKED_PTR (*prepare)(Job *job);
 
     /**
      * If the callback is not NULL, it will be invoked when all the jobs
@@ -283,7 +283,7 @@ struct JobDriver {
      * All jobs will complete with a call to either .commit() or .abort() but
      * never both.
      */
-    void (*abort)(Job *job);
+    void GRAPH_UNLOCKED_PTR (*abort)(Job *job);
 
     /**
      * If the callback is not NULL, it will be invoked after a call to either

include/system/block-backend-global-state.h

@@ -55,7 +55,7 @@ void monitor_remove_blk(BlockBackend *blk);
 
 BlockBackendPublic *blk_get_public(BlockBackend *blk);
 
-void blk_remove_bs(BlockBackend *blk);
+void GRAPH_UNLOCKED blk_remove_bs(BlockBackend *blk);
 int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp);
 int blk_replace_bs(BlockBackend *blk, BlockDriverState *new_bs, Error **errp);
 bool GRAPH_RDLOCK bdrv_has_blk(BlockDriverState *bs);
@@ -78,8 +78,8 @@ int blk_make_zero(BlockBackend *blk, BdrvRequestFlags flags);
 void blk_aio_cancel(BlockAIOCB *acb);
 int blk_commit_all(void);
 bool blk_in_drain(BlockBackend *blk);
-void blk_drain(BlockBackend *blk);
+void GRAPH_UNLOCKED blk_drain(BlockBackend *blk);
-void blk_drain_all(void);
+void GRAPH_UNLOCKED blk_drain_all(void);
 void blk_set_on_error(BlockBackend *blk, BlockdevOnError on_read_error,
                       BlockdevOnError on_write_error);
 bool blk_supports_write_perm(BlockBackend *blk);
@@ -109,7 +109,7 @@ int blk_probe_blocksizes(BlockBackend *blk, BlockSizes *bsz);
 int blk_probe_geometry(BlockBackend *blk, HDGeometry *geo);
 
 void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg);
-void blk_io_limits_disable(BlockBackend *blk);
+void GRAPH_UNLOCKED blk_io_limits_disable(BlockBackend *blk);
 void blk_io_limits_enable(BlockBackend *blk, const char *group);
 void blk_io_limits_update_group(BlockBackend *blk, const char *group);
 void blk_set_force_allow_inactivate(BlockBackend *blk);

qapi/block-core.json

@@ -462,6 +462,19 @@
             'direct': 'bool',
             'no-flush': 'bool' } }
 
+##
+# @BlockdevChild:
+#
+# @child: The name of the child, for example 'file' or 'backing'.
+#
+# @node-name: The name of the child's block driver node.
+#
+# Since: 10.1
+##
+{ 'struct': 'BlockdevChild',
+  'data': { 'child': 'str',
+            'node-name': 'str' } }
+
 ##
 # @BlockDeviceInfo:
 #
@@ -487,6 +500,8 @@
 # @backing_file_depth: number of files in the backing file chain
 #     (since: 1.2)
 #
+# @children: Information about child block nodes. (since: 10.1)
+#
 # @active: true if the backend is active; typical cases for inactive backends
 #     are on the migration source instance after migration completes and on the
 #     destination before it completes.  (since: 10.0)
@@ -559,8 +574,9 @@
 # Since: 0.14
 ##
 { 'struct': 'BlockDeviceInfo',
-  'data': { 'file': 'str', '*node-name': 'str', 'ro': 'bool', 'drv': 'str',
+  'data': { 'file': 'str', 'node-name': 'str', 'ro': 'bool', 'drv': 'str',
             '*backing_file': 'str', 'backing_file_depth': 'int',
+            'children': ['BlockdevChild'],
             'active': 'bool', 'encrypted': 'bool',
             'detect_zeroes': 'BlockdevDetectZeroesOptions',
             'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',

qemu-img-cmds.hx

@@ -84,9 +84,9 @@ SRST
 ERST
 
 DEF("snapshot", img_snapshot,
-    "snapshot [--object objectdef] [--image-opts] [-U] [-q] [-l | -a snapshot | -c snapshot | -d snapshot] filename")
+    "snapshot [--object objectdef] [-f fmt | --image-opts] [-U] [-q] [-l | -a snapshot | -c snapshot | -d snapshot] filename")
 SRST
-.. option:: snapshot [--object OBJECTDEF] [--image-opts] [-U] [-q] [-l | -a SNAPSHOT | -c SNAPSHOT | -d SNAPSHOT] FILENAME
+.. option:: snapshot [--object OBJECTDEF] [-f FMT | --image-opts] [-U] [-q] [-l | -a SNAPSHOT | -c SNAPSHOT | -d SNAPSHOT] FILENAME
 ERST
 
 DEF("rebase", img_rebase,

tests/qemu-iotests/049.out

@@ -98,8 +98,7 @@ qemu-img create -f qcow2 -o size=-1024 TEST_DIR/t.qcow2
 qemu-img: TEST_DIR/t.qcow2: Value '-1024' is out of range for parameter 'size'
 
 qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- -1k
-qemu-img: Invalid image size specified. You may use k, M, G, T, P or E suffixes for
+qemu-img: Invalid image size specified: '-1k'
-qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes.
 
 qemu-img create -f qcow2 -o size=-1k TEST_DIR/t.qcow2
 qemu-img: TEST_DIR/t.qcow2: Parameter 'size' expects a non-negative number below 2^64
@@ -107,8 +106,7 @@ Optional suffix k, M, G, T, P or E means kilo-, mega-, giga-, tera-, peta-
 and exabytes, respectively.
 
 qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- 1kilobyte
-qemu-img: Invalid image size specified. You may use k, M, G, T, P or E suffixes for
+qemu-img: Invalid image size specified: '1kilobyte'
-qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes.
 
 qemu-img create -f qcow2 -o size=1kilobyte TEST_DIR/t.qcow2
 qemu-img: TEST_DIR/t.qcow2: Parameter 'size' expects a non-negative number below 2^64
@@ -116,8 +114,7 @@ Optional suffix k, M, G, T, P or E means kilo-, mega-, giga-, tera-, peta-
 and exabytes, respectively.
 
 qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- foobar
-qemu-img: Invalid image size specified. You may use k, M, G, T, P or E suffixes for
+qemu-img: Invalid image size specified: 'foobar'
-qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes.
 
 qemu-img create -f qcow2 -o size=foobar TEST_DIR/t.qcow2
 qemu-img: TEST_DIR/t.qcow2: Parameter 'size' expects a non-negative number below 2^64

tests/qemu-iotests/153

@@ -63,7 +63,7 @@ _supported_proto file
 _run_cmd()
 {
     echo
-    (echo "$@"; "$@" 2>&1 1>/dev/null) | _filter_testdir
+    (echo "$@"; "$@" 2>&1 1>/dev/null) | _filter_testdir | _filter_qemu_img
 }
 
 _do_run_qemu()

tests/qemu-iotests/153.out

@@ -120,16 +120,16 @@ _qemu_img_wrapper compare -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
 _qemu_img_wrapper map -U TEST_DIR/t.qcow2
 
 _qemu_img_wrapper amend -o size=32M -U TEST_DIR/t.qcow2
-qemu-img: unrecognized option '-U'
+qemu-img amend: invalid option -- 'U'
-Try 'qemu-img --help' for more information
+Try 'qemu-img amend --help' for more information
 
 _qemu_img_wrapper commit -U TEST_DIR/t.qcow2
-qemu-img: unrecognized option '-U'
+qemu-img commit: invalid option -- 'U'
-Try 'qemu-img --help' for more information
+Try 'qemu-img commit --help' for more information
 
 _qemu_img_wrapper resize -U TEST_DIR/t.qcow2 32M
-qemu-img: unrecognized option '-U'
+qemu-img resize: invalid option -- 'U'
-Try 'qemu-img --help' for more information
+Try 'qemu-img resize --help' for more information
 
 _qemu_img_wrapper rebase -U TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base -F qcow2
 qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to get "write" lock
@@ -244,16 +244,16 @@ _qemu_img_wrapper compare -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
 _qemu_img_wrapper map -U TEST_DIR/t.qcow2
 
 _qemu_img_wrapper amend -o size=32M -U TEST_DIR/t.qcow2
-qemu-img: unrecognized option '-U'
+qemu-img amend: invalid option -- 'U'
-Try 'qemu-img --help' for more information
+Try 'qemu-img amend --help' for more information
 
 _qemu_img_wrapper commit -U TEST_DIR/t.qcow2
-qemu-img: unrecognized option '-U'
+qemu-img commit: invalid option -- 'U'
-Try 'qemu-img --help' for more information
+Try 'qemu-img commit --help' for more information
 
 _qemu_img_wrapper resize -U TEST_DIR/t.qcow2 32M
-qemu-img: unrecognized option '-U'
+qemu-img resize: invalid option -- 'U'
-Try 'qemu-img --help' for more information
+Try 'qemu-img resize --help' for more information
 
 _qemu_img_wrapper rebase -U TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base -F qcow2
 qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to get "write" lock
@@ -349,16 +349,16 @@ _qemu_img_wrapper compare -U TEST_DIR/t.qcow2 TEST_DIR/t.qcow2
 _qemu_img_wrapper map -U TEST_DIR/t.qcow2
 
 _qemu_img_wrapper amend -o size=32M -U TEST_DIR/t.qcow2
-qemu-img: unrecognized option '-U'
+qemu-img amend: invalid option -- 'U'
-Try 'qemu-img --help' for more information
+Try 'qemu-img amend --help' for more information
 
 _qemu_img_wrapper commit -U TEST_DIR/t.qcow2
-qemu-img: unrecognized option '-U'
+qemu-img commit: invalid option -- 'U'
-Try 'qemu-img --help' for more information
+Try 'qemu-img commit --help' for more information
 
 _qemu_img_wrapper resize -U TEST_DIR/t.qcow2 32M
-qemu-img: unrecognized option '-U'
+qemu-img resize: invalid option -- 'U'
-Try 'qemu-img --help' for more information
+Try 'qemu-img resize --help' for more information
 
 _qemu_img_wrapper rebase -U TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base -F qcow2
 

tests/qemu-iotests/178

@@ -58,7 +58,7 @@ $QEMU_IMG measure -f qcow2 # missing filename
 $QEMU_IMG measure -l snap1 # missing filename
 $QEMU_IMG measure -o , # invalid option list
 $QEMU_IMG measure -l snapshot.foo=bar # invalid snapshot option
-$QEMU_IMG measure --output foo # invalid output format
+$QEMU_IMG measure --output foo 2>&1 | _filter_qemu_img # invalid output format
 $QEMU_IMG measure --size -1 # invalid image size
 $QEMU_IMG measure -O foo "$TEST_IMG" # unknown image file format
 

tests/qemu-iotests/178.out.qcow2

@@ -12,7 +12,8 @@ qemu-img: --image-opts, -f, and -l require a filename argument.
 qemu-img: Invalid option list: ,
 qemu-img: Invalid parameter 'snapshot.foo'
 qemu-img: Failed in parsing snapshot param 'snapshot.foo=bar'
-qemu-img: --output must be used with human or json as argument.
+qemu-img: --output expects 'human' or 'json', not 'foo'
+Try 'qemu-img measure --help' for more information
 qemu-img: Invalid image size specified. Must be between 0 and 9223372036854775807.
 qemu-img: Unknown file format 'foo'
 

tests/qemu-iotests/178.out.raw

@@ -12,7 +12,8 @@ qemu-img: --image-opts, -f, and -l require a filename argument.
 qemu-img: Invalid option list: ,
 qemu-img: Invalid parameter 'snapshot.foo'
 qemu-img: Failed in parsing snapshot param 'snapshot.foo=bar'
-qemu-img: --output must be used with human or json as argument.
+qemu-img: --output expects 'human' or 'json', not 'foo'
+Try 'qemu-img measure --help' for more information
 qemu-img: Invalid image size specified. Must be between 0 and 9223372036854775807.
 qemu-img: Unknown file format 'foo'
 

tests/qemu-iotests/184.out

@@ -41,6 +41,12 @@ Testing:
             },
             "iops_wr": 0,
             "ro": false,
+            "children": [
+                {
+                    "node-name": "disk0",
+                    "child": "file"
+                }
+            ],
             "node-name": "throttle0",
             "backing_file_depth": 1,
             "drv": "throttle",
@@ -69,6 +75,8 @@ Testing:
             },
             "iops_wr": 0,
             "ro": false,
+            "children": [
+            ],
             "node-name": "disk0",
             "backing_file_depth": 0,
             "drv": "null-co",

tests/qemu-iotests/common.filter

@@ -86,6 +86,12 @@ _filter_qemu()
         -e $'s#\r##' # QEMU monitor uses \r\n line endings
 }
 
+# replace occurrences of QEMU_IMG_PROG with "qemu-img"
+_filter_qemu_img()
+{
+    sed -e "s#$QEMU_IMG_PROG#qemu-img#g"
+}
+
 # replace problematic QMP output like timestamps
 _filter_qmp()
 {

tests/qemu-iotests/tests/qom-set-drive

@@ -0,0 +1,75 @@
+#!/usr/bin/env python3
+# group: quick
+#
+# Test how changing the 'drive' property via 'qom-set' behaves.
+#
+# Copyright (C) Proxmox Server Solutions GmbH
+#
+# 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 os
+import iotests
+from iotests import imgfmt, log, qemu_img_create, QMPTestCase
+
+image_size = 1 * 1024 * 1024
+images = [os.path.join(iotests.test_dir, f'{i}.img') for i in range(0, 4)]
+
+class TestQOMSetDrive(QMPTestCase):
+    def setUp(self) -> None:
+        for image in images:
+            qemu_img_create('-f', imgfmt, image, str(image_size))
+
+        self.vm = iotests.VM()
+        for i, image in enumerate(images):
+            self.vm.add_blockdev(self.vm.qmp_to_opts({
+                'driver': imgfmt,
+                'node-name': f'node{i}',
+                'file': {
+                    'driver': 'file',
+                    'filename': image,
+                }
+            }))
+        self.vm.add_object('iothread,id=iothread0')
+        self.vm.add_device('virtio-scsi,iothread=iothread0')
+        self.vm.add_device('scsi-hd,id=iot,drive=node0')
+        self.vm.add_device('virtio-scsi')
+        self.vm.add_device('scsi-hd,id=no-iot,drive=node1')
+        self.vm.launch()
+
+    def tearDown(self) -> None:
+        self.vm.shutdown()
+        for image in images:
+            os.remove(image)
+
+    def test_qom_set_drive(self) -> None:
+        log(self.vm.qmp('qom-get', path='/machine/peripheral/iot',
+                        property='drive'))
+        log(self.vm.qmp('qom-set', path='/machine/peripheral/iot',
+                        property='drive', value='node2'))
+        log(self.vm.qmp('qom-get', path='/machine/peripheral/iot',
+                        property='drive'))
+
+        log(self.vm.qmp('qom-get', path='/machine/peripheral/no-iot',
+                        property='drive'))
+        log(self.vm.qmp('qom-set', path='/machine/peripheral/no-iot',
+                        property='drive', value='node3'))
+        log(self.vm.qmp('qom-get', path='/machine/peripheral/no-iot',
+                        property='drive'))
+
+if __name__ == '__main__':
+    iotests.activate_logging()
+    # LUKS would require special key-secret handling in add_blockdevs()
+    iotests.main(supported_fmts=['generic'],
+                 unsupported_fmts=['luks'])

tests/qemu-iotests/tests/qom-set-drive.out

@@ -0,0 +1,11 @@
+{"return": "node0"}
+{"error": {"class": "GenericError", "desc": "Different aio context is not supported for new node"}}
+{"return": "node0"}
+{"return": "node1"}
+{"return": {}}
+{"return": "node3"}
+.
+----------------------------------------------------------------------
+Ran 1 tests
+
+OK

tests/unit/test-bdrv-drain.c

@@ -193,7 +193,9 @@ static BlockBackend * no_coroutine_fn test_setup(void)
     blk_insert_bs(blk, bs, &error_abort);
 
     backing = bdrv_new_open_driver(&bdrv_test, "backing", 0, &error_abort);
+    bdrv_graph_wrlock_drained();
     bdrv_set_backing_hd(bs, backing, &error_abort);
+    bdrv_graph_wrunlock();
 
     bdrv_unref(backing);
     bdrv_unref(bs);
@@ -386,7 +388,9 @@ static void test_nested(void)
 
     backing = bdrv_new_open_driver(&bdrv_test, "backing", 0, &error_abort);
     backing_s = backing->opaque;
+    bdrv_graph_wrlock_drained();
     bdrv_set_backing_hd(bs, backing, &error_abort);
+    bdrv_graph_wrunlock();
 
     for (outer = 0; outer < DRAIN_TYPE_MAX; outer++) {
         for (inner = 0; inner < DRAIN_TYPE_MAX; inner++) {
@@ -733,10 +737,12 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type,
     src_overlay = bdrv_new_open_driver(&bdrv_test, "source-overlay",
                                        BDRV_O_RDWR, &error_abort);
 
+    bdrv_graph_wrlock_drained();
     bdrv_set_backing_hd(src_overlay, src, &error_abort);
     bdrv_unref(src);
     bdrv_set_backing_hd(src, src_backing, &error_abort);
     bdrv_unref(src_backing);
+    bdrv_graph_wrunlock();
 
     blk_src = blk_new(qemu_get_aio_context(), BLK_PERM_ALL, BLK_PERM_ALL);
     blk_insert_bs(blk_src, src_overlay, &error_abort);
@@ -772,11 +778,9 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type,
     tjob->bs = src;
     job = &tjob->common;
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     block_job_add_bdrv(job, "target", target, 0, BLK_PERM_ALL, &error_abort);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     switch (result) {
     case TEST_JOB_SUCCESS:
@@ -955,13 +959,11 @@ static void bdrv_test_top_close(BlockDriverState *bs)
 {
     BdrvChild *c, *next_c;
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     QLIST_FOREACH_SAFE(c, &bs->children, next, next_c) {
         bdrv_unref_child(bs, c);
     }
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 }
 
 static int coroutine_fn GRAPH_RDLOCK
@@ -1053,12 +1055,10 @@ static void do_test_delete_by_drain(bool detach_instead_of_delete,
 
     null_bs = bdrv_open("null-co://", NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
                         &error_abort);
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     bdrv_attach_child(bs, null_bs, "null-child", &child_of_bds,
                       BDRV_CHILD_DATA, &error_abort);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     /* This child will be the one to pass to requests through to, and
      * it will stall until a drain occurs */
@@ -1066,25 +1066,21 @@ static void do_test_delete_by_drain(bool detach_instead_of_delete,
                                     &error_abort);
     child_bs->total_sectors = 65536 >> BDRV_SECTOR_BITS;
     /* Takes our reference to child_bs */
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     tts->wait_child = bdrv_attach_child(bs, child_bs, "wait-child",
                                         &child_of_bds,
                                         BDRV_CHILD_DATA | BDRV_CHILD_PRIMARY,
                                         &error_abort);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     /* This child is just there to be deleted
      * (for detach_instead_of_delete == true) */
     null_bs = bdrv_open("null-co://", NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
                         &error_abort);
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     bdrv_attach_child(bs, null_bs, "null-child", &child_of_bds, BDRV_CHILD_DATA,
                       &error_abort);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     blk = blk_new(qemu_get_aio_context(), BLK_PERM_ALL, BLK_PERM_ALL);
     blk_insert_bs(blk, bs, &error_abort);
@@ -1167,8 +1163,7 @@ static void no_coroutine_fn detach_indirect_bh(void *opaque)
 
     bdrv_dec_in_flight(data->child_b->bs);
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     bdrv_unref_child(data->parent_b, data->child_b);
 
     bdrv_ref(data->c);
@@ -1176,7 +1171,6 @@ static void no_coroutine_fn detach_indirect_bh(void *opaque)
                                       &child_of_bds, BDRV_CHILD_DATA,
                                       &error_abort);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 }
 
 static void coroutine_mixed_fn detach_by_parent_aio_cb(void *opaque, int ret)
@@ -1274,8 +1268,7 @@ static void TSA_NO_TSA test_detach_indirect(bool by_parent_cb)
     /* Set child relationships */
     bdrv_ref(b);
     bdrv_ref(a);
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     child_b = bdrv_attach_child(parent_b, b, "PB-B", &child_of_bds,
                                 BDRV_CHILD_DATA, &error_abort);
     child_a = bdrv_attach_child(parent_b, a, "PB-A", &child_of_bds,
@@ -1286,7 +1279,6 @@ static void TSA_NO_TSA test_detach_indirect(bool by_parent_cb)
                       by_parent_cb ? &child_of_bds : &detach_by_driver_cb_class,
                       BDRV_CHILD_DATA, &error_abort);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     g_assert_cmpint(parent_a->refcnt, ==, 1);
     g_assert_cmpint(parent_b->refcnt, ==, 1);
@@ -1450,8 +1442,10 @@ static void test_drop_backing_job_commit(Job *job)
     TestDropBackingBlockJob *s =
         container_of(job, TestDropBackingBlockJob, common.job);
 
+    bdrv_graph_wrlock_drained();
     bdrv_set_backing_hd(s->bs, NULL, &error_abort);
     bdrv_set_backing_hd(s->detach_also, NULL, &error_abort);
+    bdrv_graph_wrunlock();
 
     *s->did_complete = true;
 }
@@ -1544,7 +1538,9 @@ static void test_blockjob_commit_by_drained_end(void)
         snprintf(name, sizeof(name), "parent-node-%i", i);
         bs_parents[i] = bdrv_new_open_driver(&bdrv_test, name, BDRV_O_RDWR,
                                              &error_abort);
+        bdrv_graph_wrlock_drained();
         bdrv_set_backing_hd(bs_parents[i], bs_child, &error_abort);
+        bdrv_graph_wrunlock();
     }
 
     job = block_job_create("job", &test_drop_backing_job_driver, NULL,
@@ -1693,14 +1689,13 @@ static void test_drop_intermediate_poll(void)
 
     job_node = bdrv_new_open_driver(&bdrv_test, "job-node", BDRV_O_RDWR,
                                     &error_abort);
+    bdrv_graph_wrlock_drained();
     bdrv_set_backing_hd(job_node, chain[1], &error_abort);
 
     /*
      * Establish the chain last, so the chain links are the first
      * elements in the BDS.parents lists
      */
-    bdrv_drain_all_begin();
-    bdrv_graph_wrlock();
     for (i = 0; i < 3; i++) {
         if (i) {
             /* Takes the reference to chain[i - 1] */
@@ -1709,7 +1704,6 @@ static void test_drop_intermediate_poll(void)
         }
     }
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     job = block_job_create("job", &test_simple_job_driver, NULL, job_node,
                            0, BLK_PERM_ALL, 0, 0, NULL, NULL, &error_abort);
@@ -1956,12 +1950,10 @@ static void do_test_replace_child_mid_drain(int old_drain_count,
     new_child_bs->total_sectors = 1;
 
     bdrv_ref(old_child_bs);
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     bdrv_attach_child(parent_bs, old_child_bs, "child", &child_of_bds,
                       BDRV_CHILD_COW, &error_abort);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
     parent_s->setup_completed = true;
 
     for (i = 0; i < old_drain_count; i++) {

tests/unit/test-bdrv-graph-mod.c

@@ -137,12 +137,10 @@ static void test_update_perm_tree(void)
 
     blk_insert_bs(root, bs, &error_abort);
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     bdrv_attach_child(filter, bs, "child", &child_of_bds,
                       BDRV_CHILD_DATA, &error_abort);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     ret = bdrv_append(filter, bs, NULL);
     g_assert_cmpint(ret, <, 0);
@@ -204,15 +202,13 @@ static void test_should_update_child(void)
 
     blk_insert_bs(root, bs, &error_abort);
 
+    bdrv_graph_wrlock_drained();
     bdrv_set_backing_hd(target, bs, &error_abort);
 
-    bdrv_drain_all_begin();
-    bdrv_graph_wrlock();
     g_assert(target->backing->bs == bs);
     bdrv_attach_child(filter, target, "target", &child_of_bds,
                       BDRV_CHILD_DATA, &error_abort);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
     bdrv_append(filter, bs, &error_abort);
 
     bdrv_graph_rdlock_main_loop();
@@ -248,8 +244,7 @@ static void test_parallel_exclusive_write(void)
     bdrv_ref(base);
     bdrv_ref(fl1);
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     bdrv_attach_child(top, fl1, "backing", &child_of_bds,
                       BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
                       &error_abort);
@@ -262,7 +257,6 @@ static void test_parallel_exclusive_write(void)
 
     bdrv_replace_node(fl1, fl2, &error_abort);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     bdrv_drained_end(fl2);
     bdrv_drained_end(fl1);
@@ -369,8 +363,7 @@ static void test_parallel_perm_update(void)
      */
     bdrv_ref(base);
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     bdrv_attach_child(top, ws, "file", &child_of_bds, BDRV_CHILD_DATA,
                       &error_abort);
     c_fl1 = bdrv_attach_child(ws, fl1, "first", &child_of_bds,
@@ -384,7 +377,6 @@ static void test_parallel_perm_update(void)
                       BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
                       &error_abort);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     /* Select fl1 as first child to be active */
     s->selected = c_fl1;
@@ -438,13 +430,11 @@ static void test_append_greedy_filter(void)
     BlockDriverState *base = no_perm_node("base");
     BlockDriverState *fl = exclusive_writer_node("fl1");
 
-    bdrv_drain_all_begin();
+    bdrv_graph_wrlock_drained();
-    bdrv_graph_wrlock();
     bdrv_attach_child(top, base, "backing", &child_of_bds,
                       BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
                       &error_abort);
     bdrv_graph_wrunlock();
-    bdrv_drain_all_end();
 
     bdrv_append(fl, base, &error_abort);
     bdrv_unref(fl);