This option when set prevents a master boot record (MBR) to be
initialized. This is mandatory as some operating system don't recognized
mounted disks if a MBR is present.

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

diff --git a/block/vvfat.c b/block/vvfat.c
index 814796d918..0220dd828b 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1082,6 +1082,11 @@ static QemuOptsList runtime_opts = {
             .type = QEMU_OPT_BOOL,
             .help = "Make the image writable",
         },
+        {
+            .name = "no-mbr",
+            .type = QEMU_OPT_BOOL,
+            .help = "Do not add a Master Boot Record on this disk",
+        },
         { /* end of list */ }
     },
 };
@@ -1092,6 +1097,7 @@ static void vvfat_parse_filename(const char *filename, QDict *options,
     int fat_type = 0;
     bool floppy = false;
     bool rw = false;
+    bool no_mbr = false;
     int i;
 
     if (!strstart(filename, "fat:", NULL)) {
@@ -1116,6 +1122,10 @@ static void vvfat_parse_filename(const char *filename, QDict *options,
         rw = true;
     }
 
+    if (strstr(filename, ":no-mbr:")) {
+        no_mbr = true;
+    }
+
     /* Get the directory name without options */
     i = strrchr(filename, ':') - filename;
     assert(i >= 3);
@@ -1131,6 +1141,7 @@ static void vvfat_parse_filename(const char *filename, QDict *options,
     qdict_put_int(options, "fat-type", fat_type);
     qdict_put_bool(options, "floppy", floppy);
     qdict_put_bool(options, "rw", rw);
+    qdict_put_bool(options, "no-mbr", no_mbr);
 }
 
 static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
@@ -1196,7 +1207,10 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
         if (!s->fat_type) {
             s->fat_type = 16;
         }
-        s->offset_to_bootsector = 0x3f;
+        /* Reserver space for MBR */
+        if (!qemu_opt_get_bool(opts, "no-mbr", false)) {
+            s->offset_to_bootsector = 0x3f;
+        }
         cyls = s->fat_type == 12 ? 64 : 1024;
         heads = 16;
         secs = 63;
-- 
2.34.1

Am 03.09.2025 um 09:57 hat Clément Chigot geschrieben:
> This option when set prevents a master boot record (MBR) to be
> initialized. This is mandatory as some operating system don't recognized
> mounted disks if a MBR is present.
> 
> Signed-off-by: Clément Chigot <chigot@adacore.com>

Can we actually give an example of such an OS in the commit message?

> ---
>  block/vvfat.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/block/vvfat.c b/block/vvfat.c
> index 814796d918..0220dd828b 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -1082,6 +1082,11 @@ static QemuOptsList runtime_opts = {
>              .type = QEMU_OPT_BOOL,
>              .help = "Make the image writable",
>          },
> +        {
> +            .name = "no-mbr",
> +            .type = QEMU_OPT_BOOL,
> +            .help = "Do not add a Master Boot Record on this disk",
> +        },

Let's keep option names positive to avoid double negations like
'no-mbr=false'. We can have an 'mbr' option that defaults to true. Or in
fact, maybe calling it 'partitioned' would be easier to understand.

You need to update BlockdevOptionsVVFAT in qapi/block-core.json, too, to
make the new option work with -blockdev. You should update the
description for @floppy there, too, because it says that hard disks are
always partitioned.

It should also be added to vvfat_strong_runtime_opts because the value
of this option changes the data that the guest sees.

>          { /* end of list */ }
>      },
>  };
> @@ -1092,6 +1097,7 @@ static void vvfat_parse_filename(const char *filename, QDict *options,
>      int fat_type = 0;
>      bool floppy = false;
>      bool rw = false;
> +    bool no_mbr = false;
>      int i;
>  
>      if (!strstart(filename, "fat:", NULL)) {
> @@ -1116,6 +1122,10 @@ static void vvfat_parse_filename(const char *filename, QDict *options,
>          rw = true;
>      }
>  
> +    if (strstr(filename, ":no-mbr:")) {

In the string, the negative form can stay (because the positive one
doesn't exist here).

> +        no_mbr = true;
> +    }
> +
>      /* Get the directory name without options */
>      i = strrchr(filename, ':') - filename;
>      assert(i >= 3);
> @@ -1131,6 +1141,7 @@ static void vvfat_parse_filename(const char *filename, QDict *options,
>      qdict_put_int(options, "fat-type", fat_type);
>      qdict_put_bool(options, "floppy", floppy);
>      qdict_put_bool(options, "rw", rw);
> +    qdict_put_bool(options, "no-mbr", no_mbr);
>  }
>  
>  static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
> @@ -1196,7 +1207,10 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
>          if (!s->fat_type) {
>              s->fat_type = 16;
>          }
> -        s->offset_to_bootsector = 0x3f;
> +        /* Reserver space for MBR */
> +        if (!qemu_opt_get_bool(opts, "no-mbr", false)) {
> +            s->offset_to_bootsector = 0x3f;
> +        }
>          cyls = s->fat_type == 12 ? 64 : 1024;
>          heads = 16;
>          secs = 63;

Kevin

On Thu, Oct 23, 2025 at 8:21 PM Kevin Wolf <kwolf@redhat.com> wrote:
>
> Am 03.09.2025 um 09:57 hat Clément Chigot geschrieben:
> > This option when set prevents a master boot record (MBR) to be
> > initialized. This is mandatory as some operating system don't recognized
> > mounted disks if a MBR is present.
> >
> > Signed-off-by: Clément Chigot <chigot@adacore.com>
>
> Can we actually give an example of such an OS in the commit message?
>
> > ---
> >  block/vvfat.c | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/block/vvfat.c b/block/vvfat.c
> > index 814796d918..0220dd828b 100644
> > --- a/block/vvfat.c
> > +++ b/block/vvfat.c
> > @@ -1082,6 +1082,11 @@ static QemuOptsList runtime_opts = {
> >              .type = QEMU_OPT_BOOL,
> >              .help = "Make the image writable",
> >          },
> > +        {
> > +            .name = "no-mbr",
> > +            .type = QEMU_OPT_BOOL,
> > +            .help = "Do not add a Master Boot Record on this disk",
> > +        },
>
> Let's keep option names positive to avoid double negations like
> 'no-mbr=false'. We can have an 'mbr' option that defaults to true. Or in
> fact, maybe calling it 'partitioned' would be easier to understand.
>
> You need to update BlockdevOptionsVVFAT in qapi/block-core.json, too, to
> make the new option work with -blockdev. You should update the
> description for @floppy there, too, because it says that hard disks are
> always partitioned.
>
> It should also be added to vvfat_strong_runtime_opts because the value
> of this option changes the data that the guest sees.

Just to keep you updated, I've seen your comments. I'm just waiting to
see where the discussion of patch 5 leads before pushing v2.

> >          { /* end of list */ }
> >      },
> >  };
> > @@ -1092,6 +1097,7 @@ static void vvfat_parse_filename(const char *filename, QDict *options,
> >      int fat_type = 0;
> >      bool floppy = false;
> >      bool rw = false;
> > +    bool no_mbr = false;
> >      int i;
> >
> >      if (!strstart(filename, "fat:", NULL)) {
> > @@ -1116,6 +1122,10 @@ static void vvfat_parse_filename(const char *filename, QDict *options,
> >          rw = true;
> >      }
> >
> > +    if (strstr(filename, ":no-mbr:")) {
>
> In the string, the negative form can stay (because the positive one
> doesn't exist here).
>
> > +        no_mbr = true;
> > +    }
> > +
> >      /* Get the directory name without options */
> >      i = strrchr(filename, ':') - filename;
> >      assert(i >= 3);
> > @@ -1131,6 +1141,7 @@ static void vvfat_parse_filename(const char *filename, QDict *options,
> >      qdict_put_int(options, "fat-type", fat_type);
> >      qdict_put_bool(options, "floppy", floppy);
> >      qdict_put_bool(options, "rw", rw);
> > +    qdict_put_bool(options, "no-mbr", no_mbr);
> >  }
> >
> >  static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
> > @@ -1196,7 +1207,10 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
> >          if (!s->fat_type) {
> >              s->fat_type = 16;
> >          }
> > -        s->offset_to_bootsector = 0x3f;
> > +        /* Reserver space for MBR */
> > +        if (!qemu_opt_get_bool(opts, "no-mbr", false)) {
> > +            s->offset_to_bootsector = 0x3f;
> > +        }
> >          cyls = s->fat_type == 12 ? 64 : 1024;
> >          heads = 16;
> >          secs = 63;
>
> Kevin
>

Am 29.10.2025 um 09:37 hat Clément Chigot geschrieben:
> On Thu, Oct 23, 2025 at 8:21 PM Kevin Wolf <kwolf@redhat.com> wrote:
> >
> > Am 03.09.2025 um 09:57 hat Clément Chigot geschrieben:
> > > This option when set prevents a master boot record (MBR) to be
> > > initialized. This is mandatory as some operating system don't recognized
> > > mounted disks if a MBR is present.
> > >
> > > Signed-off-by: Clément Chigot <chigot@adacore.com>
> >
> > Can we actually give an example of such an OS in the commit message?
> >
> > > ---
> > >  block/vvfat.c | 16 +++++++++++++++-
> > >  1 file changed, 15 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/block/vvfat.c b/block/vvfat.c
> > > index 814796d918..0220dd828b 100644
> > > --- a/block/vvfat.c
> > > +++ b/block/vvfat.c
> > > @@ -1082,6 +1082,11 @@ static QemuOptsList runtime_opts = {
> > >              .type = QEMU_OPT_BOOL,
> > >              .help = "Make the image writable",
> > >          },
> > > +        {
> > > +            .name = "no-mbr",
> > > +            .type = QEMU_OPT_BOOL,
> > > +            .help = "Do not add a Master Boot Record on this disk",
> > > +        },
> >
> > Let's keep option names positive to avoid double negations like
> > 'no-mbr=false'. We can have an 'mbr' option that defaults to true. Or in
> > fact, maybe calling it 'partitioned' would be easier to understand.
> >
> > You need to update BlockdevOptionsVVFAT in qapi/block-core.json, too, to
> > make the new option work with -blockdev. You should update the
> > description for @floppy there, too, because it says that hard disks are
> > always partitioned.
> >
> > It should also be added to vvfat_strong_runtime_opts because the value
> > of this option changes the data that the guest sees.
> 
> Just to keep you updated, I've seen your comments. I'm just waiting to
> see where the discussion of patch 5 leads before pushing v2.

Yes, that makes sense. So this means that for all the other patches, you
agree with my comments and it's clear to you what to change in v2?

Kevin

On Wed, Oct 29, 2025 at 11:56 AM Kevin Wolf <kwolf@redhat.com> wrote:
>
> Am 29.10.2025 um 09:37 hat Clément Chigot geschrieben:
> > On Thu, Oct 23, 2025 at 8:21 PM Kevin Wolf <kwolf@redhat.com> wrote:
> > >
> > > Am 03.09.2025 um 09:57 hat Clément Chigot geschrieben:
> > > > This option when set prevents a master boot record (MBR) to be
> > > > initialized. This is mandatory as some operating system don't recognized
> > > > mounted disks if a MBR is present.
> > > >
> > > > Signed-off-by: Clément Chigot <chigot@adacore.com>
> > >
> > > Can we actually give an example of such an OS in the commit message?
> > >
> > > > ---
> > > >  block/vvfat.c | 16 +++++++++++++++-
> > > >  1 file changed, 15 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/block/vvfat.c b/block/vvfat.c
> > > > index 814796d918..0220dd828b 100644
> > > > --- a/block/vvfat.c
> > > > +++ b/block/vvfat.c
> > > > @@ -1082,6 +1082,11 @@ static QemuOptsList runtime_opts = {
> > > >              .type = QEMU_OPT_BOOL,
> > > >              .help = "Make the image writable",
> > > >          },
> > > > +        {
> > > > +            .name = "no-mbr",
> > > > +            .type = QEMU_OPT_BOOL,
> > > > +            .help = "Do not add a Master Boot Record on this disk",
> > > > +        },
> > >
> > > Let's keep option names positive to avoid double negations like
> > > 'no-mbr=false'. We can have an 'mbr' option that defaults to true. Or in
> > > fact, maybe calling it 'partitioned' would be easier to understand.
> > >
> > > You need to update BlockdevOptionsVVFAT in qapi/block-core.json, too, to
> > > make the new option work with -blockdev. You should update the
> > > description for @floppy there, too, because it says that hard disks are
> > > always partitioned.
> > >
> > > It should also be added to vvfat_strong_runtime_opts because the value
> > > of this option changes the data that the guest sees.
> >
> > Just to keep you updated, I've seen your comments. I'm just waiting to
> > see where the discussion of patch 5 leads before pushing v2.
>
> Yes, that makes sense. So this means that for all the other patches, you
> agree with my comments and it's clear to you what to change in v2?

Globally yes. I'm still unsure about patch 2 (I'll answer directly on
this patch). But for patch 1 and, yes clearly.