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 <mpershin@whamcloud.com>
Reviewed-on: https://review.whamcloud.com/27753
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Andrew Perepechko <andrew.perepechko@hpe.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/mgs/mgs_llog.c b/lustre/mgs/mgs_llog.c
index 23890f8..9966018 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 ] &&