[PATCH 00/14] virsh: Completion improvements and checking tool

Peter Krempa posted 14 patches 2 years, 7 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/cover.1631811763.git.pkrempa@redhat.com
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(-)
[PATCH 00/14] virsh: Completion improvements and checking tool
Posted by Peter Krempa 2 years, 7 months ago
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

Re: [PATCH 00/14] virsh: Completion improvements and checking tool
Posted by Michal Prívozník 2 years, 7 months ago
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

Re: [PATCH 00/14] virsh: Completion improvements and checking tool
Posted by Daniel P. Berrangé 2 years, 7 months ago
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-l
 abels -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-va
 riable -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 :|

Re: [PATCH 00/14] virsh: Completion improvements and checking tool
Posted by Peter Krempa 2 years, 7 months ago
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 -Wod
 r -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.