[Info-mtools] Bug: mformat does not format FAT32 correctly

[Pali Rohár]

Hi!

I observed 3 problems when I tried to create FAT32 disk image with
mformat tool:


1) mformat is not able to format disk images without specifying C/H/S
geometry. Nowadays there is no hard disk or SSD disk which use C/H/S
geometry and therefore asking user for such thing does not make sense.

So what I used is to specify 64 heads and 32 sectors per track just to
easily calculates number of tracks on disk. For disk with logical sector
size as 512 bytes (which is probably for all HDD and SSD), number of
tracks would be size of disk in megabytes (64*32*512 = 1 megabyte).

So mformat parameters: -h 64 -s 32 -t SIZE_IN_MB
Probably it is also needed to specify (for hard disks): -m 0xf8

If you do not enter this C/H/S geometry mformat just show error message:

mformat: Unknown geometry (You must tell the complete geometry of the
disk, either in /etc/mtools.conf or on the command line)

I think that except this error message, mformat for hard drives should
use above C/H/S auto-calculation.



2) mformat refuse to create FAT32 if disk size is less then 257 MB.

Test to reproduce (-F means to format as FAT32):

$ truncate -s 100M fat.img
$ mformat -F -h 64 -s 32 -t 100 -m 0xf8 :: -i fat.img

mformay show just error message:

Too few clusters for this fat size. Please choose a 16-bit fat in your
/etc/mtools.conf or .mtoolsrc file

Fix is to set size of cluster (in number of sectors) to 1. So mformat
parameter: -c 1

It is pity that mformat cannot calculate cluster size for smaller FAT32
disks (when size is less then 257 MB). Note that mformat can calculate
it correctly for FAT16 (when -F is not used).



3) Cluster size is not calculated correctly according to Microsoft FAT
specification. This also answer why there is a bug 2).

Function which calculate cluster size is named calc_cluster_size and can
be found in mformat.c source code. Relevant part for FAT32 is:

switch(abs(fat_bits)) {
...
case 32:
Fs->cluster_size = 8;
/* According to
* http://support.microsoft.com/support/kb/articles/q154/9/97.asp
* Micro$oft does not support FAT32 with less than 4K
*/
return;
...

It means that cluster size is hardcoded to 8 sectors for FAT32. This is
also reason for problem 3). For FAT16 there is autodetection code. I do
not know what that comment mean or what it try to refer as link is dead.

So I would like to know, why is hardcoded 8 sectors as cluster size for
FAT32? I suspect this is a bug which needs fixing. At least to have
working mformat in FAT32 mode for small disk (or disk images).

Because, in Microsoft FAT32 specification (fatgen103.doc) on page 20 for
FAT32 is written calculation table. For disk size there is cluster size
(in number of sectors), see table below:

32.5 MB - 260 MB cluster_size = 1
260 MB - 8 GB cluster_size = 8
8 GB - 16 GB cluster_size = 16
16 GB - 32 GB cluster_size = 32
32 GB - 2 TB cluster_size = 64

Note that in that Microsoft FAT specification is written: "The rest of
this section is totally specific to drives that have 512 bytes per
sector.". So there is need to recalculate values when (logical) sector
size is not 512 bytes.

[Pali Rohár]

Post by Pali Rohár
Hi!
I observed 3 problems when I tried to create FAT32 disk image with
1) mformat is not able to format disk images without specifying C/H/S
geometry. Nowadays there is no hard disk or SSD disk which use C/H/S
geometry and therefore asking user for such thing does not make sense.
So what I used is to specify 64 heads and 32 sectors per track just to
easily calculates number of tracks on disk. For disk with logical sector
size as 512 bytes (which is probably for all HDD and SSD), number of
tracks would be size of disk in megabytes (64*32*512 = 1 megabyte).
So mformat parameters: -h 64 -s 32 -t SIZE_IN_MB
Probably it is also needed to specify (for hard disks): -m 0xf8
mformat: Unknown geometry (You must tell the complete geometry of the
disk, either in /etc/mtools.conf or on the command line)
I think that except this error message, mformat for hard drives should
use above C/H/S auto-calculation.
2) mformat refuse to create FAT32 if disk size is less then 257 MB.
$ truncate -s 100M fat.img
$ mformat -F -h 64 -s 32 -t 100 -m 0xf8 :: -i fat.img
Too few clusters for this fat size. Please choose a 16-bit fat in your
/etc/mtools.conf or .mtoolsrc file
Fix is to set size of cluster (in number of sectors) to 1. So mformat
parameter: -c 1
It is pity that mformat cannot calculate cluster size for smaller FAT32
disks (when size is less then 257 MB). Note that mformat can calculate
it correctly for FAT16 (when -F is not used).
3) Cluster size is not calculated correctly according to Microsoft FAT
specification. This also answer why there is a bug 2).
Function which calculate cluster size is named calc_cluster_size and can
switch(abs(fat_bits)) {
...
Fs->cluster_size = 8;
/* According to
* http://support.microsoft.com/support/kb/articles/q154/9/97.asp
* Micro$oft does not support FAT32 with less than 4K
*/
return;
...
It means that cluster size is hardcoded to 8 sectors for FAT32. This is
also reason for problem 3). For FAT16 there is autodetection code. I do
not know what that comment mean or what it try to refer as link is dead.
So I would like to know, why is hardcoded 8 sectors as cluster size for
FAT32? I suspect this is a bug which needs fixing. At least to have
working mformat in FAT32 mode for small disk (or disk images).

