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

[Markus Armbruster <armbru@redhat.com>]

Clément Chigot <chigot@adacore.com> writes:

> On Mon, Nov 10, 2025 at 11:09 AM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Clément Chigot <chigot@adacore.com> writes:
>>
>> > 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
>> > creates 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 fat-size parameter.
>> >
>> > Side note to mention that s->sectors_per_cluster assignments are
>> > removed because they are overidden a few line further.
>> >
>> > Signed-off-by: Clément Chigot <chigot@adacore.com>
>>
>> Is this a user-visible change?
>
> Yes, just "floppy" will now result in a 1440 KiB instead of the
> previous 2880 KiB. However, Kevin mentions in V1 that it would make
> more sense and vvfat being known to be unstable, this would be fine.
> FTR, here is the complete comment:
>
>> On Wed, Oct 29, 2025 at 5:06 PM Kevin Wolf <kwolf@redhat.com> wrote:
>> > In general, our stance is that we can change defaults whenever we want
>> > to, and if you don't want to be surprised by changing defaults, you need
>> > to specify the option explicitly.

Hmm, where is this stance on defaults documented?  Question for Kevin,
of course.

>> >                                   What's a bit strange about the vvfat
>> > interface is that the default actually represents a configuration that
>> > can't even be expressed explicitly at the moment.

Awkward.

>> > So it is a special case in a way, but given that this is vvfat, which is
>> > known to be unstable, not widely used outside of the occasional manual
>> > use and not supported by libvirt, I'm willing to just make the change.

I'm fine to treat vvfat as unstable.  But it's not marked as such in the
QAPI schema!  Is that a bug?  Again, for Kevin.