From 93dec71f65a4bbb170a35b04c60b8620b73ae4b1 Mon Sep 17 00:00:00 2001 From: jcl Date: Fri, 16 Aug 2013 19:23:21 +0200 Subject: [PATCH] LU-3756 mdt: Change HSM policy display HSM policy display must follow the format: policy1 policy2 policy3 ... with [] around active policies Add sanity-hsm test 109 to test policy setting. Signed-off-by: JC Lafoucriere Change-Id: I2000b9b8adfaf4cd2b369acbd076e65166832fc5 Reviewed-on: http://review.whamcloud.com/7365 Tested-by: Hudson Tested-by: Maloo Reviewed-by: John L. Hammond Reviewed-by: Jinshan Xiong Reviewed-by: Oleg Drokin --- lustre/mdt/mdt_coordinator.c | 150 +++++++++++++++++++++++++------------------ lustre/mdt/mdt_internal.h | 10 +-- lustre/tests/sanity-hsm.sh | 74 +++++++++++++++++---- 3 files changed, 155 insertions(+), 79 deletions(-) diff --git a/lustre/mdt/mdt_coordinator.c b/lustre/mdt/mdt_coordinator.c index fa92f75..fbde2c5 100644 --- a/lustre/mdt/mdt_coordinator.c +++ b/lustre/mdt/mdt_coordinator.c @@ -922,6 +922,7 @@ int mdt_hsm_cdt_start(struct mdt_device *mdt) RETURN(-EALREADY); } + CLASSERT(1 << (CDT_POLICY_SHIFT_COUNT - 1) == CDT_POLICY_LAST); cdt->cdt_policy = CDT_DEFAULT_POLICY; cdt->cdt_state = CDT_INIT; @@ -1716,8 +1717,8 @@ static const struct { char *name; char *nickname; } hsm_policy_names[] = { - { CDT_NONBLOCKING_RESTORE, "non_blocking_restore", "nbr"}, - { CDT_NORETRY_ACTION, "no_retry_action", "nra"}, + { CDT_NONBLOCKING_RESTORE, "NonBlockingRestore", "NBR"}, + { CDT_NORETRY_ACTION, "NoRetryAction", "NRA"}, { 0 }, }; @@ -1732,7 +1733,8 @@ static __u64 hsm_policy_str2bit(const char *name) int i; for (i = 0; hsm_policy_names[i].bit != 0; i++) - if (strcmp(hsm_policy_names[i].nickname, name) == 0) + if (strcmp(hsm_policy_names[i].nickname, name) == 0 || + strcmp(hsm_policy_names[i].name, name) == 0) return hsm_policy_names[i].bit; return 0; } @@ -1740,11 +1742,13 @@ static __u64 hsm_policy_str2bit(const char *name) /** * convert a policy bit field to a string * \param mask [IN] policy bit field + * \param hexa [IN] print mask before bit names * \param buffer [OUT] string * \param count [IN] size of buffer * \retval size filled in buffer */ -static int hsm_policy_bit2str(const __u64 mask, char *buffer, int count) +static int hsm_policy_bit2str(const __u64 mask, const bool hexa, char *buffer, + int count) { int i, j, sz; char *ptr; @@ -1752,25 +1756,31 @@ static int hsm_policy_bit2str(const __u64 mask, char *buffer, int count) ENTRY; ptr = buffer; - sz = snprintf(buffer, count, "("LPX64") ", mask); - ptr += sz; - count -= sz; - for (i = 0; i < (sizeof(mask) * 8); i++) { + if (hexa) { + sz = snprintf(buffer, count, "("LPX64") ", mask); + ptr += sz; + count -= sz; + } + for (i = 0; i < CDT_POLICY_SHIFT_COUNT; i++) { bit = (1ULL << i); - if (!(bit & mask)) - continue; for (j = 0; hsm_policy_names[j].bit != 0; j++) { - if (hsm_policy_names[j].bit == bit) { - sz = snprintf(ptr, count, "%s(%s) ", - hsm_policy_names[j].name, - hsm_policy_names[j].nickname); - ptr += sz; - count -= sz; + if (hsm_policy_names[j].bit == bit) break; - } } + if (bit & mask) + sz = snprintf(ptr, count, "[%s] ", + hsm_policy_names[j].name); + else + sz = snprintf(ptr, count, "%s ", + hsm_policy_names[j].name); + + ptr += sz; + count -= sz; } + /* remove last ' ' */ + *ptr = '\0'; + ptr--; RETURN(ptr - buffer); } @@ -1783,7 +1793,7 @@ static int lprocfs_rd_hsm_policy(char *page, char **start, off_t off, int sz; ENTRY; - sz = hsm_policy_bit2str(cdt->cdt_policy, page, count); + sz = hsm_policy_bit2str(cdt->cdt_policy, false, page, count); page[sz] = '\n'; sz++; page[sz] = '\0'; @@ -1796,24 +1806,16 @@ static int lprocfs_wr_hsm_policy(struct file *file, const char *buffer, { struct mdt_device *mdt = data; struct coordinator *cdt = &mdt->mdt_coordinator; - int sz; - char *start, *end; - __u64 policy; - int set; + char *start, *token, sign; char *buf; + __u64 policy; + __u64 add_mask, remove_mask, set_mask; + int sz; + int rc; ENTRY; - if (strncmp(buffer, "help", 4) == 0) { - sz = PAGE_SIZE; - OBD_ALLOC(buf, sz); - if (!buf) - RETURN(-ENOMEM); - - hsm_policy_bit2str(CDT_POLICY_MASK, buf, sz); - CWARN("Supported policies are: %s\n", buf); - OBD_FREE(buf, sz); - RETURN(count); - } + if (count + 1 > PAGE_SIZE) + RETURN(-EINVAL); OBD_ALLOC(buf, count + 1); if (buf == NULL) @@ -1821,48 +1823,74 @@ static int lprocfs_wr_hsm_policy(struct file *file, const char *buffer, if (copy_from_user(buf, buffer, count)) RETURN(-EFAULT); - buf[count] = '\0'; + start = buf; + CDEBUG(D_HSM, "%s: receive new policy: '%s'\n", mdt_obd_name(mdt), + start); - policy = 0; + add_mask = remove_mask = set_mask = 0; do { - end = strchr(start, ' '); - if (end != NULL) - *end = '\0'; - switch (*start) { + token = strsep(&start, "\n "); + sign = *token; + + if (sign == '\0') + continue; + + if (sign == '-' || sign == '+') + token++; + + policy = hsm_policy_str2bit(token); + if (policy == 0) { + char *msg; + + sz = PAGE_SIZE; + OBD_ALLOC(msg, sz); + if (!msg) + RETURN(-ENOMEM); + + hsm_policy_bit2str(0, false, msg, sz); + CWARN("%s: '%s' is unknown, " + "supported policies are: %s\n", mdt_obd_name(mdt), + token, msg); + OBD_FREE(msg, sz); + GOTO(out, rc = -EINVAL); + } + switch (sign) { case '-': - start++; - set = 0; + remove_mask |= policy; break; case '+': - start++; - set = 1; + add_mask |= policy; break; default: - set = 2; + set_mask |= policy; break; } - policy = hsm_policy_str2bit(start); - if (!policy) - break; - switch (set) { - case 0: - cdt->cdt_policy &= ~policy; - break; - case 1: - cdt->cdt_policy |= policy; - break; - case 2: - cdt->cdt_policy = policy; - break; - } + } while (start != NULL); + + CDEBUG(D_HSM, "%s: new policy: rm="LPX64" add="LPX64" set="LPX64"\n", + mdt_obd_name(mdt), remove_mask, add_mask, set_mask); - start = end + 1; - } while (end != NULL); + /* if no sign in all string, it is a clear and set + * if some sign found, all unsigned are converted + * to add + * P1 P2 = set to P1 and P2 + * P1 -P2 = add P1 clear P2 same as +P1 -P2 + */ + if (remove_mask == 0 && add_mask == 0) { + cdt->cdt_policy = set_mask; + } else { + cdt->cdt_policy |= set_mask | add_mask; + cdt->cdt_policy &= ~remove_mask; + } + + GOTO(out, rc = count); + +out: OBD_FREE(buf, count + 1); - RETURN(count); + RETURN(rc); } #define GENERATE_PROC_METHOD(VAR) \ diff --git a/lustre/mdt/mdt_internal.h b/lustre/mdt/mdt_internal.h index afce519..7db09e8 100644 --- a/lustre/mdt/mdt_internal.h +++ b/lustre/mdt/mdt_internal.h @@ -84,10 +84,12 @@ struct mdt_file_data { struct mdt_object *mfd_object; /* point to opened object */ }; -#define CDT_NONBLOCKING_RESTORE 0x0000000000000001ULL -#define CDT_NORETRY_ACTION 0x0000000000000002ULL -#define CDT_POLICY_MASK CDT_NONBLOCKING_RESTORE | \ - CDT_NORETRY_ACTION +#define CDT_NONBLOCKING_RESTORE (1ULL << 0) +#define CDT_NORETRY_ACTION (1ULL << 1) +#define CDT_POLICY_LAST CDT_NORETRY_ACTION +#define CDT_POLICY_SHIFT_COUNT 2 +#define CDT_POLICY_ALL (CDT_NONBLOCKING_RESTORE | \ + CDT_NORETRY_ACTION) /* when adding a new policy, do not forget to update * lustre/mdt/mdt_coordinator.c::hsm_policy_names[] diff --git a/lustre/tests/sanity-hsm.sh b/lustre/tests/sanity-hsm.sh index 7aa54d5..5209cab 100644 --- a/lustre/tests/sanity-hsm.sh +++ b/lustre/tests/sanity-hsm.sh @@ -114,6 +114,7 @@ copytool_device() { cleanup() { copytool_cleanup changelog_cleanup + cdt_set_sanity_policy } search_and_kill_copytool() { @@ -243,26 +244,33 @@ set_test_state() { } cdt_set_sanity_policy() { - # clear all - do_facet $SINGLEMDS $LCTL set_param $HSM_PARAM.policy=-nra - do_facet $SINGLEMDS $LCTL set_param $HSM_PARAM.policy=-nbr - do_facet $SINGLEMDS $LCTL set_param $HSM_PARAM.policy=-gc + if [[ "$CDT_POLICY_HAD_CHANGED" ]] + then + # clear all + do_facet $SINGLEMDS $LCTL set_param $HSM_PARAM.policy=+NRA + do_facet $SINGLEMDS $LCTL set_param $HSM_PARAM.policy=-NBR + CDT_POLICY_HAD_CHANGED= + fi } cdt_set_no_retry() { - do_facet $SINGLEMDS $LCTL set_param $HSM_PARAM.policy=+nra + do_facet $SINGLEMDS $LCTL set_param $HSM_PARAM.policy=+NRA + CDT_POLICY_HAD_CHANGED=true } cdt_clear_no_retry() { - do_facet $SINGLEMDS $LCTL set_param $HSM_PARAM.policy=-nra + do_facet $SINGLEMDS $LCTL set_param $HSM_PARAM.policy=-NRA + CDT_POLICY_HAD_CHANGED=true } -cdt_set_no_blocking_restore() { - do_facet $SINGLEMDS $LCTL set_param $HSM_PARAM.policy=+nbr +cdt_set_non_blocking_restore() { + do_facet $SINGLEMDS $LCTL set_param $HSM_PARAM.policy=+NBR + CDT_POLICY_HAD_CHANGED=true } -cdt_clear_no_blocking_restore() { - do_facet $SINGLEMDS $LCTL set_param $HSM_PARAM.policy=-nbr +cdt_clear_non_blocking_restore() { + do_facet $SINGLEMDS $LCTL set_param $HSM_PARAM.policy=-NBR + CDT_POLICY_HAD_CHANGED=true } cdt_clear_mount_state() { @@ -512,6 +520,9 @@ cdt_check_state enabled echo "Start copytool" copytool_setup +echo "Set sanity-hsm HSM policy" +cdt_set_sanity_policy + # finished requests are quickly removed from list set_hsm_param grace_delay 10 @@ -2371,6 +2382,41 @@ test_107() { } run_test 107 "Copytool re-register after MDS restart" +policy_set_and_test() +{ + local change="$1" + local target="$2" + do_facet $SINGLEMDS $LCTL set_param "$HSM_PARAM.policy=\\\"$change\\\"" + local policy=$(do_facet $SINGLEMDS $LCTL get_param -n $HSM_PARAM.policy) + [[ "$policy" == "$target" ]] || + error "Wrong policy after '$change': '$policy' != '$target'" +} + +test_109() { + # to force default policy setting if error + CDT_POLICY_HAD_CHANGED=true + + local policy=$(do_facet $SINGLEMDS $LCTL get_param -n $HSM_PARAM.policy) + local default="NonBlockingRestore [NoRetryAction]" + [[ "$policy" == "$default" ]] || + error "default policy has changed,"\ + " '$policy' != '$default' update the test" + policy_set_and_test "+NBR" "[NonBlockingRestore] [NoRetryAction]" + policy_set_and_test "+NRA" "[NonBlockingRestore] [NoRetryAction]" + policy_set_and_test "-NBR" "NonBlockingRestore [NoRetryAction]" + policy_set_and_test "-NRA" "NonBlockingRestore NoRetryAction" + policy_set_and_test "NRA NBR" "[NonBlockingRestore] [NoRetryAction]" + # useless bacause we know but safer for futur changes to use real value + local policy=$(do_facet $SINGLEMDS $LCTL get_param -n $HSM_PARAM.policy) + echo "Next set_param must failed" + policy_set_and_test "wrong" "$policy" + + # return to default + echo "Back to default policy" + cdt_set_sanity_policy +} +run_test 109 "Policy display/change" + test_110a() { # test needs a running copytool copytool_setup @@ -2381,13 +2427,13 @@ test_110a() { import_file $tdir/$tfile $f local fid=$(path2fid $f) - cdt_set_no_blocking_restore + cdt_set_non_blocking_restore md5sum $f local st=$? # cleanup wait_request_state $fid RESTORE SUCCEED - cdt_clear_no_blocking_restore + cdt_clear_non_blocking_restore # Test result [[ $st == 1 ]] || @@ -2409,13 +2455,13 @@ test_110b() { wait_request_state $fid ARCHIVE SUCCEED $LFS hsm_release $f - cdt_set_no_blocking_restore + cdt_set_non_blocking_restore md5sum $f local st=$? # cleanup wait_request_state $fid RESTORE SUCCEED - cdt_clear_no_blocking_restore + cdt_clear_non_blocking_restore # Test result [[ $st == 1 ]] || -- 1.8.3.1