When starting a migration with --timeout, we create a thread to call the
migration API and in parallel setup a timer for the timeout. The
description of --timeout says: "run action specified by --timeout-*
option (suspend by default) if live migration exceeds timeout", which is
not really the way this feature was implemented. Before live migration
starts we first need to contact the source to get the domain definition
and send it to the destination where a new QEMU process has to be
started. This can take some (unpredictably long) time while the timeout
timer is already running. If a very short timeout is set (which doesn't
really make sense, but it's allowed), we may even end up taking the
timeout action before the actual migration had a chance to start.
With this patch the timeout is started only after we get non-zero
dataTotal from virDomainGetJobInfo, which means the migration (of either
storage or memory) really started.
https://issues.redhat.com/browse/RHEL-41264
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add an error message for the rare case if json_tokener_new
fails (allocation failure) and guard any use of json_tokener_free
where tok might be NULL (this was possible in libvirt-nss
when the json file could not be opened).
https://gitlab.com/libvirt/libvirt/-/issues/581
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Simon Pilkington
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Completely remove use of g_malloc (without zeroing of the allocated
memory) and forbid further use.
Replace use of g_malloc0 in cases where the variable holding the pointer
has proper type.
In all of the above cases we can use g_new0 instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The wireshark address.h header uses 'g_memdup2' but this triggers
warnings under clang due to the max version cap:
In file included from ../tools/wireshark/src/plugin.c:27:
In file included from /usr/include/wireshark/epan/proto.h:30:
In file included from /usr/include/wireshark/epan/packet_info.h:15:
/usr/include/wireshark/epan/address.h:107:18: error: 'g_memdup2' is deprecated: Not available before 2.68 [-Werror,-Wdeprecated-declarations]
107 | addr->priv = g_memdup2(&val, sizeof(val));
| ^
/usr/include/glib-2.0/glib/gstrfuncs.h:341:1: note: 'g_memdup2' has been explicitly marked deprecated here
341 | GLIB_AVAILABLE_IN_2_68
| ^
/usr/include/glib-2.0/glib/glib-visibility.h:771:32: note: expanded from macro 'GLIB_AVAILABLE_IN_2_68'
771 | #define GLIB_AVAILABLE_IN_2_68 GLIB_UNAVAILABLE (2, 68)
| ^
/usr/include/glib-2.0/glib/glib-visibility.h:32:35: note: expanded from macro 'GLIB_UNAVAILABLE'
32 | #define GLIB_UNAVAILABLE(maj,min) G_UNAVAILABLE(maj,min) _GLIB_EXTERN
| ^
/usr/include/glib-2.0/glib/gmacros.h:1285:47: note: expanded from macro 'G_UNAVAILABLE'
1285 | #define G_UNAVAILABLE(maj,min) __attribute__((deprecated("Not available before " #maj "." #min)))
| ^
1 error generated.
It is unclear why clang warns, but gcc does not. Our plugin doesn't
actually use the inline helper in address.h that references g_memdup2,
but we get the warning regardless.
Interestingly removing the 'gmodule.h' include avoids the warning. Since
there is nothing in plugin.c that appears to need gmodule.h, removing it
should be safe & done regardless.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Expose the new parameter as '--migrate-disks-detect-zeroes' option.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
While the parsing is still done by 1K buffers, the results
are no longer filtered during the parsing, but the whole JSON
has to live in memory at once, which was also the case before
the NSS plugin dropped its dependency on libvirt_util.
Also, the new parser might be more forgiving of missing elements.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
While the parsing is still done by 1K buffers, the results
are no longer filtered during the parsing, but the whole JSON
has to live in memory at once, which was also the case before
the NSS plugin dropped its dependency on libvirt_util.
Also, the new parser might be more forgiving of missing elements.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Ensure both are required during this series to make bisecting smooth.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Commit 271940223c which strived to add
support to use '--uuid' in the table output of 'virsh list' went too far
and also allowed the default table view to be enabled when just '--uuid'
is specified.
This broke the script-friendly output which previously had this format:
$ virsh list --uuid
b6d03c07-86f8-4a57-8719-172a5d0359bb
to this script-unfriendly output:
$ virsh list --uuid
Id Name State UUID
-------------------------------------------------------------
1 ha running b6d03c07-86f8-4a57-8719-172a5d0359bb
Using the human friendly output will still be possible by using:
$ virsh list --table --uuid
Fixes: 271940223c
Closes: https://gitlab.com/libvirt/libvirt/-/issues/666
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Instead of having many if-else statements, each with its own
vshTableRowAppend() call, we can use a simple trick - have an
array of string pointers, set array members in the if bodies and
then call vshTableRowAppend() once.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
All calls to vshTableRowAppend() inside of cmdList() share couple
of same arguments: domain ID, domain name and domain state. While
the first one is stored in a variable and then passed to all
vshTableRowAppend() calls, the others are passed as a function
call. Switch the latter to variables too.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
There is a family of convenient macros: NULLSTR, NULLSTR_EMPTY,
NULLSTR_STAR, NULLSTR_MINUS which hides ternary operator.
Generated using the following spatch (and its obvious variants):
@@
expression s;
@@
<+...
- s ? s : "<null>"
+ NULLSTR(s)
...+>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This will allow to print full domains info:
Id Name State UUID
---------------------------
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Thing about vshReadlineInit() is - it's called multiple times.
The first time from vshInit(), when @ctl was filled only
partially (most notably, before any argv parsing is done, hence
ctl->imode is set to false). The second time after argv parsing,
from virshInit() -> vshInitReload(). In here, ctl->imode might
have changed and thus vshReadlineInit() can't exit early - it
needs to set up stuff for interactive mode (history basically).
To allow vshReadlineInit() to be called again,
vshReadlineDeinit() must set @autoCompleteOpaque to NULL.
Fixes: cab1e71f01
Resolves: https://issues.redhat.com/browse/RHEL-53560
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
On various occasions, virt-host-validate parses /proc/cpuinfo to
learn about CPU flags (see virHostValidateGetCPUFlags()). It does
so, by reading the file line by line until the line with CPU
flags is reached. Then the line is split into individual flags
(using space as a delimiter) and the list of flags is then
iterated over.
This works, except for cases when the line with CPU flags is too
long. Problem is - the line is capped at 1024 bytes and on newer
CPUs (and newer kernels), the line can be significantly longer.
I've seen a line that's ~1200 characters long (with 164 flags
reported).
Switch to unbounded read from the file (getline()).
Resolves: https://issues.redhat.com/browse/RHEL-39969
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
If virt-host-validate is ran on a SEV-SNP capable machine, an
extra "PASS" is printed out. This is because
virHostValidateAMDSev() prints "PASS" and then returns 1
(indicating success) which in turn makes the caller
(virHostValidateSecureGuests()) print "PASS" again. Just drop the
extra printing in the caller and let virHostValidateAMDSev() do
all the printing.
Fixes: 1a8f646f29
Resolves: https://issues.redhat.com/browse/RHEL-46868
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
With a simple cpuid (Section "E.4.17 Function
8000_001Fh—Encrypted Memory Capabilities" in "AMD64 Architecture
Programmer’s Manual Vol. 3") we can detect whether CPU is capable
of running SEV-ES and/or SEV-SNP guests. Report these in
virt-host-validate tool.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The code that validates AMD SEV is going to be expanded soon.
Move it into its own function to avoid lengthening
virHostValidateSecureGuests() where the code lives now, even
more.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
'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>
