Erik Skultety [Mon, 15 Mar 2021 14:42:13 +0000 (15:42 +0100)]
ci: util: Add a registry checker for stale images
This function checks whether there are any stale Docker images in the
registry that can be purged. Since we're pulling available container
images from our GitLab registry with the 'list-images' action, it
could happen that we'd list old (already unsupported) images and make
them available for the user to consume and run a build in them.
Naturally, the build will most likely fail leaving the user confused.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Erik Skultety [Tue, 16 Mar 2021 08:47:23 +0000 (09:47 +0100)]
ci: helper: Rewrite image listing to Python
The corresponding Bash script is dropped.
After this patch's rewrite, the Makefile's original image listing
target remains intact only to notify the user to use the Python helper
instead.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Pavel Hrdina [Tue, 16 Mar 2021 12:07:26 +0000 (13:07 +0100)]
conf: introduce support for firmware auto-selection feature filtering
When the firmware auto-selection was introduced it always picked first
usable firmware based on the JSON descriptions on the host. It is
possible to add/remove/change the JSON files but it will always be for
the whole host.
This patch introduces support for configuring the auto-selection per VM
by adding users an option to limit what features they would like to have
available in the firmware.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Pavel Hrdina [Tue, 16 Mar 2021 12:05:08 +0000 (13:05 +0100)]
docs: improve description of secure attribute for loader element
The original text was not explaining what this attribute actually
controls and could have been interpreted as a control switch for the
Secure boot feature in firmwares.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This is because the initial population of the device list is done in a
separate initialization thread. If we attempt to exit libvirtd before
this init thread has completed, we'll try to free the device list while
accessing it from the other thread. In order to guarantee that this
init thread is not accessing the device list when we're cleaning up the
nodedev driver, make it joinable and wait for it to finish before
proceding with the cleanup. This is similar to how we handle the udev
event handler thread.
The separate initialization thread was added in commit 9f0ae0b1.
virxml: Fix possible memory leak in virXMLNodeContentString()
Previously, if xml node passed to the virXMLNodeContentString()
was not of type XML_ELEMENT_NODE, @ret could have caused a memory
leak because xmlNodeGetContent() works for other types of nodes
as well.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Hao Wang [Thu, 18 Mar 2021 09:00:29 +0000 (17:00 +0800)]
NEWS: Document domain memory dirty rate APIs
Signed-off-by: Hao Wang <wanghao232@huawei.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We don't need to go to the trouble of telling users about existance of
insecure SASL mechanisms only to then say that they shouldn't be used.
We should only tell people about the GSSAPI mechanism for TCP sockets.
For the SCRAM mechanism we should be telling people about the SHA256
variant only, and also warning that the password database stores the
passwords in clear text.
Reviewed-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
util: tell users that memory locking ulimit is too low for BPF
If running libvirtd via systemd, it gets a 64 MB memlock limit, but if
running from the shell it will only get 64 KB on a Fedora 33 system.
The latter low limit causes any attempt to use BPF to fail and it is
not obvious why.
This improves the error message thus:
# virsh -c lxc:/// start sh
error: Failed to start domain 'sh'
error: internal error: guest failed to start: Failure in libvirt_lxc startup: failed to initialize device BPF map; locked memory limit for libvirtd probably needs to be raised: Operation not permitted
Reviewed-by: Pavel Hrdina <phrdina@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
src: ensure GSource background unref happens in correct event loop
The g_idle_add function adds a callback to the primary GMainContext.
To workaround the GSource unref bugs, we need to add our callbacks
to the GMainContext that is associated with the GSource being
unref'd. Thus code using the per-VM virEventThread must use its
private GMainContext.
Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Tue, 16 Mar 2021 09:19:45 +0000 (10:19 +0100)]
docs/compiling: Add note on how to check minimum meson version
Give guidance on how to check minimum meson version for a given package.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/140 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Mon, 15 Mar 2021 14:47:04 +0000 (15:47 +0100)]
qemu_process: Use accessor for def->mem.total_memory
When connecting to the monitor, a timeout is calculated that is
bigger the more memory guest has (because QEMU has to allocate
and possibly zero out the memory and what not, empirically
deducted). However, when computing the timeout the @total_memory
mmember is accessed directly even though
virDomainDefGetMemoryTotal() should have been used.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Mon, 15 Mar 2021 17:22:22 +0000 (18:22 +0100)]
meson: Don't check for addr2line
In the past, we used to have this oomtrace.pl script that
attempted to print the stack trace of where an OOM error
occurred and it used addr2line for that. But since v5.8.0-rc1~189
we don't really care about OOM anymore and the script is long
gone so there's no need to check for addr2line program either.
Fixes: 2c52ecd96086b4643b99b4570b5823d40ce2787b Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Andrea Bolognani [Fri, 12 Mar 2021 16:34:23 +0000 (17:34 +0100)]
ci: Discourage users from using the Makefile directly
We now wrap all its important functionality with the much more
user-friendly ci/helper script, and the long term plan is for
the Makefile to disappear completely.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Andrea Bolognani [Tue, 16 Feb 2021 16:21:49 +0000 (17:21 +0100)]
ci: Add helper script
This is intended to be perform a number of CI-related operations
that are currently implemented in various different scripts
written in various different programming languages.
Eventually, all existing functionality will be reimplemented in
Python and made available through this single entry point; for
now, let's start with a very basic skeleton.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Erik Skultety [Fri, 12 Mar 2021 10:53:13 +0000 (11:53 +0100)]
docs: html.in: Convert api to rst
There were a number of occurrences where we used nested inline markup
(verbatim + refs) which is currently not possible with RST syntax [1].
There is a possible workaround involving substitution definitions like
Substitutions cannot be made generic, hence we cannot create a template
for substitution and use a single template everywhere, so we'd end up
with a lot of clutter and convolution. Therefore, we can make an
exception and just link the data type without further style markup.
Pavel Hrdina [Mon, 15 Mar 2021 13:10:45 +0000 (14:10 +0100)]
qemu_driver: fix setting vcpu_quota if not all vCPUs are online
When switching to g_autoptr this was incorrectly changed from
'continue;' into 'return -1;' resulting into an error when user tries
to set vcpu_quota of running VM:
error: An error occurred, but the cause is unknown
Fixes: e4a8bbfaf2b4cdd741bb441873bb730f9134b714 Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Michal Privoznik [Thu, 11 Mar 2021 14:37:07 +0000 (15:37 +0100)]
lib: Put some variable declarations on individual lines
In short, virXXXPtr type is going away. With big bang. And to
help us rewrite the code with a sed script, it's better if each
variable is declared on its own line.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Thu, 11 Mar 2021 07:29:42 +0000 (08:29 +0100)]
syntax-check: Fix and rename virSecurity rule
The aim of virSecurity rule is to discourage from using plain
virSecurityManager*() APIs within QEMU driver in favor of their
qemuSecurity*() counterparts. The reason is simple: namespaces;
virSecurityManager*() needs additional
virSecurityManagerTransactionCommit() call to enter given
namespace and do its work from there. And that's exactly what
those qemuSecurity*() wrappers do.
To help us ensure correctness (from this POV), we have a
syntax-check rule that forbids any occurrence of
"virSecurityManager" string under src/qemu/ (except for
qemu_security of course).
But with if we want to remove virSecurityManagerPtr type, then we
have to allow "virSecurityManager *". Therefore, change the rule
so that no call of a function with "virSecurityManager" prefix is
allowed. And also change the name to better reflect what is going
on.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Thu, 11 Mar 2021 05:17:07 +0000 (06:17 +0100)]
virsysinfo: Define and use auto cleanup func for virSysinfoDef properly
What we are using really is heap allocated structure rather than
stack allocated. And for that it's better to use g_autoptr() +
G_DEFINE_AUTOPTR_CLEANUP_FUNC() combo, as Glib documentation for
g_auto() reads:
This is meant to be used with stack-allocated structures and
non-pointer types. For the (more commonly used) pointer
version, see g_autoptr().
This will be even more visible, when virSysinfoDefPtr type is
gone. Stay tuned.
Fixes: cee3a900a0d6a8fc79554db22dc262632fe487a6 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Thu, 25 Feb 2021 14:01:27 +0000 (15:01 +0100)]
qemu_shim: Don't hang if failed to start domain
The qemu shim spawns a separate thread in which the event loop is
ran. The virEventRunDefaultImpl() call is wrapped in a while()
loop, just like it should. There are few lines of code around
which try to ensure that domain is destroyed (when quitting) and
that the last round of event loop is ran after the
virDomainDestroy() call. Only after that the loop is quit from
and the thread quits.
However, if domain creation fails, there is no @dom to call
destroy over, the @quit flag is never set and while() never
exits. Set the flag regardless of @dom pointer.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1920337 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Peter Krempa [Thu, 26 Nov 2020 18:07:03 +0000 (19:07 +0100)]
qemu: command: Generate commandline of 'sev0' sev-guest object via JSON
While the 'sev0' sev-guest object will never be hotplugged, but we want
to generate it through JSON so that we'll be able to validate all
parameters of '-object' against the QAPI schema once 'object-add' is
qapified in qemu.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 26 Nov 2020 18:07:03 +0000 (19:07 +0100)]
qemu: command: Generate commandline of 'masterKey0' secret via JSON
While the 'masterKey0' secret object will never be hotplugged we want to
generate it through JSON so that we'll be able to validate all
parameters of '-object' against the QAPI schema once 'object-add' is
qapified in qemu.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Recent changes which meant to fix daemon shutdown broke the exec-restart
capability of virtlogd and virtlockd, since the code actually closed all
the sockets and shut down all the internals.
Add virNetDaemonQuitExecRestart, which requests a shutdown of the
process, but keeps all the services open and registered since they are
preserved across the restart.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Thu, 25 Feb 2021 17:20:51 +0000 (18:20 +0100)]
qemu_shim: Always pre-create root dir
This problem is reproducible only with secret driver. When
starting a domain via virt-qemu-run and both secret and
(nonexistent) root directory specified this is what happens:
1) virt-qemu-run opens "secret:///embed?root=$rootdir"
connection, which results in the secret driver initialization
(done in secretStateInitialize()). During this process, the
driver creates its own configDir (derived from $rootdir)
including those parents which don't exists yet. This is all
done with the mode S_IRWXU and thus results in the $rootdir
being created with very restrictive mode (specifically, +x is
missing for group and others).
2) now, virt-qemu-run opens "qemu:///embed?root=$rootdir" and
calls virDomainCreateXML(). This results in the master-key.aes
being written somewhere under the $rootdir and telling qemu
where to find it.
But because the secret driver created $rootdir with too
restrictive mode, qemu can't access the file (even though it
knows the full path) and fails to start.
It looks like the best solution is to pre-create the root
directory before opening any connection (letting any driver
initialize itself) and set its mode to something less
restrictive.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1859873 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Michal Privoznik [Thu, 25 Feb 2021 15:29:22 +0000 (16:29 +0100)]
virConnectOpen: Require root dir to be absolute path
In theory, users might want to use a relative path as a root
directory for embed drivers. But in practice, nothing in driver
initialization (specifically QEMU driver since it's the only one
that supports embedding now), is prepared for that. Document and
enforce absolute paths.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1883725 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Michal Privoznik [Thu, 25 Feb 2021 14:29:16 +0000 (15:29 +0100)]
docs: Lift restriction on running API from the event loop thread
Since v6.2.0-rc1~238 (and friends) QMP processing was moved to a
per-domain thread. Therefore, it is now safe to call APIs from
the event loop thread (e.g. just like qemu shim is doing in
qemuShimEventLoop(). However, it is still important to let the
event loop run after each API call (obviously).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
'res->owners' is allocated to 'res->nOwners' elements, but unfortunately
'res->nOwners' doesn't contain the proper value until after the
allocation so 0 elements are allocated. The following loop which assumes
that the array has the right number of elements then accesses the
pointer out of bounds. The bug was also faithfully converted from
VIR_ALLOC_N to g_new0.
Fixes: 4a3d6ed5ee0 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Fri, 12 Mar 2021 09:12:51 +0000 (10:12 +0100)]
virLockSpacePreExecRestart: Avoid use-after-free
Recent refactor marked 'object' which is returned from the function as
autofree but forgot to use g_steal_pointer in the return statement to
prevent freeing it.
Fixes: 9a1651f64d7 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 11 Mar 2021 15:18:50 +0000 (16:18 +0100)]
qemuBackupJobTerminate: Fix job termination for inactive VMs
Commit cb29e4e801d didn't take into account that the VM can be inactive
when it's destroyed. This means that the job would remain active also
when the VM became inactive.
To fix this properly:
1) Remove the bogus VM liveness check and early return
(reverts the aforementioned commit)
2) Conditionalize the stats assignment only when the stats object is
present
(properly fix the crash when VM dies when reconnecting)
3) end the asyncjob only when it was already set
(prevent corruption of priv->jobs_queued)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1937598 Fixes: cb29e4e801d Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 11 Mar 2021 15:14:17 +0000 (16:14 +0100)]
backup: Store 'apiFlags' in private section of virDomainBackupDef
'qemuBackupJobTerminate' needs the API flags to see whether
VIR_DOMAIN_BACKUP_BEGIN_REUSE_EXTERNAL. Unfortunately when called via
qemuProcessReconnect()->qemuProcessStop() early (e.g. if the qemu
process died while we were reconnecting) the job is cleared temporarily
so that other APIs can be called. This would mean that we couldn't clean
up the files in some cases.
Save the 'apiFlags' inside the backup object and set it from the
'qemuDomainJobObj' 'apiFlags' member when reconnecting to a VM.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 10 Mar 2021 13:33:06 +0000 (14:33 +0100)]
virSystemdCreateMachine: Use proper format string for uint64_t when constructing gvariant
g_variant_new_parsed uses '%t' for a uint64_t rather than printf-like
%llu. Additionally ensure that the passed value is a uint64_t since the
argument used is a 'unsigned int'.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1937287 Fixes: bf5f2ed09c2 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Tue, 2 Mar 2021 13:26:24 +0000 (14:26 +0100)]
util: virstring: Remove virStrncpy
The function is now unused and motivated users to write crazy parsers
which were hard to understand, had pointless error paths just to avoid
few memory allocations.
Remove the function as we're fine with g_strndup and virStrcpy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is similar to my earlier commit which documented lxc.conf
location. Just like LXC, the libxl driver has only the system
connection and thus only few places need changing.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
manpages: Remove reference to a session daemon for libxl
The libxl driver has no session daemon therefore its split daemon
(virtxend) has to be ran as root. Any attempt to start it with
euid != 0 fails. This is why the daemon does not look under any
of XDG_* paths either.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
This is similar to my earlier commit which documented qemu.conf
locations. Luckily, the LXC driver has only the system connection
and not session or embed one.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
manpages: Remove reference to a session daemon for LXC
The LXC driver has no session daemon therefore its split daemon
(virtlxcd) has to be ran as root. Any attempt to start it with
euid != 0 fails. This is why the daemon does not look under any
of XDG_* paths either.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
syntax-check: Update list of gethostname exceptions
The only place where gethostname() is acceptable is in
virGetHostnameImpl() which lives in src/util/virutil.c.
Reflect this in the list of exceptions for the syntax-check rule.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
virutil: Do not use g_get_host_name() to obtain hostname
The problem is that g_get_host_name() caches the hostname in a
thread local variable. Therefore, it doesn't reflect any
subsequent hostname changes. While this might be acceptable for
logs where the hostname is printed exactly once when the libvirtd
starts up, it is not optimal for virGetHostnameImpl() which is
what our public virConnectGetHostname() API calls. If the
hostname at the moment of the first API invocation happens to
start with "localhost" or contains a dot, then no further
hostname changes will ever be reflected.
With this, XML fails if config video type 'ramfb' contains
address, since address is not supported for 'ramfb' video
devices. Previously it didn't raise error.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1891416 Signed-off-by: Kristina Hanicova <khanicov@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Ján Tomko <jtomko@redhat.com>