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

Clément Chigot posted 5 patches 5 months ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
[PATCH 5/5] vvfat: add support for "size" options
Posted by Clément Chigot 5 months ago
This allows more flexibility to vvfat backend. The value for "Number of
Heads" and "Sectors per track" are based on SD specifications Part 2.

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.

Moreover, for flopyy, size=1M is creating a disk 1.44 MB, and size=2M a
disk of 2.88 MB. This avoids having to worry about float operations.

Signed-off-by: Clément Chigot <chigot@adacore.com>
---
 block/vvfat.c | 165 ++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 141 insertions(+), 24 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index 6526c585a2..4537c39d5c 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1091,6 +1091,11 @@ static QemuOptsList runtime_opts = {
             .type = QEMU_OPT_BOOL,
             .help = "Do not add a Master Boot Record on this disk",
         },
+        {
+            .name = BLOCK_OPT_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "Virtual disk size"
+        },
         { /* end of list */ }
     },
 };
@@ -1148,10 +1153,141 @@ static void vvfat_parse_filename(const char *filename, QDict *options,
     qdict_put_bool(options, "no-mbr", no_mbr);
 }
 
+static void vvfat_get_size_parameters(uint64_t size, BDRVVVFATState *s,
+                                      bool floppy, Error **errp)
+{
+    if (floppy) {
+        /*
+         * Floppy emulation only supports 1.44 MB or 2.88 MB (default).
+         * In order to avoid floating operations ambiguity, 1 MB is
+         * recognized for 1.44 MB and 2 MB for 2.88 MB.
+         */
+        if (!size) {
+            size = 2 * 1024 * 1024;
+        } else {
+            if (size == 1024 * 1024 && s->fat_type == 16) {
+                error_setg(errp,
+                           "floppy FAT16 unsupported size; only support 2M "
+                           "(for an effective size of 2.88 MB)");
+            } else if (size != 2 * 1024 * 1024 && size != 1024 * 1024) {
+                error_setg(errp,
+                           "floppy unsupported size; should be 1MB (for "
+                           "an effective size of 1.44 MB) or 2.88M (for "
+                           "2.88MB)");
+            }
+        }
+
+        if (s->fat_type == 12) {
+            if (size == 2 * 1024 * 1024) {
+                s->sectors_per_cluster = 2;
+            } else {
+                s->sectors_per_cluster = 1;
+            }
+        } else {
+            s->sectors_per_cluster = 1;
+        }
+
+        s->sectors_per_track = 36;
+        s->cylinders = 80;
+        s->number_of_heads = 2;
+    } else {
+        /* LATER TODO: if FAT32, adjust */
+        s->sectors_per_cluster = 0x10;
+
+        switch (s->fat_type) {
+        case 12:
+
+            /* Default is 32 MB */
+            if (!size) {
+                size = 32 * 1024 * 1024;
+            } else if (size > 32 * 1024 * 1024) {
+                error_setg(errp, "FAT12 unsupported size; higher than 32Mb");
+            }
+
+            s->cylinders = 64;
+
+            /*
+             * Based on CHS Recommandation table:
+             *  Card Capacity | Number of Headers | Sectors per track
+             *     ~ 2 MB     |         4         |       16
+             *     ~ 4 MB     |         8         |       16
+             *     ~ 8 MB     |        16         |       16
+             *     ~ 16 MB    |         2         |       32
+             *     ~ 32 MB    |         4         |       32
+             *
+             * For 2 MB, SD is recommending heads = 2 and sectors = 16, but
+             * this requires a different number of cylinders. Thus, it was
+             * adjusted to keep this number constant.
+             */
+            if (size <= 8 * 1024 * 1024) {
+                s->sectors_per_track = 16;
+            } else {
+                s->sectors_per_track = 32;
+            }
+
+            /*
+             * The formula between the size (in bytes) and the parameters are:
+             *  size = SECTOR_SIZE * sectors_per_track * number_of_headers *
+             *         cylinders
+             */
+            s->number_of_heads = size / s->sectors_per_track /
+                SECTOR_SIZE / s->cylinders;
+            return;
+
+        case 16:
+            /* Default is 504 MB */
+            if (!size) {
+                size = 504 * 1024 * 1024;
+            } else if (size / 1024 > 4 * 1024 * 1024) {
+                error_setg(errp, "FAT16 unsupported size; higher than 4Gb");
+            }
+
+            s->cylinders = 1024;
+
+            /*
+             * Based on CHS Recommandation table:
+             *  Card Capacity | Number of Headers | Sectors per track
+             *     ~64 MB     |         4         |       32
+             *     ~128 MB    |         8         |       32
+             *     ~256 MB    |        16         |       32
+             *     ~504 MB    |        16         |       63
+             *    ~1008 MB    |        32         |       63
+             *    ~2016 MB    |        64         |       63
+             */
+            if (size <= 256 * 1024 * 1024) {
+                s->sectors_per_track = 32;
+            } else {
+                s->sectors_per_track = 63;
+            }
+
+            /*
+             * The formula between the size (in bytes) and the parameters are:
+             *  size = SECTOR_SIZE * sectors_per_track * number_of_headers *
+             *         cylinders
+             */
+            s->number_of_heads = size / s->sectors_per_track /
+                SECTOR_SIZE / s->cylinders;
+            return;
+
+        case 32:
+            /* TODO FAT32 adjust  */
+            if (size) {
+                warn_report("size parameters not supported with FAT32;"
+                            "default to 504MB.");
+            }
+            s->cylinders = 1024;
+            s->number_of_heads = 16;
+            s->sectors_per_track = 63;
+            return;
+        }
+    }
+}
+
 static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
                       Error **errp)
 {
     BDRVVVFATState *s = bs->opaque;
+    uint64_t size;
     bool floppy;
     const char *dirname, *label;
     QemuOpts *opts;
@@ -1178,6 +1314,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
 
     s->fat_type = qemu_opt_get_number(opts, "fat-type", 0);
     floppy = qemu_opt_get_bool(opts, "floppy", false);
+    size = qemu_opt_get_size_del(opts, "size", 0);
 
     memset(s->volume_label, ' ', sizeof(s->volume_label));
     label = qemu_opt_get(opts, "label");
@@ -1215,35 +1352,15 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
+    vvfat_get_size_parameters(size, s, floppy, errp);
 
-    if (floppy) {
-        /* 2.88MB floppy */
-        if (s->fat_type == 12) {
-            s->sectors_per_track = 36;
-            s->sectors_per_cluster = 2;
-        } else {
-            s->sectors_per_track = 36;
-            s->sectors_per_cluster = 1;
-        }
-        s->cylinder = 80;
-        s->number_of_heads = 2;
-    } else {
-        /* Reserver space for MBR */
-        if (!qemu_opt_get_bool(opts, "no-mbr", false)) {
-            s->offset_to_bootsector = 0x3f;
-        }
-        /* 32MB or 504MB disk*/
-        s->cylinders = s->fat_type == 12 ? 64 : 1024;
-        s->number_of_heads = 16;
-        s->sectors_per_track = 63;
+    /* Reserver space for MBR */
+    if (!floppy && !qemu_opt_get_bool(opts, "no-mbr", false)) {
+        s->offset_to_bootsector = 0x3f;
     }
 
-
     s->bs = bs;
 
-    /* LATER TODO: if FAT32, adjust */
-    s->sectors_per_cluster=0x10;
-
     s->current_cluster=0xffffffff;
 
     s->qcow = NULL;
-- 
2.34.1


Re: [PATCH 5/5] vvfat: add support for "size" options
Posted by Kevin Wolf 3 months, 2 weeks ago
Am 03.09.2025 um 09:57 hat Clément Chigot geschrieben:
> This allows more flexibility to vvfat backend. The value for "Number of
> Heads" and "Sectors per track" are based on SD specifications Part 2.
> 
> 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.
> 
> Moreover, for flopyy, size=1M is creating a disk 1.44 MB, and size=2M a
> disk of 2.88 MB. This avoids having to worry about float operations.
> 
> Signed-off-by: Clément Chigot <chigot@adacore.com>
> ---
>  block/vvfat.c | 165 ++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 141 insertions(+), 24 deletions(-)
> 
> diff --git a/block/vvfat.c b/block/vvfat.c
> index 6526c585a2..4537c39d5c 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -1091,6 +1091,11 @@ static QemuOptsList runtime_opts = {
>              .type = QEMU_OPT_BOOL,
>              .help = "Do not add a Master Boot Record on this disk",
>          },
> +        {
> +            .name = BLOCK_OPT_SIZE,
> +            .type = QEMU_OPT_SIZE,
> +            .help = "Virtual disk size"
> +        },
>          { /* end of list */ }
>      },
>  };

