diff --git a/block.c b/block.c index bfd4340b24..8848e9a7ed 100644 --- a/block.c +++ b/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(); + bdrv_graph_wrlock_drained(); 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,9 +3570,8 @@ out: * * All block nodes must be drained. */ -int bdrv_set_backing_hd_drained(BlockDriverState *bs, - BlockDriverState *backing_hd, - Error **errp) +int bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd, + Error **errp) { int ret; Transaction *tran = tran_new(); @@ -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, - QDict *options, const char *bdref_key, - BlockDriverState *parent, - const BdrvChildClass *child_class, - BdrvChildRole child_role, - bool allow_none, bool parse_filename, - Error **errp) +static BdrvChild * GRAPH_UNLOCKED +bdrv_open_child_common(const char *filename, QDict *options, + const char *bdref_key, BlockDriverState *parent, + const BdrvChildClass *child_class, + BdrvChildRole child_role, bool allow_none, + bool parse_filename, 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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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; - goto out; + return -EPERM; } ret = bdrv_inactivate_recurse(bs, true); if (ret < 0) { error_setg_errno(errp, -ret, "Failed to inactivate node"); - goto out; + return ret; } -out: - bdrv_graph_rdunlock_main_loop(); - bdrv_drain_all_end(); - return ret; + return 0; } int bdrv_inactivate_all(void) diff --git a/block/backup.c b/block/backup.c index 909027c17a..d4713fa1cd 100644 --- a/block/backup.c +++ b/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(); + bdrv_graph_wrlock_drained(); block_job_add_bdrv(&job->common, "target", target, 0, BLK_PERM_ALL, &error_abort); bdrv_graph_wrunlock(); - bdrv_drain_all_end(); return &job->common; diff --git a/block/blklogwrites.c b/block/blklogwrites.c index 70ac76f401..aa1f888869 100644 --- a/block/blklogwrites.c +++ b/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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); bdrv_unref_child(bs, s->log_file); s->log_file = NULL; bdrv_graph_wrunlock(); - bdrv_drain_all_end(); qemu_mutex_destroy(&s->mutex); } diff --git a/block/blkverify.c b/block/blkverify.c index 3a71f7498c..72efcbe7ef 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -151,12 +151,10 @@ static void blkverify_close(BlockDriverState *bs) { BDRVBlkverifyState *s = bs->opaque; - bdrv_drain_all_begin(); - bdrv_graph_wrlock(); + bdrv_graph_wrlock_drained(); 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 diff --git a/block/block-backend.c b/block/block-backend.c index 68209bb2f7..f8d6ba65c1 100644 --- a/block/block-backend.c +++ b/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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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; } diff --git a/block/commit.c b/block/commit.c index 6c4b736ff8..0d9e1a16d7 100644 --- a/block/commit.c +++ b/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(); + bdrv_graph_wrlock_drained(); 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; } diff --git a/block/file-posix.c b/block/file-posix.c index 9b5f08ccb2..8c738674ce 100644 --- a/block/file-posix.c +++ b/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, - uint64_t bytes, QEMUIOVector *qiov, int type, - int flags) +static int coroutine_fn GRAPH_RDLOCK +raw_co_prw(BlockDriverState *bs, int64_t *offset_ptr, uint64_t bytes, + 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, - int64_t bytes, QEMUIOVector *qiov, - BdrvRequestFlags flags) +static int coroutine_fn GRAPH_RDLOCK +raw_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes, + 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, - int64_t bytes, QEMUIOVector *qiov, - BdrvRequestFlags flags) +static int coroutine_fn GRAPH_RDLOCK +raw_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes, + QEMUIOVector *qiov, BdrvRequestFlags flags) { return raw_co_prw(bs, &offset, bytes, qiov, QEMU_AIO_WRITE, flags); } @@ -3606,10 +3606,11 @@ 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, - int64_t *offset, - QEMUIOVector *qiov, - BdrvRequestFlags flags) { +static int coroutine_fn GRAPH_RDLOCK +raw_co_zone_append(BlockDriverState *bs, + int64_t *offset, + QEMUIOVector *qiov, + BdrvRequestFlags flags) { assert(flags == 0); int64_t zone_size_mask = bs->bl.zone_size - 1; int64_t iov_len = 0; diff --git a/block/graph-lock.c b/block/graph-lock.c index c81162b147..b7319473a1 100644 --- a/block/graph-lock.c +++ b/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 */ - bdrv_drain_all_begin_nopoll(); + 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); - bdrv_drain_all_end(); + 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) diff --git a/block/io.c b/block/io.c index ac5c7174f6..9bd8ba8431 100644 --- a/block/io.c +++ b/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 (old_quiesce_counter == 1) { + if (--bs->quiesce_counter == 0) { GRAPH_RDLOCK_GUARD_MAINLOOP(); if (bs->drv && bs->drv->bdrv_drain_end) { bs->drv->bdrv_drain_end(bs); diff --git a/block/mirror.c b/block/mirror.c index 6e8caf4b49..b344182c74 100644 --- a/block/mirror.c +++ b/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(); + bdrv_graph_wrlock_drained(); 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); diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c index 6919a49bf5..282d1c386e 100644 --- a/block/monitor/block-hmp-cmds.c +++ b/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) diff --git a/block/qapi.c b/block/qapi.c index 2c50a6bf3b..12fbf8d1b7 100644 --- a/block/qapi.c +++ b/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); + 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); diff --git a/block/qcow2.c b/block/qcow2.c index 45451a7ee8..4aa9f9e068 100644 --- a/block/qcow2.c +++ b/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(); + bdrv_graph_wrlock_drained(); bdrv_unref_child(bs, s->data_file); bdrv_graph_wrunlock(); - bdrv_drain_all_end(); s->data_file = NULL; bdrv_graph_rdlock_main_loop(); } diff --git a/block/quorum.c b/block/quorum.c index cc3bc5f4e7..76a4feb2d9 100644 --- a/block/quorum.c +++ b/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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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); } diff --git a/block/replication.c b/block/replication.c index 0879718854..3a431e908c 100644 --- a/block/replication.c +++ b/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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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 { diff --git a/block/snapshot.c b/block/snapshot.c index 28c9c43621..bd9d759b32 100644 --- a/block/snapshot.c +++ b/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(); + bdrv_graph_wrlock_drained(); 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); diff --git a/block/stream.c b/block/stream.c index f5441f27f4..c0616b69e2 100644 --- a/block/stream.c +++ b/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 - * of unfiltered_bs is drained. Drain already here and use - * bdrv_set_backing_hd_drained() instead because the polling during - * drained_begin() might change the graph, and if we do this only later, we - * may end up working with the wrong base node (or it might even have gone - * away by the time we want to use it). + * bdrv_set_backing_hd() requires that all block nodes are drained. Drain + * already here, because the polling during drained_begin() might change the + * graph, and if we do this only later, we may end up working with the wrong + * base node (or it might even have gone 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(); + bdrv_graph_wrlock_drained(); 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; diff --git a/block/vmdk.c b/block/vmdk.c index 89a7250120..7b98debc2b 100644 --- a/block/vmdk.c +++ b/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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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; diff --git a/blockdev.c b/blockdev.c index 2e7fda6780..b451fee6e1 100644 --- a/blockdev.c +++ b/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, - aio_context, NULL, NULL); + ret = bdrv_try_change_aio_context_locked(state->old_bs, + 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; - } + return; + } - if (active) { - bdrv_activate(bs, errp); - } else { - bdrv_inactivate(bs, errp); - } + 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(); + bdrv_graph_wrlock_drained(); 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) diff --git a/blockjob.c b/blockjob.c index e68181a35b..db7c3a69a0 100644 --- a/blockjob.c +++ b/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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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; } diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst index 3653adb963..5e7b85079d 100644 --- a/docs/tools/qemu-img.rst +++ b/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 - *FMT*. Depending on the file format, you can add one or more *OPTIONS* - that enable additional features of this format. + Create the new disk image *FILE* of size *SIZE* and format + *FMT*. Depending on the file format, you can add one or more *FMT_OPTS* + 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. diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h index 84a2a4ecd5..62da83c616 100644 --- a/include/block/block-global-state.h +++ b/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, - Error **errp); -BlockDriverState *bdrv_insert_node(BlockDriverState *bs, QDict *node_options, - int flags, Error **errp); +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); 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, - QDict *options, const char *bdref_key, - BlockDriverState *parent, Error **errp); +int GRAPH_UNLOCKED +bdrv_open_file_child(const char *filename, QDict *options, + const char *bdref_key, BlockDriverState *parent, + Error **errp); BlockDriverState * no_coroutine_fn bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp); @@ -100,11 +102,9 @@ 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, - Error **errp); +bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd, + Error **errp); int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options, const char *bdref_key, Error **errp); @@ -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, - BlockDriverState *bs, QDict *options, - bool keep_old_opts); +BlockReopenQueue * GRAPH_UNLOCKED +bdrv_reopen_queue(BlockReopenQueue *bs_queue, BlockDriverState *bs, + 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,9 +144,10 @@ 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, - const char *backing_file_str, - bool backing_mask_protocol); +int GRAPH_UNLOCKED +bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base, + const char *backing_file_str, + bool backing_mask_protocol); BlockDriverState * GRAPH_RDLOCK bdrv_find_overlay(BlockDriverState *active, BlockDriverState *bs); @@ -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); diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index 925a3e7353..034c0634c8 100644 --- a/include/block/block_int-common.h +++ b/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 */ diff --git a/include/block/blockjob.h b/include/block/blockjob.h index 990f3e179a..85284cb25e 100644 --- a/include/block/blockjob.h +++ b/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: diff --git a/include/block/graph-lock.h b/include/block/graph-lock.h index 2c26c72108..95bf5ede40 100644 --- a/include/block/graph-lock.h +++ b/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); diff --git a/include/block/snapshot.h b/include/block/snapshot.h index 304cc6ea61..2316a43e26 100644 --- a/include/block/snapshot.h +++ b/include/block/snapshot.h @@ -90,9 +90,9 @@ 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, - bool has_devices, strList *devices, - Error **errp); +int GRAPH_UNLOCKED +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, Error **errp); diff --git a/include/qemu/job.h b/include/qemu/job.h index a5a04155ea..ead31578d3 100644 --- a/include/qemu/job.h +++ b/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 diff --git a/include/system/block-backend-global-state.h b/include/system/block-backend-global-state.h index 35b5e8380b..c3849640df 100644 --- a/include/system/block-backend-global-state.h +++ b/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 blk_drain_all(void); +void GRAPH_UNLOCKED blk_drain(BlockBackend *blk); +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); diff --git a/qapi/block-core.json b/qapi/block-core.json index 25b57919a7..ebbe95b3d8 100644 --- a/qapi/block-core.json +++ b/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', diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx index c9dd70a892..2c5a8a28f9 100644 --- a/qemu-img-cmds.hx +++ b/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, diff --git a/qemu-img.c b/qemu-img.c index e75707180d..7a162fdc08 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -60,7 +60,8 @@ typedef struct img_cmd_t { const char *name; - int (*handler)(int argc, char **argv); + int (*handler)(const struct img_cmd_t *ccmd, int argc, char **argv); + const char *description; } img_cmd_t; enum { @@ -72,7 +73,6 @@ enum { OPTION_FLUSH_INTERVAL = 261, OPTION_NO_DRAIN = 262, OPTION_TARGET_IMAGE_OPTS = 263, - OPTION_SIZE = 264, OPTION_PREALLOCATION = 265, OPTION_SHRINK = 266, OPTION_SALVAGE = 267, @@ -96,13 +96,15 @@ typedef enum OutputFormat { /* Default to cache=writeback as data integrity is not important for qemu-img */ #define BDRV_DEFAULT_CACHE "writeback" -static void format_print(void *opaque, const char *name) +static G_NORETURN +void tryhelp(const char *argv0) { - printf(" %s", name); + error_printf("Try '%s --help' for more information\n", argv0); + exit(EXIT_FAILURE); } -static G_NORETURN G_GNUC_PRINTF(1, 2) -void error_exit(const char *fmt, ...) +static G_NORETURN G_GNUC_PRINTF(2, 3) +void error_exit(const char *argv0, const char *fmt, ...) { va_list ap; @@ -110,130 +112,45 @@ void error_exit(const char *fmt, ...) error_vreport(fmt, ap); va_end(ap); - error_printf("Try 'qemu-img --help' for more information\n"); - exit(EXIT_FAILURE); + tryhelp(argv0); } +/* + * Print --help output for a command and exit. + * @syntax and @description are multi-line with trailing EOL + * (to allow easy extending of the text) + * @syntax has each subsequent line indented by 8 chars. + * @description is indented by 2 chars for argument on each own line, + * and with 5 chars for argument description (like -h arg below). + */ static G_NORETURN -void missing_argument(const char *option) +void cmd_help(const img_cmd_t *ccmd, + const char *syntax, const char *arguments) { - error_exit("missing argument for option '%s'", option); -} - -static G_NORETURN -void unrecognized_option(const char *option) -{ - error_exit("unrecognized option '%s'", option); -} - -/* Please keep in synch with docs/tools/qemu-img.rst */ -static G_NORETURN -void help(void) -{ - const char *help_msg = - QEMU_IMG_VERSION - "usage: qemu-img [standard options] command [command options]\n" - "QEMU disk image utility\n" - "\n" - " '-h', '--help' display this help and exit\n" - " '-V', '--version' output version information and exit\n" - " '-T', '--trace' [[enable=]][,events=][,file=]\n" - " specify tracing options\n" - "\n" - "Command syntax:\n" -#define DEF(option, callback, arg_string) \ - " " arg_string "\n" -#include "qemu-img-cmds.h" -#undef DEF - "\n" - "Command parameters:\n" - " 'filename' is a disk image filename\n" - " 'objectdef' is a QEMU user creatable object definition. See the qemu(1)\n" - " manual page for a description of the object properties. The most common\n" - " object type is a 'secret', which is used to supply passwords and/or\n" - " encryption keys.\n" - " 'fmt' is the disk image format. It is guessed automatically in most cases\n" - " 'cache' is the cache mode used to write the output disk image, the valid\n" - " options are: 'none', 'writeback' (default, except for convert), 'writethrough',\n" - " 'directsync' and 'unsafe' (default for convert)\n" - " 'src_cache' is the cache mode used to read input disk images, the valid\n" - " options are the same as for the 'cache' option\n" - " 'size' is the disk image size in bytes. Optional suffixes\n" - " 'k' or 'K' (kilobyte, 1024), 'M' (megabyte, 1024k), 'G' (gigabyte, 1024M),\n" - " 'T' (terabyte, 1024G), 'P' (petabyte, 1024T) and 'E' (exabyte, 1024P) are\n" - " supported. 'b' is ignored.\n" - " 'output_filename' is the destination disk image filename\n" - " 'output_fmt' is the destination format\n" - " 'options' is a comma separated list of format specific options in a\n" - " name=value format. Use -o help for an overview of the options supported by\n" - " the used format\n" - " 'snapshot_param' is param used for internal snapshot, format\n" - " is 'snapshot.id=[ID],snapshot.name=[NAME]', or\n" - " '[ID_OR_NAME]'\n" - " '-c' indicates that target image must be compressed (qcow format only)\n" - " '-u' allows unsafe backing chains. For rebasing, it is assumed that old and\n" - " new backing file match exactly. The image doesn't need a working\n" - " backing file before rebasing in this case (useful for renaming the\n" - " backing file). For image creation, allow creating without attempting\n" - " to open the backing file.\n" - " '-h' with or without a command shows this help and lists the supported formats\n" - " '-p' show progress of command (only certain commands)\n" - " '-q' use Quiet mode - do not print any output (except errors)\n" - " '-S' indicates the consecutive number of bytes (defaults to 4k) that must\n" - " contain only zeros for qemu-img to create a sparse image during\n" - " conversion. If the number of bytes is 0, the source will not be scanned for\n" - " unallocated or zero sectors, and the destination image will always be\n" - " fully allocated\n" - " '--output' takes the format in which the output must be done (human or json)\n" - " '-n' skips the target volume creation (useful if the volume is created\n" - " prior to running qemu-img)\n" - "\n" - "Parameters to bitmap subcommand:\n" - " 'bitmap' is the name of the bitmap to manipulate, through one or more\n" - " actions from '--add', '--remove', '--clear', '--enable', '--disable',\n" - " or '--merge source'\n" - " '-g granularity' sets the granularity for '--add' actions\n" - " '-b source' and '-F src_fmt' tell '--merge' actions to find the source\n" - " bitmaps from an alternative file\n" - "\n" - "Parameters to check subcommand:\n" - " '-r' tries to repair any inconsistencies that are found during the check.\n" - " '-r leaks' repairs only cluster leaks, whereas '-r all' fixes all\n" - " kinds of errors, with a higher risk of choosing the wrong fix or\n" - " hiding corruption that has already occurred.\n" - "\n" - "Parameters to convert subcommand:\n" - " '--bitmaps' copies all top-level persistent bitmaps to destination\n" - " '-m' specifies how many coroutines work in parallel during the convert\n" - " process (defaults to 8)\n" - " '-W' allow to write to the target out of order rather than sequential\n" - "\n" - "Parameters to snapshot subcommand:\n" - " 'snapshot' is the name of the snapshot to create, apply or delete\n" - " '-a' applies a snapshot (revert disk to saved state)\n" - " '-c' creates a snapshot\n" - " '-d' deletes a snapshot\n" - " '-l' lists all snapshots in the given image\n" - "\n" - "Parameters to compare subcommand:\n" - " '-f' first image format\n" - " '-F' second image format\n" - " '-s' run in Strict mode - fail on different image size or sector allocation\n" - "\n" - "Parameters to dd subcommand:\n" - " 'bs=BYTES' read and write up to BYTES bytes at a time " - "(default: 512)\n" - " 'count=N' copy only N input blocks\n" - " 'if=FILE' read from FILE\n" - " 'of=FILE' write to FILE\n" - " 'skip=N' skip N bs-sized blocks at the start of input\n"; - - printf("%s\nSupported formats:", help_msg); - bdrv_iterate_format(format_print, NULL, false); - printf("\n\n" QEMU_HELP_BOTTOM "\n"); + printf( +"Usage:\n" +" %s %s %s\n" +"%s.\n" +"\n" +"Arguments:\n" +" -h, --help\n" +" print this help and exit\n" +"%s\n", + "qemu-img", ccmd->name, syntax, ccmd->description, arguments); exit(EXIT_SUCCESS); } +static OutputFormat parse_output_format(const char *argv0, const char *arg) +{ + if (!strcmp(arg, "json")) { + return OFORMAT_JSON; + } else if (!strcmp(arg, "human")) { + return OFORMAT_HUMAN; + } else { + error_exit(argv0, "--output expects 'human' or 'json', not '%s'", arg); + } +} + /* * Is @list safe for accumulate_options()? * It is when multiple of them can be joined together separated by ','. @@ -481,18 +398,16 @@ static int add_old_style_options(const char *fmt, QemuOpts *opts, return 0; } -static int64_t cvtnum_full(const char *name, const char *value, int64_t min, - int64_t max) +static int64_t cvtnum_full(const char *name, const char *value, + bool is_size, int64_t min, int64_t max) { int err; uint64_t res; - err = qemu_strtosz(value, NULL, &res); + err = is_size ? qemu_strtosz(value, NULL, &res) : + qemu_strtou64(value, NULL, 0, &res); if (err < 0 && err != -ERANGE) { - error_report("Invalid %s specified. You may use " - "k, M, G, T, P or E suffixes for", name); - error_report("kilobytes, megabytes, gigabytes, terabytes, " - "petabytes and exabytes."); + error_report("Invalid %s specified: '%s'", name, value); return err; } if (err == -ERANGE || res > max || res < min) { @@ -503,15 +418,15 @@ static int64_t cvtnum_full(const char *name, const char *value, int64_t min, return res; } -static int64_t cvtnum(const char *name, const char *value) +static int64_t cvtnum(const char *name, const char *value, bool is_size) { - return cvtnum_full(name, value, 0, INT64_MAX); + return cvtnum_full(name, value, is_size, 0, INT64_MAX); } -static int img_create(int argc, char **argv) +static int img_create(const img_cmd_t *ccmd, int argc, char **argv) { int c; - uint64_t img_size = -1; + int64_t img_size = -1; const char *fmt = "raw"; const char *base_fmt = NULL; const char *filename; @@ -524,29 +439,46 @@ static int img_create(int argc, char **argv) for(;;) { static const struct option long_options[] = { {"help", no_argument, 0, 'h'}, + {"format", required_argument, 0, 'f'}, + {"options", required_argument, 0, 'o'}, + {"backing", required_argument, 0, 'b'}, + {"backing-format", required_argument, 0, 'B'}, /* was -F in 10.0 */ + {"backing-unsafe", no_argument, 0, 'u'}, + {"quiet", no_argument, 0, 'q'}, {"object", required_argument, 0, OPTION_OBJECT}, {0, 0, 0, 0} }; - c = getopt_long(argc, argv, ":F:b:f:ho:qu", + c = getopt_long(argc, argv, "hf:o:b:F:B:uq", long_options, NULL); if (c == -1) { break; } switch(c) { - case ':': - missing_argument(argv[optind - 1]); - break; - case '?': - unrecognized_option(argv[optind - 1]); - break; case 'h': - help(); - break; - case 'F': - base_fmt = optarg; - break; - case 'b': - base_filename = optarg; + cmd_help(ccmd, "[-f FMT] [-o FMT_OPTS]\n" +" [-b BACKING_FILE [-B BACKING_FMT]] [-u]\n" +" [-q] [--object OBJDEF] FILE [SIZE]\n" +, +" -f, --format FMT\n" +" specifies the format of the new image (default: raw)\n" +" -o, --options FMT_OPTS\n" +" format-specific options (specify '-o help' for help)\n" +" -b, --backing BACKING_FILE\n" +" create target image to be a CoW on top of BACKING_FILE\n" +" -B, --backing-format BACKING_FMT (was -F in <= 10.0)\n" +" specifies the format of BACKING_FILE (default: probing is used)\n" +" -u, --backing-unsafe\n" +" do not fail if BACKING_FILE can not be read\n" +" -q, --quiet\n" +" quiet mode (produce only error messages if any)\n" +" --object OBJDEF\n" +" defines QEMU user-creatable object\n" +" FILE\n" +" name of the image file to create (will be overritten if already exists)\n" +" SIZE[bKMGTPE]\n" +" image size with optional multiplier suffix (powers of 1024)\n" +" (required unless BACKING_FILE is specified)\n" +); break; case 'f': fmt = optarg; @@ -556,15 +488,24 @@ static int img_create(int argc, char **argv) goto fail; } break; - case 'q': - quiet = true; + case 'b': + base_filename = optarg; + break; + case 'F': /* <=10.0 */ + case 'B': + base_fmt = optarg; break; case 'u': flags |= BDRV_O_NO_BACKING; break; + case 'q': + quiet = true; + break; case OPTION_OBJECT: user_creatable_process_cmdline(optarg); break; + default: + tryhelp(argv[0]); } } @@ -576,22 +517,19 @@ static int img_create(int argc, char **argv) } if (optind >= argc) { - error_exit("Expecting image file name"); + error_exit(argv[0], "Expecting image file name"); } optind++; /* Get image size, if specified */ if (optind < argc) { - int64_t sval; - - sval = cvtnum("image size", argv[optind++]); - if (sval < 0) { + img_size = cvtnum("image size", argv[optind++], true); + if (img_size < 0) { goto fail; } - img_size = (uint64_t)sval; } if (optind != argc) { - error_exit("Unexpected argument: %s", argv[optind]); + error_exit(argv[0], "Unexpected argument: %s", argv[optind]); } bdrv_img_create(filename, fmt, base_filename, base_fmt, @@ -716,11 +654,11 @@ static int collect_image_check(BlockDriverState *bs, * 3 - Check completed, image has leaked clusters, but is good otherwise * 63 - Checks are not supported by the image format */ -static int img_check(int argc, char **argv) +static int img_check(const img_cmd_t *ccmd, int argc, char **argv) { int c, ret; OutputFormat output_format = OFORMAT_HUMAN; - const char *filename, *fmt, *output, *cache; + const char *filename, *fmt, *cache; BlockBackend *blk; BlockDriverState *bs; int fix = 0; @@ -732,7 +670,6 @@ static int img_check(int argc, char **argv) bool force_share = false; fmt = NULL; - output = NULL; cache = BDRV_DEFAULT_CACHE; for(;;) { @@ -740,31 +677,57 @@ static int img_check(int argc, char **argv) static const struct option long_options[] = { {"help", no_argument, 0, 'h'}, {"format", required_argument, 0, 'f'}, - {"repair", required_argument, 0, 'r'}, - {"output", required_argument, 0, OPTION_OUTPUT}, - {"object", required_argument, 0, OPTION_OBJECT}, {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, + {"cache", required_argument, 0, 'T'}, + {"repair", required_argument, 0, 'r'}, {"force-share", no_argument, 0, 'U'}, + {"output", required_argument, 0, OPTION_OUTPUT}, + {"quiet", no_argument, 0, 'q'}, + {"object", required_argument, 0, OPTION_OBJECT}, {0, 0, 0, 0} }; - c = getopt_long(argc, argv, ":hf:r:T:qU", + c = getopt_long(argc, argv, "hf:T:r:Uq", long_options, &option_index); if (c == -1) { break; } switch(c) { - case ':': - missing_argument(argv[optind - 1]); - break; - case '?': - unrecognized_option(argv[optind - 1]); - break; case 'h': - help(); + cmd_help(ccmd, "[-f FMT | --image-opts] [-T CACHE_MODE] [-r leaks|all]\n" +" [-U] [--output human|json] [-q] [--object OBJDEF] FILE\n" +, +" -f, --format FMT\n" +" specifies the format of the image explicitly (default: probing is used)\n" +" --image-opts\n" +" treat FILE as an option string (key=value,..), not a file name\n" +" (incompatible with -f|--format)\n" +" -T, --cache CACHE_MODE\n" /* why not -t ? */ +" cache mode (default: " BDRV_DEFAULT_CACHE ")\n" +" -r, --repair leaks|all\n" +" repair errors of the given category in the image (image will be\n" +" opened in read-write mode, incompatible with -U|--force-share)\n" +" -U, --force-share\n" +" open image in shared mode for concurrent access\n" +" --output human|json\n" +" output format (default: human)\n" +" -q, --quiet\n" +" quiet mode (produce only error messages if any)\n" +" --object OBJDEF\n" +" defines QEMU user-creatable object\n" +" FILE\n" +" name of the image file, or an option string (key=value,..)\n" +" with --image-opts, to operate on\n" +); break; case 'f': fmt = optarg; break; + case OPTION_IMAGE_OPTS: + image_opts = true; + break; + case 'T': + cache = optarg; + break; case 'r': flags |= BDRV_O_RDWR; @@ -773,44 +736,32 @@ static int img_check(int argc, char **argv) } else if (!strcmp(optarg, "all")) { fix = BDRV_FIX_LEAKS | BDRV_FIX_ERRORS; } else { - error_exit("Unknown option value for -r " - "(expecting 'leaks' or 'all'): %s", optarg); + error_exit(argv[0], + "--repair (-r) expects 'leaks' or 'all', not '%s'", + optarg); } break; - case OPTION_OUTPUT: - output = optarg; - break; - case 'T': - cache = optarg; - break; - case 'q': - quiet = true; - break; case 'U': force_share = true; break; + case OPTION_OUTPUT: + output_format = parse_output_format(argv[0], optarg); + break; + case 'q': + quiet = true; + break; case OPTION_OBJECT: user_creatable_process_cmdline(optarg); break; - case OPTION_IMAGE_OPTS: - image_opts = true; - break; + default: + tryhelp(argv[0]); } } if (optind != argc - 1) { - error_exit("Expecting one image file name"); + error_exit(argv[0], "Expecting one image file name"); } filename = argv[optind++]; - if (output && !strcmp(output, "json")) { - output_format = OFORMAT_JSON; - } else if (output && !strcmp(output, "human")) { - output_format = OFORMAT_HUMAN; - } else if (output) { - error_report("--output must be used with human or json as argument."); - return 1; - } - ret = bdrv_parse_cache_mode(cache, &flags, &writethrough); if (ret < 0) { error_report("Invalid source cache option: %s", cache); @@ -948,7 +899,7 @@ static void run_block_job(BlockJob *job, Error **errp) } } -static int img_commit(int argc, char **argv) +static int img_commit(const img_cmd_t *ccmd, int argc, char **argv) { int c, ret, flags; const char *filename, *fmt, *cache, *base; @@ -968,38 +919,73 @@ static int img_commit(int argc, char **argv) for(;;) { static const struct option long_options[] = { {"help", no_argument, 0, 'h'}, - {"object", required_argument, 0, OPTION_OBJECT}, + {"format", required_argument, 0, 'f'}, {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, + {"cache", required_argument, 0, 't'}, + {"drop", no_argument, 0, 'd'}, + {"base", required_argument, 0, 'b'}, + {"rate-limit", required_argument, 0, 'r'}, + {"progress", no_argument, 0, 'p'}, + {"quiet", no_argument, 0, 'q'}, + {"object", required_argument, 0, OPTION_OBJECT}, {0, 0, 0, 0} }; - c = getopt_long(argc, argv, ":f:ht:b:dpqr:", + c = getopt_long(argc, argv, "hf:t:db:r:pq", long_options, NULL); if (c == -1) { break; } switch(c) { - case ':': - missing_argument(argv[optind - 1]); - break; - case '?': - unrecognized_option(argv[optind - 1]); - break; case 'h': - help(); + cmd_help(ccmd, "[-f FMT | --image-opts] [-t CACHE_MODE] [-b BASE_IMG]\n" +" [-d] [-r RATE] [-q] [--object OBJDEF] FILE\n" +, +" -f, --format FMT\n" +" specify FILE image format explicitly (default: probing is used)\n" +" --image-opts\n" +" treat FILE as an option string (key=value,..), not a file name\n" +" (incompatible with -f|--format)\n" +" -t, --cache CACHE_MODE image cache mode (default: " BDRV_DEFAULT_CACHE ")\n" +" -d, --drop\n" +" skip emptying FILE on completion\n" +" -b, --base BASE_IMG\n" +" image in the backing chain to commit change to\n" +" (default: immediate backing file; implies --drop)\n" +" -r, --rate-limit RATE\n" +" I/O rate limit, in bytes per second\n" +" -p, --progress\n" +" display progress information\n" +" -q, --quiet\n" +" quiet mode (produce only error messages if any)\n" +" --object OBJDEF\n" +" defines QEMU user-creatable object\n" +" FILE\n" +" name of the image file, or an option string (key=value,..)\n" +" with --image-opts, to operate on\n" +); break; case 'f': fmt = optarg; break; + case OPTION_IMAGE_OPTS: + image_opts = true; + break; case 't': cache = optarg; break; + case 'd': + drop = true; + break; case 'b': base = optarg; /* -b implies -d */ drop = true; break; - case 'd': - drop = true; + case 'r': + rate_limit = cvtnum("rate limit", optarg, true); + if (rate_limit < 0) { + return 1; + } break; case 'p': progress = true; @@ -1007,18 +993,11 @@ static int img_commit(int argc, char **argv) case 'q': quiet = true; break; - case 'r': - rate_limit = cvtnum("rate limit", optarg); - if (rate_limit < 0) { - return 1; - } - break; case OPTION_OBJECT: user_creatable_process_cmdline(optarg); break; - case OPTION_IMAGE_OPTS: - image_opts = true; - break; + default: + tryhelp(argv[0]); } } @@ -1028,7 +1007,7 @@ static int img_commit(int argc, char **argv) } if (optind != argc - 1) { - error_exit("Expecting one image file name"); + error_exit(argv[0], "Expecting one image file name"); } filename = argv[optind++]; @@ -1355,7 +1334,7 @@ static int check_empty_sectors(BlockBackend *blk, int64_t offset, * 1 - Images differ * >1 - Error occurred */ -static int img_compare(int argc, char **argv) +static int img_compare(const img_cmd_t *ccmd, int argc, char **argv) { const char *fmt1 = NULL, *fmt2 = NULL, *cache, *filename1, *filename2; BlockBackend *blk1, *blk2; @@ -1380,25 +1359,51 @@ static int img_compare(int argc, char **argv) for (;;) { static const struct option long_options[] = { {"help", no_argument, 0, 'h'}, - {"object", required_argument, 0, OPTION_OBJECT}, + {"a-format", required_argument, 0, 'f'}, + {"b-format", required_argument, 0, 'F'}, {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, + {"strict", no_argument, 0, 's'}, + {"cache", required_argument, 0, 'T'}, {"force-share", no_argument, 0, 'U'}, + {"progress", no_argument, 0, 'p'}, + {"quiet", no_argument, 0, 'q'}, + {"object", required_argument, 0, OPTION_OBJECT}, {0, 0, 0, 0} }; - c = getopt_long(argc, argv, ":hf:F:T:pqsU", + c = getopt_long(argc, argv, "hf:F:sT:Upq", long_options, NULL); if (c == -1) { break; } switch (c) { - case ':': - missing_argument(argv[optind - 1]); - break; - case '?': - unrecognized_option(argv[optind - 1]); - break; case 'h': - help(); + cmd_help(ccmd, +"[[-f FMT] [-F FMT] | --image-opts] [-s] [-T CACHE]\n" +" [-U] [-p] [-q] [--object OBJDEF] FILE1 FILE2\n" +, +" -f, --a-format FMT\n" +" specify FILE1 image format explicitly (default: probing is used)\n" +" -F, --b-format FMT\n" +" specify FILE2 image format explicitly (default: probing is used)\n" +" --image-opts\n" +" treat FILE1 and FILE2 as option strings (key=value,..), not file names\n" +" (incompatible with -f|--a-format and -F|--b-format)\n" +" -s, --strict\n" +" strict mode, also check if sizes are equal\n" +" -T, --cache CACHE_MODE\n" +" images caching mode (default: " BDRV_DEFAULT_CACHE ")\n" +" -U, --force-share\n" +" open images in shared mode for concurrent access\n" +" -p, --progress\n" +" display progress information\n" +" -q, --quiet\n" +" quiet mode (produce only error messages if any)\n" +" --object OBJDEF\n" +" defines QEMU user-creatable object\n" +" FILE1, FILE2\n" +" names of the image files, or option strings (key=value,..)\n" +" with --image-opts, to compare\n" +); break; case 'f': fmt1 = optarg; @@ -1406,39 +1411,29 @@ static int img_compare(int argc, char **argv) case 'F': fmt2 = optarg; break; + case OPTION_IMAGE_OPTS: + image_opts = true; + break; + case 's': + strict = true; + break; case 'T': cache = optarg; break; + case 'U': + force_share = true; + break; case 'p': progress = true; break; case 'q': quiet = true; break; - case 's': - strict = true; - break; - case 'U': - force_share = true; - break; case OPTION_OBJECT: - { - Error *local_err = NULL; - - if (!user_creatable_add_from_str(optarg, &local_err)) { - if (local_err) { - error_report_err(local_err); - exit(2); - } else { - /* Help was printed */ - exit(EXIT_SUCCESS); - } - } - break; - } - case OPTION_IMAGE_OPTS: - image_opts = true; + user_creatable_process_cmdline(optarg); break; + default: + tryhelp(argv[0]); } } @@ -1449,7 +1444,7 @@ static int img_compare(int argc, char **argv) if (optind != argc - 2) { - error_exit("Expecting two image file names"); + error_exit(argv[0], "Expecting two image file names"); } filename1 = argv[optind++]; filename2 = argv[optind++]; @@ -2231,7 +2226,7 @@ static void set_rate_limit(BlockBackend *blk, int64_t rate_limit) blk_set_io_limits(blk, &cfg); } -static int img_convert(int argc, char **argv) +static int img_convert(const img_cmd_t *ccmd, int argc, char **argv) { int c, bs_i, flags, src_flags = BDRV_O_NO_SHARE; const char *fmt = NULL, *out_fmt = NULL, *cache = "unsafe", @@ -2267,53 +2262,122 @@ static int img_convert(int argc, char **argv) for(;;) { static const struct option long_options[] = { {"help", no_argument, 0, 'h'}, - {"object", required_argument, 0, OPTION_OBJECT}, + {"source-format", required_argument, 0, 'f'}, + /* + * XXX: historic --image-opts acts on source file only, + * it seems better to have it affect both source and target, + * and have separate --source-image-opts for source, + * but this might break existing setups. + */ {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, - {"force-share", no_argument, 0, 'U'}, - {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS}, - {"salvage", no_argument, 0, OPTION_SALVAGE}, - {"target-is-zero", no_argument, 0, OPTION_TARGET_IS_ZERO}, + {"source-cache", required_argument, 0, 'T'}, + {"snapshot", required_argument, 0, 'l'}, {"bitmaps", no_argument, 0, OPTION_BITMAPS}, {"skip-broken-bitmaps", no_argument, 0, OPTION_SKIP_BROKEN}, + {"salvage", no_argument, 0, OPTION_SALVAGE}, + {"target-format", required_argument, 0, 'O'}, + {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS}, + {"target-format-options", required_argument, 0, 'o'}, + {"target-cache", required_argument, 0, 't'}, + {"backing", required_argument, 0, 'b'}, + {"backing-format", required_argument, 0, 'F'}, + {"sparse-size", required_argument, 0, 'S'}, + {"no-create", no_argument, 0, 'n'}, + {"target-is-zero", no_argument, 0, OPTION_TARGET_IS_ZERO}, + {"force-share", no_argument, 0, 'U'}, + {"rate-limit", required_argument, 0, 'r'}, + {"parallel", required_argument, 0, 'm'}, + {"oob-writes", no_argument, 0, 'W'}, + {"copy-range-offloading", no_argument, 0, 'C'}, + {"progress", no_argument, 0, 'p'}, + {"quiet", no_argument, 0, 'q'}, + {"object", required_argument, 0, OPTION_OBJECT}, {0, 0, 0, 0} }; - c = getopt_long(argc, argv, ":hf:O:B:CcF:o:l:S:pt:T:qnm:WUr:", + c = getopt_long(argc, argv, "hf:O:b:B:CcF:o:l:S:pt:T:nm:WUr:q", long_options, NULL); if (c == -1) { break; } - switch(c) { - case ':': - missing_argument(argv[optind - 1]); - break; - case '?': - unrecognized_option(argv[optind - 1]); - break; + switch (c) { case 'h': - help(); + cmd_help(ccmd, "[-f SRC_FMT | --image-opts] [-T SRC_CACHE]\n" +" [-l SNAPSHOT] [--bitmaps [--skip-broken-bitmaps]] [--salvage]\n" +" [-O TGT_FMT | --target-image-opts] [-o TGT_FMT_OPTS] [-t TGT_CACHE]\n" +" [-b BACKING_FILE [-F BACKING_FMT]] [-S SPARSE_SIZE]\n" +" [-n] [--target-is-zero] [-c]\n" +" [-U] [-r RATE] [-m NUM_PARALLEL] [-W] [-C] [-p] [-q] [--object OBJDEF]\n" +" SRC_FILE [SRC_FILE2...] TGT_FILE\n" +, +" -f, --source-format SRC_FMT\n" +" specify format of all SRC_FILEs explicitly (default: probing is used)\n" +" --image-opts\n" +" treat each SRC_FILE as an option string (key=value,...), not a file name\n" +" (incompatible with -f|--source-format)\n" +" -T, --source-cache SRC_CACHE\n" +" source image(s) cache mode (" BDRV_DEFAULT_CACHE ")\n" +" -l, --snapshot SNAPSHOT\n" +" specify source snapshot\n" +" --bitmaps\n" +" also copy any persistent bitmaps present in source\n" +" --skip-broken-bitmaps\n" +" skip (do not error out) any broken bitmaps\n" +" --salvage\n" +" ignore errors on input (convert unreadable areas to zeros)\n" +" -O, --target-format TGT_FMT\n" +" specify TGT_FILE image format (default: raw)\n" +" --target-image-opts\n" +" treat TGT_FILE as an option string (key=value,...), not a file name\n" +" (incompatible with -O|--target-format)\n" +" -o, --target-format-options TGT_FMT_OPTS\n" +" TGT_FMT-specific options\n" +" -t, --target-cache TGT_CACHE\n" +" cache mode when opening output image (default: unsafe)\n" +" -b, --backing BACKING_FILE (was -B in <= 10.0)\n" +" create target image to be a CoW on top of BACKING_FILE\n" +" -F, --backing-format BACKING_FMT\n" /* -B used for -b in <=10.0 */ +" specify BACKING_FILE image format explicitly (default: probing is used)\n" +" -S, --sparse-size SPARSE_SIZE[bkKMGTPE]\n" +" specify number of consecutive zero bytes to treat as a gap on output\n" +" (rounded down to nearest 512 bytes), with optional multiplier suffix\n" +" -n, --no-create\n" +" omit target volume creation (e.g. on rbd)\n" +" --target-is-zero\n" +" indicates that the target volume is pre-zeroed\n" +" -c, --compress\n" +" create compressed output image (qcow and qcow2 formats only)\n" +" -U, --force-share\n" +" open images in shared mode for concurrent access\n" +" -r, --rate-limit RATE\n" +" I/O rate limit, in bytes per second\n" +" -m, --parallel NUM_PARALLEL\n" +" specify parallelism (default: 8)\n" +" -C, --copy-range-offloading\n" +" try to use copy offloading\n" +" -W, --oob-writes\n" +" enable out-of-order writes to improve performance\n" +" -p, --progress\n" +" display progress information\n" +" -q, --quiet\n" +" quiet mode (produce only error messages if any)\n" +" --object OBJDEF\n" +" defines QEMU user-creatable object\n" +" SRC_FILE...\n" +" one or more source image file names,\n" +" or option strings (key=value,..) with --source-image-opts\n" +" TGT_FILE\n" +" target (output) image file name,\n" +" or option string (key=value,..) with --target-image-opts\n" +); break; case 'f': fmt = optarg; break; - case 'O': - out_fmt = optarg; + case OPTION_IMAGE_OPTS: + image_opts = true; break; - case 'B': - out_baseimg = optarg; - break; - case 'C': - s.copy_range = true; - break; - case 'c': - s.compressed = true; - break; - case 'F': - backing_fmt = optarg; - break; - case 'o': - if (accumulate_options(&options, optarg) < 0) { - goto fail_getopt; - } + case 'T': + src_cache = optarg; break; case 'l': if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) { @@ -2328,11 +2392,41 @@ static int img_convert(int argc, char **argv) snapshot_name = optarg; } break; + case OPTION_BITMAPS: + bitmaps = true; + break; + case OPTION_SKIP_BROKEN: + skip_broken = true; + break; + case OPTION_SALVAGE: + s.salvage = true; + break; + case 'O': + out_fmt = optarg; + break; + case OPTION_TARGET_IMAGE_OPTS: + tgt_image_opts = true; + break; + case 'o': + if (accumulate_options(&options, optarg) < 0) { + goto fail_getopt; + } + break; + case 't': + cache = optarg; + break; + case 'B': /* <=10.0 */ + case 'b': + out_baseimg = optarg; + break; + case 'F': /* can't use -B as it used as -b in <=10.0 */ + backing_fmt = optarg; + break; case 'S': { int64_t sval; - sval = cvtnum("buffer size for sparse output", optarg); + sval = cvtnum("buffer size for sparse output", optarg, true); if (sval < 0) { goto fail_getopt; } else if (!QEMU_IS_ALIGNED(sval, BDRV_SECTOR_SIZE) || @@ -2348,53 +2442,9 @@ static int img_convert(int argc, char **argv) explict_min_sparse = true; break; } - case 'p': - progress = true; - break; - case 't': - cache = optarg; - break; - case 'T': - src_cache = optarg; - break; - case 'q': - s.quiet = true; - break; case 'n': skip_create = true; break; - case 'm': - if (qemu_strtol(optarg, NULL, 0, &s.num_coroutines) || - s.num_coroutines < 1 || s.num_coroutines > MAX_COROUTINES) { - error_report("Invalid number of coroutines. Allowed number of" - " coroutines is between 1 and %d", MAX_COROUTINES); - goto fail_getopt; - } - break; - case 'W': - s.wr_in_order = false; - break; - case 'U': - force_share = true; - break; - case 'r': - rate_limit = cvtnum("rate limit", optarg); - if (rate_limit < 0) { - goto fail_getopt; - } - break; - case OPTION_OBJECT: - user_creatable_process_cmdline(optarg); - break; - case OPTION_IMAGE_OPTS: - image_opts = true; - break; - case OPTION_SALVAGE: - s.salvage = true; - break; - case OPTION_TARGET_IMAGE_OPTS: - tgt_image_opts = true; - break; case OPTION_TARGET_IS_ZERO: /* * The user asserting that the target is blank has the @@ -2403,12 +2453,42 @@ static int img_convert(int argc, char **argv) */ s.has_zero_init = true; break; - case OPTION_BITMAPS: - bitmaps = true; + case 'c': + s.compressed = true; break; - case OPTION_SKIP_BROKEN: - skip_broken = true; + case 'U': + force_share = true; break; + case 'r': + rate_limit = cvtnum("rate limit", optarg, true); + if (rate_limit < 0) { + goto fail_getopt; + } + break; + case 'm': + s.num_coroutines = cvtnum_full("number of coroutines", optarg, + false, 1, MAX_COROUTINES); + if (s.num_coroutines < 0) { + goto fail_getopt; + } + break; + case 'W': + s.wr_in_order = false; + break; + case 'C': + s.copy_range = true; + break; + case 'p': + progress = true; + break; + case 'q': + s.quiet = true; + break; + case OPTION_OBJECT: + user_creatable_process_cmdline(optarg); + break; + default: + tryhelp(argv[0]); } } @@ -2999,79 +3079,82 @@ err: return NULL; } -static int img_info(int argc, char **argv) +static int img_info(const img_cmd_t *ccmd, int argc, char **argv) { int c; OutputFormat output_format = OFORMAT_HUMAN; bool chain = false; - const char *filename, *fmt, *output; + const char *filename, *fmt; BlockGraphInfoList *list; bool image_opts = false; bool force_share = false; fmt = NULL; - output = NULL; for(;;) { - int option_index = 0; static const struct option long_options[] = { {"help", no_argument, 0, 'h'}, {"format", required_argument, 0, 'f'}, - {"output", required_argument, 0, OPTION_OUTPUT}, - {"backing-chain", no_argument, 0, OPTION_BACKING_CHAIN}, - {"object", required_argument, 0, OPTION_OBJECT}, {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, + {"backing-chain", no_argument, 0, OPTION_BACKING_CHAIN}, {"force-share", no_argument, 0, 'U'}, + {"output", required_argument, 0, OPTION_OUTPUT}, + {"object", required_argument, 0, OPTION_OBJECT}, {0, 0, 0, 0} }; - c = getopt_long(argc, argv, ":f:hU", - long_options, &option_index); + c = getopt_long(argc, argv, "hf:U", long_options, NULL); if (c == -1) { break; } switch(c) { - case ':': - missing_argument(argv[optind - 1]); - break; - case '?': - unrecognized_option(argv[optind - 1]); - break; case 'h': - help(); + cmd_help(ccmd, "[-f FMT | --image-opts] [--backing-chain] [-U]\n" +" [--output human|json] [--object OBJDEF] FILE\n" +, +" -f, --format FMT\n" +" specify FILE image format explicitly (default: probing is used)\n" +" --image-opts\n" +" treat FILE as an option string (key=value,..), not a file name\n" +" (incompatible with -f|--format)\n" +" --backing-chain\n" +" display information about the backing chain for copy-on-write overlays\n" +" -U, --force-share\n" +" open image in shared mode for concurrent access\n" +" --output human|json\n" +" specify output format (default: human)\n" +" --object OBJDEF\n" +" defines QEMU user-creatable object\n" +" FILE\n" +" name of the image file, or option string (key=value,..)\n" +" with --image-opts, to operate on\n" +); break; case 'f': fmt = optarg; break; + case OPTION_IMAGE_OPTS: + image_opts = true; + break; + case OPTION_BACKING_CHAIN: + chain = true; + break; case 'U': force_share = true; break; case OPTION_OUTPUT: - output = optarg; - break; - case OPTION_BACKING_CHAIN: - chain = true; + output_format = parse_output_format(argv[0], optarg); break; case OPTION_OBJECT: user_creatable_process_cmdline(optarg); break; - case OPTION_IMAGE_OPTS: - image_opts = true; - break; + default: + tryhelp(argv[0]); } } if (optind != argc - 1) { - error_exit("Expecting one image file name"); + error_exit(argv[0], "Expecting one image file name"); } filename = argv[optind++]; - if (output && !strcmp(output, "json")) { - output_format = OFORMAT_JSON; - } else if (output && !strcmp(output, "human")) { - output_format = OFORMAT_HUMAN; - } else if (output) { - error_report("--output must be used with human or json as argument."); - return 1; - } - list = collect_image_info_list(image_opts, filename, fmt, chain, force_share); if (!list) { @@ -3224,13 +3307,13 @@ static inline bool entry_mergeable(const MapEntry *curr, const MapEntry *next) return true; } -static int img_map(int argc, char **argv) +static int img_map(const img_cmd_t *ccmd, int argc, char **argv) { int c; OutputFormat output_format = OFORMAT_HUMAN; BlockBackend *blk; BlockDriverState *bs; - const char *filename, *fmt, *output; + const char *filename, *fmt; int64_t length; MapEntry curr = { .length = 0 }, next; int ret = 0; @@ -3240,78 +3323,85 @@ static int img_map(int argc, char **argv) int64_t max_length = -1; fmt = NULL; - output = NULL; for (;;) { - int option_index = 0; static const struct option long_options[] = { {"help", no_argument, 0, 'h'}, {"format", required_argument, 0, 'f'}, - {"output", required_argument, 0, OPTION_OUTPUT}, - {"object", required_argument, 0, OPTION_OBJECT}, {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, - {"force-share", no_argument, 0, 'U'}, {"start-offset", required_argument, 0, 's'}, {"max-length", required_argument, 0, 'l'}, + {"force-share", no_argument, 0, 'U'}, + {"output", required_argument, 0, OPTION_OUTPUT}, + {"object", required_argument, 0, OPTION_OBJECT}, {0, 0, 0, 0} }; - c = getopt_long(argc, argv, ":f:s:l:hU", - long_options, &option_index); + c = getopt_long(argc, argv, "hf:s:l:U", + long_options, NULL); if (c == -1) { break; } switch (c) { - case ':': - missing_argument(argv[optind - 1]); - break; - case '?': - unrecognized_option(argv[optind - 1]); - break; case 'h': - help(); + cmd_help(ccmd, "[-f FMT | --image-opts]\n" +" [--start-offset OFFSET] [--max-length LENGTH]\n" +" [--output human|json] [-U] [--object OBJDEF] FILE\n" +, +" -f, --format FMT\n" +" specify FILE image format explicitly (default: probing is used)\n" +" --image-opts\n" +" treat FILE as an option string (key=value,..), not a file name\n" +" (incompatible with -f|--format)\n" +" -s, --start-offset OFFSET\n" +" start at the given OFFSET in the image, not at the beginning\n" +" -l, --max-length LENGTH\n" +" process at most LENGTH bytes instead of up to the end of the image\n" +" --output human|json\n" +" specify output format name (default: human)\n" +" -U, --force-share\n" +" open image in shared mode for concurrent access\n" +" --object OBJDEF\n" +" defines QEMU user-creatable object\n" +" FILE\n" +" the image file name, or option string (key=value,..)\n" +" with --image-opts, to operate on\n" +); break; case 'f': fmt = optarg; break; - case 'U': - force_share = true; - break; - case OPTION_OUTPUT: - output = optarg; + case OPTION_IMAGE_OPTS: + image_opts = true; break; case 's': - start_offset = cvtnum("start offset", optarg); + start_offset = cvtnum("start offset", optarg, true); if (start_offset < 0) { return 1; } break; case 'l': - max_length = cvtnum("max length", optarg); + max_length = cvtnum("max length", optarg, true); if (max_length < 0) { return 1; } break; + case OPTION_OUTPUT: + output_format = parse_output_format(argv[0], optarg); + break; + case 'U': + force_share = true; + break; case OPTION_OBJECT: user_creatable_process_cmdline(optarg); break; - case OPTION_IMAGE_OPTS: - image_opts = true; - break; + default: + tryhelp(argv[0]); } } if (optind != argc - 1) { - error_exit("Expecting one image file name"); + error_exit(argv[0], "Expecting one image file name"); } filename = argv[optind]; - if (output && !strcmp(output, "json")) { - output_format = OFORMAT_JSON; - } else if (output && !strcmp(output, "human")) { - output_format = OFORMAT_HUMAN; - } else if (output) { - error_report("--output must be used with human or json as argument."); - return 1; - } - blk = img_open(image_opts, filename, fmt, 0, false, false, force_share); if (!blk) { return 1; @@ -3368,18 +3458,19 @@ out: return ret < 0; } -#define SNAPSHOT_LIST 1 -#define SNAPSHOT_CREATE 2 -#define SNAPSHOT_APPLY 3 -#define SNAPSHOT_DELETE 4 +/* the same as options */ +#define SNAPSHOT_LIST 'l' +#define SNAPSHOT_CREATE 'c' +#define SNAPSHOT_APPLY 'a' +#define SNAPSHOT_DELETE 'd' -static int img_snapshot(int argc, char **argv) +static int img_snapshot(const img_cmd_t *ccmd, int argc, char **argv) { BlockBackend *blk; BlockDriverState *bs; QEMUSnapshotInfo sn; - char *filename, *snapshot_name = NULL; - int c, ret = 0, bdrv_oflags; + char *filename, *fmt = NULL, *snapshot_name = NULL; + int c, ret = 0; int action = 0; bool quiet = false; Error *err = NULL; @@ -3387,86 +3478,100 @@ static int img_snapshot(int argc, char **argv) bool force_share = false; int64_t rt; - bdrv_oflags = BDRV_O_RDWR; /* Parse commandline parameters */ for(;;) { static const struct option long_options[] = { {"help", no_argument, 0, 'h'}, - {"object", required_argument, 0, OPTION_OBJECT}, + {"format", required_argument, 0, 'f'}, {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, + {"list", no_argument, 0, SNAPSHOT_LIST}, + {"apply", required_argument, 0, SNAPSHOT_APPLY}, + {"create", required_argument, 0, SNAPSHOT_CREATE}, + {"delete", required_argument, 0, SNAPSHOT_DELETE}, {"force-share", no_argument, 0, 'U'}, + {"quiet", no_argument, 0, 'q'}, + {"object", required_argument, 0, OPTION_OBJECT}, {0, 0, 0, 0} }; - c = getopt_long(argc, argv, ":la:c:d:hqU", + c = getopt_long(argc, argv, "hf:la:c:d:Uq", long_options, NULL); if (c == -1) { break; } switch(c) { - case ':': - missing_argument(argv[optind - 1]); - break; - case '?': - unrecognized_option(argv[optind - 1]); - break; case 'h': - help(); - return 0; - case 'l': - if (action) { - error_exit("Cannot mix '-l', '-a', '-c', '-d'"); - return 0; - } - action = SNAPSHOT_LIST; - bdrv_oflags &= ~BDRV_O_RDWR; /* no need for RW */ + cmd_help(ccmd, "[-f FMT | --image-opts] [-l | -a|-c|-d SNAPSHOT]\n" +" [-U] [-q] [--object OBJDEF] FILE\n" +, +" -f, --format FMT\n" +" specify FILE format explicitly (default: probing is used)\n" +" --image-opts\n" +" treat FILE as an option string (key=value,..), not a file name\n" +" (incompatible with -f|--format)\n" +" -l, --list\n" +" list snapshots in FILE (default action if no -l|-c|-a|-d is given)\n" +" -c, --create SNAPSHOT\n" +" create named snapshot\n" +" -a, --apply SNAPSHOT\n" +" apply named snapshot to the base\n" +" -d, --delete SNAPSHOT\n" +" delete named snapshot\n" +" (only one of -l|-c|-a|-d can be specified)\n" +" -U, --force-share\n" +" open image in shared mode for concurrent access\n" +" -q, --quiet\n" +" quiet mode (produce only error messages if any)\n" +" --object OBJDEF\n" +" defines QEMU user-creatable object\n" +" FILE\n" +" name of the image file, or option string (key=value,..)\n" +" with --image-opts) to operate on\n" +); break; - case 'a': - if (action) { - error_exit("Cannot mix '-l', '-a', '-c', '-d'"); - return 0; - } - action = SNAPSHOT_APPLY; - snapshot_name = optarg; - break; - case 'c': - if (action) { - error_exit("Cannot mix '-l', '-a', '-c', '-d'"); - return 0; - } - action = SNAPSHOT_CREATE; - snapshot_name = optarg; - break; - case 'd': - if (action) { - error_exit("Cannot mix '-l', '-a', '-c', '-d'"); - return 0; - } - action = SNAPSHOT_DELETE; - snapshot_name = optarg; - break; - case 'q': - quiet = true; - break; - case 'U': - force_share = true; - break; - case OPTION_OBJECT: - user_creatable_process_cmdline(optarg); + case 'f': + fmt = optarg; break; case OPTION_IMAGE_OPTS: image_opts = true; break; + case SNAPSHOT_LIST: + case SNAPSHOT_APPLY: + case SNAPSHOT_CREATE: + case SNAPSHOT_DELETE: + if (action) { + error_exit(argv[0], "Cannot mix '-l', '-a', '-c', '-d'"); + return 0; + } + action = c; + snapshot_name = optarg; + break; + case 'U': + force_share = true; + break; + case 'q': + quiet = true; + break; + case OPTION_OBJECT: + user_creatable_process_cmdline(optarg); + break; + default: + tryhelp(argv[0]); } } if (optind != argc - 1) { - error_exit("Expecting one image file name"); + error_exit(argv[0], "Expecting one image file name"); } filename = argv[optind++]; + if (!action) { + action = SNAPSHOT_LIST; + } + /* Open the image */ - blk = img_open(image_opts, filename, NULL, bdrv_oflags, false, quiet, - force_share); + blk = img_open(image_opts, filename, fmt, + action == SNAPSHOT_LIST ? 0 : BDRV_O_RDWR, + false, quiet, force_share); if (!blk) { return 1; } @@ -3533,7 +3638,7 @@ static int img_snapshot(int argc, char **argv) return 0; } -static int img_rebase(int argc, char **argv) +static int img_rebase(const img_cmd_t *ccmd, int argc, char **argv) { BlockBackend *blk = NULL, *blk_old_backing = NULL, *blk_new_backing = NULL; uint8_t *buf_old = NULL; @@ -3564,45 +3669,89 @@ static int img_rebase(int argc, char **argv) for(;;) { static const struct option long_options[] = { {"help", no_argument, 0, 'h'}, - {"object", required_argument, 0, OPTION_OBJECT}, + {"format", required_argument, 0, 'f'}, {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, - {"force-share", no_argument, 0, 'U'}, + {"cache", required_argument, 0, 't'}, {"compress", no_argument, 0, 'c'}, + {"backing", required_argument, 0, 'b'}, + {"backing-format", required_argument, 0, 'B'}, + {"backing-cache", required_argument, 0, 'T'}, + {"backing-unsafe", no_argument, 0, 'u'}, + {"force-share", no_argument, 0, 'U'}, + {"progress", no_argument, 0, 'p'}, + {"quiet", no_argument, 0, 'q'}, + {"object", required_argument, 0, OPTION_OBJECT}, {0, 0, 0, 0} }; - c = getopt_long(argc, argv, ":hf:F:b:upt:T:qUc", + c = getopt_long(argc, argv, "hf:t:cb:F:B:T:uUpq", long_options, NULL); if (c == -1) { break; } - switch(c) { - case ':': - missing_argument(argv[optind - 1]); - break; - case '?': - unrecognized_option(argv[optind - 1]); - break; + switch (c) { case 'h': - help(); + cmd_help(ccmd, "[-f FMT | --image-opts] [-t CACHE]\n" +" [-b BACKING_FILE [-B BACKING_FMT] [-T BACKING_CACHE]] [-u]\n" +" [-c] [-U] [-p] [-q] [--object OBJDEF] FILE\n" +, +" -f, --format FMT\n" +" specify FILE format explicitly (default: probing is used)\n" +" --image-opts\n" +" treat FILE as an option string (key=value,..), not a file name\n" +" (incompatible with -f|--format)\n" +" -t, --cache CACHE\n" +" cache mode for FILE (default: " BDRV_DEFAULT_CACHE ")\n" +" -b, --backing BACKING_FILE|\"\"\n" +" rebase onto this file (specify empty name for no backing file)\n" +" -B, --backing-format BACKING_FMT (was -F in <=10.0)\n" +" specify format for BACKING_FILE explicitly (default: probing is used)\n" +" -T, --backing-cache CACHE\n" +" BACKING_FILE cache mode (default: " BDRV_DEFAULT_CACHE ")\n" +" -u, --backing-unsafe\n" +" do not fail if BACKING_FILE can not be read\n" +" -c, --compress\n" +" compress image (when image supports this)\n" +" -U, --force-share\n" +" open image in shared mode for concurrent access\n" +" -p, --progress\n" +" display progress information\n" +" -q, --quiet\n" +" quiet mode (produce only error messages if any)\n" +" --object OBJDEF\n" +" defines QEMU user-creatable object\n" +" FILE\n" +" name of the image file, or option string (key=value,..)\n" +" with --image-opts, to operate on\n" +); return 0; case 'f': fmt = optarg; break; - case 'F': - out_basefmt = optarg; + case OPTION_IMAGE_OPTS: + image_opts = true; + break; + case 't': + cache = optarg; break; case 'b': out_baseimg = optarg; break; + case 'F': /* <=10.0 */ + case 'B': + out_basefmt = optarg; + break; case 'u': unsafe = 1; break; + case 'c': + compress = true; + break; + case 'U': + force_share = true; + break; case 'p': progress = 1; break; - case 't': - cache = optarg; - break; case 'T': src_cache = optarg; break; @@ -3612,15 +3761,8 @@ static int img_rebase(int argc, char **argv) case OPTION_OBJECT: user_creatable_process_cmdline(optarg); break; - case OPTION_IMAGE_OPTS: - image_opts = true; - break; - case 'U': - force_share = true; - break; - case 'c': - compress = true; - break; + default: + tryhelp(argv[0]); } } @@ -3629,10 +3771,11 @@ static int img_rebase(int argc, char **argv) } if (optind != argc - 1) { - error_exit("Expecting one image file name"); + error_exit(argv[0], "Expecting one image file name"); } if (!unsafe && !out_baseimg) { - error_exit("Must specify backing file (-b) or use unsafe mode (-u)"); + error_exit(argv[0], + "Must specify backing file (-b) or use unsafe mode (-u)"); } filename = argv[optind++]; @@ -4026,11 +4169,11 @@ out: return 0; } -static int img_resize(int argc, char **argv) +static int img_resize(const img_cmd_t *ccmd, int argc, char **argv) { Error *err = NULL; int c, ret, relative; - const char *filename, *fmt, *size; + const char *filename = NULL, *fmt = NULL, *size = NULL; int64_t n, total_size, current_size; bool quiet = false; BlockBackend *blk = NULL; @@ -4053,50 +4196,52 @@ static int img_resize(int argc, char **argv) bool image_opts = false; bool shrink = false; - /* Remove size from argv manually so that negative numbers are not treated - * as options by getopt. */ - if (argc < 3) { - error_exit("Not enough arguments"); - return 1; - } - - size = argv[--argc]; - /* Parse getopt arguments */ - fmt = NULL; for(;;) { static const struct option long_options[] = { {"help", no_argument, 0, 'h'}, - {"object", required_argument, 0, OPTION_OBJECT}, + {"format", required_argument, 0, 'f'}, {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, {"preallocation", required_argument, 0, OPTION_PREALLOCATION}, {"shrink", no_argument, 0, OPTION_SHRINK}, + {"quiet", no_argument, 0, 'q'}, + {"object", required_argument, 0, OPTION_OBJECT}, {0, 0, 0, 0} }; - c = getopt_long(argc, argv, ":f:hq", + c = getopt_long(argc, argv, "-hf:q", long_options, NULL); if (c == -1) { break; } switch(c) { - case ':': - missing_argument(argv[optind - 1]); - break; - case '?': - unrecognized_option(argv[optind - 1]); - break; case 'h': - help(); - break; + cmd_help(ccmd, "[-f FMT | --image-opts] [--preallocation PREALLOC] [--shrink]\n" +" [-q] [--object OBJDEF] FILE [+-]SIZE[bkKMGTPE]\n" +, +" -f, --format FMT\n" +" specify FILE format explicitly (default: probing is used)\n" +" --image-opts\n" +" treat FILE as an option string (key=value,...), not a file name\n" +" (incompatible with -f|--format)\n" +" --shrink\n" +" allow operation when the new size is smaller than the original\n" +" --preallocation PREALLOC\n" +" specify FMT-specific preallocation type for the new areas\n" +" -q, --quiet\n" +" quiet mode (produce only error messages if any)\n" +" --object OBJDEF\n" +" defines QEMU user-creatable object\n" +" FILE\n" +" name of the image file, or option string (key=value,..)\n" +" with --image-opts, to operate on\n" +" [+-]SIZE[bkKMGTPE]\n" +" new image size or amount by which to shrink (-)/grow (+),\n" +" with optional multiplier suffix (powers of 1024, default is bytes)\n" +); + return 0; case 'f': fmt = optarg; break; - case 'q': - quiet = true; - break; - case OPTION_OBJECT: - user_creatable_process_cmdline(optarg); - break; case OPTION_IMAGE_OPTS: image_opts = true; break; @@ -4111,12 +4256,43 @@ static int img_resize(int argc, char **argv) case OPTION_SHRINK: shrink = true; break; + case 'q': + quiet = true; + break; + case OPTION_OBJECT: + user_creatable_process_cmdline(optarg); + break; + case 1: /* a non-optional argument */ + if (!filename) { + filename = optarg; + /* see if we have -size (number) next to filename */ + if (optind < argc) { + size = argv[optind]; + if (size[0] == '-' && size[1] >= '0' && size[1] <= '9') { + ++optind; + } else { + size = NULL; + } + } + } else if (!size) { + size = optarg; + } else { + error_exit(argv[0], "Extra argument(s) in command line"); + } + break; + default: + tryhelp(argv[0]); } } - if (optind != argc - 1) { - error_exit("Expecting image file name and size"); + if (!filename && optind < argc) { + filename = argv[optind++]; + } + if (!size && optind < argc) { + size = argv[optind++]; + } + if (!filename || !size || optind < argc) { + error_exit(argv[0], "Expecting image file name and size"); } - filename = argv[optind++]; /* Choose grow, shrink, or absolute resize mode */ switch (size[0]) { @@ -4239,7 +4415,7 @@ static int print_amend_option_help(const char *format) return 0; } -static int img_amend(int argc, char **argv) +static int img_amend(const img_cmd_t *ccmd, int argc, char **argv) { Error *err = NULL; int c, ret = 0; @@ -4259,26 +4435,48 @@ static int img_amend(int argc, char **argv) for (;;) { static const struct option long_options[] = { {"help", no_argument, 0, 'h'}, - {"object", required_argument, 0, OPTION_OBJECT}, + {"options", required_argument, 0, 'o'}, + {"format", required_argument, 0, 'f'}, {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, + {"cache", required_argument, 0, 't'}, {"force", no_argument, 0, OPTION_FORCE}, + {"progress", no_argument, 0, 'p'}, + {"quiet", no_argument, 0, 'q'}, + {"object", required_argument, 0, OPTION_OBJECT}, {0, 0, 0, 0} }; - c = getopt_long(argc, argv, ":ho:f:t:pq", + c = getopt_long(argc, argv, "ho:f:t:pq", long_options, NULL); if (c == -1) { break; } switch (c) { - case ':': - missing_argument(argv[optind - 1]); - break; - case '?': - unrecognized_option(argv[optind - 1]); - break; case 'h': - help(); + cmd_help(ccmd, "-o FMT_OPTS [-f FMT | --image-opts]\n" +" [-t CACHE] [--force] [-p] [-q] [--object OBJDEF] FILE\n" +, +" -o, --options FMT_OPTS\n" +" FMT-specfic format options (required)\n" +" -f, --format FMT\n" +" specify FILE format explicitly (default: probing is used)\n" +" --image-opts\n" +" treat FILE as an option string (key=value,..), not a file name\n" +" (incompatible with -f|--format)\n" +" -t, --cache CACHE\n" +" cache mode for FILE (default: " BDRV_DEFAULT_CACHE ")\n" +" --force\n" +" allow certain unsafe operations\n" +" -p, --progres\n" +" show operation progress\n" +" -q, --quiet\n" +" quiet mode (produce only error messages if any)\n" +" --object OBJDEF\n" +" defines QEMU user-creatable object\n" +" FILE\n" +" name of the image file, or option string (key=value,..)\n" +" with --image-opts, to operate on\n" +); break; case 'o': if (accumulate_options(&options, optarg) < 0) { @@ -4289,9 +4487,15 @@ static int img_amend(int argc, char **argv) case 'f': fmt = optarg; break; + case OPTION_IMAGE_OPTS: + image_opts = true; + break; case 't': cache = optarg; break; + case OPTION_FORCE: + force = true; + break; case 'p': progress = true; break; @@ -4301,17 +4505,13 @@ static int img_amend(int argc, char **argv) case OPTION_OBJECT: user_creatable_process_cmdline(optarg); break; - case OPTION_IMAGE_OPTS: - image_opts = true; - break; - case OPTION_FORCE: - force = true; - break; + default: + tryhelp(argv[0]); } } if (!options) { - error_exit("Must specify options (-o)"); + error_exit(argv[0], "Must specify options (-o)"); } if (quiet) { @@ -4507,7 +4707,7 @@ static void bench_cb(void *opaque, int ret) } } -static int img_bench(int argc, char **argv) +static int img_bench(const img_cmd_t *ccmd, int argc, char **argv) { int c, ret = 0; const char *fmt = NULL, *filename; @@ -4517,9 +4717,9 @@ static int img_bench(int argc, char **argv) int count = 75000; int depth = 64; int64_t offset = 0; - size_t bufsize = 4096; + ssize_t bufsize = 4096; int pattern = 0; - size_t step = 0; + ssize_t step = 0; int flush_interval = 0; bool drain_on_flush = true; int64_t image_size; @@ -4535,54 +4735,106 @@ static int img_bench(int argc, char **argv) for (;;) { static const struct option long_options[] = { {"help", no_argument, 0, 'h'}, - {"flush-interval", required_argument, 0, OPTION_FLUSH_INTERVAL}, + {"format", required_argument, 0, 'f'}, {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, + {"cache", required_argument, 0, 't'}, + {"count", required_argument, 0, 'c'}, + {"depth", required_argument, 0, 'd'}, + {"offset", required_argument, 0, 'o'}, + {"buffer-size", required_argument, 0, 's'}, + {"step-size", required_argument, 0, 'S'}, + {"write", no_argument, 0, 'w'}, {"pattern", required_argument, 0, OPTION_PATTERN}, + {"flush-interval", required_argument, 0, OPTION_FLUSH_INTERVAL}, {"no-drain", no_argument, 0, OPTION_NO_DRAIN}, + {"aio", required_argument, 0, 'i'}, + {"native", no_argument, 0, 'n'}, {"force-share", no_argument, 0, 'U'}, + {"quiet", no_argument, 0, 'q'}, + {"object", required_argument, 0, OPTION_OBJECT}, {0, 0, 0, 0} }; - c = getopt_long(argc, argv, ":hc:d:f:ni:o:qs:S:t:wU", long_options, - NULL); + c = getopt_long(argc, argv, "hf:t:c:d:o:s:S:wi:nUq", + long_options, NULL); if (c == -1) { break; } switch (c) { - case ':': - missing_argument(argv[optind - 1]); - break; - case '?': - unrecognized_option(argv[optind - 1]); - break; case 'h': - help(); + cmd_help(ccmd, "[-f FMT | --image-opts] [-t CACHE]\n" +" [-c COUNT] [-d DEPTH] [-o OFFSET] [-s BUFFER_SIZE] [-S STEP_SIZE]\n" +" [-w [--pattern PATTERN] [--flush-interval INTERVAL [--no-drain]]]\n" +" [-i AIO] [-n] [-U] [-q] FILE\n" +, +" -f, --format FMT\n" +" specify FILE format explicitly\n" +" --image-opts\n" +" indicates that FILE is a complete image specification\n" +" instead of a file name (incompatible with --format)\n" +" -t, --cache CACHE\n" +" cache mode for FILE (default: " BDRV_DEFAULT_CACHE ")\n" +" -c, --count COUNT\n" +" number of I/O requests to perform\n" +" -d, --depth DEPTH\n" +" number of requests to perform in parallel\n" +" -o, --offset OFFSET\n" +" start first request at this OFFSET\n" +" -s, --buffer-size BUFFER_SIZE[bkKMGTPE]\n" +" size of each I/O request, with optional multiplier suffix\n" +" (powers of 1024, default is 4K)\n" +" -S, --step-size STEP_SIZE[bkKMGTPE]\n" +" each next request offset increment, with optional multiplier suffix\n" +" (powers of 1024, default is the same as BUFFER_SIZE)\n" +" -w, --write\n" +" perform write test (default is read)\n" +" --pattern PATTERN\n" +" write this pattern byte instead of zero\n" +" --flush-interval FLUSH_INTERVAL\n" +" issue flush after this number of requests\n" +" --no-drain\n" +" do not wait when flushing pending requests\n" +" -i, --aio AIO\n" +" async-io backend (threads, native, io_uring)\n" +" -n, --native\n" +" use native AIO backend if possible\n" +" -U, --force-share\n" +" open images in shared mode for concurrent access\n" +" -q, --quiet\n" +" quiet mode (produce only error messages if any)\n" +" --object OBJDEF\n" +" defines QEMU user-creatable object\n" +" FILE\n" +" name of the image file, or option string (key=value,..)\n" +" with --image-opts, to operate on\n" +); break; - case 'c': - { - unsigned long res; - - if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > INT_MAX) { - error_report("Invalid request count specified"); - return 1; - } - count = res; - break; - } - case 'd': - { - unsigned long res; - - if (qemu_strtoul(optarg, NULL, 0, &res) <= 0 || res > INT_MAX) { - error_report("Invalid queue depth specified"); - return 1; - } - depth = res; - break; - } case 'f': fmt = optarg; break; + case OPTION_IMAGE_OPTS: + image_opts = true; + break; + case 't': + ret = bdrv_parse_cache_mode(optarg, &flags, &writethrough); + if (ret < 0) { + error_report("Invalid cache mode"); + ret = -1; + goto out; + } + break; + case 'c': + count = cvtnum_full("request count", optarg, false, 1, INT_MAX); + if (count < 0) { + return 1; + } + break; + case 'd': + depth = cvtnum_full("queue depth", optarg, false, 1, INT_MAX); + if (depth < 0) { + return 1; + } + break; case 'n': flags |= BDRV_O_NATIVE_AIO; break; @@ -4595,89 +4847,59 @@ static int img_bench(int argc, char **argv) } break; case 'o': - { - offset = cvtnum("offset", optarg); + offset = cvtnum("offset", optarg, true); if (offset < 0) { return 1; } break; - } - break; - case 'q': - quiet = true; - break; case 's': - { - int64_t sval; - - sval = cvtnum_full("buffer size", optarg, 0, INT_MAX); - if (sval < 0) { + bufsize = cvtnum_full("buffer size", optarg, true, 1, INT_MAX); + if (bufsize < 0) { return 1; } - - bufsize = sval; break; - } case 'S': - { - int64_t sval; - - sval = cvtnum_full("step_size", optarg, 0, INT_MAX); - if (sval < 0) { + step = cvtnum_full("step size", optarg, true, 0, INT_MAX); + if (step < 0) { return 1; } - - step = sval; - break; - } - case 't': - ret = bdrv_parse_cache_mode(optarg, &flags, &writethrough); - if (ret < 0) { - error_report("Invalid cache mode"); - ret = -1; - goto out; - } break; case 'w': flags |= BDRV_O_RDWR; is_write = true; break; - case 'U': - force_share = true; - break; case OPTION_PATTERN: - { - unsigned long res; - - if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > 0xff) { - error_report("Invalid pattern byte specified"); + pattern = cvtnum_full("pattern byte", optarg, false, 0, 0xff); + if (pattern < 0) { return 1; } - pattern = res; break; - } case OPTION_FLUSH_INTERVAL: - { - unsigned long res; - - if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > INT_MAX) { - error_report("Invalid flush interval specified"); + flush_interval = cvtnum_full("flush interval", optarg, + false, 0, INT_MAX); + if (flush_interval < 0) { return 1; } - flush_interval = res; break; - } case OPTION_NO_DRAIN: drain_on_flush = false; break; - case OPTION_IMAGE_OPTS: - image_opts = true; + case 'U': + force_share = true; break; + case 'q': + quiet = true; + break; + case OPTION_OBJECT: + user_creatable_process_cmdline(optarg); + break; + default: + tryhelp(argv[0]); } } if (optind != argc - 1) { - error_exit("Expecting one image file name"); + error_exit(argv[0], "Expecting one image file name"); } filename = argv[argc - 1]; @@ -4777,7 +4999,7 @@ typedef struct ImgBitmapAction { QSIMPLEQ_ENTRY(ImgBitmapAction) next; } ImgBitmapAction; -static int img_bitmap(int argc, char **argv) +static int img_bitmap(const img_cmd_t *ccmd, int argc, char **argv) { Error *err = NULL; int c, ret = 1; @@ -4799,48 +5021,69 @@ static int img_bitmap(int argc, char **argv) for (;;) { static const struct option long_options[] = { {"help", no_argument, 0, 'h'}, - {"object", required_argument, 0, OPTION_OBJECT}, + {"format", required_argument, 0, 'f'}, {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, {"add", no_argument, 0, OPTION_ADD}, + {"granularity", required_argument, 0, 'g'}, {"remove", no_argument, 0, OPTION_REMOVE}, {"clear", no_argument, 0, OPTION_CLEAR}, {"enable", no_argument, 0, OPTION_ENABLE}, {"disable", no_argument, 0, OPTION_DISABLE}, {"merge", required_argument, 0, OPTION_MERGE}, - {"granularity", required_argument, 0, 'g'}, {"source-file", required_argument, 0, 'b'}, {"source-format", required_argument, 0, 'F'}, + {"object", required_argument, 0, OPTION_OBJECT}, {0, 0, 0, 0} }; - c = getopt_long(argc, argv, ":b:f:F:g:h", long_options, NULL); + c = getopt_long(argc, argv, "hf:g:b:F:", + long_options, NULL); if (c == -1) { break; } switch (c) { - case ':': - missing_argument(argv[optind - 1]); - break; - case '?': - unrecognized_option(argv[optind - 1]); - break; case 'h': - help(); - break; - case 'b': - src_filename = optarg; + cmd_help(ccmd, "[-f FMT | --image-opts]\n" +" ( --add [-g SIZE] | --remove | --clear | --enable | --disable |\n" +" --merge SOURCE [-b SRC_FILE [-F SRC_FMT]] )..\n" +" [--object OBJDEF] FILE BITMAP\n" +, +" -f, --format FMT\n" +" specify FILE format explicitly (default: probing is used)\n" +" --image-opts\n" +" treat FILE as an option string (key=value,..), not a file name\n" +" (incompatible with -f|--format)\n" +" --add\n" +" creates BITMAP in FILE, enables to record future edits\n" +" -g, --granularity SIZE[bKMGTPE]\n" +" sets non-default granularity for the bitmap being added,\n" +" with optional multiplier suffix (in powers of 1024)\n" +" --remove\n" +" removes BITMAP from FILE\n" +" --clear\n" +" clears BITMAP in FILE\n" +" --enable, --disable\n" +" starts and stops recording future edits to BITMAP in FILE\n" +" --merge SOURCE\n" +" merges contents of the SOURCE bitmap into BITMAP in FILE\n" +" -b, --source-file SRC_FILE\n" +" select alternative source file for --merge\n" +" -F, --source-format SRC_FMT\n" +" specify format for SRC_FILE explicitly\n" +" --object OBJDEF\n" +" defines QEMU user-creatable object\n" +" FILE\n" +" name of the image file, or option string (key=value,..)\n" +" with --image-opts, to operate on\n" +" BITMAP\n" +" name of the bitmap to add, remove, clear, enable, disable or merge to\n" +); break; case 'f': fmt = optarg; break; - case 'F': - src_fmt = optarg; - break; - case 'g': - granularity = cvtnum("granularity", optarg); - if (granularity < 0) { - return 1; - } + case OPTION_IMAGE_OPTS: + image_opts = true; break; case OPTION_ADD: act = g_new0(ImgBitmapAction, 1); @@ -4848,6 +5091,12 @@ static int img_bitmap(int argc, char **argv) QSIMPLEQ_INSERT_TAIL(&actions, act, next); add = true; break; + case 'g': + granularity = cvtnum("granularity", optarg, true); + if (granularity < 0) { + return 1; + } + break; case OPTION_REMOVE: act = g_new0(ImgBitmapAction, 1); act->act = BITMAP_REMOVE; @@ -4875,12 +5124,17 @@ static int img_bitmap(int argc, char **argv) QSIMPLEQ_INSERT_TAIL(&actions, act, next); merge = true; break; + case 'b': + src_filename = optarg; + break; + case 'F': + src_fmt = optarg; + break; case OPTION_OBJECT: user_creatable_process_cmdline(optarg); break; - case OPTION_IMAGE_OPTS: - image_opts = true; - break; + default: + tryhelp(argv[0]); } } @@ -5023,7 +5277,7 @@ static int img_dd_bs(const char *arg, { int64_t res; - res = cvtnum_full("bs", arg, 1, INT_MAX); + res = cvtnum_full("bs", arg, true, 1, INT_MAX); if (res < 0) { return 1; @@ -5037,7 +5291,7 @@ static int img_dd_count(const char *arg, struct DdIo *in, struct DdIo *out, struct DdInfo *dd) { - dd->count = cvtnum("count", arg); + dd->count = cvtnum("count", arg, true); if (dd->count < 0) { return 1; @@ -5068,7 +5322,7 @@ static int img_dd_skip(const char *arg, struct DdIo *in, struct DdIo *out, struct DdInfo *dd) { - in->offset = cvtnum("skip", arg); + in->offset = cvtnum("skip", arg, true); if (in->offset < 0) { return 1; @@ -5077,7 +5331,7 @@ static int img_dd_skip(const char *arg, return 0; } -static int img_dd(int argc, char **argv) +static int img_dd(const img_cmd_t *ccmd, int argc, char **argv) { int ret = 0; char *arg = NULL; @@ -5121,31 +5375,54 @@ static int img_dd(int argc, char **argv) }; const struct option long_options[] = { { "help", no_argument, 0, 'h'}, - { "object", required_argument, 0, OPTION_OBJECT}, + { "format", required_argument, 0, 'f'}, { "image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, + { "output-format", required_argument, 0, 'O'}, { "force-share", no_argument, 0, 'U'}, + { "object", required_argument, 0, OPTION_OBJECT}, { 0, 0, 0, 0 } }; - while ((c = getopt_long(argc, argv, ":hf:O:U", long_options, NULL))) { + while ((c = getopt_long(argc, argv, "hf:O:U", long_options, NULL))) { if (c == EOF) { break; } switch (c) { - case 'O': - out_fmt = optarg; + case 'h': + cmd_help(ccmd, "[-f FMT|--image-opts] [-O OUTPUT_FMT] [-U]\n" +" [--object OBJDEF] [bs=BLOCK_SIZE] [count=BLOCKS] if=INPUT of=OUTPUT\n" +, +" -f, --format FMT\n" +" specify format for INPUT explicitly (default: probing is used)\n" +" --image-opts\n" +" treat INPUT as an option string (key=value,..), not a file name\n" +" (incompatible with -f|--format)\n" +" -O, --output-format OUTPUT_FMT\n" +" format of the OUTPUT (default: raw)\n" +" -U, --force-share\n" +" open images in shared mode for concurrent access\n" +" --object OBJDEF\n" +" defines QEMU user-creatable object\n" +" bs=BLOCK_SIZE[bKMGTP]\n" +" size of the I/O block, with optional multiplier suffix (powers of 1024)\n" +" (default: 512)\n" +" count=COUNT\n" +" number of blocks to convert (default whole INPUT)\n" +" if=INPUT\n" +" name of the file, or option string (key=value,..)\n" +" with --image-opts, to use for input\n" +" of=OUTPUT\n" +" output file name to create (will be overridden if alrady exists)\n" +); break; case 'f': fmt = optarg; break; - case ':': - missing_argument(argv[optind - 1]); + case OPTION_IMAGE_OPTS: + image_opts = true; break; - case '?': - unrecognized_option(argv[optind - 1]); - break; - case 'h': - help(); + case 'O': + out_fmt = optarg; break; case 'U': force_share = true; @@ -5153,9 +5430,8 @@ static int img_dd(int argc, char **argv) case OPTION_OBJECT: user_creatable_process_cmdline(optarg); break; - case OPTION_IMAGE_OPTS: - image_opts = true; - break; + default: + tryhelp(argv[0]); } } @@ -5345,17 +5621,8 @@ static void dump_json_block_measure_info(BlockMeasureInfo *info) g_string_free(str, true); } -static int img_measure(int argc, char **argv) +static int img_measure(const img_cmd_t *ccmd, int argc, char **argv) { - static const struct option long_options[] = { - {"help", no_argument, 0, 'h'}, - {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, - {"object", required_argument, 0, OPTION_OBJECT}, - {"output", required_argument, 0, OPTION_OUTPUT}, - {"size", required_argument, 0, OPTION_SIZE}, - {"force-share", no_argument, 0, 'U'}, - {0, 0, 0, 0} - }; OutputFormat output_format = OFORMAT_HUMAN; BlockBackend *in_blk = NULL; BlockDriver *drv; @@ -5370,29 +5637,67 @@ static int img_measure(int argc, char **argv) QemuOpts *sn_opts = NULL; QemuOptsList *create_opts = NULL; bool image_opts = false; - uint64_t img_size = UINT64_MAX; + int64_t img_size = -1; BlockMeasureInfo *info = NULL; Error *local_err = NULL; int ret = 1; int c; - while ((c = getopt_long(argc, argv, "hf:O:o:l:U", + static const struct option long_options[] = { + {"help", no_argument, 0, 'h'}, + {"source-format", required_argument, 0, 'f'}, /* img_convert */ + {"format", required_argument, 0, 'f'}, + {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, + {"source-image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, /* img_convert */ + {"snapshot", required_argument, 0, 'l'}, + {"target-format", required_argument, 0, 'O'}, + {"target-format-options", required_argument, 0, 'o'}, /* img_convert */ + {"options", required_argument, 0, 'o'}, + {"force-share", no_argument, 0, 'U'}, + {"output", required_argument, 0, OPTION_OUTPUT}, + {"object", required_argument, 0, OPTION_OBJECT}, + {"size", required_argument, 0, 's'}, + {0, 0, 0, 0} + }; + + while ((c = getopt_long(argc, argv, "hf:l:O:o:Us:", long_options, NULL)) != -1) { switch (c) { - case '?': case 'h': - help(); + cmd_help(ccmd, "[-f FMT|--image-opts] [-l SNAPSHOT]\n" +" [-O TARGET_FMT] [-o TARGET_FMT_OPTS] [--output human|json]\n" +" [--object OBJDEF] (--size SIZE | FILE)\n" +, +" -f, --format\n" +" specify format of FILE explicitly (default: probing is used)\n" +" --image-opts\n" +" indicates that FILE is a complete image specification\n" +" instead of a file name (incompatible with --format)\n" +" -l, --snapshot SNAPSHOT\n" +" use this snapshot in FILE as source\n" +" -O, --target-format TARGET_FMT\n" +" desired target/output image format (default: raw)\n" +" -o TARGET_FMT_OPTS\n" +" options specific to TARGET_FMT\n" +" --output human|json\n" +" output format (default: human)\n" +" -U, --force-share\n" +" open images in shared mode for concurrent access\n" +" --object OBJDEF\n" +" defines QEMU user-creatable object\n" +" -s, --size SIZE[bKMGTPE]\n" +" measure file size for given image size,\n" +" with optional multiplier suffix (powers of 1024)\n" +" FILE\n" +" measure file size required to convert from FILE (either a file name\n" +" or an option string (key=value,..) with --image-options)\n" +); break; case 'f': fmt = optarg; break; - case 'O': - out_fmt = optarg; - break; - case 'o': - if (accumulate_options(&options, optarg) < 0) { - goto out; - } + case OPTION_IMAGE_OPTS: + image_opts = true; break; case 'l': if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) { @@ -5407,37 +5712,31 @@ static int img_measure(int argc, char **argv) snapshot_name = optarg; } break; + case 'O': + out_fmt = optarg; + break; + case 'o': + if (accumulate_options(&options, optarg) < 0) { + goto out; + } + break; case 'U': force_share = true; break; + case OPTION_OUTPUT: + output_format = parse_output_format(argv[0], optarg); + break; case OPTION_OBJECT: user_creatable_process_cmdline(optarg); break; - case OPTION_IMAGE_OPTS: - image_opts = true; - break; - case OPTION_OUTPUT: - if (!strcmp(optarg, "json")) { - output_format = OFORMAT_JSON; - } else if (!strcmp(optarg, "human")) { - output_format = OFORMAT_HUMAN; - } else { - error_report("--output must be used with human or json " - "as argument."); + case 's': + img_size = cvtnum("image size", optarg, true); + if (img_size < 0) { goto out; } break; - case OPTION_SIZE: - { - int64_t sval; - - sval = cvtnum("image size", optarg); - if (sval < 0) { - goto out; - } - img_size = (uint64_t)sval; - } - break; + default: + tryhelp(argv[0]); } } @@ -5452,11 +5751,11 @@ static int img_measure(int argc, char **argv) error_report("--image-opts, -f, and -l require a filename argument."); goto out; } - if (filename && img_size != UINT64_MAX) { + if (filename && img_size != -1) { error_report("--size N cannot be used together with a filename."); goto out; } - if (!filename && img_size == UINT64_MAX) { + if (!filename && img_size == -1) { error_report("Either --size N or one filename must be specified."); goto out; } @@ -5504,7 +5803,7 @@ static int img_measure(int argc, char **argv) goto out; } } - if (img_size != UINT64_MAX) { + if (img_size != -1) { qemu_opt_set_number(opts, BLOCK_OPT_SIZE, img_size, &error_abort); } @@ -5538,13 +5837,49 @@ out: } static const img_cmd_t img_cmds[] = { -#define DEF(option, callback, arg_string) \ - { option, callback }, -#include "qemu-img-cmds.h" -#undef DEF + { "amend", img_amend, + "Update format-specific options of the image" }, + { "bench", img_bench, + "Run a simple image benchmark" }, + { "bitmap", img_bitmap, + "Perform modifications of the persistent bitmap in the image" }, + { "check", img_check, + "Check basic image integrity" }, + { "commit", img_commit, + "Commit image to its backing file" }, + { "compare", img_compare, + "Check if two images have the same contents" }, + { "convert", img_convert, + "Copy one or more images to another with optional format conversion" }, + { "create", img_create, + "Create and format a new image file" }, + { "dd", img_dd, + "Copy input to output with optional format conversion" }, + { "info", img_info, + "Display information about the image" }, + { "map", img_map, + "Dump image metadata" }, + { "measure", img_measure, + "Calculate the file size required for a new image" }, + { "rebase", img_rebase, + "Change the backing file of the image" }, + { "resize", img_resize, + "Resize the image" }, + { "snapshot", img_snapshot, + "List or manipulate snapshots in the image" }, { NULL, NULL, }, }; +static void format_print(void *opaque, const char *name) +{ + int *np = opaque; + if (*np + strlen(name) > 75) { + printf("\n "); + *np = 1; + } + *np += printf(" %s", name); +} + int main(int argc, char **argv) { const img_cmd_t *cmd; @@ -5572,23 +5907,39 @@ int main(int argc, char **argv) module_call_init(MODULE_INIT_QOM); bdrv_init(); - if (argc < 2) { - error_exit("Not enough arguments"); - } qemu_add_opts(&qemu_source_opts); qemu_add_opts(&qemu_trace_opts); - while ((c = getopt_long(argc, argv, "+:hVT:", long_options, NULL)) != -1) { + while ((c = getopt_long(argc, argv, "+hVT:", long_options, NULL)) != -1) { switch (c) { - case ':': - missing_argument(argv[optind - 1]); - return 0; - case '?': - unrecognized_option(argv[optind - 1]); - return 0; case 'h': - help(); + printf( +QEMU_IMG_VERSION +"QEMU disk image utility. Usage:\n" +"\n" +" qemu-img [standard options] COMMAND [--help | command options]\n" +"\n" +"Standard options:\n" +" -h, --help\n" +" display this help and exit\n" +" -V, --version\n" +" display version info and exit\n" +" -T,--trace TRACE\n" +" specify tracing options:\n" +" [[enable=]][,events=][,file=]\n" +"\n" +"Recognized commands (run qemu-img COMMAND --help for command-specific help):\n\n"); + for (cmd = img_cmds; cmd->name != NULL; cmd++) { + printf(" %s - %s\n", cmd->name, cmd->description); + } + printf("\nSupported image formats:\n"); + c = 99; /* force a newline */ + bdrv_iterate_format(format_print, &c, false); + if (c) { + printf("\n"); + } + printf("\n" QEMU_HELP_BOTTOM "\n"); return 0; case 'V': printf(QEMU_IMG_VERSION); @@ -5596,18 +5947,16 @@ int main(int argc, char **argv) case 'T': trace_opt_parse(optarg); break; + default: + tryhelp(argv[0]); } } - cmdname = argv[optind]; - - /* reset getopt_long scanning */ - argc -= optind; - if (argc < 1) { - return 0; + if (optind >= argc) { + error_exit(argv[0], "Not enough arguments"); } - argv += optind; - qemu_reset_optind(); + + cmdname = argv[optind]; if (!trace_init_backends()) { exit(1); @@ -5618,10 +5967,16 @@ int main(int argc, char **argv) /* find the command */ for (cmd = img_cmds; cmd->name != NULL; cmd++) { if (!strcmp(cmdname, cmd->name)) { - return cmd->handler(argc, argv); + g_autofree char *argv0 = g_strdup_printf("%s %s", argv[0], cmdname); + /* reset options and getopt processing (incl return order) */ + argv += optind; + argc -= optind; + qemu_reset_optind(); + argv[0] = argv0; + return cmd->handler(cmd, argc, argv); } } /* not found */ - error_exit("Command not found: %s", cmdname); + error_exit(argv[0], "Command not found: %s", cmdname); } diff --git a/tests/qemu-iotests/049.out b/tests/qemu-iotests/049.out index 34e1b452e6..70c627538b 100644 --- a/tests/qemu-iotests/049.out +++ b/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: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes. +qemu-img: Invalid image size specified: '-1k' 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: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes. +qemu-img: Invalid image size specified: '1kilobyte' 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: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes. +qemu-img: Invalid image size specified: 'foobar' 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 diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153 index 9bc3be8f75..1e02f6a6e3 100755 --- a/tests/qemu-iotests/153 +++ b/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() diff --git a/tests/qemu-iotests/153.out b/tests/qemu-iotests/153.out index ff8e55864a..28e1a22952 100644 --- a/tests/qemu-iotests/153.out +++ b/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' -Try 'qemu-img --help' for more information +qemu-img amend: invalid option -- 'U' +Try 'qemu-img amend --help' for more information _qemu_img_wrapper commit -U TEST_DIR/t.qcow2 -qemu-img: unrecognized option '-U' -Try 'qemu-img --help' for more information +qemu-img commit: invalid option -- 'U' +Try 'qemu-img commit --help' for more information _qemu_img_wrapper resize -U TEST_DIR/t.qcow2 32M -qemu-img: unrecognized option '-U' -Try 'qemu-img --help' for more information +qemu-img resize: invalid option -- 'U' +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' -Try 'qemu-img --help' for more information +qemu-img amend: invalid option -- 'U' +Try 'qemu-img amend --help' for more information _qemu_img_wrapper commit -U TEST_DIR/t.qcow2 -qemu-img: unrecognized option '-U' -Try 'qemu-img --help' for more information +qemu-img commit: invalid option -- 'U' +Try 'qemu-img commit --help' for more information _qemu_img_wrapper resize -U TEST_DIR/t.qcow2 32M -qemu-img: unrecognized option '-U' -Try 'qemu-img --help' for more information +qemu-img resize: invalid option -- 'U' +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' -Try 'qemu-img --help' for more information +qemu-img amend: invalid option -- 'U' +Try 'qemu-img amend --help' for more information _qemu_img_wrapper commit -U TEST_DIR/t.qcow2 -qemu-img: unrecognized option '-U' -Try 'qemu-img --help' for more information +qemu-img commit: invalid option -- 'U' +Try 'qemu-img commit --help' for more information _qemu_img_wrapper resize -U TEST_DIR/t.qcow2 32M -qemu-img: unrecognized option '-U' -Try 'qemu-img --help' for more information +qemu-img resize: invalid option -- 'U' +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 diff --git a/tests/qemu-iotests/178 b/tests/qemu-iotests/178 index 8df241ead8..463c59a77f 100755 --- a/tests/qemu-iotests/178 +++ b/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 diff --git a/tests/qemu-iotests/178.out.qcow2 b/tests/qemu-iotests/178.out.qcow2 index fe193fd5f4..61506b519f 100644 --- a/tests/qemu-iotests/178.out.qcow2 +++ b/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' diff --git a/tests/qemu-iotests/178.out.raw b/tests/qemu-iotests/178.out.raw index 445e460fad..6d994a433a 100644 --- a/tests/qemu-iotests/178.out.raw +++ b/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' diff --git a/tests/qemu-iotests/184.out b/tests/qemu-iotests/184.out index 52692b6b3b..ef99bb2e9a 100644 --- a/tests/qemu-iotests/184.out +++ b/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", diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter index fc3c64bcb8..67f819d866 100644 --- a/tests/qemu-iotests/common.filter +++ b/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() { diff --git a/tests/qemu-iotests/tests/qom-set-drive b/tests/qemu-iotests/tests/qom-set-drive new file mode 100755 index 0000000000..ec8ddac4fd --- /dev/null +++ b/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 . +# + +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']) diff --git a/tests/qemu-iotests/tests/qom-set-drive.out b/tests/qemu-iotests/tests/qom-set-drive.out new file mode 100644 index 0000000000..7fc243dca6 --- /dev/null +++ b/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 diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c index 59c2793725..43b0ba8648 100644 --- a/tests/unit/test-bdrv-drain.c +++ b/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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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++) { diff --git a/tests/unit/test-bdrv-graph-mod.c b/tests/unit/test-bdrv-graph-mod.c index 7b03ebe4b0..567db99e4f 100644 --- a/tests/unit/test-bdrv-graph-mod.c +++ b/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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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(); + bdrv_graph_wrlock_drained(); 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);