Also please note that current versions of Windows (probably since 2000)
allow user to specify cluster size in Format dialog also for FAT32. And
default value depends on disk size, it is not hardcoded to 8 (=4kB).

See also:
https://support.microsoft.com/en-us/help/192322/description-of-default-cluster-sizes-for-fat32-file-system
https://support.microsoft.com/en-us/help/140365/default-cluster-size-for-ntfs--fat--and-exfat

So I would really suggest to change above hardcoded value 8 to either
mapping table or use similar algorithm as is already present for FAT16
in mformat.c

Post by Pali Rohár
Because, in Microsoft FAT32 specification (fatgen103.doc) on page 20 for
FAT32 is written calculation table. For disk size there is cluster size
32.5 MB - 260 MB cluster_size = 1
260 MB - 8 GB cluster_size = 8
8 GB - 16 GB cluster_size = 16
16 GB - 32 GB cluster_size = 32
32 GB - 2 TB cluster_size = 64
Note that in that Microsoft FAT specification is written: "The rest of
this section is totally specific to drives that have 512 bytes per
sector.". So there is need to recalculate values when (logical) sector
size is not 512 bytes.

[Pali Rohár]

Hi! Just a reminding for these 3 problems in mformat...

Correct solution for problem 3) (= calculation of cluster size on FAT32)
will solve also problem 2). And I would suggest to use same calculation
algorithm for FAT32 as for FAT16.

And for problem 1), it would be really nice to choose some sane C/H/S
defaults when creating hard disk image or when formatting non-C/H/S
disk. As it really does not make sense to ask user for supplying C/H/S
geometry when formatting disk.

Even MS format.exe and Windows GUI tools does not ask user for C/H/S
geometry when formatting SSD disk.

Post by Pali Rohár

Post by Pali Rohár
Hi!
I observed 3 problems when I tried to create FAT32 disk image with
1) mformat is not able to format disk images without specifying C/H/S
geometry. Nowadays there is no hard disk or SSD disk which use C/H/S
geometry and therefore asking user for such thing does not make sense.
So what I used is to specify 64 heads and 32 sectors per track just to
easily calculates number of tracks on disk. For disk with logical sector
size as 512 bytes (which is probably for all HDD and SSD), number of
tracks would be size of disk in megabytes (64*32*512 = 1 megabyte).
So mformat parameters: -h 64 -s 32 -t SIZE_IN_MB
Probably it is also needed to specify (for hard disks): -m 0xf8
mformat: Unknown geometry (You must tell the complete geometry of the
disk, either in /etc/mtools.conf or on the command line)
I think that except this error message, mformat for hard drives should
use above C/H/S auto-calculation.
2) mformat refuse to create FAT32 if disk size is less then 257 MB.
$ truncate -s 100M fat.img
$ mformat -F -h 64 -s 32 -t 100 -m 0xf8 :: -i fat.img
Too few clusters for this fat size. Please choose a 16-bit fat in your
/etc/mtools.conf or .mtoolsrc file
Fix is to set size of cluster (in number of sectors) to 1. So mformat
parameter: -c 1
It is pity that mformat cannot calculate cluster size for smaller FAT32
disks (when size is less then 257 MB). Note that mformat can calculate
it correctly for FAT16 (when -F is not used).
3) Cluster size is not calculated correctly according to Microsoft FAT
specification. This also answer why there is a bug 2).
Function which calculate cluster size is named calc_cluster_size and can
switch(abs(fat_bits)) {
...
Fs->cluster_size = 8;
/* According to
* http://support.microsoft.com/support/kb/articles/q154/9/97.asp
* Micro$oft does not support FAT32 with less than 4K
*/
return;
...
It means that cluster size is hardcoded to 8 sectors for FAT32. This is
also reason for problem 3). For FAT16 there is autodetection code. I do
not know what that comment mean or what it try to refer as link is dead.
So I would like to know, why is hardcoded 8 sectors as cluster size for
FAT32? I suspect this is a bug which needs fixing. At least to have
working mformat in FAT32 mode for small disk (or disk images).

Also please note that current versions of Windows (probably since 2000)
allow user to specify cluster size in Format dialog also for FAT32. And
default value depends on disk size, it is not hardcoded to 8 (=4kB).
https://support.microsoft.com/en-us/help/192322/description-of-default-cluster-sizes-for-fat32-file-system
https://support.microsoft.com/en-us/help/140365/default-cluster-size-for-ntfs--fat--and-exfat
So I would really suggest to change above hardcoded value 8 to either
mapping table or use similar algorithm as is already present for FAT16
in mformat.c

