From 36b6510e57011dc3ede3adcca6010a16393515ef Mon Sep 17 00:00:00 2001 From: Vitaly Fertman Date: Tue, 20 Dec 2022 11:50:47 -0800 Subject: [PATCH] LU-14645 utils: setstripe cleanup lfs setstripe checks stripe parameters differently for PFL and !PFL layouts. Whereas the PFL layout is checked in comp_args_to_layout() individually and in llapi_layout_sanity_cb() in pairs, !PFL layout verification is done partially in several places. Create a common llapi_stripe_param_verify() for this purpose. Make the checks for both cases symmetric. skip some excessive checks: - do not check the file is on lustre fs, the following ioctl does it; - do not check the stripe-index is valid, done on MDS side; - do not check the pool exists for a !PFL file (align with a setstripe for PFL files); Lustre-change: https://review.whamcloud.com/43465 Lustre-commit: 149934fe28dac22a51ec9b2873c4f215cb204947 Lustre-change: https://review.whamcloud.com/46151 Lustre-commit: 5e65d6a8e57a5a17c4c7e043cb46e86bf82b7782 Lustre-change: https://review.whamcloud.com/46152 Lustre-commit: cd1f8527d414a12ec7eb5b69fe30509a45b33ad4 Signed-off-by: Vitaly Fertman Change-Id: I456b1b2e876229ac1a354d4e3879624325856574 HPE-bug-id: LUS-9886 Reviewed-on: https://es-gerrit.dev.cray.com/158589 Reviewed-by: Andriy Skulysh Reviewed-by: Alexander Boyko Tested-by: Alexander Lezhoev Reviewed-by: Alexander Boyko Reviewed-by: Andriy Skulysh Reviewed-on: https://review.whamcloud.com/c/ex/lustre-release/+/49459 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Andreas Dilger Reviewed-by: Jian Yu --- lustre/include/lustre/lustreapi.h | 6 +- lustre/tests/ost-pools.sh | 8 -- lustre/utils/lfs.c | 80 +++++++++++++------- lustre/utils/liblustreapi.c | 150 +++++++++++++------------------------ lustre/utils/liblustreapi_layout.c | 28 +++---- lustre/utils/lustreapi_internal.h | 18 +++++ 6 files changed, 141 insertions(+), 149 deletions(-) diff --git a/lustre/include/lustre/lustreapi.h b/lustre/include/lustre/lustreapi.h index b23d657..05c72e0 100644 --- a/lustre/include/lustre/lustreapi.h +++ b/lustre/include/lustre/lustreapi.h @@ -857,6 +857,10 @@ int llapi_layout_stripe_count_get(const struct llapi_layout *layout, */ int llapi_layout_stripe_count_set(struct llapi_layout *layout, uint64_t count); +/** + * Check if the stripe count \a stripe_count \a is valid. + */ +bool llapi_layout_stripe_count_is_valid(int64_t stripe_count); /******************** Stripe Size ********************/ /** @@ -988,7 +992,7 @@ int llapi_layout_pool_name_get(const struct llapi_layout *layout, * \retval -1 Invalid argument, errno set to EINVAL. */ int llapi_layout_pool_name_set(struct llapi_layout *layout, - const char *pool_name); + const char *pool_name); /******************** File Creation ********************/ diff --git a/lustre/tests/ost-pools.sh b/lustre/tests/ost-pools.sh index 723e637..a2c9880 100755 --- a/lustre/tests/ost-pools.sh +++ b/lustre/tests/ost-pools.sh @@ -588,10 +588,6 @@ test_6() { $LFS setstripe -c 2 -p $INVALID_POOL $POOL_DIR 2>/dev/null [[ $? -ne 0 ]] || error "setstripe to invalid pool did not fail." - # If the pool name does not exist, the command should fail - $LFS setstripe -c 2 -p $NON_EXISTANT_POOL $POOL_DIR 2>/dev/null - [[ $? -ne 0 ]] || error "setstripe to non-existant pool did not fail." - # lfs setstripe should work as before if a pool name is not specified. $LFS setstripe -c -1 $POOL_DIR [[ $? -eq 0 ]] || error "$LFS setstripe -c -1 $POOL_DIR failed." @@ -688,10 +684,6 @@ test_7c() $LFS setstripe -c 1 $DIR/$tdir/testfile1 --pool "${pool}X" && error "setstripe succedeed" - # setstripe with the same pool name minus 1 letter - $LFS setstripe -c 1 $DIR/$tdir/testfile1 --pool "${pool%?}" && - error "setstripe succedeed" - rm -f $DIR/$tdir/testfile1 destroy_pool_int $FSNAME.$pool diff --git a/lustre/utils/lfs.c b/lustre/utils/lfs.c index 53bb376..a9d2d63 100644 --- a/lustre/utils/lfs.c +++ b/lustre/utils/lfs.c @@ -1630,7 +1630,8 @@ enum mirror_flags { * Return: 0 on success or a negative error code on failure. */ static int mirror_create_sanity_check(const char *fname, - struct mirror_args *list) + struct mirror_args *list, + bool check_fname) { int rc = 0; bool has_m_file = false; @@ -1639,7 +1640,7 @@ static int mirror_create_sanity_check(const char *fname, if (!list) return -EINVAL; - if (fname) { + if (fname && check_fname) { struct llapi_layout *layout; layout = llapi_layout_get_by_path(fname, 0); @@ -1738,7 +1739,7 @@ static int mirror_create(char *fname, struct mirror_args *mirror_list) int i = 0; int rc = 0; - rc = mirror_create_sanity_check(NULL, mirror_list); + rc = mirror_create_sanity_check(fname, mirror_list, false); if (rc) return rc; @@ -2041,7 +2042,7 @@ static int mirror_extend(char *fname, struct mirror_args *mirror_list, { int rc; - rc = mirror_create_sanity_check(fname, mirror_list); + rc = mirror_create_sanity_check(fname, mirror_list, true); if (rc) return rc; @@ -2724,6 +2725,33 @@ static inline bool setstripe_args_specified(struct lfs_setstripe_args *lsa) lsa->lsa_comp_end != 0); } +static int lsa_args_stripe_count_check(struct lfs_setstripe_args *lsa) +{ + if (lsa->lsa_nr_tgts) { + if (lsa->lsa_nr_tgts < 0 || + lsa->lsa_nr_tgts >= LOV_MAX_STRIPE_COUNT) { + fprintf(stderr, "Invalid nr_tgts(%d)\n", + lsa->lsa_nr_tgts); + errno = EINVAL; + return -1; + } + + if (lsa->lsa_stripe_count > 0 && + lsa->lsa_stripe_count != LLAPI_LAYOUT_DEFAULT && + lsa->lsa_stripe_count != LLAPI_LAYOUT_WIDE && + lsa->lsa_nr_tgts != lsa->lsa_stripe_count) { + fprintf(stderr, "stripe_count(%lld) != nr_tgts(%d)\n", + lsa->lsa_stripe_count, + lsa->lsa_nr_tgts); + errno = EINVAL; + return -1; + } + } + + return 0; + +} + /** * comp_args_to_layout() - Create or extend a composite layout. * @composite: Pointer to the composite layout. @@ -2732,6 +2760,8 @@ static inline bool setstripe_args_specified(struct lfs_setstripe_args *lsa) * This function creates or extends a composite layout by adding a new * component with stripe options from @lsa. * + * When modified, adjust llapi_stripe_param_verify() if needed as well. + * * Return: 0 on success or an error code on failure. */ static int comp_args_to_layout(struct llapi_layout **composite, @@ -2907,22 +2937,28 @@ new_comp: } } + rc = lsa_args_stripe_count_check(lsa); + if (rc) + return rc; + if (lsa->lsa_nr_tgts > 0) { - if (lsa->lsa_stripe_count > 0 && - lsa->lsa_stripe_count != LLAPI_LAYOUT_DEFAULT && - lsa->lsa_stripe_count != LLAPI_LAYOUT_WIDE && - lsa->lsa_nr_tgts != lsa->lsa_stripe_count) { - fprintf(stderr, "stripe_count(%lld) != nr_tgts(%d)\n", - lsa->lsa_stripe_count, - lsa->lsa_nr_tgts); - errno = EINVAL; - return -1; - } + bool found = false; + for (i = 0; i < lsa->lsa_nr_tgts; i++) { rc = llapi_layout_ost_index_set(layout, i, lsa->lsa_tgts[i]); if (rc) break; + + /* Make sure stripe offset is in OST list. */ + if (lsa->lsa_tgts[i] == lsa->lsa_stripe_off) + found = true; + } + if (!found) { + fprintf(stderr, "Invalid stripe offset '%lld', not in the target list", + lsa->lsa_stripe_off); + errno = EINVAL; + return -1; } } else if (lsa->lsa_stripe_off != LLAPI_LAYOUT_DEFAULT && lsa->lsa_stripe_off != -1) { @@ -4306,6 +4342,9 @@ static int lfs_setstripe_internal(int argc, char **argv, migrate_mdt_param.fp_lmv_md = lmu; migrate_mdt_param.fp_migrate = 1; } else if (!layout) { + if (lsa_args_stripe_count_check(&lsa)) + goto usage_error; + /* initialize stripe parameters */ param = calloc(1, offsetof(typeof(*param), lsp_osts[lsa.lsa_nr_tgts])); @@ -4339,19 +4378,8 @@ static int lfs_setstripe_internal(int argc, char **argv, } param->lsp_pool = lsa.lsa_pool_name; param->lsp_is_specific = false; - if (lsa.lsa_nr_tgts > 0) { - if (lsa.lsa_stripe_count > 0 && - lsa.lsa_stripe_count != LLAPI_LAYOUT_DEFAULT && - lsa.lsa_stripe_count != LLAPI_LAYOUT_WIDE && - lsa.lsa_nr_tgts != lsa.lsa_stripe_count) { - fprintf(stderr, - "error: %s: stripe count %lld doesn't match the number of OSTs: %d\n", - argv[0], lsa.lsa_stripe_count, - lsa.lsa_nr_tgts); - free(param); - goto usage_error; - } + if (lsa.lsa_nr_tgts > 0) { param->lsp_is_specific = true; param->lsp_stripe_count = lsa.lsa_nr_tgts; memcpy(param->lsp_osts, tgts, diff --git a/lustre/utils/liblustreapi.c b/lustre/utils/liblustreapi.c index d44f176..cfe0d20 100644 --- a/lustre/utils/liblustreapi.c +++ b/lustre/utils/liblustreapi.c @@ -406,11 +406,22 @@ int llapi_ioctl_unpack(struct obd_ioctl_data *data, char *pbuf, int max_len) return 0; } -/* XXX: llapi_xxx() functions return negative values upon failure */ - -int llapi_stripe_limit_check(unsigned long long stripe_size, int stripe_offset, - int stripe_count, int stripe_pattern) +/** + * Verify the setstripe parameters before using. + * This is a pair method for comp_args_to_layout()/llapi_layout_sanity_cb() + * when just 1 component or a non-PFL layout is given. + * + * \param[in] param stripe parameters + * \param[in] pool_name pool name + * \param[in] fsname lustre FS name + * + * \retval 0, success + * < 0, error code on failre + */ +static int llapi_stripe_param_verify(const struct llapi_stripe_param *param, + const char **pool_name) { + int count; static int page_size; int rc = 0; @@ -428,31 +439,51 @@ int llapi_stripe_limit_check(unsigned long long stripe_size, int stripe_offset, page_size, LOV_MIN_STRIPE_SIZE); } } - if (!llapi_stripe_size_is_aligned(stripe_size)) { + if (!llapi_stripe_size_is_aligned(param->lsp_stripe_size)) { rc = -EINVAL; llapi_error(LLAPI_MSG_ERROR, rc, "error: bad stripe_size %llu, must be an even multiple of %d bytes", - (unsigned long long)stripe_size, page_size); + param->lsp_stripe_size, page_size); goto out; } - if (!llapi_stripe_index_is_valid(stripe_offset)) { + if (!llapi_stripe_index_is_valid(param->lsp_stripe_offset)) { rc = -EINVAL; llapi_error(LLAPI_MSG_ERROR, rc, "error: bad stripe offset %d", - stripe_offset); + param->lsp_stripe_offset); goto out; } - if (!llapi_stripe_count_is_valid(stripe_count)) { + if (llapi_stripe_size_is_too_big(param->lsp_stripe_size)) { rc = -EINVAL; - llapi_error(LLAPI_MSG_ERROR, rc, "error: bad stripe count %d", - stripe_count); + llapi_error(LLAPI_MSG_ERROR, rc, + "error: stripe size '%llu' over 4GB limit", + param->lsp_stripe_size); goto out; } - if (llapi_stripe_size_is_too_big(stripe_size)) { + + count = param->lsp_stripe_count; + if (param->lsp_stripe_pattern == LOV_PATTERN_MDT) { rc = -EINVAL; llapi_error(LLAPI_MSG_ERROR, rc, - "error: stripe size '%llu' over 4GB limit", - (unsigned long long)stripe_size); + "Invalid pattern: '-L mdt', must be specified " + "with -E\n"); goto out; + } else { + if (!llapi_stripe_count_is_valid(count)) { + rc = -EINVAL; + llapi_error(LLAPI_MSG_ERROR, rc, + "Invalid stripe count %d\n", count); + goto out; + } + } + + /* Make sure we have a good pool */ + if (*pool_name != NULL) { + if (!llapi_pool_name_is_valid(pool_name)) { + rc = -EINVAL; + llapi_error(LLAPI_MSG_ERROR, rc, + "Invalid Poolname '%s'", *pool_name); + goto out; + } } out: @@ -577,98 +608,23 @@ int llapi_get_agent_uuid(char *path, char *buf, size_t bufsize) int llapi_file_open_param(const char *name, int flags, mode_t mode, const struct llapi_stripe_param *param) { - char fsname[MAX_OBD_NAME + 1] = { 0 }; - char *pool_name = param->lsp_pool; struct lov_user_md *lum = NULL; - size_t lum_size = sizeof(*lum); + const char *pool_name = param->lsp_pool; + size_t lum_size; int fd, rc; - /* Make sure we are on a Lustre file system */ - rc = llapi_search_fsname(name, fsname); - if (rc) { - llapi_error(LLAPI_MSG_ERROR, rc, - "'%s' is not on a Lustre filesystem", - name); - return rc; - } - /* Check if the stripe pattern is sane. */ - rc = llapi_stripe_limit_check(param->lsp_stripe_size, - param->lsp_stripe_offset, - param->lsp_stripe_count, - param->lsp_stripe_pattern); + rc = llapi_stripe_param_verify(param, &pool_name); if (rc != 0) return rc; - /* Make sure we have a good pool */ - if (pool_name != NULL) { - /* - * in case user gives the full pool name ., - * strip the fsname - */ - char *ptr = strchr(pool_name, '.'); - - if (ptr != NULL) { - *ptr = '\0'; - if (strcmp(pool_name, fsname) != 0) { - *ptr = '.'; - llapi_err_noerrno(LLAPI_MSG_ERROR, - "Pool '%s' is not on filesystem '%s'", - pool_name, fsname); - return -EINVAL; - } - pool_name = ptr + 1; - } - - /* Make sure the pool exists and is non-empty */ - rc = llapi_search_ost(fsname, pool_name, NULL); - if (rc < 1) { - char *err = rc == 0 ? "has no OSTs" : "does not exist"; - - llapi_err_noerrno(LLAPI_MSG_ERROR, "pool '%s.%s' %s", - fsname, pool_name, err); - return -EINVAL; - } - - lum_size = sizeof(struct lov_user_md_v3); - } - - /* sanity check of target list */ - if (param->lsp_is_specific) { - char ostname[MAX_OBD_NAME + 64]; - bool found = false; - int i; - - for (i = 0; i < param->lsp_stripe_count; i++) { - snprintf(ostname, sizeof(ostname), "%s-OST%04x_UUID", - fsname, param->lsp_osts[i]); - rc = llapi_search_ost(fsname, pool_name, ostname); - if (rc <= 0) { - if (rc == 0) - rc = -ENODEV; - - llapi_error(LLAPI_MSG_ERROR, rc, - "%s: cannot find OST %s in %s", - __func__, ostname, - pool_name != NULL ? - "pool" : "system"); - return rc; - } - - /* Make sure stripe offset is in OST list. */ - if (param->lsp_osts[i] == param->lsp_stripe_offset) - found = true; - } - if (!found) { - llapi_error(LLAPI_MSG_ERROR, -EINVAL, - "%s: stripe offset '%d' is not in the target list", - __func__, param->lsp_stripe_offset); - return -EINVAL; - } - + if (param->lsp_is_specific) lum_size = lov_user_md_size(param->lsp_stripe_count, LOV_USER_MAGIC_SPECIFIC); - } + else if (pool_name) + lum_size = sizeof(struct lov_user_md_v3); + else + lum_size = sizeof(*lum); lum = calloc(1, lum_size); if (lum == NULL) diff --git a/lustre/utils/liblustreapi_layout.c b/lustre/utils/liblustreapi_layout.c index 476dbfc..d760cf4 100644 --- a/lustre/utils/liblustreapi_layout.c +++ b/lustre/utils/liblustreapi_layout.c @@ -1288,7 +1288,7 @@ int llapi_layout_stripe_count_get(const struct llapi_layout *layout, * the old API uses 0 and -1. */ -static bool llapi_layout_stripe_count_is_valid(int64_t stripe_count) +bool llapi_layout_stripe_count_is_valid(int64_t stripe_count) { return stripe_count == LLAPI_LAYOUT_DEFAULT || stripe_count == LLAPI_LAYOUT_WIDE || @@ -1687,34 +1687,22 @@ int llapi_layout_pool_name_set(struct llapi_layout *layout, const char *pool_name) { struct llapi_layout_comp *comp; - char *ptr; comp = __llapi_layout_cur_comp(layout); if (comp == NULL) return -1; - if (pool_name == NULL) { - errno = EINVAL; - return -1; - } - if (comp->llc_pattern == LLAPI_LAYOUT_FOREIGN) { errno = EINVAL; return -1; } - /* Strip off any 'fsname.' portion. */ - ptr = strchr(pool_name, '.'); - if (ptr != NULL) - pool_name = ptr + 1; - - if (strlen(pool_name) > LOV_MAXPOOLNAME) { + if (!llapi_pool_name_is_valid(&pool_name)) { errno = EINVAL; return -1; } strncpy(comp->llc_pool_name, pool_name, sizeof(comp->llc_pool_name)); - return 0; } @@ -1750,7 +1738,7 @@ int llapi_layout_file_open(const char *path, int open_flags, mode_t mode, if (layout) { rc = llapi_layout_sanity((struct llapi_layout *)layout, false, - !!(layout->llot_mirror_count > 1)); + !!(layout->llot_mirror_count > 1)); if (rc) { llapi_layout_sanity_perror(rc); return -1; @@ -3405,6 +3393,9 @@ struct llapi_layout_sanity_args { #define LCME_USER_COMP_FLAGS (LCME_FL_PREF_RW | LCME_FL_NOSYNC | \ LCME_FL_EXTENSION) +/** + * When modified, adjust llapi_stripe_param_verify() if needed as well. + */ static int llapi_layout_sanity_cb(struct llapi_layout *layout, void *arg) { @@ -3612,6 +3603,7 @@ void llapi_layout_sanity_perror(int error) * components"? * * \param[in] layout component layout list. + * \param[in] fname file the layout to be checked for * \param[in] incomplete if layout is complete or not - some checks can * only be done on complete layouts. * \param[in] flr set when this is called from FLR mirror create @@ -3619,9 +3611,11 @@ void llapi_layout_sanity_perror(int error) * \retval 0, success, positive: various errors, see * llapi_layout_sanity_perror, -1, failure */ -int llapi_layout_sanity(struct llapi_layout *layout, bool incomplete, bool flr) +int llapi_layout_sanity(struct llapi_layout *layout, + bool incomplete, + bool flr) { - struct llapi_layout_sanity_args args; + struct llapi_layout_sanity_args args = { 0 }; struct llapi_layout_comp *curr; int rc = 0; diff --git a/lustre/utils/lustreapi_internal.h b/lustre/utils/lustreapi_internal.h index 543dc0e..be83575 100644 --- a/lustre/utils/lustreapi_internal.h +++ b/lustre/utils/lustreapi_internal.h @@ -137,6 +137,24 @@ static inline bool llapi_stripe_index_is_valid(int64_t index) return index >= -1 && index <= LOV_V1_INSANE_STRIPE_COUNT; } +static inline bool llapi_pool_name_is_valid(const char **pool_name) +{ + const char *ptr; + + if (*pool_name == NULL) + return false; + + /* Strip off any 'fsname.' portion. */ + ptr = strchr(*pool_name, '.'); + if (ptr != NULL) + *pool_name = ptr + 1; + + if (strlen(*pool_name) > LOV_MAXPOOLNAME) + return false; + + return true; +} + /* Compatibility macro for legacy llapi functions that use "offset" * terminology instead of the preferred "index". */ #define llapi_stripe_offset_is_valid(os) llapi_stripe_index_is_valid(os) -- 1.8.3.1