1
0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-22 17:34:18 +03:00
Commit Graph

3132 Commits

Author SHA1 Message Date
Peter Krempa
d226a2cd70 vsh: Introduce annotation for vsh options which are unexpectedly parsed positionally
Based on the rationale in previous commit, all commands which were
parsed as positional but not documented as such will be annotated with
this flag.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-02 14:24:29 +02:00
Peter Krempa
348010ac93 vsh: Introduce tool to find unwanted positional arguments to 'self-test'
While the virsh option definitions specify (either explicitly after
recent refactors, or implicitly before) whether an argument is
positional or not, the actual parser is way more lax and actually and
allows also arguments which were considered/documented as non-positional
to be filled positionally unless VSH_OFLAG_REQ_OPT is used in the flags.

This creates situations such as 'snapshot-create-as' which has the
following docs:

SYNOPSIS
  snapshot-create-as <domain> [--name <string>] [--description <string>]
    [--print-xml] [--no-metadata] [--halt] [--disk-only]
    [--reuse-external] [--quiesce] [--atomic] [--live] [--validate]
    [--memspec <string>] [[--diskspec] <string>]...

Thus showing as if '--name' and '--description' required the option, but
in fact the following happens when only positionals are passed:

  $ virsh snapshot-create-as --print-xml 1 2 3 4 5
  <domainsnapshot>
    <name>2</name>
    <description>3</description>
    <disks>
      <disk name='4'/>
      <disk name='5'/>
    </disks>
  </domainsnapshot>

In the above example e.g. '--memspec' is not populated.

This disconnect makes it impossible to refactor the parser itself and
allows users to write buggy interactions with virsh.

In order to address this we'll be annotating every single of these
unwanted positional options as such so that this doesn't happen in the
future, while still preserving the quirk in the parser.

This patch introduces a tool which outputs list of options which are not
marked as positional but are lacking the VSH_OFLAG_REQ_OPT flag.

This tool will be removed once all the offenders found by it will be
addressed.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-02 14:24:29 +02:00
Peter Krempa
1f3f9c81da virsh: Inline VIRSH_COMMON_OPT_NETWORK_OT_STRING macro
The macro is used in just one place and the definition of the option is
going to be modified. Inline the macro.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-02 14:24:29 +02:00
Peter Krempa
d2e29eb74c virsh: Inline VIRSH_COMMON_OPT_FILE_FULL macro
The macro is used in one place only and the command definition will be
altered. Inline it.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-02 14:24:29 +02:00
Peter Krempa
7a696e9c3d virsh: Inline VIRSH_COMMON_OPT_DOMAIN_OT_STRING macro
Upcoming patches will need to tweak some of the properties of the
command. Since the macro is used in just two places expand it inline.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-02 14:24:29 +02:00
Peter Krempa
25be987715 vshCmddefCheckInternals: Improve some checks
- move the check that completer_flags are 0 if no completer is set
   into a common place and remove duplication
 - add check that _BOOL arguments are not positional
 - add missing checks to _ALIAS

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-02 14:24:29 +02:00
Peter Krempa
334510a687 vshCmddefHelp: Drop empty line at the end
All virsh commands in non-quiet mode append another separator line thus
having two is unnecessary and in quiet mode it still has a trailing
blank line. Remove it.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-02 14:24:29 +02:00
Peter Krempa
1a883c9a7c virsh: cmdDomdisplayReload: Require option name for --type
As this command was introduced in this release add the flag requiring to
pass optionname.

This is needed to actually disallow positional parsing of the value
despite documenting that the flag name is required.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-25 16:18:32 +01:00
Adam Julis
478774eada virt-admin: Fix segfault when libvirtd dies
vshAdmCatchDisconnect requires non-NULL structure vshControl for
getting connection name (stored at opaque), but
virAdmConnectRegisterCloseCallback at vshAdmConnect called it
with NULL.

