In commit aa1bf13b9c I attempted to fix
the ordering of virtlockd and virtlogd during shutdown but I made a typo
in the name of 'virtlockd.service' (missing 'd').
Fixes: aa1bf13b9c
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The image metadata crawler code generally forbids qcow2 images which
have a 'backing_file' but don't specify a 'backing_file_fmt' header as
we don't want to probe the format due to security implications.
There's one notable exception for the last image in the chain which can
be format probed if it doesn't have another 'backing_file'.
As the comment in 'virStorageSourceGetMetadataRecurse' suggests we don't
want to allow the probe if anything probed would influence access to
more resources. Unfortunately that didn't involve 'data_file' header
which gives more access.
Fortunately there is no way for a guest OS having a 'raw' disk and
access to libvirt's snapshot API to abuse this as libvirt's snapshot API
does specify the header. Other cases where an arbitrary image is passed
to libvirt can 'backing_file'+'backing_file_fmt' directly to access
arbitrary file on disk so this bug is no worse in this regard.
Nevertheless it's a bug which should be fixed.
Closes: https://gitlab.com/libvirt/libvirt/-/work_items/904
Fixes: 0a3d177d9b
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add an image example which specifies a 'backing_file' without
'backing_format' and the backing file has 'data_file'. This was obtained
by running a hacked qemu-img with bypassed requirement to specify backing
file format:
$ qemu-img create -f qcow2 -b datafile.qcow2 -u qcow2_datafile-auto.qcow2 1024
In the test directory.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemu-11.1 was released, update the capability dump to the final state.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemu-11.1 was released, update the capability dump to the final state.
Notable change:
- 'win-dmp' dump format gained 'allowed-by-guest' feature flag
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
CVE-2026-18917
The RPC handler 'remoteDispatchNodeGetFreePages' multiplies the 'npages'
argument with the 'cellcount' argument passed to 'virNodeGetFreePages',
both of which are declared as 'unsigned int' to both do an RPC limit
check against the 'REMOTE_NODE_MAX_CELLS' constant and then to allocate
the memory to hold the result from the actual hypervisor driver.
Since both the values are 'unsigned int' the product is also unsigned
int so big enough numbers can overflow, both passing the check and also
allocating not enough memory for the result. The hypervisor driver
assumes that the passed buffer is large enough and overwrites memory.
When this happens the the hypervisor daemon crashes.
This can be triggered e.g. by passing 1023 and 4198405 as values which
multiply to 1019 after wrapping to 32 bit unsigned value.
Use the VIR_INT_MULTIPLY_OVERFLOW macro in the check to avoid the issue
the same way as we do for other APIs doing multiplication of arguments
to determine amount of required memory.
Fixes: 34f2d0319d (v1.2.5-164-g34f2d0319d)
Closes: https://gitlab.com/libvirt/libvirt/-/work_items/903
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Archives (as witnessed by recent reports) hide useful information by
requiring the maintainer to download the archive which may be dangerous.
Recent submissions also contained a lot of fluff inside the archives.
Instruct submitters of security issues to attach files directly instead
of hiding them in an archive.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Use the same heading color as we do for '.. warning::'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Data obtained by running:
virsh -q capabilities > tests/capabilityschemadata/caps-qemu-real-full.xml
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Allow '.' so that CPU features such as:
<feature name='sse4.1'/>
pass schema validation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Some guest OSes (e.g. windows) can do an internal crash dump and reboot
after a crash. They unfortunately notify the OS before doing the crash
dump though so we need a new possibility for the <on_crash> action to
keep the VM as is.
This patch introduces 'preserve-running' mode for on_crash and wires it
up into the qemu driver.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Separate the individual steps (setting of VM state, dumping, unlocking,
final state transition) under individual switch statements for future
extension.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the code directly to the only place that calls this function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove the 'ret' variable and format the panic string on a single line.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'processGuestPanicEvent' gets the full domain object; there's no point
passing it via 'struct qemuProcessEvent'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rework the code so that it'll be possible to easily extend it to set
other lifecycle actions in qemu.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rework the code so that it'll be possible to easily extend it to set
other lifecycle actions in qemu.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The capability is no longer used because all qemu versions we support do
have the 'set-action' command.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The value is no longer used. Remove it including the status XML
handling.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'set-action' QMP command was introduced in qemu-6.0 and can't be
disabled in any way. Oldest qemu we support is qemu-7.2 so all versions
now have it.
Remove all conditional code which depends on the 'set-action' command.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The qemu driver support was removed in libvirt 5.5.0 but the top level
documentation for the API didn't mention that yet.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemu-11.1 switched to the new syntax. Rather than adding a redundant
test case add a comment that we use the pinned cpu tests for the old QMP
monitor too.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Use the new 'monitor-qmp' object to instantiate the monitor instead of
the deprecated '-mon'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Detect whether qemu supports monitor instantiated via '-object' instead
of the legacy '-mon'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Use '=' to delimit identifier and value. Since we're logging VM object
log also the name of the VM.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The debug message recording the string we've got to process didn't
specify which monitor object the reply comes from which makes it harder
to follow what's happening in cases when multiple VMs communicate (e.g.
when reconnecting at startup).
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
For socket activation to work our systemd unit files use the following
pattern:
[virtlogd.socket] <----(After)--- [virtlogd.service]
[virtqemud.socket] <----(After)--- [virtqemud.service]
Now the qemu daemon also wants to use the services provided by those
daemons so we have dependency between the two too:
[virtlogd.socket] <----(After)--- [virtlogd.service]
^
+-------------(After+Requires)-------+
|
[virtqemud.socket] <----(After)--- [virtqemud.service]
Now on startup everything is fine, because with socket activation, when
'virtqemud.service' wants to use 'virtlogd' services the socket is
already up due to the dependency+ordering and opening a connection will
cause 'virtlogd.service' to be socket-activated.
On shutdown though there's no transitive 'After' ordering between
'virtqemud.service' and 'virtlogd.service' and thus nothing explicitly
telling systemd that if virtlogd was started. In fact systemd is free to
translate it that 'virtlogd' and 'virtqemud' need to be stopped before
stopping 'virtlogd.socket'.
To illustrate what happens consider the following scenario:
A host is running a VM under virtqemud. 'virtqemud' is configured to
attempt shutdown on the VMs before killing them (daemon-based guest
shutdown, but the same reproduces also with libvirt-guests). The host
is being rebooted.
(virtqemud attempts to shut down guests, but guest takes more than the
configured shutdown inhibition timeout, journald output follows):
06:44:02 fedora systemd-logind[664]: Delay lock is active (UID 0/root, PID 991/virtqemud) but inhibitor timeout is reached.
06:44:02 fedora systemd-logind[664]: System is rebooting.
[...]
06:44:02 fedora virtlogd[802]: 802: debug : virSystemdNotify:667 : Notify 'STOPPING=1'
06:44:02 fedora systemd[1]: Stopping virtlogd.service - libvirt logging daemon...
06:44:02 fedora systemd[1]: Stopping virtqemud.service - libvirt QEMU daemon...
06:44:02 fedora virtqemud[991]: 991: debug : virSystemdNotify:667 : Notify 'STOPPING=1'
06:44:02 fedora systemd[1]: virtlogd.service: Deactivated successfully.
06:44:02 fedora systemd[1]: Stopped virtlogd.service - libvirt logging daemon.
(the shutdown times out, virtqemud kills the unresponsive vm)
06:44:27 fedora virtqemud[991]: 1053: debug : qemuProcessStop:8916 : Shutting down vm=0x7f71ac032670 name=virt-vm1 id=1 pid=805, reason=destroyed, asyncJob=none, flags=0x0
06:44:27 fedora virtqemud[991]: 1053: debug : qemuDomainLogAppendMessage:5757 : Append log message (vm='virt-vm1' message='2026-06-15 10:44:27.427+0000: shutting down, reason=destroyed
) stdioLogD=1
06:44:27 fedora virtqemud[991]: 1053: error : virNetSocketReadWire:1767 : Cannot recv data: Connection reset by peer
06:44:27 fedora virtqemud[991]: 1053: debug : qemuProcessKill:8811 : vm=0x7f71ac032670 name=virt-vm1 pid=805 flags=0x5
Now the log shows that we want to add VM log file message in
'qemuDomainLogAppendMessage' but it fails because virtlogd is dead
already.
Now the same happens also with 'virtlockd' but with much worse outcome,
especially if the configured action is to save the VMs because shutdown
of 'virtlockd' when locks are held ends up 'fencing' the VMs by killing
them.
The same also happens when libvirt-guests is used to shutdown the guests
instead.
This patch adds an explicit 'After=virtlo[ck|g]d.service' to the daemons
containing the qemu driver to ensure that the shutdown ordering makes
sense. This doesn't break socket activation (e.g. the log/lock daemons
are not started unless first invoked).
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
While we don't normally try to re-wrap errors to be more descriptive,
probing of qemu capabilities is a special case as the errors can be
cryptic and don't give users enough information about what's going on.
An example is (obtained from a patched qemu which accidentally returned
a broken QMP schema):
$ virsh start VM
error: Failed to start domain 'VM'
error: internal error: Duplicate hash table key 'str'
The error is unusable. On the other hand in 'virQEMUCapsLogProbeFailure'
we do special logging of the caps probing failure with a custom UUID so
that it can be easily found in the logs which reports a much better
error message. Re-raise the same as a normal error too so that the user
gets similar experience:
$ virsh start VM
error: Failed to start domain 'VM'
error: internal error: Failed to probe capabilities for /home/user/git/qemu.git/build/qemu-system-x86_64: internal error: Duplicate hash table key 'str'
This does mean though that the error is logged twice, once via the
custom log entry and second via 'virReportError'. Given that this error
is extremely unlikely, having two log entries IMO doesn't warant
reworking the whole error reporting infra to allow passing extra
metadata just for this one case.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When probing of capabilities fails 'qemuProcessQMPStop' kills the qemu
process, but also resets the error. The virResetLastError() was
originally intended to reset the error from 'virProcessKillPainfully',
it also resets any probing error resulting in the following:
$ virsh start VM
error: Failed to start domain 'VM'
error: An error occurred, but the cause is unknown
Keep the error clearing as this might be called on success but preserve
the original error around it.
Fixes: db7b0b12b7
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'qemuValidateDomainDeviceDefVideo' ensures that only the correct video
device models are selected as well as that only QXL and VIRTIO video
devices can be selected as secondary.
Remove unnecessary checks and simplify the code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The virtio video device frontend type is either selected by the
post-parse code based on capabilities or provided by the user/existing
XML explicitly. No need to try to come up with a model when generating
commandline based on broken logic.
The difference in test output shows:
- honours user's config in case of the new 'device' attribute
- shows how incorrect fallback would be used for 'virtio-vga-gl'
(picked virtio-vga (non-gl) instead of 'virtio-gpu-gl')
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The currently existing checks are broken:
- only QEMU_CAPS_DEVICE_VHOST_USER_GPU is checked for vhostuser
backends (vhost-user-vga is actually separately packaged)
- the check for the 3d accelerated (-gl) versions checks only if one
of them exists (the commandline formatter picks a non-gl afterwards)
- 'virtio-vga'/'virtio-gpu' is not checked at all
The code also doesn't yet check if, when the user passes the new
'device' property manually the config actually makes sense.
To fix all of the above introduce a table of supported frontend devices
as well as properties that need to be checked for them.
This requires fixing a recently-introduced test case which shows a
nonsensical situation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Historically 'virtio-vga' was always picked as the first '<video>'
(virtio) device and any sub-sequent ones were 'virtio-gpu'. When support
for aarch64 VMs was being added an exception to use 'virtio-gpu' for the
primary device was added as aarch64 doesn't have anything resembling the
"legacy" 'VGA' interface. At this point this exception was only for
aarch64. The distinction between 'virtio-vga' and 'virtio-gpu' was *not*
recorded in the VM XML as it was a new feature (for aarch64) and it
didn't make sense to pick 'virtio-vga'.
Some time later the following commit:
commit 4c029e8cfa
Author: Pavel Hrdina <phrdina@redhat.com>
Date: Fri Sep 30 14:41:37 2016 +0200
qemu_command: properly detect which model to use for video device
This improves commit 706b5b6277 in a way that we check qemu capabilities
instead of what architecture we are running on to detect whether we can
use *virtio-vga* model or not. This is not a case only for arm/aarch64.
modified the code to do this picking by checking presence of
'virtio-vga' device instead. That approach didn't consider the fact that
the modular deployment of qemu allows for the 'virtio-vga' device to be
missing in certain cases, thus introducing a latent bug as we'll pick
'virtio-gpu' in such case but don't record it anywhere.
Now this creates a problem, if the deployments differ, because you can
have two *incompatible* (at migration stream level) setups which are
based on the same identical XML without the possibility for the
destination libvirt instance during migration to pick which is the
correct one.
To prevent this and actually fix any existing such deployment (which
allows upgrade of libvirt daemons on the source) we will record the
picked device frontend at post-parse time into the XML. This luckily
properly handles running VMs even if 'virtio-vga' were already
installed since we record the actual qemuCaps we've started the VM with.
Now 'virtio-vga' vs 'virtio-gpu' is not the only broken piece of logic.
In fact 'virtio-vga-gl' could have been downgraded to 'virtio-vga' based
on some very weird logic (see comments in code for explanation).
The logic in 'qemuDomainDeviceVideoDefPostParse' re-creates the logic
used to setup virtio-vga vs. virtio-gpu, and 'vhost-user-vga' vs.
'vhost-user-gpu' as those still make sense. For the 'gl' variants two
versions exist, one meant to recover running VMs and one for new VMs
where the broken logic makes no sense.
Now this patch just records what was selected into the XML, but doesn't
yet modify the commandline to actually use that value verbatim (e.g. if
the user specified an actual non-default value already).
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There's no point in falling through to the check reporting invalid
type since if the code picks a model that one will be valid.
Reorganize the code so that we can return final decision right away.
This means that the two flags 'virtio' and 'virtioBusSuffix' need to be
set prior to the return.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add a test case demonstrating the switch to 'virtio-gpu' on a host which
would normally pick 'virtio-vga'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
QEMU's commandline generator picks for virtio video between various
actual device models not only based on the XML definition but also
capabilities present. Since none of the devices is actually ABI
compatible we need to record the actually selected device in the XML.
Introduce 'device' attribute:
<video>
<model type='virtio' heads='1' primary='yes' device='virtio-gpu'/>
which will record the actually selected model so that we can preserve
ABI across restarts on deployment changes but more importantly across
migrations where the deployment differs.
The code specifically avoids an ABI stability check for the new field
because there are already possibly broken configurations that the users
may want to fix by picking the proper model which could be forbidden.
Users are instructed to not set the field in the XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add example of two running configs with distinct recorded capabilities
(presence of QEMU_CAPS_DEVICE_VIRTIO_VGA at startup) which will
demonstrate the recording of the picked actual device type on the
commandline.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Similarly to previous patch add testing of 'virtio-gpu-gl' or
'virtio-vga-gl' with missing the respective capabilities, but this time
allowing VIR_DOMAIN_DEF_PARSE_ABI_UPDATE.
This will later on show that in case when the fallback can't be honoured
the code will not pick a device that doesn't support acceleration (the
non-gl variant).
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The capability check in 'qemuValidateDomainDeviceDefVideo' which
validates whether a <video> definition with acceleration enabled is
possible is only aggregate, thus validates that any '-gl' video backend
is available.
Since qemu compiles each backend into a separate module it's possible to
have an installation where 'virtio-gpu-pci-gl' exist but 'virtio-vga-gl'
doesn't and it will not be rejected at validation. The commandline
though will generate a device *without* the '-gl' which is ABI
incompatible with the counterpart which does have '-gl', but the VM
starts. If such a VM is then migrated to a deployment which does have
the '-gl' variant available, migration will fail because qemu will
generate the '-gl' device as we don't record this fact in the XML.
This test case captures this situation which will be fixed later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The test case shows that if the 'QEMU_CAPS_DEVICE_VIRTIO_VGA' capability
is not present (e.g. if the corresponding qemu module isn't installed)
libvirt will pick:
-device '{"driver":"virtio-gpu-pci", ...
instead of:
-device '{"driver":"virtio-vga", ...
but without any discernable difference in the XML.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Upcoming patches will add additional testing for various virtio-*-gl
devices, including filling of the default model. The output file needs
to not influnece the input for this test to work properly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When querying capabilities for the default emulator with no other
arguments (e.g. 'virsh domcapabilities) fix error whithout emulator
installed an error is reported but the error would mention '(null)'
architecture:
# virsh domcapabilities
error: failed to get emulator capabilities
error: unsupported configuration: unable to find any emulator to serve '(null)' architecture
This happens as the error formatting takes 'archStr' which is NULL for
the default architecture instead of using 'arch' which is populated by
the host's architecture and converting it back.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The user configuration of added/removed qemu capabilities via the qemu
namespace element was applied only right before generating a
commandline, but the post parse code code didn't see these.
Apply the capability modification prior to running post parse code so
that defaults are properly picked based on the configuration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The user configuration of added/removed qemu capabilities via the qemu
namespace element was applied only right before generating a
commandline, but the validation code didn't see these.
Modify the validation entry points so that they apply this optionally.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Move and rename the function to 'qemuDomainUpdateCustomCapabilities' and
modify the arguments so that it will be possible to reuse it also in the
post-parse and validation code which ought to base decisions on the same
logic as VM startup would.
Since copying of the qemu capabilities object is very expensive (I've
observed an almost 4x slowdown of qemuxmlconftest)
'qemuDomainUpdateCustomCapabilities' copies the capabilities only when
necessary.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Upcoming patch will add a function which will need to optionally copy
passed capabilities for modification. Export 'virQEMUCapsNewCopy'
outside of tests.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The 'capability_filters' allows admins to globally disable some qemu
capabilities via the config file.
Until now it was applied only directly when starting the VM, but that is
too late as the capability is still present when e.g. the post-parse
code is picking defaults.
Rework the code so that 'capability_filters' is applied directly after
probing qemu so all existing capabilities will lack the filtered out
ones.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Trying to pass STDIO fds to a virCommand is very bad and test cases must
not do that.
Same way with invalid FDs.
Add code which makes qemuxmlconftest fail if any test case would attempt
that.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add 'errors' field for tracking a list of errors and
'virTestDummyFDContextMarkError' function to add to the list.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
'virTestDummyFDContext' was a copy of GHashTable to be able to register
a cleanup function. Upcoming patches will want to track more data
together with the hash table so turn virTestDummyFDContext into a proper
struct which contains the hash table.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
glib's G_REGEX_OPTIMIZE invokes pcre's JIT optimizer which requires
writable executable memory. On hosts with SELinux this by default caused
AVC denials to be logged. The solution in Fedora's SELinux policy was
to allow execmem for e.g. virtqemud, but e.g. virtlogd or
libvirt_leaseshelper on the other hand got a 'dontaudit' rule.
Now the optimizer itself does have a substantial effect; I've measured
around 3x speedup when matching VIR_LOG_REGEX against a sample of a qemu
VM log file. On the other hand, only 3 out of 10 uses of 'g_regex_new'
used the flag and also none of the regexes which used 'G_REGEX_OPTIMIZE'
are on a hot path:
- virDomainQemuMonitorEventStateRegisterID
Used only when custom qemu monitor event callback is registered,
which resides in libvirt_qemu.so so noone will actually use this
in production.
- virCommandRunRegex
Used in:
- virStorageBackendFileSystemNetFindNFSPoolSources
Invoked only via API on output of 'showmount'. Unlikely to ever
see lots of data.
- virStorageBackendLogicalFindLVs
- virStorageBackendLogicalGetPoolSources
- virStorageBackendLogicalRefreshPool
Invoked on output of lvs/pvs/vgs. Neither of them are likely to
ever see lots of output to match.
- virISCSIGetSession
- virISCSIScanTargetsInternal
Invoked on output of iscsiadm, unlikely to see lots of data.
- virLogProbablyLogMessage
- virLXCProcessIgnorableLogLine
- domainLogContextReadFiltered
Both of the above are used on code paths processing log output
when failure to startup a VM process or one of the helper
processes for devices for a qemu domain. Thus they are not invoked
on success and the processed buffer is capped to 1k of data.
Since none of the above are on anything resembling a hot path and thus
likely to substantially benefit from the optimizer, and we do have
plenty of other uses without optimization, drop the optimizations
everywhere and add a note to sc_G_REGEX_OPTIMIZE that we're currently
avoiding the use of this flag.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
glib's G_REGEX_OPTIMIZE flag for regex operations implies the use of
pcre's JIT optimizer for regexes which requires writable executable
memory for the optimized code.
Security frameworks such as SELinux can restrict program's access to
writable executable memory to harden the code.
glib's implementation of the regex functions handles downgrade to
interpreted evaluation of regexes gracefully if writable executable
memory is unavailable, but that is non-obvious and has a performance
penalty.
Add a syntax check which requires all uses of G_REGEX_OPTIMIZE to be
annotated so that the reader of the code can be hinted to why
G_REGEX_OPTIMIZE should perhaps be avoided.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The dump is based on QEMU commit 'v11.0.0-1600-g5611a9268d'
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The dump is based on QEMU commit 'v11.0.0-1600-g5611a9268d'
Notable changes:
- machine types for the 11.1.0 release added
- (auto) deprecated 8.1 machine type
- 'poll-weight' property added for 'iothread' object
- 'clipboard' property added for 'gtk' display backend
- 'vhost-user-rtc' device added
- new CPU models/versions
- Cascadelake-Server-v6-x86_64-cpu
- Cascadelake-Server-v7-x86_64-cpu
- ClearwaterForest-v4-x86_64-cpu
- DiamondRapids-v2-x86_64-cpu
- EPYC-Genoa-v3-x86_64-cpu
- EPYC-Milan-v4-x86_64-cpu
- EPYC-Turin-v2-x86_64-cpu
- GraniteRapids-v6-x86_64-cpu
- GraniteRapids-v7-x86_64-cpu
- Icelake-Server-v8-x86_64-cpu
- Icelake-Server-v9-x86_64-cpu
- SapphireRapids-v7-x86_64-cpu
- SapphireRapids-v8-x86_64-cpu
- SierraForest-v6-x86_64-cpu
- Skylake-Server-v6-x86_64-cpu
- 'query-kvm' command is now deprecated
- removed 'gluster' blockdev backend
- 'blkdebug' blockdev backend allows injecting delays
- chardev backends support 'encoding' property
- 'remaining' amount reported in 'query-migrate'
- 'target-info-x86_64' QOM type added
- 'x-rdma-chunk-size' migration parameter added
The few changed '.args' files update the machine type to the actual
version because they were frozen before we've added a newer capability
dump (latest machine type is masked to stabilize test outputs).
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
qemu-11.1 will drop support for the 'gluster' block backend driver. We
want to keep the tests around to validate that nothing in the
parser/generator has changed but there's no point in wiring up QMP
schema validation against older versions.
Skip the schema validation for gluster qemublocktests.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Drop the job definitions now that Cirrus CI was removed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Cirrus CI no longer exists. Drop the current jobs using it from the
definitions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Similarly to previous commits use real FDs so that we don't risk
collisions.
Note that for the test cases passing multiple tap and vhost FDs the
helpers which stabilize the output don't actually work, as the FDs are
concatenated. For now we'll not deal with this and simply leave them
censored by the 'XXXXXXX' string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The code was refactored to allow any FD numbers so the code to map them
to specific numbers is no longer needed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
While VDPA disks did use real FDs they used convoluted infrastructure
for mapping them to specific numbers. Remove that since we can now mask
them from the output args instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use virTestMakeDummyFD() to get a real FD for the test in
qemuxmlconftest.
Use virTestMakeDummyFD() also in qemuhotplugmock.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use 'virTestMakeDummyFD()' to create real FDs both for 'tpmfd' and
'cancelfd'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Initialize the chardev backends for vhost-user graphics to use real FDs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use the virTestMakeDummyFD() function to setup real FDs for all chardev
backends which are set up via testQemuPrepareHostBackendChardevOne.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Thanks to real FD testing we no longer need to inject test
infrastructure into production code. Remove it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Switch over to 'virTestMakeDummyFD' which allows to use real FDs in the
test and also allows to track them in the output files in a stable way.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Switch to 'virTestMakeDummyFD' which uses a real FD. No otuputs are
changed thanks to the code which censors 'fd' field of --add-fd.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use 'testCompareXMLToArgvStabilizeOne' to provide stable test outputs
even when we'll switch to real FDs in tests.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Install hanlder to stabilize 'vhostfd' fds in -device so that we can
later switch tests to use real FDs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Install the handler to stabilize output files on '-object' with
'iommufd' qom type which references a FD directly via the 'fd' property.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use 'testCompareXMLToArgvStabilizeOne' to censor FD values in '-chardev'
commands for stable test outputs without the need for fake FDs.
For monitor sockets this also creates a hint '@mon-fd@' to be used as
substitution to prevent any further churn on basically every .args file.
The substitution is done by temporarily using another fake FD (1764)
instead of 1729 so that it's unique.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add code which walks the generated argument list and allows to strip out
and replace by a substitution any field in either JSON or legacy qemu
argument string.
Use it to stabilize 'fd' field of '-add-fd'
This will allow us to rip out code which tries to allocate stable FD
numbers, which doesn't work reliably (e.g. if the environment passes
some FDs).
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Upcoming patches will introduce use of real FDs for tests. Since the
test will go through many FDs they will collide with the fake ones
allocated randomly in qemuxml2argvmock.
Move them out for now until they are replaced and masked out from the
tests.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The helper makes a dummy (file) FD to be used as FD in tests which want
to handle a FD but don't really use it. It also optionally records the
FD number along with a hint in a hash table which can be later
used to fetch the hint and stabilize test outputs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Add a helper which replaces the value part of a JSON object key. This
will be helpful in tests where we'll want to rewrite some outputs (e.g.
fds) to stabilize output files.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Access the args directly rather than fetching a copy since we have a new
helper now.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Introduce virCommandArgListAccess which returns 'args' and 'nargs'
Upcoming patches will add code which censores/stabilizes FD numbers in
test outputs. This will be done by rewriting the argument of the command
before comparing it with test output. Add a test-only function to
directly access 'args' and 'nargs' of a virCommand to do this
modification.
This accessor will also be used instead of 'virCommandGetArgList' in
'testCompareXMLToArgvValidateSchema' to avoid needles copy of all
arguments.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The 'virCommandPass' mock in 'qemuxml2argvmock.so' skipped passing most
FDs to the real implementation of 'virCommandPass', except for few fake
FDs that were hardcoded.
This meant that if a test case had an valid FD that it used for testing
that FD would be leaked. At the same time fake fds 1730, 1731, 1732
would be actually passed to the virCommand itself although the FD was
invalid.
Since neither of the above makes sense fix the implementation the
following way:
- refuse to pass any STDIO fds
They are real and they would break test program output. Some tests
do try to use them errorneously; they will be addressed later.
- pass real FDs to virCommand
Real FDs can be properly handled by virCommand. Especially they will
be closed once the virCommand object is disposed of.
- don't pass fake FDs
They create extra noise e.g. in valgrind. Skip those as it makes no
sense to handle those.
This patch addresses most failures that valgrind reports with
--track-fds=all.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Some test cases (qemuxmlconftest) currently use made up descriptors,
which cause e.g. valgrind to be unhappy:
==831186== File descriptor 1729 Invalid file descriptor
==831186== at 0x531042E: fgetxattr (in /usr/lib64/libc.so.6)
==831186== by 0x5AE2846: ??? (in /usr/lib64/libselinux.so.1)
==831186== by 0x5AE7093: fgetfilecon_raw (in /usr/lib64/libselinux.so.1)
==831186== by 0x4EB736F: qemuFDPassLogFDInfo (qemu_fd.c:92)
==831186== by 0x4EB7B8E: qemuFDPassDirectTransferCommand (qemu_fd.c:443)
when tracking FDs. Since 'qemuFDPassLogFDInfo' just logs information
about the FD we can simply not query the FD if it's made up.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Extract the lookup of used FDs and their setup to virCommandMassClose
rather than duplicate it in the two functions we have for using
different modes of FD closing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa