WIP: Add macOS support #11273

lundman · 2020-12-03T09:34:39Z

Add macOS support.

Work has progressed well, recent release of macOS-2.0.0rc7 version support macOS-10.9 to macOS-11 to include M1/arm64 architecture. It is our wish that this PR is close to the final version, and should probably start to take commits from here and promote to individual PRs where that make sense.

Motivation and Context

More the merrier, over, too many chefs.

Description

Commits prefixed with "Upstream:" change universal files and should be inspected.
Commits prefixed with "macOS:" only affects macOS specific files. Code review still appreciated though.

zfs-tester work is incomplete, much more tweaking needed for compatibility. A large zfs-tester PR is being worked on.

This is a large PR, and it is expected that code-review will ask many commit to be changed.

Currently macOS compile to pkg, or loading, would be:

# brew install automake libtool gawk coreutils openssl gettext
# ./configure CPPFLAGS="-I/usr/local/opt/gettext/include -I/usr/local/opt/[email protected]/include" LDFLAGS="-L/usr/local/opt/[email protected]/lib -L/usr/local/opt/gettext/lib" CFLAGS="-O2 -g" --sysconfdir=/etc --localstatedir=/var --prefix=/usr/local/zfs/ --sbindir=/usr/local/zfs/bin
# make

To load
# ./scripts/load_macos.sh
# ./scripts/cmd-macos.sh zpool version

To installer pkg
# ./scripts/pkg_macos.sh -L
-rw-r--r--    1 lundman  admin  14679210 Dec  2 19:49 OpenZFSonOsX-2.0.0-Catalina-10.15.pkg

How Has This Been Tested?

By users, by local zfs-tests, and Appveyor zfs-tests.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Performance enhancement (non-breaking change which improves efficiency)
Code cleanup (non-breaking change which makes code smaller or more readable)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the OpenZFS code style requirements.
I have updated the documentation accordingly.
I have read the contributing document.
I have added tests to cover my changes.
I have run the ZFS Test Suite with this change applied.
All commit messages are properly formatted and contain Signed-off-by.

lundman · 2020-12-03T09:54:58Z

Well, that's a bit of a disaster. :)

ghost

I've started looking through the common code parts and putting stars where things look like they could be easily upstreamed independently.

ghost · 2020-12-03T14:11:13Z

configure.ac

-	m4_esyscmd(grep ^Version: META | cut -d ':' -f 2 | tr -d ' \n'))
+m4_define([NAME], m4_esyscmd([awk '/^Name:/ {print $2}' META | tr -d '\n']))
+m4_define([VERS], m4_esyscmd([awk '/^Version:/ {print $2}' META | tr -d '\n']))
+AC_INIT(NAME, VERS)


config/Rules.am

ghost · 2020-12-03T14:24:50Z

cmd/zed/zed_conf.c

+#ifdef __APPLE__
+ssize_t readv(int, const struct iovec *, int);
+ssize_t writev(int, const struct iovec *, int);
+#endif


Could this go in include/os/macos/spl/sys/uio.h?

Ah, lib/libspl/include/os/macos/sys/uio.h
Nope - ZOL made the decision to not use include paths for some cmd/ entries, so that file isn't used - which is a shame. But far more work/frustration to touch their build system, then to include those prototypes.

ghost · 2020-12-03T14:34:50Z

include/sys/dbuf.h

@@ -439,7 +439,7 @@ dbuf_find_dirty_eq(dmu_buf_impl_t *db, uint64_t txg)
 	char __db_buf[32]; \
 	uint64_t __db_obj = (dbuf)->db.db_object; \
 	if (__db_obj == DMU_META_DNODE_OBJECT) \
-		(void) strcpy(__db_buf, "mdn"); \
+		(void) strlcpy(__db_buf, "mdn", sizeof (__db_buf));	\


ghost · 2020-12-03T14:35:33Z

include/sys/dnode.h

@@ -600,7 +600,7 @@ extern dnode_stats_t dnode_stats;
 	char __db_buf[32]; \
 	uint64_t __db_obj = (dn)->dn_object; \
 	if (__db_obj == DMU_META_DNODE_OBJECT) \
-		(void) strcpy(__db_buf, "mdn"); \
+		(void) strlcpy(__db_buf, "mdn", sizeof (__db_buf));	\


lib/libicp/Makefile.am

lib/libspl/Makefile.am

ghost · 2020-12-03T14:55:15Z

