[PATCH v3 2/5] vvfat: move fat_type check prior to size setup

Clément Chigot posted 5 patches 1 day, 19 hours ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
[PATCH v3 2/5] vvfat: move fat_type check prior to size setup
Posted by Clément Chigot 1 day, 19 hours ago
This allows to handle the default FAT size in a single place and make the
following part taking care only about size parameters. It will be later
moved away in a specific function.

The selection of floppy size was a bit unusual:
 - fat-type undefined: a FAT 12 2880 Kib disk (default)
 - fat-type=16: a FAT 16 2880 Kib disk
 - fat-type=12: a FAT 12 1440 Kib disk

Now, that fat-type undefined means fat-type=12, it's no longer possible
to make that size distinction. Therefore, it's being changed for the
following:
 - fat-type=12: a FAT 12 1440 Kib disk (default)
 - fat-type=16: a FAT 16 2880 Kib dis

This has been choosen for two reasons: keep fat-type=12 the default and
create a more usual size for it: 1440 Kib.

The possibility to create a FAT 12 2880 KiB floppy will be added back
later, through the fs-size parameter.

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

diff --git a/block/vvfat.c b/block/vvfat.c
index dd0b3689c1..baf04e678b 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1188,28 +1188,8 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
         memcpy(s->volume_label, "QEMU VVFAT", 10);
     }
 
-    if (floppy) {
-        /* 1.44MB or 2.88MB floppy.  2.88MB can be FAT12 (default) or FAT16. */
-        if (!s->fat_type) {
-            s->fat_type = 12;
-            secs = 36;
-            s->sectors_per_cluster = 2;
-        } else {
-            secs = s->fat_type == 12 ? 18 : 36;
-            s->sectors_per_cluster = 1;
-        }
-        cyls = 80;
-        heads = 2;
-    } else {
-        /* 32MB or 504MB disk*/
-        if (!s->fat_type) {
-            s->fat_type = 16;
-        }
-        cyls = s->fat_type == 12 ? 64 : 1024;
-        heads = 16;
-        secs = 63;
-    }
 
+    /* Verify FAT type  */
     switch (s->fat_type) {
     case 32:
         warn_report("FAT32 has not been tested. You are welcome to do so!");
@@ -1217,12 +1197,33 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
     case 16:
     case 12:
         break;
+    case 0:
+        /* Set a default type */
+        if (floppy) {
+            s->fat_type = 12;
+        } else {
+            s->fat_type = 16;
+        }
+        break;
     default:
         error_setg(errp, "Valid FAT types are only 12, 16 and 32");
         ret = -EINVAL;
         goto fail;
     }
 
+
+    if (floppy) {
+        /* Choose floppy size. 1440 KiB for FAT 12, 2880 KiB for FAT-16 */
+        secs = s->fat_type == 12 ? 18 : 36;
+        cyls = 80;
+        heads = 2;
+    } else {
+        /* 32MB or 504MB disk*/
+        cyls = s->fat_type == 12 ? 64 : 1024;
+        heads = 16;
+        secs = 63;
+    }
+
     /* Reserver space for MBR */
     if (partitioned) {
         s->offset_to_bootsector = 0x3f;
-- 
2.43.0


Re: [PATCH v3 2/5] vvfat: move fat_type check prior to size setup
Posted by BALATON Zoltan 1 day, 19 hours ago
On Thu, 27 Nov 2025, Clément Chigot wrote:
> This allows to handle the default FAT size in a single place and make the
> following part taking care only about size parameters. It will be later
> moved away in a specific function.
>
> The selection of floppy size was a bit unusual:
> - fat-type undefined: a FAT 12 2880 Kib disk (default)
> - fat-type=16: a FAT 16 2880 Kib disk
> - fat-type=12: a FAT 12 1440 Kib disk
>
> Now, that fat-type undefined means fat-type=12, it's no longer possible
> to make that size distinction. Therefore, it's being changed for the
> following:
> - fat-type=12: a FAT 12 1440 Kib disk (default)
> - fat-type=16: a FAT 16 2880 Kib dis
>
> This has been choosen for two reasons: keep fat-type=12 the default and
> create a more usual size for it: 1440 Kib.
>
> The possibility to create a FAT 12 2880 KiB floppy will be added back
> later, through the fs-size parameter.
>
> Signed-off-by: Clément Chigot <chigot@adacore.com>
> ---
> block/vvfat.c | 43 ++++++++++++++++++++++---------------------
> 1 file changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/block/vvfat.c b/block/vvfat.c
> index dd0b3689c1..baf04e678b 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -1188,28 +1188,8 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
>         memcpy(s->volume_label, "QEMU VVFAT", 10);
>     }
>
> -    if (floppy) {
> -        /* 1.44MB or 2.88MB floppy.  2.88MB can be FAT12 (default) or FAT16. */
> -        if (!s->fat_type) {
> -            s->fat_type = 12;
> -            secs = 36;
> -            s->sectors_per_cluster = 2;
> -        } else {
> -            secs = s->fat_type == 12 ? 18 : 36;
> -            s->sectors_per_cluster = 1;
> -        }
> -        cyls = 80;
> -        heads = 2;
> -    } else {
> -        /* 32MB or 504MB disk*/
> -        if (!s->fat_type) {
> -            s->fat_type = 16;
> -        }
> -        cyls = s->fat_type == 12 ? 64 : 1024;
> -        heads = 16;
> -        secs = 63;
> -    }
>
> +    /* Verify FAT type  */
>     switch (s->fat_type) {
>     case 32:
>         warn_report("FAT32 has not been tested. You are welcome to do so!");
> @@ -1217,12 +1197,33 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
>     case 16:
>     case 12:
>         break;
> +    case 0:
> +        /* Set a default type */
> +        if (floppy) {
> +            s->fat_type = 12;
> +        } else {
> +            s->fat_type = 16;
> +        }
> +        break;
>     default:
>         error_setg(errp, "Valid FAT types are only 12, 16 and 32");
>         ret = -EINVAL;
>         goto fail;
>     }
>
> +
> +    if (floppy) {
> +        /* Choose floppy size. 1440 KiB for FAT 12, 2880 KiB for FAT-16 */
> +        secs = s->fat_type == 12 ? 18 : 36;
> +        cyls = 80;
> +        heads = 2;
> +    } else {
> +        /* 32MB or 504MB disk*/
> +        cyls = s->fat_type == 12 ? 64 : 1024;
> +        heads = 16;
> +        secs = 63;
> +    }
> +
>     /* Reserver space for MBR */

Previous patch but only noticed here. This may be correct in French but in 
English to many r-s in Reserve.

Regards,
BALATON Zoltan

>     if (partitioned) {
>         s->offset_to_bootsector = 0x3f;
>