OSDev.org

BenLunt wrote:Anyway, if you are writing for a hard drive, I would actually recommend the LEAN file system. I had/have the honor of working with the original author and have added and implemented a lot of its current format. I believe he (we) are ready to release version 0.7 soon.

BenLunt wrote:The LeanFS has a lot of advantages: Large files, long filenames, extents, forks, etc.

BenLunt wrote:So, why the LeanFS? There are no other utilities or driver code. I have to make everything myself. From scratch. It is the enjoyment of doing so.

BenLunt wrote:You know about my (open source) utility, but yes it is Windows only.

BenLunt wrote:However, on that same page is an image (or two) that has a LeanFS partition. You could easily test your code with it. If you need another image, please let me know what kind of details you would like on it, and I will create it and post it.

BenLunt wrote:I recommended the LeanFS to the original post in the hope that the poster would try something that isn't already done by every other project here, that they might get the satisfaction of creating something new, rather than stealing someone else's code. I learn(ed) a lot more when I start from scratch, coding the project myself, than copying someone else's code, and simply modifying it for my taste.

BenLunt wrote:Yes, the original author of the LeanFS has moved on to other projects. I don't blame him. In fact, if it wasn't for my persuasion, he probably wouldn't have even got as far as he did. However, when I have the time (and interest), I come back to it and add function to my code. Therefore, if you need help with your implementation, I am more than happy to answer any questions you have, look over your code, and run my tests on your image(s).

BenLunt wrote:The term "re-inventing the wheel" has been used a lot in this forum over the years, but I believe that if we build a new wheel for ourselves, we can learn how to do it and possibly make it better. Why not start with the LeanFS?

BenLunt wrote:Anyway, thank you for your interest. As the original author has told me many times, and I agree, this file system has been an enjoyment for the both of us. It would be exciting for myself, as with the original author, if others would implement this file system in their own projects, giving feedback throughout.

BenLunt wrote:Okay, I see what you are asking. I am in the process of adding more documentation to the specification, in preparation for the next version release. I will take your comments, which are completely justified, into consideration.

BenLunt wrote:A few example images for comparison is a simple task.

BenLunt wrote:Thanks again. I appreciate and agree with your comments.

BenLunt wrote:P.S. You did see that the author has a test suite and driver suite for Linux at http://freedos-32.sourceforge.net/lean/downloads.php, though you need wxWidgets as well.

mbrfile=../bootboot/dist/boot.bin
bootfile=../bootboot/dist/bootboot.bin
imgfile=../bootboot/images/test.bin
base_lba=0
tot_sects=32768
param0=12
head=16
spt=63

• It is unclear if bitmaps are allowed to record sectors which are *before* the bitmap itself. It seems from your code that they can.
• According to the spec, sectorCount in inode includes "The number of data sectors (thus excluding indirect sectors, and including the first sector with the inode structure) allocated for the file." (meaning empty file would have sectorCount = 1)
• However your tool seems to only record the number of actual file content data sectors (meaning empty file would have sectorCount = 0, which makes more sense imho, this field should either include all allocated sectors (including inode and indirect sectors) or it should only include the number of data sectors. Including inode but not the indirect sectors seems odd)
• It is not very well specified what bytes make up the inode checksum. I've finally gave this up, and used the same define as in your code.
• The spec should be more specific than "The checksum value for the inode structure. Any inline extended attributes following the inode structure are not part of the inode structure itself, thus must not be included in checksum calculation.", it should read as "checksum from bytes 4 to 176" or something.
• I think there's no reason why not to include the extended attributes in the checksum. Would be more robust, and you'll have to read and write the entire inode sector anyway for those.
• The size of the inode field (which I think grows when you add extended attributes) included in the checksum, so the checksum needs to be updated already when extended attributes change.

LEAN_CHK  v2.00.00    Forever Young Software 1984-2019
 Super found at LBA 1 with a checksum of 0xD0AE3DEC.
 Found version number 0.7.
 PreAllocate Count is 0
   Count would be more logical as an odd number, calculating to an even number.
 Logical Sectors per band is 12 (4096 sectors per band)
 State is 0x00000001
 Volume sector count is 32768
 Free sector count is 32745
 Bitmap starts at 2
 Super indicates root inode is at 14
 Super indicates bad sector inode is at 0
 Checking the backup Super Block sector...
  Found backup super at sector: 4095
 Total bands found: 8
 Each bands bitmap size in sectors: 1
 Counted 32745 free sectors.  Super reported 32745 free sectors
 Checking: \