lib/libspl/asm-x86_64/atomic.S

 	 * We define them separately, instead of using an ALTENTRY
 	 * definitions to alias them together, so that DTrace and
-	 * debuggers will see a unique address for them, allowing 
+	 * debuggers will see a unique address for them, allowing


lib/libuutil/Makefile.am

lundman · 2020-12-04T06:46:18Z

./scripts/zfs-tests.sh:669:11: warning: Assigning an array to a string! Assign as array, or use * instead of @ to concatenate. [SC2124]
make: *** [shellcheck] Error 1
Makefile:1489: recipe for target 'shellcheck' failed

Very clearly says warning!

config/Rules.am

ghost · 2020-12-05T13:16:38Z

As a general note, the dist target (so DIST_SUBDIRS, EXTRA_DIST, stuff like that) is used for building the source distribution archive. It should include all the files necessary to build regardless of platform.

PrivatePuffin

I've gone through some of the more style and procedure based changes.
While it looks generally sane, there are some nits here and there imho...

PrivatePuffin · 2020-12-12T12:39:27Z

appveryor.yml

+version: 1.0.{build}
+branches:
+  only:
+  - macos
+image: macOS
+build_script:
+- sh: >-
+    pwd
+    ls -l
+    sh autoconf.sh
+	./configure CPPFLAGS="-I/usr/local/opt/gettext/include -I/usr/local/opt/[email protected]/include" LDFLAGS="-L/usr/local/opt/gettext/lib/ -L/usr/local/opt/[email protected]/lib"
+	make


Github seems to also have a workflow runner for OSX, as we currently use either buildbot or workflows, that might be prefered....

PrivatePuffin · 2020-12-12T12:42:11Z

cmd/os/macos/InvariantDisks/.gitignore

+/DerivedData/
+*.xcworkspace/
+xcuserdata/


is CMD the right location for InvariantDisks?
Maybe also add a short readme listing what this thing is actually for, for those of us not familiar with the OSX codestack?

Made a cmd/os/macos/ so that other platforms wouldn't need to worry about it, but some general description could be nice (same goes for ZOL cmds I have no idea what they do) As it happens, InvariantDisk is a separate process started by launchd to maintain a symlink-tree of storage devices, ie, /var/run/disk/by-serial/X -> /dev/diskX. To work around disk renumbering problem.

Same indeed also goes for some ZoL commands, yes...

With that description, as a sub process is cmd the right location?

PrivatePuffin · 2020-12-12T12:42:55Z

cmd/os/macos/zfs_util/PkgInfo

@@ -0,0 +1 @@
+fs  ????


PrivatePuffin · 2020-12-12T12:46:08Z

contrib/macOS/resources/English.lproj/License.txt

+Much of OpenZFS on OS X is a port of ZFS on Linux.
+
+--------------------------------------------------------------------
+ZFS on Linux COPYRIGHT


many ZoL references...
While we are actually working actively on removing those, in favor of OpenZFS

Ah these pkg files haven't been looked at since the start - could do with a refresher.

PrivatePuffin · 2020-12-12T12:47:19Z

contrib/macOS/resources/English.lproj/License.txt

+OpenZFS on OS X consists of software that is covered under the
+Common Development and Distribution License 1.0 and the Apple
+Public Source License 2.0.


Are those actually compatible?
Why use a totally difference license for the OSX code? It doesn't really make sense to me.
We already have CDDL, GPL and BSD code mixed in, and now we also have 2(!?) different apple OSS licences?!

Before sneaking in licenses, it might actually be appropiate to get legal council specialised in US copyright law to sign off on mixing those.

Some files are used verbatim, like kextsymboltool.c - so its more of a "3rd party license" list I think. All code I have written personally is CDDL.

Good to know, but how is this made clear in-code?

PrivatePuffin · 2020-12-12T12:48:10Z

contrib/macOS/resources/English.lproj/ReadMe.rtf

+\
+The Original Code and all software distributed under the License are distributed on an 'AS IS' basis, WITHOUT WARRANTY OF ANY KIND, EITHER EXPRESS OR IMPLIED, AND APPLE HEREBY DISCLAIMS ALL SUCH WARRANTIES, INCLUDING WITHOUT LIMITATION, ANY WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE, QUIET ENJOYMENT OR NON-INFRINGEMENT.\
+\
+Please see the License for the specific language governing rights and limitations under the License.}


sempervictus · 2020-12-16T20:55:32Z