Post by Pali Rohár
Because, in Microsoft FAT32 specification (fatgen103.doc) on page 20 for
FAT32 is written calculation table. For disk size there is cluster size
32.5 MB - 260 MB cluster_size = 1
260 MB - 8 GB cluster_size = 8
8 GB - 16 GB cluster_size = 16
16 GB - 32 GB cluster_size = 32
32 GB - 2 TB cluster_size = 64
Note that in that Microsoft FAT specification is written: "The rest of
this section is totally specific to drives that have 512 bytes per
sector.". So there is need to recalculate values when (logical) sector
size is not 512 bytes.

[Pali Rohár]

Post by Pali Rohár

Post by Pali Rohár
2) mformat refuse to create FAT32 if disk size is less then 257 MB.
$ truncate -s 100M fat.img
$ mformat -F -h 64 -s 32 -t 100 -m 0xf8 :: -i fat.img
Too few clusters for this fat size. Please choose a 16-bit fat in your
/etc/mtools.conf or .mtoolsrc file
Fix is to set size of cluster (in number of sectors) to 1. So mformat
parameter: -c 1
It is pity that mformat cannot calculate cluster size for smaller FAT32
disks (when size is less then 257 MB). Note that mformat can calculate
it correctly for FAT16 (when -F is not used).

Also there is a problem with large disks. By default, 2TB disk formatted
by mformat to FAT32 is not readable by Linux kernel. It just says:

FAT-fs (loop0): count of clusters too big (536739966)
FAT-fs (loop0): Can't find a valid FAT filesystem

To workaround this problem, it is needed to specify -c 64 parameter due
to the same problem. mformat does not calculates cluster size correctly.

Post by Pali Rohár

Post by Pali Rohár
3) Cluster size is not calculated correctly according to Microsoft FAT
specification. This also answer why there is a bug 2).
Function which calculate cluster size is named calc_cluster_size and can
switch(abs(fat_bits)) {
...
Fs->cluster_size = 8;
/* According to
* http://support.microsoft.com/support/kb/articles/q154/9/97.asp
* Micro$oft does not support FAT32 with less than 4K
*/
return;
...
It means that cluster size is hardcoded to 8 sectors for FAT32. This is
also reason for problem 3). For FAT16 there is autodetection code. I do
not know what that comment mean or what it try to refer as link is dead.
So I would like to know, why is hardcoded 8 sectors as cluster size for
FAT32? I suspect this is a bug which needs fixing. At least to have
working mformat in FAT32 mode for small disk (or disk images).

Also please note that current versions of Windows (probably since 2000)
allow user to specify cluster size in Format dialog also for FAT32. And
default value depends on disk size, it is not hardcoded to 8 (=4kB).
https://support.microsoft.com/en-us/help/192322/description-of-default-cluster-sizes-for-fat32-file-system
https://support.microsoft.com/en-us/help/140365/default-cluster-size-for-ntfs--fat--and-exfat
So I would really suggest to change above hardcoded value 8 to either
mapping table or use similar algorithm as is already present for FAT16
in mformat.c

Post by Pali Rohár
Because, in Microsoft FAT32 specification (fatgen103.doc) on page 20 for
FAT32 is written calculation table. For disk size there is cluster size
32.5 MB - 260 MB cluster_size = 1
260 MB - 8 GB cluster_size = 8
8 GB - 16 GB cluster_size = 16
16 GB - 32 GB cluster_size = 32
32 GB - 2 TB cluster_size = 64
Note that in that Microsoft FAT specification is written: "The rest of
this section is totally specific to drives that have 512 bytes per
sector.". So there is need to recalculate values when (logical) sector
size is not 512 bytes.

In attachment is a patch for mtools which calculates FAT32 cluster size
according to that Microsoft specification. With that patch I'm able to
correctly format small FAT32 disks (below 100M) and also big disks (2TB)
and even also very huge disks 16TB with 4k sectors.

Examples:

small 100 MB FAT32 disk image
$ truncate -s 100MB /tmp/fat
$ mformat -F -h 64 -s 32 -t $((100-1)) :: -i /tmp/fat

big 2 TB FAT32 disk image (this is maximal size for 512 byte sectors)
$ truncate -s 2TB /tmp/fat
$ mformat -F -h 64 -s 32 -t $((2*1024*1024-1)) :: -i /tmp/fat

very huge 16 TB FAT32 disk image with 4k sectors
$ truncate -s 16TB /tmp/fat
$ mformat -F -M 4096 -h 8 -s 32 -t $((16*1024*1024-1)) :: -i /tmp/fat

Everything is correctly detected and mounted on Linux.

[Alain Knaff]

Hi,

First, thanks for your patch, and sorry for the slow response. When I
got your mails, I was rather busy, and eventually they ended up below
the pile...

On 11/08/18 12:45, Pali Rohár wrote:
[...]

Post by Pali Rohár

Post by Pali Rohár

