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

[Laszlo Ersek]

On 03/07/19 11:05, Markus Armbruster wrote:
> 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.

I'm fine with that.

Thanks,
Laszlo


>  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
>