Add bdrv_get_device_or_node_name()
v4 cover Add bdrv_get_device_or_node_name()
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(-)
Signature · 1 line
2.1.4
PATCH v4 1/3 block: add bdrv_get_device_or_node_name()
This function gets the device name associated with a BlockDriverState, or its node name if the device name is empty.
--- block.c | 9 +++++++++ block/quorum.c | 5 +---- include/block/block.h | 1 + 3 files changed, 11 insertions(+), 4 deletions(-)
block.c +9 -0
@@ -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;
block/quorum.c +1 -4
@@ -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); }
include/block/block.h +1 -0
@@ -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);
Signature · 1 line
2.1.4
Re: [Qemu-devel] [PATCH 1/3] block: add bdrv_get_device_or_node_name()
Quoted 16 lines · Alberto Garcia
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 <[email protected]>Reviewed-by Max Reitz <[email protected]>--- block.c | 9 +++++++++ block/quorum.c | 5 +---- include/block/block.h | 1 + 3 files changed, 11 insertions(+), 4 deletions(-)block.c +0
-0
block.c const char *bdrv_get_device_name(const BlockDriverState *bs) · line 3962
Quoted 10 lines
block.c +7
-0@@ -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.
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
PATCH v4 2/3 block: use bdrv_get_device_or_node_name() in error messages
In reply to Alberto GarciaThere 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'.
--- 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(-)
block.c +12 -12
@@ -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;
block/qcow.c +4 -4
@@ -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);
block/qcow2.c +1 -1
@@ -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,
block/qed.c +1 -1
@@ -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)) {
block/snapshot.c +6 -6
@@ -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 ; }
block/vdi.c +3 -3
@@ -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);
block/vhdx.c +3 -3
@@ -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 ;
block/vmdk.c +4 -4
@@ -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 ;
block/vpc.c +3 -3
@@ -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 ;
block/vvfat.c +4 -3
@@ -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); }
blockdev.c +5 -4
@@ -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);
include/qapi/qmp/qerror.h +0 -6
@@ -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"
Signature · 1 line
2.1.4
Re: [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages
Quoted 28 lines · Alberto Garcia
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 <[email protected]>Reviewed-by Max Reitz <[email protected]>---
+++
b/block/snapshot.cblock/snapshot.c +3
-3@@ -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 ; }
block/snapshot.c int bdrv_snapshot_load_tmp(BlockDriverState *bs, · line 334
Quoted 9 lines
block/snapshot.c +3
-3@@ -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)
blockdev.c static void internal_snapshot_prepare(BlkTransactionState *common, · line 1253
Quoted 9 lines
blockdev.c +2
-1+++ 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;
blockdev.c static void internal_snapshot_prepare(BlkTransactionState *common, · line 1258
Quoted 7 lines
blockdev.c +3
-2if (!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:
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH 2/3] block: use bdrv_get_device_or_node_name() in error messages
Quoted 29 lines · Eric Blake
Quoted 15 lines
+++
b/block/snapshot.cblock/snapshot.c +3
-3@@ -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 ; }block/snapshot.c int bdrv_snapshot_load_tmp(BlockDriverState *bs, · line 334
Quoted 9 lines
block/snapshot.c +3
-3@@ -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)
Quoted 9 lines
blockdev.c static void internal_snapshot_prepare(BlkTransactionState *common, · line 1258
Quoted 7 lines
blockdev.c +4
-2+++ b/blockdev.cif (!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
PATCH v4 3/3 block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED
In reply to Alberto GarciaSince this event can occur in nodes that cannot have a device name associated, include also a field with the node name.
--- block/qcow2.c | 8 ++++++-- docs/qmp/qmp-events.txt | 21 +++++++++++++-------- qapi/block-core.json | 17 +++++++++++------ 3 files changed, 30 insertions(+), 16 deletions(-)
block/qcow2.c +6 -2
@@ -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);
docs/qmp/qmp-events.txt +13 -8
@@ -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 } }
qapi/block-core.json +11 -6
@@ -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
Signature · 1 line
2.1.4
Re: [Qemu-devel] [PATCH 3/3] block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED
Quoted 15 lines · Alberto Garcia
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 <[email protected]>--- 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 accessdocs/qmp/qmp-events.txt +0
-1- offset into the image (json-int)
docs/qmp/qmp-events.txt Example: · line 48
Quoted 10 lines
docs/qmp/qmp-events.txt +8
-2-- "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).
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH 3/3] block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED
Quoted 9 lines · Eric Blake
docs/qmp/qmp-events.txt Example: · line 48
Quoted 8 lines
+- "device": Device name (json-string) +- "node-name": Node name (json-string, optional) +- "msg": Informative message (e.g., reason for the corruption)docs/qmp/qmp-events.txt +5
-0+ (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)
Quoted 3 lines
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
Re: [Qemu-devel] [PATCH 3/3] block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED
In reply to Alberto GarciaQuoted 9 lines · Alberto Garcia
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 <[email protected]>--- block/qcow2.c | 8 ++++++-- docs/qmp/qmp-events.txt | 21 +++++++++++++-------- qapi/block-core.json | 17 +++++++++++------ 3 files changed, 30 insertions(+), 16 deletions(-)
Stefan Hajnoczi
AppliedQuoted 41 lines · Alberto Garcia
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 inBLOCK_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 inBLOCK_IMAGE_CORRUPTEDis now Since: 2.4
- Remove theQERR_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.
- TheBLOCK_IMAGE_CORRUPTEDevent has a new 'node-name' field.Regards,
Berto
Alberto Garcia (3):
block: addbdrv_get_device_or_node_name()
block: usebdrv_get_device_or_node_name()in error messages
block: add 'node-name' field toBLOCK_IMAGE_CORRUPTEDblock.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 ++++----
Quoted 8 lines
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