Signed-off-by: Adam Julis <ajulis@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
2024-03-19 16:34:04 +01:00
Peter Krempa
78993f618f vsh: remove VSH_OFLAG_REQ
The flag was replaced by the 'required' field in the option definition.
Remove last few uses and all assignments.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:52 +01:00
Peter Krempa
cce3e049a9 vsh: Replace VSH_OT_DATA by VSH_OT_STRING
Use the new 'positional' field to do decisions rather than have a
special type for positional strings.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:52 +01:00
Peter Krempa
36132ff984 vshCmddefHelp: Refactor and fix printing of help for _STRING/_INT arguments
Use the new flags to do the decisions which will also fix the case when
an _INT option is required but non-positional.

This fixes the help for the 'timeout' argument of 'daemon-timeout'
virt-admin command:

     SYNOPSIS
  -    daemon-timeout <timeout>
  +    daemon-timeout --timeout <number>

[...]
     OPTIONS
  -    [--timeout] <number>  number of seconds the daemon will run without any active connection
  +    --timeout <number>  number of seconds the daemon will run without any active connection

Resolves: https://issues.redhat.com/browse/RHEL-25993
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:52 +01:00
Peter Krempa
e177b0fca6 vshCmdGrpHelp: Refactor formatting of help for VSH_OT_ARGV
Use the new properties rather than infer the states.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:52 +01:00
Peter Krempa
55a07252ec vshCmddefCheckInternals: Remove refactoring safety checks
Now that the code was refactored and proved identical, remove the checks
so that they don't impede further refactors.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:52 +01:00
Peter Krempa
a455220166 vsh: Require that positional non-argv arguments are required
This is logically enforced by existing checks, thus we can formalize it.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:52 +01:00
Peter Krempa
c53c064ef2 vsh: Fix broken assumption that required VSH_OT_INT must be positional
In at least one case we've wanted a mandatory argument which requires
the explicit flag. Fix the assumption before converting everything over
to the new flags.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:52 +01:00
Peter Krempa
ac150162fd vsh: Annotate 'required' and 'positional' arguments explicitly
Add 'positional' and 'required' fields to vshCmdOptDef, which will
explicitly track the two properties of arguments.

To ensure that we have proper coverage, add checks to
vshCmddefCheckInternals validating the state of the above flags by
infering it from existing data.

This conversion will allow us:
 - remove VSH_OT_DATA in favor of VSH_OT_STRING
 - use VSH_OT_INT when required both as positional and non-positional
 - properly annotate which VSH_OT_ARGV are positional and which are not
   (currently inferred by whether an previous positional option exists)

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:52 +01:00
Peter Krempa
162b184d0e virsh: Inline only use of VIRSH_COMMON_OPT_DOMAIN_OT_ARGV macro
There's just one command taking a list of domains as argument, thus
declare it inline.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:52 +01:00
Peter Krempa
34151214f3 vshCmddefGetOption: Improve readability
Declare one argument per line, separate disticnt conditions by newline,
move some checks earlier.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:52 +01:00
Peter Krempa
a191c5d455 vshCmddefHelp: Refactor printing of help (argument description)
Extract flag check to a separate variable and replace ternary operators
by normal conditions and use allocated buffer instead of a static one
to improve readability.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:52 +01:00
Peter Krempa
e879e63f5b vshCmddefHelp: Refactor printing of help (list of arguments)
Extract flag check to a separate variable and replace ternary operators
by normal conditions and directly output the text rather than using
extra variable to improve readability.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:52 +01:00
Peter Krempa
465091d2b8 vsh: Refactor store of command help and description
Store the pointers to 'help' and 'description' information in the struct
directly rather than in a key-value list.

The generic approach never got any extra use.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:52 +01:00
Peter Krempa
7df38644bc vsh: Add '--dump-help' option for 'self-test' command
The new option dumps the full help outputs for every command so that
it's possible to conveniently check that subsequent refactors will not
impact any of the external functionality.

