Commit Graph

3190 Commits

Author SHA1 Message Date
Daniel P. Berrangé
6759137f72 meson: fix missing use of unitdir for systemd directory
This conversion was missed in the previous commit:

  commit a7eb7de531
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Thu Jun 6 12:57:08 2024 +0100

    meson: allow systemd unitdir to be changed

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Yaakov Selkowitz <yselkowi@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2024-06-13 10:23:07 +01:00
Daniel P. Berrangé
2b80329a6a tools: fix paths in PKI validation error messages
A couple of paths passed in the error messages, didnt match the paths
that were actually being tested.

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2024-06-13 10:22:21 +01:00
Daniel P. Berrangé
acb26f22a1 tools: support validating user/custom PKI certs
The virt-pki-validate command can validate the system certificate
directories. The remote driver, however, also supports a standard
per-user certs location, as well as a runtime custom path. This
extends the validation tool to be able to cope with these alternate
locations too.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2024-06-11 12:50:23 +01:00
Daniel P. Berrangé
8e97fd4181 tools: reimplement virt-pki-validate in C
The virt-pki-validate tool is currently a shell script. We have a
general goal of eliminating use of shell in the project. By doing a
new implementation in C, we can also make use of our more thorough
sanity checking code to validate the certificate setup.

This new implementation the same output format as the host validation
tool for a more consistent user experiance.

It also eliminates the requirement to have certtool installed on
libvirt hosts, which has been an issue for Fedora flatpak packages
since certtool isn't in the default platform runtime.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2024-06-11 12:50:23 +01:00
Daniel P. Berrangé
3faa78d98d tools: stop checking init scripts & iptables config
The /etc/sysconfig/libvirtd file is a Fedora/RHEL specific concept.
Since those distros switched to systemd socket activation, the
existance of --listen parameter in /etc/sysconfig/libvirtd is no
longer a reliable check. This was further degraded with the switch
to modular daemons where virtproxyd takes over the role.

The /etc/sysconfig/iptables file is a Fedora/RHEL specific concept.
Since those distros switched to firewalld, this file is no longer
a reliable check.

Rather than complicating these checks, just remove them, so that
the virt-pki-validate tool focuses exclusively on TLS configuration
validation.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2024-06-11 12:50:23 +01:00
Daniel P. Berrangé
1d7ce1c071 tools: drop unused --version argument
These tools never supported passing an argument to --version, this is
a copy+paste mistake from virsh, which did support an argument.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2024-06-11 12:50:23 +01:00
Daniel P. Berrangé
8ee395d843 tools: split off common helpers for host validate tool
The common messaging helpers will be reused in the new impl of the
virt-pki-validate tool.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2024-06-11 12:50:23 +01:00
Michal Privoznik
86e511fafb lib: Annotate more function as NULL terminated
While __attribute((sentinel)) (exposed by glib under
G_GNUC_NULL_TERMINATED macro) is a gcc extension, it's supported
by clang too. It's already being used throughout our code but
some functions that take variadic arguments and expect NULL at
the end were lacking such annotation. Fill them in.

After this, there are still some functions left untouched because
they expect a different sentinel than NULL. Unfortunately, glib
does not provide macro for different sentinels. We may come up
with our own, but let's save that for future work.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-06-06 09:29:58 +02:00
Michal Privoznik
0a2508ee77 vsh: Don't crash when @text is NULL in vshCompleterFilter()
This can happen only for cmdComplete() in interactive mode (which
I'm still not convinced is any useful for users and whether we
should support it). Anyway, running plain 'complete' command with
no additional arguments boils down to @text being NULL in
vshReadlineParse() which handles the case just right but is then
subsequently passed to vshCompleterFilter() which isn't prepared
for this case.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2024-05-28 08:52:31 +02:00
Michal Privoznik
f6ab0d5bdd vsh: Restore original rl_line_buffer after completion
Problem with readline is its API. It's basically a bunch of
global variables with no clear dependencies between them. In this
specific case that I'm seeing: in interactive mode the
cmdComplete() causes instant crash of virsh/virt-admin:

==27999== Invalid write of size 1
==27999==    at 0x516EF71: _rl_init_line_state (readline.c:742)
==27999==    by 0x5170054: rl_initialize (readline.c:1192)
==27999==    by 0x516E5E4: readline (readline.c:379)
==27999==    by 0x1B7024: vshReadline (vsh.c:3048)
==27999==    by 0x140DCF: main (virsh.c:905)
==27999==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

