| Message ID | cover.1706598705.git.manos.pitsidianakis@linaro.org |
|---|---|
| Headers | show |
| Series | hw/block/block.c: improve confusing error | expand |
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
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