Re: [PATCH] m25p80: Warn the user when the backend file is too small for the device

[Cédric Le Goater]

On 11/15/22 15:55, Peter Maydell wrote:
> On Tue, 15 Nov 2022 at 14:51, Cédric Le Goater <clg@kaod.org> wrote:
> > On 11/15/22 15:34, Peter Maydell wrote:
> > > On Tue, 15 Nov 2022 at 14:22, Cédric Le Goater <clg@kaod.org> 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.
> > >
> > > The commit message says we get an unhelpful error if the
> > > file size "does not match"...
> > >
> > > > Improve the reported error with a new message regarding the file size.
> > > >
> > > > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > > > ---
> > > >    hw/block/m25p80.c | 8 ++++++++
> > > >    1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> > > > index 02adc87527..e0515e2a1e 100644
> > > > --- a/hw/block/m25p80.c
> > > > +++ b/hw/block/m25p80.c
> > > > @@ -1606,6 +1606,14 @@ static void m25p80_realize(SSIPeripheral *ss, Error 
> > > > **errp)
> > > >        if (s->blk) {
> > > >            uint64_t perm = BLK_PERM_CONSISTENT_READ |
> > > >                            (blk_supports_write_perm(s->blk) ? BLK_PERM_WRITE : 
> > > > 0);
> > > > +
> > > > +        if (blk_getlength(s->blk) < s->size) {
> > >
> > > ...but the code change is only checking for "too small".
> > >
> > > What happens if the user provides a backing file that is too large ?
> >
> > That's ok because the blk_pread() call following, which loads in RAM
> > the initial data, won't fail.
> >
> > It might be better to enforce a strict check on the size to avoid
> > further confusion ? and change the error message to be clear.
>
> Can we use blk_check_size_and_read_all() here rather than
> a manual "check size, and then pread" ? That will take care
> of the error message for you and make this device behave
> the same way as other flash devices which use block backends.

ok. I wasn't aware of this routine. I will check.

Thanks,
C.