From b87233158a1db225f36b47aff4cb95ced2e70f87 Mon Sep 17 00:00:00 2001 From: Fan Yong Date: Thu, 25 Dec 2025 10:59:05 +0800 Subject: [PATCH] DAOS-18381 ddb: properly transfer ddb cmdline options The db_path maybe not contained inside the vos_path parameter, especially under md-on-ssd mode. Related go interfaces need to properly transfer cmdline options (db_path, write_mode, and so on) from control plane to the lower ddb utils. The patch also adds more check for the vos pool open and close status before real VOS operation. Features: control recovery Signed-off-by: Fan Yong --- src/control/cmd/ddb/commands_wrapper.go | 4 +- src/control/cmd/ddb/ddb_commands.go | 5 +- src/control/cmd/ddb/main.go | 32 ++++++-- src/utils/ddb/ddb.h | 1 + src/utils/ddb/ddb_commands.c | 100 ++++++++++++++---------- src/utils/ddb/ddb_parse.h | 2 +- src/utils/ddb/ddb_vos.c | 7 +- src/utils/ddb/ddb_vos.h | 2 +- src/utils/ddb/tests/ddb_test_driver.c | 2 +- 9 files changed, 101 insertions(+), 54 deletions(-) diff --git a/src/control/cmd/ddb/commands_wrapper.go b/src/control/cmd/ddb/commands_wrapper.go index e393b7b7b47..b0900a1b6c4 100644 --- a/src/control/cmd/ddb/commands_wrapper.go +++ b/src/control/cmd/ddb/commands_wrapper.go @@ -258,11 +258,13 @@ func ddbFeature(ctx *DdbContext, path, db_path, enable, disable string, show boo return daosError(C.ddb_run_feature(&ctx.ctx, &options)) } -func ddbRmPool(ctx *DdbContext, path string) error { +func ddbRmPool(ctx *DdbContext, path string, db_path string) error { /* Set up the options */ options := C.struct_rm_pool_options{} options.path = C.CString(path) defer freeString(options.path) + options.db_path = C.CString(db_path) + defer freeString(options.db_path) /* Run the c code command */ return daosError(C.ddb_run_rm_pool(&ctx.ctx, &options)) } diff --git a/src/control/cmd/ddb/ddb_commands.go b/src/control/cmd/ddb/ddb_commands.go index aa8bb7f968a..4a0f57b96f4 100644 --- a/src/control/cmd/ddb/ddb_commands.go +++ b/src/control/cmd/ddb/ddb_commands.go @@ -330,11 +330,14 @@ the path must include the extent, otherwise, it must not.`, Help: "Remove a vos pool.", LongHelp: "", HelpGroup: "vos", + Flags: func(f *grumble.Flags) { + f.String("p", "db_path", "", "Path to the sys db") + }, Args: func(a *grumble.Args) { a.String("path", "Optional, Path to the vos file", grumble.Default("")) }, Run: func(c *grumble.Context) error { - return ddbRmPool(ctx, c.Args.String("path")) + return ddbRmPool(ctx, c.Args.String("path"), c.Flags.String("db_path")) }, Completer: rmPoolCompleter, }) diff --git a/src/control/cmd/ddb/main.go b/src/control/cmd/ddb/main.go index b7ed3672f81..bf544a6c13f 100644 --- a/src/control/cmd/ddb/main.go +++ b/src/control/cmd/ddb/main.go @@ -179,6 +179,10 @@ Example Paths: return nil } + if opts.Args.RunCmd != "" && opts.CmdFile != "" { + return errors.New("Cannot use both command file and a command string") + } + if opts.Debug { log.WithLogLevel(logging.LogLevelDebug) log.Debug("debug output enabled") @@ -193,6 +197,10 @@ Example Paths: if opts.Args.VosPath != "" { if !strings.HasPrefix(string(opts.Args.RunCmd), "feature") && + !strings.HasPrefix(string(opts.Args.RunCmd), "open") && + !strings.HasPrefix(string(opts.Args.RunCmd), "close") && + !strings.HasPrefix(string(opts.Args.RunCmd), "prov_mem") && + !strings.HasPrefix(string(opts.Args.RunCmd), "smd_sync") && !strings.HasPrefix(string(opts.Args.RunCmd), "rm_pool") && !strings.HasPrefix(string(opts.Args.RunCmd), "dev_list") && !strings.HasPrefix(string(opts.Args.RunCmd), "dev_replace") { @@ -201,20 +209,30 @@ Example Paths: return errors.Wrapf(err, "Error opening path: %s", opts.Args.VosPath) } } - } - - if opts.Args.RunCmd != "" && opts.CmdFile != "" { - return errors.New("Cannot use both command file and a command string") - } - if opts.Args.VosPath != "" { ctx.ctx.dc_pool_path = C.CString(string(opts.Args.VosPath)) defer C.free(unsafe.Pointer(ctx.ctx.dc_pool_path)) } + if opts.Args.RunCmd != "" || opts.CmdFile != "" { // Non-interactive mode if opts.Args.RunCmd != "" { - err := runCmdStr(app, string(opts.Args.RunCmd), opts.Args.RunCmdArgs...) + if ddbPoolIsOpen(ctx) { + err = runCmdStr(app, string(opts.Args.RunCmd), opts.Args.RunCmdArgs...) + } else { + param := []string{} + skip := true + + for _, v := range args { + if v == string(opts.Args.VosPath) && skip { + skip = false + } else if v != string(opts.Args.RunCmd) { + param = append(param, v) + } + } + + err = runCmdStr(app, string(opts.Args.RunCmd), param...) + } if err != nil { log.Errorf("Error running command %q %s\n", string(opts.Args.RunCmd), err) } diff --git a/src/utils/ddb/ddb.h b/src/utils/ddb/ddb.h index 3bc63c8f40f..4de44cdb52b 100644 --- a/src/utils/ddb/ddb.h +++ b/src/utils/ddb/ddb.h @@ -204,6 +204,7 @@ struct feature_options { struct rm_pool_options { const char *path; + const char *db_path; }; struct dev_list_options { diff --git a/src/utils/ddb/ddb_commands.c b/src/utils/ddb/ddb_commands.c index 705c0eaabda..a17c55ba84d 100644 --- a/src/utils/ddb/ddb_commands.c +++ b/src/utils/ddb/ddb_commands.c @@ -27,6 +27,24 @@ #define ilog_path_required_error_message "Path to object, dkey, or akey required\n" #define error_msg_write_mode_only "Can only modify the VOS tree in 'write mode'\n" +/* clang-format off */ +#define DDB_POOL_SHOULD_OPEN(ctx) \ + do { \ + if (daos_handle_is_inval((ctx)->dc_poh)) { \ + ddb_error(ctx, "Cannot operate on a closed pool. Open it firstly.\n"); \ + return -DER_NO_HDL; \ + } \ + } while (0) + +#define DDB_POOL_SHOULD_CLOSE(ctx) \ + do { \ + if (daos_handle_is_valid((ctx)->dc_poh)) { \ + ddb_error(ctx, "Cannot operate on an opened pool. Close it firstly.\n"); \ + return -DER_BUSY; \ + } \ + } while (0) +/* clang-format on */ + int ddb_run_version(struct ddb_ctx *ctx) { @@ -62,10 +80,8 @@ ddb_pool_is_open(struct ddb_ctx *ctx) int ddb_run_open(struct ddb_ctx *ctx, struct open_options *opt) { - if (ddb_pool_is_open(ctx)) { - ddb_error(ctx, "Must close pool before can open another\n"); - return -DER_EXIST; - } + DDB_POOL_SHOULD_CLOSE(ctx); + ctx->dc_write_mode = opt->write_mode; return dv_pool_open(opt->path, opt->db_path, &ctx->dc_poh, 0); } @@ -75,10 +91,8 @@ ddb_run_close(struct ddb_ctx *ctx) { int rc; - if (!ddb_pool_is_open(ctx)) { - ddb_error(ctx, "No pool open to close\n"); + if (!ddb_pool_is_open(ctx)) return 0; - } rc = dv_pool_close(ctx->dc_poh); ctx->dc_poh = DAOS_HDL_INVAL; @@ -217,12 +231,9 @@ ddb_run_ls(struct ddb_ctx *ctx, struct ls_options *opt) struct dv_tree_path vtp; struct ls_ctx lsctx = {0}; - if (daos_handle_is_inval(ctx->dc_poh)) { - ddb_error(ctx, "Not connected to a pool. Use 'open' to connect to a pool.\n"); - return -DER_NONEXIST; - } - rc = init_path(ctx, opt->path, &itp); + DDB_POOL_SHOULD_OPEN(ctx); + rc = init_path(ctx, opt->path, &itp); if (!SUCCESS(rc)) return rc; @@ -266,8 +277,9 @@ ddb_run_superblock_dump(struct ddb_ctx *ctx) { int rc; - rc = dv_superblock(ctx->dc_poh, print_superblock_cb, ctx); + DDB_POOL_SHOULD_OPEN(ctx); + rc = dv_superblock(ctx->dc_poh, print_superblock_cb, ctx); if (rc == -DER_DF_INVAL) ddb_error(ctx, "Error with pool superblock"); @@ -331,6 +343,8 @@ ddb_run_value_dump(struct ddb_ctx *ctx, struct value_dump_options *opt) dv_dump_value_cb cb = NULL; int rc; + DDB_POOL_SHOULD_OPEN(ctx); + if (!opt->path) { ddb_error(ctx, "A VOS path to dump is required.\n"); return -DER_INVAL; @@ -383,6 +397,8 @@ ddb_run_ilog_dump(struct ddb_ctx *ctx, struct ilog_dump_options *opt) daos_handle_t coh; int rc; + DDB_POOL_SHOULD_OPEN(ctx); + if (!opt->path) { ddb_error(ctx, ilog_path_required_error_message); return -DER_INVAL; @@ -460,6 +476,8 @@ ddb_run_dtx_dump(struct ddb_ctx *ctx, struct dtx_dump_options *opt) bool both = !(opt->committed ^ opt->active); struct dtx_cb_args args = {.ctx = ctx, .entry_count = 0}; + DDB_POOL_SHOULD_OPEN(ctx); + rc = init_path(ctx, opt->path, &itp); if (!SUCCESS(rc)) return rc; @@ -512,6 +530,8 @@ ddb_run_rm(struct ddb_ctx *ctx, struct rm_options *opt) struct dv_tree_path vtp; int rc; + DDB_POOL_SHOULD_OPEN(ctx); + if (!ctx->dc_write_mode) { ddb_error(ctx, error_msg_write_mode_only); return -DER_INVAL; @@ -549,6 +569,8 @@ ddb_run_value_load(struct ddb_ctx *ctx, struct value_load_options *opt) size_t file_size; int rc; + DDB_POOL_SHOULD_OPEN(ctx); + if (!ctx->dc_write_mode) { ddb_error(ctx, error_msg_write_mode_only); return -DER_INVAL; @@ -616,6 +638,8 @@ process_ilog_op(struct ddb_ctx *ctx, char *path, enum ddb_ilog_op op) daos_handle_t coh = {0}; int rc; + DDB_POOL_SHOULD_OPEN(ctx); + if (!ctx->dc_write_mode) { ddb_error(ctx, error_msg_write_mode_only); return -DER_INVAL; @@ -686,6 +710,8 @@ ddb_run_dtx_cmt_clear(struct ddb_ctx *ctx, struct dtx_cmt_clear_options *opt) daos_handle_t coh = {0}; int rc; + DDB_POOL_SHOULD_OPEN(ctx); + if (!ctx->dc_write_mode) { ddb_error(ctx, error_msg_write_mode_only); return -DER_INVAL; @@ -764,10 +790,7 @@ ddb_run_smd_sync(struct ddb_ctx *ctx, struct smd_sync_options *opt) char db_path[DDB_PATH_MAX] = DEFAULT_DB_PATH; int rc; - if (daos_handle_is_valid(ctx->dc_poh)) { - ddb_print(ctx, "Close pool connection before attempting to sync smd\n"); - return -DER_INVAL; - } + DDB_POOL_SHOULD_CLOSE(ctx); if (opt->nvme_conf != NULL) { if (strlen(opt->nvme_conf) == 0 || strlen(opt->nvme_conf) >= DDB_PATH_MAX) { @@ -816,6 +839,8 @@ ddb_run_vea_dump(struct ddb_ctx *ctx) struct dump_vea_cb_args args = {.dva_ctx = ctx, .dva_count = 0}; int rc; + DDB_POOL_SHOULD_OPEN(ctx); + rc = dv_enumerate_vea(ctx->dc_poh, dump_vea_cb, &args); ddb_printf(ctx, "Total Free Regions: %d\n", args.dva_count); @@ -894,6 +919,8 @@ ddb_run_vea_update(struct ddb_ctx *ctx, struct vea_update_options *opt) uint32_t blk_cnt; int rc; + DDB_POOL_SHOULD_OPEN(ctx); + if (!ctx->dc_write_mode) { ddb_error(ctx, error_msg_write_mode_only); return -DER_INVAL; @@ -983,6 +1010,8 @@ ddb_run_dtx_act_commit(struct ddb_ctx *ctx, struct dtx_act_options *opt) struct dtx_modify_args args = {0}; int rc; + DDB_POOL_SHOULD_OPEN(ctx); + if (!ctx->dc_write_mode) { ddb_error(ctx, error_msg_write_mode_only); return -DER_INVAL; @@ -1013,6 +1042,8 @@ ddb_run_dtx_act_abort(struct ddb_ctx *ctx, struct dtx_act_options *opt) struct dtx_modify_args args = {0}; int rc; + DDB_POOL_SHOULD_OPEN(ctx); + if (!ctx->dc_write_mode) { ddb_error(ctx, error_msg_write_mode_only); return -DER_INVAL; @@ -1115,12 +1146,9 @@ ddb_run_feature(struct ddb_ctx *ctx, struct feature_options *opt) int ddb_run_rm_pool(struct ddb_ctx *ctx, struct rm_pool_options *opt) { - if (ddb_pool_is_open(ctx)) { - ddb_error(ctx, "Must close pool before can open another\n"); - return -DER_BUSY; - } + DDB_POOL_SHOULD_CLOSE(ctx); - return dv_pool_destroy(opt->path); + return dv_pool_destroy(opt->path, opt->db_path); } #define DTI_ALL "all" @@ -1161,6 +1189,8 @@ ddb_run_dtx_act_discard_invalid(struct ddb_ctx *ctx, struct dtx_act_options *opt struct dtx_active_entry_discard_invalid_cb_arg bundle = {.ctx = ctx, .args = &args}; int rc; + DDB_POOL_SHOULD_OPEN(ctx); + if (!ctx->dc_write_mode) { ddb_error(ctx, error_msg_write_mode_only); return -DER_INVAL; @@ -1197,10 +1227,7 @@ ddb_run_dev_list(struct ddb_ctx *ctx, struct dev_list_options *opt) d_list_t dev_list; int rc, dev_cnt = 0; - if (daos_handle_is_valid(ctx->dc_poh)) { - ddb_print(ctx, "Close pool connection before attempting to list devices\n"); - return -DER_INVAL; - } + DDB_POOL_SHOULD_CLOSE(ctx); if (opt->db_path != NULL) { if (strlen(opt->db_path) == 0 || strlen(opt->db_path) >= DDB_PATH_MAX) { @@ -1240,10 +1267,7 @@ ddb_run_dev_replace(struct ddb_ctx *ctx, struct dev_replace_options *opt) uuid_t old_devid, new_devid; int rc; - if (daos_handle_is_valid(ctx->dc_poh)) { - ddb_print(ctx, "Close pool connection before attempting to replace device\n"); - return -DER_INVAL; - } + DDB_POOL_SHOULD_CLOSE(ctx); if (opt->db_path != NULL) { if (strlen(opt->db_path) == 0 || strlen(opt->db_path) >= DDB_PATH_MAX) { @@ -1591,11 +1615,7 @@ ddb_run_dtx_stat(struct ddb_ctx *ctx, struct dtx_stat_options *opt) struct vos_iter_anchors anchors = {0}; int rc; - if (daos_handle_is_inval(ctx->dc_poh)) { - ddb_error(ctx, "Not connected to a pool. Use 'open' to connect to a pool.\n"); - rc = -DER_NONEXIST; - goto done; - } + DDB_POOL_SHOULD_OPEN(ctx); args.ctx = ctx; args.opt = opt; @@ -1723,18 +1743,14 @@ ddb_run_dtx_aggr(struct ddb_ctx *ctx, struct dtx_aggr_options *opt) struct vos_iter_anchors anchors = {0}; int rc; + DDB_POOL_SHOULD_OPEN(ctx); + if (!ctx->dc_write_mode) { ddb_error(ctx, error_msg_write_mode_only); rc = -DER_INVAL; goto done; } - if (daos_handle_is_inval(ctx->dc_poh)) { - ddb_error(ctx, "Not connected to a pool. Use 'open' to connect to a pool.\n"); - rc = -DER_NONEXIST; - goto done; - } - switch (opt->format) { case DDB_DTX_AGGR_NOW: args.cmt_time = NULL; @@ -1774,6 +1790,8 @@ ddb_run_prov_mem(struct ddb_ctx *ctx, struct prov_mem_options *opt) { int rc = 0; + DDB_POOL_SHOULD_CLOSE(ctx); + if (opt->db_path == NULL || strlen(opt->db_path) == 0 || strlen(opt->db_path) >= DDB_PATH_MAX) { ddb_errorf(ctx, "db_path '%s' either too short (==0) or too long (>=%d).\n", diff --git a/src/utils/ddb/ddb_parse.h b/src/utils/ddb/ddb_parse.h index df5d43771db..881452cf1b7 100644 --- a/src/utils/ddb/ddb_parse.h +++ b/src/utils/ddb/ddb_parse.h @@ -23,7 +23,7 @@ struct program_args { bool pa_write_mode; bool pa_get_help; }; -#define DB_PATH_LEN 64 +#define DB_PATH_LEN 256 struct vos_file_parts { char vf_db_path[DB_PATH_LEN]; uuid_t vf_pool_uuid; diff --git a/src/utils/ddb/ddb_vos.c b/src/utils/ddb/ddb_vos.c index 22160ae0e8f..1eabb12539e 100644 --- a/src/utils/ddb/ddb_vos.c +++ b/src/utils/ddb/ddb_vos.c @@ -61,7 +61,7 @@ dv_pool_open(const char *path, const char *db_path, daos_handle_t *poh, uint32_t } int -dv_pool_destroy(const char *path) +dv_pool_destroy(const char *path, const char *db_path) { struct vos_file_parts path_parts = {0}; int rc, flags = 0; @@ -70,6 +70,11 @@ dv_pool_destroy(const char *path) if (!SUCCESS(rc)) return rc; + if (db_path != NULL && strnlen(db_path, PATH_MAX) != 0) { + memset(path_parts.vf_db_path, 0, sizeof(path_parts.vf_db_path)); + strncpy(path_parts.vf_db_path, db_path, sizeof(path_parts.vf_db_path) - 1); + } + rc = vos_self_init(path_parts.vf_db_path, true, path_parts.vf_target_idx); if (!SUCCESS(rc)) { D_ERROR("Failed to initialize VOS with path '%s': " DF_RC "\n", diff --git a/src/utils/ddb/ddb_vos.h b/src/utils/ddb/ddb_vos.h index 3303c643340..978a11306f0 100644 --- a/src/utils/ddb/ddb_vos.h +++ b/src/utils/ddb/ddb_vos.h @@ -55,7 +55,7 @@ int dv_pool_open(const char *path, const char *db_path, daos_handle_t *poh, uint32_t flags); int dv_pool_close(daos_handle_t poh); int -dv_pool_destroy(const char *path); +dv_pool_destroy(const char *path, const char *db_path); /* Update vos pool flags */ int diff --git a/src/utils/ddb/tests/ddb_test_driver.c b/src/utils/ddb/tests/ddb_test_driver.c index 07e0b0c8694..d66d15198d8 100644 --- a/src/utils/ddb/tests/ddb_test_driver.c +++ b/src/utils/ddb/tests/ddb_test_driver.c @@ -314,7 +314,7 @@ ddb_teardown_vos(void **state) } if (tctx->dvt_special_pool_destroy) { - rc = dv_pool_destroy(tctx->dvt_pmem_file); + rc = dv_pool_destroy(tctx->dvt_pmem_file, NULL); } else { vos_self_init("/mnt/daos", false, 0); assert_success(vos_pool_destroy(tctx->dvt_pmem_file, tctx->dvt_pool_uuid));