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

["Clément Chigot" <chigot@adacore.com>]

On Fri, Oct 31, 2025 at 12:57 PM Kevin Wolf <kwolf@redhat.com> wrote:
>
> Am 31.10.2025 um 10:47 hat Clément Chigot geschrieben:
> > > > I was unable to make `vvfat.c` recognize the "size" argument passed
> > > > along `format=raw`, even if hardcoding the value in `vvfat.c` did make
> > > > a difference. And that's why I'm adding the warning in the commit
> > > > message.
> > >
> > > This one:
> > >
> > >     Some limitations remains, the size parameter is recognized only when
> > >     "format=vvfat" is passed. In particular, "format=raw,size=xxx" is
> > >     keeping the previously hardcoded value: 504MB for FAT16 and 32 MB for
> > >     FAT12. FAT32 has not been adjusted and thus still default to 504MB.
>
> I actually wonder if we should give the vvfat option a different name to
> make it less error prone. Forgetting format=vvfat is easy and then raw
> will be autodetected and size will be interpreted as an option for raw.

We can clearly name it "fat-size".

> > > > I've also realized that following my patch, the mismatch in between
> > > > the disk and the partition in Linux was going away when using `-drive
> > > > format=vvfat,size=xxx`. Making it not just QNX-oriented.
> > > >   | (host) $ qemu-system-aarch64 -M raspi4b -kernel raspi4b-kernel
> > > > -nographic -no-reboot -append "earlycon=pl011,mmio32,0xfe201000
> > > > console=ttyAMA0 noreboot" -dtb bcm2711-rpi-4-b.dtb -initrd
> > > > rootfs.cpio.gz  -drive
> > > > id=sdcard,file=fat:rw:<host_folder>,format=vvfat,if=sd,size=256M
> > > >   | (QEMU) # fdisk -l /dev/mmcblk1
> > > >   | Disk /dev/mmcblk1: 256 MB, 268435456 bytes, 524288 sectors
> > > >   | 1024 cylinders, 16 heads, 32 sectors/track
> > > >   | Units: sectors of 1 * 512 = 512 bytes
> > > >   |
> > > >   | Device       Boot StartCHS    EndCHS        StartLBA     EndLBA
> > > > Sectors  Size Id Type
> > > >   | /dev/mmcblk1p1 *  0,1,32      1023,15,32          63     524287
> > > >  524225  255M  6 FAT16
> > >
> > > EndLBA matches disk size.  EndCHS still matches EndLBA.  Good.
> > >
> > > The difference to the command line you showed above is format=raw (bad)
> > > vs. format=vvfat (good).
> > >
> > > With format=raw,size=256M you get that size, but a bogus partition
> > > table.
> > >
> > > With format=vvfat,size=256M you get that size, and a sensible partition
> > > table.
> > >
> > > Correct?
> >
> > Yes and the main reason for that is because I don't know how to
> > retrieve the size given to "raw" within `vvfat_open`.
> > My understanding is that raw-format.c is suppressing that option,
> > through `qemu_opts_del` in `raw_read_options`, before calling its
> > block childs (here vvfat). Hence, it assumes the default size 512M.
>
> The order is the other way around. You always get the protocol level
> openen first and only then the image format level.
>
> Imagine the simple case of a qcow2 image file used for the VM. You get
> things stacked like this:
>
>         virtio-blk
>             |
>             v
>           qcow2
>             |
>             v
>           file
>
> You need to open them from bottom to top. Opening a qcow2 image must be
> able to read from the file, so first the file layer must be opened. And
> obvious a virtio-blk device can only use the image after the qcow2
> layered has been opened.
>
> In your case, this is raw over vvfat. vvfat gets opened first, and then
> raw gets instantiated on top of it. (If you use format=vvfat, then the
> raw layer is left away.)
>
> Top level options you give to -drive go to the topmost block driver. You
> should be able to still set it on the vvfat level with -drive
> format=raw,file.size=... Deciding which option goes to which node is
> part of the (rather complicated) bdrv_open() logic in block.c.
>
> What raw does when a size option is given is that it just truncates the
> lower level to the given size. So as vvfat doesn't know the size, it
> still creates a 504 MB image, but raw shows only the first part of it to
> the guest. This results not only in an invalid partition table, but also
> means that as soon as vvfat decides to put a cluster after your limited
> size, you'll see filesystem corruption in the guest.
>
> So your approach to deal with this in vvfat and create a smaller
> filesystem to start with does look right to me.

Ok thanks for the explanation. It's a bit counter-intuitive that
"size" does not propagate to lower levels, especially if it generates
wrong ones behind the scene. But IIUC, this would be a much more
complex patch (i.e. changing bdrv_open logic).

Hence, I'm fine keeping this series narrowed to "format=vvfat".


> Kevin
>