Like in patch 1, you need additional changes, in particular to add the
option to the QAPI schema in qapi/block-core.json.

> @@ -1148,10 +1153,141 @@ static void vvfat_parse_filename(const char *filename, QDict *options,
>      qdict_put_bool(options, "no-mbr", no_mbr);
>  }
>  
> +static void vvfat_get_size_parameters(uint64_t size, BDRVVVFATState *s,
> +                                      bool floppy, Error **errp)
> +{
> +    if (floppy) {
> +        /*
> +         * Floppy emulation only supports 1.44 MB or 2.88 MB (default).
> +         * In order to avoid floating operations ambiguity, 1 MB is
> +         * recognized for 1.44 MB and 2 MB for 2.88 MB.
> +         */
> +        if (!size) {
> +            size = 2 * 1024 * 1024;
> +        } else {
> +            if (size == 1024 * 1024 && s->fat_type == 16) {
> +                error_setg(errp,
> +                           "floppy FAT16 unsupported size; only support 2M "
> +                           "(for an effective size of 2.88 MB)");
> +            } else if (size != 2 * 1024 * 1024 && size != 1024 * 1024) {
> +                error_setg(errp,
> +                           "floppy unsupported size; should be 1MB (for "
> +                           "an effective size of 1.44 MB) or 2.88M (for "
> +                           "2.88MB)");
> +            }
> +        }

This is horrible. To be fair, it's pretty hard to do something not
horrible when the usual units to describe floppy sizes are already
horrible. :-)

But I'd still like us to do better here.

To me it looks a bit like what we really want is an enum for floppy
sizes (though is there any real reason why we have only those two?), but
an arbitrary size for hard disks.

Without the enum, obviously, users could specify 1440k and that would do
the right thing. Maybe special casing whatever 1.44M and 2.88M result
in and translating them into 1440k and 2880k could be more justifiable
than special casing 1M and 2M, but it would still be ugly.

Markus, do you have any advice how this should be represented in QAPI?

Kevin

> +
> +        if (s->fat_type == 12) {
> +            if (size == 2 * 1024 * 1024) {
> +                s->sectors_per_cluster = 2;
> +            } else {
> +                s->sectors_per_cluster = 1;
> +            }
> +        } else {
> +            s->sectors_per_cluster = 1;
> +        }
> +
> +        s->sectors_per_track = 36;
> +        s->cylinders = 80;
> +        s->number_of_heads = 2;
> +    } else {
> +        /* LATER TODO: if FAT32, adjust */
> +        s->sectors_per_cluster = 0x10;
> +
> +        switch (s->fat_type) {
> +        case 12:
> +
> +            /* Default is 32 MB */
> +            if (!size) {
> +                size = 32 * 1024 * 1024;
> +            } else if (size > 32 * 1024 * 1024) {
> +                error_setg(errp, "FAT12 unsupported size; higher than 32Mb");
> +            }
> +
> +            s->cylinders = 64;
> +
> +            /*
> +             * Based on CHS Recommandation table:
> +             *  Card Capacity | Number of Headers | Sectors per track
> +             *     ~ 2 MB     |         4         |       16
> +             *     ~ 4 MB     |         8         |       16
> +             *     ~ 8 MB     |        16         |       16
> +             *     ~ 16 MB    |         2         |       32
> +             *     ~ 32 MB    |         4         |       32
> +             *
> +             * For 2 MB, SD is recommending heads = 2 and sectors = 16, but
> +             * this requires a different number of cylinders. Thus, it was
> +             * adjusted to keep this number constant.
> +             */
> +            if (size <= 8 * 1024 * 1024) {
> +                s->sectors_per_track = 16;
> +            } else {
> +                s->sectors_per_track = 32;
> +            }
> +
> +            /*
> +             * The formula between the size (in bytes) and the parameters are:
> +             *  size = SECTOR_SIZE * sectors_per_track * number_of_headers *
> +             *         cylinders
> +             */
> +            s->number_of_heads = size / s->sectors_per_track /
> +                SECTOR_SIZE / s->cylinders;
> +            return;
> +
> +        case 16:
> +            /* Default is 504 MB */
> +            if (!size) {
> +                size = 504 * 1024 * 1024;
> +            } else if (size / 1024 > 4 * 1024 * 1024) {
> +                error_setg(errp, "FAT16 unsupported size; higher than 4Gb");
> +            }
> +
> +            s->cylinders = 1024;
> +
> +            /*
> +             * Based on CHS Recommandation table:
> +             *  Card Capacity | Number of Headers | Sectors per track
> +             *     ~64 MB     |         4         |       32
> +             *     ~128 MB    |         8         |       32
> +             *     ~256 MB    |        16         |       32
> +             *     ~504 MB    |        16         |       63
> +             *    ~1008 MB    |        32         |       63
> +             *    ~2016 MB    |        64         |       63
> +             */
> +            if (size <= 256 * 1024 * 1024) {
> +                s->sectors_per_track = 32;
> +            } else {
> +                s->sectors_per_track = 63;
> +            }
> +
> +            /*
> +             * The formula between the size (in bytes) and the parameters are:
> +             *  size = SECTOR_SIZE * sectors_per_track * number_of_headers *
> +             *         cylinders
> +             */
> +            s->number_of_heads = size / s->sectors_per_track /
> +                SECTOR_SIZE / s->cylinders;
> +            return;
> +
> +        case 32:
> +            /* TODO FAT32 adjust  */
> +            if (size) {
> +                warn_report("size parameters not supported with FAT32;"
> +                            "default to 504MB.");
> +            }
> +            s->cylinders = 1024;
> +            s->number_of_heads = 16;
> +            s->sectors_per_track = 63;
> +            return;
> +        }
> +    }
> +}
> +
>  static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
>                        Error **errp)
>  {
>      BDRVVVFATState *s = bs->opaque;
> +    uint64_t size;
>      bool floppy;
>      const char *dirname, *label;
>      QemuOpts *opts;
> @@ -1178,6 +1314,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
>  
>      s->fat_type = qemu_opt_get_number(opts, "fat-type", 0);
>      floppy = qemu_opt_get_bool(opts, "floppy", false);
> +    size = qemu_opt_get_size_del(opts, "size", 0);
>  
>      memset(s->volume_label, ' ', sizeof(s->volume_label));
>      label = qemu_opt_get(opts, "label");
> @@ -1215,35 +1352,15 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
>          goto fail;
>      }
>  
> +    vvfat_get_size_parameters(size, s, floppy, errp);
>  
> -    if (floppy) {
> -        /* 2.88MB floppy */
> -        if (s->fat_type == 12) {
> -            s->sectors_per_track = 36;
> -            s->sectors_per_cluster = 2;
> -        } else {
> -            s->sectors_per_track = 36;
> -            s->sectors_per_cluster = 1;
> -        }
> -        s->cylinder = 80;
> -        s->number_of_heads = 2;
> -    } else {
> -        /* Reserver space for MBR */
> -        if (!qemu_opt_get_bool(opts, "no-mbr", false)) {
> -            s->offset_to_bootsector = 0x3f;
> -        }
> -        /* 32MB or 504MB disk*/
> -        s->cylinders = s->fat_type == 12 ? 64 : 1024;
> -        s->number_of_heads = 16;
> -        s->sectors_per_track = 63;
> +    /* Reserver space for MBR */
> +    if (!floppy && !qemu_opt_get_bool(opts, "no-mbr", false)) {
> +        s->offset_to_bootsector = 0x3f;
>      }
>  
> -
>      s->bs = bs;
>  
> -    /* LATER TODO: if FAT32, adjust */
> -    s->sectors_per_cluster=0x10;
> -
>      s->current_cluster=0xffffffff;
>  
>      s->qcow = NULL;
> -- 
> 2.34.1
> 
Re: [PATCH 5/5] vvfat: add support for "size" options
Posted by Markus Armbruster 3 months ago
Kevin Wolf <kwolf@redhat.com> writes:

[...]

> To me it looks a bit like what we really want is an enum for floppy
> sizes (though is there any real reason why we have only those two?), but
> an arbitrary size for hard disks.
>
> Without the enum, obviously, users could specify 1440k and that would do
> the right thing. Maybe special casing whatever 1.44M and 2.88M result
> in and translating them into 1440k and 2880k could be more justifiable
> than special casing 1M and 2M, but it would still be ugly.
>
> Markus, do you have any advice how this should be represented in QAPI?

Still want answers here?

[...]
Re: [PATCH 5/5] vvfat: add support for "size" options
Posted by Kevin Wolf 3 months ago
Am 05.11.2025 um 08:08 hat Markus Armbruster geschrieben:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
> [...]
> 
> > To me it looks a bit like what we really want is an enum for floppy
> > sizes (though is there any real reason why we have only those two?), but
> > an arbitrary size for hard disks.
> >
> > Without the enum, obviously, users could specify 1440k and that would do
> > the right thing. Maybe special casing whatever 1.44M and 2.88M result
> > in and translating them into 1440k and 2880k could be more justifiable
> > than special casing 1M and 2M, but it would still be ugly.
> >
> > Markus, do you have any advice how this should be represented in QAPI?
> 
> Still want answers here?

Yes, I'm still not sure how we could best represent both hard disk and
floppy sizes in vvfat in a way that isn't completely counterintuitive
for users, that also isn't just arbitrary magic and that works on the
command line.

Unless the need for different sizes has gone away, but I don't think we
found any other solution for the problem that would not require a
configurable disk/file system size?

Kevin
Re: [PATCH 5/5] vvfat: add support for "size" options
Posted by Markus Armbruster 3 months ago
Kevin Wolf <kwolf@redhat.com> writes:

> Am 05.11.2025 um 08:08 hat Markus Armbruster geschrieben:
>> Kevin Wolf <kwolf@redhat.com> writes:
>> 
>> [...]
>> 
>> > To me it looks a bit like what we really want is an enum for floppy
>> > sizes (though is there any real reason why we have only those two?), but
>> > an arbitrary size for hard disks.
>> >
>> > Without the enum, obviously, users could specify 1440k and that would do
>> > the right thing. Maybe special casing whatever 1.44M and 2.88M result
>> > in and translating them into 1440k and 2880k could be more justifiable
>> > than special casing 1M and 2M, but it would still be ugly.
>> >
>> > Markus, do you have any advice how this should be represented in QAPI?
>> 
>> Still want answers here?
>
> Yes, I'm still not sure how we could best represent both hard disk and
> floppy sizes in vvfat in a way that isn't completely counterintuitive
> for users, that also isn't just arbitrary magic and that works on the
> command line.
>
> Unless the need for different sizes has gone away, but I don't think we
> found any other solution for the problem that would not require a
> configurable disk/file system size?

Let me recap the problem.  Please correct my misunderstandings, if any.

Hard disks can have almost arbitrary sizes.  Almost, because it still
needs to be a multiple of the block size.

Floppy disks have one of a small set of well-known sizes.

I vaguely recall that we generally derive the device's actual size from
the backend's size.

Some devices reject certain sizes.  For instance, SD cards require a
power of 2.

Most devices seem to accept anything.  I can create an IDE, SCSI, or
floppy disk backed by a raw image of one byte.  I have no idea how it
would behave.

As is, the vvfat backend can only do certain sizes, configurable with
parameters @floppy and @fat-type.  They work for floppies, but not for
SD cards, since they're not powers of two.

Instead of deriving size and CHS from @floppy and @fat-type, Clément
proposes to specify the size (and derive fat-type and CHS[*]?).

In QMP, we specify the size in bytes.  This is fine regardless of size;
management applications don't mind sending things like "size": 1474560.

In HMP and the command line, big byte sizes are inconvenient.  That's
why we support suffixes there.  size=256M is a fine way to pick an SD
card's size.

The size suffixes seem inconvenient for floppies, though.  For instance,
2 heads * 80 tracks * 18 sectors * 512 bytes = 1474560 bytes = 1440
KiBytes, but size=1.44M does not work: 1.44 MiBytes = 1509949.44 Bytes.
However, size=1440K does.

This leads me to suggest to simply stick to numeric size, and use
appropriate suffixes.  These are obvious enough for anything but
floppies.  So advise users "use K for floppies"[**].

If this isn't good enough, I can help you explore fancier parts of QAPI,
such as alternate types.




[*] I guess we could support specifying an optional fat-type in addition
to size, and derive only CHS then.

[**] Even for a hypothetical floppy with an odd number of 512 byte
sectors: .5K works, because .5 * 1024 is an integer.
Re: [PATCH 5/5] vvfat: add support for "size" options
Posted by Clément Chigot 3 months ago
On Wed, Nov 5, 2025 at 10:36 AM Kevin Wolf <kwolf@redhat.com> wrote:
>
> Am 05.11.2025 um 08:08 hat Markus Armbruster geschrieben:
> > Kevin Wolf <kwolf@redhat.com> writes:
> >
> > [...]
> >
> > > To me it looks a bit like what we really want is an enum for floppy
> > > sizes (though is there any real reason why we have only those two?), but
> > > an arbitrary size for hard disks.
> > >
> > > Without the enum, obviously, users could specify 1440k and that would do
> > > the right thing. Maybe special casing whatever 1.44M and 2.88M result
> > > in and translating them into 1440k and 2880k could be more justifiable
> > > than special casing 1M and 2M, but it would still be ugly.
> > >
> > > Markus, do you have any advice how this should be represented in QAPI?
> >
> > Still want answers here?
>
> Yes, I'm still not sure how we could best represent both hard disk and
> floppy sizes in vvfat in a way that isn't completely counterintuitive
> for users, that also isn't just arbitrary magic and that works on the
> command line.

For v2 (probably pushed sometimes this week), I've changed the
command-line approach to allow only `fat-size=1440K` and
`fat-size=2880K`. Other values will be rejected (including `1.44M` or
`2.88M`).
I'm not familiar with QAPI to see if that approach would fit properly.

> Unless the need for different sizes has gone away, but I don't think we
> found any other solution for the problem that would not require a
> configurable disk/file system size?
>
> Kevin
>
Re: [PATCH 5/5] vvfat: add support for "size" options
Posted by Markus Armbruster 3 months, 2 weeks ago
Kevin Wolf <kwolf@redhat.com> writes:

> Am 03.09.2025 um 09:57 hat Clément Chigot geschrieben:
>> This allows more flexibility to vvfat backend. The value for "Number of
>> Heads" and "Sectors per track" are based on SD specifications Part 2.

This is too terse to remind me of how vvfat picks cylinders, heads, and
sectors before this patch, so I need to go dig through the source code.
I figure it depends on configuration parameters @floppy and @fat-type
like this:

    floppy  fat-type    cyls heads secs   cyls*heads*secs*512
    false      12         64    16   63         31.5 MiB
    false      16       1024    16   63        504   MiB
    false      32       1024    16   63        504   MiB
    true       12         80     2   18       1440   KiB
    true       16         80     2   36       2880   KiB
    true       32         80     2   36       2880   KiB

How exactly does the new parameter @size change this?

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

31.5MiB unless I'm mistaken.

I'm not sure what you're trying to convey in this paragraph.  As far as
I can tell, you're adding a @size parameter to vvfat, so of course it
doesn't affect raw.

>> Moreover, for flopyy, size=1M is creating a disk 1.44 MB, and size=2M a

floppy

>> disk of 2.88 MB. This avoids having to worry about float operations.

More on this part below.

>> Signed-off-by: Clément Chigot <chigot@adacore.com>
>> ---
>>  block/vvfat.c | 165 ++++++++++++++++++++++++++++++++++++++++++--------
>>  1 file changed, 141 insertions(+), 24 deletions(-)
>> 
>> diff --git a/block/vvfat.c b/block/vvfat.c
>> index 6526c585a2..4537c39d5c 100644
>> --- a/block/vvfat.c
>> +++ b/block/vvfat.c
>> @@ -1091,6 +1091,11 @@ static QemuOptsList runtime_opts = {
>>              .type = QEMU_OPT_BOOL,
>>              .help = "Do not add a Master Boot Record on this disk",
>>          },
>> +        {
>> +            .name = BLOCK_OPT_SIZE,
>> +            .type = QEMU_OPT_SIZE,
>> +            .help = "Virtual disk size"
>> +        },
>>          { /* end of list */ }
>>      },
>>  };
>
> Like in patch 1, you need additional changes, in particular to add the
> option to the QAPI schema in qapi/block-core.json.
>
>> @@ -1148,10 +1153,141 @@ static void vvfat_parse_filename(const char *filename, QDict *options,
>>      qdict_put_bool(options, "no-mbr", no_mbr);
>>  }
>>  
>> +static void vvfat_get_size_parameters(uint64_t size, BDRVVVFATState *s,
>> +                                      bool floppy, Error **errp)
>> +{
>> +    if (floppy) {
>> +        /*
>> +         * Floppy emulation only supports 1.44 MB or 2.88 MB (default).
>> +         * In order to avoid floating operations ambiguity, 1 MB is
>> +         * recognized for 1.44 MB and 2 MB for 2.88 MB.
>> +         */
>> +        if (!size) {
>> +            size = 2 * 1024 * 1024;
>> +        } else {
>> +            if (size == 1024 * 1024 && s->fat_type == 16) {
>> +                error_setg(errp,
>> +                           "floppy FAT16 unsupported size; only support 2M "
>> +                           "(for an effective size of 2.88 MB)");
>> +            } else if (size != 2 * 1024 * 1024 && size != 1024 * 1024) {
>> +                error_setg(errp,
>> +                           "floppy unsupported size; should be 1MB (for "
>> +                           "an effective size of 1.44 MB) or 2.88M (for "
>> +                           "2.88MB)");
>> +            }
>> +        }
>
> This is horrible. To be fair, it's pretty hard to do something not
> horrible when the usual units to describe floppy sizes are already
> horrible. :-)

Yes :)

