Author: mav
Date: Tue Jan 26 13:49:46 2016
New Revision: 294821
URL: https://svnweb.freebsd.org/changeset/base/294821

Log:
  6529 Properly handle updates of variably-sized SA entries.
  
  Reviewed by: Brian Behlendorf <[email protected]>
  Reviewed by: Matthew Ahrens <[email protected]>
  Reviewed by: Ned Bass <[email protected]>
  Reviewed by: Tim Chase <[email protected]>
  Approved by: Gordon Ross <[email protected]>
  Author: Andriy Gapon <[email protected]>
  
  illumos/illumos-gate@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:
  vendor-sys/illumos/dist/uts/common/fs/zfs/sa.c

Modified: vendor-sys/illumos/dist/uts/common/fs/zfs/sa.c
==============================================================================
--- vendor-sys/illumos/dist/uts/common/fs/zfs/sa.c      Tue Jan 26 13:45:41 
2016        (r294820)
+++ vendor-sys/illumos/dist/uts/common/fs/zfs/sa.c      Tue Jan 26 13:49:46 
2016        (r294821)
@@ -1628,8 +1628,11 @@ sa_replace_all_by_template(sa_handle_t *
 }
 
 /*
- * add/remove/replace a single attribute and then rewrite the entire set
+ * Add/remove a single attribute or replace a variable-sized attribute value
+ * with a value of a different size, and then rewrite the entire set
  * of attributes.
+ * Same-length attribute value replacement (including fixed-length attributes)
+ * is handled more efficiently by the upper layers.
  */
 static int
 sa_modify_attrs(sa_handle_t *hdl, sa_attr_type_t newattr,
@@ -1646,7 +1649,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;
@@ -1706,26 +1709,45 @@ 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) {
-                               if (action == SA_REMOVE) {
-                                       j++;
+                               /*
+                                * There is nothing to do for SA_REMOVE,
+                                * so it is just skipped.
+                                */
+                               if (action == SA_REMOVE)
                                        continue;
-                               }
-                               ASSERT(SA_REGISTERED_LEN(sa, attr) == 0);
-                               ASSERT(action == SA_REPLACE);
+
+                               /*
+                                * 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]) +
@@ -1741,13 +1763,12 @@ 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);
        }
+       ASSERT3U(j, ==, attr_count);
 
        error = sa_build_layouts(hdl, attr_desc, attr_count, tx);
 
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"

Reply via email to