Great to see this coming under the umbrella. Convincing clients to use it (on non-boot media) is somewhat of a challenge as its "out-of-tree" for OpenZFS and they worry that it will eat their data. We've used @lundman's installers/work for years whenever we have to do mac-stuff where ZFS helps achieve the goal. Will be great to have it in-sync with master and reviewed by that many more skilled eyes.

Add macOS to the build system. Signed-off-by: Jorgen Lundman <[email protected]>

DurvalMenezes · 2021-02-24T07:41:03Z

Great to see this coming under the umbrella. Convincing clients to use it (on non-boot media) is somewhat of a challenge as its "out-of-tree" for OpenZFS and they worry that it will eat their data. We've used @lundman's installers/work for years whenever we have to do mac-stuff where ZFS helps achieve the goal. Will be great to have it in-sync with master and reviewed by that many more skilled eyes.

Ditto here. In my case I've managed to convince the clients and I'm using an old version from Lundman's (1.9.3) successfully, but am unable to use newer (1.9.4+) versions due to subtle incompatibilities in zfs send/recv streams between it and FreeBSS 11 ZFS. My hope is that when it's all under one umbrella, these incompatibilities will be solved and it will all be one great ZFS family :-) So please, everyone keep up the good work and let's get this thing working. If needed, I will volunteer as a tester.

Attempt to group most of the sweeping changes to headers in there, unless they fit better with an individual commit Signed-off-by: Jorgen Lundman <[email protected]> It appears FreeBSD did the same for zfs_ioctl_register_dataset_nolog() as they use it, so following suit for zfs_ioctl_register_pool()

Honestly, don't know if this is required, but XNU does like va_type and va_mode Signed-off-by: Jorgen Lundman <[email protected]>

Apparently upstream has already made this change for mount, so add the same support to umount. OSX wants it for devdisks, and handling of snapshots. How unattractive is it with the type difference between mount, and unmount? Signed-off-by: Jorgen Lundman <[email protected]>

Signed-off-by: Jorgen Lundman <[email protected]>

Is this the best way? We could add ", func, private" to the existing IO, and either send by uio, or by func(private). Signed-off-by: Jorgen Lundman <[email protected]>

"zfs mount dataset@snapshot" as mounting of snapshot has to be done manually from userland in macOS. Signed-off-by: Jorgen Lundman <[email protected]>

For kernel to send snapshot mount/unmount events to zed. Signed-off-by: Jorgen Lundman <[email protected]>

Add zfs_rollback_os() call to the rollback logic, so platforms can do specific requirements. macOS: need to kick Finder to update. Signed-off-by: Jorgen Lundman <[email protected]>

Add all files required for the macOS port. Add new cmd/os/ for tools which are only expected to be used on macOS. This has support for all macOS version up to Catalina. (Not BigSur). Signed-off-by: Jorgen Lundman <[email protected]> macOS: big uio change over. Make uio be internal (ZFS) struct, possibly referring to supplied (XNU) uio from kernel. This means zio_crypto.c can now be identical to upstream. Update for draid, and other changes macOS: Use SET_ERROR with uiomove. [squash] macOS: they went and added vdev_draid macOS: compile fixes from rebase macOS: oh cstyle, how you vex me so macOS: They added new methods - squash macOS: arc_register_hotplug for userland too Upstream: avoid warning zio_crypt.c:1302:3: warning: passing 'const struct iovec *' to parameter of type 'void *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers] kmem_free(uio->uio_iov, uio->uio_iovcnt * sizeof (iovec_t)); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ macOS: Update zfs_acl.c to latest This includes commits like: 65c7cc4 1b376d1 cfdc432 716b53d a741b38 485b50b macOS: struct vdev changes macOS: cstyle, how you vex me [squash] Upstream: booo Werror booo Upstream: squash baby Not defined gives warnings. Upstream: Include all Makefiles Signed-off-by: Jorgen Lundman <[email protected]> double draid!

Due to some differences in assembler work, macOS will have own copies. It would be desirable to change all assembler files to use asm_linkage.h and the macros inside for better portability. Signed-off-by: Jorgen Lundman <[email protected]>

Signed-off-by: Jorgen Lundman <[email protected]>

Start to add macOS support to the zfs-tester environment, much more work is required still. Signed-off-by: Jorgen Lundman <[email protected]>

Signed-off-by: Jorgen Lundman <[email protected]>

Hoping to remove it eventually.

Upstream: cut redundant lines [squash autoconf]

Upstream: define EMPTY_TASKQ

This reverts commit 6d5691c. The recent upstream changes to abd and how the size is calculated appears to have addressed the size issue (as the VERIFY no longer gets triggered).

It appears that abd_get_offset_struct() can be called and supply an existing abd reference, which is initialised. But there is no "put" method, so when it is re-used, it will re-initialise the fields again, including mutex_init() which causes a panic: (abd.c:568)> abd_init_struct: avoiding panic for abd 0xffffff802f4f26e0 (abd.c:568)> abd_init_struct: avoiding panic for abd 0xffffff802f4e1900 (abd.c:568)> abd_init_struct: avoiding panic for abd 0xffffff802f4f8220 (abd.c:568)> abd_init_struct: avoiding panic for abd 0xffffff802f4e1900 (abd.c:568)> abd_init_struct: avoiding panic for abd 0xffffff802f4f26e0 (abd.c:568)> abd_init_struct: avoiding panic for abd 0xffffff802f4f8220

Even when supplied with an abd to abd_get_offset_struct(), the call to abd_get_offset_impl() can allocate a different abd. Ensure to call abd_fini_struct() on the abd that is not used. Signed-off-by: Jorgen Lundman <[email protected]>

This reverts commit 2e94b1a. Fixed by 5815421

jaredjones · 2021-03-28T23:25:57Z

Any updates on getting this merged? I absolutely love using ZFS on macOS and am excited to see this ported to the official repo.

lundman · 2021-03-30T09:36:21Z

Working on it, just needs to ferment a bit more, to get a kick. We only just got ARM64 version to work, and it's been a decent step backwards to appease APPLE, that we need to recover from.

lundman · 2021-05-19T03:17:43Z

Hmm this PR no longer notices that I've pushed to the branch. But it could have something to do with us de-Forking. Anything special needed to make the PR wake again?

behlendorf · 2021-05-19T04:36:39Z

@lundman force updating the branch should be enough, maybe give it another try. It looks like for whatever reason Github didn't notice the branch update and post an event.

lundman · 2021-05-24T06:50:29Z

Closing in favour of #12110

ghost reviewed Dec 3, 2020

View reviewed changes

behlendorf added the Status: Code Review Needed Ready for review and testing label Dec 3, 2020

lundman force-pushed the macOS branch from 8ce236b to ad71f9d Compare December 4, 2020 06:38

ghost reviewed Dec 4, 2020

View reviewed changes

config/Rules.am Outdated Show resolved Hide resolved

PrivatePuffin reviewed Dec 12, 2020

View reviewed changes

lundman force-pushed the macOS branch from c7e9bd6 to 2c7287f Compare December 16, 2020 05:28

behlendorf self-requested a review December 16, 2020 19:12

lundman force-pushed the macOS branch from fce44f2 to 093da6c Compare December 17, 2020 04:57

behlendorf mentioned this pull request Jan 20, 2021

libzutil: optimize zpool_read_label with AIO #11467

Closed

13 tasks

Upstream: Add macOS to autoconf and makefiles

0527f66

Add macOS to the build system. Signed-off-by: Jorgen Lundman <[email protected]>

lundman force-pushed the macOS branch from 093da6c to d81ad0e Compare March 1, 2021 00:24

lundman added 13 commits March 1, 2021 09:44

Upstream: macOS can also set va_type

e48e9fb

Honestly, don't know if this is required, but XNU does like va_type and va_mode Signed-off-by: Jorgen Lundman <[email protected]>

Upstream: macOS default mount is /Volumes

ecfb378

Signed-off-by: Jorgen Lundman <[email protected]>

Upstream: add IO calls for iokit

8f046e1

Is this the best way? We could add ", func, private" to the existing IO, and either send by uio, or by func(private). Signed-off-by: Jorgen Lundman <[email protected]>

Upstream: Allow cmd/zfs mount unmount of snapshots

747d1be

"zfs mount dataset@snapshot" as mounting of snapshot has to be done manually from userland in macOS. Signed-off-by: Jorgen Lundman <[email protected]>

Upstream: Add snapshot events

ff119e8

For kernel to send snapshot mount/unmount events to zed. Signed-off-by: Jorgen Lundman <[email protected]>

Upstream: cmd/zfs rollback [squash]

dfebf05

Add zfs_rollback_os() call to the rollback logic, so platforms can do specific requirements. macOS: need to kick Finder to update. Signed-off-by: Jorgen Lundman <[email protected]>

Upstream: ASM changes to support macOS

a93f986

