-
Notifications
You must be signed in to change notification settings - Fork 67
8589 zfs send | recv crashes receiving system #685
Conversation
| uint64_t maxobj = DNODES_PER_BLOCK * | ||
| (DMU_META_DNODE(dsp->dsa_os)->dn_maxblkid + 1); | ||
|
|
||
| /* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this comment be updated for openzfs, describing which openzfs "versions" are affected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, but how do I find out the version number? Do you use the git commit hash as version number? Or is there somewhere I can find that information?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A date might be the only useful way to express this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we should update the comment. The simplest change would be to say Older versions of ZFS did not handle .... If we want to be more specific, we could say Prior to the fix for bug 1234, ZFS did not handle...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once the comment is updated to reflect ZFS as a whole, rather than ZoL specifically, this looks good to me.
usr/src/uts/common/fs/zfs/dmu_send.c
Outdated
|
|
||
| /* | ||
| * Older versions of OpenZFS (since 7104) does not handle large | ||
| * FREEOBJECTS records correctly, leading to zfs recv never completing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update, @waikontse.
It looks like the indentation here is inconsistent. Each line should start with tab space asterisk.
I think that in this context "since" means "more recent than", but I think you mean "less recent than" / "older than"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ahrens, thanks for the guidance on the commenting.
I need to get used to tab spaces, I usually use spaces.
You're right, what I mean is that versions "less recent"/"older than". I'll update the formatting with the comment later today.
Reviewed by: Paul Dagnelie <pcd@delphix.com> Reviewed by: Matt Ahrens <matt@delphix.com>
|
In the RTI, Dan asks that code comments be updated to reflect the review feedback; see here |
|
This repo will be archived in the coming week, and therefore this PR is being closed. Alternate processes for contributing ZFS changes (including those in open PR's) include following the illumos procedures, or opening a PR against ZFSonLinux. The reasoning behind this are outlined in an email to the developer@open-zfs.org mailing list, which is reproduced below:
|
The code is the same as the one on openzfs/zfs#6616. I just included the missing part.