From 98f107b53e4daa3bfaf026c379c0a9c41cb5f161 Mon Sep 17 00:00:00 2001 From: Jadhav Vikram Date: Tue, 25 Jul 2017 12:31:37 +0530 Subject: [PATCH] LU-9699 osp: don't assert on OSP duplicating Writeconf on an MDT with index > 0000 will cause "add mdc" to be added to $FSNAME-client config and "add osp" to be added to $FSNAME-MDTXXXX configs. However, the configs may already contain these directives. Duplicating the OSP device will cause the assertion failure in osp_obd_connect(): ASSERTION( osp->opd_connects == 1 ) failed Duplicating the MDC just returns -EEXIST in similar situation. A possible solution is to check configs for duplicates before writing to them. However, sometimes we would like to change nids which are part of "add mdc" and "add osp". Another solution is to mark previous entries with SKIP flags. This patch implements this approach. Since after revoking the config lock, the clients and the MDTs will receive the updated log and apply its newer entries, we still have to handle OSP duplication, but this is only an issue immediately after writeconf processing. Seagate-bug-id: MRP-2634, MRP-3865 Change-Id: Idd7ad43c78d50e6bbe715850503aa0b01fcbf071 Signed-off-by: Mikhail Pershin Reviewed-on: https://review.whamcloud.com/27753 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Andreas Dilger Reviewed-by: Andrew Perepechko Reviewed-by: Oleg Drokin --- lustre/mgs/mgs_llog.c | 10 +++++++++ lustre/osp/osp_dev.c | 51 +++++++-------------------------------------- lustre/osp/osp_internal.h | 2 +- lustre/tests/conf-sanity.sh | 45 ++++++++++++++++++++++++++++++++++++++- 4 files changed, 63 insertions(+), 45 deletions(-) diff --git a/lustre/mgs/mgs_llog.c b/lustre/mgs/mgs_llog.c index 23890f8..9966018 100644 --- a/lustre/mgs/mgs_llog.c +++ b/lustre/mgs/mgs_llog.c @@ -2543,6 +2543,11 @@ static int mgs_write_log_mdc_to_lmv(const struct lu_env *env, if (rc) GOTO(out_free, rc); + rc = mgs_modify(env, mgs, fsdb, mti, logname, mti->mti_svname, + "add mdc", CM_SKIP); + if (rc < 0) + GOTO(out_free, rc); + rc = record_start_log(env, mgs, &llh, logname); if (rc) GOTO(out_free, rc); @@ -2689,6 +2694,11 @@ static int mgs_write_log_osp_to_mdt(const struct lu_env *env, if (rc) GOTO(out_destory, rc); + rc = mgs_modify(env, mgs, fsdb, mti, logname, mti->mti_svname, + "add osp", CM_SKIP); + if (rc < 0) + GOTO(out_destory, rc); + rc = record_start_log(env, mgs, &llh, logname); if (rc) GOTO(out_destory, rc); diff --git a/lustre/osp/osp_dev.c b/lustre/osp/osp_dev.c index b36842f..4413ef8 100644 --- a/lustre/osp/osp_dev.c +++ b/lustre/osp/osp_dev.c @@ -1428,52 +1428,22 @@ static int osp_obd_connect(const struct lu_env *env, struct obd_export **exp, struct obd_device *obd, struct obd_uuid *cluuid, struct obd_connect_data *data, void *localdata) { - struct osp_device *osp = lu2osp_dev(obd->obd_lu_dev); - struct obd_connect_data *ocd; - struct obd_import *imp; - struct lustre_handle conn; - int rc; + struct osp_device *osp = lu2osp_dev(obd->obd_lu_dev); + int rc; ENTRY; - CDEBUG(D_CONFIG, "connect #%d\n", osp->opd_connects); - - rc = class_connect(&conn, obd, cluuid); - if (rc) - RETURN(rc); - - *exp = class_conn2export(&conn); - /* Why should there ever be more than 1 connect? */ - osp->opd_connects++; - LASSERT(osp->opd_connects == 1); - - osp->opd_exp = *exp; - - imp = osp->opd_obd->u.cli.cl_import; - imp->imp_dlm_handle = conn; - LASSERT(data != NULL); LASSERT(data->ocd_connect_flags & OBD_CONNECT_INDEX); - ocd = &imp->imp_connect_data; - *ocd = *data; - - imp->imp_connect_flags_orig = ocd->ocd_connect_flags; - imp->imp_connect_flags2_orig = ocd->ocd_connect_flags2; - ocd->ocd_version = LUSTRE_VERSION_CODE; - ocd->ocd_index = data->ocd_index; + rc = client_connect_import(env, &osp->opd_exp, obd, cluuid, data, + localdata); + if (rc) + RETURN(rc); - rc = ptlrpc_connect_import(imp); - if (rc) { - CERROR("%s: can't connect obd: rc = %d\n", obd->obd_name, rc); - GOTO(out, rc); - } else { - osp->opd_obd->u.cli.cl_seq->lcs_exp = - class_export_get(osp->opd_exp); - } + osp->opd_obd->u.cli.cl_seq->lcs_exp = class_export_get(osp->opd_exp); + *exp = osp->opd_exp; - ptlrpc_pinger_add_import(imp); -out: RETURN(rc); } @@ -1492,14 +1462,9 @@ out: static int osp_obd_disconnect(struct obd_export *exp) { struct obd_device *obd = exp->exp_obd; - struct osp_device *osp = lu2osp_dev(obd->obd_lu_dev); int rc; ENTRY; - /* Only disconnect the underlying layers on the final disconnect. */ - LASSERT(osp->opd_connects == 1); - osp->opd_connects--; - rc = class_disconnect(exp); if (rc) { CERROR("%s: class disconnect error: rc = %d\n", diff --git a/lustre/osp/osp_internal.h b/lustre/osp/osp_internal.h index 2684966..72e79f2 100644 --- a/lustre/osp/osp_internal.h +++ b/lustre/osp/osp_internal.h @@ -181,7 +181,7 @@ struct osp_device { struct obd_device *opd_obd; struct obd_export *opd_exp; struct obd_connect_data *opd_connect_data; - int opd_connects; + /* connection status. */ unsigned int opd_new_connection:1, opd_got_disconnected:1, diff --git a/lustre/tests/conf-sanity.sh b/lustre/tests/conf-sanity.sh index 8f9e321..258b3ff 100644 --- a/lustre/tests/conf-sanity.sh +++ b/lustre/tests/conf-sanity.sh @@ -8049,7 +8049,6 @@ test_108b() { } run_test 108b "migrate from ZFS to ldiskfs" - # # set number of permanent parameters # @@ -8630,6 +8629,50 @@ test_117() { } run_test 117 "lctl get_param return errors properly" +test_119() { + local had_config + local size_mb + + [[ "$MDSCOUNT" -ge 2 ]] || skip "Need more at least 2 MDTs" + + had_config=$(do_facet mds1 "$LCTL get_param debug | grep config") + do_facet mds1 "$LCTL set_param debug=+config" + do_facet mds1 "$LCTL clear" + + setup + do_facet mds2 "$TUNEFS --writeconf $(mdsdevname 2)" &>/dev/null + # mount after writeconf will make "add osp" added to mdt0 config: + # 53 (224)marker 60 (flags=0x01, v2.5.1.0) lustre-MDT0001 'add osp' + # 54 (080)add_uuid nid=... 0: 1:... + # 55 (144)attach 0:lustre-MDT0001-osp-MDT0000 1:osp 2:... + # 56 (144)setup 0:lustre-MDT0001-osp-MDT0000 1:... 2:... + # 57 (136)modify_mdc_tgts add 0:lustre-MDT0000-mdtlov 1:... 2:1 3:1 + # duplicate modify_mdc_tgts caused crashes + + debug_size_save + # using larger debug_mb size to avoid lctl dk log truncation + size_mb=$((DEBUG_SIZE_SAVED * 4)) + for i in {1..3}; do + stop_mdt 2 + # though config processing stops after failed attach and setup + # it will proceed after the failed command after each writeconf + # this is the original scenario of the issue + do_facet mds2 "$TUNEFS --writeconf $(mdsdevname 2)" &>/dev/null + do_facet mds1 "$LCTL set_param debug_mb=$size_mb" + start_mdt 2 + + wait_update_cond mds1 \ + "$LCTL dk | grep -c Processed.log.$FSNAME-MDT0000" \ + ">" 1 300 + done + debug_size_restore + + [[ -z "$had_config" ]] && do_facet mds1 lctl set_param debug=-config + + reformat +} +run_test 119 "writeconf on slave mdt shouldn't duplicate mdc/osp and crash" + test_120() { # LU-11130 [ "$MDSCOUNT" -lt 2 ] && skip "mdt count < 2" [ "$mds1_FSTYPE" != ldiskfs ] && -- 1.8.3.1