This is because readline keeps a copy of pointer to
rl_line_buffer and the moment cmdComplete() returns and readline
takes over, it accesses the copy which is now a dangling pointer.

To fix this, just keep the original state of rl_line_buffer and
restore it.

Fixes: 41400ac1dd
Fixes: a0e1ada63c
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2024-05-28 08:52:25 +02:00
Michal Privoznik
e4afe64825 vsh: Close stderr among with stdin in cmdComplete
Our completer callbacks must refrain from printing anything onto
stderr, but unfortunately that's not how service code around
behaves. It may call vshError() and what not. Rather trying to
fix all possible paths (just consider opening a connection), just
close the stderr. We're already closing stdin.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2024-05-28 08:52:06 +02:00
Michal Privoznik
6aa94ce105 bash-completion: Run virsh/virt-admin in quiet mode
In some cases (e.g. when virt-admin connects to the default URI)
some info message is printed onto stdout (using vshPrintExtra()).
This hurts user experience, just consider:

  virt-admin<TAB><TAB>
  NOTE\:\ Connecting\ to\ default\ daemon.\ Specify\ daemon\ using\ -c\ \(e.g.\ virtqemud\:///system\)

when no daemon is running. Suppress extra prints by passing '-q'
in the bash-completion script.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2024-05-28 08:51:45 +02:00
Michal Privoznik
a0c45432e0 virt-admin: Make --timeout of daemon-timeout positional argument
We currently require full argument specification:

  virt-admin daemon-timeout --timeout X

Well, the '--timeout' feels a bit redundant. Turn the argument
into a positional so that the following works too:

  virt-admin daemon-timeout X

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2024-05-28 08:51:37 +02:00
Peter Krempa
f63cbc7365 virGetGroupList: Refactor and fix callers
Use contemporary style for declarations and automatic memory clearing
for a helper string.

Since the function can't fail any more, remove any mention of returning
errno and remove error checks from all callers.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
2024-05-23 14:32:24 +02:00
Peter Krempa
cb78302a51 virt-host-validate: Improve translatability of messages printed by 'virHostMsgCheck()'
Move the word 'Checking' into the appropriate formatting strings and
mark all outstanding ones for translation.

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/637
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-05-23 14:31:22 +02:00
Peter Krempa
811ce0e13b vsh: Fix '--help' option for virsh/virt-admin
The refactor of the libvirt tools command parser introduced a bug where
the '--help' option would cause an error:

 $ virsh list --help
 error: command 'list' doesn't support option --help

rather than printing the help for the command as the help option is
supposed to be handled separately from the real options.

Re-introduce the separate handling to the new parser code.

Fixes: 5540c3d241
Resolves: https://issues.redhat.com/browse/RHEL-36565
Reported-by: Lili Zhu <lizhu@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2024-05-16 09:03:48 +02:00
Michal Privoznik
0287b5dfd2 tools: Introduce SSH proxy
This allows users to SSH into a domain with a VSOCK device:

  ssh user@qemu/machineName

So far, only QEMU domains are supported AND qemu:///system is
looked for the first for 'machineName' followed by
qemu:///session. I took an inspiration from Systemd's ssh proxy
[1] [2].

To just work out of the box, it requires (yet unreleased) systemd
to be running inside the guest to set up a socket activated SSHD
on the VSOCK. Alternatively, users can set up the socket
activation themselves, or just run a socat that'll forward vsock
<-> TCP communication.

1: https://github.com/systemd/systemd/blob/main/src/ssh-generator/ssh-proxy.c
2: https://github.com/systemd/systemd/blob/main/src/ssh-generator/20-systemd-ssh-proxy.conf.in

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/579
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-05-13 08:56:35 +02:00
Michal Privoznik
cab1e71f01 vsh: Don't init history in cmdComplete()
Recent rework of virshtest uncovered a subtle bug that was
dormant in now vsh but before that even in monolithic virsh.

In vsh.c there's this vshReadlineInit() function that's supposed
to initialize readline library, i.e. set those global rl_*
pointers.  But it also initializes history library. Then, when
virsh/virt-admin quits, vshReadlineDeinit() is called which
writes history into a file (ensuring the parent directory
exists). So far no problem.

Problem arises when cmdComplete() is called (from a bash
completer, for instance). It does not guard call to
vshReadlineInit() with check for interactive shell (and it should
not), but it sets ctl->historyfile which signals to
vshReadlineDeinit() the history should be written.

Now, no real history is written, because nothing was entered on
the stdin, but the parent directory is created nevertheless. With
recent movement in virshtest.c this means some test cases might
create virsh history file which breaks our promise of not
touching user's data in test suite.

Resolves: https://bugs.gentoo.org/931109
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-05-06 13:18:07 +02:00
Peter Krempa
5540c3d241 vsh: Refactor logic in vshCommandParse
Refactor the existing logic using two nested loops with a jump into the
middle of both with 3 separate places fetching next token to a single
loop using a state machine with one centralized place to fetch next
tokens and add explanation comments.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-04-25 14:13:20 +02:00
Peter Krempa
c27070f738 vsh: Move option assignment debugging from vshCommandParse to vshCmdOptAssign
As we now have a centralized point to assign values to options move the
debugging logic there.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-04-25 14:13:19 +02:00
Peter Krempa
166fe3c7b5 vshCmddefCheckInternals: Remove check for "too many options"
This check was needed due to the use "unsigned long long" as bitmap
which was refactored recently.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-04-25 14:13:19 +02:00
Peter Krempa
bf3e734fac vsh: Refactor parsed option and command assignment
Refactor the very old opaque logic (using multiple bitmaps) by
fully-allocating vshCmdOpt for each possible argument and then filling
them as they go rather than allocating them each time after it's parsed.

This simplifies the checkers and removes the need to cross-reference
multiple arrays.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-04-25 14:13:19 +02:00
Peter Krempa
22cf91618d vsh: Unexport command lookup helpers 'vshCmddefSearch', 'vshCmdGrpSearch', 'vshCmdGrpHelp'
Neither of them is used outside of vsh.c. 'vshCmddefSearch' needed to be
rearranged as it was called earlier in vsh.c than it was defined.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-04-25 14:13:19 +02:00
Peter Krempa
41efec103d vsh: Remove unused infrastructure for command completion
Remove the old helpers which were used previously to pick which field to
complete.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-04-25 14:13:19 +02:00
Peter Krempa
da3c5638f9 virsh: Introduce new 'VSH_OT_ARGV' accessors
In preparation for internal parser refactor introduce new accessors for
the VSH_OT_ARGV type which will return a NULL-terminated string list or
even a concatenated string for the given argument.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-04-25 14:13:19 +02:00
Peter Krempa
1818cbda3b vsh: Rework logic for picking which argument is to be completed
Currently the code decides which option to complete by looking into the
input string and trying to infer it based on whether we are at the
end position as we truncate the string to complete to the current cursor
position.

That basically means that only the last-parsed option will be up for
completion.

Replace the logic by remembering which is the last option rather than
using two different position checks and base the completion decision on
that and the actual value of the last argument (see comment).

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-04-25 14:13:19 +02:00
Peter Krempa
9950aef282 vsh: Add a VSH_OT_STRING argument for 'virsh echo'
The argument will be used for testing the command/option completer
function.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-04-25 14:13:19 +02:00
Peter Krempa
bb9bb55211 vsh: Fix 'stdin' closing in 'cmdComplete'
While the 'complete' command is meant to be hidden and used only for
the completion script, there's nothing preventing it being used in all
virsh modes.

This poses a problem as the command tries to close 'stdin' to avoid the
possibility that an auth callback would want to read the password.

In interactive mode this immediately terminates virsh and in
non-interactive mode it attempts to close it multiple times if you use
virsh in batch mode.

Fix the issues by using virOnce() to close it exactly once and do so
only in non-interactive mode.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-04-25 14:13:19 +02:00
Peter Krempa
0d687d13ed tools: Rename vshCommandOptStringReq to vshCommandOptString
Shorten the function name as there isn't any vshCommandOptString.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
2024-04-25 14:13:19 +02:00
Peter Krempa
dbf7b727fb vshReadlineInit: Initialize only once
'vshReadlineInit' is called when interactive virsh is started but also
on each call to 'cmdComplete'. Calling it repeatedly (using the
'complete' command interactively, or multiple times in batch mode) leaks
the buffers for history file configuration.

Avoid multiple setups of this function by returning success in case the
history file config is already present.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-25 14:13:19 +02:00
Peter Krempa
41400ac1dd vsh: cmdComplete: Don't leak buffer for completion
The buffer which we assign to the 'rl_line_buffer' variable of readline
would be overwritten and thus leaked on multiple invocations of
cmdComplete in one session.

Free/clear it after it's used.

Hitting this leak was until recenly possible only in non-interactive
batch mode and recently also in interactive mode as 'complete' can be
used multiple times now interactively.

Fixes: a0e1ada63c
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-25 14:13:19 +02:00
Michal Privoznik
753064963c libvirt_nss: Fix ERROR() macro
The purpose of ERROR() macro in our NSS module is to print error
message provided as arguments followed by error string
corresponding to errno. Historically, we've used strerror_r() for
that (please note, we want our NSS module to be free of libvirt
internal functions, or glib even - hence, g_strerror() is off the
table).

Now strerror_r() is documented as:

  Returns ... a pointer to a string that the function stores in
  buf, or a pointer to some (immutable) static string (in which
  case buf is unused).

Therefore, we can't rely the string being stored in the buf and
really need to store the retval and print that instead.

While touching this area, decrease the ebuf size, since its
current size (1KiB) is triggering our stack limit (2KiB) in some
cases.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2024-04-18 14:57:30 +02:00
Michal Privoznik
2e72cfe25d vsh: Drop fwd declaration of a nonexistent function
The vshFindTypedParamByName() function no longer exists (as of
v1.0.2-rc1~82), but its header file declaration was still kept
around. Drop it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2024-04-12 14:27:58 +02:00
Benjamin Taubmann
a39dd25715 Extend libvirt-guests to shutdown only persistent VMs
At the moment, there is no configuration option for the libvirt-guests
service that allows users to define that only persistent virtual machines
should be shutdown on host shutdown.

Currently, the service config allows to choose between two ON_SHUTDOWN
actions that are executed on running virtual machines when the host goes
down: shutdown, suspend.
The ON_SHUTDOWN action should be orthogonal to the type of the virtual
machine. However, the existing implementation, does not suspend
transient virtual machines.
This is the matrix of actions that is executed on virtual machines based
on the configured ON_SHUTDOWN action and the type of a virtual machine.

         | persistent | transient
shutdown | shutdown   | shutdown (what we want to change)
suspend  | suspend    | nothing

Add config option PERSISTENT_ONLY to libvirt-guests config that allows
users to define if the ON_SHUTDOWN action should be applied only on
persistent virtual machines. PERSISTENT_ONLY can be set to true, false,
default. The default option will implement the already existing logic.

Case 1: PERSISTENT_ONLY=default
         | persistent | transient
shutdown | shutdown   | shutdown
suspend  | suspend    | nothing

Case 2: PERSISTENT_ONLY=true
         | persistent | transient
shutdown | shutdown   | nothing
suspend  | suspend    | nothing

Case 3: PERSISTENT_ONLY=false
         | persistent | transient
shutdown | shutdown   | shutdown
suspend  | suspend    | suspend

Signed-off-by: Benjamin Taubmann <benjamin.taubmann@nutanix.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
2024-04-04 09:10:00 +02:00
Andrea Bolognani
16d37076be virsh: Show 'connected to console' message later
Right now, we display the message before actually attempting
to connect to the VM console. That operation, however, can
fail for a number of reasons: for example, is the VM doesn't
have a serial device, the output ends up looking like

  $ virsh console cirros
  Connected to domain 'cirros'
  Escape character is ^] (Ctrl + ])
  error: internal error: cannot find character device <null>

