[PATCH 00/14] virsh: Completion improvements and checking tool
When inspecting whether https://gitlab.com/libvirt/libvirt/-/issues/9 is still valid I wrote a tool which outputs command options missing completers. Now that I had a bit of time with lot of interruptions which is ideal for going through such a thing I decided to clean up the tool and post it along with a few fixes and additions to completers. After this patchset the following completers are still missing: $ virsh self-test --completers-missing attach-disk: source, targetbus, driver, subdriver, cache, io, type, mode, sourcetype, source-protocol, source-host-transport, source-host-socket attach-interface: type, source, script, model blockcopy: dest change-media: source detach-interface: type domdisplay: type domxml-from-native: format domxml-to-native: format dump: file get-user-sshkeys: user metadata: uri, key numatune: mode, nodeset qemu-monitor-event: event restore: file save: file save-image-define: file save-image-dumpxml: file save-image-edit: file screenshot: file set-user-sshkeys: user set-user-password: user cpu-models: arch domcapabilities: virttype, emulatorbin, arch, machine hypervisor-cpu-baseline: virttype, emulator, arch, machine hypervisor-cpu-compare: virttype, emulator, arch, machine maxvcpus: type iface-bridge: bridge iface-unbridge: bridge net-update: command, section nodedev-detach: driver snapshot-create-as: memspec find-storage-pool-sources-as: type find-storage-pool-sources: type pool-create-as: source-path, source-dev, source-name, target, source-format, auth-type, secret-usage, secret-uuid, adapter-name, adapter-wwnn, adapter-wwpn, adapter-parent, adapter-parent-wwnn, adapter-parent-wwpn, adapter-parent-fabric-wwn, source-protocol-ver pool-define-as: source-path, source-dev, source-name, target, source-format, auth-type, secret-usage, secret-uuid, adapter-name, adapter-wwnn, adapter-wwpn, adapter-parent, adapter-parent-wwnn, adapter-parent-wwpn, adapter-parent-fabric-wwn, source-protocol-ver vol-create-as: format, backing-vol, backing-vol-format vol-download: file vol-wipe: algorithm cd: dir The list is not so long any more. Mostly missing completers are for remote file paths (passed to libvirt or the VM itself), few enums, guest-agent-based user list and a few which need to be assesed. Peter Krempa (14): virshCheckpointNameCompleter: Sanitize forward declaration use virsh-completer*.h: Use modern header style virsh: Remove hack using 'VSH_CMD_FLAG_ALIAS' to hide virsh commands vshCmddefCheckInternals: Sanitize command alias validation vsh: Introduce '--completers-missing' for 'self-test' command virsh-snapshot: Use 'virshSnapshotNameCompleter' for '--from' of 'snapshot-list' virsh: Use 'virshStoragePoolNameCompleter' for two options vsh: Add completer for '--command' of 'help' command virsh: Provide completers for options taking comma separated list of disk targets virsh: Expand VIRSH_COMMON_OPT_FILE for cases when it's not a local file used by virsh virsh: completer: Introduce dummy completer for local files virsh: Use 'virshCompletePathLocalExisting' for options reading local files virsh: Introduce virshCompleteEmpty and use it for places where we can't suggest anything virsh-completer: Provide completer for '--top' and '--base' for blockjobs tools/virsh-backup.c | 2 + tools/virsh-checkpoint.c | 3 + tools/virsh-completer-checkpoint.h | 7 +- tools/virsh-completer-domain.c | 136 +++++++++++++++++++++++ tools/virsh-completer-domain.h | 170 ++++++++++++++++++----------- tools/virsh-completer-host.h | 28 +++-- tools/virsh-completer-interface.h | 14 ++- tools/virsh-completer-network.h | 35 +++--- tools/virsh-completer-nodedev.h | 21 ++-- tools/virsh-completer-nwfilter.h | 14 ++- tools/virsh-completer-pool.h | 21 ++-- tools/virsh-completer-secret.h | 14 ++- tools/virsh-completer-snapshot.h | 7 +- tools/virsh-completer-volume.h | 14 ++- tools/virsh-completer.c | 35 ++++++ tools/virsh-completer.h | 18 ++- tools/virsh-domain.c | 84 +++++++++++++- tools/virsh-network.c | 1 + tools/virsh-pool.c | 9 ++ tools/virsh-secret.c | 2 + tools/virsh-snapshot.c | 4 + tools/virsh-volume.c | 12 +- tools/virsh.c | 4 +- tools/virsh.h | 2 + tools/virt-admin.c | 3 +- tools/vsh.c | 66 +++++++++-- tools/vsh.h | 10 +- 27 files changed, 568 insertions(+), 168 deletions(-) -- 2.31.1
Include the proper header instead of duplicating the declaration. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-completer.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 7edb8e2f7e..874813335d 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -20,6 +20,7 @@ #pragma once +#include "virsh-completer-checkpoint.h" #include "virsh-completer-domain.h" #include "virsh-completer-host.h" #include "virsh-completer-interface.h" @@ -33,7 +34,3 @@ char ** virshCommaStringListComplete(const char *input, const char **options); - -char ** virshCheckpointNameCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); -- 2.31.1
Prevent the need to edit the function declarations to put them into the header. There was even inconsistent use. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-completer-checkpoint.h | 7 +- tools/virsh-completer-domain.h | 155 +++++++++++++++++------------ tools/virsh-completer-host.h | 28 +++--- tools/virsh-completer-interface.h | 14 +-- tools/virsh-completer-network.h | 35 ++++--- tools/virsh-completer-nodedev.h | 21 ++-- tools/virsh-completer-nwfilter.h | 14 +-- tools/virsh-completer-pool.h | 21 ++-- tools/virsh-completer-secret.h | 14 +-- tools/virsh-completer-snapshot.h | 7 +- tools/virsh-completer-volume.h | 14 +-- tools/virsh-completer.h | 5 +- 12 files changed, 192 insertions(+), 143 deletions(-) diff --git a/tools/virsh-completer-checkpoint.h b/tools/virsh-completer-checkpoint.h index c536a3ffda..f76af14817 100644 --- a/tools/virsh-completer-checkpoint.h +++ b/tools/virsh-completer-checkpoint.h @@ -22,6 +22,7 @@ #include "vsh.h" -char ** virshCheckpointNameCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshCheckpointNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h index 39cedf0141..f23ec2735f 100644 --- a/tools/virsh-completer-domain.h +++ b/tools/virsh-completer-domain.h @@ -22,37 +22,44 @@ #include "vsh.h" -char ** virshDomainNameCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshDomainNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); enum { VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC = 1 << 0, /* Return just MACs */ }; -char ** virshDomainInterfaceCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshDomainInterfaceCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshDomainDiskTargetCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshDomainDiskTargetCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshDomainEventNameCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshDomainEventNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshDomainInterfaceStateCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshDomainInterfaceStateCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshDomainDeviceAliasCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshDomainDeviceAliasCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshDomainShutdownModeCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshDomainShutdownModeCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); char ** virshDomainInterfaceAddrSourceCompleter(vshControl *ctl, @@ -64,70 +71,86 @@ virshDomainInterfaceSourceModeCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); -char ** virshDomainHostnameSourceCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshDomainHostnameSourceCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshDomainPerfEnableCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshDomainPerfEnableCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshDomainPerfDisableCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshDomainPerfDisableCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshDomainUUIDCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshDomainUUIDCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshDomainIOThreadIdCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshDomainIOThreadIdCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshDomainVcpuCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshDomainVcpuCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + +char ** +virshDomainVcpulistCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + +char ** +virshDomainCpulistCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshDomainVcpulistCompleter(vshControl *ctl, +char ** +virshDomainVcpulistViaAgentCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); -char ** virshDomainCpulistCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshDomainConsoleCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshDomainVcpulistViaAgentCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshDomainSignalCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshDomainConsoleCompleter(vshControl *ctl, +char ** +virshDomainLifecycleCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + +char ** +virshDomainLifecycleActionCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); -char ** virshDomainSignalCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); - -char ** virshDomainLifecycleCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); - -char ** virshDomainLifecycleActionCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshCodesetNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshCodesetNameCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshKeycodeNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshKeycodeNameCompleter(vshControl *ctl, +char ** +virshDomainFSMountpointsCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); -char ** virshDomainFSMountpointsCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); - char ** virshDomainCoreDumpFormatCompleter(vshControl *ctl, const vshCmd *cmd, diff --git a/tools/virsh-completer-host.h b/tools/virsh-completer-host.h index 88106ec3db..e71ccff536 100644 --- a/tools/virsh-completer-host.h +++ b/tools/virsh-completer-host.h @@ -22,18 +22,22 @@ #include "vsh.h" -char ** virshAllocpagesPagesizeCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshAllocpagesPagesizeCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshCellnoCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshCellnoCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshNodeCpuCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshNodeCpuCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshNodeSuspendTargetCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshNodeSuspendTargetCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-completer-interface.h b/tools/virsh-completer-interface.h index 733f18cb88..3c2ad7d9cb 100644 --- a/tools/virsh-completer-interface.h +++ b/tools/virsh-completer-interface.h @@ -22,10 +22,12 @@ #include "vsh.h" -char ** virshInterfaceNameCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshInterfaceNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshInterfaceMacCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshInterfaceMacCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-completer-network.h b/tools/virsh-completer-network.h index 80df5c468e..ffcda68355 100644 --- a/tools/virsh-completer-network.h +++ b/tools/virsh-completer-network.h @@ -22,22 +22,27 @@ #include "vsh.h" -char ** virshNetworkNameCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshNetworkNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshNetworkEventNameCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshNetworkEventNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshNetworkPortUUIDCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshNetworkPortUUIDCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshNetworkUUIDCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshNetworkUUIDCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshNetworkDhcpMacCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshNetworkDhcpMacCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-completer-nodedev.h b/tools/virsh-completer-nodedev.h index 45c0b73c8c..743f7b2465 100644 --- a/tools/virsh-completer-nodedev.h +++ b/tools/virsh-completer-nodedev.h @@ -22,14 +22,17 @@ #include "vsh.h" -char ** virshNodeDeviceNameCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshNodeDeviceNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshNodeDeviceEventNameCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshNodeDeviceEventNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshNodeDeviceCapabilityNameCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshNodeDeviceCapabilityNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-completer-nwfilter.h b/tools/virsh-completer-nwfilter.h index 86e7df7da4..6a5cda8880 100644 --- a/tools/virsh-completer-nwfilter.h +++ b/tools/virsh-completer-nwfilter.h @@ -22,10 +22,12 @@ #include "vsh.h" -char ** virshNWFilterNameCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshNWFilterNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshNWFilterBindingNameCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshNWFilterBindingNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-completer-pool.h b/tools/virsh-completer-pool.h index 510233fb65..059b6ba9e0 100644 --- a/tools/virsh-completer-pool.h +++ b/tools/virsh-completer-pool.h @@ -22,14 +22,17 @@ #include "vsh.h" -char ** virshStoragePoolNameCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshStoragePoolNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshPoolEventNameCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshPoolEventNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshPoolTypeCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshPoolTypeCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-completer-secret.h b/tools/virsh-completer-secret.h index 3ed6ba5198..0b1843605c 100644 --- a/tools/virsh-completer-secret.h +++ b/tools/virsh-completer-secret.h @@ -22,10 +22,12 @@ #include "vsh.h" -char ** virshSecretUUIDCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshSecretUUIDCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshSecretEventNameCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshSecretEventNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-completer-snapshot.h b/tools/virsh-completer-snapshot.h index 1af32e28ca..c7f389685c 100644 --- a/tools/virsh-completer-snapshot.h +++ b/tools/virsh-completer-snapshot.h @@ -22,6 +22,7 @@ #include "vsh.h" -char ** virshSnapshotNameCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshSnapshotNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-completer-volume.h b/tools/virsh-completer-volume.h index b41d8f4f3e..dc6eeb4b3c 100644 --- a/tools/virsh-completer-volume.h +++ b/tools/virsh-completer-volume.h @@ -23,11 +23,13 @@ #include "vsh.h" -char ** virshStorageVolNameCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshStorageVolNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); -char ** virshStorageVolKeyCompleter(vshControl *ctl, - const vshCmd *cmd, - unsigned int flags); +char ** +virshStorageVolKeyCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 874813335d..1243a13a5e 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -32,5 +32,6 @@ #include "virsh-completer-snapshot.h" #include "virsh-completer-volume.h" -char ** virshCommaStringListComplete(const char *input, - const char **options); +char ** +virshCommaStringListComplete(const char *input, + const char **options); -- 2.31.1
Introduce a proper flag 'VSH_CMD_FLAG_HIDDEN' for hiding commands from output so that we can validate that there aren't any loops or misconfigured commands. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh.c | 3 ++- tools/virt-admin.c | 3 ++- tools/vsh.c | 9 ++++++--- tools/vsh.h | 7 +++---- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index ce5fd247dd..3c6f60f176 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -464,7 +464,8 @@ virshUsage(void) fprintf(stdout, _(" %s (help keyword '%s')\n"), grp->name, grp->keyword); for (cmd = grp->commands; cmd->name; cmd++) { - if (cmd->flags & VSH_CMD_FLAG_ALIAS) + if (cmd->flags & VSH_CMD_FLAG_ALIAS || + cmd->flags & VSH_CMD_FLAG_HIDDEN) continue; fprintf(stdout, " %-30s %s\n", cmd->name, diff --git a/tools/virt-admin.c b/tools/virt-admin.c index c8e7ee794a..c0818e850a 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -1235,7 +1235,8 @@ vshAdmUsage(void) fprintf(stdout, _(" %s (help keyword '%s')\n"), grp->name, grp->keyword); for (cmd = grp->commands; cmd->name; cmd++) { - if (cmd->flags & VSH_CMD_FLAG_ALIAS) + if (cmd->flags & VSH_CMD_FLAG_ALIAS || + cmd->flags & VSH_CMD_FLAG_HIDDEN) continue; fprintf(stdout, " %-30s %s\n", cmd->name, diff --git a/tools/vsh.c b/tools/vsh.c index ea078c707b..eb17a58dc0 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -577,7 +577,8 @@ vshCmdGrpHelp(vshControl *ctl, const vshCmdGrp *grp) grp->keyword); for (cmd = grp->commands; cmd->name; cmd++) { - if (cmd->flags & VSH_CMD_FLAG_ALIAS) + if (cmd->flags & VSH_CMD_FLAG_ALIAS || + cmd->flags & VSH_CMD_FLAG_HIDDEN) continue; vshPrint(ctl, " %-30s %s\n", cmd->name, _(vshCmddefGetInfo(cmd, "help"))); @@ -2544,7 +2545,8 @@ vshReadlineCommandGenerator(void) for (cmd_list_index = 0; cmds[cmd_list_index].name; cmd_list_index++) { const char *name = cmds[cmd_list_index].name; - if (cmds[cmd_list_index].flags & VSH_CMD_FLAG_ALIAS) + if (cmds[cmd_list_index].flags & VSH_CMD_FLAG_ALIAS || + cmds[cmd_list_index].flags & VSH_CMD_FLAG_HIDDEN) continue; VIR_REALLOC_N(ret, ret_size + 2); @@ -3029,7 +3031,8 @@ cmdHelp(vshControl *ctl, const vshCmd *cmd) grp->keyword); for (def = grp->commands; def->name; def++) { - if (def->flags & VSH_CMD_FLAG_ALIAS) + if (def->flags & VSH_CMD_FLAG_ALIAS || + def->flags & VSH_CMD_FLAG_HIDDEN) continue; vshPrint(ctl, " %-30s %s\n", def->name, _(vshCmddefGetInfo(def, "help"))); diff --git a/tools/vsh.h b/tools/vsh.h index 39f70913fe..ae50f6b220 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -163,6 +163,7 @@ struct _vshCmdOpt { enum { VSH_CMD_FLAG_NOCONNECT = (1 << 0), /* no prior connection needed */ VSH_CMD_FLAG_ALIAS = (1 << 1), /* command is an alias */ + VSH_CMD_FLAG_HIDDEN = (1 << 2), /* command is hidden/internal */ }; /* @@ -446,8 +447,7 @@ bool cmdComplete(vshControl *ctl, const vshCmd *cmd); .handler = cmdSelfTest, \ .opts = NULL, \ .info = info_selftest, \ - .flags = VSH_CMD_FLAG_NOCONNECT | VSH_CMD_FLAG_ALIAS, \ - .alias = "self-test" \ + .flags = VSH_CMD_FLAG_NOCONNECT | VSH_CMD_FLAG_HIDDEN, \ } #define VSH_CMD_COMPLETE \ @@ -456,8 +456,7 @@ bool cmdComplete(vshControl *ctl, const vshCmd *cmd); .handler = cmdComplete, \ .opts = opts_complete, \ .info = info_complete, \ - .flags = VSH_CMD_FLAG_NOCONNECT | VSH_CMD_FLAG_ALIAS, \ - .alias = "complete" \ + .flags = VSH_CMD_FLAG_NOCONNECT | VSH_CMD_FLAG_HIDDEN, \ } -- 2.31.1
We don't need to validate the real command twice, but it's better to check that the real command name exists and it's not an alias to prevent loops. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/vsh.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tools/vsh.c b/tools/vsh.c index eb17a58dc0..05da50eace 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -269,11 +269,27 @@ vshCmddefCheckInternals(vshControl *ctl, /* in order to perform the validation resolve the alias first */ if (cmd->flags & VSH_CMD_FLAG_ALIAS) { + const vshCmdDef *alias; + if (!cmd->alias) { vshError(ctl, _("command '%s' has inconsistent alias"), cmd->name); return -1; } - cmd = vshCmddefSearch(cmd->alias); + + if (!(alias = vshCmddefSearch(cmd->alias))) { + vshError(ctl, _("command alias '%s' is pointing to a non-existant command '%s'"), + cmd->name, cmd->alias); + return -1; + } + + if (alias->flags & VSH_CMD_FLAG_ALIAS) { + vshError(ctl, _("command alias '%s' is pointing to another command alias '%s'"), + cmd->name, cmd->alias); + return -1; + } + + /* we don't need to continue as the real command will be checked separately */ + return 0; } /* Each command has to provide a non-empty help string. */ -- 2.31.1
Make it simple to spot which options of which commands are missing autocompletion functions by introducing this hidden option. In the future when we'll have completers for everything this can be also used as a hard fail so that completers are always added. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/vsh.c | 28 ++++++++++++++++++++++++---- tools/vsh.h | 3 ++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 05da50eace..9c6783dad1 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -261,11 +261,13 @@ vshCmddefGetInfo(const vshCmdDef * cmd, const char *name) /* Check if the internal command definitions are correct */ static int vshCmddefCheckInternals(vshControl *ctl, - const vshCmdDef *cmd) + const vshCmdDef *cmd, + bool missingCompleters) { size_t i; const char *help = NULL; bool seenOptionalOption = false; + g_auto(virBuffer) complbuf = VIR_BUFFER_INITIALIZER; /* in order to perform the validation resolve the alias first */ if (cmd->flags & VSH_CMD_FLAG_ALIAS) { @@ -309,6 +311,11 @@ vshCmddefCheckInternals(vshControl *ctl, return -1; /* too many options */ } + if (missingCompleters && + (opt->type == VSH_OT_STRING || opt->type == VSH_OT_DATA) && + !opt->completer) + virBufferStrcat(&complbuf, opt->name, ", ", NULL); + switch (opt->type) { case VSH_OT_STRING: case VSH_OT_BOOL: @@ -391,6 +398,12 @@ vshCmddefCheckInternals(vshControl *ctl, break; } } + + virBufferTrim(&complbuf, ", "); + + if (missingCompleters && virBufferUse(&complbuf) > 0) + vshPrintExtra(ctl, "%s: %s\n", cmd->name, virBufferCurrentContent(&complbuf)); + return 0; } @@ -3250,6 +3263,13 @@ cmdQuit(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) * Command self-test * ----------------- */ +const vshCmdOptDef opts_selftest[] = { + {.name = "completers-missing", + .type = VSH_OT_BOOL, + .help = N_("output the list of options which are missing completers") + }, + {.name = NULL} +}; const vshCmdInfo info_selftest[] = { {.name = "help", .data = N_("internal command for testing virt shells") @@ -3261,15 +3281,15 @@ const vshCmdInfo info_selftest[] = { }; bool -cmdSelfTest(vshControl *ctl, - const vshCmd *cmd G_GNUC_UNUSED) +cmdSelfTest(vshControl *ctl, const vshCmd *cmd) { const vshCmdGrp *grp; const vshCmdDef *def; + bool completers = vshCommandOptBool(cmd, "completers-missing"); for (grp = cmdGroups; grp->name; grp++) { for (def = grp->commands; def->name; def++) { - if (vshCmddefCheckInternals(ctl, def) < 0) + if (vshCmddefCheckInternals(ctl, def, completers) < 0) return false; } } diff --git a/tools/vsh.h b/tools/vsh.h index ae50f6b220..e208d957bb 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -375,6 +375,7 @@ extern const vshCmdOptDef opts_echo[]; extern const vshCmdInfo info_echo[]; extern const vshCmdInfo info_pwd[]; extern const vshCmdInfo info_quit[]; +extern const vshCmdOptDef opts_selftest[]; extern const vshCmdInfo info_selftest[]; extern const vshCmdOptDef opts_complete[]; extern const vshCmdInfo info_complete[]; @@ -445,7 +446,7 @@ bool cmdComplete(vshControl *ctl, const vshCmd *cmd); { \ .name = "self-test", \ .handler = cmdSelfTest, \ - .opts = NULL, \ + .opts = opts_selftest, \ .info = info_selftest, \ .flags = VSH_CMD_FLAG_NOCONNECT | VSH_CMD_FLAG_HIDDEN, \ } -- 2.31.1
On 9/16/21 7:10 PM, Peter Krempa wrote:
Make it simple to spot which options of which commands are missing autocompletion functions by introducing this hidden option.
In the future when we'll have completers for everything this can be also used as a hard fail so that completers are always added.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/vsh.c | 28 ++++++++++++++++++++++++---- tools/vsh.h | 3 ++- 2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c index 05da50eace..9c6783dad1 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -261,11 +261,13 @@ vshCmddefGetInfo(const vshCmdDef * cmd, const char *name) /* Check if the internal command definitions are correct */ static int vshCmddefCheckInternals(vshControl *ctl, - const vshCmdDef *cmd) + const vshCmdDef *cmd, + bool missingCompleters) { size_t i; const char *help = NULL; bool seenOptionalOption = false; + g_auto(virBuffer) complbuf = VIR_BUFFER_INITIALIZER;
/* in order to perform the validation resolve the alias first */ if (cmd->flags & VSH_CMD_FLAG_ALIAS) { @@ -309,6 +311,11 @@ vshCmddefCheckInternals(vshControl *ctl, return -1; /* too many options */ }
+ if (missingCompleters && + (opt->type == VSH_OT_STRING || opt->type == VSH_OT_DATA) &&
Technically, only VSH_OT_BOOL options can't have a completer because bool --options don't accept any argument. For everything else the completer callback is called (if defined), see vshReadlineParse(). But we can start small and expand later. There is plenty of say VSH_OT_ARGV/VSH_OT_INT arguments that can't have completer because their value don't depend on anything domain/network/nwfilter/... related. Mind you, some DO depend and have completers already: virshKeycodeNameCompleter() or virshDomainVcpuCompleter().
+ !opt->completer) + virBufferStrcat(&complbuf, opt->name, ", ", NULL); + switch (opt->type) { case VSH_OT_STRING: case VSH_OT_BOOL:
Michal
On Fri, Sep 17, 2021 at 09:31:13 +0200, Michal Prívozník wrote:
On 9/16/21 7:10 PM, Peter Krempa wrote:
Make it simple to spot which options of which commands are missing autocompletion functions by introducing this hidden option.
In the future when we'll have completers for everything this can be also used as a hard fail so that completers are always added.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/vsh.c | 28 ++++++++++++++++++++++++---- tools/vsh.h | 3 ++- 2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c index 05da50eace..9c6783dad1 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -261,11 +261,13 @@ vshCmddefGetInfo(const vshCmdDef * cmd, const char *name) /* Check if the internal command definitions are correct */ static int vshCmddefCheckInternals(vshControl *ctl, - const vshCmdDef *cmd) + const vshCmdDef *cmd, + bool missingCompleters) { size_t i; const char *help = NULL; bool seenOptionalOption = false; + g_auto(virBuffer) complbuf = VIR_BUFFER_INITIALIZER;
/* in order to perform the validation resolve the alias first */ if (cmd->flags & VSH_CMD_FLAG_ALIAS) { @@ -309,6 +311,11 @@ vshCmddefCheckInternals(vshControl *ctl, return -1; /* too many options */ }
+ if (missingCompleters && + (opt->type == VSH_OT_STRING || opt->type == VSH_OT_DATA) &&
Technically, only VSH_OT_BOOL options can't have a completer because bool --options don't accept any argument. For everything else the completer callback is called (if defined), see vshReadlineParse().
But we can start small and expand later. There is plenty of say VSH_OT_ARGV/VSH_OT_INT arguments that can't have completer because their value don't depend on anything domain/network/nwfilter/... related. Mind you, some DO depend and have completers already: virshKeycodeNameCompleter() or virshDomainVcpuCompleter().
Hmm, yeah I kind of forgot about VSH_OT_ARGV. I've specifically omitted VSH_OT_INT as in most cases it's just taking an arbitrary number form a range not tied to an object but to a user configuration. I'll perhaps do another round later.
On 9/17/21 9:36 AM, Peter Krempa wrote:
On Fri, Sep 17, 2021 at 09:31:13 +0200, Michal Prívozník wrote:
On 9/16/21 7:10 PM, Peter Krempa wrote:
Make it simple to spot which options of which commands are missing autocompletion functions by introducing this hidden option.
In the future when we'll have completers for everything this can be also used as a hard fail so that completers are always added.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/vsh.c | 28 ++++++++++++++++++++++++---- tools/vsh.h | 3 ++- 2 files changed, 26 insertions(+), 5 deletions(-)
I'll perhaps do another round later.
I've already ACKed the whole series. It's okay to do it as a follow up patch. In fact I've came across couple of places that could use our attention and I'll send patches as soon as you merge yours. Michal
When listing a snapshot tree, the '--from' option takes a name of a snapshot to limit the subset. Use virshSnapshotNameCompleter as completer for the option. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-snapshot.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index e08ecb6910..60a68b334b 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -1411,6 +1411,7 @@ static const vshCmdOptDef opts_snapshot_list[] = { }, {.name = "from", .type = VSH_OT_STRING, + .completer = virshSnapshotNameCompleter, .help = N_("limit list to children of given snapshot") }, VIRSH_COMMON_OPT_CURRENT(N_("limit list to children of current snapshot")), -- 2.31.1
'--pool' of the 'pool-event' command and '--inputpool' of 'vol-create-from' use the above mentioned completer. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-pool.c | 1 + tools/virsh-volume.c | 1 + 2 files changed, 2 insertions(+) diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 1a2ab8cc53..f1dfe892e1 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -1988,6 +1988,7 @@ static const vshCmdInfo info_pool_event[] = { static const vshCmdOptDef opts_pool_event[] = { {.name = "pool", .type = VSH_OT_STRING, + .completer = virshStoragePoolNameCompleter, .help = N_("filter by storage pool name or uuid") }, {.name = "event", diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index c51dc023e3..197ed2489c 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -454,6 +454,7 @@ static const vshCmdOptDef opts_vol_create_from[] = { VIRSH_COMMON_OPT_VOL_FULL, {.name = "inputpool", .type = VSH_OT_STRING, + .completer = virshStoragePoolNameCompleter, .help = N_("pool name or uuid of the input volume's pool") }, {.name = "prealloc-metadata", -- 2.31.1
Wrap 'vshReadlineCommandGenerator' into a function with proper prototype to provide a completer for the help command. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/vsh.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tools/vsh.c b/tools/vsh.c index 9c6783dad1..cf24586b25 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -3026,9 +3026,20 @@ vshDeinit(vshControl *ctl) * Generic commands available to use by any client * ----------------------------------------------- */ + +static char ** +vshCompleteHelpCommand(vshControl *ctl G_GNUC_UNUSED, + const vshCmd *cmd G_GNUC_UNUSED, + unsigned int completerflags G_GNUC_UNUSED) +{ + return vshReadlineCommandGenerator(); +} + + const vshCmdOptDef opts_help[] = { {.name = "command", .type = VSH_OT_STRING, + .completer = vshCompleteHelpCommand, .help = N_("Prints global help, command specific help, or help for a group of related commands") }, {.name = NULL} -- 2.31.1
'--storage' of the 'undefine' command and '--migrate-disks' of the 'migrate' command take a list of disk targets as an argument. We can simply combine 'virshDomainDiskTargetCompleter' with 'virshCommaStringListComplete' to provide the completions. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-completer-domain.c | 36 ++++++++++++++++++++++++++++++++++ tools/virsh-completer-domain.h | 10 ++++++++++ tools/virsh-domain.c | 2 ++ 3 files changed, 48 insertions(+) diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c index 3ef6c82388..34985f6281 100644 --- a/tools/virsh-completer-domain.c +++ b/tools/virsh-completer-domain.c @@ -220,6 +220,42 @@ virshDomainDiskTargetCompleter(vshControl *ctl, } +static char ** +virshDomainDiskTargetListCompleter(vshControl *ctl, + const vshCmd *cmd, + const char *argname) +{ + const char *curval = NULL; + g_auto(GStrv) targets = virshDomainDiskTargetCompleter(ctl, cmd, 0); + + if (vshCommandOptStringQuiet(ctl, cmd, argname, &curval) < 0) + return NULL; + + if (!targets) + return NULL; + + return virshCommaStringListComplete(curval, (const char **) targets); +} + + +char ** +virshDomainMigrateDisksCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int completeflags G_GNUC_UNUSED) +{ + return virshDomainDiskTargetListCompleter(ctl, cmd, "migrate-disks"); +} + + +char ** +virshDomainUndefineStorageDisksCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int completeflags G_GNUC_UNUSED) +{ + return virshDomainDiskTargetListCompleter(ctl, cmd, "storage"); +} + + char ** virshDomainEventNameCompleter(vshControl *ctl G_GNUC_UNUSED, const vshCmd *cmd G_GNUC_UNUSED, diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h index f23ec2735f..1ed3f94094 100644 --- a/tools/virsh-completer-domain.h +++ b/tools/virsh-completer-domain.h @@ -166,3 +166,13 @@ char ** virshDomainStorageFileFormatCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); + +char ** +virshDomainMigrateDisksCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int completeflags); + +char ** +virshDomainUndefineStorageDisksCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int completeflags); diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 4d328d2174..05fa5c07f6 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -3552,6 +3552,7 @@ static const vshCmdOptDef opts_undefine[] = { }, {.name = "storage", .type = VSH_OT_STRING, + .completer = virshDomainUndefineStorageDisksCompleter, .help = N_("remove associated storage volumes (comma separated list of " "targets or source paths) (see domblklist)") }, @@ -10371,6 +10372,7 @@ static const vshCmdOptDef opts_migrate[] = { }, {.name = "migrate-disks", .type = VSH_OT_STRING, + .completer = virshDomainMigrateDisksCompleter, .help = N_("comma separated list of disks to be migrated") }, {.name = "disks-port", -- 2.31.1
In cases such as the APIs for managed save management, the file path provided via the '--file' option is passed to the API. We'll need to make them distinct from cases for when virsh is using the file so that different completers can be used. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-domain.c | 36 ++++++++++++++++++++++++++++++------ tools/virsh-volume.c | 6 +++++- tools/virsh.h | 1 + 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 05fa5c07f6..f45ab5b9d1 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4129,7 +4129,11 @@ static const vshCmdInfo info_save[] = { static const vshCmdOptDef opts_save[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), - VIRSH_COMMON_OPT_FILE(N_("where to save the data")), + {.name = "file", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("where to save the data") + }, {.name = "bypass-cache", .type = VSH_OT_BOOL, .help = N_("avoid file system cache when saving") @@ -4474,7 +4478,11 @@ static const vshCmdInfo info_save_image_dumpxml[] = { }; static const vshCmdOptDef opts_save_image_dumpxml[] = { - VIRSH_COMMON_OPT_FILE(N_("saved state file to read")), + {.name = "file", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("saved state file to read") + }, {.name = "security-info", .type = VSH_OT_BOOL, .help = N_("include security sensitive information in XML dump") @@ -4518,7 +4526,11 @@ static const vshCmdInfo info_save_image_define[] = { }; static const vshCmdOptDef opts_save_image_define[] = { - VIRSH_COMMON_OPT_FILE(N_("saved state file to modify")), + {.name = "file", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("saved state file to modify") + }, {.name = "xml", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -4581,7 +4593,11 @@ static const vshCmdInfo info_save_image_edit[] = { }; static const vshCmdOptDef opts_save_image_edit[] = { - VIRSH_COMMON_OPT_FILE(N_("saved state file to edit")), + {.name = "file", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("saved state file to edit") + }, {.name = "running", .type = VSH_OT_BOOL, .help = N_("set domain to be running on restore") @@ -5221,7 +5237,11 @@ static const vshCmdInfo info_restore[] = { }; static const vshCmdOptDef opts_restore[] = { - VIRSH_COMMON_OPT_FILE(N_("the state to restore")), + {.name = "file", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("the state to restore") + }, {.name = "bypass-cache", .type = VSH_OT_BOOL, .help = N_("avoid file system cache when restoring") @@ -5293,7 +5313,11 @@ static const vshCmdInfo info_dump[] = { static const vshCmdOptDef opts_dump[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), - VIRSH_COMMON_OPT_FILE(N_("where to dump the core")), + {.name = "file", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("where to dump the core") + }, VIRSH_COMMON_OPT_LIVE(N_("perform a live core dump if supported")), {.name = "crash", .type = VSH_OT_BOOL, diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 197ed2489c..b73837f4c8 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -775,7 +775,11 @@ static const vshCmdInfo info_vol_download[] = { static const vshCmdOptDef opts_vol_download[] = { VIRSH_COMMON_OPT_VOL_FULL, - VIRSH_COMMON_OPT_FILE(N_("file")), + {.name = "file", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("file") + }, VIRSH_COMMON_OPT_POOL_OPTIONAL, {.name = "offset", .type = VSH_OT_INT, diff --git a/tools/virsh.h b/tools/virsh.h index 4d777545ff..8e1b8ced90 100644 --- a/tools/virsh.h +++ b/tools/virsh.h @@ -95,6 +95,7 @@ .help = _helpstr \ } +/* Use this only for files which are existing and used locally by virsh */ #define VIRSH_COMMON_OPT_FILE(_helpstr) \ {.name = "file", \ .type = VSH_OT_DATA, \ -- 2.31.1
On 9/16/21 7:10 PM, Peter Krempa wrote:
In cases such as the APIs for managed save management, the file path provided via the '--file' option is passed to the API.
We'll need to make them distinct from cases for when virsh is using the file so that different completers can be used.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-domain.c | 36 ++++++++++++++++++++++++++++++------ tools/virsh-volume.c | 6 +++++- tools/virsh.h | 1 + 3 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 05fa5c07f6..f45ab5b9d1 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4129,7 +4129,11 @@ static const vshCmdInfo info_save[] = {
static const vshCmdOptDef opts_save[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), - VIRSH_COMMON_OPT_FILE(N_("where to save the data")), + {.name = "file", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("where to save the data") + }, {.name = "bypass-cache", .type = VSH_OT_BOOL, .help = N_("avoid file system cache when saving")
Maybe have new macro VIRSH_COMMON_OPT_FILE_REMOTE? If we ever come with a completer for remote paths we have just one place to put .completer = XXX? Michal
On Fri, Sep 17, 2021 at 09:31:11 +0200, Michal Prívozník wrote:
On 9/16/21 7:10 PM, Peter Krempa wrote:
In cases such as the APIs for managed save management, the file path provided via the '--file' option is passed to the API.
We'll need to make them distinct from cases for when virsh is using the file so that different completers can be used.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-domain.c | 36 ++++++++++++++++++++++++++++++------ tools/virsh-volume.c | 6 +++++- tools/virsh.h | 1 + 3 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 05fa5c07f6..f45ab5b9d1 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4129,7 +4129,11 @@ static const vshCmdInfo info_save[] = {
static const vshCmdOptDef opts_save[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), - VIRSH_COMMON_OPT_FILE(N_("where to save the data")), + {.name = "file", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("where to save the data") + }, {.name = "bypass-cache", .type = VSH_OT_BOOL, .help = N_("avoid file system cache when saving")
Maybe have new macro VIRSH_COMMON_OPT_FILE_REMOTE? If we ever come with a completer for remote paths we have just one place to put .completer = XXX?
I'm not a fan of this kind of macros as they aren't very flexible and the number of lines saved is IMO not worth it in this case. On the other hand I've briefly thought about a dummy 'Remote' version of the file completer to annotate the rest of the file input fields even without implementing it, but that's a fairly trivial task once the completer is ready.
For now the completion does the correct thing of completing a local path if NULL is returned. Introduce 'virshCompletePathLocalExisting' and use it in the 'VIRSH_COMMON_OPT_FILE' macro. This for now serves as an annotation for the function which want to read a file on the host running virsh. In the future this can be used with a more sophisticated implementation. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-completer.c | 17 +++++++++++++++++ tools/virsh-completer.h | 5 +++++ tools/virsh.h | 1 + 3 files changed, 23 insertions(+) diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index 57e0c3905c..100f206598 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -128,3 +128,20 @@ virshCommaStringListComplete(const char *input, return g_steal_pointer(&ret); } + + +/** + * virshCompletePathLocalExisting: + * + * Complete a path to a existing file used as input. The file is used as input + * for virsh so only local files are considered. + * + * Note: For now this is a no-op. Readline does the correct thing. + */ +char ** +virshCompletePathLocalExisting(vshControl *ctl G_GNUC_UNUSED, + const vshCmd *cmd G_GNUC_UNUSED, + unsigned int completerflags G_GNUC_UNUSED) +{ + return NULL; +} diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 1243a13a5e..9a77aa117f 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -35,3 +35,8 @@ char ** virshCommaStringListComplete(const char *input, const char **options); + +char ** +virshCompletePathLocalExisting(vshControl *ctl, + const vshCmd *cmd, + unsigned int completerflags); diff --git a/tools/virsh.h b/tools/virsh.h index 8e1b8ced90..cacd54db57 100644 --- a/tools/virsh.h +++ b/tools/virsh.h @@ -100,6 +100,7 @@ {.name = "file", \ .type = VSH_OT_DATA, \ .flags = VSH_OFLAG_REQ, \ + .completer = virshCompletePathLocalExisting, \ .help = _helpstr \ } -- 2.31.1
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-backup.c | 2 ++ tools/virsh-checkpoint.c | 1 + tools/virsh-domain.c | 10 ++++++++++ tools/virsh-network.c | 1 + tools/virsh-pool.c | 1 + tools/virsh-secret.c | 1 + tools/virsh-snapshot.c | 1 + 7 files changed, 17 insertions(+) diff --git a/tools/virsh-backup.c b/tools/virsh-backup.c index 9125b7da97..7bac1923a6 100644 --- a/tools/virsh-backup.c +++ b/tools/virsh-backup.c @@ -39,10 +39,12 @@ static const vshCmdOptDef opts_backup_begin[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "backupxml", .type = VSH_OT_STRING, + .completer = virshCompletePathLocalExisting, .help = N_("domain backup XML"), }, {.name = "checkpointxml", .type = VSH_OT_STRING, + .completer = virshCompletePathLocalExisting, .help = N_("domain checkpoint XML"), }, {.name = "reuse-external", diff --git a/tools/virsh-checkpoint.c b/tools/virsh-checkpoint.c index 1f3a318014..78272b43c4 100644 --- a/tools/virsh-checkpoint.c +++ b/tools/virsh-checkpoint.c @@ -88,6 +88,7 @@ static const vshCmdOptDef opts_checkpoint_create[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "xmlfile", .type = VSH_OT_STRING, + .completer = virshCompletePathLocalExisting, .help = N_("domain checkpoint XML") }, {.name = "redefine", diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index f45ab5b9d1..25e50064bd 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2243,6 +2243,7 @@ static const vshCmdOptDef opts_blockcopy[] = { }, {.name = "xml", .type = VSH_OT_STRING, + .completer = virshCompletePathLocalExisting, .help = N_("filename containing XML description of the copy destination") }, {.name = "format", @@ -4140,6 +4141,7 @@ static const vshCmdOptDef opts_save[] = { }, {.name = "xml", .type = VSH_OT_STRING, + .completer = virshCompletePathLocalExisting, .help = N_("filename containing updated XML for the target") }, {.name = "running", @@ -4534,6 +4536,7 @@ static const vshCmdOptDef opts_save_image_define[] = { {.name = "xml", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshCompletePathLocalExisting, .help = N_("filename containing updated XML for the target") }, {.name = "running", @@ -4946,6 +4949,7 @@ static const vshCmdOptDef opts_managed_save_define[] = { {.name = "xml", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshCompletePathLocalExisting, .help = N_("filename containing updated XML for the target") }, {.name = "running", @@ -5248,6 +5252,7 @@ static const vshCmdOptDef opts_restore[] = { }, {.name = "xml", .type = VSH_OT_STRING, + .completer = virshCompletePathLocalExisting, .help = N_("filename containing updated XML for the target") }, {.name = "running", @@ -10007,6 +10012,7 @@ static const vshCmdOptDef opts_domxmlfromnative[] = { {.name = "config", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshCompletePathLocalExisting, .help = N_("config data file to import from") }, {.name = NULL} @@ -10062,6 +10068,7 @@ static const vshCmdOptDef opts_domxmltonative[] = { VIRSH_COMMON_OPT_DOMAIN_OT_STRING_FULL(VSH_OFLAG_REQ_OPT, 0), {.name = "xml", .type = VSH_OT_STRING, + .completer = virshCompletePathLocalExisting, .help = N_("xml data file to export from") }, {.name = NULL} @@ -10392,6 +10399,7 @@ static const vshCmdOptDef opts_migrate[] = { }, {.name = "xml", .type = VSH_OT_STRING, + .completer = virshCompletePathLocalExisting, .help = N_("filename containing updated XML for the target") }, {.name = "migrate-disks", @@ -10438,6 +10446,7 @@ static const vshCmdOptDef opts_migrate[] = { }, {.name = "persistent-xml", .type = VSH_OT_STRING, + .completer = virshCompletePathLocalExisting, .help = N_("filename containing updated persistent XML for the target") }, {.name = "tls", @@ -13973,6 +13982,7 @@ static const vshCmdOptDef opts_set_user_sshkeys[] = { }, {.name = "file", .type = VSH_OT_STRING, + .completer = virshCompletePathLocalExisting, .help = N_("optional file to read keys from"), }, {.name = "reset", diff --git a/tools/virsh-network.c b/tools/virsh-network.c index a8f7f46905..5c35310344 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -937,6 +937,7 @@ static const vshCmdOptDef opts_network_update[] = { {.name = "xml", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshCompletePathLocalExisting, .help = N_("name of file containing xml (or, if it starts with '<', the complete " "xml element itself) to add/modify, or to be matched for search") }, diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index f1dfe892e1..6ab0490b89 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -1509,6 +1509,7 @@ static const vshCmdOptDef opts_find_storage_pool_sources[] = { }, {.name = "srcSpec", .type = VSH_OT_STRING, + .completer = virshCompletePathLocalExisting, .help = N_("optional file of source xml to query for pools") }, {.name = NULL} diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c index 173a77fd90..d23cbf04bf 100644 --- a/tools/virsh-secret.c +++ b/tools/virsh-secret.c @@ -186,6 +186,7 @@ static const vshCmdOptDef opts_secret_set_value[] = { {.name = "file", .type = VSH_OT_STRING, .flags = VSH_OFLAG_REQ_OPT, + .completer = virshCompletePathLocalExisting, .help = N_("read secret from file"), }, {.name = "plain", diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 60a68b334b..5a3c468c53 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -115,6 +115,7 @@ static const vshCmdOptDef opts_snapshot_create[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "xmlfile", .type = VSH_OT_STRING, + .completer = virshCompletePathLocalExisting, .help = N_("domain snapshot XML") }, {.name = "redefine", -- 2.31.1
For now this serves just as an annotation because readline and also the bash completion script insist on completing local paths when an empty list is returned. This will serve for future reference once we'll be able to properly refuse to suggest anything. The completer is used for fields such as names for new objects, description strings, password strings etc, URIs and hostnames which we can't feasibly autocomplete. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-checkpoint.c | 2 ++ tools/virsh-completer.c | 18 ++++++++++++++++++ tools/virsh-completer.h | 5 +++++ tools/virsh-domain.c | 33 +++++++++++++++++++++++++++++++++ tools/virsh-pool.c | 7 +++++++ tools/virsh-secret.c | 1 + tools/virsh-snapshot.c | 2 ++ tools/virsh-volume.c | 5 +++++ tools/virsh.c | 1 + 9 files changed, 74 insertions(+) diff --git a/tools/virsh-checkpoint.c b/tools/virsh-checkpoint.c index 78272b43c4..8ad37ece69 100644 --- a/tools/virsh-checkpoint.c +++ b/tools/virsh-checkpoint.c @@ -200,10 +200,12 @@ static const vshCmdOptDef opts_checkpoint_create_as[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "name", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("name of checkpoint") }, {.name = "description", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("description of checkpoint") }, {.name = "print-xml", diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index 100f206598..3d77be3121 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -145,3 +145,21 @@ virshCompletePathLocalExisting(vshControl *ctl G_GNUC_UNUSED, { return NULL; } + + +/** + * virshCompleteEmpty: + * + * Complete nothing. For cases where an user input is required and we can't + * suggest anything. + * + * TODO: This is currently just an annotation, readline still completes local + * file list. + */ +char ** +virshCompleteEmpty(vshControl *ctl G_GNUC_UNUSED, + const vshCmd *cmd G_GNUC_UNUSED, + unsigned int completerflags G_GNUC_UNUSED) +{ + return NULL; +} diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 9a77aa117f..1d7affbb23 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -40,3 +40,8 @@ char ** virshCompletePathLocalExisting(vshControl *ctl, const vshCmd *cmd, unsigned int completerflags); + +char ** +virshCompleteEmpty(vshControl *ctl, + const vshCmd *cmd, + unsigned int completerflags); diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 25e50064bd..d2e604f5dd 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -395,6 +395,7 @@ static const vshCmdOptDef opts_attach_disk[] = { {.name = "target", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshCompleteEmpty, .help = N_("target of disk device") }, {.name = "targetbus", @@ -440,14 +441,17 @@ static const vshCmdOptDef opts_attach_disk[] = { }, {.name = "serial", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("serial of disk device") }, {.name = "wwn", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("wwn of disk device") }, {.name = "alias", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("custom alias name of disk device") }, {.name = "rawio", @@ -456,6 +460,7 @@ static const vshCmdOptDef opts_attach_disk[] = { }, {.name = "address", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("address of disk device") }, {.name = "multifunction", @@ -472,6 +477,7 @@ static const vshCmdOptDef opts_attach_disk[] = { }, {.name = "source-host-name", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("host name for source of disk device") }, {.name = "source-host-transport", @@ -793,10 +799,12 @@ static const vshCmdOptDef opts_attach_interface[] = { }, {.name = "target", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("target network name") }, {.name = "mac", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("MAC address") }, {.name = "script", @@ -809,14 +817,17 @@ static const vshCmdOptDef opts_attach_interface[] = { }, {.name = "alias", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("custom alias name of interface device") }, {.name = "inbound", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("control domain's incoming traffics") }, {.name = "outbound", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("control domain's outgoing traffics") }, VIRSH_COMMON_OPT_DOMAIN_PERSISTENT, @@ -1277,6 +1288,7 @@ static const vshCmdOptDef opts_blkdeviotune[] = { }, {.name = "group-name", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("group name to share I/O quota between multiple drives") }, {.name = "total_bytes_sec_max_length", @@ -1493,22 +1505,27 @@ static const vshCmdOptDef opts_blkiotune[] = { }, {.name = "device-weights", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("per-device IO Weights, in the form of /path/to/device,weight,...") }, {.name = "device-read-iops-sec", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("per-device read I/O limit per second, in the form of /path/to/device,read_iops_sec,...") }, {.name = "device-write-iops-sec", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("per-device write I/O limit per second, in the form of /path/to/device,write_iops_sec,...") }, {.name = "device-read-bytes-sec", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("per-device bytes read per second, in the form of /path/to/device,read_bytes_sec,...") }, {.name = "device-write-bytes-sec", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("per-device bytes wrote per second, in the form of /path/to/device,write_bytes_sec,...") }, VIRSH_COMMON_OPT_DOMAIN_CONFIG, @@ -3210,10 +3227,12 @@ static const vshCmdOptDef opts_domiftune[] = { }, {.name = "inbound", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("control domain's incoming traffics") }, {.name = "outbound", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("control domain's outgoing traffics") }, VIRSH_COMMON_OPT_DOMAIN_CONFIG, @@ -3991,6 +4010,7 @@ static const vshCmdOptDef opts_start[] = { }, {.name = "pass-fds", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("pass file descriptors N,M,... to the guest") }, {.name = NULL} @@ -5741,6 +5761,7 @@ static const vshCmdOptDef opts_set_user_password[] = { {.name = "password", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshCompleteEmpty, .help = N_("the new password") }, {.name = "encrypted", @@ -8100,6 +8121,7 @@ static const vshCmdOptDef opts_create[] = { }, {.name = "pass-fds", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("pass file descriptors N,M,... to the guest") }, {.name = "validate", @@ -8457,6 +8479,7 @@ static const vshCmdOptDef opts_metadata[] = { }, {.name = "set", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("new metadata to set"), }, {.name = "remove", @@ -8718,6 +8741,7 @@ static const vshCmdOptDef opts_send_process_signal[] = { {.name = "pid", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshCompleteEmpty, .help = N_("the process ID") }, {.name = "signame", @@ -9659,6 +9683,7 @@ static const vshCmdOptDef opts_qemu_attach[] = { {.name = "pid", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshCompleteEmpty, .help = N_("pid") }, {.name = NULL} @@ -10173,6 +10198,7 @@ static const vshCmdOptDef opts_domrename[] = { {.name = "new-name", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshCompleteEmpty, .help = N_("new domain name") }, {.name = NULL} @@ -10289,6 +10315,7 @@ static const vshCmdOptDef opts_migrate[] = { {.name = "desturi", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshCompleteEmpty, .help = N_("connection URI of the destination host as seen from the client(normal migration) or source(p2p migration)") }, VIRSH_COMMON_OPT_LIVE(N_("live migration")), @@ -10370,18 +10397,22 @@ static const vshCmdOptDef opts_migrate[] = { }, {.name = "migrateuri", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("migration URI, usually can be omitted") }, {.name = "graphicsuri", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("graphics URI to be used for seamless graphics migration") }, {.name = "listen-address", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("listen address that destination should bind to for incoming migration") }, {.name = "dname", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("rename to new name during migration (if supported)") }, {.name = "timeout", @@ -10413,6 +10444,7 @@ static const vshCmdOptDef opts_migrate[] = { }, {.name = "disks-uri", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("URI to use for disks migration (overrides --disks-port)") }, {.name = "comp-methods", @@ -10471,6 +10503,7 @@ static const vshCmdOptDef opts_migrate[] = { }, {.name = "tls-destination", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("override the destination host name used for TLS verification") }, {.name = NULL} diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 6ab0490b89..b9948ea622 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -59,6 +59,7 @@ {.name = "name", \ .type = VSH_OT_DATA, \ .flags = VSH_OFLAG_REQ, \ + .completer = virshCompleteEmpty, \ .help = N_("name of the pool") \ }, \ {.name = "type", \ @@ -73,6 +74,7 @@ }, \ {.name = "source-host", \ .type = VSH_OT_STRING, \ + .completer = virshCompleteEmpty, \ .help = N_("source-host for underlying storage") \ }, \ {.name = "source-path", \ @@ -101,6 +103,7 @@ }, \ {.name = "auth-username", \ .type = VSH_OT_STRING, \ + .completer = virshCompleteEmpty, \ .help = N_("auth username to be used for underlying storage") \ }, \ {.name = "secret-usage", \ @@ -145,6 +148,7 @@ }, \ {.name = "source-initiator", \ .type = VSH_OT_STRING, \ + .completer = virshCompleteEmpty, \ .help = N_("initiator iqn for underlying storage") \ } @@ -1425,14 +1429,17 @@ static const vshCmdOptDef opts_find_storage_pool_sources_as[] = { }, {.name = "host", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("optional host to query") }, {.name = "port", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("optional port to query") }, {.name = "initiator", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("optional initiator IQN to use for query") }, {.name = NULL} diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c index d23cbf04bf..dff2710928 100644 --- a/tools/virsh-secret.c +++ b/tools/virsh-secret.c @@ -199,6 +199,7 @@ static const vshCmdOptDef opts_secret_set_value[] = { }, {.name = "base64", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("base64-encoded secret value") }, {.name = NULL} diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 5a3c468c53..2659b4340d 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -324,10 +324,12 @@ static const vshCmdOptDef opts_snapshot_create_as[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "name", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("name of snapshot") }, {.name = "description", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("description of snapshot") }, {.name = "print-xml", diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index b73837f4c8..103a9b9237 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -185,15 +185,18 @@ static const vshCmdOptDef opts_vol_create_as[] = { {.name = "name", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshCompleteEmpty, .help = N_("name of the volume") }, {.name = "capacity", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshCompleteEmpty, .help = N_("size of the vol, as scaled integer (default bytes)") }, {.name = "allocation", .type = VSH_OT_STRING, + .completer = virshCompleteEmpty, .help = N_("initial allocation size, as scaled integer (default bytes)") }, {.name = "format", @@ -561,6 +564,7 @@ static const vshCmdOptDef opts_vol_clone[] = { {.name = "newname", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshCompleteEmpty, .help = N_("clone name") }, VIRSH_COMMON_OPT_POOL_OPTIONAL, @@ -1131,6 +1135,7 @@ static const vshCmdOptDef opts_vol_resize[] = { {.name = "capacity", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshCompleteEmpty, .help = N_("new capacity for the vol, as scaled integer (default bytes)") }, VIRSH_COMMON_OPT_POOL_OPTIONAL, diff --git a/tools/virsh.c b/tools/virsh.c index 3c6f60f176..b9f3f851d3 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -252,6 +252,7 @@ static const vshCmdOptDef opts_connect[] = { {.name = "name", .type = VSH_OT_STRING, .flags = VSH_OFLAG_EMPTY_OK, + .completer = virshCompleteEmpty, .help = N_("hypervisor connection URI") }, {.name = "readonly", -- 2.31.1
On a Thursday in 2021, Peter Krempa wrote:
For now this serves just as an annotation because readline and also the bash completion script insist on completing local paths when an empty list is returned.
This will serve for future reference once we'll be able to properly refuse to suggest anything.
The completer is used for fields such as names for new objects, description strings, password strings etc, URIs and hostnames which we can't feasibly autocomplete.
My bash autocompletes hostnames from ssh's known_hosts. But I agree that it might not be feasible. Passwods, on the other hand, are trivially completable: https://listman.redhat.com/archives/libvir-list/2019-April/msg00018.html Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-checkpoint.c | 2 ++ tools/virsh-completer.c | 18 ++++++++++++++++++ tools/virsh-completer.h | 5 +++++ tools/virsh-domain.c | 33 +++++++++++++++++++++++++++++++++ tools/virsh-pool.c | 7 +++++++ tools/virsh-secret.c | 1 + tools/virsh-snapshot.c | 2 ++ tools/virsh-volume.c | 5 +++++ tools/virsh.c | 1 + 9 files changed, 74 insertions(+)
Complete with the indexed targets (e.g. vda[3]) based on existing indexes. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-completer-domain.c | 100 +++++++++++++++++++++++++++++++++ tools/virsh-completer-domain.h | 5 ++ tools/virsh-domain.c | 3 + 3 files changed, 108 insertions(+) diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c index 34985f6281..93e992e9b1 100644 --- a/tools/virsh-completer-domain.c +++ b/tools/virsh-completer-domain.c @@ -32,6 +32,7 @@ #include "virperf.h" #include "virbitmap.h" #include "virkeycode.h" +#include "virglibutil.h" #include "virkeynametable_linux.h" #include "virkeynametable_osx.h" #include "virkeynametable_win32.h" @@ -256,6 +257,105 @@ virshDomainUndefineStorageDisksCompleter(vshControl *ctl, } +static GSList * +virshDomainBlockjobBaseTopCompleteDisk(const char *target, + xmlXPathContext *ctxt) +{ + g_autofree xmlNodePtr *indexlist = NULL; + int nindexlist = 0; + size_t i; + GSList *ret = NULL; + + if ((nindexlist = virXPathNodeSet("./source|./backingStore", + ctxt, &indexlist)) < 0) + return NULL; + + ret = g_slist_prepend(ret, g_strdup(target)); + + for (i = 0; i < nindexlist; i++) { + g_autofree char *idx = virXMLPropString(indexlist[i], "index"); + + if (!idx) + continue; + + ret = g_slist_prepend(ret, g_strdup_printf("%s[%s]", target, idx)); + } + + return ret; +} + + +char ** +virshDomainBlockjobBaseTopCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags) +{ + virshControl *priv = ctl->privData; + g_autoptr(xmlDoc) xmldoc = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; + g_autofree xmlNodePtr *disks = NULL; + int ndisks; + size_t i; + const char *path = NULL; + g_autoptr(virGSListString) list = NULL; + GSList *n; + GStrv ret = NULL; + size_t nelems; + + virCheckFlags(0, NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + if (virshDomainGetXML(ctl, cmd, 0, &xmldoc, &ctxt) < 0) + return NULL; + + ignore_value(vshCommandOptStringQuiet(ctl, cmd, "path", &path)); + + if ((ndisks = virXPathNodeSet("./devices/disk", ctxt, &disks)) <= 0) + return NULL; + + for (i = 0; i < ndisks; i++) { + g_autofree char *disktarget = NULL; + + ctxt->node = disks[i]; + disktarget = virXPathString("string(./target/@dev)", ctxt); + + if (STREQ_NULLABLE(path, disktarget)) + break; + } + + if (i == ndisks) + path = NULL; + + for (i = 0; i < ndisks; i++) { + g_autofree char *disktarget = NULL; + GSList *tmplist; + + ctxt->node = disks[i]; + + if (!(disktarget = virXPathString("string(./target/@dev)", ctxt))) + return NULL; + + if (path && STRNEQ(path, disktarget)) + continue; + + /* note that ctxt->node moved */ + if ((tmplist = virshDomainBlockjobBaseTopCompleteDisk(disktarget, ctxt))) + list = g_slist_concat(tmplist, list); + } + + list = g_slist_reverse(list); + nelems = g_slist_length(list); + ret = g_new0(char *, nelems + 1); + i = 0; + + for (n = list; n; n = n->next) + ret[i++] = g_strdup(n->data); + + return ret; +} + char ** virshDomainEventNameCompleter(vshControl *ctl G_GNUC_UNUSED, const vshCmd *cmd G_GNUC_UNUSED, diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h index 1ed3f94094..ec7909888e 100644 --- a/tools/virsh-completer-domain.h +++ b/tools/virsh-completer-domain.h @@ -176,3 +176,8 @@ char ** virshDomainUndefineStorageDisksCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int completeflags); + +char ** +virshDomainBlockjobBaseTopCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index d2e604f5dd..1d8af2d274 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1991,6 +1991,7 @@ static const vshCmdOptDef opts_blockcommit[] = { }, {.name = "base", .type = VSH_OT_STRING, + .completer = virshDomainBlockjobBaseTopCompleter, .help = N_("path of base file to commit into (default bottom of chain)") }, {.name = "shallow", @@ -1999,6 +2000,7 @@ static const vshCmdOptDef opts_blockcommit[] = { }, {.name = "top", .type = VSH_OT_STRING, + .completer = virshDomainBlockjobBaseTopCompleter, .help = N_("path of top file to commit from (default top of chain)") }, {.name = "active", @@ -2772,6 +2774,7 @@ static const vshCmdOptDef opts_blockpull[] = { }, {.name = "base", .type = VSH_OT_STRING, + .completer = virshDomainBlockjobBaseTopCompleter, .help = N_("path of backing file in chain for a partial pull") }, {.name = "wait", -- 2.31.1
On 9/16/21 7:10 PM, Peter Krempa wrote:
When inspecting whether https://gitlab.com/libvirt/libvirt/-/issues/9 is still valid I wrote a tool which outputs command options missing completers. Now that I had a bit of time with lot of interruptions which is ideal for going through such a thing I decided to clean up the tool and post it along with a few fixes and additions to completers.
After this patchset the following completers are still missing:
$ virsh self-test --completers-missing attach-disk: source, targetbus, driver, subdriver, cache, io, type, mode, sourcetype, source-protocol, source-host-transport, source-host-socket attach-interface: type, source, script, model blockcopy: dest change-media: source detach-interface: type domdisplay: type domxml-from-native: format domxml-to-native: format dump: file get-user-sshkeys: user metadata: uri, key numatune: mode, nodeset qemu-monitor-event: event restore: file save: file save-image-define: file save-image-dumpxml: file save-image-edit: file screenshot: file set-user-sshkeys: user set-user-password: user cpu-models: arch domcapabilities: virttype, emulatorbin, arch, machine hypervisor-cpu-baseline: virttype, emulator, arch, machine hypervisor-cpu-compare: virttype, emulator, arch, machine maxvcpus: type iface-bridge: bridge iface-unbridge: bridge net-update: command, section nodedev-detach: driver snapshot-create-as: memspec find-storage-pool-sources-as: type find-storage-pool-sources: type pool-create-as: source-path, source-dev, source-name, target, source-format, auth-type, secret-usage, secret-uuid, adapter-name, adapter-wwnn, adapter-wwpn, adapter-parent, adapter-parent-wwnn, adapter-parent-wwpn, adapter-parent-fabric-wwn, source-protocol-ver pool-define-as: source-path, source-dev, source-name, target, source-format, auth-type, secret-usage, secret-uuid, adapter-name, adapter-wwnn, adapter-wwpn, adapter-parent, adapter-parent-wwnn, adapter-parent-wwpn, adapter-parent-fabric-wwn, source-protocol-ver vol-create-as: format, backing-vol, backing-vol-format vol-download: file vol-wipe: algorithm cd: dir
The list is not so long any more. Mostly missing completers are for remote file paths (passed to libvirt or the VM itself), few enums, guest-agent-based user list and a few which need to be assesed.
Peter Krempa (14): virshCheckpointNameCompleter: Sanitize forward declaration use virsh-completer*.h: Use modern header style virsh: Remove hack using 'VSH_CMD_FLAG_ALIAS' to hide virsh commands vshCmddefCheckInternals: Sanitize command alias validation vsh: Introduce '--completers-missing' for 'self-test' command virsh-snapshot: Use 'virshSnapshotNameCompleter' for '--from' of 'snapshot-list' virsh: Use 'virshStoragePoolNameCompleter' for two options vsh: Add completer for '--command' of 'help' command virsh: Provide completers for options taking comma separated list of disk targets virsh: Expand VIRSH_COMMON_OPT_FILE for cases when it's not a local file used by virsh virsh: completer: Introduce dummy completer for local files virsh: Use 'virshCompletePathLocalExisting' for options reading local files virsh: Introduce virshCompleteEmpty and use it for places where we can't suggest anything virsh-completer: Provide completer for '--top' and '--base' for blockjobs
tools/virsh-backup.c | 2 + tools/virsh-checkpoint.c | 3 + tools/virsh-completer-checkpoint.h | 7 +- tools/virsh-completer-domain.c | 136 +++++++++++++++++++++++ tools/virsh-completer-domain.h | 170 ++++++++++++++++++----------- tools/virsh-completer-host.h | 28 +++-- tools/virsh-completer-interface.h | 14 ++- tools/virsh-completer-network.h | 35 +++--- tools/virsh-completer-nodedev.h | 21 ++-- tools/virsh-completer-nwfilter.h | 14 ++- tools/virsh-completer-pool.h | 21 ++-- tools/virsh-completer-secret.h | 14 ++- tools/virsh-completer-snapshot.h | 7 +- tools/virsh-completer-volume.h | 14 ++- tools/virsh-completer.c | 35 ++++++ tools/virsh-completer.h | 18 ++- tools/virsh-domain.c | 84 +++++++++++++- tools/virsh-network.c | 1 + tools/virsh-pool.c | 9 ++ tools/virsh-secret.c | 2 + tools/virsh-snapshot.c | 4 + tools/virsh-volume.c | 12 +- tools/virsh.c | 4 +- tools/virsh.h | 2 + tools/virt-admin.c | 3 +- tools/vsh.c | 66 +++++++++-- tools/vsh.h | 10 +- 27 files changed, 568 insertions(+), 168 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
On Thu, Sep 16, 2021 at 07:10:31PM +0200, Peter Krempa wrote:
When inspecting whether https://gitlab.com/libvirt/libvirt/-/issues/9 is still valid I wrote a tool which outputs command options missing completers. Now that I had a bit of time with lot of interruptions which is ideal for going through such a thing I decided to clean up the tool and post it along with a few fixes and additions to completers.
This series seems to have broken the build for most layered project CI pipelines FAILED: tools/libvirt_shell.a.p/vsh.c.o ccache cc -Itools/libvirt_shell.a.p -Itools -I../tools -Iinclude -I../include -Isrc -I../src -Isrc/util -I../src/util -I. -I.. -I/usr/include/libxml2 -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/gio-unix-2.0/ -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu99 -O2 -g -Werror -fasynchronous-unwind-tables -fexceptions -fipa-pure-const -fno-common -Waddress -Waggressive-loop-optimizations -Walloc-size-larger-than=9223372036854775807 -Walloca -Warray-bounds=2 -Wattributes -Wbool-compare -Wbool-operation -Wbuiltin-declaration-mismatch -Wbuiltin-macro-redefined -Wcast-align -Wcast-align=strict -Wno-cast-function-type -Wchar-subscripts -Wclobbered -Wcomment -Wcomments -Wcoverage-mismatch -Wcpp -Wdangling-else -Wdate-time -Wdeclaration-after-statement -Wdeprecated-declarations -Wdesignated-init -Wdiscarded-array-qualifiers -Wdiscarded-qualifiers -Wdiv-by-zero -Wduplicated-cond -Wduplicate-decl-specifier -Wempty-body -Wendif-labels -Wexpansion-to-defined -Wformat-contains-nul -Wformat-extra-args -Wno-format-nonliteral -Wformat-overflow=2 -Wformat-security -Wno-format-truncation -Wformat-y2k -Wformat-zero-length -Wframe-address -Wframe-larger-than=4096 -Wfree-nonheap-object -Whsa -Wif-not-aligned -Wignored-attributes -Wignored-qualifiers -Wimplicit -Wimplicit-fallthrough=5 -Wimplicit-function-declaration -Wimplicit-int -Wincompatible-pointer-types -Winit-self -Winline -Wint-conversion -Wint-in-bool-context -Wint-to-pointer-cast -Winvalid-memory-model -Winvalid-pch -Wjump-misses-init -Wlogical-not-parentheses -Wlogical-op -Wmain -Wmaybe-uninitialized -Wmemset-elt-size -Wmemset-transposed-args -Wmisleading-indentation -Wmissing-attributes -Wmissing-braces -Wmissing-declarations -Wmissing-field-initializers -Wmissing-include-dirs -Wmissing-parameter-type -Wmissing-prototypes -Wmultichar -Wmultistatement-macros -Wnarrowing -Wnested-externs -Wnonnull -Wnonnull-compare -Wnormalized=nfc -Wnull-dereference -Wodr -Wold-style-declaration -Wold-style-definition -Wopenmp-simd -Woverflow -Woverride-init -Wpacked-bitfield-compat -Wpacked-not-aligned -Wparentheses -Wpointer-arith -Wpointer-compare -Wpointer-sign -Wpointer-to-int-cast -Wpragmas -Wpsabi -Wrestrict -Wreturn-local-addr -Wreturn-type -Wscalar-storage-order -Wsequence-point -Wshadow -Wshift-count-negative -Wshift-count-overflow -Wshift-negative-value -Wshift-overflow=2 -Wno-sign-compare -Wsizeof-array-argument -Wsizeof-pointer-div -Wsizeof-pointer-memaccess -Wstrict-aliasing -Wstrict-prototypes -Wstringop-overflow=2 -Wstringop-truncation -Wsuggest-attribute=cold -Wno-suggest-attribute=const -Wsuggest-attribute=format -Wsuggest-attribute=noreturn -Wno-suggest-attribute=pure -Wsuggest-final-methods -Wsuggest-final-types -Wswitch -Wswitch-bool -Wswitch-enum -Wswitch-unreachable -Wsync-nand -Wtautological-compare -Wtrampolines -Wtrigraphs -Wtype-limits -Wuninitialized -Wunknown-pragmas -Wunused -Wunused-but-set-parameter -Wunused-but-set-variable -Wunused-const-variable=2 -Wunused-function -Wunused-label -Wunused-local-typedefs -Wunused-parameter -Wunused-result -Wunused-value -Wunused-variable -Wvarargs -Wvariadic-macros -Wvector-operation-performance -Wvla -Wvolatile-register-var -Wwrite-strings -fstack-protector-strong -Wdouble-promotion -fPIC -pthread -MD -MQ tools/libvirt_shell.a.p/vsh.c.o -MF tools/libvirt_shell.a.p/vsh.c.o.d -o tools/libvirt_shell.a.p/vsh.c.o -c ../tools/vsh.c ../tools/vsh.c: In function ‘vshCompleteHelpCommand’: ../tools/vsh.c:3035:12: error: implicit declaration of function ‘vshReadlineCommandGenerator’ [-Werror=implicit-function-declaration] return vshReadlineCommandGenerator(); ^~~~~~~~~~~~~~~~~~~~~~~~~~~ ../tools/vsh.c:3035:12: error: nested extern declaration of ‘vshReadlineCommandGenerator’ [-Werror=nested-externs] ../tools/vsh.c:3035:12: error: returning ‘int’ from a function with return type ‘char **’ makes pointer from integer without a cast [-Werror=int-conversion] return vshReadlineCommandGenerator(); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Fri, Sep 17, 2021 at 09:27:15 +0100, Daniel P. Berrangé wrote:
On Thu, Sep 16, 2021 at 07:10:31PM +0200, Peter Krempa wrote:
When inspecting whether https://gitlab.com/libvirt/libvirt/-/issues/9 is still valid I wrote a tool which outputs command options missing completers. Now that I had a bit of time with lot of interruptions which is ideal for going through such a thing I decided to clean up the tool and post it along with a few fixes and additions to completers.
This series seems to have broken the build for most layered project CI pipelines
Could you link to them?
FAILED: tools/libvirt_shell.a.p/vsh.c.o ccache cc -Itools/libvirt_shell.a.p -Itools -I../tools -Iinclude -I../include -Isrc -I../src -Isrc/util -I../src/util -I. -I.. -I/usr/include/libxml2 -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/gio-unix-2.0/ -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu99 -O2 -g -Werror -fasynchronous-unwind-tables -fexceptions -fipa-pure-const -fno-common -Waddress -Waggressive-loop-optimizations -Walloc-size-larger-than=9223372036854775807 -Walloca -Warray-bounds=2 -Wattributes -Wbool-compare -Wbool-operation -Wbuiltin-declaration-mismatch -Wbuiltin-macro-redefined -Wcast-align -Wcast-align=strict -Wno-cast-function-type -Wchar-subscripts -Wclobbered -Wcomment -Wcomments -Wcoverage-mismatch -Wcpp -Wdangling-else -Wdate-time -Wdeclaration-after-statement -Wdeprecated-declarations -Wdesignated-init -Wdiscarded-array-qualifiers -Wdiscarded-qualifiers -Wdiv-by-zero -Wduplicated-cond -Wduplicate-decl-specifier -Wempty-body -Wendif-labels -Wexpansion-to-defined -Wformat-contains-nul -Wformat-extra-args -Wno-format-nonliteral -Wformat-overflow=2 -Wformat-security -Wno-format-truncation -Wformat-y2k -Wformat-zero-length -Wframe-address -Wframe-larger-than=4096 -Wfree-nonheap-object -Whsa -Wif-not-aligned -Wignored-attributes -Wignored-qualifiers -Wimplicit -Wimplicit-fallthrough=5 -Wimplicit-function-declaration -Wimplicit-int -Wincompatible-pointer-types -Winit-self -Winline -Wint-conversion -Wint-in-bool-context -Wint-to-pointer-cast -Winvalid-memory-model -Winvalid-pch -Wjump-misses-init -Wlogical-not-parentheses -Wlogical-op -Wmain -Wmaybe-uninitialized -Wmemset-elt-size -Wmemset-transposed-args -Wmisleading-indentation -Wmissing-attributes -Wmissing-braces -Wmissing-declarations -Wmissing-field-initializers -Wmissing-include-dirs -Wmissing-parameter-type -Wmissing-prototypes -Wmultichar -Wmultistatement-macros -Wnarrowing -Wnested-externs -Wnonnull -Wnonnull-compare -Wnormalized=nfc -Wnull-dereference -Wodr -Wold-style-declaration -Wold-style-definition -Wopenmp-simd -Woverflow -Woverride-init -Wpacked-bitfield-compat -Wpacked-not-aligned -Wparentheses -Wpointer-arith -Wpointer-compare -Wpointer-sign -Wpointer-to-int-cast -Wpragmas -Wpsabi -Wrestrict -Wreturn-local-addr -Wreturn-type -Wscalar-storage-order -Wsequence-point -Wshadow -Wshift-count-negative -Wshift-count-overflow -Wshift-negative-value -Wshift-overflow=2 -Wno-sign-compare -Wsizeof-array-argument -Wsizeof-pointer-div -Wsizeof-pointer-memaccess -Wstrict-aliasing -Wstrict-prototypes -Wstringop-overflow=2 -Wstringop-truncation -Wsuggest-attribute=cold -Wno-suggest-attribute=const -Wsuggest-attribute=format -Wsuggest-attribute=noreturn -Wno-suggest-attribute=pure -Wsuggest-final-methods -Wsuggest-final-types -Wswitch -Wswitch-bool -Wswitch-enum -Wswitch-unreachable -Wsync-nand -Wtautological-compare -Wtrampolines -Wtrigraphs -Wtype-limits -Wuninitialized -Wunknown-pragmas -Wunused -Wunused-but-set-parameter -Wunused-but-set-variable -Wunused-const-variable=2 -Wunused-function -Wunused-label -Wunused-local-typedefs -Wunused-parameter -Wunused-result -Wunused-value -Wunused-variable -Wvarargs -Wvariadic-macros -Wvector-operation-performance -Wvla -Wvolatile-register-var -Wwrite-strings -fstack-protector-strong -Wdouble-promotion -fPIC -pthread -MD -MQ tools/libvirt_shell.a.p/vsh.c.o -MF tools/libvirt_shell.a.p/vsh.c.o.d -o tools/libvirt_shell.a.p/vsh.c.o -c ../tools/vsh.c ../tools/vsh.c: In function ‘vshCompleteHelpCommand’: ../tools/vsh.c:3035:12: error: implicit declaration of function ‘vshReadlineCommandGenerator’ [-Werror=implicit-function-declaration] return vshReadlineCommandGenerator(); ^~~~~~~~~~~~~~~~~~~~~~~~~~~ ../tools/vsh.c:3035:12: error: nested extern declaration of ‘vshReadlineCommandGenerator’ [-Werror=nested-externs] ../tools/vsh.c:3035:12: error: returning ‘int’ from a function with return type ‘char **’ makes pointer from integer without a cast [-Werror=int-conversion] return vshReadlineCommandGenerator(); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Looks like they build without 'readline'. All of libvirt's jobs use readline btw. I've fixed it in the meanwhile, but for a more stable CI layered products should use the same set of deps we use in our CI.
On Fri, Sep 17, 2021 at 11:19:46AM +0200, Peter Krempa wrote:
On Fri, Sep 17, 2021 at 09:27:15 +0100, Daniel P. Berrangé wrote:
On Thu, Sep 16, 2021 at 07:10:31PM +0200, Peter Krempa wrote:
When inspecting whether https://gitlab.com/libvirt/libvirt/-/issues/9 is still valid I wrote a tool which outputs command options missing completers. Now that I had a bit of time with lot of interruptions which is ideal for going through such a thing I decided to clean up the tool and post it along with a few fixes and additions to completers.
This series seems to have broken the build for most layered project CI pipelines
Could you link to them?
../tools/vsh.c: In function ‘vshCompleteHelpCommand’: ../tools/vsh.c:3035:12: error: implicit declaration of function ‘vshReadlineCommandGenerator’ [-Werror=implicit-function-declaration] return vshReadlineCommandGenerator(); ^~~~~~~~~~~~~~~~~~~~~~~~~~~ ../tools/vsh.c:3035:12: error: nested extern declaration of ‘vshReadlineCommandGenerator’ [-Werror=nested-externs] ../tools/vsh.c:3035:12: error: returning ‘int’ from a function with return type ‘char **’ makes pointer from integer without a cast [-Werror=int-conversion] return vshReadlineCommandGenerator(); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Looks like they build without 'readline'. All of libvirt's jobs use readline btw.
I've fixed it in the meanwhile, but for a more stable CI layered products should use the same set of deps we use in our CI.
They all use a minimal set of dependancies to cut down on the build time, as they ideally only want the libvirt.so library and nothing else. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (4)
-
Daniel P. Berrangé -
Ján Tomko -
Michal Prívozník -
Peter Krempa