> But I'd still like us to do better here.
>
> To me it looks a bit like what we really want is an enum for floppy
> sizes (though is there any real reason why we have only those two?), but
> an arbitrary size for hard disks.
>
> Without the enum, obviously, users could specify 1440k and that would do
> the right thing. Maybe special casing whatever 1.44M and 2.88M result
> in and translating them into 1440k and 2880k could be more justifiable
> than special casing 1M and 2M, but it would still be ugly.
>
> Markus, do you have any advice how this should be represented in QAPI?

Maybe, but first I'd like to understand what @size does.
Re: [PATCH 5/5] vvfat: add support for "size" options
Posted by Clément Chigot 3 months, 2 weeks ago
On Fri, Oct 24, 2025 at 10:35 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> Kevin Wolf <kwolf@redhat.com> writes:
>
> > Am 03.09.2025 um 09:57 hat Clément Chigot geschrieben:
> >> This allows more flexibility to vvfat backend. The value for "Number of
> >> Heads" and "Sectors per track" are based on SD specifications Part 2.
>
> This is too terse to remind me of how vvfat picks cylinders, heads, and
> sectors before this patch, so I need to go dig through the source code.
> I figure it depends on configuration parameters @floppy and @fat-type
> like this:
>
>     floppy  fat-type    cyls heads secs   cyls*heads*secs*512
>     false      12         64    16   63         31.5 MiB
>     false      16       1024    16   63        504   MiB
>     false      32       1024    16   63        504   MiB
>     true       12         80     2   18       1440   KiB
>     true       16         80     2   36       2880   KiB
>     true       32         80     2   36       2880   KiB
>
> How exactly does the new parameter @size change this?

