From d954035a30843e636468722eebacaae030bd8da3 Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Tue, 26 Jan 2016 13:49:46 +0000 Subject: [PATCH 1/2] 6529 Properly handle updates of variably-sized SA entries. Reviewed by: Brian Behlendorf Reviewed by: Matthew Ahrens Reviewed by: Ned Bass Reviewed by: Tim Chase Approved by: Gordon Ross Author: Andriy Gapon 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. --- uts/common/fs/zfs/sa.c | 55 +++++++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 17 deletions(-) diff --git a/uts/common/fs/zfs/sa.c b/uts/common/fs/zfs/sa.c index c8534214c5bd..ff27c7218aa6 100644 --- a/uts/common/fs/zfs/sa.c +++ b/uts/common/fs/zfs/sa.c @@ -1628,8 +1628,11 @@ sa_replace_all_by_template(sa_handle_t *hdl, sa_bulk_attr_t *attr_desc, } /* - * 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_attr_type_t newattr, 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_attr_type_t newattr, 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_attr_type_t newattr, } } 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); From 0fbf79a6b36fe6fb037fe53a038b547729a6ebda Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Fri, 29 Jan 2016 17:18:50 +0000 Subject: [PATCH 2/2] 6358 A faulted pool with only unavailable vdevs triggers assertion failure in libzfs Reviewed by: Matthew Ahrens Reviewed by: Andrew Stormont Reviewed by: Serban Maduta Approved by: Dan McDonald Author: Dan Vatca illumos/illumos-gate@b289d045e084af53efcc025255af8242e41f28fa --- lib/libzfs/common/libzfs_config.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/libzfs/common/libzfs_config.c b/lib/libzfs/common/libzfs_config.c index d5ba20fde0cf..c3dafd6a777c 100644 --- a/lib/libzfs/common/libzfs_config.c +++ b/lib/libzfs/common/libzfs_config.c @@ -26,6 +26,7 @@ /* * Copyright (c) 2012 by Delphix. All rights reserved. + * Copyright (c) 2015 by Syneto S.R.L. All rights reserved. */ /* @@ -246,8 +247,9 @@ zpool_get_features(zpool_handle_t *zhp) config = zpool_get_config(zhp, NULL); } - verify(nvlist_lookup_nvlist(config, ZPOOL_CONFIG_FEATURE_STATS, - &features) == 0); + if (nvlist_lookup_nvlist(config, ZPOOL_CONFIG_FEATURE_STATS, + &features) != 0) + return (NULL); return (features); }