From 3718d8ab65f68de2acccbe6a315907805f54e3cc Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Fri, 23 May 2014 21:29:43 +0800 Subject: block: Replace in_use with operation blocker This drops BlockDriverState.in_use with op_blockers: - Call bdrv_op_block_all in place of bdrv_set_in_use(bs, 1). - Call bdrv_op_unblock_all in place of bdrv_set_in_use(bs, 0). - Check bdrv_op_is_blocked() in place of bdrv_in_use(bs). The specific types are used, e.g. in place of starting block backup, bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP, ...). There is one exception in block_job_create, where bdrv_op_blocker_is_empty() is used, because we don't know the operation type here. This doesn't matter because in a few commits away we will drop the check and move it to callers that _do_ know the type. - Check bdrv_op_blocker_is_empty() in place of assert(!bs->in_use). Note: there is only bdrv_op_block_all and bdrv_op_unblock_all callers at this moment. So although the checks are specific to op types, this changes can still be seen as identical logic with previously with in_use. The difference is error message are improved because of blocker error info. Signed-off-by: Fam Zheng Reviewed-by: Jeff Cody Reviewed-by: Stefan Hajnoczi Signed-off-by: Stefan Hajnoczi --- blockdev.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) (limited to 'blockdev.c') diff --git a/blockdev.c b/blockdev.c index 1cbcc1c785..0a519029b8 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1334,8 +1334,8 @@ static void external_snapshot_prepare(BlkTransactionState *common, return; } - if (bdrv_in_use(state->old_bs)) { - error_set(errp, QERR_DEVICE_IN_USE, device); + if (bdrv_op_is_blocked(state->old_bs, + BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, errp)) { return; } @@ -1557,8 +1557,7 @@ exit: static void eject_device(BlockDriverState *bs, int force, Error **errp) { - if (bdrv_in_use(bs)) { - error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs)); + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) { return; } if (!bdrv_dev_has_removable_media(bs)) { @@ -1760,14 +1759,16 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) { const char *id = qdict_get_str(qdict, "id"); BlockDriverState *bs; + Error *local_err = NULL; bs = bdrv_find(id); if (!bs) { qerror_report(QERR_DEVICE_NOT_FOUND, id); return -1; } - if (bdrv_in_use(bs)) { - qerror_report(QERR_DEVICE_IN_USE, id); + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, &local_err)) { + error_report("%s", error_get_pretty(local_err)); + error_free(local_err); return -1; } @@ -2023,8 +2024,7 @@ void qmp_drive_backup(const char *device, const char *target, } } - if (bdrv_in_use(bs)) { - error_set(errp, QERR_DEVICE_IN_USE, device); + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) { return; } @@ -2157,8 +2157,7 @@ void qmp_drive_mirror(const char *device, const char *target, } } - if (bdrv_in_use(bs)) { - error_set(errp, QERR_DEVICE_IN_USE, device); + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR, errp)) { return; } -- cgit 1.4.1 From 628ff683034c83ce54a1ae91d898d44e34f4851a Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Fri, 23 May 2014 21:29:44 +0800 Subject: block: Move op_blocker check from block_job_create to its caller It makes no sense to check for "any" blocker on bs, we are here only because of the mechanical conversion from in_use to op_blockers. Remove it now, and let the callers check specific operation types. Backup and mirror already have it, add checker to stream and commit. Signed-off-by: Fam Zheng Reviewed-by: Benoit Canet Reviewed-by: Jeff Cody Reviewed-by: Stefan Hajnoczi Signed-off-by: Stefan Hajnoczi --- blockdev.c | 8 ++++++++ blockjob.c | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) (limited to 'blockdev.c') diff --git a/blockdev.c b/blockdev.c index 0a519029b8..9a9bdec7da 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1889,6 +1889,10 @@ void qmp_block_stream(const char *device, bool has_base, return; } + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_STREAM, errp)) { + return; + } + if (base) { base_bs = bdrv_find_backing_image(bs, base); if (base_bs == NULL) { @@ -1933,6 +1937,10 @@ void qmp_block_commit(const char *device, return; } + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT, errp)) { + return; + } + /* default top_bs is the active layer */ top_bs = bs; diff --git a/blockjob.c b/blockjob.c index 60e72f5d90..7d84ca1d6c 100644 --- a/blockjob.c +++ b/blockjob.c @@ -41,7 +41,7 @@ void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs, { BlockJob *job; - if (bs->job || !bdrv_op_blocker_is_empty(bs)) { + if (bs->job) { error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs)); return NULL; } -- cgit 1.4.1 From e8817e7b0e0c355690d34bb499d50373137d240f Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 16 May 2014 11:00:08 +0200 Subject: blockdev: Don't use qerror_report_err() in drive_init() qerror_report_err() is a transitional interface to help with converting existing HMP commands to QMP. It should not be used elsewhere. drive_init() is not meant to be used by QMP commands. It uses both qerror_report_err() and error_report(). Convert the former to the latter. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Signed-off-by: Stefan Hajnoczi --- blockdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'blockdev.c') diff --git a/blockdev.c b/blockdev.c index 9a9bdec7da..264b07ca4e 100644 --- a/blockdev.c +++ b/blockdev.c @@ -730,7 +730,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) &error_abort); qemu_opts_absorb_qdict(legacy_opts, bs_opts, &local_err); if (local_err) { - qerror_report_err(local_err); + error_report("%s", error_get_pretty(local_err)); error_free(local_err); goto fail; } @@ -942,7 +942,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type) dinfo = blockdev_init(filename, bs_opts, &local_err); if (dinfo == NULL) { if (local_err) { - qerror_report_err(local_err); + error_report("%s", error_get_pretty(local_err)); error_free(local_err); } goto fail; -- cgit 1.4.1 From b1422f20400e9dccd4388a4d107df5b67dba78d8 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 16 May 2014 11:00:09 +0200 Subject: blockdev: Don't use qerror_report() in do_drive_del() qerror_report() is a transitional interface to help with converting existing HMP commands to QMP. It should not be used elsewhere. do_drive_del() is an HMP command that won't be converted to QMP (we'll create a new QMP command instead). It uses both qerror_report() and error_report(). Convert the former to the latter. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Signed-off-by: Stefan Hajnoczi --- blockdev.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'blockdev.c') diff --git a/blockdev.c b/blockdev.c index 264b07ca4e..8cc42fb1d0 100644 --- a/blockdev.c +++ b/blockdev.c @@ -34,7 +34,6 @@ #include "hw/block/block.h" #include "block/blockjob.h" #include "monitor/monitor.h" -#include "qapi/qmp/qerror.h" #include "qemu/option.h" #include "qemu/config-file.h" #include "qapi/qmp/types.h" @@ -1763,7 +1762,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) bs = bdrv_find(id); if (!bs) { - qerror_report(QERR_DEVICE_NOT_FOUND, id); + error_report("Device '%s' not found", id); return -1; } if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, &local_err)) { -- cgit 1.4.1