My prime goal was to create a 256 Mib VVFAT disk. As you can see,
today for hard-disks there are only two possibilities: 31.5 Mib or 504
Mib. Hence, I've introduced the option `size=xxx` to allow more
granular choices.
This option changes how cyls, heads and secs parameters are computed
to be as closed as possible of its value.

I did try to keep it simple. I could have introduced options to select
cylinders, heads, etc. But I think "size=xxx" would be more intuitive.
There are also approximations made, as not all sizes can be reached. I
didn't add errors or warnings for them. I'm fine adding them.

> >> 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.
>
> 31.5MiB unless I'm mistaken.

True, I will fix it.

> I'm not sure what you're trying to convey in this paragraph.  As far as
> I can tell, you're adding a @size parameter to vvfat, so of course it
> doesn't affect raw.

Yes, but AFAICT, `if=sd,format=raw` will result in vvfat backend being
called. I didn't manage to make the new option work with
`if=sd,format=raw,size=256Mb`. Thus, when the "size" option is not
provided, I keep the previous value (those for your above comment).
Hence this paragraph to mostly warn people about the current
limitation.

> >> Moreover, for flopyy, size=1M is creating a disk 1.44 MB, and size=2M a
>
> floppy
>
> >> disk of 2.88 MB. This avoids having to worry about float operations.
>
> More on this part below.
>
> >> Signed-off-by: Clément Chigot <chigot@adacore.com>
> >> ---
> >>  block/vvfat.c | 165 ++++++++++++++++++++++++++++++++++++++++++--------
> >>  1 file changed, 141 insertions(+), 24 deletions(-)
> >>
> >> diff --git a/block/vvfat.c b/block/vvfat.c
> >> index 6526c585a2..4537c39d5c 100644
> >> --- a/block/vvfat.c
> >> +++ b/block/vvfat.c
> >> @@ -1091,6 +1091,11 @@ static QemuOptsList runtime_opts = {
> >>              .type = QEMU_OPT_BOOL,
> >>              .help = "Do not add a Master Boot Record on this disk",
> >>          },
> >> +        {
> >> +            .name = BLOCK_OPT_SIZE,
> >> +            .type = QEMU_OPT_SIZE,
> >> +            .help = "Virtual disk size"
> >> +        },
> >>          { /* end of list */ }
> >>      },
> >>  };
> >
> > Like in patch 1, you need additional changes, in particular to add the
> > option to the QAPI schema in qapi/block-core.json.
> >
> >> @@ -1148,10 +1153,141 @@ static void vvfat_parse_filename(const char *filename, QDict *options,
> >>      qdict_put_bool(options, "no-mbr", no_mbr);
> >>  }
> >>
> >> +static void vvfat_get_size_parameters(uint64_t size, BDRVVVFATState *s,
> >> +                                      bool floppy, Error **errp)
> >> +{
> >> +    if (floppy) {
> >> +        /*
> >> +         * Floppy emulation only supports 1.44 MB or 2.88 MB (default).
> >> +         * In order to avoid floating operations ambiguity, 1 MB is
> >> +         * recognized for 1.44 MB and 2 MB for 2.88 MB.
> >> +         */
> >> +        if (!size) {
> >> +            size = 2 * 1024 * 1024;
> >> +        } else {
> >> +            if (size == 1024 * 1024 && s->fat_type == 16) {
> >> +                error_setg(errp,
> >> +                           "floppy FAT16 unsupported size; only support 2M "
> >> +                           "(for an effective size of 2.88 MB)");
> >> +            } else if (size != 2 * 1024 * 1024 && size != 1024 * 1024) {
> >> +                error_setg(errp,
> >> +                           "floppy unsupported size; should be 1MB (for "
> >> +                           "an effective size of 1.44 MB) or 2.88M (for "
> >> +                           "2.88MB)");
> >> +            }
> >> +        }
> >
> > This is horrible. To be fair, it's pretty hard to do something not
> > horrible when the usual units to describe floppy sizes are already
> > horrible. :-)
>
> Yes :)

I did have a first version that ignored this new size option for
floppy. I did extend it because why not. But if you find it will bring
too much complexity I can bring it back.