Post by Pali Rohár
2) mformat refuse to create FAT32 if disk size is less then 257 MB.
$ truncate -s 100M fat.img
$ mformat -F -h 64 -s 32 -t 100 -m 0xf8 :: -i fat.img
Too few clusters for this fat size. Please choose a 16-bit fat in your
/etc/mtools.conf or .mtoolsrc file
Fix is to set size of cluster (in number of sectors) to 1. So mformat
parameter: -c 1

Exactly.

Post by Pali Rohár

Post by Pali Rohár

Post by Pali Rohár
It is pity that mformat cannot calculate cluster size for smaller FAT32
disks (when size is less then 257 MB). Note that mformat can calculate
it correctly for FAT16 (when -F is not used).

As some Microsoft documents (the cited knowledge base article) claimed
that 4KB clusters where the default minimum size, this was used by
default (... while still allowing to override it using -c if wanted).

[...]

Post by Pali Rohár

Post by Pali Rohár

Post by Pali Rohár
switch(abs(fat_bits)) {
...
Fs->cluster_size = 8;
/* According to
* http://support.microsoft.com/support/kb/articles/q154/9/97.asp
* Micro$oft does not support FAT32 with less than 4K
*/
return;
...
It means that cluster size is hardcoded to 8 sectors for FAT32. This is
also reason for problem 3). For FAT16 there is autodetection code. I do
not know what that comment mean

Really? Or are you joking? :-)

It gives the web address of a document published at Microsoft's support
site. This document contains a sectence which could be understood to
mean that FAT32 filesystems should not have cluster sizes less than 4K
(i.e. 8 sectors).

Post by Pali Rohár
or what it try to refer as link is dead.

Indeed, they moved it to a different address:
https://support.microsoft.com/en-us/help/154997/description-of-the-fat32-file-system

The relevant sentence is:
"The FAT32 file system allows for a default cluster size as small as 4 KB"

Ok, so this is the _default_ size, but smaller sizes are apparently
permissible.

Post by Pali Rohár

Post by Pali Rohár

Post by Pali Rohár
So I would like to know, why is hardcoded 8 sectors as cluster size for
FAT32?

Actually, in mtools 4.0.18 this was not hardcoded, but overrideable
using the -c option, as you noticed yourself in your earlier comment.

I'm not sure why you disabled this possibility in your patch, or maybe
that was just some leftover from earlier testing.

Post by Pali Rohár

Post by Pali Rohár

Post by Pali Rohár
I suspect this is a bug which needs fixing. At least to have
working mformat in FAT32 mode for small disk (or disk images).

I've now implemented the table you suggested (starting with 1 and going
to 64), but I still kept the possibility to override with -c

Post by Pali Rohár

Post by Pali Rohár
Also please note that current versions of Windows (probably since 2000)
allow user to specify cluster size in Format dialog also for FAT32. And

Exactly. And that's why I re-enabled the -c option.

Post by Pali Rohár

Post by Pali Rohár
default value depends on disk size, it is not hardcoded to 8 (=4kB).
https://support.microsoft.com/en-us/help/192322/description-of-default-cluster-sizes-for-fat32-file-system
https://support.microsoft.com/en-us/help/140365/default-cluster-size-for-ntfs--fat--and-exfat

Oddly, these leave out the step with cluster_size = 1 but start with 4K
(8 sectors)

Post by Pali Rohár

Post by Pali Rohár
So I would really suggest to change above hardcoded value 8 to either
mapping table or use similar algorithm as is already present for FAT16
in mformat.c

Post by Pali Rohár
Because, in Microsoft FAT32 specification (fatgen103.doc) on page 20 for
FAT32 is written calculation table. For disk size there is cluster size
32.5 MB - 260 MB cluster_size = 1
260 MB - 8 GB cluster_size = 8
8 GB - 16 GB cluster_size = 16
16 GB - 32 GB cluster_size = 32
32 GB - 2 TB cluster_size = 64

Indeed, fatgen103.doc starts with cluster_size = 1.

Post by Pali Rohár

Post by Pali Rohár

Post by Pali Rohár
Note that in that Microsoft FAT specification is written: "The rest of
this section is totally specific to drives that have 512 bytes per
sector.". So there is need to recalculate values when (logical) sector
size is not 512 bytes.

In attachment is a patch for mtools which calculates FAT32 cluster size
according to that Microsoft specification. With that patch I'm able to
correctly format small FAT32 disks (below 100M) and also big disks (2TB)
and even also very huge disks 16TB with 4k sectors.
small 100 MB FAT32 disk image
$ truncate -s 100MB /tmp/fat
$ mformat -F -h 64 -s 32 -t $((100-1)) :: -i /tmp/fat
big 2 TB FAT32 disk image (this is maximal size for 512 byte sectors)
$ truncate -s 2TB /tmp/fat
$ mformat -F -h 64 -s 32 -t $((2*1024*1024-1)) :: -i /tmp/fat
very huge 16 TB FAT32 disk image with 4k sectors
$ truncate -s 16TB /tmp/fat
$ mformat -F -M 4096 -h 8 -s 32 -t $((16*1024*1024-1)) :: -i /tmp/fat
Everything is correctly detected and mounted on Linux.

