Laine Stump [Wed, 12 Feb 2025 03:43:22 +0000 (22:43 -0500)]
conf: change virDomainDefMaybeAddController() to return true/false
This function can't fail, but it has always returned 1 if a controller
is added and 0 if not, and there is one place that checks for a 1
return, so we remove the -1 return and change it to return true/false
instead of 1/0.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Pavel Hrdina [Tue, 18 Feb 2025 18:10:58 +0000 (19:10 +0100)]
ci: use iptables to run libvirt-tck
On current Fedora libvirt uses nftables by default but the libvirt-tck
tests are not ready for it and most of the nwfilter tests fail. We need
to keep using iptables for now.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Michal Privoznik [Tue, 25 Feb 2025 16:49:20 +0000 (17:49 +0100)]
meson: Generate runstatedir according to newer standard
While previously FHS 2.3 defined /var/run as a place to store
runtime information [1] it's no longer 2004 and newer
specification was released which favors /run [2]. Since it was
released 10 years ago, maybe it's time we start honouring it.
On majority of Linux systems (if not all), /var/run is a symlink
to /run anyways.
Users can still pass old location via -Drunstatedir.
virDomainHostdevDefNew: update users not to check return value
virDomainHostdevDefNew() has been using g_new0() for a while now. As it
calls abort() on OOM, it's not necessary to check whether
the return value is NULL.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> Reviewed-by: Laine Stump <laine@redhat.com>
Jim Fehlig [Fri, 14 Feb 2025 23:26:12 +0000 (16:26 -0700)]
include: Define constant for save image format
Add a new VIR_DOMAIN_SAVE_PARAM_IMAGE_FORMAT typed parameter for
specifying the save image format. A format specified via the
virDomainSaveParams API overrides the save_image_format setting
in qemu.conf. The 'raw' format remains the default.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Jim Fehlig [Thu, 20 Feb 2025 21:28:35 +0000 (14:28 -0700)]
qemu: Check for valid save image formats when loading driver config
Checking for valid 'foo_image_format' settings in qemu.conf is not done
until the settings are used. Move the checks to
virQEMUDriverConfigLoadSaveEntry, allowing to report incorrect format
settings at driver startup.
This change was made easier by also changing the corresponding fields
in the virQEMUDriverConfig to 'int', which is more in line with the
other fields that represent enumerated types.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Jim Fehlig [Wed, 12 Feb 2025 04:46:59 +0000 (21:46 -0700)]
qemu: Change return value of SaveImageGetCompressionProgram
qemuSaveImageGetCompressionProgram is a bit overloaded. Along with
getting a compression program, it checks the validity of the image
format and returns the integer representation of the format. Change
the function to only handle retrieving the specified compression
program, returning success or failure. Checking the validity of
the image format can be left to the calling functions.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Jim Fehlig [Thu, 20 Feb 2025 01:02:06 +0000 (18:02 -0700)]
qemu: Don't ignore dump image format errors
Long ago, without justification, commit 48cb9f0542 changed
qemuGetCompressionProgram (since renamed to
qemuSaveImageGetCompressionProgram) to ignore configuration errors
for dump operations. Like the other save-related operations, user
provided configuration should be verified and an error reported if
it cannot be honored.
Remove the special handling of configuration errors in
qemuSaveImageGetCompressionProgram and change the dump logic to
fail when dump image format cannot be supported.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
In case when user provides custom paths (those not covered by the JSON
firmware descriptor files or the default locations) for the
loader and nvram template no auto-detection will be performed and user's
config will be taken at face value. Historically when 'templateFormat'
didn't exist we assumed that the 'format' field covers both.
Thus if 'templateFormat' is VIR_STORAGE_FILE_NONE we need to skip the
check forbidding image format conversion for 'file' backed to avoid
breaking legacy configs with manual/non-detected format assuming that
user picked the correct format.
Add a comment to the declaration of 'nvramTemplateFormat' noting the
above for future reference.
Ján Tomko [Thu, 20 Feb 2025 21:50:06 +0000 (22:50 +0100)]
conf: metadata: remove metadata node if all metadata is removed
When removing the last child element from a network or domain
metadata, free the metadata node itself as well, to prevent
displaying an empty metadata element.
https://issues.redhat.com/browse/RHEL-27172
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Resolves: https://issues.redhat.com/browse/RHEL-71883 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Jiri Denemark [Mon, 24 Feb 2025 10:24:27 +0000 (11:24 +0100)]
cpu: Do not call g_strv_contains on NULL list
When virCPUx86UpdateLive checks whether a feature was added to a CPU
model after the model was already released (vmx-* features in most Intel
models), the following assert could be logged by glib:
g_strv_contains: assertion 'strv != NULL' failed
While most of our CPU models have a non-empty list of added feature, new
models added in 2024 and versioned variants of older models have
addedFeatures == NULL.
Fixes: e622970c8785ec1f7e142d72f792d89f870e07d0 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jiri Denemark [Thu, 20 Feb 2025 13:41:32 +0000 (14:41 +0100)]
virsh: Let prohibit_newline_at_end_of_diagnostic check pass
The prohibit_newline_at_end_of_diagnostic syntax check is confused when
another unrelated translatable message with a newline is too close to
the function it is supposed to check. Refactoring the code to make the
two strings further apart seems like the easiest solution.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jiri Denemark [Thu, 20 Feb 2025 12:30:36 +0000 (13:30 +0100)]
virsh: Warn when hypervisor-cpu-* is used with host CPU
While using host CPU definition from capabilities XML is allowed for
historical reasons, it will likely provide incorrect results and should
be avoided.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jiri Denemark [Thu, 20 Feb 2025 11:49:43 +0000 (12:49 +0100)]
virsh: Do not format messages twice
The same message was formatted both in vshOutputLogFile and in vshDebug
and vshError functions. This patch refactor vshOutputLogFile and its
callers to only format each message once.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jiri Denemark [Wed, 19 Feb 2025 15:16:58 +0000 (16:16 +0100)]
docs: Clarify documentation of virsh hypervisor-cpu-baseline
Using host CPU definition with hypervisor-cpu-baseline is possible, but
it provide incorrect results and thus it should not be documented the
same way we describe the correct usage. Also using host-model CPU from
domain capabilities was not described clearly enough.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jiri Denemark [Wed, 19 Feb 2025 13:42:04 +0000 (14:42 +0100)]
docs: Clarify documentation of virsh hypervisor-cpu-compare
Using host CPU definition with hypervisor-cpu-compare is possible, but
it provide incorrect results and thus it should not be documented the
same way we describe the correct usage. Also using host-model CPU from
domain capabilities was not described clearly enough.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Fri, 21 Feb 2025 01:13:06 +0000 (20:13 -0500)]
schema: fix <interleave> errors when validating <domain> subelements
I first noticed a problem when I added a <memoryBacking> element at an
unusual (but still correct) place in the domain XML, and validation
failed. Then I tried adding that element in several different places
and it failed in many, but not all of them.
(NB: from here on, I will use '' for the names of attributes in the
domain XML, <> for elements in the domain XML, and "" for the names of
grammar rule definitions in the RNG file, and "<>" for the names of
elements in the RNG file's own XML. Confused yet? If so, please tell
me a better way - everything I know about RNG I've picked up
informally by looking at examples in already existing RNG files)
Starting from the top level of the grammar for <domain>
("domaincontents" in domaincommon.rng), I noticed that
1) the "<attribute>" for the 'id' attribute of <domain> is defined
inside an "<interleave>" down in the definition of "ids" (which is
referenced from "domaincontents") (I'm not familiar with the
nomenclature - does that make it a "sub-grammer", "child-grammar",
???)
2) although the definition of "ids", had all of its
"<attribute>"s/"<element>"s inside an "<interleave>",
"domaincontents" already had the reference to "ids" inside an
"<interleave>", so there were nested "<interleave>"s.
It's not clear to me how an "<attribute>" or "<interleave>" inside
another "<interleave>" is supposed to behave, but they both seemed a
bit suspicious.
I tried all of the below modifications:
1) moving the grammar for the 'id' attribute out of the "<interleave>"
but still inside "ids"
2) moving the grammer for the 'id' attribute directly into
"domaincontents" (and outside of its "interleave"
3) removing the "<interleave>" that was inside "ids"
4) (2) + (3)
5) move the entire grammar rule "ids" up directly in place of <ref
name="ids"> in "domaincontents".
6) (5), but with the grammar for the 'id' attribute moved outside of
the "<interleave>"
(6) was the only change that allowed all of the following (using
modifications to the subelements of <domain> in
net-vhostuser-passt.xml as example):
a) a <memoryBacking> element in between *any* two existing elements
b) moving <name> in between any two elements
c) oddly, in addition to the problem with putting <memoryBacking> in
odd places, I also found that the original RNG did not allow the
<clock> element to be placed in between <on_poweroff> and
<on_reboot>, but once I'd made the change in (6), this was no
longer problematic. Why should this have any effect? No idea, but
it works :-/
(NB: there are many other cases of referencing "sub-grammar" from
inside an "<interleave>", and they all seem to work just fine;
possibly in this case it was problematic because the sub-grammar a)
also contained an "<interleave>", b) had an "<attribute>" at its
toplevel, or c) had multiple "<element>"s.)
(inexplicably (to me) at one point during my experimentation, I tried
reordering the references to "clock", "resources", "features", and
"events", and that *also* made it legal to put a <clock> element in
between the <on_*> elements:-O)
Since I was no longer able to reproduce the error described in (c)
once I had made mod (6) (move all of "ids" directly into
"domaincontent", I decided it was pointless for me to spend any more
time randomly poking and just add that to the new test case for that
in case some other random change to the RNG causes it to start failing
again.
(I thought of writing a test program that would try all possible
orderings of the subelements of <domain>, but since doing that for
even 10 subelements would mean testing > 3.2 million different XML
documents, I decided we could continue in this adhoc manner, just
adding a single new test case if/when a new validation failure is
found.)
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Fri, 21 Feb 2025 04:24:05 +0000 (23:24 -0500)]
tests: be consistent about following DO_TEST_*() with a ;
As is often the case with macros (especially those that resolve to
multiple statements), it isn't technically necessary to end any of the
invocations of the DO_TEST_*() macros with a semicolon (as evidenced
by the lines changed in this path). Having does make some
auto-indenters (e.g. cc-mode in emacs) more likely to do the right
thing, though, and it also looks nicer if all the lines are similar.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Fri, 14 Feb 2025 15:30:58 +0000 (16:30 +0100)]
conf: Validate that iothreads are used only with 'virtio-scsi' controllers
The documentation states:
``iothread``
Supported for controller type ``scsi`` using model ``virtio-scsi`` for
``address`` types ``pci`` and ``ccw`` :since:`since 1.3.5 (QEMU 2.4)`. The
The code itself didn't validate if iothread is specified for any other
controller type.
Add test case showing the issue on one example.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 17 Feb 2025 15:11:23 +0000 (16:11 +0100)]
docs: formatdomain: Mention that vhostuser interface with mode='server' waits for connection
When starting a VM with a vhost-user interface in server mode qemu will
wait for the incoming connection without running CPUs. This isn't really
documented in our XML. Additionally when hotplugging the same interface
the above will not happen.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 17 Feb 2025 13:33:31 +0000 (14:33 +0100)]
qemuDomainGetStats: Convert worker functions to void
The presence of a return value made it seem that it's expected to fail
on errors which is not the case. The function is designed to skip
anything it can't fill and not fail when fetching individual stats.
Convert the workers to void to make it clear that it's expected not
to fail.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 17 Feb 2025 13:22:46 +0000 (14:22 +0100)]
qemuDomainGetStatsIOThread: Don't error out if fetching iothread info fails
The bulk domain stats API is meant to collect as much data as possible
without erroring out. Ignore errors from 'qemuDomainGetIOThreadsMon()'
and skip the data if an error happens.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 17 Feb 2025 13:19:54 +0000 (14:19 +0100)]
qemuDomainGetStatsPerfOneEvent: Ignore erros from 'virPerfReadEvent'
The bulk domain stats API is meant to collect as much data as possible
without erroring out. Skip the perf stats if we can't fetch them instead
of erroring out.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 17 Feb 2025 13:16:04 +0000 (14:16 +0100)]
virPerfReadEvent: Refactor to return -errno on failure
The function didn't comply with libvirt's error reporting scheme as it
reported libvirt errors only sometimes. As callers may want to ignore
errors convert it to returning -errno on failure instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
projects / thirdparty / libvirt.git / log