> > But I'd still like us to do better here.
> >
> > To me it looks a bit like what we really want is an enum for floppy
> > sizes (though is there any real reason why we have only those two?), but
> > an arbitrary size for hard disks.
> >
> > Without the enum, obviously, users could specify 1440k and that would do
> > the right thing. Maybe special casing whatever 1.44M and 2.88M result
> > in and translating them into 1440k and 2880k could be more justifiable
> > than special casing 1M and 2M, but it would still be ugly.
> >
> > Markus, do you have any advice how this should be represented in QAPI?
>
> Maybe, but first I'd like to understand what @size does.
>
Re: [PATCH 5/5] vvfat: add support for "size" options
Posted by Markus Armbruster 3 months, 1 week ago
Clément Chigot <chigot@adacore.com> writes:

> On Fri, Oct 24, 2025 at 10:35 AM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Kevin Wolf <kwolf@redhat.com> writes:
>>
>> > Am 03.09.2025 um 09:57 hat Clément Chigot geschrieben:
>> >> This allows more flexibility to vvfat backend. The value for "Number of
>> >> Heads" and "Sectors per track" are based on SD specifications Part 2.
>>
>> This is too terse to remind me of how vvfat picks cylinders, heads, and
>> sectors before this patch, so I need to go dig through the source code.
>> I figure it depends on configuration parameters @floppy and @fat-type
>> like this:
>>
>>     floppy  fat-type    cyls heads secs   cyls*heads*secs*512
>>     false      12         64    16   63         31.5 MiB
>>     false      16       1024    16   63        504   MiB
>>     false      32       1024    16   63        504   MiB
>>     true       12         80     2   18       1440   KiB
>>     true       16         80     2   36       2880   KiB
>>     true       32         80     2   36       2880   KiB
>>
>> How exactly does the new parameter @size change this?
>
> My prime goal was to create a 256 Mib VVFAT disk. As you can see,
> today for hard-disks there are only two possibilities: 31.5 Mib or 504
> Mib. Hence, I've introduced the option `size=xxx` to allow more
> granular choices.
> This option changes how cyls, heads and secs parameters are computed
> to be as closed as possible of its value.
>
> I did try to keep it simple. I could have introduced options to select
> cylinders, heads, etc. But I think "size=xxx" would be more intuitive.
> There are also approximations made, as not all sizes can be reached. I
> didn't add errors or warnings for them. I'm fine adding them.

I don't have an opinion on whether we should support more sizes and/or
provide full control over CHS geometry.

>> >> 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.
>>
>> 31.5MiB unless I'm mistaken.
>
> True, I will fix it.
>
>> I'm not sure what you're trying to convey in this paragraph.  As far as
>> I can tell, you're adding a @size parameter to vvfat, so of course it
>> doesn't affect raw.
>
> Yes, but AFAICT, `if=sd,format=raw` will result in vvfat backend being
> called. I didn't manage to make the new option work with
> `if=sd,format=raw,size=256Mb`. Thus, when the "size" option is not
> provided, I keep the previous value (those for your above comment).
> Hence this paragraph to mostly warn people about the current
> limitation.

Are you talking about -drive?

Complete examples, please.

I'm confused about the connection between SD (from if=sd here, and "SD
specification" above) and vvfat.  SD is a frontend.  vvfat is a backend.

[...]
Re: [PATCH 5/5] vvfat: add support for "size" options
Posted by Clément Chigot 3 months, 1 week ago
On Mon, Oct 27, 2025 at 1:09 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> Clément Chigot <chigot@adacore.com> writes:
>
> > On Fri, Oct 24, 2025 at 10:35 AM Markus Armbruster <armbru@redhat.com> wrote:
> >>
> >> Kevin Wolf <kwolf@redhat.com> writes:
> >>
> >> > Am 03.09.2025 um 09:57 hat Clément Chigot geschrieben:
> >> >> This allows more flexibility to vvfat backend. The value for "Number of
> >> >> Heads" and "Sectors per track" are based on SD specifications Part 2.
> >>
> >> This is too terse to remind me of how vvfat picks cylinders, heads, and
> >> sectors before this patch, so I need to go dig through the source code.
> >> I figure it depends on configuration parameters @floppy and @fat-type
> >> like this:
> >>
> >>     floppy  fat-type    cyls heads secs   cyls*heads*secs*512
> >>     false      12         64    16   63         31.5 MiB
> >>     false      16       1024    16   63        504   MiB
> >>     false      32       1024    16   63        504   MiB
> >>     true       12         80     2   18       1440   KiB
> >>     true       16         80     2   36       2880   KiB
> >>     true       32         80     2   36       2880   KiB
> >>
> >> How exactly does the new parameter @size change this?
> >
> > My prime goal was to create a 256 Mib VVFAT disk. As you can see,
> > today for hard-disks there are only two possibilities: 31.5 Mib or 504
> > Mib. Hence, I've introduced the option `size=xxx` to allow more
> > granular choices.
> > This option changes how cyls, heads and secs parameters are computed
> > to be as closed as possible of its value.
> >
> > I did try to keep it simple. I could have introduced options to select
> > cylinders, heads, etc. But I think "size=xxx" would be more intuitive.
> > There are also approximations made, as not all sizes can be reached. I
> > didn't add errors or warnings for them. I'm fine adding them.
>
> I don't have an opinion on whether we should support more sizes and/or
> provide full control over CHS geometry.
>
> >> >> 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.
> >>
> >> 31.5MiB unless I'm mistaken.
> >
> > True, I will fix it.
> >
> >> I'm not sure what you're trying to convey in this paragraph.  As far as
> >> I can tell, you're adding a @size parameter to vvfat, so of course it
> >> doesn't affect raw.
> >
> > Yes, but AFAICT, `if=sd,format=raw` will result in vvfat backend being
> > called. I didn't manage to make the new option work with
> > `if=sd,format=raw,size=256Mb`. Thus, when the "size" option is not
> > provided, I keep the previous value (those for your above comment).
> > Hence this paragraph to mostly warn people about the current
> > limitation.
>
> Are you talking about -drive?
>
> Complete examples, please.
>
> I'm confused about the connection between SD (from if=sd here, and "SD
> specification" above) and vvfat.  SD is a frontend.  vvfat is a backend.

Alright, I'll try to explain how I came up with this patch. And sorry
if it's a bit blurry, I made it some months ago hence I don't remember
all the details...
So, first, my prime goal was to access a local folder in a QNX system
running on Raspi 4B emulation.
My usual way to pass such a local folder is through `-drive
file=fat:rw:<host_folder>,format=raw`. For virt, it's usually
connected to virtio-blk-device: `-drive id=disk0,if=none,... -device
virtio-blk-device,drive=disk0`. For the Raspi 4b, adding the
virtio-blk-device is not possible, hence I have to connect it as a SD
card: `-drive if=sd,...`.

However, without any `size=` argument, QEMU will complain that the SD
card has not a valid size:
  | (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=raw,if=sd
  | qemu-system-aarch64: Invalid SD card size: 504 MiB
  | SD card size has to be a power of 2, e.g. 512 MiB.
  | You can resize disk images with 'qemu-img resize <imagefile> <new-size>'
  | (note that this will lose data if you make the image smaller than
it currently is).

("raspi4b-kernel", the dtb and the rootfs come from
functional/aarch64/test_raspi4.py)

Hence, I've added `size=256M` to reduce the size of that SD card and
make QEMU happy. This allows me to mount my host folder on Linux:
  | (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=raw,if=sd,size=256M
  | (QEMU) # fdisk -l /dev/mmcblk1
  | Disk /dev/mmcblk1: 256 MB, 268435456 bytes, 524288 sectors
  | 520 cylinders, 16 heads, 63 sectors/track
  | Units: sectors of 1 * 512 = 512 bytes
  |
  | Device       Boot StartCHS    EndCHS        StartLBA     EndLBA
Sectors  Size Id Type
  | /dev/mmcblk1p1 *  0,1,1       1023,15,63          63    1032191
1032129  503M  6 FAT16

As you can see the "Disk" has the right size (256MB) but the partition
still has a 503M size. However, Linux doesn't seem to care too much
about that as I'm able to mount this partition, and perform IO
operations.
  | (QEMU) # mount /dev/mmcblk1p1 /mnt
  | (QEMU) # ls /mnt
  | file.txt
  | (QEMU) # cat /mnt/file.txt
  | Hello World
  | (QEMU) # echo "OK" > /mnt/test.txt
  | (host) $ cat <host_folder>/test.txt
  | OK

Now, QNX comes into play.
First, the SD card must be connected to another bus. That patch is not
part of this series as I'm considering it a QNX issue. Just FTR here
is the patch:
  | --- a/hw/arm/bcm2838_peripherals.c
  | +++ b/hw/arm/bcm2838_peripherals.c
  | @@ -190,7 +190,7 @@ static void
bcm2838_peripherals_realize(DeviceState *dev, Error **errp)
  |          &s_base->peri_mr, GPIO_OFFSET,
  |          sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->gpio), 0));
  |
  | -    object_property_add_alias(OBJECT(s), "sd-bus",
OBJECT(&s->gpio), "sd-bus");
  | +    object_property_add_alias(OBJECT(s), "sd-bus",
OBJECT(&s->emmc2), "sd-bus");
  |
  |      /* BCM2838 RPiVid ASB must be mapped to prevent kernel crash */