This is now part of the development version of mtools, to be released in
a week or so.

Thanks,

Alain

[Pali Rohár]

Post by Alain Knaff
Hi,
First, thanks for your patch, and sorry for the slow response. When I
got your mails, I was rather busy, and eventually they ended up below
the pile...
[...]

Post by Pali Rohár

Post by Pali Rohár

Post by Pali Rohár
2) mformat refuse to create FAT32 if disk size is less then 257 MB.
$ truncate -s 100M fat.img
$ mformat -F -h 64 -s 32 -t 100 -m 0xf8 :: -i fat.img
Too few clusters for this fat size. Please choose a 16-bit fat in your
/etc/mtools.conf or .mtoolsrc file
Fix is to set size of cluster (in number of sectors) to 1. So mformat
parameter: -c 1

Exactly.

Post by Pali Rohár

Post by Pali Rohár

Post by Pali Rohár
It is pity that mformat cannot calculate cluster size for smaller FAT32
disks (when size is less then 257 MB). Note that mformat can calculate
it correctly for FAT16 (when -F is not used).

As some Microsoft documents (the cited knowledge base article) claimed
that 4KB clusters where the default minimum size, this was used by
default (... while still allowing to override it using -c if wanted).
[...]

Post by Pali Rohár

Post by Pali Rohár

Post by Pali Rohár
switch(abs(fat_bits)) {
...
Fs->cluster_size = 8;
/* According to
* http://support.microsoft.com/support/kb/articles/q154/9/97.asp
* Micro$oft does not support FAT32 with less than 4K
*/
return;
...
It means that cluster size is hardcoded to 8 sectors for FAT32. This is
also reason for problem 3). For FAT16 there is autodetection code. I do
not know what that comment mean

Really? Or are you joking? :-)

No. I understand "Implementation XYZ does not support feature ABC" as if
I prepare feature ABC, then whatever I do XYZ would not read or access
ABC. Therefore I understood above comment as Microsoft's FAT driver
cannot read FA532 filesystem with less then 4K.

But in reality it can.

Post by Alain Knaff
It gives the web address of a document published at Microsoft's support
site. This document contains a sectence which could be understood to
mean that FAT32 filesystems should not have cluster sizes less than 4K
(i.e. 8 sectors).

Post by Pali Rohár
or what it try to refer as link is dead.

https://support.microsoft.com/en-us/help/154997/description-of-the-fat32-file-system
"The FAT32 file system allows for a default cluster size as small as 4 KB"
Ok, so this is the _default_ size, but smaller sizes are apparently
permissible.

That is then something different. As it describe parameters for
formatting new filesystem. Not what filesystem driver can mount.

Post by Alain Knaff

Post by Pali Rohár

Post by Pali Rohár

Post by Pali Rohár
So I would like to know, why is hardcoded 8 sectors as cluster size for
FAT32?

Actually, in mtools 4.0.18 this was not hardcoded, but overrideable
using the -c option, as you noticed yourself in your earlier comment.
I'm not sure why you disabled this possibility in your patch, or maybe
that was just some leftover from earlier testing.

Hm... I'm not aware of nothing that I disabled... Maybe bug?

Post by Alain Knaff

Post by Pali Rohár

Post by Pali Rohár

Post by Pali Rohár
I suspect this is a bug which needs fixing. At least to have
working mformat in FAT32 mode for small disk (or disk images).

I've now implemented the table you suggested (starting with 1 and going
to 64), but I still kept the possibility to override with -c

Ok.

Post by Alain Knaff

Post by Pali Rohár

Post by Pali Rohár
Also please note that current versions of Windows (probably since 2000)
allow user to specify cluster size in Format dialog also for FAT32. And

Exactly. And that's why I re-enabled the -c option.

Post by Pali Rohár

Post by Pali Rohár
default value depends on disk size, it is not hardcoded to 8 (=4kB).
https://support.microsoft.com/en-us/help/192322/description-of-default-cluster-sizes-for-fat32-file-system
https://support.microsoft.com/en-us/help/140365/default-cluster-size-for-ntfs--fat--and-exfat

Oddly, these leave out the step with cluster_size = 1 but start with 4K
(8 sectors)

Post by Pali Rohár

Post by Pali Rohár
So I would really suggest to change above hardcoded value 8 to either
mapping table or use similar algorithm as is already present for FAT16
in mformat.c

Post by Pali Rohár
Because, in Microsoft FAT32 specification (fatgen103.doc) on page 20 for
FAT32 is written calculation table. For disk size there is cluster size
32.5 MB - 260 MB cluster_size = 1
260 MB - 8 GB cluster_size = 8
8 GB - 16 GB cluster_size = 16
16 GB - 32 GB cluster_size = 32
32 GB - 2 TB cluster_size = 64

Indeed, fatgen103.doc starts with cluster_size = 1.

