Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for APFS and Sierra's HFS #1

Closed
wants to merge 2 commits into from

Conversation

TylerLoch
Copy link

Sierra changes HFS' fs_type_num to 23 (as mentioned here: jrk/afsctool#3), and APFS, which seems to support compression using the same API, uses fs_type_num 29.
This expands the check in fileIsCompressable to accept all three fs_type_num possibilities.

Tested using HFS and APFS disk images on macOS 10.12.4.

This is my first pull request...please be gentle :)

Sierra changes HFS' fs_type_num to 23, and APFS (which also seems to support compression using the same API) uses fs_type_num 29. This expands the check in fileIsCompressable to accept all three fs_type_num possibilities.
@RJVB
Copy link
Owner

RJVB commented Mar 29, 2017 via email

@TylerLoch
Copy link
Author

TylerLoch commented Mar 29, 2017

I just did a cursory check of bsd/vfs/vfs_conf.c XNU sources, and it seems those numbers were never used, and numbers are never re-used.
Though now vfs_conf.c is no longer the canonical source for these values. HFS and APFS don't even show up there:
https://twitter.com/raimue/status/801723843224240128
https://twitter.com/realmrpippy/status/803684601017159680

APFS's value I found through adding a simple printf debug to afsctool. There's conflicting info found here, which states that FAT32 uses 29 in Sierra: https://groups.google.com/forum/#!msg/uk.comp.sys.mac/2BaK45HenzU/5VN8Ksb-CQAJ
In my tests, FAT32 returns 31...

A version check seems reasonable, or perhaps there's a better way to detect filesystem type?

As for APFS transparent compression, no mention was ever made about compression during presentations, and no currently-existing documentation mentions it. Final documentation has yet to be published, of course. In the meantime, APFS must support HFS' extended-attribute compression "hack" if it is to convert HFS volumes.

@RJVB
Copy link
Owner

RJVB commented Mar 29, 2017 via email

@TylerLoch
Copy link
Author

The most recent edit now compares by string (case-insensitively).

@RJVB
Copy link
Owner

RJVB commented Mar 30, 2017 via email

RJVB added a commit that referenced this pull request Mar 31, 2017
This should be more future-proof and less prone to side-effects than using
the undocumented f_type statfs field.

Contributed by Tyler Loch:
#1
@RJVB
Copy link
Owner

RJVB commented Mar 31, 2017

Thanks for the contribution. I've cherry-picked it.

afsctool.c Outdated
@@ -92,7 +92,7 @@ bool fileIsCompressable(const char *inFile, struct stat *inFileInfo)
// ret >= 0 && fsInfo.f_type == 17
// && S_ISREG(inFileInfo->st_mode)
// && (inFileInfo->st_flags & UF_COMPRESSED) == 0 );
return (ret >= 0 && (fsInfo.f_type == 17 || fsInfo.f_type == 23 || fsInfo.f_type == 29)
return (ret >= 0 && (!strcasecmp("hfs", fsInfo.f_fstypename) || !strcasecmp("apfs", fsInfo.f_fstypename))
&& S_ISREG(inFileInfo->st_mode)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm so would need to add "zfs" here too, since we finally figured out how that decmpfs stuff works (..enough to undo it!). What WOULD have been neat here is a test like
VOL_CAPABILITIES_FORMAT] & VOL_CAP_FMT_DECMPFS_COMPRESSION
So the FS can decide if it supports it or not. Since testing for "hfs" was the reason we had to add mimic-feature :) rsync and ditto both use capabilites test.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RJVB
Copy link
Owner

RJVB commented Dec 19, 2017 via email

@lundman
Copy link

lundman commented Dec 19, 2017

Well, this all started, because Apple's Mail does not check VOL_CAP_FMT_DECMPFS_COMPRESSION and just plows on with decmpfs anyway (since 10.13). So quite a lot of debugging was done just because they do not use their own API, and got confused when their files where 0-length on ZFS.
Also, we added mimic just because applications would string compare against "hfs", like that of Adobe, again, just because they didn't use VOL_CAP_FMT_DECMPFS_COMPRESSION.

So, It would be pleasing if you guys would also use VOL_CAP_FMT_DECMPFS_COMPRESSION since that is what it is for. If you don't want to, then fine, but we will then lie to afsctool just to get our way :)

