* [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages
2015-03-19 15:43 [Qemu-devel] [PATCH 0/3] Add bdrv_get_device_or_node_name() Alberto Garcia
@ 2015-03-19 15:43 ` Alberto Garcia
2015-03-19 19:37 ` Max Reitz
2015-03-20 7:52 ` Markus Armbruster
0 siblings, 2 replies; 19+ messages in thread
From: Alberto Garcia @ 2015-03-19 15:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Alberto Garcia, Stefan Hajnoczi
There are several error messages that identify a BlockDriverState by
its device name. However those errors can be produced in nodes that
don't have a device name associated.
In those cases we should use bdrv_get_device_or_node_name() to fall
back to the node name and produce a more meaningful message.
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
block.c | 13 +++++++------
block/qcow.c | 4 ++--
block/qcow2.c | 2 +-
block/qed.c | 2 +-
block/vdi.c | 2 +-
block/vhdx.c | 2 +-
block/vmdk.c | 4 ++--
block/vpc.c | 2 +-
block/vvfat.c | 3 ++-
9 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/block.c b/block.c
index af284e3..655d9aa 100644
--- a/block.c
+++ b/block.c
@@ -1225,7 +1225,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
} else if (backing_hd) {
error_setg(&bs->backing_blocker,
"device is used as backing hd of '%s'",
- bdrv_get_device_name(bs));
+ bdrv_get_device_or_node_name(bs));
}
bs->backing_hd = backing_hd;
@@ -1813,7 +1813,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
if (!(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR) &&
reopen_state->flags & BDRV_O_RDWR) {
error_set(errp, QERR_DEVICE_IS_READ_ONLY,
- bdrv_get_device_name(reopen_state->bs));
+ bdrv_get_device_or_node_name(reopen_state->bs));
goto error;
}
@@ -1840,7 +1840,8 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
/* It is currently mandatory to have a bdrv_reopen_prepare()
* handler for each supported drv. */
error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- drv->format_name, bdrv_get_device_name(reopen_state->bs),
+ drv->format_name,
+ bdrv_get_device_or_node_name(reopen_state->bs),
"reopening of file");
ret = -1;
goto error;
@@ -3765,7 +3766,7 @@ void bdrv_add_key(BlockDriverState *bs, const char *key, Error **errp)
if (key) {
if (!bdrv_is_encrypted(bs)) {
error_setg(errp, "Device '%s' is not encrypted",
- bdrv_get_device_name(bs));
+ bdrv_get_device_or_node_name(bs));
} else if (bdrv_set_key(bs, key) < 0) {
error_set(errp, QERR_INVALID_PASSWORD);
}
@@ -3773,7 +3774,7 @@ void bdrv_add_key(BlockDriverState *bs, const char *key, Error **errp)
if (bdrv_key_required(bs)) {
error_set(errp, ERROR_CLASS_DEVICE_ENCRYPTED,
"'%s' (%s) is encrypted",
- bdrv_get_device_name(bs),
+ bdrv_get_device_or_node_name(bs),
bdrv_get_encrypted_filename(bs));
}
}
@@ -5545,7 +5546,7 @@ bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp)
blocker = QLIST_FIRST(&bs->op_blockers[op]);
if (errp) {
error_setg(errp, "Device '%s' is busy: %s",
- bdrv_get_device_name(bs),
+ bdrv_get_device_or_node_name(bs),
error_get_pretty(blocker->reason));
}
return true;
diff --git a/block/qcow.c b/block/qcow.c
index 0558969..d4287ca 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -124,7 +124,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
snprintf(version, sizeof(version), "QCOW version %" PRIu32,
header.version);
error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- bdrv_get_device_name(bs), "qcow", version);
+ bdrv_get_device_or_node_name(bs), "qcow", version);
ret = -ENOTSUP;
goto fail;
}
@@ -231,7 +231,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
/* Disable migration when qcow images are used */
error_set(&s->migration_blocker,
QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "qcow", bdrv_get_device_name(bs), "live migration");
+ "qcow", bdrv_get_device_or_node_name(bs), "live migration");
migrate_add_blocker(s->migration_blocker);
qemu_co_mutex_init(&s->lock);
diff --git a/block/qcow2.c b/block/qcow2.c
index 32bdf75..168006b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -207,7 +207,7 @@ static void GCC_FMT_ATTR(3, 4) report_unsupported(BlockDriverState *bs,
va_end(ap);
error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- bdrv_get_device_name(bs), "qcow2", msg);
+ bdrv_get_device_or_node_name(bs), "qcow2", msg);
}
static void report_unsupported_feature(BlockDriverState *bs,
diff --git a/block/qed.c b/block/qed.c
index 892b13c..7d32ce4 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -408,7 +408,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,
snprintf(buf, sizeof(buf), "%" PRIx64,
s->header.features & ~QED_FEATURE_MASK);
error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- bdrv_get_device_name(bs), "QED", buf);
+ bdrv_get_device_or_node_name(bs), "QED", buf);
return -ENOTSUP;
}
if (!qed_is_cluster_size_valid(s->header.cluster_size)) {
diff --git a/block/vdi.c b/block/vdi.c
index 53bd02f..f0dc364 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -504,7 +504,7 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
/* Disable migration when vdi images are used */
error_set(&s->migration_blocker,
QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vdi", bdrv_get_device_name(bs), "live migration");
+ "vdi", bdrv_get_device_or_node_name(bs), "live migration");
migrate_add_blocker(s->migration_blocker);
qemu_co_mutex_init(&s->write_lock);
diff --git a/block/vhdx.c b/block/vhdx.c
index bb3ed45..d5dfe00 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1004,7 +1004,7 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
/* Disable migration when VHDX images are used */
error_set(&s->migration_blocker,
QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vhdx", bdrv_get_device_name(bs), "live migration");
+ "vhdx", bdrv_get_device_or_node_name(bs), "live migration");
migrate_add_blocker(s->migration_blocker);
return 0;
diff --git a/block/vmdk.c b/block/vmdk.c
index 8410a15..0230070 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -669,7 +669,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
snprintf(buf, sizeof(buf), "VMDK version %" PRId32,
le32_to_cpu(header.version));
error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- bdrv_get_device_name(bs), "vmdk", buf);
+ bdrv_get_device_or_node_name(bs), "vmdk", buf);
return -ENOTSUP;
} else if (le32_to_cpu(header.version) == 3 && (flags & BDRV_O_RDWR)) {
/* VMware KB 2064959 explains that version 3 added support for
@@ -964,7 +964,7 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
/* Disable migration when VMDK images are used */
error_set(&s->migration_blocker,
QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vmdk", bdrv_get_device_name(bs), "live migration");
+ "vmdk", bdrv_get_device_or_node_name(bs), "live migration");
migrate_add_blocker(s->migration_blocker);
g_free(buf);
return 0;
diff --git a/block/vpc.c b/block/vpc.c
index 43e768e..fd418b9 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -320,7 +320,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
/* Disable migration when VHD images are used */
error_set(&s->migration_blocker,
QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vpc", bdrv_get_device_name(bs), "live migration");
+ "vpc", bdrv_get_device_or_node_name(bs), "live migration");
migrate_add_blocker(s->migration_blocker);
return 0;
diff --git a/block/vvfat.c b/block/vvfat.c
index 9be632f..9ed3291 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1182,7 +1182,8 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
if (s->qcow) {
error_set(&s->migration_blocker,
QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vvfat (rw)", bdrv_get_device_name(bs), "live migration");
+ "vvfat (rw)", bdrv_get_device_or_node_name(bs),
+ "live migration");
migrate_add_blocker(s->migration_blocker);
}
--
2.1.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages
2015-03-19 15:43 ` [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages Alberto Garcia
@ 2015-03-19 19:37 ` Max Reitz
2015-03-20 7:52 ` Markus Armbruster
1 sibling, 0 replies; 19+ messages in thread
From: Max Reitz @ 2015-03-19 19:37 UTC (permalink / raw)
To: Alberto Garcia, qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi
On 2015-03-19 at 11:43, Alberto Garcia wrote:
> There are several error messages that identify a BlockDriverState by
> its device name. However those errors can be produced in nodes that
> don't have a device name associated.
>
> In those cases we should use bdrv_get_device_or_node_name() to fall
> back to the node name and produce a more meaningful message.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
> block.c | 13 +++++++------
> block/qcow.c | 4 ++--
> block/qcow2.c | 2 +-
> block/qed.c | 2 +-
> block/vdi.c | 2 +-
> block/vhdx.c | 2 +-
> block/vmdk.c | 4 ++--
> block/vpc.c | 2 +-
> block/vvfat.c | 3 ++-
> 9 files changed, 18 insertions(+), 16 deletions(-)
Well, it may pose a problem that the error messages often state "Device
'%s'", but with this change, it's not always a device, but sometimes
just a node. Maybe it would be better to replace the "Device" in the
error messages by "Node" (a node can be specified both by the node name
and the device name, whereas a device can only be referenced by its
device name).
Apart from this, the idea of this change looks good, though, as do the
changes done by this patch.
Max
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages
2015-03-19 15:43 ` [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages Alberto Garcia
2015-03-19 19:37 ` Max Reitz
@ 2015-03-20 7:52 ` Markus Armbruster
1 sibling, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-03-20 7:52 UTC (permalink / raw)
To: Alberto Garcia; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
Alberto Garcia <berto@igalia.com> writes:
> There are several error messages that identify a BlockDriverState by
> its device name. However those errors can be produced in nodes that
> don't have a device name associated.
>
> In those cases we should use bdrv_get_device_or_node_name() to fall
> back to the node name and produce a more meaningful message.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
> block.c | 13 +++++++------
> block/qcow.c | 4 ++--
> block/qcow2.c | 2 +-
> block/qed.c | 2 +-
> block/vdi.c | 2 +-
> block/vhdx.c | 2 +-
> block/vmdk.c | 4 ++--
> block/vpc.c | 2 +-
> block/vvfat.c | 3 ++-
> 9 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/block.c b/block.c
> index af284e3..655d9aa 100644
> --- a/block.c
> +++ b/block.c
> @@ -1225,7 +1225,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
> } else if (backing_hd) {
> error_setg(&bs->backing_blocker,
> "device is used as backing hd of '%s'",
> - bdrv_get_device_name(bs));
> + bdrv_get_device_or_node_name(bs));
> }
>
> bs->backing_hd = backing_hd;
Before:
* if @bs belongs to a BB with name N, print "backing hd of 'N'"
* else print "backing hd of ''" (not nice, but works)
After:
* if @bs belongs to a BB with name N, print "backing hd of 'N'" (no change)
* else if @bs has a node name NN, print "backing hd of 'NN'" (improvement)
* else print a null pointer, which crashes on some systems (bug)
If you want your bdrv_get_device_or_node_name() serve as drop-in
replacement for bdrv_get_device_name(), it must not return null.
In my opinion, such a drop-in replacement can only be a stop gap. We
need to review every error message and figure out how to update it for
node names. We should've done it back when we added node names.
Your misuse of bdrv_get_device_or_node_name() shows misuse is likely.
As long as that's the case, it needs a function comment.
[...]
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages
2015-03-20 10:19 [Qemu-devel] [PATCH v2 0/3] Add bdrv_get_device_or_node_name() Alberto Garcia
@ 2015-03-20 10:19 ` Alberto Garcia
2015-03-20 13:22 ` Max Reitz
0 siblings, 1 reply; 19+ messages in thread
From: Alberto Garcia @ 2015-03-20 10:19 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Alberto Garcia, Markus Armbruster, Max Reitz,
Stefan Hajnoczi
There are several error messages that identify a BlockDriverState by
its device name. However those errors can be produced in nodes that
don't have a device name associated.
In those cases we should use bdrv_get_device_or_node_name() to fall
back to the node name and produce a more meaningful message. The
messages are also updated to use the more generic term 'node' instead
of 'device'.
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
block.c | 21 +++++++++++----------
block/qcow.c | 4 ++--
block/qcow2.c | 2 +-
block/qed.c | 2 +-
block/vdi.c | 2 +-
block/vhdx.c | 2 +-
block/vmdk.c | 4 ++--
block/vpc.c | 2 +-
block/vvfat.c | 3 ++-
blockdev.c | 4 ++--
include/qapi/qmp/qerror.h | 8 ++++----
11 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/block.c b/block.c
index 98b90b4..8247f0a 100644
--- a/block.c
+++ b/block.c
@@ -1224,8 +1224,8 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker);
} else if (backing_hd) {
error_setg(&bs->backing_blocker,
- "device is used as backing hd of '%s'",
- bdrv_get_device_name(bs));
+ "node is used as backing hd of '%s'",
+ bdrv_get_device_or_node_name(bs));
}
bs->backing_hd = backing_hd;
@@ -1812,8 +1812,8 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
* to r/w */
if (!(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR) &&
reopen_state->flags & BDRV_O_RDWR) {
- error_set(errp, QERR_DEVICE_IS_READ_ONLY,
- bdrv_get_device_name(reopen_state->bs));
+ error_set(errp, QERR_NODE_IS_READ_ONLY,
+ bdrv_get_device_or_node_name(reopen_state->bs));
goto error;
}
@@ -1840,7 +1840,8 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
/* It is currently mandatory to have a bdrv_reopen_prepare()
* handler for each supported drv. */
error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- drv->format_name, bdrv_get_device_name(reopen_state->bs),
+ drv->format_name,
+ bdrv_get_device_or_node_name(reopen_state->bs),
"reopening of file");
ret = -1;
goto error;
@@ -3764,8 +3765,8 @@ void bdrv_add_key(BlockDriverState *bs, const char *key, Error **errp)
{
if (key) {
if (!bdrv_is_encrypted(bs)) {
- error_setg(errp, "Device '%s' is not encrypted",
- bdrv_get_device_name(bs));
+ error_setg(errp, "Node '%s' is not encrypted",
+ bdrv_get_device_or_node_name(bs));
} else if (bdrv_set_key(bs, key) < 0) {
error_set(errp, QERR_INVALID_PASSWORD);
}
@@ -3773,7 +3774,7 @@ void bdrv_add_key(BlockDriverState *bs, const char *key, Error **errp)
if (bdrv_key_required(bs)) {
error_set(errp, ERROR_CLASS_DEVICE_ENCRYPTED,
"'%s' (%s) is encrypted",
- bdrv_get_device_name(bs),
+ bdrv_get_device_or_node_name(bs),
bdrv_get_encrypted_filename(bs));
}
}
@@ -5548,8 +5549,8 @@ bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp)
if (!QLIST_EMPTY(&bs->op_blockers[op])) {
blocker = QLIST_FIRST(&bs->op_blockers[op]);
if (errp) {
- error_setg(errp, "Device '%s' is busy: %s",
- bdrv_get_device_name(bs),
+ error_setg(errp, "Node '%s' is busy: %s",
+ bdrv_get_device_or_node_name(bs),
error_get_pretty(blocker->reason));
}
return true;
diff --git a/block/qcow.c b/block/qcow.c
index 0558969..d4287ca 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -124,7 +124,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
snprintf(version, sizeof(version), "QCOW version %" PRIu32,
header.version);
error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- bdrv_get_device_name(bs), "qcow", version);
+ bdrv_get_device_or_node_name(bs), "qcow", version);
ret = -ENOTSUP;
goto fail;
}
@@ -231,7 +231,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
/* Disable migration when qcow images are used */
error_set(&s->migration_blocker,
QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "qcow", bdrv_get_device_name(bs), "live migration");
+ "qcow", bdrv_get_device_or_node_name(bs), "live migration");
migrate_add_blocker(s->migration_blocker);
qemu_co_mutex_init(&s->lock);
diff --git a/block/qcow2.c b/block/qcow2.c
index 32bdf75..168006b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -207,7 +207,7 @@ static void GCC_FMT_ATTR(3, 4) report_unsupported(BlockDriverState *bs,
va_end(ap);
error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- bdrv_get_device_name(bs), "qcow2", msg);
+ bdrv_get_device_or_node_name(bs), "qcow2", msg);
}
static void report_unsupported_feature(BlockDriverState *bs,
diff --git a/block/qed.c b/block/qed.c
index 892b13c..7d32ce4 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -408,7 +408,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,
snprintf(buf, sizeof(buf), "%" PRIx64,
s->header.features & ~QED_FEATURE_MASK);
error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- bdrv_get_device_name(bs), "QED", buf);
+ bdrv_get_device_or_node_name(bs), "QED", buf);
return -ENOTSUP;
}
if (!qed_is_cluster_size_valid(s->header.cluster_size)) {
diff --git a/block/vdi.c b/block/vdi.c
index 53bd02f..f0dc364 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -504,7 +504,7 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
/* Disable migration when vdi images are used */
error_set(&s->migration_blocker,
QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vdi", bdrv_get_device_name(bs), "live migration");
+ "vdi", bdrv_get_device_or_node_name(bs), "live migration");
migrate_add_blocker(s->migration_blocker);
qemu_co_mutex_init(&s->write_lock);
diff --git a/block/vhdx.c b/block/vhdx.c
index bb3ed45..d5dfe00 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1004,7 +1004,7 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
/* Disable migration when VHDX images are used */
error_set(&s->migration_blocker,
QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vhdx", bdrv_get_device_name(bs), "live migration");
+ "vhdx", bdrv_get_device_or_node_name(bs), "live migration");
migrate_add_blocker(s->migration_blocker);
return 0;
diff --git a/block/vmdk.c b/block/vmdk.c
index 8410a15..0230070 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -669,7 +669,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
snprintf(buf, sizeof(buf), "VMDK version %" PRId32,
le32_to_cpu(header.version));
error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- bdrv_get_device_name(bs), "vmdk", buf);
+ bdrv_get_device_or_node_name(bs), "vmdk", buf);
return -ENOTSUP;
} else if (le32_to_cpu(header.version) == 3 && (flags & BDRV_O_RDWR)) {
/* VMware KB 2064959 explains that version 3 added support for
@@ -964,7 +964,7 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
/* Disable migration when VMDK images are used */
error_set(&s->migration_blocker,
QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vmdk", bdrv_get_device_name(bs), "live migration");
+ "vmdk", bdrv_get_device_or_node_name(bs), "live migration");
migrate_add_blocker(s->migration_blocker);
g_free(buf);
return 0;
diff --git a/block/vpc.c b/block/vpc.c
index 43e768e..fd418b9 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -320,7 +320,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
/* Disable migration when VHD images are used */
error_set(&s->migration_blocker,
QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vpc", bdrv_get_device_name(bs), "live migration");
+ "vpc", bdrv_get_device_or_node_name(bs), "live migration");
migrate_add_blocker(s->migration_blocker);
return 0;
diff --git a/block/vvfat.c b/block/vvfat.c
index 9be632f..9ed3291 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1182,7 +1182,8 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
if (s->qcow) {
error_set(&s->migration_blocker,
QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vvfat (rw)", bdrv_get_device_name(bs), "live migration");
+ "vvfat (rw)", bdrv_get_device_or_node_name(bs),
+ "live migration");
migrate_add_blocker(s->migration_blocker);
}
diff --git a/blockdev.c b/blockdev.c
index 0b50985..f83b975 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1248,7 +1248,7 @@ static void internal_snapshot_prepare(BlkTransactionState *common,
}
if (bdrv_is_read_only(bs)) {
- error_set(errp, QERR_DEVICE_IS_READ_ONLY, device);
+ error_set(errp, QERR_NODE_IS_READ_ONLY, device);
return;
}
@@ -2055,7 +2055,7 @@ void qmp_block_resize(bool has_device, const char *device,
error_set(errp, QERR_UNSUPPORTED);
break;
case -EACCES:
- error_set(errp, QERR_DEVICE_IS_READ_ONLY, device);
+ error_set(errp, QERR_NODE_IS_READ_ONLY, device);
break;
case -EBUSY:
error_set(errp, QERR_DEVICE_IN_USE, device);
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 57a62d4..46cad14 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -38,7 +38,7 @@ void qerror_report_err(Error *err);
ERROR_CLASS_GENERIC_ERROR, "Base '%s' not found"
#define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
- ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by device '%s' does not support feature '%s'"
+ ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by node '%s' does not support feature '%s'"
#define QERR_BLOCK_JOB_NOT_READY \
ERROR_CLASS_GENERIC_ERROR, "The active block job for device '%s' cannot be completed"
@@ -58,9 +58,6 @@ void qerror_report_err(Error *err);
#define QERR_DEVICE_IN_USE \
ERROR_CLASS_GENERIC_ERROR, "Device '%s' is in use"
-#define QERR_DEVICE_IS_READ_ONLY \
- ERROR_CLASS_GENERIC_ERROR, "Device '%s' is read only"
-
#define QERR_DEVICE_NO_HOTPLUG \
ERROR_CLASS_GENERIC_ERROR, "Device '%s' does not support hotplugging"
@@ -103,6 +100,9 @@ void qerror_report_err(Error *err);
#define QERR_MISSING_PARAMETER \
ERROR_CLASS_GENERIC_ERROR, "Parameter '%s' is missing"
+#define QERR_NODE_IS_READ_ONLY \
+ ERROR_CLASS_GENERIC_ERROR, "Node '%s' is read only"
+
#define QERR_PERMISSION_DENIED \
ERROR_CLASS_GENERIC_ERROR, "Insufficient permission to perform this operation"
--
2.1.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages
2015-03-20 10:19 ` [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages Alberto Garcia
@ 2015-03-20 13:22 ` Max Reitz
2015-03-20 13:57 ` Markus Armbruster
0 siblings, 1 reply; 19+ messages in thread
From: Max Reitz @ 2015-03-20 13:22 UTC (permalink / raw)
To: Alberto Garcia, qemu-devel; +Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi
On 2015-03-20 at 06:19, Alberto Garcia wrote:
> There are several error messages that identify a BlockDriverState by
> its device name. However those errors can be produced in nodes that
> don't have a device name associated.
>
> In those cases we should use bdrv_get_device_or_node_name() to fall
> back to the node name and produce a more meaningful message. The
> messages are also updated to use the more generic term 'node' instead
> of 'device'.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
> block.c | 21 +++++++++++----------
> block/qcow.c | 4 ++--
> block/qcow2.c | 2 +-
> block/qed.c | 2 +-
> block/vdi.c | 2 +-
> block/vhdx.c | 2 +-
> block/vmdk.c | 4 ++--
> block/vpc.c | 2 +-
> block/vvfat.c | 3 ++-
> blockdev.c | 4 ++--
> include/qapi/qmp/qerror.h | 8 ++++----
> 11 files changed, 28 insertions(+), 26 deletions(-)
>
> diff --git a/block.c b/block.c
> index 98b90b4..8247f0a 100644
> --- a/block.c
> +++ b/block.c
> @@ -1224,8 +1224,8 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
> bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker);
> } else if (backing_hd) {
> error_setg(&bs->backing_blocker,
> - "device is used as backing hd of '%s'",
> - bdrv_get_device_name(bs));
> + "node is used as backing hd of '%s'",
> + bdrv_get_device_or_node_name(bs));
Actually, the change of "device" to "node" here is independent of the
use of bdrv_get_device_or_node_name(). But it's correct anyway.
> }
>
> bs->backing_hd = backing_hd;
> @@ -1812,8 +1812,8 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
> * to r/w */
> if (!(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR) &&
> reopen_state->flags & BDRV_O_RDWR) {
> - error_set(errp, QERR_DEVICE_IS_READ_ONLY,
> - bdrv_get_device_name(reopen_state->bs));
> + error_set(errp, QERR_NODE_IS_READ_ONLY,
> + bdrv_get_device_or_node_name(reopen_state->bs));
I think Markus would be happier with just removing the macro.
(Make it error_setg(errp, "Node '%s' is read only",
bdrv_get_device_or_node_name(reopen_state->bs)) and remove the
QERR_DEVICE_IS_READ_ONLY macro from qerror.h)
[snip]
> diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
> index 57a62d4..46cad14 100644
> --- a/include/qapi/qmp/qerror.h
> +++ b/include/qapi/qmp/qerror.h
> @@ -38,7 +38,7 @@ void qerror_report_err(Error *err);
> ERROR_CLASS_GENERIC_ERROR, "Base '%s' not found"
>
> #define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
> - ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by device '%s' does not support feature '%s'"
> + ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by node '%s' does not support feature '%s'"
Preexisting, but first: This line has over 80 characters.
Second: This seems like a good opportunity to drop this macro and
replace its occurrences by error_setg(errp, "Block format...") and the like.
> #define QERR_BLOCK_JOB_NOT_READY \
> ERROR_CLASS_GENERIC_ERROR, "The active block job for device '%s' cannot be completed"
> @@ -58,9 +58,6 @@ void qerror_report_err(Error *err);
> #define QERR_DEVICE_IN_USE \
> ERROR_CLASS_GENERIC_ERROR, "Device '%s' is in use"
>
> -#define QERR_DEVICE_IS_READ_ONLY \
> - ERROR_CLASS_GENERIC_ERROR, "Device '%s' is read only"
> -
> #define QERR_DEVICE_NO_HOTPLUG \
> ERROR_CLASS_GENERIC_ERROR, "Device '%s' does not support hotplugging"
>
> @@ -103,6 +100,9 @@ void qerror_report_err(Error *err);
> #define QERR_MISSING_PARAMETER \
> ERROR_CLASS_GENERIC_ERROR, "Parameter '%s' is missing"
>
> +#define QERR_NODE_IS_READ_ONLY \
> + ERROR_CLASS_GENERIC_ERROR, "Node '%s' is read only"
> +
> #define QERR_PERMISSION_DENIED \
> ERROR_CLASS_GENERIC_ERROR, "Insufficient permission to perform this operation"
As said above, the same goes for this macro.
I'm not so ardent about this, so if you want to keep the macros, it's
fine with me; but the overly long line need to be fixed (only the one
you're touching, not necessarily the rest, because I guess they'll be
dropped in the future anyway).
Markus?
Max
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages
2015-03-20 13:22 ` Max Reitz
@ 2015-03-20 13:57 ` Markus Armbruster
0 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-03-20 13:57 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, Alberto Garcia, qemu-devel, Stefan Hajnoczi
Max Reitz <mreitz@redhat.com> writes:
> On 2015-03-20 at 06:19, Alberto Garcia wrote:
>> There are several error messages that identify a BlockDriverState by
>> its device name. However those errors can be produced in nodes that
>> don't have a device name associated.
>>
>> In those cases we should use bdrv_get_device_or_node_name() to fall
>> back to the node name and produce a more meaningful message. The
>> messages are also updated to use the more generic term 'node' instead
>> of 'device'.
>>
>> Signed-off-by: Alberto Garcia <berto@igalia.com>
>> ---
>> block.c | 21 +++++++++++----------
>> block/qcow.c | 4 ++--
>> block/qcow2.c | 2 +-
>> block/qed.c | 2 +-
>> block/vdi.c | 2 +-
>> block/vhdx.c | 2 +-
>> block/vmdk.c | 4 ++--
>> block/vpc.c | 2 +-
>> block/vvfat.c | 3 ++-
>> blockdev.c | 4 ++--
>> include/qapi/qmp/qerror.h | 8 ++++----
>> 11 files changed, 28 insertions(+), 26 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 98b90b4..8247f0a 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -1224,8 +1224,8 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
>> bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker);
>> } else if (backing_hd) {
>> error_setg(&bs->backing_blocker,
>> - "device is used as backing hd of '%s'",
>> - bdrv_get_device_name(bs));
>> + "node is used as backing hd of '%s'",
>> + bdrv_get_device_or_node_name(bs));
>
> Actually, the change of "device" to "node" here is independent of the
> use of bdrv_get_device_or_node_name(). But it's correct anyway.
>
>> }
>> bs->backing_hd = backing_hd;
>> @@ -1812,8 +1812,8 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
>> * to r/w */
>> if (!(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR) &&
>> reopen_state->flags & BDRV_O_RDWR) {
>> - error_set(errp, QERR_DEVICE_IS_READ_ONLY,
>> - bdrv_get_device_name(reopen_state->bs));
>> + error_set(errp, QERR_NODE_IS_READ_ONLY,
>> + bdrv_get_device_or_node_name(reopen_state->bs));
>
> I think Markus would be happier with just removing the macro.
>
> (Make it error_setg(errp, "Node '%s' is read only",
> bdrv_get_device_or_node_name(reopen_state->bs)) and remove the
> QERR_DEVICE_IS_READ_ONLY macro from qerror.h)
>
> [snip]
>
>> diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
>> index 57a62d4..46cad14 100644
>> --- a/include/qapi/qmp/qerror.h
>> +++ b/include/qapi/qmp/qerror.h
>> @@ -38,7 +38,7 @@ void qerror_report_err(Error *err);
>> ERROR_CLASS_GENERIC_ERROR, "Base '%s' not found"
>> #define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
>> - ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by device '%s' does not support feature '%s'"
>> + ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by node '%s' does not support feature '%s'"
>
> Preexisting, but first: This line has over 80 characters.
>
> Second: This seems like a good opportunity to drop this macro and
> replace its occurrences by error_setg(errp, "Block format...") and the
> like.
>
>> #define QERR_BLOCK_JOB_NOT_READY \
>> ERROR_CLASS_GENERIC_ERROR, "The active block job for device '%s' cannot be completed"
>> @@ -58,9 +58,6 @@ void qerror_report_err(Error *err);
>> #define QERR_DEVICE_IN_USE \
>> ERROR_CLASS_GENERIC_ERROR, "Device '%s' is in use"
>> -#define QERR_DEVICE_IS_READ_ONLY \
>> - ERROR_CLASS_GENERIC_ERROR, "Device '%s' is read only"
>> -
>> #define QERR_DEVICE_NO_HOTPLUG \
>> ERROR_CLASS_GENERIC_ERROR, "Device '%s' does not support hotplugging"
>> @@ -103,6 +100,9 @@ void qerror_report_err(Error *err);
>> #define QERR_MISSING_PARAMETER \
>> ERROR_CLASS_GENERIC_ERROR, "Parameter '%s' is missing"
>> +#define QERR_NODE_IS_READ_ONLY \
>> + ERROR_CLASS_GENERIC_ERROR, "Node '%s' is read only"
>> +
>> #define QERR_PERMISSION_DENIED \
>> ERROR_CLASS_GENERIC_ERROR, "Insufficient permission to perform this operation"
>
> As said above, the same goes for this macro.
>
> I'm not so ardent about this, so if you want to keep the macros, it's
> fine with me; but the overly long line need to be fixed (only the one
> you're touching, not necessarily the rest, because I guess they'll be
> dropped in the future anyway).
>
> Markus?
I've been chipping away at qerror.h for a while. I think I can empty it
out in the next development cycle. So every new QERR_ macro you add
I'll have to replace again. Using literal strings instead will save me
the trouble.
If you find yourself using the same error string in many places, chances
are that some of these places do the same thing. Consider factoring
out.
Other than that, I wouldn't worry about duplicating error strings. Drop
in the bucket.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages
2015-03-20 14:33 [Qemu-devel] [PATCH v3 " Alberto Garcia
@ 2015-03-20 14:33 ` Alberto Garcia
2015-03-20 19:24 ` Max Reitz
0 siblings, 1 reply; 19+ messages in thread
From: Alberto Garcia @ 2015-03-20 14:33 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Alberto Garcia, Markus Armbruster, Max Reitz,
Stefan Hajnoczi
There are several error messages that identify a BlockDriverState by
its device name. However those errors can be produced in nodes that
don't have a device name associated.
In those cases we should use bdrv_get_device_or_node_name() to fall
back to the node name and produce a more meaningful message. The
messages are also updated to use the more generic term 'node' instead
of 'device'.
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
block.c | 24 ++++++++++++------------
block/qcow.c | 8 ++++----
block/qcow2.c | 2 +-
block/qed.c | 2 +-
block/snapshot.c | 12 ++++++------
block/vdi.c | 6 +++---
block/vhdx.c | 6 +++---
block/vmdk.c | 8 ++++----
block/vpc.c | 6 +++---
block/vvfat.c | 7 ++++---
blockdev.c | 9 +++++----
include/qapi/qmp/qerror.h | 6 ------
12 files changed, 46 insertions(+), 50 deletions(-)
diff --git a/block.c b/block.c
index 98b90b4..737ab68 100644
--- a/block.c
+++ b/block.c
@@ -1224,8 +1224,8 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker);
} else if (backing_hd) {
error_setg(&bs->backing_blocker,
- "device is used as backing hd of '%s'",
- bdrv_get_device_name(bs));
+ "node is used as backing hd of '%s'",
+ bdrv_get_device_or_node_name(bs));
}
bs->backing_hd = backing_hd;
@@ -1812,8 +1812,8 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
* to r/w */
if (!(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR) &&
reopen_state->flags & BDRV_O_RDWR) {
- error_set(errp, QERR_DEVICE_IS_READ_ONLY,
- bdrv_get_device_name(reopen_state->bs));
+ error_setg(errp, "Node '%s' is read only",
+ bdrv_get_device_or_node_name(reopen_state->bs));
goto error;
}
@@ -1839,9 +1839,9 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
} else {
/* It is currently mandatory to have a bdrv_reopen_prepare()
* handler for each supported drv. */
- error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- drv->format_name, bdrv_get_device_name(reopen_state->bs),
- "reopening of file");
+ error_setg(errp, "Block format '%s' used by node '%s' "
+ "does not support reopening files", drv->format_name,
+ bdrv_get_device_or_node_name(reopen_state->bs));
ret = -1;
goto error;
}
@@ -3764,8 +3764,8 @@ void bdrv_add_key(BlockDriverState *bs, const char *key, Error **errp)
{
if (key) {
if (!bdrv_is_encrypted(bs)) {
- error_setg(errp, "Device '%s' is not encrypted",
- bdrv_get_device_name(bs));
+ error_setg(errp, "Node '%s' is not encrypted",
+ bdrv_get_device_or_node_name(bs));
} else if (bdrv_set_key(bs, key) < 0) {
error_set(errp, QERR_INVALID_PASSWORD);
}
@@ -3773,7 +3773,7 @@ void bdrv_add_key(BlockDriverState *bs, const char *key, Error **errp)
if (bdrv_key_required(bs)) {
error_set(errp, ERROR_CLASS_DEVICE_ENCRYPTED,
"'%s' (%s) is encrypted",
- bdrv_get_device_name(bs),
+ bdrv_get_device_or_node_name(bs),
bdrv_get_encrypted_filename(bs));
}
}
@@ -5548,8 +5548,8 @@ bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp)
if (!QLIST_EMPTY(&bs->op_blockers[op])) {
blocker = QLIST_FIRST(&bs->op_blockers[op]);
if (errp) {
- error_setg(errp, "Device '%s' is busy: %s",
- bdrv_get_device_name(bs),
+ error_setg(errp, "Node '%s' is busy: %s",
+ bdrv_get_device_or_node_name(bs),
error_get_pretty(blocker->reason));
}
return true;
diff --git a/block/qcow.c b/block/qcow.c
index 0558969..ab89328 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -124,7 +124,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
snprintf(version, sizeof(version), "QCOW version %" PRIu32,
header.version);
error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- bdrv_get_device_name(bs), "qcow", version);
+ bdrv_get_device_or_node_name(bs), "qcow", version);
ret = -ENOTSUP;
goto fail;
}
@@ -229,9 +229,9 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
}
/* Disable migration when qcow images are used */
- error_set(&s->migration_blocker,
- QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "qcow", bdrv_get_device_name(bs), "live migration");
+ error_setg(&s->migration_blocker, "The qcow format used by node '%s' "
+ "does not support live migration",
+ bdrv_get_device_or_node_name(bs));
migrate_add_blocker(s->migration_blocker);
qemu_co_mutex_init(&s->lock);
diff --git a/block/qcow2.c b/block/qcow2.c
index 32bdf75..168006b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -207,7 +207,7 @@ static void GCC_FMT_ATTR(3, 4) report_unsupported(BlockDriverState *bs,
va_end(ap);
error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- bdrv_get_device_name(bs), "qcow2", msg);
+ bdrv_get_device_or_node_name(bs), "qcow2", msg);
}
static void report_unsupported_feature(BlockDriverState *bs,
diff --git a/block/qed.c b/block/qed.c
index 892b13c..7d32ce4 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -408,7 +408,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,
snprintf(buf, sizeof(buf), "%" PRIx64,
s->header.features & ~QED_FEATURE_MASK);
error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- bdrv_get_device_name(bs), "QED", buf);
+ bdrv_get_device_or_node_name(bs), "QED", buf);
return -ENOTSUP;
}
if (!qed_is_cluster_size_valid(s->header.cluster_size)) {
diff --git a/block/snapshot.c b/block/snapshot.c
index 698e1a1..50ae610 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -246,9 +246,9 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
if (bs->file) {
return bdrv_snapshot_delete(bs->file, snapshot_id, name, errp);
}
- error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- drv->format_name, bdrv_get_device_name(bs),
- "internal snapshot deletion");
+ error_setg(errp, "Block format '%s' used by device '%s' "
+ "does not support internal snapshot deletion",
+ drv->format_name, bdrv_get_device_name(bs));
return -ENOTSUP;
}
@@ -329,9 +329,9 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
if (drv->bdrv_snapshot_load_tmp) {
return drv->bdrv_snapshot_load_tmp(bs, snapshot_id, name, errp);
}
- error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- drv->format_name, bdrv_get_device_name(bs),
- "temporarily load internal snapshot");
+ error_setg(errp, "Block format '%s' used by device '%s' "
+ "does not support temporarily loading internal snapshots",
+ drv->format_name, bdrv_get_device_name(bs));
return -ENOTSUP;
}
diff --git a/block/vdi.c b/block/vdi.c
index 53bd02f..7642ef3 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -502,9 +502,9 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
}
/* Disable migration when vdi images are used */
- error_set(&s->migration_blocker,
- QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vdi", bdrv_get_device_name(bs), "live migration");
+ error_setg(&s->migration_blocker, "The vdi format used by node '%s' "
+ "does not support live migration",
+ bdrv_get_device_or_node_name(bs));
migrate_add_blocker(s->migration_blocker);
qemu_co_mutex_init(&s->write_lock);
diff --git a/block/vhdx.c b/block/vhdx.c
index bb3ed45..01970c2 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1002,9 +1002,9 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
/* TODO: differencing files */
/* Disable migration when VHDX images are used */
- error_set(&s->migration_blocker,
- QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vhdx", bdrv_get_device_name(bs), "live migration");
+ error_setg(&s->migration_blocker, "The vhdx format used by node '%s' "
+ "does not support live migration",
+ bdrv_get_device_or_node_name(bs));
migrate_add_blocker(s->migration_blocker);
return 0;
diff --git a/block/vmdk.c b/block/vmdk.c
index 8410a15..fd94b8f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -669,7 +669,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
snprintf(buf, sizeof(buf), "VMDK version %" PRId32,
le32_to_cpu(header.version));
error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- bdrv_get_device_name(bs), "vmdk", buf);
+ bdrv_get_device_or_node_name(bs), "vmdk", buf);
return -ENOTSUP;
} else if (le32_to_cpu(header.version) == 3 && (flags & BDRV_O_RDWR)) {
/* VMware KB 2064959 explains that version 3 added support for
@@ -962,9 +962,9 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
qemu_co_mutex_init(&s->lock);
/* Disable migration when VMDK images are used */
- error_set(&s->migration_blocker,
- QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vmdk", bdrv_get_device_name(bs), "live migration");
+ error_setg(&s->migration_blocker, "The vmdk format used by node '%s' "
+ "does not support live migration",
+ bdrv_get_device_or_node_name(bs));
migrate_add_blocker(s->migration_blocker);
g_free(buf);
return 0;
diff --git a/block/vpc.c b/block/vpc.c
index 43e768e..37572ba 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -318,9 +318,9 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
qemu_co_mutex_init(&s->lock);
/* Disable migration when VHD images are used */
- error_set(&s->migration_blocker,
- QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vpc", bdrv_get_device_name(bs), "live migration");
+ error_setg(&s->migration_blocker, "The vpc format used by node '%s' "
+ "does not support live migration",
+ bdrv_get_device_or_node_name(bs));
migrate_add_blocker(s->migration_blocker);
return 0;
diff --git a/block/vvfat.c b/block/vvfat.c
index 9be632f..e803589 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1180,9 +1180,10 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
/* Disable migration when vvfat is used rw */
if (s->qcow) {
- error_set(&s->migration_blocker,
- QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vvfat (rw)", bdrv_get_device_name(bs), "live migration");
+ error_setg(&s->migration_blocker,
+ "The vvfat (rw) format used by node '%s' "
+ "does not support live migration",
+ bdrv_get_device_or_node_name(bs));
migrate_add_blocker(s->migration_blocker);
}
diff --git a/blockdev.c b/blockdev.c
index fbb3a79..30dc9d2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1248,13 +1248,14 @@ static void internal_snapshot_prepare(BlkTransactionState *common,
}
if (bdrv_is_read_only(bs)) {
- error_set(errp, QERR_DEVICE_IS_READ_ONLY, device);
+ error_setg(errp, "Device '%s' is read only", device);
return;
}
if (!bdrv_can_snapshot(bs)) {
- error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- bs->drv->format_name, device, "internal snapshot");
+ error_setg(errp, "Block format '%s' used by device '%s' "
+ "does not support internal snapshots",
+ bs->drv->format_name, device);
return;
}
@@ -2055,7 +2056,7 @@ void qmp_block_resize(bool has_device, const char *device,
error_set(errp, QERR_UNSUPPORTED);
break;
case -EACCES:
- error_set(errp, QERR_DEVICE_IS_READ_ONLY, device);
+ error_setg(errp, "Device '%s' is read only", device);
break;
case -EBUSY:
error_set(errp, QERR_DEVICE_IN_USE, device);
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 57a62d4..e567339 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -37,9 +37,6 @@ void qerror_report_err(Error *err);
#define QERR_BASE_NOT_FOUND \
ERROR_CLASS_GENERIC_ERROR, "Base '%s' not found"
-#define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
- ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by device '%s' does not support feature '%s'"
-
#define QERR_BLOCK_JOB_NOT_READY \
ERROR_CLASS_GENERIC_ERROR, "The active block job for device '%s' cannot be completed"
@@ -58,9 +55,6 @@ void qerror_report_err(Error *err);
#define QERR_DEVICE_IN_USE \
ERROR_CLASS_GENERIC_ERROR, "Device '%s' is in use"
-#define QERR_DEVICE_IS_READ_ONLY \
- ERROR_CLASS_GENERIC_ERROR, "Device '%s' is read only"
-
#define QERR_DEVICE_NO_HOTPLUG \
ERROR_CLASS_GENERIC_ERROR, "Device '%s' does not support hotplugging"
--
2.1.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages
2015-03-20 14:33 ` [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages Alberto Garcia
@ 2015-03-20 19:24 ` Max Reitz
0 siblings, 0 replies; 19+ messages in thread
From: Max Reitz @ 2015-03-20 19:24 UTC (permalink / raw)
To: Alberto Garcia, qemu-devel; +Cc: Kevin Wolf, Markus Armbruster, Stefan Hajnoczi
On 2015-03-20 at 10:33, Alberto Garcia wrote:
> There are several error messages that identify a BlockDriverState by
> its device name. However those errors can be produced in nodes that
> don't have a device name associated.
>
> In those cases we should use bdrv_get_device_or_node_name() to fall
> back to the node name and produce a more meaningful message. The
> messages are also updated to use the more generic term 'node' instead
> of 'device'.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
> block.c | 24 ++++++++++++------------
> block/qcow.c | 8 ++++----
> block/qcow2.c | 2 +-
> block/qed.c | 2 +-
> block/snapshot.c | 12 ++++++------
> block/vdi.c | 6 +++---
> block/vhdx.c | 6 +++---
> block/vmdk.c | 8 ++++----
> block/vpc.c | 6 +++---
> block/vvfat.c | 7 ++++---
> blockdev.c | 9 +++++----
> include/qapi/qmp/qerror.h | 6 ------
> 12 files changed, 46 insertions(+), 50 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v4 0/3] Add bdrv_get_device_or_node_name()
@ 2015-04-08 9:29 Alberto Garcia
2015-04-08 9:29 ` [Qemu-devel] [PATCH 1/3] block: add bdrv_get_device_or_node_name() Alberto Garcia
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: Alberto Garcia @ 2015-04-08 9:29 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Alberto Garcia, qemu-block, Markus Armbruster,
Max Reitz, Stefan Hajnoczi
This series contains a couple of minor changes suggested by Markus in
the previous one. I removed the Reviewed-by line by Max in the
modified patch even if the changes are small, hope that's ok.
v4:
- Fix documentation of the 'node-name' field in BLOCK_IMAGE_CORRUPTED.
Its annotation is now (json-string, optional).
- Clarify that the 'device' field can be empty even if it's always
present for compatibility reasons.
v3:
- The node-name field in BLOCK_IMAGE_CORRUPTED is now Since: 2.4
- Remove the QERR_ macros instead of updating them. The text message
is adapted to each case where applicable, and 'device' is renamed to
'node' only where it makes sense.
v2:
- bdrv_get_device_or_node_name() includes a comment explaining its
usage.
- The error messages have been updated to say 'node' instead of
'device' where appropriate.
- The BLOCK_IMAGE_CORRUPTED event has a new 'node-name' field.
Regards,
Berto
Alberto Garcia (3):
block: add bdrv_get_device_or_node_name()
block: use bdrv_get_device_or_node_name() in error messages
block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED
block.c | 33 +++++++++++++++++++++------------
block/qcow.c | 8 ++++----
block/qcow2.c | 10 +++++++---
block/qed.c | 2 +-
block/quorum.c | 5 +----
block/snapshot.c | 12 ++++++------
block/vdi.c | 6 +++---
block/vhdx.c | 6 +++---
block/vmdk.c | 8 ++++----
block/vpc.c | 6 +++---
block/vvfat.c | 7 ++++---
blockdev.c | 9 +++++----
docs/qmp/qmp-events.txt | 21 +++++++++++++--------
include/block/block.h | 1 +
include/qapi/qmp/qerror.h | 6 ------
qapi/block-core.json | 17 +++++++++++------
16 files changed, 87 insertions(+), 70 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 1/3] block: add bdrv_get_device_or_node_name()
2015-04-08 9:29 [Qemu-devel] [PATCH v4 0/3] Add bdrv_get_device_or_node_name() Alberto Garcia
@ 2015-04-08 9:29 ` Alberto Garcia
2015-04-08 16:32 ` Eric Blake
2015-04-08 9:29 ` [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages Alberto Garcia
` (2 subsequent siblings)
3 siblings, 1 reply; 19+ messages in thread
From: Alberto Garcia @ 2015-04-08 9:29 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Alberto Garcia, qemu-block, Markus Armbruster,
Max Reitz, Stefan Hajnoczi
This function gets the device name associated with a BlockDriverState,
or its node name if the device name is empty.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block.c | 9 +++++++++
block/quorum.c | 5 +----
include/block/block.h | 1 +
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/block.c b/block.c
index f2f8ae7..24adfc1 100644
--- a/block.c
+++ b/block.c
@@ -3953,6 +3953,15 @@ const char *bdrv_get_device_name(const BlockDriverState *bs)
return bs->blk ? blk_name(bs->blk) : "";
}
+/* This can be used to identify nodes that might not have a device
+ * name associated. Since node and device names live in the same
+ * namespace, the result is unambiguous. The exception is if both are
+ * absent, then this returns an empty (non-null) string. */
+const char *bdrv_get_device_or_node_name(const BlockDriverState *bs)
+{
+ return bs->blk ? blk_name(bs->blk) : bs->node_name;
+}
+
int bdrv_get_flags(BlockDriverState *bs)
{
return bs->open_flags;
diff --git a/block/quorum.c b/block/quorum.c
index 437b122..f91ef75 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -226,10 +226,7 @@ static void quorum_report_bad(QuorumAIOCB *acb, char *node_name, int ret)
static void quorum_report_failure(QuorumAIOCB *acb)
{
- const char *reference = bdrv_get_device_name(acb->common.bs)[0] ?
- bdrv_get_device_name(acb->common.bs) :
- acb->common.bs->node_name;
-
+ const char *reference = bdrv_get_device_or_node_name(acb->common.bs);
qapi_event_send_quorum_failure(reference, acb->sector_num,
acb->nb_sectors, &error_abort);
}
diff --git a/include/block/block.h b/include/block/block.h
index 4c57d63..b285e0d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -398,6 +398,7 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
void *opaque);
const char *bdrv_get_node_name(const BlockDriverState *bs);
const char *bdrv_get_device_name(const BlockDriverState *bs);
+const char *bdrv_get_device_or_node_name(const BlockDriverState *bs);
int bdrv_get_flags(BlockDriverState *bs);
int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
const uint8_t *buf, int nb_sectors);
--
2.1.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages
2015-04-08 9:29 [Qemu-devel] [PATCH v4 0/3] Add bdrv_get_device_or_node_name() Alberto Garcia
2015-04-08 9:29 ` [Qemu-devel] [PATCH 1/3] block: add bdrv_get_device_or_node_name() Alberto Garcia
@ 2015-04-08 9:29 ` Alberto Garcia
2015-04-08 16:27 ` Eric Blake
2015-04-08 9:29 ` [Qemu-devel] [PATCH 3/3] block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED Alberto Garcia
2015-04-21 13:47 ` [Qemu-devel] [Qemu-block] [PATCH v4 0/3] Add bdrv_get_device_or_node_name() Stefan Hajnoczi
3 siblings, 1 reply; 19+ messages in thread
From: Alberto Garcia @ 2015-04-08 9:29 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Alberto Garcia, qemu-block, Markus Armbruster,
Max Reitz, Stefan Hajnoczi
There are several error messages that identify a BlockDriverState by
its device name. However those errors can be produced in nodes that
don't have a device name associated.
In those cases we should use bdrv_get_device_or_node_name() to fall
back to the node name and produce a more meaningful message. The
messages are also updated to use the more generic term 'node' instead
of 'device'.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block.c | 24 ++++++++++++------------
block/qcow.c | 8 ++++----
block/qcow2.c | 2 +-
block/qed.c | 2 +-
block/snapshot.c | 12 ++++++------
block/vdi.c | 6 +++---
block/vhdx.c | 6 +++---
block/vmdk.c | 8 ++++----
block/vpc.c | 6 +++---
block/vvfat.c | 7 ++++---
blockdev.c | 9 +++++----
include/qapi/qmp/qerror.h | 6 ------
12 files changed, 46 insertions(+), 50 deletions(-)
diff --git a/block.c b/block.c
index 24adfc1..25289ef 100644
--- a/block.c
+++ b/block.c
@@ -1224,8 +1224,8 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker);
} else if (backing_hd) {
error_setg(&bs->backing_blocker,
- "device is used as backing hd of '%s'",
- bdrv_get_device_name(bs));
+ "node is used as backing hd of '%s'",
+ bdrv_get_device_or_node_name(bs));
}
bs->backing_hd = backing_hd;
@@ -1812,8 +1812,8 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
* to r/w */
if (!(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR) &&
reopen_state->flags & BDRV_O_RDWR) {
- error_set(errp, QERR_DEVICE_IS_READ_ONLY,
- bdrv_get_device_name(reopen_state->bs));
+ error_setg(errp, "Node '%s' is read only",
+ bdrv_get_device_or_node_name(reopen_state->bs));
goto error;
}
@@ -1839,9 +1839,9 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
} else {
/* It is currently mandatory to have a bdrv_reopen_prepare()
* handler for each supported drv. */
- error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- drv->format_name, bdrv_get_device_name(reopen_state->bs),
- "reopening of file");
+ error_setg(errp, "Block format '%s' used by node '%s' "
+ "does not support reopening files", drv->format_name,
+ bdrv_get_device_or_node_name(reopen_state->bs));
ret = -1;
goto error;
}
@@ -3797,8 +3797,8 @@ void bdrv_add_key(BlockDriverState *bs, const char *key, Error **errp)
{
if (key) {
if (!bdrv_is_encrypted(bs)) {
- error_setg(errp, "Device '%s' is not encrypted",
- bdrv_get_device_name(bs));
+ error_setg(errp, "Node '%s' is not encrypted",
+ bdrv_get_device_or_node_name(bs));
} else if (bdrv_set_key(bs, key) < 0) {
error_set(errp, QERR_INVALID_PASSWORD);
}
@@ -3806,7 +3806,7 @@ void bdrv_add_key(BlockDriverState *bs, const char *key, Error **errp)
if (bdrv_key_required(bs)) {
error_set(errp, ERROR_CLASS_DEVICE_ENCRYPTED,
"'%s' (%s) is encrypted",
- bdrv_get_device_name(bs),
+ bdrv_get_device_or_node_name(bs),
bdrv_get_encrypted_filename(bs));
}
}
@@ -5581,8 +5581,8 @@ bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp)
if (!QLIST_EMPTY(&bs->op_blockers[op])) {
blocker = QLIST_FIRST(&bs->op_blockers[op]);
if (errp) {
- error_setg(errp, "Device '%s' is busy: %s",
- bdrv_get_device_name(bs),
+ error_setg(errp, "Node '%s' is busy: %s",
+ bdrv_get_device_or_node_name(bs),
error_get_pretty(blocker->reason));
}
return true;
diff --git a/block/qcow.c b/block/qcow.c
index 0558969..ab89328 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -124,7 +124,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
snprintf(version, sizeof(version), "QCOW version %" PRIu32,
header.version);
error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- bdrv_get_device_name(bs), "qcow", version);
+ bdrv_get_device_or_node_name(bs), "qcow", version);
ret = -ENOTSUP;
goto fail;
}
@@ -229,9 +229,9 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
}
/* Disable migration when qcow images are used */
- error_set(&s->migration_blocker,
- QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "qcow", bdrv_get_device_name(bs), "live migration");
+ error_setg(&s->migration_blocker, "The qcow format used by node '%s' "
+ "does not support live migration",
+ bdrv_get_device_or_node_name(bs));
migrate_add_blocker(s->migration_blocker);
qemu_co_mutex_init(&s->lock);
diff --git a/block/qcow2.c b/block/qcow2.c
index 32bdf75..168006b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -207,7 +207,7 @@ static void GCC_FMT_ATTR(3, 4) report_unsupported(BlockDriverState *bs,
va_end(ap);
error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- bdrv_get_device_name(bs), "qcow2", msg);
+ bdrv_get_device_or_node_name(bs), "qcow2", msg);
}
static void report_unsupported_feature(BlockDriverState *bs,
diff --git a/block/qed.c b/block/qed.c
index 892b13c..7d32ce4 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -408,7 +408,7 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,
snprintf(buf, sizeof(buf), "%" PRIx64,
s->header.features & ~QED_FEATURE_MASK);
error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- bdrv_get_device_name(bs), "QED", buf);
+ bdrv_get_device_or_node_name(bs), "QED", buf);
return -ENOTSUP;
}
if (!qed_is_cluster_size_valid(s->header.cluster_size)) {
diff --git a/block/snapshot.c b/block/snapshot.c
index 698e1a1..50ae610 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -246,9 +246,9 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
if (bs->file) {
return bdrv_snapshot_delete(bs->file, snapshot_id, name, errp);
}
- error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- drv->format_name, bdrv_get_device_name(bs),
- "internal snapshot deletion");
+ error_setg(errp, "Block format '%s' used by device '%s' "
+ "does not support internal snapshot deletion",
+ drv->format_name, bdrv_get_device_name(bs));
return -ENOTSUP;
}
@@ -329,9 +329,9 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
if (drv->bdrv_snapshot_load_tmp) {
return drv->bdrv_snapshot_load_tmp(bs, snapshot_id, name, errp);
}
- error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- drv->format_name, bdrv_get_device_name(bs),
- "temporarily load internal snapshot");
+ error_setg(errp, "Block format '%s' used by device '%s' "
+ "does not support temporarily loading internal snapshots",
+ drv->format_name, bdrv_get_device_name(bs));
return -ENOTSUP;
}
diff --git a/block/vdi.c b/block/vdi.c
index 53bd02f..7642ef3 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -502,9 +502,9 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
}
/* Disable migration when vdi images are used */
- error_set(&s->migration_blocker,
- QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vdi", bdrv_get_device_name(bs), "live migration");
+ error_setg(&s->migration_blocker, "The vdi format used by node '%s' "
+ "does not support live migration",
+ bdrv_get_device_or_node_name(bs));
migrate_add_blocker(s->migration_blocker);
qemu_co_mutex_init(&s->write_lock);
diff --git a/block/vhdx.c b/block/vhdx.c
index bb3ed45..01970c2 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1002,9 +1002,9 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
/* TODO: differencing files */
/* Disable migration when VHDX images are used */
- error_set(&s->migration_blocker,
- QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vhdx", bdrv_get_device_name(bs), "live migration");
+ error_setg(&s->migration_blocker, "The vhdx format used by node '%s' "
+ "does not support live migration",
+ bdrv_get_device_or_node_name(bs));
migrate_add_blocker(s->migration_blocker);
return 0;
diff --git a/block/vmdk.c b/block/vmdk.c
index 8410a15..fd94b8f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -669,7 +669,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
snprintf(buf, sizeof(buf), "VMDK version %" PRId32,
le32_to_cpu(header.version));
error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- bdrv_get_device_name(bs), "vmdk", buf);
+ bdrv_get_device_or_node_name(bs), "vmdk", buf);
return -ENOTSUP;
} else if (le32_to_cpu(header.version) == 3 && (flags & BDRV_O_RDWR)) {
/* VMware KB 2064959 explains that version 3 added support for
@@ -962,9 +962,9 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
qemu_co_mutex_init(&s->lock);
/* Disable migration when VMDK images are used */
- error_set(&s->migration_blocker,
- QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vmdk", bdrv_get_device_name(bs), "live migration");
+ error_setg(&s->migration_blocker, "The vmdk format used by node '%s' "
+ "does not support live migration",
+ bdrv_get_device_or_node_name(bs));
migrate_add_blocker(s->migration_blocker);
g_free(buf);
return 0;
diff --git a/block/vpc.c b/block/vpc.c
index 43e768e..37572ba 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -318,9 +318,9 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
qemu_co_mutex_init(&s->lock);
/* Disable migration when VHD images are used */
- error_set(&s->migration_blocker,
- QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vpc", bdrv_get_device_name(bs), "live migration");
+ error_setg(&s->migration_blocker, "The vpc format used by node '%s' "
+ "does not support live migration",
+ bdrv_get_device_or_node_name(bs));
migrate_add_blocker(s->migration_blocker);
return 0;
diff --git a/block/vvfat.c b/block/vvfat.c
index 9be632f..e803589 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1180,9 +1180,10 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
/* Disable migration when vvfat is used rw */
if (s->qcow) {
- error_set(&s->migration_blocker,
- QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "vvfat (rw)", bdrv_get_device_name(bs), "live migration");
+ error_setg(&s->migration_blocker,
+ "The vvfat (rw) format used by node '%s' "
+ "does not support live migration",
+ bdrv_get_device_or_node_name(bs));
migrate_add_blocker(s->migration_blocker);
}
diff --git a/blockdev.c b/blockdev.c
index fbb3a79..30dc9d2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1248,13 +1248,14 @@ static void internal_snapshot_prepare(BlkTransactionState *common,
}
if (bdrv_is_read_only(bs)) {
- error_set(errp, QERR_DEVICE_IS_READ_ONLY, device);
+ error_setg(errp, "Device '%s' is read only", device);
return;
}
if (!bdrv_can_snapshot(bs)) {
- error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- bs->drv->format_name, device, "internal snapshot");
+ error_setg(errp, "Block format '%s' used by device '%s' "
+ "does not support internal snapshots",
+ bs->drv->format_name, device);
return;
}
@@ -2055,7 +2056,7 @@ void qmp_block_resize(bool has_device, const char *device,
error_set(errp, QERR_UNSUPPORTED);
break;
case -EACCES:
- error_set(errp, QERR_DEVICE_IS_READ_ONLY, device);
+ error_setg(errp, "Device '%s' is read only", device);
break;
case -EBUSY:
error_set(errp, QERR_DEVICE_IN_USE, device);
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 57a62d4..e567339 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -37,9 +37,6 @@ void qerror_report_err(Error *err);
#define QERR_BASE_NOT_FOUND \
ERROR_CLASS_GENERIC_ERROR, "Base '%s' not found"
-#define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
- ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by device '%s' does not support feature '%s'"
-
#define QERR_BLOCK_JOB_NOT_READY \
ERROR_CLASS_GENERIC_ERROR, "The active block job for device '%s' cannot be completed"
@@ -58,9 +55,6 @@ void qerror_report_err(Error *err);
#define QERR_DEVICE_IN_USE \
ERROR_CLASS_GENERIC_ERROR, "Device '%s' is in use"
-#define QERR_DEVICE_IS_READ_ONLY \
- ERROR_CLASS_GENERIC_ERROR, "Device '%s' is read only"
-
#define QERR_DEVICE_NO_HOTPLUG \
ERROR_CLASS_GENERIC_ERROR, "Device '%s' does not support hotplugging"
--
2.1.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 3/3] block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED
2015-04-08 9:29 [Qemu-devel] [PATCH v4 0/3] Add bdrv_get_device_or_node_name() Alberto Garcia
2015-04-08 9:29 ` [Qemu-devel] [PATCH 1/3] block: add bdrv_get_device_or_node_name() Alberto Garcia
2015-04-08 9:29 ` [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages Alberto Garcia
@ 2015-04-08 9:29 ` Alberto Garcia
2015-04-08 23:32 ` Eric Blake
2015-04-15 13:24 ` Max Reitz
2015-04-21 13:47 ` [Qemu-devel] [Qemu-block] [PATCH v4 0/3] Add bdrv_get_device_or_node_name() Stefan Hajnoczi
3 siblings, 2 replies; 19+ messages in thread
From: Alberto Garcia @ 2015-04-08 9:29 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Alberto Garcia, qemu-block, Markus Armbruster,
Max Reitz, Stefan Hajnoczi
Since this event can occur in nodes that cannot have a device name
associated, include also a field with the node name.
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
block/qcow2.c | 8 ++++++--
docs/qmp/qmp-events.txt | 21 +++++++++++++--------
qapi/block-core.json | 17 +++++++++++------
3 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 168006b..e7c78f1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2809,6 +2809,7 @@ void qcow2_signal_corruption(BlockDriverState *bs, bool fatal, int64_t offset,
int64_t size, const char *message_format, ...)
{
BDRVQcowState *s = bs->opaque;
+ const char *node_name;
char *message;
va_list ap;
@@ -2832,8 +2833,11 @@ void qcow2_signal_corruption(BlockDriverState *bs, bool fatal, int64_t offset,
"corruption events will be suppressed\n", message);
}
- qapi_event_send_block_image_corrupted(bdrv_get_device_name(bs), message,
- offset >= 0, offset, size >= 0, size,
+ node_name = bdrv_get_node_name(bs);
+ qapi_event_send_block_image_corrupted(bdrv_get_device_name(bs),
+ *node_name != '\0', node_name,
+ message, offset >= 0, offset,
+ size >= 0, size,
fatal, &error_abort);
g_free(message);
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index d759d19..b19e490 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -31,21 +31,26 @@ Example:
BLOCK_IMAGE_CORRUPTED
---------------------
-Emitted when a disk image is being marked corrupt.
+Emitted when a disk image is being marked corrupt. The image can be
+identified by its device or node name. The 'device' field is always
+present for compatibility reasons, but it can be empty ("") if the
+image does not have a device name associated.
Data:
-- "device": Device name (json-string)
-- "msg": Informative message (e.g., reason for the corruption) (json-string)
-- "offset": If the corruption resulted from an image access, this is the access
- offset into the image (json-int)
-- "size": If the corruption resulted from an image access, this is the access
- size (json-int)
+- "device": Device name (json-string)
+- "node-name": Node name (json-string, optional)
+- "msg": Informative message (e.g., reason for the corruption)
+ (json-string)
+- "offset": If the corruption resulted from an image access, this
+ is the access offset into the image (json-int)
+- "size": If the corruption resulted from an image access, this
+ is the access size (json-int)
Example:
{ "event": "BLOCK_IMAGE_CORRUPTED",
- "data": { "device": "ide0-hd0",
+ "data": { "device": "ide0-hd0", "node-name": "node0",
"msg": "Prevented active L1 table overwrite", "offset": 196608,
"size": 65536 },
"timestamp": { "seconds": 1378126126, "microseconds": 966463 } }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7873084..21e6cb5 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1754,7 +1754,11 @@
#
# Emitted when a corruption has been detected in a disk image
#
-# @device: device name
+# @device: device name. This is always present for compatibility
+# reasons, but it can be empty ("") if the image does not
+# have a device name associated.
+#
+# @node-name: #optional node name (Since: 2.4)
#
# @msg: informative message for human consumption, such as the kind of
# corruption being detected. It should not be parsed by machine as it is
@@ -1773,11 +1777,12 @@
# Since: 1.7
##
{ 'event': 'BLOCK_IMAGE_CORRUPTED',
- 'data': { 'device' : 'str',
- 'msg' : 'str',
- '*offset': 'int',
- '*size' : 'int',
- 'fatal' : 'bool' } }
+ 'data': { 'device' : 'str',
+ '*node-name' : 'str',
+ 'msg' : 'str',
+ '*offset' : 'int',
+ '*size' : 'int',
+ 'fatal' : 'bool' } }
##
# @BLOCK_IO_ERROR
--
2.1.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages
2015-04-08 9:29 ` [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages Alberto Garcia
@ 2015-04-08 16:27 ` Eric Blake
2015-04-09 8:25 ` Alberto Garcia
0 siblings, 1 reply; 19+ messages in thread
From: Eric Blake @ 2015-04-08 16:27 UTC (permalink / raw)
To: Alberto Garcia, qemu-devel
Cc: Kevin Wolf, qemu-block, Markus Armbruster, Stefan Hajnoczi,
Max Reitz
[-- Attachment #1: Type: text/plain, Size: 2980 bytes --]
On 04/08/2015 03:29 AM, Alberto Garcia wrote:
> There are several error messages that identify a BlockDriverState by
> its device name. However those errors can be produced in nodes that
> don't have a device name associated.
>
> In those cases we should use bdrv_get_device_or_node_name() to fall
> back to the node name and produce a more meaningful message. The
> messages are also updated to use the more generic term 'node' instead
> of 'device'.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
> +++ b/block/snapshot.c
> @@ -246,9 +246,9 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
> if (bs->file) {
> return bdrv_snapshot_delete(bs->file, snapshot_id, name, errp);
> }
> - error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
> - drv->format_name, bdrv_get_device_name(bs),
> - "internal snapshot deletion");
> + error_setg(errp, "Block format '%s' used by device '%s' "
> + "does not support internal snapshot deletion",
> + drv->format_name, bdrv_get_device_name(bs));
> return -ENOTSUP;
> }
>
> @@ -329,9 +329,9 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
> if (drv->bdrv_snapshot_load_tmp) {
> return drv->bdrv_snapshot_load_tmp(bs, snapshot_id, name, errp);
> }
> - error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
> - drv->format_name, bdrv_get_device_name(bs),
> - "temporarily load internal snapshot");
> + error_setg(errp, "Block format '%s' used by device '%s' "
> + "does not support temporarily loading internal snapshots",
> + drv->format_name, bdrv_get_device_name(bs));
Should these two messages use 'node' instead of 'device'? After all, a
format is tied to a node (as a backing chain can involve multiple nodes
using different formats)
> +++ b/blockdev.c
> @@ -1248,13 +1248,14 @@ static void internal_snapshot_prepare(BlkTransactionState *common,
> }
>
> if (bdrv_is_read_only(bs)) {
> - error_set(errp, QERR_DEVICE_IS_READ_ONLY, device);
> + error_setg(errp, "Device '%s' is read only", device);
> return;
> }
This one is probably fine as device;
>
> if (!bdrv_can_snapshot(bs)) {
> - error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
> - bs->drv->format_name, device, "internal snapshot");
> + error_setg(errp, "Block format '%s' used by device '%s' "
> + "does not support internal snapshots",
> + bs->drv->format_name, device);
but this is probably another one where node may be better.
But it's already a strict improvement, so I can live with:
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] block: add bdrv_get_device_or_node_name()
2015-04-08 9:29 ` [Qemu-devel] [PATCH 1/3] block: add bdrv_get_device_or_node_name() Alberto Garcia
@ 2015-04-08 16:32 ` Eric Blake
0 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2015-04-08 16:32 UTC (permalink / raw)
To: Alberto Garcia, qemu-devel
Cc: Kevin Wolf, qemu-block, Markus Armbruster, Stefan Hajnoczi,
Max Reitz
[-- Attachment #1: Type: text/plain, Size: 1447 bytes --]
On 04/08/2015 03:29 AM, Alberto Garcia wrote:
> This function gets the device name associated with a BlockDriverState,
> or its node name if the device name is empty.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
> block.c | 9 +++++++++
> block/quorum.c | 5 +----
> include/block/block.h | 1 +
> 3 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/block.c b/block.c
> index f2f8ae7..24adfc1 100644
> --- a/block.c
> +++ b/block.c
> @@ -3953,6 +3953,15 @@ const char *bdrv_get_device_name(const BlockDriverState *bs)
> return bs->blk ? blk_name(bs->blk) : "";
> }
>
> +/* This can be used to identify nodes that might not have a device
> + * name associated. Since node and device names live in the same
> + * namespace, the result is unambiguous. The exception is if both are
> + * absent, then this returns an empty (non-null) string. */
> +const char *bdrv_get_device_or_node_name(const BlockDriverState *bs)
> +{
> + return bs->blk ? blk_name(bs->blk) : bs->node_name;
I had to check; bs->node_name is an array rather than a pointer, so it
always contains a non-null string (whether or not it is the empty
string), so your comment is correct.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED
2015-04-08 9:29 ` [Qemu-devel] [PATCH 3/3] block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED Alberto Garcia
@ 2015-04-08 23:32 ` Eric Blake
2015-04-09 7:30 ` Alberto Garcia
2015-04-15 13:24 ` Max Reitz
1 sibling, 1 reply; 19+ messages in thread
From: Eric Blake @ 2015-04-08 23:32 UTC (permalink / raw)
To: Alberto Garcia, qemu-devel
Cc: Kevin Wolf, qemu-block, Markus Armbruster, Stefan Hajnoczi,
Max Reitz
On 04/08/2015 03:29 AM, Alberto Garcia wrote:
> Since this event can occur in nodes that cannot have a device name
> associated, include also a field with the node name.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
> block/qcow2.c | 8 ++++++--
> docs/qmp/qmp-events.txt | 21 +++++++++++++--------
> qapi/block-core.json | 17 +++++++++++------
> 3 files changed, 30 insertions(+), 16 deletions(-)
>
>
> -- "device": Device name (json-string)
> -- "msg": Informative message (e.g., reason for the corruption) (json-string)
> -- "offset": If the corruption resulted from an image access, this is the access
> - offset into the image (json-int)
> -- "size": If the corruption resulted from an image access, this is the access
> - size (json-int)
> +- "device": Device name (json-string)
> +- "node-name": Node name (json-string, optional)
> +- "msg": Informative message (e.g., reason for the corruption)
> + (json-string)
> +- "offset": If the corruption resulted from an image access, this
> + is the access offset into the image (json-int)
> +- "size": If the corruption resulted from an image access, this
> + is the access size (json-int)
Not your fault (so don't worry about fixing it here), but I still find
this definition of 'offset' confusing - is it the guest's offset, or the
host's offset? I'm going to assume the host's offset (remember, on
qcow2, the guest offset 0 is never at host offset 0, because that is
reserved for the qcow2 header - but we CAN encounter a read error while
reading the qcow2 header).
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED
2015-04-08 23:32 ` Eric Blake
@ 2015-04-09 7:30 ` Alberto Garcia
0 siblings, 0 replies; 19+ messages in thread
From: Alberto Garcia @ 2015-04-09 7:30 UTC (permalink / raw)
To: Eric Blake
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
Stefan Hajnoczi
On Wed, Apr 08, 2015 at 05:32:47PM -0600, Eric Blake wrote:
> > +- "device": Device name (json-string)
> > +- "node-name": Node name (json-string, optional)
> > +- "msg": Informative message (e.g., reason for the corruption)
> > + (json-string)
> > +- "offset": If the corruption resulted from an image access, this
> > + is the access offset into the image (json-int)
> > +- "size": If the corruption resulted from an image access, this
> > + is the access size (json-int)
>
> Not your fault (so don't worry about fixing it here), but I still
> find this definition of 'offset' confusing - is it the guest's
> offset, or the host's offset?
From the code it looks like it's the host's offset. And now that it
look into it both 'offset' and 'size' should be marked as optional
there (they are in block-core.json). I'll send a separate patch
correcting all those.
Berto
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages
2015-04-08 16:27 ` Eric Blake
@ 2015-04-09 8:25 ` Alberto Garcia
0 siblings, 0 replies; 19+ messages in thread
From: Alberto Garcia @ 2015-04-09 8:25 UTC (permalink / raw)
To: Eric Blake
Cc: Kevin Wolf, qemu-block, Markus Armbruster, qemu-devel, Max Reitz,
Stefan Hajnoczi
On Wed, Apr 08, 2015 at 10:27:34AM -0600, Eric Blake wrote:
> > +++ b/block/snapshot.c
> > @@ -246,9 +246,9 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
> > if (bs->file) {
> > return bdrv_snapshot_delete(bs->file, snapshot_id, name, errp);
> > }
> > - error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
> > - drv->format_name, bdrv_get_device_name(bs),
> > - "internal snapshot deletion");
> > + error_setg(errp, "Block format '%s' used by device '%s' "
> > + "does not support internal snapshot deletion",
> > + drv->format_name, bdrv_get_device_name(bs));
> > return -ENOTSUP;
> > }
> >
> > @@ -329,9 +329,9 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
> > if (drv->bdrv_snapshot_load_tmp) {
> > return drv->bdrv_snapshot_load_tmp(bs, snapshot_id, name, errp);
> > }
> > - error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
> > - drv->format_name, bdrv_get_device_name(bs),
> > - "temporarily load internal snapshot");
> > + error_setg(errp, "Block format '%s' used by device '%s' "
> > + "does not support temporarily loading internal snapshots",
> > + drv->format_name, bdrv_get_device_name(bs));
>
> Should these two messages use 'node' instead of 'device'? After
> all, a format is tied to a node (as a backing chain can involve
> multiple nodes using different formats)
>
> > +++ b/blockdev.c
> > if (!bdrv_can_snapshot(bs)) {
> > - error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
> > - bs->drv->format_name, device, "internal snapshot");
> > + error_setg(errp, "Block format '%s' used by device '%s' "
> > + "does not support internal snapshots",
> > + bs->drv->format_name, device);
>
> but this is probably another one where node may be better.
I decided to leave 'device' in all cases where 'bs' cannot possibly be
anything else that a root node.
In this latter case, for example, that bs is obtained using
blk_bs(blk_by_name(device)).
Berto
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED
2015-04-08 9:29 ` [Qemu-devel] [PATCH 3/3] block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED Alberto Garcia
2015-04-08 23:32 ` Eric Blake
@ 2015-04-15 13:24 ` Max Reitz
1 sibling, 0 replies; 19+ messages in thread
From: Max Reitz @ 2015-04-15 13:24 UTC (permalink / raw)
To: Alberto Garcia, qemu-devel
Cc: Kevin Wolf, qemu-block, Markus Armbruster, Stefan Hajnoczi
On 08.04.2015 11:29, Alberto Garcia wrote:
> Since this event can occur in nodes that cannot have a device name
> associated, include also a field with the node name.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
> block/qcow2.c | 8 ++++++--
> docs/qmp/qmp-events.txt | 21 +++++++++++++--------
> qapi/block-core.json | 17 +++++++++++------
> 3 files changed, 30 insertions(+), 16 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH v4 0/3] Add bdrv_get_device_or_node_name()
2015-04-08 9:29 [Qemu-devel] [PATCH v4 0/3] Add bdrv_get_device_or_node_name() Alberto Garcia
` (2 preceding siblings ...)
2015-04-08 9:29 ` [Qemu-devel] [PATCH 3/3] block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED Alberto Garcia
@ 2015-04-21 13:47 ` Stefan Hajnoczi
3 siblings, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2015-04-21 13:47 UTC (permalink / raw)
To: Alberto Garcia
Cc: Max Reitz, Stefan Hajnoczi, qemu-devel, qemu-block,
Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 2164 bytes --]
On Wed, Apr 08, 2015 at 12:29:17PM +0300, Alberto Garcia wrote:
> This series contains a couple of minor changes suggested by Markus in
> the previous one. I removed the Reviewed-by line by Max in the
> modified patch even if the changes are small, hope that's ok.
>
> v4:
> - Fix documentation of the 'node-name' field in BLOCK_IMAGE_CORRUPTED.
> Its annotation is now (json-string, optional).
> - Clarify that the 'device' field can be empty even if it's always
> present for compatibility reasons.
>
> v3:
> - The node-name field in BLOCK_IMAGE_CORRUPTED is now Since: 2.4
> - Remove the QERR_ macros instead of updating them. The text message
> is adapted to each case where applicable, and 'device' is renamed to
> 'node' only where it makes sense.
>
> v2:
> - bdrv_get_device_or_node_name() includes a comment explaining its
> usage.
> - The error messages have been updated to say 'node' instead of
> 'device' where appropriate.
> - The BLOCK_IMAGE_CORRUPTED event has a new 'node-name' field.
>
> Regards,
>
> Berto
>
> Alberto Garcia (3):
> block: add bdrv_get_device_or_node_name()
> block: use bdrv_get_device_or_node_name() in error messages
> block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED
>
> block.c | 33 +++++++++++++++++++++------------
> block/qcow.c | 8 ++++----
> block/qcow2.c | 10 +++++++---
> block/qed.c | 2 +-
> block/quorum.c | 5 +----
> block/snapshot.c | 12 ++++++------
> block/vdi.c | 6 +++---
> block/vhdx.c | 6 +++---
> block/vmdk.c | 8 ++++----
> block/vpc.c | 6 +++---
> block/vvfat.c | 7 ++++---
> blockdev.c | 9 +++++----
> docs/qmp/qmp-events.txt | 21 +++++++++++++--------
> include/block/block.h | 1 +
> include/qapi/qmp/qerror.h | 6 ------
> qapi/block-core.json | 17 +++++++++++------
> 16 files changed, 87 insertions(+), 70 deletions(-)
Thanks, applied to my block-next tree:
https://github.com/stefanha/qemu/commits/block-next
Stefan
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2015-04-21 13:47 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-08 9:29 [Qemu-devel] [PATCH v4 0/3] Add bdrv_get_device_or_node_name() Alberto Garcia
2015-04-08 9:29 ` [Qemu-devel] [PATCH 1/3] block: add bdrv_get_device_or_node_name() Alberto Garcia
2015-04-08 16:32 ` Eric Blake
2015-04-08 9:29 ` [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages Alberto Garcia
2015-04-08 16:27 ` Eric Blake
2015-04-09 8:25 ` Alberto Garcia
2015-04-08 9:29 ` [Qemu-devel] [PATCH 3/3] block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED Alberto Garcia
2015-04-08 23:32 ` Eric Blake
2015-04-09 7:30 ` Alberto Garcia
2015-04-15 13:24 ` Max Reitz
2015-04-21 13:47 ` [Qemu-devel] [Qemu-block] [PATCH v4 0/3] Add bdrv_get_device_or_node_name() Stefan Hajnoczi
-- strict thread matches above, loose matches on Subject: below --
2015-03-20 14:33 [Qemu-devel] [PATCH v3 " Alberto Garcia
2015-03-20 14:33 ` [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages Alberto Garcia
2015-03-20 19:24 ` Max Reitz
2015-03-20 10:19 [Qemu-devel] [PATCH v2 0/3] Add bdrv_get_device_or_node_name() Alberto Garcia
2015-03-20 10:19 ` [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages Alberto Garcia
2015-03-20 13:22 ` Max Reitz
2015-03-20 13:57 ` Markus Armbruster
2015-03-19 15:43 [Qemu-devel] [PATCH 0/3] Add bdrv_get_device_or_node_name() Alberto Garcia
2015-03-19 15:43 ` [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages Alberto Garcia
2015-03-19 19:37 ` Max Reitz
2015-03-20 7:52 ` Markus Armbruster
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).