From c152f7b0c84cab95e55fb0ba19d0b6bdeefd6e12 Mon Sep 17 00:00:00 2001 From: Ben Evans Date: Thu, 14 Jan 2016 14:58:15 -0600 Subject: [PATCH] LU-7670 mdt: allow changelog commands to return errors Return errors to lctl/lfs users for out of range, and invalid IDs. Currently only 0 is ever returned, regardless of outcome. Some information is printed in the MDS logs, but nothing on the client to indicate success or failure. Split purge and clear code into 2 separate paths. Signed-off-by: Ben Evans Change-Id: Ie139b170da0c8bef1c315ddb5361783230bb51ad Reviewed-on: https://review.whamcloud.com/18030 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Quentin Bouget Reviewed-by: Frank Zago Reviewed-by: Oleg Drokin --- lustre/mdd/mdd_device.c | 376 +++++++++++++++++++++++++++++++---------------- lustre/mdt/mdt_handler.c | 28 +--- lustre/tests/sanity.sh | 30 ++++ lustre/utils/lfs.c | 31 ++-- 4 files changed, 301 insertions(+), 164 deletions(-) diff --git a/lustre/mdd/mdd_device.c b/lustre/mdd/mdd_device.c index a6697d5..88378b1 100644 --- a/lustre/mdd/mdd_device.c +++ b/lustre/mdd/mdd_device.c @@ -1331,135 +1331,253 @@ out: RETURN(rc); } -struct mdd_changelog_user_data { - __u64 mcud_endrec; /**< purge record for this user */ - __u64 mcud_minrec; /**< lowest changelog recno still referenced */ - __u32 mcud_id; - __u32 mcud_minid; /**< user id with lowest rec reference */ - __u32 mcud_usercount; - unsigned int mcud_found:1; +struct mdd_changelog_user_purge { + __u32 mcup_id; + __u32 mcup_usercount; + __u64 mcup_minrec; + bool mcup_found; }; -#define MCUD_UNREGISTER -1LL -/** Two things: - * 1. Find the smallest record everyone is willing to purge - * 2. Update the last purgeable record for this user +/** + * changelog_user_purge callback + * + * Is called once per user. + * + * Check to see the user requested is available from rec. + * Truncate the changelog. + * Keep track of the total number of users (calls). */ static int mdd_changelog_user_purge_cb(const struct lu_env *env, struct llog_handle *llh, struct llog_rec_hdr *hdr, void *data) { struct llog_changelog_user_rec *rec; - struct mdd_changelog_user_data *mcud = data; + struct mdd_changelog_user_purge *mcup = data; + struct llog_cookie cookie; int rc; - ENTRY; + ENTRY; - LASSERT(llh->lgh_hdr->llh_flags & LLOG_F_IS_PLAIN); + if ((llh->lgh_hdr->llh_flags & LLOG_F_IS_PLAIN) == 0) + RETURN(-ENXIO); - rec = (struct llog_changelog_user_rec *)hdr; + rec = container_of(hdr, struct llog_changelog_user_rec, cur_hdr); - mcud->mcud_usercount++; + mcup->mcup_usercount++; - /* If we have a new endrec for this id, use it for the following - min check instead of its old value */ - if (rec->cur_id == mcud->mcud_id) - rec->cur_endrec = max(rec->cur_endrec, mcud->mcud_endrec); + if (rec->cur_id != mcup->mcup_id) { + /* truncate to the lowest endrec that is not this user */ + mcup->mcup_minrec = min(mcup->mcup_minrec, + rec->cur_endrec); + RETURN(0); + } - /* Track the minimum referenced record */ - if (mcud->mcud_minid == 0 || mcud->mcud_minrec > rec->cur_endrec) { - mcud->mcud_minid = rec->cur_id; - mcud->mcud_minrec = rec->cur_endrec; - } + /* Unregister this user */ + cookie.lgc_lgl = llh->lgh_id; + cookie.lgc_index = hdr->lrh_index; - if (rec->cur_id != mcud->mcud_id) - RETURN(0); + rc = llog_cat_cancel_records(env, llh->u.phd.phd_cat_handle, + 1, &cookie); + if (rc == 0) { + mcup->mcup_found = true; + mcup->mcup_usercount--; + } - /* Update this user's record */ - mcud->mcud_found = 1; + RETURN(rc); +} - /* Special case: unregister this user */ - if (mcud->mcud_endrec == MCUD_UNREGISTER) { - struct llog_cookie cookie; +static int mdd_changelog_user_purge(const struct lu_env *env, + struct mdd_device *mdd, __u32 id) +{ + struct mdd_changelog_user_purge mcup = { + .mcup_id = id, + .mcup_found = false, + .mcup_usercount = 0, + .mcup_minrec = ULLONG_MAX, + }; + struct llog_ctxt *ctxt; + int rc; - cookie.lgc_lgl = llh->lgh_id; - cookie.lgc_index = hdr->lrh_index; + ENTRY; - rc = llog_cat_cancel_records(env, llh->u.phd.phd_cat_handle, - 1, &cookie); - if (rc == 0) - mcud->mcud_usercount--; + CDEBUG(D_IOCTL, "%s: Purge request: id=%u\n", + mdd2obd_dev(mdd)->obd_name, id); - RETURN(rc); - } + ctxt = llog_get_context(mdd2obd_dev(mdd), + LLOG_CHANGELOG_USER_ORIG_CTXT); + if (ctxt == NULL || + (ctxt->loc_handle->lgh_hdr->llh_flags & LLOG_F_IS_CAT) == 0) + GOTO(out, rc = -ENXIO); + + rc = llog_cat_process(env, ctxt->loc_handle, + mdd_changelog_user_purge_cb, &mcup, + 0, 0); + + if ((rc == 0) && mcup.mcup_found) { + CDEBUG(D_IOCTL, "%s: Purging changelog entries for user %d " + "record=%llu\n", + mdd2obd_dev(mdd)->obd_name, id, mcup.mcup_minrec); + /* Cancelling record 0 destroys the entire changelog, make sure + we don't do that unless we mean it. */ + if (mcup.mcup_minrec != 0 || mcup.mcup_usercount == 0) { + rc = mdd_changelog_llog_cancel(env, mdd, + mcup.mcup_minrec); + } + } else { + CWARN("%s: No changelog for user %u; rc=%d\n", + mdd2obd_dev(mdd)->obd_name, id, rc); + GOTO(out, rc = -ENOENT); + } + + if ((rc == 0) && (mcup.mcup_usercount == 0)) { + /* No more users; turn changelogs off */ + CDEBUG(D_IOCTL, "turning off changelogs\n"); + rc = mdd_changelog_off(env, mdd); + } + + EXIT; +out: + if (ctxt != NULL) + llog_ctxt_put(ctxt); + + return rc; +} + +struct mdd_changelog_user_clear { + __u64 mcuc_endrec; + __u64 mcuc_minrec; + __u32 mcuc_id; + bool mcuc_flush; +}; + +/** + * changelog_clear callback + * + * Is called once per user. + * + * Check to see the user requested is available from rec. + * Check the oldest (smallest) record for boundary conditions. + * Truncate the changelog. + */ +static int mdd_changelog_clear_cb(const struct lu_env *env, + struct llog_handle *llh, + struct llog_rec_hdr *hdr, + void *data) +{ + struct llog_changelog_user_rec *rec; + struct mdd_changelog_user_clear *mcuc = data; + int rc; + + ENTRY; + + if ((llh->lgh_hdr->llh_flags & LLOG_F_IS_PLAIN) == 0) + RETURN(-ENXIO); + + rec = container_of(hdr, struct llog_changelog_user_rec, cur_hdr); + + /* Does the changelog id match the requested id? */ + if (rec->cur_id != mcuc->mcuc_id) { + mcuc->mcuc_minrec = min(mcuc->mcuc_minrec, + rec->cur_endrec); + RETURN(0); + } + + /* cur_endrec is the oldest purgeable record, make sure we're newer */ + if (rec->cur_endrec > mcuc->mcuc_endrec) { + CDEBUG(D_IOCTL, "Request %llu out of range: %llu\n", + mcuc->mcuc_endrec, rec->cur_endrec); + RETURN(-EINVAL); + } + + /* Flag that we've met all the range and user checks. + * We now know the record to flush. + */ + rec->cur_endrec = mcuc->mcuc_endrec; + mcuc->mcuc_flush = true; - /* Update the endrec */ - CDEBUG(D_IOCTL, "Rewriting changelog user %d endrec to %llu\n", - mcud->mcud_id, rec->cur_endrec); + CDEBUG(D_IOCTL, "Rewriting changelog user %u endrec to %llu\n", + mcuc->mcuc_id, rec->cur_endrec); + /* Update the endrec */ rc = llog_write(env, llh, hdr, hdr->lrh_index); - RETURN(rc); + RETURN(rc); } -static int mdd_changelog_user_purge(const struct lu_env *env, - struct mdd_device *mdd, int id, - long long endrec) +/** + * Clear a changelog up to entry specified by endrec for user id. + */ +static int mdd_changelog_clear(const struct lu_env *env, + struct mdd_device *mdd, __u32 id, + __u64 endrec) { - struct mdd_changelog_user_data data; - struct llog_ctxt *ctxt; - int rc; - ENTRY; + struct mdd_changelog_user_clear mcuc = { + .mcuc_id = id, + .mcuc_minrec = endrec, + .mcuc_flush = false, + }; + struct llog_ctxt *ctxt; + __u64 start_rec; + int rc; - CDEBUG(D_IOCTL, "Purge request: id=%d, endrec=%lld\n", id, endrec); + ENTRY; - data.mcud_id = id; - data.mcud_minid = 0; - data.mcud_minrec = 0; - data.mcud_usercount = 0; - data.mcud_endrec = endrec; + CDEBUG(D_IOCTL, "%s: Purge request: id=%u, endrec=%llu\n", + mdd2obd_dev(mdd)->obd_name, id, endrec); + /* start_rec is the newest (largest value) entry in the changelogs*/ spin_lock(&mdd->mdd_cl.mc_lock); - endrec = mdd->mdd_cl.mc_index; + start_rec = mdd->mdd_cl.mc_index; spin_unlock(&mdd->mdd_cl.mc_lock); - if ((data.mcud_endrec == 0) || - ((data.mcud_endrec > endrec) && - (data.mcud_endrec != MCUD_UNREGISTER))) - data.mcud_endrec = endrec; - ctxt = llog_get_context(mdd2obd_dev(mdd), - LLOG_CHANGELOG_USER_ORIG_CTXT); - if (ctxt == NULL) - return -ENXIO; + if (start_rec < endrec) { + CDEBUG(D_IOCTL, "%s: Could not clear changelog, requested "\ + "address out of range\n", mdd2obd_dev(mdd)->obd_name); + RETURN(-EINVAL); + } - LASSERT(ctxt->loc_handle->lgh_hdr->llh_flags & LLOG_F_IS_CAT); + if (endrec == 0) { + mcuc.mcuc_endrec = start_rec; + mcuc.mcuc_minrec = ULLONG_MAX; + } else { + mcuc.mcuc_endrec = endrec; + } + + ctxt = llog_get_context(mdd2obd_dev(mdd), + LLOG_CHANGELOG_USER_ORIG_CTXT); + if (ctxt == NULL || + (ctxt->loc_handle->lgh_hdr->llh_flags & LLOG_F_IS_CAT) == 0) + GOTO(out, rc = -ENXIO); rc = llog_cat_process(env, ctxt->loc_handle, - mdd_changelog_user_purge_cb, (void *)&data, + mdd_changelog_clear_cb, (void *)&mcuc, 0, 0); - if ((rc >= 0) && (data.mcud_minrec > 0)) { - CDEBUG(D_IOCTL, "Purging changelog entries up to %lld" - ", referenced by "CHANGELOG_USER_PREFIX"%d\n", - data.mcud_minrec, data.mcud_minid); - rc = mdd_changelog_llog_cancel(env, mdd, data.mcud_minrec); - } else { - CWARN("Could not determine changelog records to purge; rc=%d\n", - rc); - } - - llog_ctxt_put(ctxt); - if (!data.mcud_found) { - CWARN("No entry for user %d. Last changelog reference is " - "%lld by changelog user %d\n", data.mcud_id, - data.mcud_minrec, data.mcud_minid); - rc = -ENOENT; - } + if (rc < 0) { + CWARN("%s: Failure to clear the changelog for user %d: %d\n", + mdd2obd_dev(mdd)->obd_name, id, rc); + } else if (mcuc.mcuc_flush) { + /* Cancelling record 0 destroys the entire changelog, make sure + we don't do that unless we mean it. */ + if (mcuc.mcuc_minrec != 0) { + CDEBUG(D_IOCTL, "%s: Purging changelog entries up "\ + "to %llu\n", mdd2obd_dev(mdd)->obd_name, + mcuc.mcuc_minrec); + + rc = mdd_changelog_llog_cancel(env, mdd, + mcuc.mcuc_minrec); + } + } else { + CWARN("%s: No entry for user %d\n", + mdd2obd_dev(mdd)->obd_name, id); + rc = -ENOENT; + } - if (!rc && data.mcud_usercount == 0) - /* No more users; turn changelogs off */ - rc = mdd_changelog_off(env, mdd); + EXIT; +out: + if (ctxt != NULL) + llog_ctxt_put(ctxt); - RETURN(rc); + return rc; } /** mdd_iocontrol @@ -1473,26 +1591,27 @@ static int mdd_changelog_user_purge(const struct lu_env *env, static int mdd_iocontrol(const struct lu_env *env, struct md_device *m, unsigned int cmd, int len, void *karg) { - struct mdd_device *mdd; - struct obd_ioctl_data *data = karg; - int rc; - ENTRY; + struct mdd_device *mdd; + struct obd_ioctl_data *data = karg; + int rc; - mdd = lu2mdd_dev(&m->md_lu_dev); + ENTRY; - /* Doesn't use obd_ioctl_data */ - switch (cmd) { - case OBD_IOC_CHANGELOG_CLEAR: { - struct changelog_setinfo *cs = karg; - rc = mdd_changelog_user_purge(env, mdd, cs->cs_id, - cs->cs_recno); - RETURN(rc); - } - case OBD_IOC_GET_MNTOPT: { - mntopt_t *mntopts = (mntopt_t *)karg; - *mntopts = mdd->mdd_dt_conf.ddp_mntopts; - RETURN(0); - } + mdd = lu2mdd_dev(&m->md_lu_dev); + + /* Doesn't use obd_ioctl_data */ + switch (cmd) { + case OBD_IOC_CHANGELOG_CLEAR: { + struct changelog_setinfo *cs = karg; + rc = mdd_changelog_clear(env, mdd, cs->cs_id, + cs->cs_recno); + RETURN(rc); + } + case OBD_IOC_GET_MNTOPT: { + mntopt_t *mntopts = (mntopt_t *)karg; + *mntopts = mdd->mdd_dt_conf.ddp_mntopts; + RETURN(0); + } case OBD_IOC_START_LFSCK: { rc = lfsck_start(env, mdd->mdd_bottom, (struct lfsck_start_param *)karg); @@ -1510,30 +1629,29 @@ static int mdd_iocontrol(const struct lu_env *env, struct md_device *m, } } - /* Below ioctls use obd_ioctl_data */ - if (len != sizeof(*data)) { - CERROR("Bad ioctl size %d\n", len); - RETURN(-EINVAL); - } - if (data->ioc_version != OBD_IOCTL_VERSION) { - CERROR("Bad magic %x != %x\n", data->ioc_version, - OBD_IOCTL_VERSION); - RETURN(-EINVAL); - } + /* Below ioctls use obd_ioctl_data */ + if (len != sizeof(*data)) { + CERROR("Bad ioctl size %d\n", len); + RETURN(-EINVAL); + } + if (data->ioc_version != OBD_IOCTL_VERSION) { + CERROR("Bad magic %x != %x\n", data->ioc_version, + OBD_IOCTL_VERSION); + RETURN(-EINVAL); + } - switch (cmd) { - case OBD_IOC_CHANGELOG_REG: - rc = mdd_changelog_user_register(env, mdd, &data->ioc_u32_1); - break; - case OBD_IOC_CHANGELOG_DEREG: - rc = mdd_changelog_user_purge(env, mdd, data->ioc_u32_1, - MCUD_UNREGISTER); - break; - default: - rc = -ENOTTY; - } + switch (cmd) { + case OBD_IOC_CHANGELOG_REG: + rc = mdd_changelog_user_register(env, mdd, &data->ioc_u32_1); + break; + case OBD_IOC_CHANGELOG_DEREG: + rc = mdd_changelog_user_purge(env, mdd, data->ioc_u32_1); + break; + default: + rc = -ENOTTY; + } - RETURN (rc); + RETURN(rc); } /* type constructor/destructor: mdd_type_init, mdd_type_fini */ diff --git a/lustre/mdt/mdt_handler.c b/lustre/mdt/mdt_handler.c index e5da354..7463c7d 100644 --- a/lustre/mdt/mdt_handler.c +++ b/lustre/mdt/mdt_handler.c @@ -5857,30 +5857,6 @@ int mdt_get_info(struct tgt_session_info *tsi) RETURN(rc); } -/* Pass the ioc down */ -static int mdt_ioc_child(struct lu_env *env, struct mdt_device *mdt, - unsigned int cmd, int len, void *data) -{ - struct lu_context ioctl_session; - struct md_device *next = mdt->mdt_child; - int rc; - ENTRY; - - rc = lu_context_init(&ioctl_session, LCT_SERVER_SESSION); - if (rc) - RETURN(rc); - ioctl_session.lc_thread = (struct ptlrpc_thread *)current; - lu_context_enter(&ioctl_session); - env->le_ses = &ioctl_session; - - LASSERT(next->md_ops->mdo_iocontrol); - rc = next->md_ops->mdo_iocontrol(env, next, cmd, len, data); - - lu_context_exit(&ioctl_session); - lu_context_fini(&ioctl_session); - RETURN(rc); -} - static int mdt_ioc_version_get(struct mdt_thread_info *mti, void *karg) { struct obd_ioctl_data *data = karg; @@ -5962,7 +5938,9 @@ static int mdt_iocontrol(unsigned int cmd, struct obd_export *exp, int len, case OBD_IOC_CHANGELOG_REG: case OBD_IOC_CHANGELOG_DEREG: case OBD_IOC_CHANGELOG_CLEAR: - rc = mdt_ioc_child(&env, mdt, cmd, len, karg); + rc = mdt->mdt_child->md_ops->mdo_iocontrol(&env, + mdt->mdt_child, + cmd, len, karg); break; case OBD_IOC_START_LFSCK: { struct md_device *next = mdt->mdt_child; diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index c24e419..e2d5416 100755 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -10945,6 +10945,36 @@ test_160d() { } run_test 160d "verify that changelog log catch the migrate event" +test_160e() { + # Create a user + CL_USER=$(do_facet $SINGLEMDS $LCTL --device $MDT0 \ + changelog_register -n) + echo "Registered as changelog user $CL_USER" + trap cleanup_changelog EXIT + + # Delete a future user (expect fail) + do_facet $SINGLEMDS $LCTL --device $MDT0 changelog_deregister cl77 + local rc=$? + + if [ $rc -eq 0 ]; then + error "Deleted non-existant user cl77" + elif [ $rc -ne 2 ]; then + error "changelog_deregister failed with $rc, " \ + "expected 2 (ENOENT)" + fi + + # Clear to a bad index (1 billion should be safe) + $LFS changelog_clear $MDT0 $CL_USER 1000000000 + rc=$? + + if [ $rc -eq 0 ]; then + error "Successfully cleared to invalid CL index" + elif [ $rc -ne 22 ]; then + error "changelog_clear failed with $rc, expected 22 (EINVAL)" + fi +} +run_test 160e "changelog negative testing" + test_161a() { [ $PARALLEL == "yes" ] && skip "skip parallel run" && return test_mkdir -p -c1 $DIR/$tdir diff --git a/lustre/utils/lfs.c b/lustre/utils/lfs.c index e2dc5bb..933f455 100644 --- a/lustre/utils/lfs.c +++ b/lustre/utils/lfs.c @@ -3607,19 +3607,30 @@ static int lfs_changelog(int argc, char **argv) static int lfs_changelog_clear(int argc, char **argv) { - long long endrec; - int rc; + long long endrec; + int rc; - if (argc != 4) - return CMD_HELP; + if (argc != 4) + return CMD_HELP; - endrec = strtoll(argv[3], NULL, 10); + endrec = strtoll(argv[3], NULL, 10); - rc = llapi_changelog_clear(argv[1], argv[2], endrec); - if (rc) - fprintf(stderr, "%s error: %s\n", argv[0], - strerror(errno = -rc)); - return rc; + rc = llapi_changelog_clear(argv[1], argv[2], endrec); + + if (rc == -EINVAL) + fprintf(stderr, "%s: record out of range: %llu\n", + argv[0], endrec); + else if (rc == -ENOENT) + fprintf(stderr, "%s: no changelog user: %s\n", + argv[0], argv[2]); + else if (rc) + fprintf(stderr, "%s error: %s\n", argv[0], + strerror(-rc)); + + if (rc) + errno = -rc; + + return rc; } static int lfs_fid2path(int argc, char **argv) -- 1.8.3.1