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

[Kevin Wolf <kwolf@redhat.com>]

Am 10.11.2025 um 16:36 hat BALATON Zoltan geschrieben:
> On Mon, 10 Nov 2025, Kevin Wolf wrote:
> > 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.
> 
> Why doesn't file=fat: imply format=vvfat? For what is the fat: part in
> file then?

-drive is built pretty much on the assumption that you have an image
format that runs on top of a protocol. Format probing probes the image
format, not the protocol, while prefixes like fat: (or nbd:, http: etc.)
specify the protocol.

So if you don't specify the format, we first open the protocol level
(which is vvfat) and then probing will detect that over this protocol,
we access a raw image. So it's mostly like saying format=raw.

I think that format=<protocol driver> works is really more accidental,
but we can't change it now (and probably also don't want to). It results
in opening only the protocol layer and not stacking any format driver on
top of it.

Options that you specify in -drive generally go to the top layer. So the
consequence in our case is that with format=vvfat, the option goes to
vvfat, but with format=raw (or unspecified format), it goes to the raw
forma driver.

> I currently recommend using:
> 
> -drive if=none,id=ufat,format=raw,file=fat:rw:/dir/to/export
> -device usb-storage,drive=ufat
> 
> to my users which I got from somewhere but don't remember where and it
> seems to work but maybe not the best way to specify this.

It's fine, and I might use the same one myself (though you should be
aware that fat:rw: is risky, it's full of bugs).

But if you add an option like size=64M, it goes to the raw driver, which
will take whatever image you access on the protocol level and truncate
it at 64 MiB.

If you want to give the size option on the vvfat level (and create a
filesystem that is actually only 64 MiB instead of truncating a larger
one), then obviously format=vvfat allows you to do that because then
there is no raw format layer to begin with. Or if you do have the raw
format layer, you can access options of the protocol layer by prefixing
"file.". So format=raw,file.size=64M would still pass the size option to
vvfat.

So the command line does allow you to get the option to the right place,
it's just very easy to get confused about this and to specify the option
for the wrong layer.

> After reading this thread I'm confused about how to use this
> correctly. Is there some documentation on this? There only seems to be
> some mentions in docs/system/qemu-block-drivers.rst.inc but all of
> them using older options:
> 
>   |qemu_system| linux.img -hdb fat:/my_directory
>   |qemu_system| linux.img -fda fat:floppy:/my_directory
>   |qemu_system| linux.img -fda fat:floppy:rw:/my_directory

All of those are honestly fine, too, if you're happy with the defaults
and don't want to set more advanced options.

I'll give you this bonus option if you want to be modern:

    -blockdev vvfat,node-name=ufat,dir=/my_directory,rw=on
    -device usb-storage,drive=ufat

But I don't think any of the other options is going away anytime soon.

Kevin