Kevin Wolf <[email protected]> writes:
> Am 03.09.2025 um 09:57 hat Clément Chigot geschrieben:
>> This allows more flexibility to vvfat backend. The value for "Number of
>> Heads" and "Sectors per track" are based on SD specifications Part 2.
This is too terse to remind me of how vvfat picks cylinders, heads, and
sectors before this patch, so I need to go dig through the source code.
I figure it depends on configuration parameters @floppy and @fat-type
like this:
floppy fat-type cyls heads secs cyls*heads*secs*512
false 12 64 16 63 31.5 MiB
false 16 1024 16 63 504 MiB
false 32 1024 16 63 504 MiB
true 12 80 2 18 1440 KiB
true 16 80 2 36 2880 KiB
true 32 80 2 36 2880 KiB
How exactly does the new parameter @size change this?
>> Some limitations remains, the size parameter is recognized only when
>> "format=vvfat" is passed. In particular, "format=raw,size=xxx" is
>> keeping the previously hardcoded value: 504MB for FAT16 and 32 MB for
>> FAT12. FAT32 has not been adjusted and thus still default to 504MB.
31.5MiB unless I'm mistaken.
I'm not sure what you're trying to convey in this paragraph. As far as
I can tell, you're adding a @size parameter to vvfat, so of course it
doesn't affect raw.
>> Moreover, for flopyy, size=1M is creating a disk 1.44 MB, and size=2M a
floppy
>> disk of 2.88 MB. This avoids having to worry about float operations.
More on this part below.
>> Signed-off-by: Clément Chigot <[email protected]>
>> ---
>> block/vvfat.c | 165 ++++++++++++++++++++++++++++++++++++++++++--------
>> 1 file changed, 141 insertions(+), 24 deletions(-)
>>
>> diff --git a/block/vvfat.c b/block/vvfat.c
>> index 6526c585a2..4537c39d5c 100644
>> --- a/block/vvfat.c
>> +++ b/block/vvfat.c
>> @@ -1091,6 +1091,11 @@ static QemuOptsList runtime_opts = {
>> .type = QEMU_OPT_BOOL,
>> .help = "Do not add a Master Boot Record on this disk",
>> },
>> + {
>> + .name = BLOCK_OPT_SIZE,
>> + .type = QEMU_OPT_SIZE,
>> + .help = "Virtual disk size"
>> + },
>> { /* end of list */ }
>> },
>> };
>
> Like in patch 1, you need additional changes, in particular to add the
> option to the QAPI schema in qapi/block-core.json.
>
>> @@ -1148,10 +1153,141 @@ static void vvfat_parse_filename(const char
>> *filename, QDict *options,
>> qdict_put_bool(options, "no-mbr", no_mbr);
>> }
>>
>> +static void vvfat_get_size_parameters(uint64_t size, BDRVVVFATState *s,
>> + bool floppy, Error **errp)
>> +{
>> + if (floppy) {
>> + /*
>> + * Floppy emulation only supports 1.44 MB or 2.88 MB (default).
>> + * In order to avoid floating operations ambiguity, 1 MB is
>> + * recognized for 1.44 MB and 2 MB for 2.88 MB.
>> + */
>> + if (!size) {
>> + size = 2 * 1024 * 1024;
>> + } else {
>> + if (size == 1024 * 1024 && s->fat_type == 16) {
>> + error_setg(errp,
>> + "floppy FAT16 unsupported size; only support 2M "
>> + "(for an effective size of 2.88 MB)");
>> + } else if (size != 2 * 1024 * 1024 && size != 1024 * 1024) {
>> + error_setg(errp,
>> + "floppy unsupported size; should be 1MB (for "
>> + "an effective size of 1.44 MB) or 2.88M (for "
>> + "2.88MB)");
>> + }
>> + }
>
> This is horrible. To be fair, it's pretty hard to do something not
> horrible when the usual units to describe floppy sizes are already
> horrible. :-)
Yes :)
> But I'd still like us to do better here.
>
> To me it looks a bit like what we really want is an enum for floppy
> sizes (though is there any real reason why we have only those two?), but
> an arbitrary size for hard disks.
>
> Without the enum, obviously, users could specify 1440k and that would do
> the right thing. Maybe special casing whatever 1.44M and 2.88M result
> in and translating them into 1440k and 2880k could be more justifiable
> than special casing 1M and 2M, but it would still be ugly.
>
> Markus, do you have any advice how this should be represented in QAPI?
Maybe, but first I'd like to understand what @size does.