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

[Kevin Wolf <kwolf@redhat.com>]

Am 10.11.2025 um 14:13 hat Markus Armbruster geschrieben:
> 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.

Probably nowhere. More importantly, I don't think a compatibility
promise that says otherwise is documented either. And we know that
defaults have changed before, and that libvirt tries to be as explicit
as possible to avoid being impacted by changed defaults.

Do you disagree? If so, is there any way to change defaults or do we
have to stick to the existing defaults forever? To me not specifying an
option means "just pick anything that makes sense", without any promise
that this stays the same across versions.

> >> >                                   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.

Maybe? Though the kind of unstable I think of with vvfat is more than
just API instability that the QAPI feature is about. vvfat is more a
dirty (and clever) hack that sometimes works and can be useful enough,
but if it breaks, you get to keep both pieces. Good for one-off uses on
your personal toy VM, but keep it far away from production. We never
seriously tried to get it to a properly supportable level.

(And yes, probably none of this is documented as clearly as it should
be.)

Kevin