[PATCH 0/4] virsh: Follow up improvements
These are the fixes I spotted when reviewing Peter's patchset earlier. Michal Prívozník (4): virsh: Provide local path completer for screenshot --file virsh: Provide local path completer for vol-download --file vsh: Extend checks for aliased commands vsh: Ensure that bool --options don't have completer tools/virsh-domain.c | 1 + tools/virsh-volume.c | 6 +----- tools/vsh.c | 30 +++++++++++++++++++++++++++++- 3 files changed, 31 insertions(+), 6 deletions(-) -- 2.32.0
The screenshot command takes optional --file argument which can point to an existing local path (in which case the file is overwritten). Set the argument's completer so that self-test doesn't report it as missing. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 70aa4167c2..f876f30cc5 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5520,6 +5520,7 @@ static const vshCmdOptDef opts_screenshot[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "file", .type = VSH_OT_STRING, + .completer = virshCompletePathLocalExisting, .help = N_("where to store the screenshot") }, {.name = "screen", -- 2.32.0
The vol-download command takes mandatory --file argument which points to a local (possibly non-existent) path. If the file exists then it's overwritten. Set the argument's completer so that self-test doesn't report it as missing. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-volume.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 103a9b9237..152f5b0dbe 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -779,11 +779,7 @@ static const vshCmdInfo info_vol_download[] = { static const vshCmdOptDef opts_vol_download[] = { VIRSH_COMMON_OPT_VOL_FULL, - {.name = "file", - .type = VSH_OT_DATA, - .flags = VSH_OFLAG_REQ, - .help = N_("file") - }, + VIRSH_COMMON_OPT_FILE(N_("file")), VIRSH_COMMON_OPT_POOL_OPTIONAL, {.name = "offset", .type = VSH_OT_INT, -- 2.32.0
If a command is an alias, then it can only have .name, .flags and .alias set and .flags should contain just VSH_CMD_FLAG_ALIAS. Check if that's the case in self-test. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/vsh.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tools/vsh.c b/tools/vsh.c index cf24586b25..3d5fef84f0 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -290,6 +290,26 @@ vshCmddefCheckInternals(vshControl *ctl, return -1; } + if (cmd->handler) { + vshError(ctl, _("command '%s' has handler set"), cmd->name); + return -1; + } + + if (cmd->opts) { + vshError(ctl, _("command '%s' has options set"), cmd->name); + return -1; + } + + if (cmd->info) { + vshError(ctl, _("command '%s' has info set"), cmd->name); + return -1; + } + + if (cmd->flags & ~VSH_CMD_FLAG_ALIAS) { + vshError(ctl, _("command '%s' has multiple flags set"), cmd->name); + return -1; + } + /* we don't need to continue as the real command will be checked separately */ return 0; } -- 2.32.0
Let's check whether a boolean --option doesn't have completer or completer_flags set. These options are just flags and don't accept any value, thus they can't have any completer. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/vsh.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tools/vsh.c b/tools/vsh.c index 3d5fef84f0..7343387842 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -337,8 +337,16 @@ vshCmddefCheckInternals(vshControl *ctl, virBufferStrcat(&complbuf, opt->name, ", ", NULL); switch (opt->type) { - case VSH_OT_STRING: case VSH_OT_BOOL: + if (opt->completer || opt->completer_flags) { + vshError(ctl, _("bool parameter '%s' of command '%s' has completer set"), + opt->name, cmd->name); + return -1; + } + + G_GNUC_FALLTHROUGH; + + case VSH_OT_STRING: if (opt->flags & VSH_OFLAG_REQ) { vshError(ctl, _("parameter '%s' of command '%s' misused VSH_OFLAG_REQ"), opt->name, cmd->name); -- 2.32.0
On a Friday in 2021, Michal Privoznik wrote:
These are the fixes I spotted when reviewing Peter's patchset earlier.
Michal Prívozník (4): virsh: Provide local path completer for screenshot --file virsh: Provide local path completer for vol-download --file vsh: Extend checks for aliased commands vsh: Ensure that bool --options don't have completer
tools/virsh-domain.c | 1 + tools/virsh-volume.c | 6 +----- tools/vsh.c | 30 +++++++++++++++++++++++++++++- 3 files changed, 31 insertions(+), 6 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Michal Privoznik