On Tue, Jan 30, 2024 at 09:30:30AM +0200, Manos Pitsidianakis wrote:
> In cases where a device tries to read more bytes than the block device
> contains with the blk_check_size_and_read_all() function, the error is
> vague: "device requires X bytes, block backend provides Y bytes".
>
> This patch changes the errors of this function to include the block
> backend name, the device id and device type name where appropriate.
>
> Version 3:
> - Changed phrasing "%s device with id='%s'" to "%s device '%s'" since
> second parameter might be either device id or device path.
> (thanks Stefan Hajnoczi <stefanha@redhat.com>)
>
> Version 2:
> - Assert dev is not NULL on qdev_get_human_name
> (thanks Phil Mathieu-Daudé <philmd@linaro.org>)
>
> Manos Pitsidianakis (2):
> hw/core/qdev.c: add qdev_get_human_name()
> hw/block/block.c: improve confusing blk_check_size_and_read_all()
> error
>
> include/hw/block/block.h | 4 ++--
> include/hw/qdev-core.h | 14 ++++++++++++++
> hw/block/block.c | 25 +++++++++++++++----------
> hw/block/m25p80.c | 3 ++-
> hw/block/pflash_cfi01.c | 4 ++--
> hw/block/pflash_cfi02.c | 2 +-
> hw/core/qdev.c | 8 ++++++++
> 7 files changed, 44 insertions(+), 16 deletions(-)
>
> Range-diff against v2:
> 1: 5fb5879708 ! 1: 8b566bfced hw/core/qdev.c: add qdev_get_human_name()
> @@ Commit message
> Add a simple method to return some kind of human readable identifier for
> use in error messages.
>
> + Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
>
> ## include/hw/qdev-core.h ##
> 2: 8e7eb17fbd ! 2: 7260eadff2 hw/block/block.c: improve confusing blk_check_size_and_read_all() error
> @@ hw/block/block.c: static int blk_pread_nonzeroes(BlockBackend *blk, hwaddr size,
> - "block backend provides %" PRIu64 " bytes",
> - size, blk_len);
> + dev_id = qdev_get_human_name(dev);
> -+ error_setg(errp, "%s device with id='%s' requires %" HWADDR_PRIu
> ++ error_setg(errp, "%s device '%s' requires %" HWADDR_PRIu
> + " bytes, %s block backend provides %" PRIu64 " bytes",
> + object_get_typename(OBJECT(dev)), dev_id, size,
> + blk_name(blk), blk_len);
> @@ hw/block/block.c: bool blk_check_size_and_read_all(BlockBackend *blk, void *buf,
> - error_setg_errno(errp, -ret, "can't read block backend");
> + dev_id = qdev_get_human_name(dev);
> + error_setg_errno(errp, -ret, "can't read %s block backend"
> -+ "for %s device with id='%s'",
> ++ " for %s device '%s'",
> + blk_name(blk), object_get_typename(OBJECT(dev)),
> + dev_id);
> return false;
>
> base-commit: 11be70677c70fdccd452a3233653949b79e97908
> --
> γαῖα πυρί μιχθήτω
>
Thanks, applied to my block tree:
https://gitlab.com/stefanha/qemu/commits/block
Stefan