Re: [Qemu-devel] [RFC PATCH v6 2/4] hw/block: Pad undersized read-only images with 0xFF

[Markus Armbruster]

Markus Armbruster <address@hidden> writes:

> From: Alex Bennée <address@hidden>
>
> We reject undersized images.  As of the previous commit, even with a
> decent error message.  Still, this is a potentially confusing
> stumbling block when you move from using -bios to using -drive
> if=pflash,file=blob,format=raw,readonly for loading your firmware
> code. To mitigate that we automatically pad in the read-only case and
> warn the user when we have performed magic to enable things to Just
> Work (tm).
>
> Signed-off-by: Alex Bennée <address@hidden>
> Reviewed-by: Laszlo Ersek <address@hidden>
> Signed-off-by: Markus Armbruster <address@hidden>

I think this convenience feature is a bad idea, and this patch should
not be applied.  Here are my reasons.

1. As I explained in my disccussion of v5[*], there is no single true
   way to pad.  This patch pads with 0xFF.  When working with physical
   devices, you'd sometimes pad that way, but at other times, you'd pad
   differently.

2. Except this patch doesn't *actually* pad with 0xFF.  The block layer
   silently pads with zeros up to the next multiple of 512.  Evidence:

    $ yes | dd of=4090b.img bs=1 count=4090
    4090+0 records in
    4090+0 records out
    4090 bytes (4.1 kB, 4.0 KiB) copied, 0.0186459 s, 219 kB/s
    $ qemu-io -f raw -c 'read -v 4000 96' 4090b.img
    00000fa0:  79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a  y.y.y.y.y.y.y.y.
    00000fb0:  79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a  y.y.y.y.y.y.y.y.
    00000fc0:  79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a  y.y.y.y.y.y.y.y.
    00000fd0:  79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a  y.y.y.y.y.y.y.y.
    00000fe0:  79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a 79 0a  y.y.y.y.y.y.y.y.
    00000ff0:  79 0a 79 0a 79 0a 79 0a 79 0a 00 00 00 00 00 00  y.y.y.y.y.......
    read 96/96 bytes at offset 4000
    96 bytes, 1 ops; 0.0001 sec (694.444 KiB/sec and 7407.4074 ops/sec)

   This patch then pads some more with 0xFF to the flash memory size.

   Because of that the way the magic padding works makes no sense, to be
   frank.  Going back to v3's zero padding would at least be explainable
   without blushing.

   I consider the block layer's padding a misfeature here, but right now
   we got to play the hand we've been dealt.

3. Convenience magic has bitten us in the posterior so many times.  Just
   say no unless there's a really compelling use case.  Where's the use
   case for this one?  We've rejected undersized images for ages, and
   nobody complained.  Why add convenience magic now?


[*] Message-ID: <address@hidden>
https://lists.nongnu.org/archive/html/qemu-devel/2019-03/msg01115.html