The initial message is misleading. Change things so that it's
only printed if we actually successfully connected to the VM
console.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
2024-04-02 18:26:35 +02:00
Peter Krempa
8f10dd32af tests: Re-implement '(virsh|virt-admin)-self-test' directly in meson
The self-test command for both virsh and virt-admin is self contained
and directly reports success, thus we don't actually need to run a shell
wrapper around it.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-02 14:24:30 +02:00
Peter Krempa
eb82c632e3 vsh: Allow non-interactive use of 'cd' command
For testing purposes it will come handy to change the directory from a
batch-mode script. Remove the check forbidding use of the 'cd' command
in batch mode.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-02 14:24:30 +02:00
Peter Krempa
6193c4656e virsh: Fix '--name' and '--parent' used together in '(snapshot|checkpoint)-list' command
Until now when '--name' was used the parent was not printed and the
option was ignored. One option would be to declare the options mutually
exclusive, but for testing it may come handy to print both the snapshot
name and parent. Adjust the code to print them tab-separated and adjust
the docs.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-02 14:24:30 +02:00
Peter Krempa
de9dfeee9a vshCmdOptDef: Remove unused 'flags' member
Drop the last enum member VSH_OFLAG_NONE and remove the 'flags' variable
from vshCmdOptDef.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-02 14:24:30 +02:00
Peter Krempa
4b44113d7b vsh: Replace 'VSH_OFLAG_EMPTY_OK' bitwise flag with a separate struct member
Replace the last bitwise flag with a separate member.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-02 14:24:30 +02:00
Peter Krempa
8e39542a03 vsh: Make positional parsing of arguments opt-in
Switch the command parser from using the VSH_OFLAG_REQ_OPT flag
opting out from positional parsing of arguments to a combination of the
'positional' flags for truly positional arguments and
'unwanted_positional' preserving semantics for the existing arguments
where the parser did it due to bad design.

