Re: [PATCH v2 5/5] vvfat: add support for "fat-size" options

[Markus Armbruster <armbru@redhat.com>]

Kevin Wolf <kwolf@redhat.com> writes:

> Am 10.11.2025 um 14:42 hat Markus Armbruster geschrieben:
>> Clément Chigot <chigot@adacore.com> writes:
>> 
>> > On Mon, Nov 10, 2025 at 2:09 PM Markus Armbruster <armbru@redhat.com> wrote:
>> >>
>> >> Clément Chigot <chigot@adacore.com> writes:
>> >>
>> >> > On Mon, Nov 10, 2025 at 11:13 AM Markus Armbruster <armbru@redhat.com> wrote:
>> >> >>
>> >> >> Clément Chigot <chigot@adacore.com> writes:
>> >> >>
>> >> >> > This allows more flexibility to vvfat backend. The values of "Number of
>> >> >> > Heads" and "Sectors per track" are based on SD specifications Part 2.
>> >> >> >
>> >> >> > Due to the FAT architecture, not all sizes are reachable. Therefore, it
>> >> >> > could be round up to the closest available size.
>> >> >> >
>> >> >> > FAT32 has not been adjusted and thus still default to 504 Mib.
>> >> >> >
>> >> >> > For floppy, only 1440 Kib and 2880 Kib are supported.
>> >> >> >
>> >> >> > Signed-off-by: Clément Chigot <chigot@adacore.com>
>> >> >>
>> >> >> [...]
>> >> >>
>> >> >> > diff --git a/qapi/block-core.json b/qapi/block-core.json
>> >> >> > index 8a479ba090..0bcb360320 100644
>> >> >> > --- a/qapi/block-core.json
>> >> >> > +++ b/qapi/block-core.json
>> >> >> > @@ -3478,11 +3478,17 @@
>> >> >> >  #     (default: true)
>> >> >> >  #     (since 10.2)
>> >> >> >  #
>> >> >> > +# @fat-size: size of the device in bytes.  Due to FAT underlying
>> >> >> > +#     architecture, this size can be rounded up to the closest valid
>> >> >> > +#     size.
>> >> >> > +#     (since 10.2)
>> >> >> > +#
>> >> >>
>> >> >> Can you explain again why you moved from @size to @fat-size?
>> >> >
>> >> > Just to be sure, you mean in the above comment, in the commit message or both ?
>> >>
>> >> Just to me, because I'm not sure I like the change, but that may well be
>> >> due to a lack of understanding of your reasons.
>> >
>> > Naming `fat-size` instead of `size` ensures the parameter is only
>> > recognized by the vvfat backend. In particular, it will be refused by
>> > the default raw format, avoiding confusion:
>> >  "-drive file=fat:<path>,size=256M" results in a 504M FAT disk
>> > truncated to 256M, raw format being implicit.
>> >  "-drive file=fat:<path>,fat-size=256M" is refused. "fat-size" is
>> > unsupported by raw format.
>> 
>> I figure throwing in format=raw to make raw format explicit doesn't
>> change anything.  Correct?
>> 
>> >  "-drive file=fat:<path>,format=vvfat,fat-size=256M" results in a 256M FAT disk.
>> >  "-drive file=fat:<path>,format=vvfat,size=256M" is refused. "size" is
>> > unsupported by vvfat format.
>> 
>> If it was called @size, what behavior would we get?  Just two cases, I
>> think:
>> 
>> 1. With raw format:
>> 
>>     -drive file=fat:<path>,size=256M
>
> You'd silently get a 504 MiB filesystem truncated to 256 MiB (i.e. a
> corrupted file system). It's quite easy to forget format=vvfat, so
> something that initially looks like it might be working is not a great
> result for this user error.
>
>> 2. Without raw format:
>> 
>>     -drive file=fat:<path>,format=vvfat,size=256M
>
> This does the thing that you actually want, a 256 MiB file system.
>
> I suggested to rename the vvfat option in v1 to make accidents at least
> a bit less likely.

Valid point.

The "raw" format's slicing feature has dangerous sharp edges.

I'm all for with giving users poweful tools, even if they're dangerous.
However, as we can see here, this one can interact badly with the
implicit use of "raw".  Adding the slicing feature to "raw" may have
been a mistake, and naming one of its configuration options "size"
definitely was a mistake.  Something like @slice-offset and @slize-size
would've been safer.

Anyway, the interface is set in stone now.

>                    I'm not completely sure if "fat-size" is the best
> name, though, as it sounds as if it referred to the FAT itself instead
> of the FAT filesystem. Maybe "fs-size"?

Better.  It's the size of the image, though, not the size of the
filesystem.  They are the same only if there's no MBR.