Afterwards, QNX is able to see the SD card, but not able to mount it.
It complains about the filesystem being corrupted. Looking at `fdisk`
output, it shows a mismatch between the "total section" value and the
Cylinders/Heads/etc values.
  | (QEMU) # fdisk /dev/hd0 info
  | Physical disk characteristics: (/dev/hd0)
  |     Disk type        : Direct Access (0)
  |     Cylinders        : 520
  |     Heads            : 16
  |     Sectors/Track    : 63
  |     Total Sectors    : 524288
  |     Block Size       : 512
  |
  | Warning: total sectors field does not agree with
  |            cylinders*sectors/track*heads!! (524288 vs 524160)

The "no-mbr" option introduced in patch 1 is something we (Adacore's
QEMU team) have for a long time. I don't remember the details but we
are using it for other OSes as well (notably RTEMS).
Once added, and the drive command line updated for `-drive
id=sdcard,file=fat:rw:no-mbr:<host_folder>,format=raw,if=sd,size=256M`,
I don't have this warning anymore. Though, I'm still getting the
corrupted filesystem error.

Afterwards, it's a bit blurry but I think by trial and errors we ended
up removing the SD size error and realize that `-drive
id=sdcard,file=fat:rw:no-mbr:<host_folder>,format=raw,if=sd` was
working. However, `size=256M` still results in a corrupted filesystem.
As a comment in vvfat.c states that it either creates a "32MB or 504
MB disk". I decided to check if I can adapt, hence this patch.

I didn't find any VFAT documentation explaining the relation between
the size and the cylinders, heads, sector per track values. However,
the SD documentation was giving some recommandations, hence I used it
as a base.

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.

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

I probably missed a few things, but I hope this is clearer to you why
and how this series was made.
Re: [PATCH 5/5] vvfat: add support for "size" options
Posted by Markus Armbruster 3 months, 1 week ago
Clément Chigot <chigot@adacore.com> writes:

> On Mon, Oct 27, 2025 at 1:09 PM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Clément Chigot <chigot@adacore.com> writes:
>>
>> > On Fri, Oct 24, 2025 at 10:35 AM Markus Armbruster <armbru@redhat.com> wrote:
>> >>
>> >> Kevin Wolf <kwolf@redhat.com> writes:
>> >>
>> >> > Am 03.09.2025 um 09:57 hat Clément Chigot geschrieben:
>> >> >> This allows more flexibility to vvfat backend. The value for "Number of
>> >> >> Heads" and "Sectors per track" are based on SD specifications Part 2.
>> >>
>> >> This is too terse to remind me of how vvfat picks cylinders, heads, and
>> >> sectors before this patch, so I need to go dig through the source code.
>> >> I figure it depends on configuration parameters @floppy and @fat-type
>> >> like this:
>> >>
>> >>     floppy  fat-type    cyls heads secs   cyls*heads*secs*512
>> >>     false      12         64    16   63         31.5 MiB
>> >>     false      16       1024    16   63        504   MiB
>> >>     false      32       1024    16   63        504   MiB
>> >>     true       12         80     2   18       1440   KiB
>> >>     true       16         80     2   36       2880   KiB
>> >>     true       32         80     2   36       2880   KiB
>> >>
>> >> How exactly does the new parameter @size change this?
>> >
>> > My prime goal was to create a 256 Mib VVFAT disk. As you can see,
>> > today for hard-disks there are only two possibilities: 31.5 Mib or 504
>> > Mib. Hence, I've introduced the option `size=xxx` to allow more
>> > granular choices.
>> > This option changes how cyls, heads and secs parameters are computed
>> > to be as closed as possible of its value.
>> >
>> > I did try to keep it simple. I could have introduced options to select
>> > cylinders, heads, etc. But I think "size=xxx" would be more intuitive.
>> > There are also approximations made, as not all sizes can be reached. I
>> > didn't add errors or warnings for them. I'm fine adding them.
>>
>> I don't have an opinion on whether we should support more sizes and/or
>> provide full control over CHS geometry.
>>
>> >> >> 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.
>> >>
>> >> 31.5MiB unless I'm mistaken.
>> >
>> > True, I will fix it.
>> >
>> >> I'm not sure what you're trying to convey in this paragraph.  As far as
>> >> I can tell, you're adding a @size parameter to vvfat, so of course it
>> >> doesn't affect raw.
>> >
>> > Yes, but AFAICT, `if=sd,format=raw` will result in vvfat backend being
>> > called. I didn't manage to make the new option work with
>> > `if=sd,format=raw,size=256Mb`. Thus, when the "size" option is not
>> > provided, I keep the previous value (those for your above comment).
>> > Hence this paragraph to mostly warn people about the current
>> > limitation.
>>
>> Are you talking about -drive?
>>
>> Complete examples, please.
>>
>> I'm confused about the connection between SD (from if=sd here, and "SD
>> specification" above) and vvfat.  SD is a frontend.  vvfat is a backend.
>
> Alright, I'll try to explain how I came up with this patch. And sorry
> if it's a bit blurry, I made it some months ago hence I don't remember
> all the details...
> So, first, my prime goal was to access a local folder in a QNX system
> running on Raspi 4B emulation.
> My usual way to pass such a local folder is through `-drive
> file=fat:rw:<host_folder>,format=raw`. For virt, it's usually
> connected to virtio-blk-device: `-drive id=disk0,if=none,... -device
> virtio-blk-device,drive=disk0`. For the Raspi 4b, adding the
> virtio-blk-device is not possible, hence I have to connect it as a SD
> card: `-drive if=sd,...`.
>
> However, without any `size=` argument, QEMU will complain that the SD
> card has not a valid size:
>   | (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=raw,if=sd
>   | qemu-system-aarch64: Invalid SD card size: 504 MiB
>   | SD card size has to be a power of 2, e.g. 512 MiB.
>   | You can resize disk images with 'qemu-img resize <imagefile> <new-size>'
>   | (note that this will lose data if you make the image smaller than
> it currently is).

Fun!

> ("raspi4b-kernel", the dtb and the rootfs come from
> functional/aarch64/test_raspi4.py)
>
> Hence, I've added `size=256M` to reduce the size of that SD card and
> make QEMU happy. This allows me to mount my host folder on Linux:
>   | (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=raw,if=sd,size=256M
>   | (QEMU) # fdisk -l /dev/mmcblk1
>   | Disk /dev/mmcblk1: 256 MB, 268435456 bytes, 524288 sectors
>   | 520 cylinders, 16 heads, 63 sectors/track
>   | Units: sectors of 1 * 512 = 512 bytes

520 * 16 * 63 is 524160 sectors, 128 less than the 524288 reported.  I
figure that's harmless.  Only ancient software should look at CHS, and
losing a few sectors with ancient software is fine.

>   |
>   | Device       Boot StartCHS    EndCHS        StartLBA     EndLBA
> Sectors  Size Id Type
>   | /dev/mmcblk1p1 *  0,1,1       1023,15,63          63    1032191
> 1032129  503M  6 FAT16
>
> As you can see the "Disk" has the right size (256MB) but the partition
> still has a 503M size.

The partition table's EndLBA is 1032191 even though the disk has only
524288 cylinders.  Scary!

Its EndCHS is consistent with its EndLBA: 1024*16*63 = 1032191 + 1.

>                        However, Linux doesn't seem to care too much
> about that as I'm able to mount this partition, and perform IO
> operations.
>   | (QEMU) # mount /dev/mmcblk1p1 /mnt
>   | (QEMU) # ls /mnt
>   | file.txt
>   | (QEMU) # cat /mnt/file.txt
>   | Hello World
>   | (QEMU) # echo "OK" > /mnt/test.txt
>   | (host) $ cat <host_folder>/test.txt
>   | OK

Have you tried with a host folder containing more than 256MiB?  What
happens if you try to read all of it?

> Now, QNX comes into play.
> First, the SD card must be connected to another bus. That patch is not
> part of this series as I'm considering it a QNX issue. Just FTR here
> is the patch:
>   | --- a/hw/arm/bcm2838_peripherals.c
>   | +++ b/hw/arm/bcm2838_peripherals.c
>   | @@ -190,7 +190,7 @@ static void
> bcm2838_peripherals_realize(DeviceState *dev, Error **errp)
>   |          &s_base->peri_mr, GPIO_OFFSET,
>   |          sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->gpio), 0));
>   |
>   | -    object_property_add_alias(OBJECT(s), "sd-bus",
> OBJECT(&s->gpio), "sd-bus");
>   | +    object_property_add_alias(OBJECT(s), "sd-bus",
> OBJECT(&s->emmc2), "sd-bus");
>   |
>   |      /* BCM2838 RPiVid ASB must be mapped to prevent kernel crash */
>
> Afterwards, QNX is able to see the SD card, but not able to mount it.
> It complains about the filesystem being corrupted. Looking at `fdisk`
> output, it shows a mismatch between the "total section" value and the
> Cylinders/Heads/etc values.
>   | (QEMU) # fdisk /dev/hd0 info
>   | Physical disk characteristics: (/dev/hd0)
>   |     Disk type        : Direct Access (0)
>   |     Cylinders        : 520
>   |     Heads            : 16
>   |     Sectors/Track    : 63
>   |     Total Sectors    : 524288
>   |     Block Size       : 512
>   |
>   | Warning: total sectors field does not agree with
>   |            cylinders*sectors/track*heads!! (524288 vs 524160)

