On Tue, Feb 14, 2023 at 06:18:23PM +0100, Cédric Le Goater wrote:
> Currently, when a block backend is attached to a m25p80 device and the
> associated file size does not match the flash model, QEMU complains
> with the error message "failed to read the initial flash content".
> This is confusing for the user.
>
> Use blk_check_size_and_read_all() instead of blk_pread() to improve
> the reported error.
>
> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
> Reviewed-by: Peter Delevoryas <pe...@pjd.dev>
> Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>
> Message-Id: <20221115151000.2080833-1-...@kaod.org>
> Signed-off-by: Cédric Le Goater <c...@kaod.org>
> ---
>
> breakage with commit a4b15a8b9e ("pflash: Only read non-zero parts
> of backend image") when using -snaphot.
I guess it's not obvious to me, what broke?
1. BlockBackend *blk = -drive file=blob.mtd,format=raw,if=mtd,snapshot=on
2. uint8_t *storage = malloc(...)
3. blk_check_size_and_read_all(blk, storage, size)
4. commit a4b15a8b9e
for sector in blk:
if any(b != 0 for b in sector):
memcpy(&storage[...], sector, sizeof(sector))
else:
# Skip memcpy of zeroes
But this is probably a regression for us because we actually expect the zeroes
to be copied?
- Peter
>
> hw/block/m25p80.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> index 802d2eb021..dc5ffbc4ff 100644
> --- a/hw/block/m25p80.c
> +++ b/hw/block/m25p80.c
> @@ -24,6 +24,7 @@
> #include "qemu/osdep.h"
> #include "qemu/units.h"
> #include "sysemu/block-backend.h"
> +#include "hw/block/block.h"
> #include "hw/qdev-properties.h"
> #include "hw/qdev-properties-system.h"
> #include "hw/ssi/ssi.h"
> @@ -1615,8 +1616,7 @@ static void m25p80_realize(SSIPeripheral *ss, Error
> **errp)
> trace_m25p80_binding(s);
> s->storage = blk_blockalign(s->blk, s->size);
>
> - if (blk_pread(s->blk, 0, s->size, s->storage, 0) < 0) {
> - error_setg(errp, "failed to read the initial flash content");
> + if (!blk_check_size_and_read_all(s->blk, s->storage, s->size, errp))
> {
> return;
> }
> } else {
> --
> 2.39.1
>