No man page entry is needed as the command is internal/undocumented.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:52 +01:00
Peter Krempa
345f31bc4c virsh-domain: Don't explicitly break lines in help
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:52 +01:00
Peter Krempa
e09e643b71 virt-admin: Drop trailing whitespaces from description of some commands
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:52 +01:00
Peter Krempa
21f64f2acd virsh: Remove uncommon redundant descriptions of virsh commands
Some description of virsh commands referenced itself in a multi-line
example of usage, which is pointless as virsh help already shows how to
use the command:

     .data = N_("Get or set the current memory parameters for a guest"
                " domain.\n"
                "    To get the memory parameters use following command: \n\n"
                "    virsh # memtune <domain>")

Change it to just state what the command does and leave the example for
the help printer.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:52 +01:00
Peter Krempa
3b0b43b485 vshCmddefCheckInternals: Fix listing of missing completers for 'VSH_OT_ARGV'
Use a switch statement to cover all cases and check for missing
completers for arguments declared as VSH_OT_ARGV.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:51 +01:00
Peter Krempa
7abb44f5f7 vsh: Remove VSH_CMD_FLAG_ALIAS
It's obvious that a command is an alias when the 'alias' property is
set, thus an extra flag is redundant. Remove it.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:51 +01:00
Peter Krempa
3fcae7a028 vsh: Add VSH_OT_NONE option type to catch programming errors
Add a check that the default 0 assignment will not mean that an option
is considered to be VSH_OT_BOOL.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:51 +01:00
Peter Krempa
e1666a088b vsh: Don't translate error messages for 'self-test'
The command invoking the code is internal and meant for developers,
there's no point in translating the errors.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 15:02:51 +01:00
Peter Krempa
ea16531d36 vsh: Always assume that command groups are used
None of the clients use the 'command set' approach and other pieces of
code such as the command validator already assume that command groups
are in use. Remove the unused 'command set' stuff.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-03-13 14:53:01 +01:00
Ján Tomko
47a8f6a99b vsh: introduce vshEditString
Remove some code repetition between desc and net-desc commands.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-03-11 16:06:46 +01:00
Ján Tomko
3b31aeaae3 virsh: remove trailing whitespace even when editing the description
When editing the title of a domain or network via the `desc` or
`net-desc` commands, we strip the final newline that is added by some
editors.

Do the same when editing the description as well.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-03-11 16:06:46 +01:00
Zheng Yan
c9caf53d9b virsh: Introduce domdisplay-reload command
Introduce the domdisplay-reload command to make the domain reload
its graphics certificates
   #virsh domdisplay-reload <domain> --type <type>

Signed-off-by: Zheng Yan <yanzheng759@huawei.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-03-08 17:00:15 +01:00
Adam Julis
bdee774285 virsh: Fix overflow error of freepages command
Trying to print pages of a size larger than the UINT_MAX of the
given platform (for example, 4G on 64-bit ARM), results in a
system error even though this is a legitimate request.

The vshCommandOptScaledInt() used for parsing the pagesize is
given UINT_MAX as the upper limit. The parsed value is then
divided by 1024 and fed to virNodeGetFreePages() which expects an
unsigned int. We can't change the public API but the upper limit
can be raised by the factor of 1024.

Resolves: https://issues.redhat.com/browse/RHEL-23608

