Mellvik / TLVC

Tiny Linux for Vintage Computers
Other
9 stars 0 forks source link

[cmd] Added medium size check to mkfs #27

Closed Mellvik closed 1 year ago

Mellvik commented 1 year ago

mkfs used to accept any oil system size up to the maximum # of blocks for Minix1 file systems.

With this update, mkfs checks the size of the media by different means to ensure the request is sane.

Also added is a means to dry-run the mkfs command: Add a '+' in front of the block size and mkfs will do its job but not write anything.

tlvc15# mkfs /dev/dhda +31752
10584 inodes
31752 blocks
Firstdatazone=339 (339)
Zonesize=1024
Maxsize=32514048

mkfs: No data written to device
tlvc15#

tlvc15# mkfs /dev/dhda 31753
mkfs: Requested size larger than device
tlvc15#
ghaerr commented 1 year ago

The rest is trial and error. The extra lseek and read is insurance

I'm not sure about "insurance", but I can now see why a read might be required given the current driver design problem (see below). The block driver "max size" is stored in only one place (inode->i_size), and the lseek uses this information for SEEK_END to return an error or not.

Looking at the driver, I see the problem: inode->i_size = floppy->size is set at open time, but this is before any auto probing. So the block device size is set to the initial base_type[] entry from the CMOS drive type. When the driver is finally given its first read/write request through do_fd_request, the driver will try probing (since current_type[] == NULL) and reset the global floppy to the correct floppy type, but doesn't set inode->i_size again. The fix is a bit complicated since inode isn't available at I/O time, so either a copy has to be made (could work but its a hack), or probing needs to occur at open time (this changes the current probing design somewhat heavily).

Thus, given the current design, a read is required to force a probe after which the driver could set save_inode->i_size for retrieval with a later stat. Executing a read past end of media could display a kernel I/O error message which mkfs wouldn't know about, then get a -1 read return. However, the read doesn't need to be from the end of media (just read from position 0), and it doesn't need to be 512 bytes. Last I checked, mkfs was extremely short on heap space for filesystems with large inode counts, so you might consider reading into a very small non-malloc'd buffer instead of calling malloc.

Mellvik commented 1 year ago

Thus, given the current design, a read is required to force a probe after which the driver could set save_inode->i_size for retrieval with a later stat. Executing a read past end of media could display a kernel I/O error message which mkfs wouldn't know about, then get a -1 read return. However, the read doesn't need to be from the end of media (just read from position 0), and it doesn't need to be 512 bytes. Last I checked, mkfs was extremely short on heap space for filesystems with large inode counts, so you might consider reading into a very small non-malloc'd buffer instead of calling malloc.

Thanks for the code review, @ghaerr - this is indeed a good point. The malloc is completely superfluous, I'll go ahead and change that!