This patch retires VSH_OFLAG_REQ_OPT along with the infrastructure that
was needed to refactor all uses properly.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-02 14:24:30 +02:00
Peter Krempa
a67f737ddf virt-admin: Annodate 'unwanted_positional' arguments
Historically the command parser in virsh parses/fills even optional
arguments with values as if they were positional unless opted out using
VSH_OFLAG_REQ_OPT. This creates unexpected situations when commands can
break in this unwanted semantics:

 $ virsh snapshot-create-as --print-xml 1 2 3
 <domainsnapshot>
   <name>2</name>
   <description>3</description>
 </domainsnapshot>

To prevent any further addition annotate the rest of the arguments with
the 'unwanted_positional' flag, so that the parser can keep parsing them
as such but any further optional argument will not have this behaviour.

Certain arguments where it makes sense are annotated as 'positional' too
in this patch.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-02 14:24:30 +02:00
Peter Krempa
100cbccecd virsh: Annodate 'unwanted_positional' arguments
Historically the command parser in virsh parses/fills even optional
arguments with values as if they were positional unless opted out using
VSH_OFLAG_REQ_OPT. This creates unexpected situations when commands can
break in this unwanted semantics:

 $ virsh snapshot-create-as --print-xml 1 2 3
 <domainsnapshot>
   <name>2</name>
   <description>3</description>
 </domainsnapshot>

