svn commit: r295125 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs

[Andriy Gapon]

Author: avg
Date: Mon Feb  1 15:40:40 2016
New Revision: 295125
URL: https://svnweb.freebsd.org/changeset/base/295125

Log:
  MFV r294821: 6529 Properly handle updates of variably-sized SA entries.
  
  Reviewed by: Brian Behlendorf <behlendorf1 at llnl.gov>
  Reviewed by: Matthew Ahrens <mahrens at delphix.com>
  Reviewed by: Ned Bass <bass6 at llnl.gov>
  Reviewed by: Tim Chase <tim at chase2k.com>
  Approved by: Gordon Ross <gwr at nexenta.com>
  Author: Andriy Gapon <avg at icyb.net.ua>
  
  illumos/illumos-gate at e7e978b1f75353cb29673af9b35453c20c2827bf
  
  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.

Modified:
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sa.c
Directory Properties:
  head/sys/cddl/contrib/opensolaris/   (props changed)

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sa.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sa.c	Mon Feb  1 14:56:11 2016	(r295124)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sa.c	Mon Feb  1 15:40:40 2016	(r295125)
@@ -1652,7 +1652,7 @@ sa_modify_attrs(sa_handle_t *hdl, sa_att
 	int spill_data_size = 0;
 	int spill_attr_count = 0;
 	int error;
-	uint16_t length;
+	uint16_t length, reg_length;
 	int i, j, k, length_idx;
 	sa_hdr_phys_t *hdr;
 	sa_idx_tab_t *idx_tab;
@@ -1712,34 +1712,50 @@ sa_modify_attrs(sa_handle_t *hdl, sa_att
 	hdr = SA_GET_HDR(hdl, SA_BONUS);
 	idx_tab = SA_IDX_TAB_GET(hdl, SA_BONUS);
 	for (; k != 2; k++) {
-		/* iterate over each attribute in layout */
+		/*
+		 * Iterate over each attribute in layout.  Fetch the
+		 * size of variable-length attributes needing rewrite
+		 * from sa_lengths[].
+		 */
 		for (i = 0, length_idx = 0; i != count; i++) {
 			sa_attr_type_t attr;
 
 			attr = idx_tab->sa_layout->lot_attrs[i];
+			reg_length = SA_REGISTERED_LEN(sa, attr);
+			if (reg_length == 0) {
+				length = hdr->sa_lengths[length_idx];
+				length_idx++;
+			} else {
+				length = reg_length;
+			}
 			if (attr == newattr) {
-				/* duplicate attributes are not allowed */
-				ASSERT(action == SA_REPLACE ||
-				    action == SA_REMOVE);
-				/* must be variable-sized to be replaced here */
-				if (action == SA_REPLACE) {
-					ASSERT(SA_REGISTERED_LEN(sa, attr) == 0);
-					SA_ADD_BULK_ATTR(attr_desc, j, attr,
-					    locator, datastart, buflen);
-				}
+				/*
+				 * There is nothing to do for SA_REMOVE,
+				 * so it is just skipped.
+				 */
+				if (action == SA_REMOVE)
+					continue;
+
+				/*
+				 * Duplicate attributes are not allowed, so the
+				 * action can not be SA_ADD here.
+				 */
+				ASSERT3S(action, ==, SA_REPLACE);
+
+				/*
+				 * Only a variable-sized attribute can be
+				 * replaced here, and its size must be changing.
+				 */
+				ASSERT3U(reg_length, ==, 0);
+				ASSERT3U(length, !=, buflen);
+				SA_ADD_BULK_ATTR(attr_desc, j, attr,
+				    locator, datastart, buflen);
 			} else {
-				length = SA_REGISTERED_LEN(sa, attr);
-				if (length == 0) {
-					length = hdr->sa_lengths[length_idx];
-				}
-
 				SA_ADD_BULK_ATTR(attr_desc, j, attr,
 				    NULL, (void *)
 				    (TOC_OFF(idx_tab->sa_idx_tab[attr]) +
 				    (uintptr_t)old_data[k]), length);
 			}
-			if (SA_REGISTERED_LEN(sa, attr) == 0)
-				length_idx++;
 		}
 		if (k == 0 && hdl->sa_spill) {
 			hdr = SA_GET_HDR(hdl, SA_SPILL);
@@ -1750,10 +1766,8 @@ sa_modify_attrs(sa_handle_t *hdl, sa_att
 		}
 	}
 	if (action == SA_ADD) {
-		length = SA_REGISTERED_LEN(sa, newattr);
-		if (length == 0) {
-			length = buflen;
-		}
+		reg_length = SA_REGISTERED_LEN(sa, newattr);
+		IMPLY(reg_length != 0, reg_length == buflen);
 		SA_ADD_BULK_ATTR(attr_desc, j, newattr, locator,
 		    datastart, buflen);
 	}