Signed-off-by: Adam Julis <ajulis@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-03-05 12:22:06 +01:00
Boris Fiuczynski
d36ce2f252 nodedev: allow modify on define of a persistent node device
Allow to modify a node device by using virNodeDeviceDefineXML() to align
its behavior with other drivers define methods.

Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-02-26 11:15:27 +01:00
Boris Fiuczynski
8cb1616226 virsh: add nodedev-update
Add ability to update node devices.

Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-02-26 11:13:32 +01:00
Boris Fiuczynski
4476751e53 tools: add switches persistent and transient to nodedev-list
Now that we can filter persistent and transient node devices in
virConnectListAllNodeDevices(), add these switches also to the
virsh nodedev-list command.

Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-02-26 11:02:44 +01:00
Boris Fiuczynski
6fcf613c6f tools: add option inactive to nodedev-dumpxml
Allow to dump the XML of the persistent mdev when the mdev has been
started instead of the current state only.

Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-02-26 11:00:04 +01:00
Adam Julis
7cb4a68500 virsh-completer: modify and fix bug in virshPoolTypeCompleter, now used for more commands
Signed-off-by: Adam Julis <ajulis@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-02-20 12:08:22 +01:00
Michal Privoznik
ce41108ecc virsh-domain: Fix return of virshGetDBusDisplay() in one error path
The virshGetDBusDisplay() function is declared to return a
pointer and yet, in one error path false is returned. Switch the
statement to return NULL, which is what other error paths use to
indicate an error.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-02-07 18:01:03 +01:00
Peter Krempa
442061583e virt-admin: Add warning when connection to default daemon fails
The admin connection defaults to the system-wide 'libvirtd' daemon to
manage (libvirtd:///system). As we've now switched to modular daemons
this will not work for most users out of the box:

 $ virt-admin version
 error: Failed to connect to the admin server
 error: no valid connection
 error: Failed to connect socket to '/run/user/1000/libvirt/libvirt-admin-sock': No such file or directory

As we don't want to assume which daemon the user wants to manage in the
modular topology there's no reasonable default to pick.

Give a hint to the users to use the '-c' if the connection to the
default URI fails:

 $ virt-admin version
 NOTE: Connecting to default daemon. Specify daemon using '-c' (e.g. virtqemud:///system)
 error: Failed to connect to the admin server
 error: no valid connection
 error: Failed to connect socket to '/run/user/1000/libvirt/libvirt-admin-sock': No such file or directory

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
2024-02-02 16:06:15 +01:00
Peter Krempa
9139cf44d4 vshAdmCatchDisconnect: Don't probe connection URI
virsh already stores the connection URI in 'ctl->connname', use that
instead.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-01-15 14:05:20 +01:00
Peter Krempa
6f956696cb vshAdmConnect: Preserve connection URI in ctl->connname if auto-detected
Probe the current URI so that other places don't need to do that.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-01-15 14:05:20 +01:00
Peter Krempa
4763f62a96 virshCatchDisconnect: Don't probe connection URI
virsh already stores the connection URI in 'ctl->connname', use that
instead.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-01-15 14:05:20 +01:00
Peter Krempa
06f6d76334 virshReconnect: Preserve current URI if not explicitly set in ctl->connname
Probe the current URI so that other places don't need to do that.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-01-15 14:05:20 +01:00
Yalei Li
9e555699a2 virsh: Command completion, add comp-methods with zlib and zstd
Signed-off-by: Yalei Li <liyl43@chinatelecom.cn>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-01-11 13:15:08 +01:00
Laine Stump
a435e7e6c8 conf: move/rename hostdev PCI driver type enum to device_conf.h
Currently this enum is defined in domain_conf.h and named
virDomainHostdevSubsysPCIDriverType. I want to use it in parts of the
network and networkport config, so am moving its definition to
device_conf.h which is / can be included by all interested parties,
and renaming it to match the name of the corresponding XML attribute
("driver name"). The name change (which includes enum values) does cause a
lot of churn, but it's all mechanical.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2024-01-07 23:57:09 -05:00
Peter Krempa
04adeac76e virDomainBlockResize: Introduce VIR_DOMAIN_BLOCK_RESIZE_CAPACITY
Allow users to easily resize 'raw' images on block devices to the full
capacity of the block device. Obviously this won't work on file-backed
storage (filling the remaining capacity is most likely wrong) or for
formats with metadata due to the overhead.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2023-12-14 16:09:05 +01:00
Peter Krempa
a7b357fd5c vsh: Introduce simple version of VSH_ALTERNATIVE_OPTIONS_EXPR
VSH_ALTERNATIVE_OPTIONS takes just the name of the options instead of
requiring also the getter functions.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2023-12-14 16:06:23 +01:00