I'm not sure this mismatch causes the problem.  I suspect the bogus
EndLBA does.

> The "no-mbr" option introduced in patch 1 is something we (Adacore's
> QEMU team) have for a long time. I don't remember the details but we
> are using it for other OSes as well (notably RTEMS).

Suppressing MBR initialization avoids the partially bogus partition
table.

But if we create a partition table, it better make sense, don't you
think?

> Once added, and the drive command line updated for `-drive
> id=sdcard,file=fat:rw:no-mbr:<host_folder>,format=raw,if=sd,size=256M`,
> I don't have this warning anymore. Though, I'm still getting the
> corrupted filesystem error.
>
> Afterwards, it's a bit blurry but I think by trial and errors we ended
> up removing the SD size error and realize that `-drive
> id=sdcard,file=fat:rw:no-mbr:<host_folder>,format=raw,if=sd` was
> working. However, `size=256M` still results in a corrupted filesystem.
> As a comment in vvfat.c states that it either creates a "32MB or 504
> MB disk". I decided to check if I can adapt, hence this patch.
>
> I didn't find any VFAT documentation explaining the relation between
> the size and the cylinders, heads, sector per track values. However,
> the SD documentation was giving some recommandations, hence I used it
> as a base.
>
> 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'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?

> I probably missed a few things, but I hope this is clearer to you why
> and how this series was made.
Re: [PATCH 5/5] vvfat: add support for "size" options
Posted by Clément Chigot 3 months, 1 week ago
On Fri, Oct 31, 2025 at 8:46 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> Clément Chigot <chigot@adacore.com> writes:
>
> > On Mon, Oct 27, 2025 at 1:09 PM Markus Armbruster <armbru@redhat.com> wrote:
> >>
> >> Clément Chigot <chigot@adacore.com> writes:
> >>
> >> > On Fri, Oct 24, 2025 at 10:35 AM Markus Armbruster <armbru@redhat.com> wrote:
> >> >>
> >> >> Kevin Wolf <kwolf@redhat.com> writes:
> >> >>
> >> >> > Am 03.09.2025 um 09:57 hat Clément Chigot geschrieben:
> >> >> >> This allows more flexibility to vvfat backend. The value for "Number of
> >> >> >> Heads" and "Sectors per track" are based on SD specifications Part 2.
> >> >>
> >> >> This is too terse to remind me of how vvfat picks cylinders, heads, and
> >> >> sectors before this patch, so I need to go dig through the source code.
> >> >> I figure it depends on configuration parameters @floppy and @fat-type
> >> >> like this:
> >> >>
> >> >>     floppy  fat-type    cyls heads secs   cyls*heads*secs*512
> >> >>     false      12         64    16   63         31.5 MiB
> >> >>     false      16       1024    16   63        504   MiB
> >> >>     false      32       1024    16   63        504   MiB
> >> >>     true       12         80     2   18       1440   KiB
> >> >>     true       16         80     2   36       2880   KiB
> >> >>     true       32         80     2   36       2880   KiB
> >> >>
> >> >> How exactly does the new parameter @size change this?
> >> >
> >> > My prime goal was to create a 256 Mib VVFAT disk. As you can see,
> >> > today for hard-disks there are only two possibilities: 31.5 Mib or 504
> >> > Mib. Hence, I've introduced the option `size=xxx` to allow more
> >> > granular choices.
> >> > This option changes how cyls, heads and secs parameters are computed
> >> > to be as closed as possible of its value.
> >> >
> >> > I did try to keep it simple. I could have introduced options to select
> >> > cylinders, heads, etc. But I think "size=xxx" would be more intuitive.
> >> > There are also approximations made, as not all sizes can be reached. I
> >> > didn't add errors or warnings for them. I'm fine adding them.
> >>
> >> I don't have an opinion on whether we should support more sizes and/or
> >> provide full control over CHS geometry.
> >>
> >> >> >> 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.
> >> >>
> >> >> 31.5MiB unless I'm mistaken.
> >> >
> >> > True, I will fix it.
> >> >
> >> >> I'm not sure what you're trying to convey in this paragraph.  As far as
> >> >> I can tell, you're adding a @size parameter to vvfat, so of course it
> >> >> doesn't affect raw.
> >> >
> >> > Yes, but AFAICT, `if=sd,format=raw` will result in vvfat backend being
> >> > called. I didn't manage to make the new option work with
> >> > `if=sd,format=raw,size=256Mb`. Thus, when the "size" option is not
> >> > provided, I keep the previous value (those for your above comment).
> >> > Hence this paragraph to mostly warn people about the current
> >> > limitation.
> >>
> >> Are you talking about -drive?
> >>
> >> Complete examples, please.
> >>
> >> I'm confused about the connection between SD (from if=sd here, and "SD
> >> specification" above) and vvfat.  SD is a frontend.  vvfat is a backend.
> >
> > Alright, I'll try to explain how I came up with this patch. And sorry
> > if it's a bit blurry, I made it some months ago hence I don't remember
> > all the details...
> > So, first, my prime goal was to access a local folder in a QNX system
> > running on Raspi 4B emulation.
> > My usual way to pass such a local folder is through `-drive
> > file=fat:rw:<host_folder>,format=raw`. For virt, it's usually
> > connected to virtio-blk-device: `-drive id=disk0,if=none,... -device
> > virtio-blk-device,drive=disk0`. For the Raspi 4b, adding the
> > virtio-blk-device is not possible, hence I have to connect it as a SD
> > card: `-drive if=sd,...`.
> >
> > However, without any `size=` argument, QEMU will complain that the SD
> > card has not a valid size:
> >   | (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=raw,if=sd
> >   | qemu-system-aarch64: Invalid SD card size: 504 MiB
> >   | SD card size has to be a power of 2, e.g. 512 MiB.
> >   | You can resize disk images with 'qemu-img resize <imagefile> <new-size>'
> >   | (note that this will lose data if you make the image smaller than
> > it currently is).
>
> Fun!
>
> > ("raspi4b-kernel", the dtb and the rootfs come from
> > functional/aarch64/test_raspi4.py)
> >
> > Hence, I've added `size=256M` to reduce the size of that SD card and
> > make QEMU happy. This allows me to mount my host folder on Linux:
> >   | (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=raw,if=sd,size=256M
> >   | (QEMU) # fdisk -l /dev/mmcblk1
> >   | Disk /dev/mmcblk1: 256 MB, 268435456 bytes, 524288 sectors
> >   | 520 cylinders, 16 heads, 63 sectors/track
> >   | Units: sectors of 1 * 512 = 512 bytes
>
> 520 * 16 * 63 is 524160 sectors, 128 less than the 524288 reported.  I
> figure that's harmless.  Only ancient software should look at CHS, and
> losing a few sectors with ancient software is fine.
>
> >   |
> >   | Device       Boot StartCHS    EndCHS        StartLBA     EndLBA
> > Sectors  Size Id Type
> >   | /dev/mmcblk1p1 *  0,1,1       1023,15,63          63    1032191
> > 1032129  503M  6 FAT16
> >
> > As you can see the "Disk" has the right size (256MB) but the partition
> > still has a 503M size.
>
> The partition table's EndLBA is 1032191 even though the disk has only
> 524288 cylinders.  Scary!
>
> Its EndCHS is consistent with its EndLBA: 1024*16*63 = 1032191 + 1.
>
> >                        However, Linux doesn't seem to care too much
> > about that as I'm able to mount this partition, and perform IO
> > operations.
> >   | (QEMU) # mount /dev/mmcblk1p1 /mnt
> >   | (QEMU) # ls /mnt
> >   | file.txt
> >   | (QEMU) # cat /mnt/file.txt
> >   | Hello World
> >   | (QEMU) # echo "OK" > /mnt/test.txt
> >   | (host) $ cat <host_folder>/test.txt
> >   | OK
>
> Have you tried with a host folder containing more than 256MiB?  What
> happens if you try to read all of it?

The access is crashing:
  | (host) $ du -sh vvfat-test/huge_file
  | 251M vvfat-test/huge_file
  | (host) $ qemu-system ... -drive
file=fat:rw:vvfat-test,format=raw,if=sd,size=128M
  | (QEMU) # tail /mnt/huge_file
  | [   29.325885] attempt to access beyond end of device
  | [   29.325885] mmcblk1p1: rw=524288, want=510657, limit=262081
  | [   29.337672] attempt to access beyond end of device
  | [   29.337672] mmcblk1p1: rw=0, want=510657, limit=262081
  | tail: read error: I/O error


> > Now, QNX comes into play.
> > First, the SD card must be connected to another bus. That patch is not
> > part of this series as I'm considering it a QNX issue. Just FTR here
> > is the patch:
> >   | --- a/hw/arm/bcm2838_peripherals.c
> >   | +++ b/hw/arm/bcm2838_peripherals.c
> >   | @@ -190,7 +190,7 @@ static void
> > bcm2838_peripherals_realize(DeviceState *dev, Error **errp)
> >   |          &s_base->peri_mr, GPIO_OFFSET,
> >   |          sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->gpio), 0));
> >   |
> >   | -    object_property_add_alias(OBJECT(s), "sd-bus",
> > OBJECT(&s->gpio), "sd-bus");
> >   | +    object_property_add_alias(OBJECT(s), "sd-bus",
> > OBJECT(&s->emmc2), "sd-bus");
> >   |
> >   |      /* BCM2838 RPiVid ASB must be mapped to prevent kernel crash */
> >
> > Afterwards, QNX is able to see the SD card, but not able to mount it.
> > It complains about the filesystem being corrupted. Looking at `fdisk`
> > output, it shows a mismatch between the "total section" value and the
> > Cylinders/Heads/etc values.
> >   | (QEMU) # fdisk /dev/hd0 info
> >   | Physical disk characteristics: (/dev/hd0)
> >   |     Disk type        : Direct Access (0)
> >   |     Cylinders        : 520
> >   |     Heads            : 16
> >   |     Sectors/Track    : 63
> >   |     Total Sectors    : 524288
> >   |     Block Size       : 512
> >   |
> >   | Warning: total sectors field does not agree with
> >   |            cylinders*sectors/track*heads!! (524288 vs 524160)
>
> I'm not sure this mismatch causes the problem.  I suspect the bogus
> EndLBA does.
>
> > The "no-mbr" option introduced in patch 1 is something we (Adacore's
> > QEMU team) have for a long time. I don't remember the details but we
> > are using it for other OSes as well (notably RTEMS).
>
> Suppressing MBR initialization avoids the partially bogus partition
> table.
>
> But if we create a partition table, it better make sense, don't you
> think?

