qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: "BALATON Zoltan" <balaton@eik.bme.hu>,
	"Clément Chigot" <chigot@adacore.com>,
	qemu-block@nongnu.org, qemu-devel@nongnu.org, hreitz@redhat.com,
	eblake@redhat.com
Subject: Re: [PATCH v2 1/5] vvfat: introduce partitioned option
Date: Mon, 10 Nov 2025 16:08:10 +0100	[thread overview]
Message-ID: <aRH_2gcYOH31UB38@redhat.com> (raw)
In-Reply-To: <87346mkos9.fsf@pond.sub.org>

Am 10.11.2025 um 14:20 hat Markus Armbruster geschrieben:
> BALATON Zoltan <balaton@eik.bme.hu> writes:
> 
> > On Mon, 10 Nov 2025, Clément Chigot wrote:
> >> On Mon, Nov 10, 2025 at 11:07 AM Markus Armbruster <armbru@redhat.com> wrote:
> >>>
> >>> Clément Chigot <chigot@adacore.com> writes:
> >>>
> >>>> This option tells whether a hard disk should be partitioned or not. It
> >>>> defaults to true and have the prime effect of preventing 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>
> >>>
> >>> [...]
> >>>
> >>>> diff --git a/qapi/block-core.json b/qapi/block-core.json
> >>>> index b82af74256..8a479ba090 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 a hard disk will be partitioned
> >>>
> >>> How does "partitioned" combine with "floppy": true?
> >>>
> >>> Is it silently ignored?
> >>>
> >>> Is it an error if present?
> >>>
> >>> Is it an error if true?
> >>>
> >>> Does it add a partition table if true?
> >>>
> >>>> +#     (default: true)
> >>>
> >>> Hmm, this suggests it's silently ignored.
> >>>
> >>> Silently ignoring nonsensical configuration is usually a bad idea.
> >>
> >> True, but that would mean "unpartitioned" must always be passed when
> >> "floppy" is requested. That would make such command lines a bit more
> >> verbose, but otherwise I don't think there is any issue to that.
> >>
> >> Note that I didn't add "partition" as a keyword in the command line.
> >> Currently, it's either the default (thus partitioned) or
> >> "unpartitioned" being requested. Do you think it makes sense to add it
> >> as well, even if it's redundant ?
> >>
> >>>> +#     (since 10.2)
> >>>> +#
> >>>
> >>> Not sure I like "partitioned".  Is a disk with an MBR and a partition
> >>> table contraining a single partition partitioned?  Call it "mbr"?
> >>
> >> It used to be called "mbr/no-mbr" but Kevin suggested renaming it in
> >> V1. Honestly I'm fine with both options:
> >> - Technically, the option prevents MBR which has a side effect for
> >> preventing partition tables
> 
> Yes, because the partition table is part of the MBR.  I'd rather name
> the option after the entire thing it controls, not one of its parts.
> 
> >> - Even it has a single partition, I think it makes sense to call a
> >> disk "partitioned" as long as it has a partition table
> >>
> >> But I'm not that familiar with disk formats, etc. I'll let you decide
> >> with Kevin, which one you prefer.
> 
> Kevin is the maintainer, I just serve as advisor here.

I figured that the meaning of "partitioned" is easier to understand for
a casual user than having or not having an MBR ("I don't want to boot
from this disk, why would I care about a boot record?").

But if people think that "mbr" is better, that's fine with me.

The only thing I really didn't want is the negative "no-mbr" and the
double negation in "no-mbr=off" that comes with it.

> > I'd also vote for mbr or similar shorter name; unpartitioned is
> > awkward to type out in a command line. Maybe it can default to false
> > for floppy and true for disk to preserve current behaviour but allow
> > controlling it.
> 
> I'm not a fan of conditional defaults, but I think it's better than a
> nonsensical default that gets ignored.

I think in this case a conditional default makes sense, not only for
compatibility reasons. Hard disks almost always have a partition, floppy
disks with partitions are basically unheard of.

Kevin



  reply	other threads:[~2025-11-10 15:09 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-07 14:53 [PATCH v2 0/5] block/vvfat: introduce "fat-size" option Clément Chigot
2025-11-07 14:53 ` [PATCH v2 1/5] vvfat: introduce partitioned option Clément Chigot
2025-11-10 10:07   ` Markus Armbruster
2025-11-10 11:09     ` Clément Chigot
2025-11-10 12:55       ` BALATON Zoltan
2025-11-10 13:20         ` Markus Armbruster
2025-11-10 15:08           ` Kevin Wolf [this message]
2025-11-10 15:25             ` BALATON Zoltan
2025-11-11  7:43             ` Markus Armbruster
2025-11-14  8:20               ` Clément Chigot
2025-11-14 13:25                 ` BALATON Zoltan
2025-11-14 13:47                   ` Clément Chigot
2025-11-07 14:53 ` [PATCH v2 2/5] vvfat: move fat_type check prior to size setup Clément Chigot
2025-11-10 10:09   ` Markus Armbruster
2025-11-10 11:15     ` Clément Chigot
2025-11-10 13:13       ` Markus Armbruster
2025-11-10 15:29         ` Kevin Wolf
2025-11-11  8:16           ` Markus Armbruster
2025-11-11  8:17           ` Markus Armbruster
2025-11-07 14:53 ` [PATCH v2 3/5] vvfat: add a define for VVFAT_SECTOR_BITS and VVFAT_SECTOR_SIZE Clément Chigot
2025-11-07 14:53 ` [PATCH v2 4/5] vvfat: move size parameters within driver structure Clément Chigot
2025-11-07 14:53 ` [PATCH v2 5/5] vvfat: add support for "fat-size" options Clément Chigot
2025-11-10 10:13   ` Markus Armbruster
2025-11-10 12:46     ` Clément Chigot
2025-11-10 13:09       ` Markus Armbruster
2025-11-10 13:26         ` Clément Chigot
2025-11-10 13:42           ` Markus Armbruster
2025-11-10 14:04             ` Clément Chigot
2025-11-10 15:20             ` Kevin Wolf
2025-11-10 15:36               ` BALATON Zoltan
2025-11-10 16:31                 ` Kevin Wolf
2025-11-10 21:36                   ` BALATON Zoltan
2025-11-12  9:50                   ` Clément Chigot
2025-11-12 12:29                     ` Kevin Wolf
2025-11-11  7:59               ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aRH_2gcYOH31UB38@redhat.com \
    --to=kwolf@redhat.com \
    --cc=armbru@redhat.com \
    --cc=balaton@eik.bme.hu \
    --cc=chigot@adacore.com \
    --cc=eblake@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).