On Fri, Oct 24, 2025 at 10:35 AM Markus Armbruster <[email protected]> wrote: > > 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?
My prime goal was to create a 256 Mib VVFAT disk. As you can see, today for hard-disks there are only two possibilities: 31.5 Mib or 504 Mib. Hence, I've introduced the option `size=xxx` to allow more granular choices. This option changes how cyls, heads and secs parameters are computed to be as closed as possible of its value. I did try to keep it simple. I could have introduced options to select cylinders, heads, etc. But I think "size=xxx" would be more intuitive. There are also approximations made, as not all sizes can be reached. I didn't add errors or warnings for them. I'm fine adding them. > >> 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. True, I will fix it. > 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. Yes, but AFAICT, `if=sd,format=raw` will result in vvfat backend being called. I didn't manage to make the new option work with `if=sd,format=raw,size=256Mb`. Thus, when the "size" option is not provided, I keep the previous value (those for your above comment). Hence this paragraph to mostly warn people about the current limitation. > >> 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 :) I did have a first version that ignored this new size option for floppy. I did extend it because why not. But if you find it will bring too much complexity I can bring it back. > > 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. >
