[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v15 03/14] block: Replace in_use with operation
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v15 03/14] block: Replace in_use with operation blocker |
Date: |
Thu, 27 Feb 2014 13:12:08 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) |
Fam Zheng <address@hidden> writes:
> 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, ...).
> - 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.
[...]
> diff --git a/blockdev.c b/blockdev.c
> index 357f760..5c5a9c4 100644
> --- a/blockdev.c
> +++ b/blockdev.c
[...]
> @@ -1723,7 +1722,7 @@ int do_drive_del(Monitor *mon, const QDict *qdict,
> QObject **ret_data)
> qerror_report(QERR_DEVICE_NOT_FOUND, id);
> return -1;
> }
> - if (bdrv_in_use(bs)) {
> + if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, NULL)) {
> qerror_report(QERR_DEVICE_IN_USE, id);
> return -1;
> }
Loses the nice message you stored in bs->blockers[]. You could put it
to use like this:
diff --git a/blockdev.c b/blockdev.c
index 0843ca7..4ab9832 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1763,6 +1763,7 @@ void qmp_block_set_io_throttle(const char *device,
int64_t bps, int64_t bps_rd,
int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
const char *id = qdict_get_str(qdict, "id");
+ Error *local_err = NULL;
BlockDriverState *bs;
bs = bdrv_find(id);
@@ -1770,8 +1771,9 @@ int do_drive_del(Monitor *mon, const QDict *qdict,
QObject **ret_data)
qerror_report(QERR_DEVICE_NOT_FOUND, id);
return -1;
}
- if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, NULL)) {
- 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;
}
I can do it on top, if you prefer.
[...]
- [Qemu-devel] [PATCH v15 00/14] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD, Fam Zheng, 2014/02/22
- [Qemu-devel] [PATCH v15 01/14] block: Add BlockOpType enum, Fam Zheng, 2014/02/22
- [Qemu-devel] [PATCH v15 02/14] block: Introduce op_blockers to BlockDriverState, Fam Zheng, 2014/02/22
- [Qemu-devel] [PATCH v15 03/14] block: Replace in_use with operation blocker, Fam Zheng, 2014/02/22
- Re: [Qemu-devel] [PATCH v15 03/14] block: Replace in_use with operation blocker,
Markus Armbruster <=
- [Qemu-devel] [PATCH v15 04/14] block: Move op_blocker check from block_job_create to its caller, Fam Zheng, 2014/02/22
- [Qemu-devel] [PATCH v15 05/14] block: Add bdrv_set_backing_hd(), Fam Zheng, 2014/02/22
- [Qemu-devel] [PATCH v15 06/14] block: Add backing_blocker in BlockDriverState, Fam Zheng, 2014/02/22
- [Qemu-devel] [PATCH v15 07/14] block: Parse "backing" option to reference existing BDS, Fam Zheng, 2014/02/22
- [Qemu-devel] [PATCH v15 08/14] block: Support dropping active in bdrv_drop_intermediate, Fam Zheng, 2014/02/22
- [Qemu-devel] [PATCH v15 09/14] stream: Use bdrv_drop_intermediate and drop close_unused_images, Fam Zheng, 2014/02/22