To prevent any further addition annotate the rest of the arguments with
the 'unwanted_positional' flag, so that the parser can keep parsing them
as such but any further optional argument will not have this behaviour.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-02 14:24:30 +02:00
Peter Krempa
32a89fc670 virsh: Annotate "unwanted_positional" arguments for 'pool-(define|create)-as' commands
Annotate arguments which can be unintentionally parsed positionally.
(See previous commits for explanation.)

All of these options were added in order thus we must declare all of
them as 'unwanted_positional'.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-02 14:24:30 +02:00
Peter Krempa
e31cd46364 virsh: volume: Mark optional 'pool' argument as 'positional'
Annotate arguments which can be unintentionally parsed positionally.
(See previous commits for explanation.)

The pool name is optional but in all cases it can be promoted to an
optional positional argument so that it can be properly aligned with the
expectations of the parser.

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
1304ef40a9 virsh: Annotate some optional arguments as positional
Make certain optional arguments truly positional in cases when it makes
semantic sense.

Previously it wasn't possible to have optional positional arguments, but
the parser filled them regardless, thus this preserves functionality.

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
d19b12a681 virsh-backup: Fix argument annotations of 'backup-begin' command
Mark the 'backupxml' as positional optional and the 'checkpointxml' as
'unwanted_positional' to preserve the positional parsing quirk.

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
3bbc97620a virsh: Make '(snapshot|checkpoint)-create' 'xmlfile' argument positional
The argument is optional thus couldn't be marked as positional until now,
despite being parsed positionally.

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
29f3ac4223 virsh: snapshot: Make 'snapshotname' argument positional
The 'snapshotname' argument is optional as by default "current" snapshot
is considered. Regardless of that we should treat it as positional as
it's the common usage. This is now possible as we can have one optional
positional argument.

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
0ce337b20c vsh: Make the only argument of 'connect', 'cd', and 'help' commands positional
The intended use of those commands is to use the argument directly
without the flag. Since the argument is optional in all cases we
couldn't declare them as positional until now.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2024-04-02 14:24:29 +02:00