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>
VIR_XPATH_NODE_AUTORESTORE_NAME is a more generic version of the
VIR_XPATH_NODE_AUTORESTORE macro used to save the 'node' inside a XPath
context struct. The new macro allows specifying the name of the variable
used to save the context so that it can be used multiple times inside a
function's nested scopes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 16 Sep 2022 13:50:54 +0000 (15:50 +0200)]
virDomainNetDefParseXML: Extract network device model earlier
Move the code fetching the model of the net device before the main code
parsing individual device types so that the data is available before the
upcoming refactor.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 15 Sep 2022 16:13:30 +0000 (18:13 +0200)]
virDomainNetDefParseXML: Convert parsing of 'source_node' to a switch() statement
Convert the individual 'if' clauses to a switch statement.
By moving the check that 'source_node' is non-null inside of each case
rather we will be able to move more type specific code into the switch
statement when it will be refactored in subsequent patches.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 16 Sep 2022 07:14:42 +0000 (09:14 +0200)]
util: xml: Adjust documentation of virXMLPropString
All callers treat NULL as if the string is not present in the XML.
Adjust the description so that it's implied that it's not an error and
thus also no error reporting is expected.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 15 Sep 2022 16:04:12 +0000 (18:04 +0200)]
virDomainNetIPInfoParseXML: Don't force callers to set proper 'ctxt->node'
In certain cases it's inconvenient to move the XPath's context current
node in the caller. Add a 'node' argument and override it inside the
function. VIR_XPATH_NODE_AUTORESTORE handles the cleanup.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Pavel Hrdina [Mon, 26 Sep 2022 16:33:20 +0000 (18:33 +0200)]
spec: add systemd-rpm-macros build dependency
We should explicitly depend on this package as we use some macros
defined by files that are installed from systemd-rpm-macros.
Currently the build works only because some of our other build
dependencies pull in systemd which will install systemd-rpm-macros if
rpm-build is installed as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Wed, 28 Sep 2022 09:05:47 +0000 (11:05 +0200)]
virgdbusmock: Mock g_dbus_connection_call_with_unix_fd_list_sync() only on UNIX-like systems
As of GLib commit c9874f817be51d3b652457daef3b10766b59db9a,
relesed in GLib 2.73, the g_dbus_connection_call_with_unix_fd_list_sync()
function is not defined always, only when G_OS_UNIX condition is
set. This is also the reason why our virGDBusCallMethodWithFD()
function is guarded with that macro. But virgdbusmock is compiled
always, even on !UNIX systems, e.g. mingw. Therefore, we need to
use the same macro to condition compile its mock.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
virdomainjob: check if cb is not NULL before dereferencing
The callback struct does not always have to be set which could
cause a dereferencing of a NULL pointer. This patch adds check
against NULL in missing places before dereferencing.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Wed, 28 Sep 2022 08:12:36 +0000 (10:12 +0200)]
qemuProcessReconnect: Don't build memory paths
Let me take you on a short trip to history. A long time ago,
libvirt would configure all QEMUs to use $hugetlbfs/libvirt/qemu
for their hugepages setup. This was problematic, because it did
not allow enough separation between guests. Therefore in
v3.0.0-rc1~367 the path changed to a per-domain basis:
$hugetlbfs/libvirt/qemu/$domainShortName
And to help with migration on daemon restart a call to
qemuProcessBuildDestroyMemoryPaths() was added to
qemuProcessReconnect() (well, it was named
qemuProcessBuildDestroyHugepagesPath() back then, see
v3.10.0-rc1~174). This was desirable then, because the memory
hotplug code did not call the function, it simply assumes
per-domain paths to exist. But this changed in v3.5.0-rc1~92
after which the per-domain paths are created on memory hotplug
too.
Therefore, it's no longer necessary to create these paths in
qemuProcessReconnect(). They are created exactly when needed
(domain startup and memory hotplug).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Wed, 28 Sep 2022 07:53:47 +0000 (09:53 +0200)]
qemuNamespaceMknodOne: Call g_file_read_link() in async-signal-safe fashion
When creating a node in QEMU's namespace the whole link chain is
created with it. Here, we use g_file_read_link() from the child
(running inside the namespace) to learn whether a link exists and
points to expected target. Now, when building the namespace there
can't be any symlinks and this g_file_read_link() returns NULL
always. And because we pass a local GError variable to it, glib
tries to set it to a localized error message. This comes with
creating a (static) hash table inside of g_strerror() and is
guarded with a mutex. The hash table is also allocated using
GSlice allocator instead of g_malloc, and since the latter is
safe to use after fork (because it's documented to use plain
malloc), glib went with the former, naturally. Now, GSlice
allocator has plenty of internal mutexes and thus hitting a
locked mutex is not that hard.
Fortunately, we don't care about any error from
g_file_read_link() and thus we can pass NULL which avoids calling
g_strerror().
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2120965 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Michal Privoznik [Wed, 28 Sep 2022 07:47:37 +0000 (09:47 +0200)]
qemuNamespaceMknodPaths: Don't fork needlessly
The qemuNamespaceMknodPaths() function is responsible for
creating files/directories in QEMU's mount namespace. When
called, it is given list of paths that have to be created in the
namespace. It processes this list and removes items that are not
directly under /dev, but on a 'shared' filesystem (note that all
other mount points are preserved). And it may so happen that
after this pre-process no files/directories need to be created in
the namespace. If that's the case, exit early and avoid
fork()-ing only to find out the same.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Lin Ma [Thu, 8 Sep 2022 07:07:22 +0000 (15:07 +0800)]
qemu: Remove host-passthrough validation check for host-phys-bits=on
Besides the -cpu host, The host-phys-bits=on applies to custom or max
cpu model, So the host-passthrough validation check is unnecessary for
maxphysaddr with mode='passthrough'.
Signed-off-by: Lin Ma <lma@suse.com> Reviewed-by: Jim Fehlig <jfehlig@suse.com>
virdomainjob: preserveJob: memdup the cb structure instead of copying it
In case of variable 'oldjob' (job structure) in
qemuProcessReconnect() the cb pointer was just copied from the
existing job structure in virDomainObjPreserveJob(). This caused
the job and oldjob sharing the same pointer, which was later
freed at the end of the qemuProcessReconnect() function by
automatic call to virDomainObjClearJob(). This caused an invalid
read in and subsequent daemon crash as the job structure was
trying to read cb which had been already freed.
This patch changes the copying to g_memdup that allocates
different pointer, which can be later safely freed.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
rpmbuild is complaining it's not recommended to have unversioned
Obsoletes. On the other hand using dynamic version/release is a bit too
much as we know in which release a particular subpackage was removed.
Let's just use the corresponding version in both cases to be consistent
with all other Obsoletes in our spec file.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
libvirt-guests: Fix dependency ordering in service file
After some debugging and discussion with systemd team it turns out we
are misusing the ordering in libvirt-guests.service. That happened
because we want to support both monolithic and modular daemon setups and
on top of that we also want to support socket activation and services
without socket activation. Unfortunately this is impossible to express
in the unit file because of how transactions are handled in systemd when
dependencies are resolved and multiple actions (jobs) are queued. For
explanation from Michal Sekletar see comment #7 in the BZ this patch is
fixing:
In order to support all the scenarios this patch also amends the
manpages so that users that are changing the default can also read how
to correct the dependency ordering in libvirt-guests unit file.
Ideally we would also keep the existing configuration during upgrade,
but due to our huge support matrix this seems hardly feasible as it
could introduce even more problems.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
qemu_process.c: Propagate hugetlbfs mounts on reconnect
When reconnecting to a running QEMU process, we construct the
per-domain path in all hugetlbfs mounts. This is a relict from
the past (v3.4.0-100-g5b24d25062) where we switched to a
per-domain path and we want to create those paths when libvirtd
restarts on upgrade.
And with namespaces enabled there is one corner case where the
path is not created. In fact an error is reported and the
reconnect fails. Ideally, all mount events are propagated into
the QEMU's namespace. And they probably are, except when the
target path does not exist inside the namespace. Now, it's pretty
common for users to mount hugetlbfs under /dev (e.g.
/dev/hugepages), but if domain is started without hugepages (or
more specifically - private hugetlbfs path wasn't created on
domain startup), then the reconnect code tries to create it.
But it fails to do so, well, it fails to set seclabels on the
path because, because the path does not exist in the private
namespace. And it doesn't exist because we specifically create
only a subset of all possible /dev nodes. Therefore, the mount
event, whilst propagated, is not successful and hence the
filesystem is not mounted. We have to do it ourselves.
If hugetlbfs is mount anywhere else there's no problem and this
is effectively a dead code.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2123196 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Sometimes it may come handy to just bind mount a directory/file
into domain's namespace. Implement a thin wrapper over
qemuNamespaceMknodPaths() which has all the logic we need.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
qemu_namespace: Fix a corner case in qemuDomainGetPreservedMounts()
When setting up namespace for QEMU we look at mount points under
/dev (like /dev/pts, /dev/mqueue/, etc.) because we want to
preserve those (which is done by moving them to a temp location,
unshare(), and then moving them back). We have a convenience
helper - qemuDomainGetPreservedMounts() - that processes the
mount table and (optionally) moves the other filesystems too.
This helper is also used when attempting to create a path in NS,
because the path, while starting with "/dev/" prefix, may
actually lead to one of those filesystems that we preserved.
And here comes the corner case: while we require the parent mount
table to be in shared mode (equivalent of `mount --make-rshared /'),
these mount events propagate iff the target path exist inside the
slave mount table (= QEMU's private namespace). And since we
create only a subset of /dev nodes, well, that assumption is not
always the case.
For instance, assume that a domain is already running, no
hugepages were configured for it nor any hugetlbfs is mounted.
Now, when a hugetlbfs is mounted into '/dev/hugepages', this is
propagated into the QEMU's namespace, but since the target dir
does not exist in the private /dev, the FS is not mounted in the
namespace.
Fortunately, this difference between namespaces is visible when
comparing /proc/mounts and /proc/$PID/mounts (where PID is the
QEMU's PID). Therefore, if possible we should look at the latter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
qemu_namespace: Tolerate missing ACLs when creating a path in namespace
When creating a path in a domain's mount namespace we try to set
ACLs on it, so that it's a verbatim copy of the path in parent's
namespace. The ACLs are queried upfront (by
qemuNamespaceMknodItemInit()) but this is fault tolerant so the
pointer to ACLs might be NULL (meaning no ACLs were queried, for
instance because the underlying filesystem does not support
them). But then we take this NULL and pass it to virFileSetACLs()
which immediately returns an error because NULL is invalid value.
Mimic what we do with SELinux label - only set ACLs if they are
non-NULL which includes symlinks.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Michal Privoznik [Fri, 23 Sep 2022 13:06:19 +0000 (15:06 +0200)]
qemu_domain: Ignore all but SCSI hostdevs in qemuDomainDeviceHostdevDefPostParseRestoreBackendAlias()
When retiring QEMU_CAPS_BLOCKDEV_HOSTDEV_SCSI capability the
commit removed a bit too much. Previously, all other devices than
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI were ignored in
qemuDomainDeviceHostdevDefPostParseRestoreBackendAlias(). But the
commit in question removed not only the capability check but also
this return early statement. Restore it back.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2129239 Fixes: dc8dbb27d40968c9d9bfad2c6181bccc20c0e44e Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Michal Privoznik [Wed, 21 Sep 2022 13:56:13 +0000 (15:56 +0200)]
security_selinux: Don't ignore NVMe disks when setting image label
For NVMe disks we skip setting SELinux label on corresponding
VFIO group (/dev/vfio/X). This bug is only visible with
namespaces and goes as follows:
1) libvirt assigns NVMe disk to vfio-pci driver,
2) kernel creates /dev/vfio/X node with generic device_t SELinux
label,
3) our namespace code creates the exact copy of the node in
domain's private /dev,
4) SELinux policy kicks in an changes the label on the node to
vfio_device_t (in the top most namespace),
5) libvirt tells QEMU to attach the NVMe disk, which is denied by
SELinux policy.
While one can argue that kernel should have created the
/dev/vfio/X node with the correct SELinux label from the
beginning (step 2), libvirt can't rely on that and needs to set
label on its own.
Surprisingly, I already wrote the code that aims on this specific
case (v6.0.0-rc1~241), but because of a shortcut we take earlier
it is never ran. The reason is that
virStorageSourceIsLocalStorage() considers NVMe disks as
non-local because their source is not accessible via src->path
(or even if it is, it's not a local path).
Therefore, do not exit early for NVMe disks and let the function
continue.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2121441 Fixes: 284a12bae0e4cf93ea72797965d6c12e3a103f40 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
* UUID
* CPU model, vendor, microcode, signature, counters,
topology, maxphysaddr, features,
* Power management
* NUMA page size info, multiple nodes, CPU topology IDs, distances
* CPU cache bank info
* Multiple secmodels
Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Mon, 19 Sep 2022 08:18:14 +0000 (10:18 +0200)]
virdomainjob: virDomainObjInitJob: Avoid borrowing memory from 'virDomainXMLOption'
The 'cb' and 'jobDataPrivateCb' pointers are stored in the job object
but made point to the memory owned by the virDomainXMLOption struct in
the callers.
Since the 'virdomainjob' module isn't in control the lifetime of the
virDomainXMLOption, which in some cases is freed before the domain job
data, freed memory would be dereferenced in some cases.
Copy the structs from virDomainXMLOption to ensure the lifetime. This is
possible since the callback functions are immutable.
Fixes: 84e9fd068ccad6e19e037cd6680df437617e2de5 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
And the rest of distros has something newer anyways. Requiring
0.8.1 or newer allows us to drop the terrible hack where we
rename functions at meson level using #define. Note, 0.8.0 is
the version of libssh where the rename happened. It also allows
us to stick with SHA-256 hash algorithm for public keys.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 9 Sep 2022 13:57:40 +0000 (15:57 +0200)]
docs: compiling: Add notes on starting compiled libvirt daemons
In the basic configuration with monolithic libvirtd users are required
to also start virtlogd. Add a general note with a specific example
hinting that this is needed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 9 Sep 2022 13:33:25 +0000 (15:33 +0200)]
docs: compiling: Separate 'prepare', 'configure', and 'build' steps
Only the preparation of sources differs between a build from a git
checkout vs a build from tarball. Restructure the docs to outline the
difference and combine information on how to configure libvirt.
Most notably the suggestion to use '-Dsystem=true' was present only for
the steps to build a git checkout.
Suggest also running the testsuite as part of the build step.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 8 Sep 2022 15:04:01 +0000 (17:04 +0200)]
remote: Don't attempt remote connection from libvirtd
When a hypervisor driver is not compiled in and a user enables the
monolithic libvirtd, they get the following misleading error:
$ virsh -c qemu:///system
error: failed to connect to the hypervisor
error: Failed to connect socket to '/var/run/libvirt/virtqemud-sock': No such file or directory
The issue is that the daemon side of the remote driver can't find the
appropriate driver, but the remote driver always accepts everything and
thus attempts to delegate further, which in case of libvirtd makes no
sense.
Refuse opening a connection for local URIS even when the requested
driver is not registered in case when we are inside 'libvirtd' as
libvirtd doesn't have anything to delegate to.
$ virsh -c qemu:///system
error: failed to connect to the hypervisor
error: no connection driver available for qemu:///system
Discovered when investigating https://gitlab.com/libvirt/libvirt/-/issues/370
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 9 Sep 2022 11:16:42 +0000 (13:16 +0200)]
virStateInitialize: Propagate whether running in monolithic daemon mode to stateful driver init
Upcoming patch which is fixing the opening of drivers in monolithic mode
needs to know whether we are inside 'libvirtd' but the code where the
decision needs to happen is not re-compiled per daemon. Thus we need to
pass this information to the stateful driver init function so that it
can be remebered.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
1) the mount points are set before starting a guest,
2) the / and its submounts are marked as shared, so that mount
events propagate into child namespaces when assumption 1) is
not held.
But what's obvious to me might not be obvious to our users.
Document these known limitations.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2123196 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
projects / libvirt.git / log