So that brings us to now, with ZFS I was forced to add support to handle decmpfs, so that Mail will work on a ZFS dataset. I actually do NOT set VOL_CAP_FMT_DECMPFS_COMPRESSION because it would be better to not bother using decmpfs with ZFS (it has internal compression(s) far better than decmpfs after all) - but yes, it can handle ditto/rsync --hfscompression copying a file over, creating the namedstream and truncating the file (it has not yet landed in master branch yet). So, I intend to make us say no to VOL_CAP_FMT_DECMPFS_COMPRESSION for zfs, but yes if mimic is set. Which is how it should be really. Both apfs and hfs reply yes, as they should.

@RJVB
Copy link
Owner

RJVB commented Dec 19, 2017 via email

@lundman
Copy link

lundman commented Dec 19, 2017

actually happens with libarchive (bsdtar)
Ah neat, another one I did not know about. It should in theory now work.

Yeah, and what would your way be? As far as I can tell the whole problem is that you may already by lying to afsctool, telling it you're HFS while you're evidently not.

Yes and no. I do not want to lie, I want to wave the ZFS flag proudly, and zfs will always report as zfs, and with the correct capabilities, by default. But, users required the mimic feature for the apps that fail to check capabilities. So, in for a penny, in for a pound. If (and only if) we are to mimic hfs, we will do so fully. This is actually also needed when booting OSX on zfs, on at the very least the "/" mount. After that, mimic can be off. Mimic "on" means we have about 40 HFS IOCTLs that we suddenly had to support (get next hard-link etc). Quite tedious.

But, as it happens, you can detect zfs under mimic too, as it happens, the zfs userland needs to know. I set the statfs "subtype" to MNTTYPE_ZFS_SUBTYPE ('Z'<<24|'F'<<16|'S'<<8).

I do not want much from Afsctool really, but I want you to want to do the right thing. :) It is much better to use capabilities, than hardcoded string check. It is what creates this hassle to start with.

All this still doesn't really tell me what happens when you receive a hfs-compressed file

Oh right, I forgot. Right now, the last thing they do when using decmpfs, is to set UF_COMPRESSED. So in vnop_setattr (chflags) when zfs writes the flag change, we call decmpfs_decompress_file(), which converts the compressed namedstream back to plain text file, cleans up the xattrs and unsets UF_COMPRESSED.

Yep. I big waste of cpu cycles, all because Mail blindly compresses. (Scarily, I tried to return error in makenamedstream, in setxattr, and chflags - Mail ignores all errors and just keeps going. hurrraah)

So, in a perfect world, zfs reports as zfs - and nothing tries to compress. But, if mimic is on, or poor software is running, we can at least handle it by decompressing. Fragmentation is not an issue, as ZFS is COW. The decompress takes just as many cycles as HFS read would, but the net effect is a waste.

So, what about afsctool and zfs? Well, nothing really. But, I do need to test that we handle decmpfs correctly (in the tester environment we run all the time) so it would totally be neat if I could turn mimic on, (hence capabilities say yes) and confirm that a compressed write to zfs will not be a zero-length file using the afsctool.

But, I can just keep using ditto, if that is ultimately preferred. but opensource projects tend to be better, as Apple sometimes just change things without telling anyone.

@RJVB
Copy link
Owner

RJVB commented Dec 19, 2017 via email

@ilovezfs
Copy link

Second guessing the capability the file system reports doesn't make much sense to me. If the capability is asserted as supported, assume it is supported and proceed accordingly. If capability is asserted as unsupported, assume it is unsupported and proceed accordingly.

I don't see what is gained by substituting your judgment for the judgment of the person setting the capability flag on or off.

@RJVB
Copy link
Owner

RJVB commented Dec 19, 2017 via email

RJVB added a commit that referenced this pull request Dec 27, 2017
This is the preferred method to determine if the filesystem hosting a
file supports HFS (decmpfs) compression. Also detect ZFS for Mac
(o3x.org) which can mimic HFS and will then decompress files on the fly.
afsctool will not waste cycles on compression and rewriting files
unless requested with the HFSCOMPRESS_TO_ZFS CMake option (for testing).

#1 (comment)
@RJVB
Copy link
Owner

RJVB commented Dec 27, 2017 via email

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.

4 participants