Due to some differences in assembler work, macOS will have own copies. It would be desirable to change all assembler files to use asm_linkage.h and the macros inside for better portability. Signed-off-by: Jorgen Lundman <[email protected]>

Upstream: autoconf revisions

2a1e266

Signed-off-by: Jorgen Lundman <[email protected]>

Upstream: zfs-tests to support macOS

c0749c6

Start to add macOS support to the zfs-tester environment, much more work is required still. Signed-off-by: Jorgen Lundman <[email protected]>

Upstream: skip cstyle on kextsymboltool.c

7267096

Signed-off-by: Jorgen Lundman <[email protected]>

lundman added 10 commits March 1, 2021 10:40

Upstream: macOS currently has own zfs_fsync

666dff8

Hoping to remove it eventually.

Upstream: work around different API for sbuf_finish()

e09a973

Upstream: revision changes.

68ac9f1

Upstream: cut redundant lines [squash autoconf]

macOS: If we don't set SOURCES it makes up zfs.c from nowhere

bdbf932

Upstream: Why is linux even trying to look at etc/launchd

f95e2a8

random build test - does it need DIST_SUBDIRS?

ed65f67

Upstream: Didn't move asm files after all

19454ba

Upstream: Missing empty taskq for userland

1f00192

Upstream: define EMPTY_TASKQ

Upstream: stick with linux style function for now

75aa9eb

Upstream: dmu_redact.c does not call bqueue_destroy

b4a3afd

lundman force-pushed the macOS branch from d81ad0e to b4a3afd Compare March 1, 2021 01:41

lundman added 3 commits March 1, 2021 11:28

macOS: missing function prototype

7226d92

Revert "Upstream: abd keep orig_size for free"

f8e04c2

This reverts commit 6d5691c. The recent upstream changes to abd and how the size is calculated appears to have addressed the size issue (as the VERIFY no longer gets triggered).

lundman force-pushed the macOS branch from 01b73b9 to 2e94b1a Compare March 1, 2021 08:24

lundman added 2 commits March 3, 2021 13:57

Revert "Temporary: hack to handle abd reuse"

8460150

This reverts commit 2e94b1a. Fixed by 5815421

lundman force-pushed the macOS branch from bacf1f9 to 8460150 Compare April 12, 2021 00:17

lundman closed this May 24, 2021

lundman reopened this May 24, 2021

lundman force-pushed the macOS branch from f5aa016 to 8460150 Compare May 24, 2021 06:33

lundman closed this May 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Add macOS support #11273

WIP: Add macOS support #11273

lundman commented Dec 3, 2020 •

edited

Loading

lundman commented Dec 3, 2020

ghost left a comment

ghost Dec 3, 2020

ghost Dec 3, 2020

lundman Dec 4, 2020

ghost Dec 3, 2020

ghost Dec 3, 2020

ghost Dec 3, 2020

lundman commented Dec 4, 2020

ghost commented Dec 5, 2020

PrivatePuffin left a comment

PrivatePuffin Dec 12, 2020

PrivatePuffin Dec 12, 2020

lundman Dec 13, 2020

PrivatePuffin Dec 13, 2020

PrivatePuffin Dec 12, 2020

PrivatePuffin Dec 12, 2020

lundman Dec 13, 2020

PrivatePuffin Dec 12, 2020 •

edited

Loading

lundman Dec 13, 2020

PrivatePuffin Dec 13, 2020

PrivatePuffin Dec 12, 2020

sempervictus commented Dec 16, 2020

DurvalMenezes commented Feb 24, 2021

jaredjones commented Mar 28, 2021

lundman commented Mar 30, 2021

lundman commented May 19, 2021

behlendorf commented May 19, 2021

lundman commented May 24, 2021

WIP: Add macOS support #11273

WIP: Add macOS support #11273

Conversation

lundman commented Dec 3, 2020 • edited Loading

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

lundman commented Dec 3, 2020

ghost left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lundman commented Dec 4, 2020

ghost commented Dec 5, 2020

PrivatePuffin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PrivatePuffin Dec 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sempervictus commented Dec 16, 2020

DurvalMenezes commented Feb 24, 2021

jaredjones commented Mar 28, 2021

lundman commented Mar 30, 2021

lundman commented May 19, 2021

behlendorf commented May 19, 2021

lundman commented May 24, 2021

lundman commented Dec 3, 2020 •

edited

Loading

PrivatePuffin Dec 12, 2020 •

edited

Loading