Re: [PATCH v3 1/5] vvfat: introduce partitioned option

[BALATON Zoltan]

On Tue, 9 Dec 2025, Clément Chigot wrote:
On Thu, Nov 27, 2025 at 3:40 PM BALATON Zoltan <bala...@eik.bme.hu> 
wrote:

On Thu, 27 Nov 2025, Clément Chigot wrote:
This option tells whether the volume should be partitioned or not. Its
default varies: true for hard disks and false for floppy. Its prime
effect is to prevent a master boot record (MBR) to be initialized.

This is useful as some operating system (QNX, Rtems) don't
recognized FAT mounted disks (especially SD cards) if a MBR is present.

Signed-off-by: Clément Chigot <chi...@adacore.com>
---
block/vvfat.c        | 19 ++++++++++++++++---
qapi/block-core.json | 10 +++++++---
2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index 814796d918..dd0b3689c1 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -306,7 +306,8 @@ typedef struct BDRVVVFATState {
    array_t fat,directory,mapping;
    char volume_label[11];

-    uint32_t offset_to_bootsector; /* 0 for floppy, 0x3f for disk */
+    /* 0x3f for partitioned disk, 0x0 otherwise */
+    uint32_t offset_to_bootsector;

    unsigned int cluster_size;
    unsigned int sectors_per_cluster;
@@ -1082,6 +1083,11 @@ static QemuOptsList runtime_opts = {
            .type = QEMU_OPT_BOOL,
            .help = "Make the image writable",
        },
+        {
+            .name = "partitioned",
+            .type = QEMU_OPT_BOOL,
+            .help = "Do not add a Master Boot Record on this disk",

Maybe should say "Add MBR to disk" and not "Do not add" as that's what
true means now.

+        },
        { /* end of list */ }
    },
};
@@ -1138,7 +1144,7 @@ static int vvfat_open(BlockDriverState *bs, QDict 
*options, int flags,
{
    BDRVVVFATState *s = bs->opaque;
    int cyls, heads, secs;
-    bool floppy;
+    bool floppy, partitioned;
    const char *dirname, *label;
    QemuOpts *opts;
    int ret;
@@ -1165,6 +1171,9 @@ 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);

+    /* Hard disk are partitioned by default; floppy aren't.  */

Singular plural mismatch. Either disks/floppies are/aren't or is/isn't
when singular.

+    partitioned = qemu_opt_get_bool(opts, "partitioned", floppy ? false : 
true);
+
    memset(s->volume_label, ' ', sizeof(s->volume_label));
    label = qemu_opt_get(opts, "label");
    if (label) {
@@ -1196,7 +1205,6 @@ static int vvfat_open(BlockDriverState *bs, QDict 
*options, int flags,
        if (!s->fat_type) {
            s->fat_type = 16;
        }
-        s->offset_to_bootsector = 0x3f;
        cyls = s->fat_type == 12 ? 64 : 1024;
        heads = 16;
        secs = 63;
@@ -1215,6 +1223,10 @@ static int vvfat_open(BlockDriverState *bs, QDict 
*options, int flags,
        goto fail;
    }

+    /* Reserver space for MBR */
+    if (partitioned) {
+        s->offset_to_bootsector = 0x3f;
+    }

    s->bs = bs;

@@ -3246,6 +3258,7 @@ static const char *const vvfat_strong_runtime_opts[] = {
    "floppy",
    "label",
    "rw",
+    "partitioned",

Does this also needs to be parsed in vvfat_parse_filename like other
options seem to be?

It used to be the case, but I've removed that possibility in v3. This
"vvfat_parse_filename" looks like a "hack" to ease setting vvfat
options when instantiating such blocks through the format=raw.

If those aren't deprecated then I'd call it convenience than a hack.

However, options can still be passed using "file." prefix. For
example, "format=raw,file.partitioned=false" will create an
unpartitioned vvfat block.
Overall, I think avoiding new options in the filename is a better
cleaner approach.

If options are possible to pass that way then omitting for some creates 
inconsistency so better be consistent unless we remove other options too. 
But I'd let the maintainer decide on this.

Regards,
BALATON Zoltan