Inode Number: 14
*Found unknown or illegal type in directory entry.  Type = 0x21
 Found 0 orphand sectors.
 Found 0 unmarked sectors.

 Extent Sectors used: 1
     Total Directors: 1
         Total Files: 0

        Errors Found: 0
   Diagnostics Found: 1

bzt wrote:

Code: Select all

Inode Number: 14
*Found unknown or illegal type in directory entry.  Type = 0x21

• - An inode must not be less than (32 + size of the first bitmap). LBAs 0 through 32 are reserved for the boot code and the Superblock. If you have a single sector for the bitmap, this means that the lowest Inode number must be no less than 34.
  - If I look at your Superblock, it has the following:

  Code: Select all

                 ...
            Total Sectors: 0x0000000000008000  (32768)
             Free Sectors: 0x0000000000007FE9  (32745)
        Primary Super LBA: 0x0000000000000001  (1)
         Backup Super LBA: 0x0000000000000FFF  (4095)
         Bitmap Start LBA: 0x0000000000000002  (2)
           Root Start LBA: 0x000000000000000E  (14)
            Bad Start LBA: 0x0000000000000000  (0)
                 ...

  - The Bitmap Start LBA must not be less than 33. You have it as 2.
  - The Root Start LBA must not be less than 32 + size of first bitmap. You have it as 14.
  - Next, I looked at the Inode at LBA 14, which is the Inode of the root directory:

  Code: Select all

                    ...
     Last Indirect Sector: 0x0000000000000000 (0)
                     Fork: 0x0000000000000000 (0)
                 Extent 0:  start 0x000000000000000F (15), size 0x00000001 (1)
                 Extent 1:  start 0x0000000000000000 (0), size 0x00000000 (0)
                 ...
  

  - The starting extent value must be the same as the Inode LBA. You have it as 15 (14+1).
  - I haven't looked why, but your code is placing a value of 0x2C in the first byte of the reserved[3] field within the inode.
  - You also place a value of 4 in the Links Count field. Currently there is only 1 link pointing to this file. When deleting a file, you first decrement this value. If, and only if, this value becomes zero, do you actually remove (delete) the inode.

mleanfs: it does not accept the "tot_sects", complaining about "Total Sectors does not end on cylinder boundary", which is strange, because I'm pretty sure 32768 is multiple of 4096 (1<<param0). I've also tried with spt=64 (in case cylinder here means head*sectorpertrack, so make sure 32768 is multiple of 1024), but the same result. This is just a minor issue, if I remove the check (USB sticks and SSDs don't use tracks nor cylinders anyway) everything seems to be working just fine.

lean_chk: this is the bigger problem. I've tried it on the image generated by mleanfs, and it complains about "Found version number 0.6. *Only version 0.7 is supported." and there's an error for incorrect inode size. Not sure what to do with this.

BenLunt wrote:- An inode must not be less than (32 + size of the first bitmap). LBAs 0 through 32 are reserved for the boot code and the Superblock.

The superblock must be stored in one of any sector in range from 1 to 32, inclusive.

uint64_t bitmapStart	The address of the sector where the bitmap of the first band (that is, band 0) starts. This is usually the sector right after the superblock.

BenLunt wrote:The Bitmap Start LBA must not be less than 33. You have it as 2.

BenLunt wrote:The starting extent value must be the same as the Inode LBA.

Besides being unused, each sector in the data area can be either a data sector or an indirect sector. The former contains actual data, the latter contains book keeping. Data sectors are grouped together to form extents when they are contiguous.

BenLunt wrote:I haven't looked why, but your code is placing a value of 0x2C in the first byte of the reserved[3] field within the inode.

struct S_LEAN_INODE {
  bit32u checksum;                // bit32u sum of all fields before this one.
  bit32u magic;                   // 0x45444F4E  ('NODE')
  bit8u  extent_count;            // count of extents in this inode struct.
  bit8u  inode_size;              // Inode size in dwords (176 / 4)        <-------------- used to be reserved in 0.6
  bit16u reserved;                // reserved

BenLunt wrote:You also place a value of 4 in the Links Count field. Currently there is only 1 link pointing to this file.

bzt wrote:

BenLunt wrote:- An inode must not be less than (32 + size of the first bitmap). LBAs 0 through 32 are reserved for the boot code and the Superblock.

Are you sure? The specification doesn't say anything like that. It says

Code: Select all

The superblock must be stored in one of any sector in range from 1 to 32, inclusive.

and about the bitmapStart field:

Code: Select all

uint64_t bitmapStart	The address of the sector where the bitmap of the first band (that is, band 0) starts. This is usually the sector right after the superblock.

Now if superblock can be at sector 1, and bitmap starts at the sector right after the superblock, this means the lowest root inode number cloud be at sector 3.

BenLunt wrote:The Bitmap Start LBA must not be less than 33. You have it as 2.

I haven't seen this written anywhere in the spec. It only says that bitmap should start at the sector right after the superblock.

bzt wrote:

BenLunt wrote:The starting extent value must be the same as the Inode LBA.

Are you sure? No other file system does that, and the specification says:

Code: Select all

Besides being unused, each sector in the data area can be either a data sector or an indirect sector. The former contains actual data, the latter contains book keeping. Data sectors are grouped together to form extents when they are contiguous.

I interpret this as neither inode nor indirect sectors are part of the extents, because they are not storing actual data, rather book keeping data, and extents only store actual data sectors.

 Note that the first element of this array is also -by definition- the address of the sector containing this inode structure.

bzt wrote:

BenLunt wrote:I haven't looked why, but your code is placing a value of 0x2C in the first byte of the reserved[3] field within the inode.

Because your tool checks for it? From your code:

Code: Select all

struct S_LEAN_INODE {
  bit32u checksum;                // bit32u sum of all fields before this one.
  bit32u magic;                   // 0x45444F4E  ('NODE')
  bit8u  extent_count;            // count of extents in this inode struct.
  bit8u  inode_size;              // Inode size in dwords (176 / 4)        <-------------- used to be reserved in 0.6
  bit16u reserved;                // reserved

bzt wrote:

BenLunt wrote:You also place a value of 4 in the Links Count field. Currently there is only 1 link pointing to this file.

Nope, link calculation is accurate. There are 4 directory entries pointing to the root inode: there's "." and ".." in the root directory, that's 2, and there are "a" and "b" directories which have a ".." entry pointing to the root directory. That's 4 links in total.

Actual file data start right after the inode structure or, if the iaInlineExtAttr attribute is set, at the beginning of the subsequent data sector (perhaps in the same extent).

BenLunt wrote:You're right. It doesn't actually specify this. I guess it was just implied that it was. This is something that I will need to clarify. I guess in all reality, it can be just as you state. However, the idea was that if you ever modified the boot code, you wouldn't have to move any of the data blocks if the current size was not large enough. The only thing you would have to do is move the Superblock.

BenLunt wrote:This I am absolutely sure on, since the Inode is only 176 bytes in size, *and* the file data can start directly after the Inode, thereby using the first sector as data as well. It also states right in the specs under the extentStarts[] definition:

Code: Select all

 Note that the first element of this array is also -by definition- the address of the sector containing this inode structure.

BenLunt wrote:Oh, sorry. This was a rat dropping from an old draft. I (we) haven't decided if it will remain or not.

BenLunt wrote:For example, the current image file you posted has the iaInlineExtAttr (bit 19) as zero. This indicates the opposite of "The remaining bytes after the inode structure in the first sector are reserved for inline extended attributes". Since this bit is zero, meaning do not place the extended attributes after the inode, the file data is to start after the inode. Therefore, your root directory, using only 64 bytes, will start at offset 176 in the first sector and not occupy any other sectors. Therefore, a driver could read the one and only sector and retrieve the whole inode and all of the file data with a single sector read. The ExtentStart[0] entry should be the same as the inode lba, and the size entry as you have it (1).

Code: Select all

Actual file data start right after the inode structure or, if the iaInlineExtAttr attribute is set, at the beginning of the subsequent data sector (perhaps in the same extent).

I will have to add clarification for the first few notes you mention.

BenLunt wrote:This image passes all the tests I just threw at it. I will look at it some more at a later time.

BenLunt wrote:I believe the version 0.7 specification is nearly complete. After a few more details, I just need permission to finalize it and post it.

BenLunt wrote:Thanks again for your comments. They are appreciated.