Post by Pali Rohár

Post by Pali Rohár

Post by Pali Rohár
Note that in that Microsoft FAT specification is written: "The rest of
this section is totally specific to drives that have 512 bytes per
sector.". So there is need to recalculate values when (logical) sector
size is not 512 bytes.

In attachment is a patch for mtools which calculates FAT32 cluster size
according to that Microsoft specification. With that patch I'm able to
correctly format small FAT32 disks (below 100M) and also big disks (2TB)
and even also very huge disks 16TB with 4k sectors.
small 100 MB FAT32 disk image
$ truncate -s 100MB /tmp/fat
$ mformat -F -h 64 -s 32 -t $((100-1)) :: -i /tmp/fat
big 2 TB FAT32 disk image (this is maximal size for 512 byte sectors)
$ truncate -s 2TB /tmp/fat
$ mformat -F -h 64 -s 32 -t $((2*1024*1024-1)) :: -i /tmp/fat
very huge 16 TB FAT32 disk image with 4k sectors
$ truncate -s 16TB /tmp/fat
$ mformat -F -M 4096 -h 8 -s 32 -t $((16*1024*1024-1)) :: -i /tmp/fat
Everything is correctly detected and mounted on Linux.

This is now part of the development version of mtools, to be released in
a week or so.

Ok. Is somewhere available that development version?

Post by Alain Knaff
Thanks,
Alain

[Pali Rohár]

@@ -564,13 +576,13 @@ static void calc_cluster_size(struct Fs_
/* double the cluster size until we can fill up the disk with
* the maximal number of sectors of this size */
while(Fs->cluster_size * max_clusters + max_fat_size < rem_sect) {
+ Fs->cluster_size <<= 1;
if(Fs->cluster_size > 64) {
/* bigger than 64. Should fit */
fprintf(stderr,
"Internal error while calculating cluster size\n");
exit(1);
}
- Fs->cluster_size <<= 1;
}
}

Looks like above change is wrong. Both Linux and Windows FAT drivers can
read FAT also with cluster size 128. So there is no need to restrict it
to 64, and use previous code as before.

[Pali Rohár]

Post by Pali Rohár
1) mformat is not able to format disk images without specifying C/H/S
geometry. Nowadays there is no hard disk or SSD disk which use C/H/S
geometry and therefore asking user for such thing does not make sense.
So what I used is to specify 64 heads and 32 sectors per track just to
easily calculates number of tracks on disk. For disk with logical sector
size as 512 bytes (which is probably for all HDD and SSD), number of
tracks would be size of disk in megabytes (64*32*512 = 1 megabyte).
So mformat parameters: -h 64 -s 32 -t SIZE_IN_MB
Probably it is also needed to specify (for hard disks): -m 0xf8
mformat: Unknown geometry (You must tell the complete geometry of the
disk, either in /etc/mtools.conf or on the command line)
I think that except this error message, mformat for hard drives should
use above C/H/S auto-calculation.

Hi! In attachment is a patch which automatically calculates C/H/S
geometry based on LBA Assist Translation formula just from the size of
disk. So formatting non-C/H/S disks (today *all* disks) via mformat is
automatic without need to specify C/H/S numbers. This applies also for
disk images.

This patch also adds support for detecting logical sector size of disk
and propagates it to FAT sector size. This is needed for a new Native 4K
disks which have sector size 4096 bytes (and not 512).

So with this patch (and previous one) is formatting Native 4K disk of
16 TB to FAT32 easily, just with command:

$ mformat -F :: -i /dev/sdc

[Alain Knaff]

Hi,

Post by Pali Rohár

Post by Pali Rohár
1) mformat is not able to format disk images without specifying C/H/S
geometry. Nowadays there is no hard disk or SSD disk which use C/H/S
geometry and therefore asking user for such thing does not make sense.

Actually, with physical disks, mtools asks the kernel for a geometry.

However, you're right: the issue is indeed relevant for image files,
which have no innate geometry.

[...]

Post by Pali Rohár
Hi! In attachment is a patch which automatically calculates C/H/S
geometry based on LBA Assist Translation formula just from the size of
disk. So formatting non-C/H/S disks (today *all* disks) via mformat is
automatic without need to specify C/H/S numbers. This applies also for
disk images.
This patch also adds support for detecting logical sector size of disk
and propagates it to FAT sector size. This is needed for a new Native 4K
disks which have sector size 4096 bytes (and not 512).
So with this patch (and previous one) is formatting Native 4K disk of
$ mformat -F :: -i /dev/sdc

I looked at the patch, and it seems to contain more than bargained for :-)

Indeed, there seems to be debugging code left in: messages printed to
stdout, or messages meant for final diagnostic being printed out right
away before all available options (drive definitions) have been tried.

Also, there is some puzzling "late capping" of sector_size which should
be unneeded (ssize is already being capped at input), and which would
introduce inconsistencies if for some reason it did indeed kick in.

