Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2033879 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Wed, 21 Dec 2022 10:08:02 +0000 (11:08 +0100)]
qemumonitortestutils: Fix line counting in qemuMonitorTestProcessFileEntries()
It just so happens that our JSON snippets in
qemucapabilitiesdata/*.replies files are separated by an empty
line. These empty lines are then overwritten to make a single
line JSON. Nevertheless, the line counter @line is not
incremented which then leads to a misleading numbers in errors.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Tue, 20 Dec 2022 08:04:48 +0000 (09:04 +0100)]
secret: Inhibit shutdown for ephemeral secrets
Our secret driver divides secrets into two groups: ephemeral
(stored only in memory) and persistent (stored on disk). Now, the
aim of ephemeral secrets is to define them shortly before being
used and then undefine them. But 'shortly before being used' is a
very vague time frame. And since we default to socket activation
and thus pass '--timeout 120' to every daemon it may happen that
just defined ephemeral secret is gone among with the virtsecretd.
This is no problem for persistent secrets as their definition
(and value) is restored when the virtsecretd starts again, but
ephemeral secrets can't be restored.
Therefore, we could view ephemeral secrets as active objects that
the daemon manages and thus inhibit automatic shutdown (just like
hypervisor daemons do when a guest is running).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Xen 4.17 has strict parsing of 'soundhw' option that allows only
specific values (instead of passing through any value directly to
qemu's -soundhw option, it uses -device now). For 'intel-hda' audio
device, it requires "hda" string. "hda" works with older libxl too.
Other supported models are the same as in libvirt XML.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
libxl: add validation if sound device is supported
Xen supports only subset of libvirt's sound devices, and starting with
Xen 4.17 it is enforced by libxl. Verify it early.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Fri, 9 Dec 2022 15:49:12 +0000 (16:49 +0100)]
virStorageBackendRBDOpenRADOSConn: Don't log the RBD key
'virStorageBackendRBDRADOSConfSet' logs its arguments but it's also
used to set the RBD secret/key.
All the security theatre with securely erasing the string we do to fetch
the secret would be quite pointless if we log it thus introduce
virStorageBackendRBDRADOSConfSetQuiet and use it to avoid logging the
password.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 8 Dec 2022 11:37:30 +0000 (12:37 +0100)]
virCryptoEncryptDataAESgnutls: Properly initialize data structures
The initialization vector is not optional thus we also don't need to
check whether the caller passed it in. Additionally we can use c99
initializers for the gnutls_datum_t structs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Wed, 14 Dec 2022 18:30:07 +0000 (19:30 +0100)]
kbase: Reorder sections
Users are likely more interested in the main deployment
scenarios than in the detailed list of every existing RPM
package. Reorder sections accordingly.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Jim Fehlig <jfehlig@suse.com>
docs: replace footer link to the bird site with mastodon
Since the takeover of the bird site, the bulk of tech people who want
a more friendly and inclusive media site have jumped over to Mastodon.
With its decentralized nature, there's no one replacement that captures
everything, but the fosstodon.org site is a topic relevant choice.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Andrea Bolognani [Tue, 13 Dec 2022 10:09:40 +0000 (11:09 +0100)]
spec: List more directories
The storage-backend/ and storage-file/ directories are currently
considered unowned by RPM. Have the libvirt-daemon package take
ownership of them, just as it already owns the connection-driver/
and lock-driver/ directories.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Jim Fehlig <jfehlig@suse.com>
virnuma: Allow multiple nodes for preferred policy
In the past, the preferred policy
(VIR_DOMAIN_NUMATUNE_MEM_PREFERRED) required exactly one (host)
NUMA node. This made sense because:
1) the libnuma API - numa_set_preferred() allowed exactly one
node, because
2) corresponding kernel syscall (__NR_set_mempolicy) accepted
exactly one node (for MPOL_PREFERRED mode).
But things have changed since then. Firstly, kernel introduced
new MPOL_PREFERRED_MANY mode (v5.15-rc1~107^2~21) which was then
exposed in libnuma as numa_set_preferred_many() (v2.0.15~24).
Fortunately, libnuma also exposes numa_has_preferred_many() which
returns whether the kernel has support for the new mode (1) or
not (0).
Putting this all together, we can lift our check for sufficiently
new kernel and libnuma.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2151064 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jiri Denemark [Fri, 9 Dec 2022 23:04:41 +0000 (00:04 +0100)]
qemu_migration: Fix p2p post-copy recovery
Although the qemuMigrationSrcPerformResume actually got called
indirectly via qemuMigrationSrcPerformNative and the recovery process
worked, wrong job phases were used for the "perform" phase, which could
cause issues when libvirt daemon crashed (or was otherwise restarted)
during post-copy recovery.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Jiri Denemark [Wed, 7 Dec 2022 11:33:13 +0000 (12:33 +0100)]
qemu: Don't warn when releasing a released job
When qemuDomainObjReleaseAsyncJob is called when the current async job
is already released we emit quite useless warning which was implemented
to warn about releasing a job owned by another thread.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Jiri Denemark [Mon, 5 Dec 2022 11:17:56 +0000 (12:17 +0100)]
qemu: Fix warning in qemuMigrationDstPostcopyFailed
The function is called even if QEMU reports migration as
postcopy-paused, i.e., it's not migrating anymore. And while changing
the warning, we can drop the part about unattended migration to make the
warning shorter and consistent with qemuMigrationSrcPostcopyFailed.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
qemu_process: add tray changed event to queue in refresh disks
There are some cases when the internal state of disks can change
without qemu sending events about it (e.g. a disk can close
during reset). In case this happens, we should emit an event
about the modified disk.
While only a couple of the message types include sensitive data,
the overhead of calling secure erase is not noticable enough
to worry about making the erasure selective per type. Thus it is
simplest to unconditionally securely erase the buffer.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
rpc: fix buffer offset updates after decoding payload
The buffer length refers to the allocated buffer memory size,
while the offset refers to have much of the buffer we have
read/written. After reading the message payload we must thus
update the latter.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Mon, 12 Dec 2022 11:46:41 +0000 (12:46 +0100)]
meson: Provide default values for nonexistent xenlight pkgconfig vars
It may happen that xenlight pkgconfig file does not contain
'xenfirmwaredir' and/or 'libexec_bin' variables, which is okay
and we have code that deals with this situation. But that code is
executed when the queried value is an empty string. This may not
always be the case and we should specifically set 'default_value'
so that the empty string is returned if pkgconfig variable
doesn't exist.
Fixes: 968479adcfa5c49b29b7b6680dcaffde1408f044 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
meson: simplify check for virnetdevbridge.c headers
The headers required by virnetdevbridge.c have all exited since
before Linux moved to git. It is sufficient to check for just
one of them in order to give an error message about needing
kernel headers installed.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
ethtool: Introduce n-tuple filter programming support
This is old enough that all our supported platforms can be assumed
to have this feature.
A typo in the existing condition "NTUBLE" instead of "NTUPLE" meant the
code was never enabled in the first place, which is an illustration of
why it is worth eliminating redundant conditional checks.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
ethtool is a Linux specific feature that has existed since before Linux
moved to git. Checking against SIOCETHTOOL + WITH_STRUCT_IFREQ is
overkill for our needs.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
[PATCH] unshare system call -v5: system call registration for i386
This is old enough that all our supported platforms can be assumed
to have this feature. Furthermore, the virprocess.c file was already
using unshare() with nothing more than a #ifdef __linux__ check.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is old enough that all our supported platforms can be assumed
to have this feature. For added fun this whole meson check was
semantically insane because EPOLL_CLOEXEC is not a valid arg
to unshare().
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is old enough that all our supported platforms can be assumed
to have this feature. For added fun this whole meson check was
semantically insane because EPOLL_CLOEXEC is not a valid arg
to unshare().
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
loop: add management interface for on-demand device allocation
This is old enough that all our supported platforms can be assumed
to have this feature. As a plus point, this meson check is going
to start failing with future GCC. It fails to set _GNU_SOURCE, thus
'unshare' is not defined by the header, and its relying on an
implicit function decl. For added fun this whole meson check was
semantically insane because LOOP_CTL_GET_FREE is not a valid arg
to unshare().
Fixes https://fedoraproject.org/wiki/Toolchain/PortingToModernC Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When starting a guest with <interface/> which has the target
device name set (i.e. not generated by us), it may happen that
the TAP device already exists. This then may lead to all sorts of
problems. For instance: for <interface type='network'/> the TAP
device is plugged into the network's bridge, but since the TAP
device is persistent it remains plugged there even after the
guest is shut off. We don't have a code that unplugs TAP devices
from the bridge because TAP devices we create are transient, i.e.
are removed automatically when QEMU closes their FD.
The only exception is <interface type='ethernet'/> with <target
managed='no'/> where we specifically want to let users use
pre-created TAP device and basically not touch it at all.
There's another reason for denying to use a pre-created TAP
devices: if we ever have bug in TAP name generation, we may
re-use a TAP device from another domain.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2144738 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
virnetdev: Make virNetDevGenerateName() return 1 if no name was generated
A caller might be interested in the case when @ifname was already
set and it wasn't a template. In such case the
virNetDevGenerateName() does not touch the @ifname at all and
returns 0 to indicate success. Make it return 1 to distinguish
this case from the other case, in which a new name was generated.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Laine Stump <laine@redhat.com>
The script had an incorrect interpreter line until commit f6a19d7264bb, so the flake8 check would not realize it needed
to pick it up and these issues, some of which were present it
the very first version that was committed, were not being
reported.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Scripts from the following list were installed with group write
bit set: virt-xml-validate, virt-pki-validate,
virt-sanlock-cleanup, libvirt-guests.sh. This is very unusual and
in contrast with the way other scripts/binaries are installed.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2151202 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Haruka Ohata [Wed, 30 Nov 2022 08:04:07 +0000 (17:04 +0900)]
virsh: Add message to terminal when running snapshot-revert
When running virsh snapshot-* command, such as snapshot-create-as /
snapshot-delete, it prints a result message.
On the other hand virsh snapshot-revert command doesn't print a result
message.
So, This patch fixes to add message when running virsh snapshot-revert
command.
Historically, QEMU took screenshots in PPM. While this might use
to be popular format, as of v7.1.0-rc0~125^2~6 it is possible to
take screenshots in PNG. This is more popular and renders almost
everywhere, which is not the case for PPM (for instance, modern
browsers do not render it).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jim Fehlig [Fri, 2 Dec 2022 22:50:05 +0000 (15:50 -0700)]
spec: Remove extra blank lines
The spec file contains inconsistent use of blank lines. While trying to
make significant changes to the file, I found it hurts both readability
and maintainability. Remove blank lines that interrupt the overall flow
and consistency.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Internal domain state may change during the reset and qemu does
not always send events about it. In case it happens, internal
state of the domain in libvirt would be inconsistent with the
internal state in qemu which could cause additional problems
(e.g. cdrom tray state can change from open to closed). The
solution is to refresh state after a successful reset to query
qemu about the current internal domain state.
Paths for external devices (well, so far only vTPM) are not
stored in the status XML. Therefore, we need to regenerate them
after we've been restarted and reconnecting to a running domain.
Otherwise these will remain NULL which may later lead to a NULL
dereference.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2150760 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemu_extdevice: Init paths in qemuExtDevicesPrepareDomain()
The path generation phase belongs conceptually into domain
preparation phase and not host preparation. Move
qemuExtDevicesInitPaths() call from qemuExtDevicesPrepareHost()
into qemuExtDevicesPrepareDomain().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemu_process: Document qemuProcessPrepare{Domain,Host}() order
The domain startup process is split into multiple phases. One of
them is preparing the domain (at that point live) XML, private
data, various paths, etc - see qemuProcessPrepareDomain(); the
other prepares the host - see qemuProcessPrepareHost(). It's
obvious that the domain XML preparation function must be called
before the host preparation function (e.g. the host preparation
might try to create a file which path is generated in the domain
preparation phase). Nevertheless, let's document this
expectation.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 22 Nov 2022 16:01:31 +0000 (17:01 +0100)]
lxcDomainDetachDeviceFlags: Parse XML twice rather than use virDomainDeviceDefCopy
'virDomainDeviceDefCopy' formats the definition and parses it back.
Since we already are parsing the XML here, we're better off parsing it
twice and save the formatting step.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 22 Nov 2022 16:01:31 +0000 (17:01 +0100)]
lxcDomainAttachDeviceFlags: Parse XML twice rather than use virDomainDeviceDefCopy
'virDomainDeviceDefCopy' formats the definition and parses it back.
Since we already are parsing the XML here, we're better off parsing it
twice and save the formatting step.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 22 Nov 2022 16:01:31 +0000 (17:01 +0100)]
qemuDomainDetachDeviceLiveAndConfig: Parse XML twice rather than use virDomainDeviceDefCopy
'virDomainDeviceDefCopy' formats the definition and parses it back.
Since we already are parsing the XML here, we're better off parsing it
twice and save the formatting step.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 22 Nov 2022 16:01:31 +0000 (17:01 +0100)]
qemuDomainUpdateDeviceFlags: Parse XML twice rather than use virDomainDeviceDefCopy
'virDomainDeviceDefCopy' formats the definition and parses it back.
Since we already are parsing the XML here, we're better off parsing it
twice and save the formatting step.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
We don't refuse override definitions for device which doesn't exist and
the same way don't care about 'remove' being used on a property which is
not actually formatted by libvirt. Drop the paragraph claiming the
contrary.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 21 Nov 2022 12:44:54 +0000 (13:44 +0100)]
docs: drvqemu: Fix and improve docs about device override types
The 'number' override type didn't exist in the final version so change
it to the corresponding 'signed' and 'unsigned'.
Additionally clarify which override type is used for a corresponding
qemu type and also that we use base 10 numbers so users will need to
convert the numbers if needed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Shaleen Bathla [Fri, 11 Nov 2022 09:24:38 +0000 (14:54 +0530)]
qemu: Don't report spurious errors from vCPU tid validation on hotunplug timeout
Use of qemuDomainValidateVcpuInfo in the helpers for hotplug and unplug
of vCPUs can lead to spurious errors reported such as:
internal error: qemu didn't report thread id for vcpu 'XX'"
The reason for this is that qemuDomainValidateVcpuInfo validates the
state of all vCPUs against the expected state of vCPUs. If an unplug
operation completed before libvirt was unable to process it yet the
expected state could not reflect the current state.
To avoid spurious errors the qemuDomainHotplugAddVcpu and
qemuDomainRemoveVcpu functions are modified to do localized validation
only for the vCPUs they actually modify.
We also now ensure that the cgroups are modified before bailing out on
error for any vCPUs which passed validation.
Additionally in order for qemuDomainRemoveVcpuAlias to be able to find
the unplugged vCPU we must ensure that qemuDomainRefreshVcpuInfo does
not clear out the alias in case when the vCPU is no longer reported by
qemu.
Co-authored-by: Partha Satapathy <partha.satapathy@oracle.com> Signed-off-by: Shaleen Bathla <shaleen.bathla@oracle.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Recently, the QEMU driver gained support for migration with TPM
state on a shared volume (e.g. NFS). As a part of that, the
destination side avoids setting seclabels on it to avoid cutting
off the source while it is still using it. Makes sense, except
for a wee bit: the secdriver API does a bit more - it also sets
label on the swtpm log file. And this one definitely needs to be
labeled (it lives under /var/log/swtpm/libvirt/qemu/..., i.e. not
on a shared volume).
Previously, qemuSecurityStartTPMEmulator() took care of that. But
during rework to shared volume migration, the code was changed so
now plain qemuSecurityCommandRun() would be run (i.e. no
relabelling).
But after previous commits, we can now chose whether the TPM
state should be relabelled or just the log file.
Fixes: 2e669ec789231d39e0d5f5f6a201d2a661b8070c
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2130192#c7 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is basically just a continuation of the previous commit.
Now that the security driver APIs have a boolean flag that
controls setting/restoring seclabel of either both TPM state and
log files, or just the log file, propagate this boolean into
those APIs that start/stop swtpm emulator. For now, just pass
true. The juicy bits are soon to come.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virSecurityDomainSetTPMLabels() and
virSecurityDomainRestoreTPMLabels() APIs set/restore label on two
files/directories:
1) the TPM state (tpm->data.emulator.storagepath), and
2) the TPM log file (tpm->data.emulator.logfile).
Soon there will be a need to set the label on the log file but
not on the state. Therefore, extend these APIs for a boolean flag
that when set does both, but when unset does only 2).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 19 Oct 2022 11:59:17 +0000 (13:59 +0200)]
virshFindDisk: Sanitize use of 'tmp' variable
The return value of virXMLPropString was assigned into 'tmp' multiple
times and to prevent static analyzers moaning about a potential leak a
short-circuited if logic or was used.
Replace the code by having a helper variable for each possibility and
also replace the for-loop to iterate elements.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 2 Dec 2022 09:35:13 +0000 (10:35 +0100)]
util: xml: Introduce virXMLNodeGetSubelement
Introduce a simple helper fetching a sub-element node by name. This is
meant as a simple replacement for either open-coded versions of this or
use of XPath for this trivial lookup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>