pengutronix / genimage

tool to generate multiple filesystem and flash images from a tree
GNU General Public License v2.0
308 stars 110 forks source link

config.c: disable mtools C-H-S adressing check by default #35

Closed jacmet closed 6 years ago

jacmet commented 6 years ago

Mtools by default refuses to work with filesystems not fitting the legacy C-H-S adressing, E.G. bigger than 511MB or with "odd" sizes.

mkdosfs writes a BIOS parameter block containing (among others) the number of sectors per track and heads. This parameter block is what mtools reads and validates that total sectors is a multiple of sectors/track times heads.

For small(<512MB) filesystems, mkdosfs uses sectors/track = 32 and heads = 64, allowing filesystems of multiples of 1MB to validate. For bigger filesystems it instead uses sectors/track = 63 and heads = 255 to signify LBA addressing. 63 255 512 bytes is not a multiple of 1MB, so the validation in mtools fails for filesystems of multiples of 1MB, E.G:

image test.fat { vfat { files = { "test.txt" } } size = 512M }

Errors out with:

Total number of sectors (10444800) not a multiple of sectors per track (63)! Add mtools_skip_check=1 to your .mtoolsrc file to skip this test

This legacy check can be disabled by the MTOOLS_SKIP_CHECK environment variable (or editing the .mtoolsrc as described by the error message), so pass MTOOLS_SKIP_CHECK=1 by default to mcopy/mmd.

Signed-off-by: Peter Korsgaard peter@korsgaard.com

michaelolbrich commented 6 years ago

I like the idea but this should be added to the command-lines in image-vfat.c. Just add it unconditionally to all calls. And change the image size in the vfat test config to catch this.

jacmet commented 6 years ago

Ok, but they are really specific to mcopy / mmd, so wouldn't make sense if you configure different commands. I guess that isn't a very realistic use case though

jacmet commented 6 years ago

Moved logic to image-vfat.c and adjusted the vfat test so it fails before the 2nd patch (and succeeds afterwards)

michaelolbrich commented 6 years ago

Don't user setenv(). It affects everything, not just one image. Just put it in the command lines like I suggested. We already do something like this for in image-ext.c. Also, reorder the commits. I want all tests to succeed during bisection if possible.

jacmet commented 6 years ago

Ok, reworked and reordered. I used setenv to ensure all (future) cases would be handled, but OK. I added the test case before the fix to show the issue and confirm that the fix really fixes it, but I've swapped them around now as requested.