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>
Peter Krempa [Wed, 19 Oct 2022 11:31:00 +0000 (13:31 +0200)]
virshFindDisk: Sanitize removable media check
The XPath lookup guarantees that the top level element is always 'disk'
so there's no need to check that it actually is. We can also remove the
two unnecessary temporary variables.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 1 Dec 2022 16:19:50 +0000 (17:19 +0100)]
qemu: monitor: Use qemuMonitorJSONGetReply in conjunction with virJSONValueArrayToStringList
In two instances (qemuMonitorJSONGetStringListProperty,
qemuMonitorJSONGetStringArray) the return value is checked by
qemuMonitorJSONCheckReply and extracted by
virJSONValueObjectGetStringArray.
We can use qemuMonitorJSONGetReply which returns it directly and then
virJSONValueArrayToStringList to convert it without the additional
lookup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 1 Dec 2022 15:13:05 +0000 (16:13 +0100)]
qemuMonitorJSONGetCPUDefinitions: Rework lookup of 'unavailable-features'
Rather than checking that the object has the correct key and then
fetching it again use fetch the array first and then use
virJSONValueArrayToStringList to directly convert it.
Additionally we can avoid the conversion if there are no members
simplifying the surrounding logic.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 1 Dec 2022 16:02:42 +0000 (17:02 +0100)]
qemuAgentGetDisks: Don't use virJSONValueObjectGetStringArray for optional data
The 'dependencies' field in the return data may be missing in some
cases. Historically 'virJSONValueObjectGetStringArray' didn't report
error in such case, but later refactor (commit 043b50b948ef3c2 ) added
an error in order to use it in other places too.
Unfortunately this results in the error log being spammed with an
irrelevant error in case when qemuAgentGetDisks is invoked on a VM
running windows.
Replace the use of virJSONValueObjectGetStringArray by fetching the
array first and calling virJSONValueArrayToStringList only when we have
an array.
Fixes: 043b50b948ef3c2a4adf5fa32a93ec2589851ac6
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2149752 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 1 Dec 2022 12:32:07 +0000 (13:32 +0100)]
util: json: Split out array->strinlist conversion from virJSONValueObjectGetStringArray
Introduce virJSONValueArrayToStringList which does only the conversion
from an array to a stringlist.
This will allow refactoring the callers to be more careful in case when
they want to handle the existance of the member in the parent object
differently.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Thu, 1 Dec 2022 15:18:52 +0000 (16:18 +0100)]
qemu: monitor: Introduce qemuMonitorJSONGetReply, a better qemuMonitorJSONCheckReply
Rather than simply checking that the 'return' field is of the expected
type we can directly return it as the caller is very likely going to use
it. Extract the code into the new function and add a wrapper to preserve
old functionality.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Tue, 22 Nov 2022 10:44:50 +0000 (11:44 +0100)]
internal: Refuse values exceeding range of 'unsigned int' in virCheckFlags
Historically our migration APIs declare 'unsigned long flags'. Since
it's baked into our API we can't change that but we can avoid
compatibility problems by preemptively refusing the extra range on
certain arches to prevent future surprise.
Modify the macro to verify that value passed inside 'flags' doesn't
exceed the range of 'unsigned int'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jim Fehlig [Fri, 2 Dec 2022 00:08:38 +0000 (17:08 -0700)]
spec: Remove use of %{name} macro
The spec file uses both "libvirt" and "%{name}", but in reality the
expanded value of %{name} will never change. Drop the macro in favor
of the explicit and more readable "libvirt".
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
In one of recent commits I've introduced a new test case to
commandtest. In the test case I'm using poll() to wait for data
on a pipe (the write end is passed to commandhelper). However, on
FreeBSD the POLLIN semantic is a bit different:
POLLIN Data other than high priority data may be read
without blocking.
Well, the pipe is non-blocking, so even if there's no data to be
read the flag is set (and subsequent read() returns 0). On the
other hand, POLLHUP is set too, BUT, if the commandhelper manages
to write everything into the pipe and die right after we'd get
both POLLIN and POLLHUP after the very first time poll() returns.
That's very unfortunate, but okay - we can just check whether
read() returned zero and break from the reading loop.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Michal Privoznik [Wed, 30 Nov 2022 08:57:49 +0000 (09:57 +0100)]
tests: Use virTestCompareToString() more
Instead of using:
if (STRNEQ(a, b)) {
virTestDifference(stderr, a, b);
...
}
we can use:
if (virTestCompareToString(a, b) < ) {
...
}
Generated by the following spatch:
@@
expression a, b;
@@
- if (STRNEQ(a, b)) {
+ if (virTestCompareToString(a, b) < 0) {
...
- virTestDifference(stderr, a, b);
...
}
and its variations (STRNEQ_NULLABLE() instead of STRNEQ(), then
in some cases variables passed to STRNEQ() are in reversed order
when compared to virTestCompareToString()).
However, coccinelle failed to recognize the pattern in
testNWFilterEBIPTablesAllTeardown() so I had to fix it manually.
Also, I manually fixed testFormat() in tests/sockettest.c as I
didn't bother writing another spatch rule just for that.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com>
projects / thirdparty / libvirt.git / log