Could you please review your patch, and only keep in those items that
are actually relevant for the purpose at hand (C/H/S calculation and
reading sector size), and resend it?

... or, if these are indeed needed, maybe include a comment and/or
explanation what they are doing?

Thanks,

Alain

[Pali Rohár]

Post by Alain Knaff
Hi,

Post by Pali Rohár

Post by Pali Rohár
1) mformat is not able to format disk images without specifying C/H/S
geometry. Nowadays there is no hard disk or SSD disk which use C/H/S
geometry and therefore asking user for such thing does not make sense.

Actually, with physical disks, mtools asks the kernel for a geometry.

Yes, and kernel can return error (geometry not available). This happens
also for block devices (e.g SD card, or loopback).

And maybe same problem would be also for NVME disks.

Post by Alain Knaff
However, you're right: the issue is indeed relevant for image files,
which have no innate geometry.

Yes, it is relevant also for image files.

Post by Alain Knaff
[...]

Post by Pali Rohár
Hi! In attachment is a patch which automatically calculates C/H/S
geometry based on LBA Assist Translation formula just from the size of
disk. So formatting non-C/H/S disks (today *all* disks) via mformat is
automatic without need to specify C/H/S numbers. This applies also for
disk images.
This patch also adds support for detecting logical sector size of disk
and propagates it to FAT sector size. This is needed for a new Native 4K
disks which have sector size 4096 bytes (and not 512).
So with this patch (and previous one) is formatting Native 4K disk of
$ mformat -F :: -i /dev/sdc

I looked at the patch, and it seems to contain more than bargained for :-)
Indeed, there seems to be debugging code left in: messages printed to
stdout, or messages meant for final diagnostic being printed out right
away before all available options (drive definitions) have been tried.

Hm.. there are no messages printed to stdout. And also there are no
debugging code. Those messages are printed to string buffer (sprintf)
and contains errors. In same way how other functions do it.

Post by Alain Knaff
Also, there is some puzzling "late capping" of sector_size which should
be unneeded (ssize is already being capped at input), and which would
introduce inconsistencies if for some reason it did indeed kick in.

No. ssize (= sector size) is no set correctly from block device. It is
set only when specified in config file or hardcoded for specific devices
(like floppy, etc.).

To get sector size of block device you need to call BLKSSZGET ioctl. And
this call is not called in mtools 4.0.18 (you can grep it).

It should be part of the get_block_geom() function which get disk
geometry. In past all disks had BLKSSZGET just 512, but today it is not
truth.

And without correct sector size of block device (BLKSSZGET) it would not
work correctly.

Post by Alain Knaff
Could you please review your patch, and only keep in those items that
are actually relevant for the purpose at hand (C/H/S calculation and
reading sector size), and resend it?
... or, if these are indeed needed, maybe include a comment and/or
explanation what they are doing?

I think that all parts in patch are relevant and needed. First one
updates code for reading disk geometry to read also sector size. Second,
function get_lba_geom add code which calculates LBA/CHS geometry -- it
supports file images and block devices (on linux you need special code
for every type; either stat() or BLKGETSIZE/BLKSSZGET). And the last
part changes error messages to reflect new code.

Post by Alain Knaff
Thanks,
Alain

[Alain Knaff]

On 2018-09-24 10:23, Pali Rohár wrote:
[...]

Post by Pali Rohár
Hm.. there are no messages printed to stdout. And also there are no
debugging code.

I'm speaking about your patch from August 11th 15:42, which does indeed
print messages to stdout (see the lines just before and after the call
to get_lba_geom).

+ printf("sector size: %u, sectors: %u, heads: %u, tracks: %u\n",
+ 128 << (used_dev.ssize & 0x7f), used_dev.sectors, used_dev.heads, used_dev.tracks);

Post by Pali Rohár
Those messages are printed to string buffer (sprintf)
and contains errors. In same way how other functions do it.

That's how it was in 4.0.18, but your patch changes the sprintf(errmsg,
...) to printf(...) to stdout.

