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>
qemu: don't strip audio elements with user config present
To support backwards live migration we must strip the default added
audio element, however, we are too aggressive in doing so. We are only
comparing a couple of attributes for equality, so risk stripping config
that was user customized. To improve this we need to a deep comparison
of the audio config.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Ján Tomko [Wed, 24 Nov 2021 12:44:56 +0000 (13:44 +0100)]
qemu: qemuDomainObjExitMonitor: do not warn on unused result
This wrapper for qemuDomainObjExitMonitorInternal was
extended by my commit dc2fd51fd727bbb6de172e0ca4b7dd307bb99180
to check whether the domain is still alive, because
we were observing crashes if the QEMU process died
while some of our APIs were in the monitor and the thread
processing the EOF event freed the domain definition.
This bug was fixed by:
commit 81f50cb92d16643bcd749e3ab5b404b8b7cec643
qemu: Avoid calling qemuProcessStop without a job
but we kept checking for the return value since.
Remove the G_GNUC_WARN_UNUSED_RESULT attribute since
all of the calls that could set def->id to -1 are protected
by qemuProcessBeginStopJob and cannot happen while we have a job
in the monitor.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
tools: fix iterating over argv when recovering xattr
The libvirt_recover_xattrs.sh tool hangs when run. When no flags
are provided OPTIND is 1, so the loop expands to 'shift 0' which
has not effect. Rewrite to just loop over $@ instead which involves
less cleverness.
Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Mon, 29 Nov 2021 09:20:05 +0000 (10:20 +0100)]
wireshark: Drop needless comment in dissect_xdr_bytes()
In the dissect_xdr_bytes() there's a comment that the string
allocated by xdr_bytes() can't be freed using xdr_free(). Well,
that is expected because xdr_bytes() used plain calloc() AND the
string is not an XDR struct but plain 'char *' type. Passing it
to xdr_free() must result in weird things happening.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Michal Privoznik [Mon, 29 Nov 2021 08:57:49 +0000 (09:57 +0100)]
wireshark: Switch to tvb_bytes_to_str()
When the dissector sees a byte sequence that is either an opaque
data (xdr_opaque) or a byte sequence (xdr_bytes) it formats the
bytes as a hex numbers using our own implementation. But
wireshark already provides a function for it: tvb_bytes_to_str().
NB, the reason why it returns a const string is so that callers
don't try to free it - the string is allocated using an allocator
which will decide when to free it.
The wireshark formatter was introduced in wireshark commit of
v1.99.2~479 and thus is present in the version we require at
least (2.6.0).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Peter Krempa [Thu, 25 Nov 2021 15:17:16 +0000 (16:17 +0100)]
qemu: Fix validation of PCI option rom settings on hotplug
Commit 24be92b8e moved the option rom settings validation code to the
validation callbacks, but that doesn't work properly with device hotplug
as we assign addresses only after parsing the whole XML. The check is
too strict for that and caused failures when hotplugging devices such
as:
This patch relaxes the check in the validation callback to accept also
_NONE and _UNASSIGNED address types and returns the check to
'qemuBuildRomProps' so that we preserve the full validation as we've
used to.
Fixes: 24be92b8e38762e9ba13e
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2021437 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 24 Nov 2021 16:22:29 +0000 (17:22 +0100)]
qemu: monitor: Fix usage of 'query-blockstats'
Commit bc24810c2cab modified code querying blockstats to use the
'query-nodes' parameter so that we can fetch stats also for images which
are not attached to a frontend such as block copy and backup scratch
images.
Unfortunately that broke the old blockstats because if 'query-nodes' is
enabled qemu doesn't output the 'qdev' parameter which our code used for
matching to the disk and also qemu neglects to populate the frontend
stats at all so we can't even switch to using nodename for matching.
To fix this we need to do two calls, one with 'query-nodes' disabled
using the old logic to populate everything and then an additional one
which populates all the remaining images.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/246 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Tested-by: Erik Skultety <eskultet@redhat.com>
projects / libvirt.git / log