Re: [PATCH v2 2/2] hw/block/block.c: improve confusing blk_check_size_and_read_all() error

[Stefan Hajnoczi]

On Tue, Jan 23, 2024 at 05:35:31PM +0200, Manos Pitsidianakis wrote:
>      if (blk_len != size) {
> -        error_setg(errp, "device requires %" HWADDR_PRIu " bytes, "
> -                   "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

Since qdev_get_human_name() falls back to returning the path instead of
the id, this error message could be confusing. Perhaps avoid saying what
dev_id is and let the user interpret it:

  %s device '%s'

> +                   " bytes, %s block backend provides %" PRIu64 " bytes",
> +                   object_get_typename(OBJECT(dev)), dev_id, size,
> +                   blk_name(blk), blk_len);
>          return false;
>      }
>  
> @@ -89,7 +90,11 @@ bool blk_check_size_and_read_all(BlockBackend *blk, void 
> *buf, hwaddr size,
>      assert(size <= BDRV_REQUEST_MAX_BYTES);
>      ret = blk_pread_nonzeroes(blk, size, buf);
>      if (ret < 0) {
> -        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'",

Same here.

signature.asc
Description: PGP signature