- sprintf(errmsg,
- "Unknown geometry "
- "(You must tell the complete geometry "
- "of the disk, \neither in /etc/mtools.conf or "
- "on the command line) ");
- continue;
+ printf("%s: "
+ "Complete geometry of the disk was not specified, \n"



Ok, maybe that wasn't the version you intended to send me? That's why I
asked you to doublecheck, and send me it again if ever you've
accidentally sent me the wrong version. Mistakes do happen, that's not a
problem, but right now I worry that there might be other issues in the
version I got which might be less easy to spot than the error messages...

Post by Pali Rohár

Post by Alain Knaff
Also, there is some puzzling "late capping" of sector_size which should
be unneeded (ssize is already being capped at input), and which would
introduce inconsistencies if for some reason it did indeed kick in.

No. ssize (= sector size) is no set correctly from block device.

... and that's where it should be capped, and not late in the game where
other code parts may already have used the initial "too high" setting.
The risk here is inconsistencies, where in some parts of the code assume
a sector size bigger than 4K, and others 4K. And another risk is that it
breaks those situations (2m floppies) where a bigger size than 4K _is_
acceptable.

[...]

Regards,

Alain

[Pali Rohár]

Post by Alain Knaff
[...]

Post by Pali Rohár
Hm.. there are no messages printed to stdout. And also there are no
debugging code.

I'm speaking about your patch from August 11th 15:42, which does indeed
print messages to stdout (see the lines just before and after the call
to get_lba_geom).
+ printf("sector size: %u, sectors: %u, heads: %u, tracks: %u\n",
+ 128 << (used_dev.ssize & 0x7f), used_dev.sectors, used_dev.heads, used_dev.tracks);

Post by Pali Rohár
Those messages are printed to string buffer (sprintf)
and contains errors. In same way how other functions do it.

That's how it was in 4.0.18, but your patch changes the sprintf(errmsg,
...) to printf(...) to stdout.
- sprintf(errmsg,
- "Unknown geometry "
- "(You must tell the complete geometry "
- "of the disk, \neither in /etc/mtools.conf or "
- "on the command line) ");
- continue;
+ printf("%s: "
+ "Complete geometry of the disk was not specified, \n"
Ok, maybe that wasn't the version you intended to send me?

It is correct version. I forgot that I put those messages on stdout.
Sorry for that.

Anyway, I thought that it would be useful message for end-user that
autodetection of parameters did not work and that some calculation was
done. So it print parameters which it then use.

So do you think that those messages about CHS geometry should not be
printed?

Post by Alain Knaff
That's why I asked you to doublecheck, and send me it again if ever you've
accidentally sent me the wrong version. Mistakes do happen, that's not a
problem, but right now I worry that there might be other issues in the
version I got which might be less easy to spot than the error messages...

Post by Pali Rohár

Post by Alain Knaff
Also, there is some puzzling "late capping" of sector_size which should
be unneeded (ssize is already being capped at input), and which would
introduce inconsistencies if for some reason it did indeed kick in.

No. ssize (= sector size) is no set correctly from block device.

... and that's where it should be capped, and not late in the game where
other code parts may already have used the initial "too high" setting.
The risk here is inconsistencies, where in some parts of the code assume
a sector size bigger than 4K, and others 4K. And another risk is that it
breaks those situations (2m floppies) where a bigger size than 4K _is_
acceptable.

So... where in the code it should be? I looked at the code and I thought
that correct place for BLKSSZGET is in get_block_geom().

... Or when talking about 4K, do you mean check "Fs.sector_size > 4096"?
Because there are two different things, one is file system sector size
and one block device sector size.

If there are devices/systems which uses "Fs.sector_size > 4096", then
that check should not be introduced.

But I was told that such devices/systems do not exists and FAT sector
size is maximally 4096.

Post by Alain Knaff
[...]
Regards,
Alain

[Alain Knaff]

On 2018-09-24 12:26, Pali Rohár wrote:
[...]

Post by Pali Rohár
It is correct version. I forgot that I put those messages on stdout.
Sorry for that.
Anyway, I thought that it would be useful message for end-user that
autodetection of parameters did not work and that some calculation was
done. So it print parameters which it then use.
So do you think that those messages about CHS geometry should not be
printed?

I think they should only be printed if/when an actual error occurs (...
or if mformat has been invoked with a "verbose" flag...).

It is also expected that more than one drive definition might need to be
tried, hence the idea of first "printing" error messages into a buffer,
and only actually outputting them once it is clear that *all* drive
definitions have failed.

==> so I say: ok with the messages, if it's an actual error, and if they
are handled like all the other messages (errmsg)

[...]

Post by Pali Rohár

Post by Alain Knaff
... and that's where it should be capped, and not late in the game where
other code parts may already have used the initial "too high" setting.
The risk here is inconsistencies, where in some parts of the code assume
a sector size bigger than 4K, and others 4K. And another risk is that it
breaks those situations (2m floppies) where a bigger size than 4K _is_
acceptable.

So... where in the code it should be? I looked at the code and I thought
that correct place for BLKSSZGET is in get_block_geom().

Yes, indeed, that would be an appropriate place.

Post by Pali Rohár
... Or when talking about 4K, do you mean check "Fs.sector_size > 4096"?

No, that's exactly what I *don't* want. I'd rather have a check for
used_dev.ssize <= 5
That way, if other code parts depend on used_dev.ssize, they get the
capped information too. I'd hate to end up with a disk specifying a
sector size of 16K in one place and actually using 4K in another place.

Also, 2m disks or other exotic floppy formats may have bigger "sectors"
legitimately.

Post by Pali Rohár
Because there are two different things, one is file system sector size
and one block device sector size.

I'd prefer to keep them as consistent with each other as possible.

Post by Pali Rohár
If there are devices/systems which uses "Fs.sector_size > 4096", then
that check should not be introduced.

There are.

Post by Pali Rohár
But I was told that such devices/systems do not exists and FAT sector
size is maximally 4096.

Post by Alain Knaff
[...]
Regards,
Alain

Regards,

Alain