Peter Krempa [Tue, 30 Nov 2021 15:20:50 +0000 (16:20 +0100)]
qemuMonitorJSONGraphicsRelocate: Clean up command argument construction
Move the construction of the command from the variable declaration so
that it doesn't exceed the line length and we can also move the logic of
determining the protocol outside of the command construction.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 30 Nov 2021 15:12:00 +0000 (16:12 +0100)]
qemuMonitorJSONMigrate: Extract flags prior to constructing command
The migration API takes specific flags which are then converted to
boolean parameters for the command. Extract the flag into helper
variables rather than using ternary operators while constructing the
command itself.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 1 Dec 2021 08:21:33 +0000 (09:21 +0100)]
virLockDaemonClientPreExecRestart: Modernize JSON object construction
Use virJSONValueObjectAdd instead of step-by-step construction of the
object. This also removes a handful impossible to reach errors with
translatable messages.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Laine Stump [Thu, 2 Dec 2021 18:52:48 +0000 (13:52 -0500)]
util: call virNetDevGetPhysPortID() in less places
Whenever virPCIGetNetName() is called, it is either called with
physPortID = NULL, or with it set by the caller calling
virNetDevGetPhysPortID() soon before virPCIGetNetName(). The
physPortID is then used *only* in virPCIGetNetName().
Rather than replicating that same call to virNetDevGetPhysPortID() in
all the callers of virPCIGetNetName(), lets just have all those
callers send the NetDevName whose physPortID they want down to
virPCIGetNetName(), and let virPCIGetNetName() call
virNetDevGetPhysPortID().
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Laine Stump [Thu, 2 Dec 2021 19:13:18 +0000 (14:13 -0500)]
util: fix erroneous requirement for phys_port_id to get ifname of a VF
Commit 795e9e05c3 (libvirt-7.7.0) refactored the code in virpci.c and
virnetdev.c that gathered lists of the Virtual Functions (VF) of an
SRIOV Physical Function (PF) to simplify the code.
Unfortunately the simplification made the assumption, in the new
function virPCIGetVirtualFunctionsFull(), that a VF's netdev
interface name should only be retrieved if the PF had a valid
phys_port_id. That is an incorrect assumption - only a small handful
of (now previous-generation) Mellanox SRIOV cards actually use
phys_port_id (this is for an odd design where there are multiple
physical network ports on a single PCI address); all other SRIOV cards
(including new Mellanox cards) have a file in sysfs called
phys_port_id, but it can't be read, and so the pfPhysPortID string is
NULL.
The result of this logic error is that virtual networks that are a
pool of VFs to be used for macvtap connections will be unable to
start, giving an errror like this:
VF 0 of SRIOV PF enp130s0f0 couldn't be added to the interface pool because it isn't bound to a network driver - possibly in use elsewhere
This error message is misinformed - the caller of
virNetDevGetVirtualFunctionsFull() only *thinks* that the VF isn't
bound to a network driver because it doesn't see a netdev name for the
VF in the list. But that's only because
virNetDevGetVirtualFunctionsFull() didn't even try to get the names!
We do need a way for virPCIGetVirtualFunctionsFull() to sometimes
retrieve the netdev names and sometimes not. One way of doing that
would be to send down the netdev name of the PF whenever we also want
to know the netdev names of the VFs, but send a NULL when we
don't. This can conveniently be done by just *replacing* pfPhysPortID
in the arglist with pfNetDevName - pfPhysPortID is determined by
simply calling virNetDevGetPhysPortID(pfNetDevName) so we can just
make that call down in virPCIGetVirtualFunctionsFull() (when needed).
This solves the regression introduced by commit 795e9e05c3, and also
nicely sets us up to (in a subsequent commit) move the call to
virNetDevGetPhysPortID() down one layer further to virPCIGetNetName(),
where it really belongs!
Resolves: https://bugzilla.redhat.com/2025432 Fixes: 795e9e05c3b6b9ef3abe6f6078a6373a136ec23b Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The Homebrew package explicitly enables this driver despite us
disabling it by default on macOS, so it must be functional to
at least some extent and certainly can't be causing any build
failures.
Additionally, if the user has explicitly asked for the network
driver to be enabled but libvirtd is disabled for whatever
reason, we should error out instead of silently disabling the
network driver.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Joachim Falk [Thu, 2 Dec 2021 18:56:07 +0000 (19:56 +0100)]
Fix reboot command for LXC containers
The virNetDaemonQuit(dmn) command in virLXCControllerSignalChildIO triggers an
early close of all clients of lxc_controller. Here, libvirtd itself is a client
of this controller, and the client connection is used to notify libvirtd if a
reboot of the container is required. However, the client connection was closed
before such a status could be sent to libvirtd. To fix this bug, we will
immediately send the reboot or shutdown status of the container to libvirtd,
and only after client disconnect will we trigger virNetDaemonQuit.
Fixes: https://gitlab.com/libvirt/libvirt/-/issues/237
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=991773 Signed-off-by: Joachim Falk <joachim.falk@gmx.de> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jim Fehlig [Thu, 18 Nov 2021 19:03:20 +0000 (12:03 -0700)]
libxl: Protect access to libxlLogger files hash table
The hash table of log file objects in libxlLogger is not protected against
concurrent access. It is possible for one thread to remove an entry while
another is updating it. Add a mutex to the libxlLogger object and lock it
when accessing the files hash table.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jim Fehlig [Wed, 24 Nov 2021 18:48:51 +0000 (11:48 -0700)]
libxl: Search for virDomainObj in event handler threads
libxl can deliver events and invoke callbacks on any application thread
calling into libxl. This can cause deadlock in the libvirt libxl driver
Thread 19 (Thread 0x7f31411ec700 (LWP 14068) "libvirtd"):
#0 0x00007f318520cc7d in __lll_lock_wait () from /lib64/libpthread.so.0
#1 0x00007f3185205ed5 in pthread_mutex_lock () from /lib64/libpthread.so.0
#2 0x00007f3189488015 in virMutexLock (m=<optimized out>) at ../../src/util/virthread.c:79
#3 0x00007f3189463f3b in virObjectLock (anyobj=<optimized out>) at ../../src/util/virobject.c:433
#4 0x00007f31894f2f41 in virDomainObjListSearchID (payload=0x7f317400a6d0, name=<optimized out>, data=0x7f31411eaeac) at ../../src/conf/virdomainobjlist.c:105
#5 0x00007f3189437ac5 in virHashSearch (ctable=0x7f3124025a30, iter=iter@entry=0x7f31894f2f30 <virDomainObjListSearchID>, data=data@entry=0x7f31411eaeac, name=name@entry=0x0) at ../../src/util/virhash.c:745
#6 0x00007f31894f3919 in virDomainObjListFindByID (doms=0x7f3124025430, id=<optimized out>) at ../../src/conf/virdomainobjlist.c:121
#7 0x00007f3152f292e5 in libxlDomainEventHandler (data=0x7f3124023d80, event=0x7f310c010ae0) at ../../src/libxl/libxl_domain.c:660
#8 0x00007f3152c6ff5d in egc_run_callbacks (egc=egc@entry=0x7f31411eaf50) at libxl_event.c:1427
#9 0x00007f3152c718bd in libxl__egc_cleanup (egc=0x7f31411eaf50) at libxl_event.c:1458
#10 libxl__ao_inprogress (ao=ao@entry=0x7f310c00b8a0, file=file@entry=0x7f3152cce987 "libxl_domain.c", line=line@entry=730, func=func@entry=0x7f3152ccf750 <__func__.22238> "libxl_domain_unpause") at libxl_event.c:2047
#11 0x00007f3152c8c5b8 in libxl_domain_unpause (ctx=0x7f3124015a40, domid=<optimized out>, ao_how=ao_how@entry=0x0) at libxl_domain.c:730
#12 0x00007f3152f2a584 in libxl_domain_unpause_0x041200 (domid=<optimized out>, ctx=<optimized out>) at /usr/include/libxl.h:1756
#13 libxlDomainStart (driver=driver@entry=0x7f3124023d80, vm=vm@entry=0x7f317400a6d0, start_paused=start_paused@entry=false, restore_fd=restore_fd@entry=-1, restore_ver=<optimized out>, restore_ver@entry=2) at ../../src/libxl/libxl_domain.c:1482
#14 0x00007f3152f2a6e3 in libxlDomainStartNew (driver=driver@entry=0x7f3124023d80, vm=vm@entry=0x7f317400a6d0, start_paused=start_paused@entry=false) at ../../src/libxl/libxl_domain.c:1545
#15 0x00007f3152f2a789 in libxlDomainShutdownHandleRestart (driver=0x7f3124023d80, vm=0x7f317400a6d0) at ../../src/libxl/libxl_domain.c:464
#16 0x00007f3152f2a9e4 in libxlDomainShutdownThread (opaque=<optimized out>) at ../../src/libxl/libxl_domain.c:559
#17 0x00007f3189487ee2 in virThreadHelper (data=<optimized out>) at ../../src/util/virthread.c:196
#18 0x00007f3185203539 in start_thread () from /lib64/libpthread.so.0
#19 0x00007f3184f3becf in clone () from /lib64/libc.so.6
Frame 16 runs a thread created to handle domain shutdown processing for
domid 28712. In this case the event contained the reboot reason, so the
old domain is destroyed and a new one is created by libxlDomainStart new.
After starting the domain, it is unpaused by calling libxl_domain_unpause
in frame 12. While the thread is running within libxl, libxl takes the
opportunity to deliver a pending domain shutdown event for unrelated domid
28710. While searching for the associated virDomainObj by ID, a deadlock is
encountered when attempting to lock the virDomainObj for domid 28712, which
is already locked since this thread is processing its shutdown event.
The deadlock can be avoided by moving the search for a virDomainObj
associated with the event domid to the shutdown thread. The same is done
for the death thread.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jim Fehlig [Wed, 24 Nov 2021 18:36:55 +0000 (11:36 -0700)]
libxl: Handle domain death events in a thread
Similar to domain shutdown events, processing domain death events can be a
lengthy process and we don't want to block the event handler while the
operation completes. Move the death handling function to a thread.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jim Fehlig [Wed, 24 Nov 2021 18:16:38 +0000 (11:16 -0700)]
libxl: Modify name of shutdown thread
The current thread name 'ev-<domid>' is a bit terse. Change the name
to 'shutdown-event-<domid>', allowing it to be distinguished between
thread handling other event types.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jim Fehlig [Wed, 24 Nov 2021 18:10:19 +0000 (11:10 -0700)]
libxl: Rename libxlShutdownThreadInfo struct
An upcoming change will use the struct in a thread created to process
death events. Rename libxlShutdownThreadInfo to libxlEventHandlerThreadInfo
to reflect the more generic usage.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jim Fehlig [Fri, 29 Oct 2021 20:16:33 +0000 (14:16 -0600)]
libxl: Disable death events after receiving a shutdown event
The libxl driver will handle all domain destruction and cleanup
when receiving a domain shutdown event from libxl. Commit fa30ee04a2a
introduced the ignoreDeathEvent boolean in the DomainObjPrivate struct
to ignore subsequent death events from libxl. But libxl already provides
a mechanism to disable death events via libxl_evdisable_domain_death.
This patch partially reverts commit fa30ee04a2a and instead uses
libxl_evdisable_domain_death to disable subsequent death events when
processing a shutdown event.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
In qemuConnectGetAllDomainStats() there a loop that iterates over
all domains that stats are to be fetched for. Within this loop
the qemuDomainGetStats() is called which is responsible for
fetching stats for an individual domain. Now, the code that
handles successful and failure cases is almost the same. Rework
it, so that the code is deduplicated. Note, that the check for
!tmp is dropped because upon successful return from
qemuDomainGetStats() it is always allocated.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Thu, 11 Nov 2021 10:20:29 +0000 (11:20 +0100)]
qemu: prefer .requiredCaps for VIR_DOMAIN_STATS_IOTHREAD
Since f29d7c3e698 we have an option for checking capabilities
required for given type of statistics upfront, instead of the
callback. Switch qemuDomainGetStatsIOThread() callback to the new
style.
This will now error out properly if user requests IOTHREAD stats
forcibly (via VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS
flag) but QEMU doesn't support IOThreads. Previously, this was
silently ignored.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Thu, 11 Nov 2021 10:17:26 +0000 (11:17 +0100)]
qemu: Drop comma after QEMU_CAPS_LAST in queryDirtyRateRequired[]
The idea of queryDirtyRateRequired[] is that it lists QEMU
capabilities required for given domstats record
(VIR_DOMAIN_STATS_DIRTYRATE in this particular case) and
QEMU_CAPS_LAST is used as a sentinel. Therefore, there can never
be anything after it. Drop the comma to make it more obvious.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
virnetdevveth: Do report error if creating veth fails
For some weird reason we are ignoring errors when creating veth
pair that netlink reports. This affects the LXC driver which
creates interfaces for container in
virLXCProcessSetupInterfaces(). If creating a veth pair fails, no
error is reported and the control jumps onto cleanup label where
some cryptic error message is reported instead (something about
inability to remove veth pair).
Let's report error that netlink returned - it's probably the most
accurate reason anyways.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/225 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Jiri Denemark [Fri, 26 Nov 2021 16:23:03 +0000 (17:23 +0100)]
virnetserver: Make pool job name less generic
The generic "rpc-worker" name becomes a name of the associated task,
which may than appear in logs and bring some confusion. Let's add a
server name to it so that one can easily see which daemon the task
belongs to, which is especially useful for split daemons. And since the
name would be too long, we can drop the "-worker" part and just keep it
as "rpc-*" and "prio-rpc-*".
Such confusing entries can, for example, be found in audit log when
SELinux is complaining that "rpc-worker" was denied access to something.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jiri Denemark [Fri, 26 Nov 2021 15:58:07 +0000 (16:58 +0100)]
virnetserver: Format functions consistently
The file used a pretty inconsistent style for formatting function
headers. Return types were both separate and on the same line as
function names and functions were separated by one, two, and sometimes
even three empty lines. Let's make it consistent by honoring our
preferred coding style.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Jiri Denemark [Fri, 26 Nov 2021 15:22:43 +0000 (16:22 +0100)]
virthreadpool: Copy job name
Currently virThreadPoolNewFull relies on the caller to ensure the job
name outlives the thread pool. Which basically enforces static strings.
Let's drop this implicit requirement by making a copy of the job name.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Fri, 26 Nov 2021 12:21:47 +0000 (13:21 +0100)]
virStorageSourceIsSameLocation: Special-case storage sources of type 'volume'
The function is used also to compare virStorageSource which may not be
resolved to the image at that point in which case the 'path' is not yet
populated and the actual type is not yet set. This means that the
function fails to consider two identical volume-based disks as pointing
to the same thing.
Add a special case for both images being type=volume in which case we
compare only the pool/volume names.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/240 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Peter Krempa [Wed, 24 Nov 2021 16:03:07 +0000 (17:03 +0100)]
internal: Add STRLIM macro for checking string length using strnlen()
As a microoprimization when checking whether length of a string fits
into a limit we don't necessarily need to calculate the full length but
can use strnlen to check only LIMIT+1 chars. Add a macro which will
simplify the expressions.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
The run script tries to detect when a daemon is being run in order to
shutdown other systemd unit files that clash. As implemented this
only works if the daemon name is the first argument. This won't be the
case if running via GDB or strace eg
The domain capabilities won't report TPM support unless SWTPM can be
initialized. To avoid relying on the swtpm install in the host, mock
the entire initialization method, since all it needs todo is return
a non-error value.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
util: pull TPM capabilities probing out of main init method
Many methods merely want to know that the swtpm binaries have been
found, and don't care about probing for capabilities. Even when
starting a guest, the QEMU driver may not need the capabilities.
Skipping probing ensures the VM startup path is as fast as possible
when capabilities are not required. It also removes various error
scenarios from the main init method.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
util: ensure all TPM global vars access is protected by lock
The virTPMEmulatorInit method updates various global variables
and holds a lock while doing so. Other methods which access
these variables, however, don't reliably hold locks over all
of their accesses.
Since virTPMEmulatorInit is no longer exported, we can push
the locking up into all the callers and achieve proper safety
for concurrent usage.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
util: replace TPM global variables with a struct array
The virTPMEmulatorInit function defines a struct that gets filled with
pointers to global variables. It will be simpler to just use the struct
for the global variables directly.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
util: refactor TPM helper methods to reduce duplicationm
The TPM helper methods for querying the binary path and capabilities
have the same patterns across all swtpm binaries. This code duplication
can be reduced by introducing helper methods.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
projects / libvirt.git / log