Skip to content

DAOS-19440 ddb: Fix --db_path/--vos_path placement inconsistency #18862

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
knard38 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

DAOS-19440 ddb: Fix --db_path/--vos_path placement inconsistency #18862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 33 additions & 21 deletions src/control/cmd/ddb/ddb_commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,18 @@ func onlyOne(bools ...bool) bool {
return count == 1
}

// lifecycleCmdHelpNote is appended to pool-lifecycle commands' LongHelp;
// see lifecycleCommands in main.go for the full explanation. This text is
// embedded verbatim into the groff man page (see fprintManPage), so no
// line may start with a quote character: groff treats a leading "'" as a
// control character and silently drops the rest of that line.
const lifecycleCmdHelpNote = `

This command manages its own pool lifecycle: it does not accept the
top-level --vos_path/--db_path when given as a single bare command
(see 'ddb --help' or the POOL-CONTENT VS. POOL-LIFECYCLE COMMANDS
section of the man page for details).`

func addAppCommands(app *grumble.App, ctx *DdbContext) {
// Command: ls
app.AddCommand(&grumble.Command{
Expand Down Expand Up @@ -57,7 +69,7 @@ func addAppCommands(app *grumble.App, ctx *DdbContext) {
Help: "Opens the VOS file at <path>",
LongHelp: `Opens the VOS file at <path>. The '-w' option allows for modifying the VOS file
with the rm, load, commit_ilog, etc commands. The path <path> should be an absolute path to the
pool shard. Part of the path is used to determine what the pool uuid is.`,
pool shard. Part of the path is used to determine what the pool uuid is.` + lifecycleCmdHelpNote,
HelpGroup: "vos",
Flags: func(f *grumble.Flags) {
f.Bool("w", "write_mode", false, "Open the VOS file in write mode.")
Expand Down Expand Up @@ -88,7 +100,7 @@ pool shard. Part of the path is used to determine what the pool uuid is.`,
Name: "close",
Aliases: nil,
Help: "Close the currently opened VOS file",
LongHelp: "",
LongHelp: "Close the currently opened VOS file." + lifecycleCmdHelpNote,
HelpGroup: "vos",
Run: func(c *grumble.Context) error {
return ctx.Close()
Expand Down Expand Up @@ -245,16 +257,17 @@ the path must include the extent, otherwise, it must not.`,
// Command: smd_sync
app.AddCommand(&grumble.Command{
Name: "smd_sync",
Aliases: nil,
Help: "Restore the SMD file with backup from blob",
LongHelp: "",
LongHelp: "Restore the SMD file with backup from blob." + lifecycleCmdHelpNote,
HelpGroup: "smd",
Flags: func(f *grumble.Flags) {
f.String("p", "db_path", "", "Path to the sys db. (default /mnt/daos)")
},
Args: func(a *grumble.Args) {
a.String("nvme_conf", "Path to the nvme conf file. (default /mnt/daos/daos_nvme.conf)", grumble.Default(""))
a.String("db_path", "Path to the sys db. (default /mnt/daos)", grumble.Default(""))
},
Run: func(c *grumble.Context) error {
return ctx.SmdSync(c.Args.String("nvme_conf"), c.Args.String("db_path"))
return ctx.SmdSync(c.Args.String("nvme_conf"), c.Flags.String("db_path"))
},
Completer: nil,
})
Expand Down Expand Up @@ -324,7 +337,7 @@ the path must include the extent, otherwise, it must not.`,
Aliases: nil,
Help: "Manage VOS pool features",
LongHelp: `Manage VOS pool features. Exactly one of --enable, --disable, or --show must be provided.
If --db_path is provided, a VOS file path must also be given as a positional argument.`,
If --db_path is provided, a VOS file path must also be given as a positional argument.` + lifecycleCmdHelpNote,
HelpGroup: "vos",
Flags: func(f *grumble.Flags) {
f.String("e", "enable", "", "Enable VOS pool features")
Expand Down Expand Up @@ -356,7 +369,7 @@ If --db_path is provided, a VOS file path must also be given as a positional arg
Name: "rm_pool",
Aliases: nil,
Help: "Remove a VOS pool file.",
LongHelp: "",
LongHelp: "Remove a VOS pool file." + lifecycleCmdHelpNote,
HelpGroup: "vos",
Flags: func(f *grumble.Flags) {
f.String("p", "db_path", "", "Path to the sys db")
Expand Down Expand Up @@ -388,32 +401,32 @@ If --db_path is provided, a VOS file path must also be given as a positional arg
// Command: dev_list
app.AddCommand(&grumble.Command{
Name: "dev_list",
Aliases: nil,
Help: "List all devices",
LongHelp: "",
LongHelp: "List all devices." + lifecycleCmdHelpNote,
HelpGroup: "vos",
Args: func(a *grumble.Args) {
a.String("db_path", "Path to the sys db.")
Flags: func(f *grumble.Flags) {
f.String("p", "db_path", "", "Path to the sys db.")
},
Run: func(c *grumble.Context) error {
return ctx.DevList(c.Args.String("db_path"))
return ctx.DevList(c.Flags.String("db_path"))
},
Completer: nil,
})
// Command dev_replace
app.AddCommand(&grumble.Command{
Name: "dev_replace",
Aliases: nil,
Help: "Replace an old device with a new unused device",
LongHelp: "",
LongHelp: "Replace an old device with a new unused device." + lifecycleCmdHelpNote,
HelpGroup: "vos",
Flags: func(f *grumble.Flags) {
f.String("p", "db_path", "", "Path to the sys db.")
},
Args: func(a *grumble.Args) {
a.String("db_path", "Path to the sys db.")
a.String("old_dev", "Old device UUID.")
a.String("new_dev", "New device UUID.")
},
Run: func(c *grumble.Context) error {
return ctx.DevReplace(c.Args.String("db_path"), c.Args.String("old_dev"), c.Args.String("new_dev"))
return ctx.DevReplace(c.Flags.String("db_path"), c.Args.String("old_dev"), c.Args.String("new_dev"))
},
Completer: nil,
})
Expand All @@ -438,19 +451,18 @@ If --db_path is provided, a VOS file path must also be given as a positional arg
// Command prov_mem
app.AddCommand(&grumble.Command{
Name: "prov_mem",
Aliases: nil,
Help: "Prepare the memory environment for md-on-ssd mode",
LongHelp: "",
LongHelp: "Prepare the memory environment for md-on-ssd mode." + lifecycleCmdHelpNote,
HelpGroup: "vos",
Flags: func(f *grumble.Flags) {
f.String("p", "db_path", "", "Path to the sys db.")
f.Uint("s", "tmpfs_size", 0, "Specify tmpfs size(GiB) for mount. By default, The total size of all VOS files will be used")
},
Args: func(a *grumble.Args) {
a.String("db_path", "Path to the sys db.")
a.String("tmpfs_mount", "Path to the tmpfs mountpoint.")
},
Run: func(c *grumble.Context) error {
return ctx.ProvMem(c.Args.String("db_path"), c.Args.String("tmpfs_mount"), c.Flags.Uint("tmpfs_size"))
return ctx.ProvMem(c.Flags.String("db_path"), c.Args.String("tmpfs_mount"), c.Flags.Uint("tmpfs_size"))
},
Completer: nil,
})
Expand Down
105 changes: 104 additions & 1 deletion src/control/cmd/ddb/ddb_commands_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ func TestDdb_Cmds(t *testing.T) {
// --- prov_mem command: flag conflict ---
// -s / --tmpfs_size: short flag -s was consumed as global VosPath before PassAfterNonOption.
"prov_mem with tmpfs_size short flag": {
args: []string{"prov_mem", "-s", "10", "/db", "/mnt"},
args: []string{"prov_mem", "-s", "10", "-p", "/db", "/mnt"},
setup: func(t *testing.T) {
ddb_run_prov_mem_Fn = func(dbPath, tmpfsMount string, tmpfsMountSize uint) error {
fmt.Println("prov_mem called")
Expand All @@ -406,6 +406,109 @@ func TestDdb_Cmds(t *testing.T) {
},
expStdout: []string{"prov_mem called"},
},
"prov_mem with long db_path flag": {
args: []string{"prov_mem", "--db_path", "/db", "/mnt"},
setup: func(t *testing.T) {
ddb_run_prov_mem_Fn = func(dbPath, tmpfsMount string, tmpfsMountSize uint) error {
fmt.Println("prov_mem called")
test.CmpAny(t, "dbPath", "/db", dbPath)
test.CmpAny(t, "tmpfsMount", "/mnt", tmpfsMount)
test.CmpAny(t, "tmpfsMountSize", uint(0), tmpfsMountSize)
return nil
}
},
expStdout: []string{"prov_mem called"},
},

// --- dev_list command ---
"dev_list with short db_path flag": {
args: []string{"dev_list", "-p", "/db"},
setup: func(t *testing.T) {
ddb_run_dev_list_Fn = func(dbPath string) error {
fmt.Println("dev_list called")
test.CmpAny(t, "dbPath", "/db", dbPath)
return nil
}
},
expStdout: []string{"dev_list called"},
},
"dev_list with long db_path flag": {
args: []string{"dev_list", "--db_path", "/db"},
setup: func(t *testing.T) {
ddb_run_dev_list_Fn = func(dbPath string) error {
fmt.Println("dev_list called")
test.CmpAny(t, "dbPath", "/db", dbPath)
return nil
}
},
expStdout: []string{"dev_list called"},
},

// --- dev_replace command ---
"dev_replace with short db_path flag": {
args: []string{"dev_replace", "-p", "/db", "old-uuid", "new-uuid"},
setup: func(t *testing.T) {
ddb_run_dev_replace_Fn = func(dbPath, oldDev, newDev string) error {
fmt.Println("dev_replace called")
test.CmpAny(t, "dbPath", "/db", dbPath)
test.CmpAny(t, "oldDev", "old-uuid", oldDev)
test.CmpAny(t, "newDev", "new-uuid", newDev)
return nil
}
},
expStdout: []string{"dev_replace called"},
},
"dev_replace with long db_path flag": {
args: []string{"dev_replace", "--db_path", "/db", "old-uuid", "new-uuid"},
setup: func(t *testing.T) {
ddb_run_dev_replace_Fn = func(dbPath, oldDev, newDev string) error {
fmt.Println("dev_replace called")
test.CmpAny(t, "dbPath", "/db", dbPath)
test.CmpAny(t, "oldDev", "old-uuid", oldDev)
test.CmpAny(t, "newDev", "new-uuid", newDev)
return nil
}
},
expStdout: []string{"dev_replace called"},
},

// --- smd_sync command ---
"smd_sync with short db_path flag": {
args: []string{"smd_sync", "-p", "/db"},
setup: func(t *testing.T) {
ddb_run_smd_sync_Fn = func(nvmeConf, dbPath string) error {
fmt.Println("smd_sync called")
test.CmpAny(t, "nvmeConf", "", nvmeConf)
test.CmpAny(t, "dbPath", "/db", dbPath)
return nil
}
},
expStdout: []string{"smd_sync called"},
},
"smd_sync with long db_path flag": {
args: []string{"smd_sync", "--db_path", "/db"},
setup: func(t *testing.T) {
ddb_run_smd_sync_Fn = func(nvmeConf, dbPath string) error {
fmt.Println("smd_sync called")
test.CmpAny(t, "nvmeConf", "", nvmeConf)
test.CmpAny(t, "dbPath", "/db", dbPath)
return nil
}
},
expStdout: []string{"smd_sync called"},
},
"smd_sync with nvme_conf and db_path flag": {
args: []string{"smd_sync", "--db_path", "/db", "/nvme.conf"},
setup: func(t *testing.T) {
ddb_run_smd_sync_Fn = func(nvmeConf, dbPath string) error {
fmt.Println("smd_sync called")
test.CmpAny(t, "nvmeConf", "/nvme.conf", nvmeConf)
test.CmpAny(t, "dbPath", "/db", dbPath)
return nil
}
},
expStdout: []string{"smd_sync called"},
},

// --- csum_dump command ---
"csum_dump missing path": {
Expand Down
56 changes: 32 additions & 24 deletions src/control/cmd/ddb/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,22 @@ func exitWithError(err error) {
os.Exit(1)
}

// lifecycleCommands lists the pool-lifecycle commands (as opposed to
// pool-content commands like ls/rm) -- see ddbLongDescription for the full
// explanation of how --vos_path/--db_path apply to each category. Used to
// skip auto-open in run() and to reject top-level flags for a bare command
// in parseOpts() (see vosPathTopLevelLifecycleErr).
var lifecycleCommands = []string{
"feature",
"open",
"close",
"prov_mem",
"smd_sync",
"rm_pool",
"dev_list",
"dev_replace",
}

type cliOptions struct {
WriteMode bool `long:"write_mode" short:"w" description:"Open the VOS file in write mode."`
CmdFile string `long:"cmd_file" short:"f" description:"Path to a file containing a sequence of ddb commands to execute."`
Expand Down Expand Up @@ -92,11 +108,9 @@ shell mode. If neither a single command or '-f' option is provided, then
the tool will run in interactive mode. In order to modify the VOS file,
the '-w' option must be included.

If the command requires it, the VOS file must be provided with the parameter
--vos_path. The VOS file will be opened before any commands are executed,
except for commands that manage their own pool lifecycle (open, close, feature,
rm_pool, prov_mem, smd_sync, dev_list, dev_replace). See the command-specific
help for details.
ddb's subcommands fall into two categories -- pool-content and pool-lifecycle -- which differ
in how the top-level --vos_path/--db_path options apply to them. See the POOL-CONTENT VS.
POOL-LIFECYCLE COMMANDS section of the manpage for details.

A DAOS file system can operate in different modes depending on the available hardware resources.
The two primary modes are MD-on-SSD and PMEM. In MD-on-SSD mode (the default), metadata is stored
Expand All @@ -106,6 +120,7 @@ MODE section of the manpage for details.

const grumbleUnknownCmdErr = "unknown command, try 'help'"
const runCmdArgsErr = "Cannot use both command file and a command string"
const vosPathTopLevelLifecycleErr = "%q manages its own pool lifecycle and does not accept --vos_path/--db_path as a single bare command; provide its path directly to %q (see 'ddb %s --help'), or use --vos_path/--db_path only in interactive or -f command-file mode to pre-open a pool"
const loggerInitErr = "Logging facilities cannot be initialized"
const ctxInitErr = "DDB Context cannot be initialized"
const vosPathOpenErr = "Error opening VOS path '%s'"
Expand Down Expand Up @@ -309,6 +324,11 @@ func parseOpts(args []string, ctx *DdbContext) (cliOptions, *flags.Parser, error
return opts, nil, errors.New(vosPathMissErr)
}

// Reject --vos_path/--db_path for a bare pool-lifecycle command (see lifecycleCommands).
if opts.Args.RunCmd != "" && slices.Contains(lifecycleCommands, opts.Args.RunCmd) && opts.VosPath != "" {
return opts, nil, errors.Errorf(vosPathTopLevelLifecycleErr, opts.Args.RunCmd, opts.Args.RunCmd, opts.Args.RunCmd)
}

return opts, parser, nil
}

Expand All @@ -320,26 +340,14 @@ func run(ctx *DdbContext, log *logging.LeveledLogger, opts cliOptions, parser *f
defer cleanup()
app := createGrumbleApp(ctx)

if opts.VosPath != "" {
// Commands that manage the pool open/close lifecycle themselves and must
// not have the pool pre-opened by the CLI layer.
noAutoOpen := []string{
"feature",
"open",
"close",
"prov_mem",
"smd_sync",
"rm_pool",
"dev_list",
"dev_replace",
}
if !slices.Contains(noAutoOpen, opts.Args.RunCmd) {
log.Debugf("Connect to path: %s\n", opts.VosPath)
if err := ctx.Open(string(opts.VosPath), string(opts.SysdbPath), opts.WriteMode); err != nil {
return errors.Wrapf(err, vosPathOpenErr, opts.VosPath)
}
defer closePoolIfOpen(ctx, log)
// !slices.Contains(...) is always true today (parseOpts rejects this
// combo for a bare command); kept as a safe fallback if that ever changes.
if opts.VosPath != "" && !slices.Contains(lifecycleCommands, opts.Args.RunCmd) {
log.Debugf("Connect to path: %s\n", opts.VosPath)
if err := ctx.Open(string(opts.VosPath), string(opts.SysdbPath), opts.WriteMode); err != nil {
return errors.Wrapf(err, vosPathOpenErr, opts.VosPath)
}
defer closePoolIfOpen(ctx, log)
}

if opts.Args.RunCmd != "" || opts.CmdFile != "" {
Expand Down
Loading
Loading