Fam Zheng <f...@redhat.com> 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.
[...]
