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

Clément Chigot posted 5 patches 2 months, 1 week ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
[PATCH v3 1/5] vvfat: introduce partitioned option
Posted by Clément Chigot 2 months, 1 week ago
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 <chigot@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",
+        },
         { /* 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.  */
+    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",
 
     NULL
 };
diff --git a/qapi/block-core.json b/qapi/block-core.json
index b82af74256..ca438fba51 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3464,8 +3464,8 @@
 #
 # @fat-type: FAT type: 12, 16 or 32
 #
-# @floppy: whether to export a floppy image (true) or partitioned hard
-#     disk (false; default)
+# @floppy: whether to export a floppy image (true) or hard disk
+#     (false; default)
 #
 # @label: set the volume label, limited to 11 bytes.  FAT16 and FAT32
 #     traditionally have some restrictions on labels, which are
@@ -3474,11 +3474,15 @@
 #
 # @rw: whether to allow write operations (default: false)
 #
+# @partitioned: whether the volume will be partitioned;
+#     (default: true for hard disk, false for floppy)
+#     (since 10.2)
+#
 # Since: 2.9
 ##
 { 'struct': 'BlockdevOptionsVVFAT',
   'data': { 'dir': 'str', '*fat-type': 'int', '*floppy': 'bool',
-            '*label': 'str', '*rw': 'bool' } }
+            '*label': 'str', '*rw': 'bool', '*partitioned': 'bool' } }
 
 ##
 # @BlockdevOptionsGenericFormat:
-- 
2.43.0


Re: [PATCH v3 1/5] vvfat: introduce partitioned option
Posted by BALATON Zoltan 2 months, 1 week ago
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 <chigot@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?

>
>     NULL
> };
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index b82af74256..ca438fba51 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -3464,8 +3464,8 @@
> #
> # @fat-type: FAT type: 12, 16 or 32
> #
> -# @floppy: whether to export a floppy image (true) or partitioned hard
> -#     disk (false; default)
> +# @floppy: whether to export a floppy image (true) or hard disk
> +#     (false; default)

I was wondering what does this option do now. It seems to restrict size to 
different values, maybe that could be explained here.

Regards,
BALATON Zoltan

> #
> # @label: set the volume label, limited to 11 bytes.  FAT16 and FAT32
> #     traditionally have some restrictions on labels, which are
> @@ -3474,11 +3474,15 @@
> #
> # @rw: whether to allow write operations (default: false)
> #
> +# @partitioned: whether the volume will be partitioned;
> +#     (default: true for hard disk, false for floppy)
> +#     (since 10.2)
> +#
> # Since: 2.9
> ##
> { 'struct': 'BlockdevOptionsVVFAT',
>   'data': { 'dir': 'str', '*fat-type': 'int', '*floppy': 'bool',
> -            '*label': 'str', '*rw': 'bool' } }
> +            '*label': 'str', '*rw': 'bool', '*partitioned': 'bool' } }
>
> ##
> # @BlockdevOptionsGenericFormat:
>
Re: [PATCH v3 1/5] vvfat: introduce partitioned option
Posted by Clément Chigot 2 months ago
"

On Thu, Nov 27, 2025 at 3:40 PM BALATON Zoltan <balaton@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 <chigot@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.
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.

> >
> >     NULL
> > };
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index b82af74256..ca438fba51 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -3464,8 +3464,8 @@
> > #
> > # @fat-type: FAT type: 12, 16 or 32
> > #
> > -# @floppy: whether to export a floppy image (true) or partitioned hard
> > -#     disk (false; default)
> > +# @floppy: whether to export a floppy image (true) or hard disk
> > +#     (false; default)
>
> I was wondering what does this option do now. It seems to restrict size to
> different values, maybe that could be explained here.
>
> Regards,
> BALATON Zoltan
>
> > #
> > # @label: set the volume label, limited to 11 bytes.  FAT16 and FAT32
> > #     traditionally have some restrictions on labels, which are
> > @@ -3474,11 +3474,15 @@
> > #
> > # @rw: whether to allow write operations (default: false)
> > #
> > +# @partitioned: whether the volume will be partitioned;
> > +#     (default: true for hard disk, false for floppy)
> > +#     (since 10.2)
> > +#
> > # Since: 2.9
> > ##
> > { 'struct': 'BlockdevOptionsVVFAT',
> >   'data': { 'dir': 'str', '*fat-type': 'int', '*floppy': 'bool',
> > -            '*label': 'str', '*rw': 'bool' } }
> > +            '*label': 'str', '*rw': 'bool', '*partitioned': 'bool' } }
> >
> > ##
> > # @BlockdevOptionsGenericFormat:
> >
Re: [PATCH v3 1/5] vvfat: introduce partitioned option
Posted by BALATON Zoltan 2 months ago
On Tue, 9 Dec 2025, Clément Chigot wrote:
> On Thu, Nov 27, 2025 at 3:40 PM BALATON Zoltan <balaton@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 <chigot@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