3:13 a.m.
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
3:13 a.m.
New subject: [PATCH 1/4] virsh: Provide local path completer for screenshot --file
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(a)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
3:13 a.m.
New subject: [PATCH 2/4] virsh: Provide local path completer for vol-download --file
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(a)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
3:13 a.m.
New subject: [PATCH 3/4] vsh: Extend checks for aliased commands
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(a)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
3:13 a.m.
New subject: [PATCH 4/4] vsh: Ensure that bool --options don't have completer
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(a)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
11:41 a.m.
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(a)redhat.com>
Jano
1194
days inactive
1197
days old
5 comments
2 participants
participants (2)
-
Ján Tomko -
Michal Privoznik