d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

6529 Properly handle updates of variably-sized SA entries.

Browse Source 
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>
Author: Andriy Gapon <avg@icyb.net.ua>

illumos/illumos-gate@e7e978b1f7

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.
This commit is contained in:
Alexander Motin 2016-01-26 13:49:46 +00:00
parent b1f532a6b9
commit d954035a30
1 changed files with 38 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
uts/common/fs/zfs/sa.c
View File

@ -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. * of attributes.
* Same-length attribute value replacement (including fixed-length attributes)
* is handled more efficiently by the upper layers.
*/ */
static int static int
sa_modify_attrs(sa_handle_t *hdl, sa_attr_type_t newattr, 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_data_size = 0;
int spill_attr_count = 0; int spill_attr_count = 0;
int error; int error;
uint16_t length; uint16_t length, reg_length;
int i, j, k, length_idx; int i, j, k, length_idx;
sa_hdr_phys_t *hdr; sa_hdr_phys_t *hdr;
sa_idx_tab_t *idx_tab; 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); hdr = SA_GET_HDR(hdl, SA_BONUS);
idx_tab = SA_IDX_TAB_GET(hdl, SA_BONUS); idx_tab = SA_IDX_TAB_GET(hdl, SA_BONUS);
for (; k != 2; k++) { 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++) { for (i = 0, length_idx = 0; i != count; i++) {
sa_attr_type_t attr; sa_attr_type_t attr;
attr = idx_tab->sa_layout->lot_attrs[i]; 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 (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; 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, SA_ADD_BULK_ATTR(attr_desc, j, attr,
locator, datastart, buflen); locator, datastart, buflen);
} else { } else {
length = SA_REGISTERED_LEN(sa, attr);
if (length == 0) {
length = hdr->sa_lengths[length_idx++];
}
SA_ADD_BULK_ATTR(attr_desc, j, attr, SA_ADD_BULK_ATTR(attr_desc, j, attr,
NULL, (void *) NULL, (void *)
(TOC_OFF(idx_tab->sa_idx_tab[attr]) + (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) { if (action == SA_ADD) {
length = SA_REGISTERED_LEN(sa, newattr); reg_length = SA_REGISTERED_LEN(sa, newattr);
if (length == 0) { IMPLY(reg_length != 0, reg_length == buflen);
length = buflen;
}
SA_ADD_BULK_ATTR(attr_desc, j, newattr, locator, SA_ADD_BULK_ATTR(attr_desc, j, newattr, locator,
datastart, buflen); datastart, buflen);
} }
ASSERT3U(j, ==, attr_count);
error = sa_build_layouts(hdl, attr_desc, attr_count, tx); error = sa_build_layouts(hdl, attr_desc, attr_count, tx);