OpenZFS 6529 - Properly handle updates of variably-sized SA entries.

[gmelikov]

Authored by: Andriy Gapon avg@icyb.net.ua
Reviewed by: Brian Behlendorf behlendorf1@llnl.gov
Reviewed by: Matthew Ahrens mahrens@delphix.com
Reviewed by: Ned Bass bass6@llnl.gov
Reviewed by: Tim Chase tim@chase2k.com
Approved by: Gordon Ross gwr@nexenta.com
Ported-by: George Melikov mail@gmelikov.ru

OpenZFS-issue: https://www.illumos.org/issues/6529
OpenZFS-commit: openzfs/openzfs@e7e978b

This issue was first fixed in ZoL
(5d862cb)
and then a modified version of the patch was landed in the upstream codebase.
Check out the discussion in openzfs/openzfs#24 .

This commit aligns ZoL with OpenZFS codebase.

During the update process in sa_modify_attrs(), the sizes of existing
variably-sized SA entries are obtained from sa_lengths[]. The case where
a variably-sized SA was being replaced neglected to increment the index
into sa_lengths[], so subsequent variable-length SAs would be rewritten
with the wrong length. This patch adds the missing increment operation
so all variably-sized SA entries are stored with their correct lengths.

Another problem was that index into attr_desc[] was increased even when
an attribute was removed. If that attribute was not the last attribute,
then the last attribute was lost.

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)

Checklist:

• My code follows the ZFS on Linux code style requirements.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.
• Change has been approved by a ZFS on Linux member.

[mention-bot]

@gmelikov, thanks for your PR! By analyzing the history of the files in this pull request, we identified @behlendorf, @dweeezil and @nedbass to be potential reviewers.

[dweeezil]

@gmelikov My memory is a bit fuzzy on this one, however, IIRC, this is effectively a slightly different implementation of 5d862cb which was merged to ZoL sometime earlier. There's certainly no problem adopting this version of the patch for the sake of code alignment, however, the comments should indicate a bit more about the history; specifically that the issue was first fixed in ZoL and then a modified version of the patch was landed in the upstream codebase. Check out the discussion in openzfs/openzfs/issues/24.

[gmelikov]

@dweeezil thank you for information, i'll add comments about this.

[behlendorf]

LGTM. The updated comment clarifying the history of this looks good.

[behlendorf]

@dweeezil the update PR LGTM and just brings us inline with the upstream version of the fix. But can you look it over and approve the patch to make sure I didn't overlook something.

[dweeezil]

This LGTM. It aligns properly with the upstream code and accomplishes the same as the existing fix.

[dweeezil]

The commit comment is malformed. It needs a blank line.

[behlendorf]

@dweeezil thanks for the double check. I can fix the comment when merging this.

[gmelikov]

@dweeezil @behlendorf thanks for help!