> You mention that the diffs may be slightly different by the time they land in
> illumos. Lets say that for a certain patch, the OpenZFS group approves it as
> it is tn the GH PR. Then when it goes to illumos for RTI, they would prefer
> an ASSERT be added. Weeks later after the next illumos merge, if I were
> running OpenZFS/master, would I expect the ASSERT to be there or not? If I
> knew the bug ID and searched for it in the logs, I would find the two
> versions, but which one would I expect to be running, and how do I determine
> which one it is?
In this case, the illumos version should be authoritative. Any changes
(additions or removals) should be reflected in the OpenZFS tree after the
upstream'ed commit is merged back into OpenZFS. This information should be able
to be retrieved manually, by inspecting the tree of commits (e.g. `git log
--oneline --graph`), looking for the commit in both "branches" and then
inspecting the merge commit to ensure it was merged properly. For example, take
this pull request: https://github.com/openzfs/openzfs/pull/16
I can look at the history and see exactly where it was landed in both OpenZFS
and illumos:
```
$ git log --oneline --graph openzfs/master | cut -c 1-80 | head -n 40
* adec812 6414 vdev_config_sync could be simpler Reviewed by: George Wilson <geo
* fabbbb0 6391 Override default SPA config location via environment Reviewed by:
* 1fca770 6421 Add missing multilist_destroy calls to arc_fini Reviewed by: Dan
* b621ee9 6388 Failure of userland copy should return EFAULT Reviewed by: Brian
* 48d66fc 6368 Remove superfluous statement Reviewed-by: Ned Bass <[email protected]
* 7da97da 6401 run.py should reopen files if they encounter transient I/O errors
* fc00a90 5752 dump_nvlist() is not aware of boolean array Reviewed by: Dan Kimm
* a9272e3 6389 Use (void) memcpy(), not (void *) memcpy() Reviewed by: Brian Beh
* f2ec8f3 6386 Fix function call with uninitialized value in vdev_inuse Reviewed
* 186ab8d Merge pull request #28 from ahrens/merge
|\
| * 00c4ece Merge remote-tracking branch 'illumos/master' into openzfs-merge
| |\
|/ /
| * f34d737 5963 rmis should be removed from arcstat manpage Reviewed by: Matthe
| * 1df447e 6390 Free props in ztest_init() Reviewed by: Brian Behlendorf <behle
| * eaef6a9 6385 Fix unlocking order in zfs_zget Reviewed by: Brian Behlendorf <
| * 07b64d1 6376 segmentation fault when sharing with 'sec=none,root=*' options
| * 4eab410 6369 remove SVM tests from ZFS test suite Reviewed by: John Kennedy
| * cd8037e 6344 mdb choked on core file from newer platform Reviewed by: Robert
| * 4224188 6338 cannot use umem tools on mdb itself Reviewed by: Dave Pacheco <
| * b3700b0 6352 Updated DC locator for SMB and idmap Portions contributed by: M
| * ed81dd5 6351 Update smbsrv dtrace scripts and install them Reviewed by: Gord
| * 12b6558 1122 smbsrv should use SPNEGO (inbound authentication) Portions cont
| * 056d3a7 6343 zoneadmd parent needs to close open fds Reviewed by: Robert Mus
| * 78b013d 6394 ::mac_flow -s segfaults Reviewed by: Jason King <jason.brian.ki
| * 29e5475 6347 microfind is somewhat less than ideal Reviewed by: Patrick Moon
| * 075fab9 6353 POLLHUP not generated for disconnected Unix domain sockets Revi
| * 30a92f3 6373 README.mapfiles refers to wrong symbol version Reviewed by: Rya
* | b5e85c0 6390 Free props in ztest_init() Reviewed by: Brian Behlendorf <behle
* | d578e0e 6385 Fix unlocking order in zfs_zget() Reviewed by: Brian Behlendorf
* | 2bc93fe 6369 remove SVM tests from ZFS test suite Reviewed by: John Kennedy
* | 5e0929b Merge illumos
|\ \
| |/
| * a443cc8 6292 exporting a pool while an async destroy is running can leave en
| * 1a49874 6372 remove ON_CRYPTO_BINS references from the env file and Install
| * 38e64a3 6339 env file could figure out CODEMGR_WS automatically Reviewed by:
| * 4cb69ec 6335 need to use sed and awk from Makefile.master in head/Makefile R
| * b39b744 6319 assertion failed in zio_ddt_write: bp->blk_birth == txg Reviewe
```
It was first landed in OpenZFS here:
```
* | 2bc93fe 6369 remove SVM tests from ZFS test suite Reviewed by: John Kennedy
```
And then later in illumos here:
```
| * 4eab410 6369 remove SVM tests from ZFS test suite Reviewed by: John Kennedy
```
OpenZFS then picked up the version in illumos via the merge commit, here:
```
* 186ab8d Merge pull request #28 from ahrens/merge
|\
| * 00c4ece Merge remote-tracking branch 'illumos/master' into openzfs-merge
```
there's two merge commits because Matt likely merged illumos to a local branch
in his local repository, then opened a pull request with that merge commit, and
then merged the merge commit into OpenZFS.
The data is all there, if we need to write some tools to get at this
information a bit easier, then we can try to make some time to do that.
Ultimately, the question of "who's version is authoritative" is up to the
maintainers of OpenZFS. If changes are made during the RTI process and those
changes are also deemed appropriate for OpenZFS, then it's likely that the
changes will get pulled in via the merge with illumos. With that said, it's
possible that the illumos RTI process requires changes to the patch that are
not appropriate to land in OpenZFS, in which case the merge commit will not
pull in these additional changes and will use the version of the patch that had
already landed in OpenZFS (via the original pull request).
Matt's comment:
> I would add that I don't want to make it a hard requirement for the OpenZFS
> repo to be tightly in sync with illumos. The OpenZFS repo needs to serve the
> needs of the broader OpenZFS community, beyond just illumos. While I intend
> to keep OpenZFS tightly in sync with illumos, I don't want to make technical
> decisions that would preclude OpenZFS-only commits if necessary.
Tells me that, while we want to stay as much in sync with illumos as possible,
that's not a strict requirement. It's possible that we'll maintain differences
that will never be brought into illumos, and that's OK (maybe differences
introduced by the RTI process, but likely not). Just like there may be changes
made in downstream versions of OpenZFS that will never be brought into OpenZFS.
> I think the advantage for the downstream repos is going to be the PR process
> and the discussions. I don’t expect downstream maintainers to source this
> repo as a submodule or as a sub-tree, because it would pull in all of
> illumos. In that case, neither method really affects them, because they will
> just be reaching in and taking out the files they want (HEAD SHA will not
> really matter to them). Once we reach the goal of independant files not
> tethered to any implementation, this whole discussion becomes moot, because
> illumos will no longer be merged in.
I disagree with this. As an example, Delphix may or may not start to merge in
OpenZFS into our internal DxOS tree. We haven't really discussed in detail if
we actually want to start to use OpenZFS as our "upstream" (maybe in addition
to illumos), but with a merge strategy that is a possibility; with a rebase
strategy that is practially impossible because we can't simply "fetch and
merge". In that case, DxOS would be a downstream consumer of OpenZFS, pulling
code directly from the OpenZFS tree; we would not be "reaching in and taking
out the files" that we needed.
---
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/28#issuecomment-153855251
_______________________________________________
developer mailing list
[email protected]
http://lists.open-zfs.org/mailman/listinfo/developer