My understanding is that `format=vvfat,size=xxxM` results in a valid
EndLBA. Is that enough to make a valid partition table ?
However, I'm still getting the corrupted error without `no-mbr`.
Though, the warning is gone too (compared to `format=raw,size=xxx`).
So this is something else QNX doesn't like...

Something weird if just noticed, the disk CHS values are changed when
passing `no-mbr`
  | (host) $ qemu-system-aarch64 -drive
file=fat:rw:<host_folder>:format=vvfat,size=128M
  | (QEMU) # fdisk -l /dev/mmcblk1
  | Disk /dev/mmcblk1: 128 MB, 134217728 bytes, 262144 sectors
  | 1024 cylinders, 8 heads, 32 sectors/track
  |
  | (host) $ qemu-system-aarch64 -drive
file=fat:rw:no-mbr:<host_folder>:format=vvfat,size=128M
  | (QEMU) # fdisk -l /dev/mmcblk1
  | Disk /dev/mmcblk1: 128 MB, 134217728 bytes, 262144 sectors
  | 4096 cylinders, 4 heads, 16 sectors/track

Not sure if it could be related.

> > Once added, and the drive command line updated for `-drive
> > id=sdcard,file=fat:rw:no-mbr:<host_folder>,format=raw,if=sd,size=256M`,
> > I don't have this warning anymore. Though, I'm still getting the
> > corrupted filesystem error.
> >
> > Afterwards, it's a bit blurry but I think by trial and errors we ended
> > up removing the SD size error and realize that `-drive
> > id=sdcard,file=fat:rw:no-mbr:<host_folder>,format=raw,if=sd` was
> > working. However, `size=256M` still results in a corrupted filesystem.
> > As a comment in vvfat.c states that it either creates a "32MB or 504
> > MB disk". I decided to check if I can adapt, hence this patch.
> >
> > I didn't find any VFAT documentation explaining the relation between
> > the size and the cylinders, heads, sector per track values. However,
> > the SD documentation was giving some recommandations, hence I used it
> > as a base.
> >
> > 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'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.

> > I probably missed a few things, but I hope this is clearer to you why
> > and how this series was made.
>
Re: [PATCH 5/5] vvfat: add support for "size" options
Posted by Kevin Wolf 3 months, 1 week ago
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.

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

Kevin
Re: [PATCH 5/5] vvfat: add support for "size" options
Posted by Clément Chigot 3 months, 1 week ago
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
>
Re: [PATCH 5/5] vvfat: add support for "size" options
Posted by Markus Armbruster 3 months ago
Clément Chigot <chigot@adacore.com> writes:

> On Fri, Oct 31, 2025 at 12:57 PM Kevin Wolf <kwolf@redhat.com> wrote:

[...]

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

Desirable, because it's simpler.

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

Format "raw" was designed to do nothing, so we have a "do nothing"
format for the rigid "format over protocol" system.

We've long acquired the means to use a protocol without a format.  This
made "raw" redundant.  I advocate omitting it, because it only
complicates matters.

Except when you want the one feature "raw" provides beyond "do nothing":
carve a slice with options offset and size (commit 2fdc70452a5 "raw_bsd:
add offset and size options", 2016).

>                              But IIUC, this would be a much more
> complex patch (i.e. changing bdrv_open logic).

"size" is for slicing.  Can't see how it could simultaneously be
forwarded to the next block driver.

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