Peter Krempa [Thu, 13 Oct 2022 12:01:24 +0000 (14:01 +0200)]
schema: Re-structure schema for <filesystem> to avoid broken validation
The validation of a '<filesystem type='mount'>' device fails if the
elements inside are not ordered in the order in the schema despite using
<interleave>. This is a bug in libxml2's validator as removing the
'<optional>' property from the definition of the 'type' attribute with
'mount' variable fixes the problem.
I've reported it as another instance of a seemingly related issue:
Meanwhile libvirt can re-arrange the schema by extracting the common
bits into a new definition and referencing them from each of the choice
groups explicitly.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/392 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
tests: refactor pseries features parse failure tests
The qemuxml2argvtest pseries-feature parse failure tests uses a symlink
to the pseries-features.xml test domain and control which feature it is
supposed to fail by excluding it from the capabilities list. The
advantage of this approach is that the same XML can be used in multiple
tests.
One downside is that any new pseries capability must be declared in all
existent tests, otherwise all other tests can break if this new
capability happens to be validated early in qemu_validate.c. Any new
parse_error test must declare all other existent capabilities.
Another downside is the fact that we're testing fairly improbable
scenarios: all pseries capabilities being tested here were introduced by
the same QEMU version, 4.2.0, at least as far as libvirt is aware of.
This means that it's no possible to have a scenario where, for example,
ccf-assist is not present but cfpc is. And last, but not the least, it's
getting in the way of our effort to convert all pseries tests to not use
explicit capabilities.
Changing all these tests to use DO_TEST_PARSE_ERROR_NOCAPS() will allow
us to test exactly what we want to test, which is the parse error given
for each feature if the binary does not have support for it. The XML
being used for each test can be simplified to just declare a single
feature. In the end we'll end up with more XML lines, but less
complexity inside qemuxml2argvtest.c.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
tests: change qemuxml2argv pseries tests to TEST_CAPS_LATEST
Eliminate most of the pseries tests that are using explicit qemu caps
instead of CAPS_LATEST. The remaining tests will be handled in the next
patch.
The changes made are trivial: rename the .args/.err files to
ppc64-latest.(args/err) and do the required changes to match the newest
capabilities.
A notable expection were tests pseries-console-native,
pseries-serial-compat and pseries-serial+console-native. These are
aliases of the pseries-serial-native tests. In this case we needed to
re-create the symlink accordingly.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Peter Krempa [Mon, 10 Oct 2022 08:51:01 +0000 (10:51 +0200)]
ci: Install python RPMs from libvirt/libvirt-python CI artifacts
After addition of the new libvirt-client-qemu sub-package which is using
python bindings (thus creating a circular dependency between the libvirt
and libvirt-python projects) the integration jobs fail with:
qemu: Init address before qemuProcessShutdownOrReboot during reconnect process
When libvirt is restarted, the qemuProcessShutdownReboot command is
executed to restore the VM that is being restarted. In this case, a
coredump may occur when we hotplug a pci device since the PCI address
hasn't be inited yet. Moving the initialization of address to the front
of qemuProcessShutdownOrReboot to ensure that we have the address inited.
Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Pierre LIBEAU [Wed, 31 Aug 2022 12:22:51 +0000 (14:22 +0200)]
qemu: Fix race condition when detaching a device
If QEMU replies to device_del command with "DeviceNotFound"
error, then libvirt doesn't clean the device from the live
configuration.
This is because qemuMonitorDelDevice() returns -2 to
qemuDomainDeleteDevice() and instead of calling
qemuDomainRemoveDevice() the qemuDomainDetachDeviceLive() jumps
right onto cleanup label.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/359 Signed-off-by: Pierre LIBEAU <pierre.libeau@corp.ovh.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Tue, 11 Oct 2022 07:27:24 +0000 (09:27 +0200)]
ci: potfile: Add 'variables' to definition
The 'potfile' job is lacking the 'variables' section which defines the
'NAME' variable used by the 'image' property resulting in the 'potfile'
job failing with:
Pulling docker image registry.gitlab.com/libvirt/libvirt/ci-:latest ...
WARNING: Failed to pull image with policy "always": invalid reference format (manager.go:235:0s)
ERROR: Job failed: failed to pull image "registry.gitlab.com/libvirt/libvirt/ci-:latest" with specified policies [always]: invalid reference format (manager.go:235:0s)
Fixes: 491d918502e50bf15c75d183bb41e3c0de5a0e1b Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Mon, 10 Oct 2022 13:37:17 +0000 (15:37 +0200)]
qemu: Avoid memory leak in virQEMUCapsCPUDefsToModels
The @vendor variable inside virQEMUCapsCPUDefsToModels() is
allocated, but never freed. But there is actually no need for it
to be allocated, because it merely passes a retval of
virCPUGetVendorForModel() (which returns a const string) to
virDomainCapsCPUModelsAdd() (which ten accepts the argument as
const string). Therefore, drop the g_strdup() call and fix the
type of the variable.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
meson: Replace external_program.path() with external_program.full_path()
The path() method is deprecated in 0.55.0 and we're recommended
to use full_path() instead. Interestingly, we were already doing
do in couple of places, but not all of them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Since commit "cpu_x86: Disable blockers from unusable CPU models"
(v3.8.0-99-g9c9620af1d) we explicitly disable CPU features reported by
QEMU as usability blockers for a particular CPU model when creating
baseline or host-model CPU definition. When QEMU changed canonical names
for some features (mostly those with '_' in their names), we forgot to
translate the blocker lists to names used by libvirt and the renamed
features would no longer be explicitly disabled in the created CPU model
even if they were reported as blockers by QEMU.
For example, on a host where EPYC CPU model has the following blockers
The 'monitor' feature is disabled even though it is not reported as a
blocker by QEMU because libvirt's definition of EPYC includes the
feature while it is missing in EPYC definition in QEMU.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jiri Denemark [Fri, 7 Oct 2022 16:36:50 +0000 (18:36 +0200)]
Document specifics of virConnectBaselineHypervisorCPU
The API can be used to get usability blockers for an unusable CPU model,
which is not obvious. Let's explicitly document this behavior as it is
now mentioned in the documentation of domain capabilities XML.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
This patch is effectively a no-op, but I wanted to initialize
.getVendorForModel explicitly as implementing this function does not
even make sense on ARM. The CPU models in our CPU map are only used for
describing host CPU in capabilities XML and cannot be used for guest CPU
definition in domain XML anyway. The CPU models listed as supported in
domain capabilities XML are just passed through from QEMU.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
domain_capabilities: Add vendor attribute for CPU models
Even though several CPU models from various vendors are reported as
usable on a given host, user may still want to use only those that match
the host vendor. Currently the only place where users can check the
vendor of each CPU model is our CPU map, which is considered internal
and users should not really be using it directly. So to allow for such
filtering we now advertise the vendor of each CPU model in domain
capabilities.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemu: Do not pass qemuCaps to virQEMUCapsCPUFeature{To,From}QEMU
The only part of qemuCaps both functions are interested in is the CPU
architecture. Changing them to expect just virArch makes the functions
more reusable.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
The ppc64 CPU code still has to load and parse the CPU map everytime it
needs to look at it, which can make some operations pretty slow. Other
archs already switched to loading the CPU map once and keeping the
parsed structure in memory. Let's switch ppc64 as well.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
In recent commit of v8.8.0-41-g41eb0f446c I've suggested during
review to put both xdr_free() calls under error label, assuming
that xdr_free() accepts NULL and thus is a NOP when the control
jumps onto the label even before either of @arg or @ret was
allocated. Well, turns out, xdr_free() does no accept NULL and
thus we have to guard its call. But since @dispatcher is already
set by the time either of the variables is allocated, we can
replace the condition from 'if (dispatcher)' to 'if (arg)' and
'if (ret)'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Since they are simply normal RPC messages, the keep alive packets are
subject to the "max_client_requests" limit just like any API calls.
Thus, if a client hits the 'max_client_requests' limit and all the
pending API calls take a long time to complete, it may result in
keep-alives firing and dropping the client connection.
This has been seen by a number of users with the default value of
max_client_requests=5, by issuing 5 concurrent live migration
operations.
By printing a warning message when this happens, admins will be alerted
to the fact that their active clients are exceeding the default client
requests limit.
Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
tools: add virt-qemu-qmp-proxy for proxying QMP via libvirt QEMU guests
Libvirt provides QMP passthrough APIs for the QEMU driver and these are
exposed in virsh. It is not especially pleasant, however, using the raw
QMP JSON syntax. QEMU has a tool 'qmp-shell' which can speak QMP and
exposes a human friendly interactive shell. It is not possible to use
this with libvirt managed guest, however, since only one client can
attach to the QMP socket at any point in time. While it would be
possible to configure a second QMP socket for a VM, it may not be
an known requirement at the time the guest is provisioned.
The virt-qmp-proxy tool aims to solve this problem. It opens a UNIX
socket and listens for incoming client connections, speaking QMP on
the connected socket. It will forward any QMP commands received onto
the running libvirt QEMU guest, and forward any replies back to the
QMP client. It will also forward back events.
$ virsh start demo
$ virt-qmp-proxy demo demo.qmp &
$ qmp-shell demo.qmp
Welcome to the QMP low-level shell!
Connected to QEMU 6.2.0
Note this tool of course has the same risks as the raw libvirt
QMP passthrough. It is safe to run query commands to fetch information
but commands which change the QEMU state risk disrupting libvirt's
management of QEMU, potentially resulting in data loss/corruption in
the worst case. Any use of this tool will cause the guest to be marked
as tainted as an warning that it could be in an unexpected state.
Since this tool introduces a python dependency it is not desirable
to include it in any of the existing RPMs in libvirt. This tool is
also QEMU specific, so isn't appropriate to bundle with the generic
tools. Thus a new RPM is introduced 'libvirt-clients-qemu', to
contain additional QEMU specific tools, with extra external deps.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This refresh switches the CI for contributors to be triggered by merge
requests. Pushing to a branch in a fork will no longer run CI pipelines,
in order to avoid consuming CI minutes. To regain the original behaviour
contributors can opt-in to a pipeline on push
git push <remote> -o ci.variable=RUN_PIPELINE=1
This variable can also be set globally on the repository, through the
web UI options Settings -> CI/CD -> Variables, though this is not
recommended. Upstream repo pushes to branches will run CI.
The use of containers has changed in this update, with only the upstream
repo creating containers, in order to avoid consuming contributors'
limited storage quotas. A fork with existing container images may delete
them. Containers will be rebuilt upstream when pushing commits with CI
changes to the default branch. Any other scenario with CI changes will
simply install build pre-requisite packages in a throaway environment,
using the ci/buildenv/ scripts. These scripts may also be used on a
contributor's local machines.
With pipelines triggered by merge requests, it is also now possible to
workaround the inability of contributors to run pipelines if they have
run out of CI quota. A project member can trigger a pipeline from the
merge request, which will run in context of upstream, however, note
this should only be done after reviewing the code for any malicious
CI changes.
Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Fri, 23 Sep 2022 13:03:19 +0000 (15:03 +0200)]
conf: savecookie: Remove virSaveCookieParseNode
The function provided just checking of the root XML node name which can
be easily moved into the caller wich doesn't do that already and
checking of the pointers which is trivial. Remove the helper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 23 Sep 2022 11:34:36 +0000 (13:34 +0200)]
conf: domain: Remove virDomainObjParseNode
virDomainObjParseFile is the only caller of virDomainObjParseNode.
The code can be merged into it, simplified by using virXMLParse and
the function removed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 22 Sep 2022 16:02:50 +0000 (18:02 +0200)]
conf: backup: Remove virDomainBackupDefParseNode
Rename virDomainBackupDefParse to virDomainBackupDefParseXML and use
it in place of virDomainBackupDefParseNode. This is possible as
virXMLParse can be used to replace XPath context allocation and root
node checking.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 22 Sep 2022 15:00:25 +0000 (17:00 +0200)]
conf: storage: Remove virStoragePoolDefParseNode
Replace it by proper use of virXMLParse to validate the root node and
allocate the context. The use in the test driver can be directly
replaced by virStoragePoolDefParseXML as both are validated.
The change to the storage driver isn't trivial though as it requires
careful xpath context juggling to parse the nested volumes properly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 22 Sep 2022 13:47:40 +0000 (15:47 +0200)]
virNWFilterBindingDefParse: Properly use virXMLParse
Fetch the XPath context and validate the node by using virXMLParse's
features.
This allows to completely remove virNWFilterBindingDefParseNode as
all callers now properly validate the root element name and have a XPath
context handy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
virVBoxSnapshotConfGetRWDisksPathsFromLibvirtXML and
virVBoxSnapshotConfGetRODisksPathsFromLibvirtXML were doing the same
thing, except for one XPath query.
Factor out the common code into a helper and bring it up to modern
standard.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 22 Sep 2022 12:30:53 +0000 (14:30 +0200)]
util: xml: Expose all arguments of virXMLParseHelper in virXMLParse macro
The generic helper also has helper code to validate the root element and
create an XPath context. Many places in the code duplicate code for
doing these operations.
Extend the helper to provide all arguments and fix all callers.
In many cases this patch refactors the passing of the 'validate'
field into a separate variable to avoid quirky looking arguments.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Erik Skultety [Thu, 6 Oct 2022 08:40:35 +0000 (10:40 +0200)]
ci: integration: Update the perl CI job names
libvirt-derived repos recently changed the way how and when CI
containers are built and for that a different naming scheme was adopted
to differentiate between the 2. Update the integration pipeline config
to reflect this change.
Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Stefan Berger [Tue, 4 Oct 2022 13:38:14 +0000 (09:38 -0400)]
qemu: tpm: Remove TPM state after successful migration
This patch 'fixes' the behavior of the persistent_state TPM domain XML
attribute that intends to preserve the state of the TPM but should not
keep the state around on all the hosts a VM has been migrated to. It
removes the TPM state directory structure from the source host upon
successful migration when non-shared storage is used. Similarly, it
removes it from the destination host upon migration failure when
non-shared storage is used.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Stefan Berger [Tue, 4 Oct 2022 13:38:13 +0000 (09:38 -0400)]
qemu: Add UNDEFINE_TPM and UNDEFINE_KEEP_TPM flags
Add UNDEFINE_TPM and UNDEFINE_KEEP_TPM flags to qemuDomainUndefineFlags()
API and --tpm and --keep-tpm to 'virsh undefine'. Pass the
virDomainUndefineFlagsValues via qemuDomainRemoveInactive()
from qemuDomainUndefineFlags() all the way down to
qemuTPMEmulatorCleanupHost() and delete TPM storage there considering that
the UNDEFINE_TPM flag has priority over the persistent_state attribute
from the domain XML. Pass 0 in all other API call sites to
qemuDomainRemoveInactive() for now.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virpcivpd: reduce errors in log due to invalid VPD
Sadly some devices provide invalid VPD data even with fully updated
firmware. Former hardning like 600f580d "PCI VPD: Skip fields with
invalid values" have already helped for those to some extent.
But if one happens to have such a device installed in the system,
despite all other things working properly the log potentially
flooded with messages like:
internal error: The keyword is not comprised only of uppercase ASCII
letters or digits
internal error: A field data length violates the resource length boundary.
The user can't do anything about it to change that, they will be there on
any libvirt restart and potentially distract from other more important
issues.
Since the vpd decoding is implemented rather resilient (if parsing fails
all goes on fine, the respective device just has no VPD data populated
eventually) we can lower those from virReportError(VIR_ERR_INTERNAL_ERROR
to just VIR_INFO. If needed for debugging people can set the level
accordingly, but otherwise we would no more fill the logs with errors
without a strong reason.
Fixes: https://launchpad.net/bugs/1990949 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Riscv64 usually uses u-boot as external -kernel and a loader from
the open implementation of RISC-V SBI. The paths for those binaries
as packaged in Debian and Ubuntu are in paths which are usually
forbidden to be added by the user under /usr/lib...
People used to start riscv64 guests only manually via qemu cmdline,
but trying to encapsulate that via libvirt now causes failures when
starting the guest due to the apparmor isolation not allowing that:
virt-aa-helper: error: skipped restricted file
virt-aa-helper: error: invalid VM definition
Explicitly allow the sub-paths used by u-boot-qemu and opensbi
under /usr/lib/ as readonly rules.
Fixes: https://launchpad.net/bugs/1990499 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It probes for the cssid-unrestricted property of
virtual-css-bridge, which was introduced in QEMU v2.12 by:
commit 99577c492fb2916165ed9bc215f058877f0a4106
s390x/css: unrestrict cssids
Since we bumped the minimum QEMU version to 4.2.0, assume
this property is always present.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
rpc: fix memory leak in virNetServerClientNew and virNetServerProgramDispatchCall
In virNetServerProgramDispatchCall, The arg is passed as a void*
and used to point to a certain struct depended on the dispatcher,
so I think it's the memory of the struct's member that leaks and
this memory shuld be freed by xdr_free.
In virNetServerClientNew, client->rx is assigned by invoking
virNetServerClientNew, but isn't freed if client->privateData's
initialization failed, which leads to a memory leak. Thanks to
Liang Peng's suggestion, put virNetMessageFree(client->rx) into
virNetServerClientDispose() to release the memory.
Signed-off-by: jiangjiacheng <jiangjiacheng@huawei.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Fri, 16 Sep 2022 15:56:59 +0000 (17:56 +0200)]
virDomainNetDefParseXML: Refactor parsing of <virtualport>
Base whether virtualport is supported for a given interface on a new
variable named 'virtualport_flags' which also configures the parser for
the virtualports subelement and fill it in the appropriate interface
type branches.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>