Skip to content

Conversation

@StoneMonarch
Copy link
Contributor

Going down the rabbit hole of trying to get permissions out of the ext4 img, turns out that wasn't implemented.

Started by adding permissionsToMode() to inode as the heavy lifting was already done and we just needed to convert the fields in the inode struct to os.FileMode

Then in ext4.Stat() we can add the file mode with mode: in.permissionsToMode(),

Getting the owner and group I added a StatT struct to FileInfo that contains the UID and GID of the file, this is set when the FileInfo object is created.

chown and chmod are relatively easy. Check the file exists, apply the permissions and then call fs.writeInode()

Added the tests as well for chmod and chown

In the test we can see the usage of .Sys(). This returns any so we have to tell it what it is returning with stat, ok := fi.Sys().(*StatT). For now StatT only contains UID and GID.

Also got the ext4 test time down by 3x

@StoneMonarch StoneMonarch changed the title Add inital suport for chmod and chown as well as the abllity to read permissions. Add inital suport for chmod and chown on ext4 as well as the abllity to read permissions. Jan 5, 2026
@StoneMonarch StoneMonarch marked this pull request as draft January 6, 2026 01:23
@StoneMonarch
Copy link
Contributor Author

I really dont like the useage of stat, ok := fi.Sys().(*StatT). Im going to look for a better solution. I think this should just return the interface in a way to be used like stat := fi.Sys()

@StoneMonarch StoneMonarch marked this pull request as ready for review January 6, 2026 01:46
@StoneMonarch
Copy link
Contributor Author

It seems like type assertion with fi.Sys().(*StatT) is the way to keep the interface genaric and implement and ext4 specific interface.

In another test youve already used fileImpl, ok := fileIntf.(*File) to assert the ext4 File struct on top of filesystem.File

@deitch
Copy link
Collaborator

deitch commented Jan 6, 2026

This is a nice bit of addition. Thank you.

turns out that wasn't implemented

Correct (as you discovered). We needed the Chmod and Chown to be in place in order to implement the Filesystem interface, and we knew we would need it anyways, just didn't get to it.

It seems like type assertion with fi.Sys().(*StatT) is the way to keep the interface genaric and implement and ext4 specific interface.

Right. Not just here, that is the way it is done in general with fs.FileInfo.Sys().

Also got the ext4 test time down by 3x

Yeah, that bs=1024 should have been rather obvious. 🤦‍♂️ Thanks for catching it.

@deitch deitch merged commit c593020 into diskfs:master Jan 6, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants