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>
qemu: Remove 'else' branches after 'return' or 'goto'
I think it makes no sense to have else branches after return or
goto as it will never reach them in cases it should not. This
patch makes the code more readable (at least to me).
virDomainObjListAdd: Transfer definition ownership
Upon successful return from virDomainObjListAdd() the
virDomainObj is the owner of secret definition. To make this
ownership transfer even more visible, lets pass the definition as
a double pointer and use g_steal_pointer().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Michal Privoznik [Tue, 23 Nov 2021 16:09:59 +0000 (17:09 +0100)]
virStoragePoolObjListAdd: Transfer definition ownership
Upon successful return from virStoragePoolObjListAdd() the
virStoragePoolObj is the owner of secret definition. To make this
ownership transfer even more visible, lets pass the definition as
a double pointer and use g_steal_pointer().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Michal Privoznik [Tue, 23 Nov 2021 16:03:44 +0000 (17:03 +0100)]
virSecretObjListAdd: Transfer definition ownership
Upon successful return from virSecretObjListAdd() the
virSecretObj is the owner of secret definition. To make this
ownership transfer even more visible, lets pass the definition as
a double pointer and use g_steal_pointer().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
virInterfaceObjListAssignDef: Transfer definition ownership
Upon successful return from virInterfaceObjListAssignDef() the
virInterfaceObj is the owner of secret definition. To make this
ownership transfer even more visible, lets pass the definition as
a double pointer and use g_steal_pointer().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
util: fix cache invalidation of swtpm capabilities
The check for whether the swtpm binary was modified is checking pointers
to the mtime field in two distinct structs, so will always compare
different. This resulted in re-probing swtpm capabilities every time,
as many as 20 times for a single VM launch.
tpm: Check whether previously found executables were updated
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When a build fails it is helpful to know what packages were installed,
because by the time we look at the build job output, the original
container image might have changed.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Xu Chao [Wed, 24 Nov 2021 02:33:11 +0000 (10:33 +0800)]
util: virExec may blocked by reading pipe if grandchild prematurely exit
When VIR_EXEC_DAEMON is set, if virPidFileAcquirePath/virSetInherit failed,
then pipesync[0] can not be closed when granchild process exit, because
pipesync[1] still opened in child process. and then saferead in child
process may blocked forever, and left grandchild process in defunct state.
Signed-off-by: Xu Chao <xu.chao6@zte.com.cn> Signed-off-by: Yi Wang <wang.yi59@zte.com.cn> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Mon, 22 Nov 2021 16:44:49 +0000 (17:44 +0100)]
util: xml: Remove virXMLPropStringLimit and virXPathStringLimit
The functions have very difficult semantics where callers are not able
to tell whether the property is missing or failed the length check. Only
the latter produces errors.
Since usage of the functions was phased out, remove them completely to
avoid further broken code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
projects / libvirt.git / log