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>
Peter Krempa [Mon, 22 Nov 2021 15:33:37 +0000 (16:33 +0100)]
virSecurityLabelDefParseXML: Directly assign strings into appropriate variables
'seclabel->label', 'seclabel->imagelabel' and 'seclabel->baselabel' are
populated by stealing the pointer from the 'p' temporary string. Remove
the extra step.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
The cpuset_getaffinity() function is checked in sys/cpuset.h to see if
BSD CPU affinity APIs are available. This check requires including
sys/param.h to work properly, otherwise the test program fails with
unrelated errors like:
/usr/include/sys/cpuset.h:155:1: error: unknown type name
'__BEGIN_DECLS'
__BEGIN_DECLS
^
/usr/include/sys/cpuset.h:156:12: error: unknown type name 'cpusetid_t';
did you mean 'cpuset_t'?
int cpuset(cpusetid_t *);
and so forth.
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This patch removes variables such as 'ret', 'rc' and others which
are easily replaced. Therefore, making the code look cleaner and
easier to understand.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This eliminates one incorrect parsing implementation which relied on the
command field not having a closing bracket. This possibility is already
tested against in the virProcessGetStat() tests.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
This reads and separates all fields from /proc/<pid>/stat or
/proc/<pid>/task/<tid>/stat as there are easy mistakes to be done in the
implementation. Some tests are added to show it works correctly. No number
parsing is done as it would be unused for most of the fields most, if not all,
of the time. No struct is used for the result as the length can vary (new
fields can be added in the future).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Pavel Hrdina [Tue, 23 Nov 2021 14:15:43 +0000 (15:15 +0100)]
virsh: man: update snapshot-revert description
We've changed the behavior of this API that from now on it will always
restart the VM process and we are no longer able to revert to snapshots
created by libvirt older then 0.9.5.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
virsh: Do not try connecting first time without polkit agent
Trying to connect once without a polkit agent will generate an error on the
server side which seems too rough given it only serves the purpose of the client
(virsh in this case) to figure out that an agent is needed. Thankfully we can
just try running the agent. It does not break anything as we are running it
with `--fallback`, which makes sure it does not replace an existing agent in
case there is one already registered.
The second piece of code trying to start the polkit text agent is kept in order
to _really_ try out starting the agent (and error out when failing to do so)
just in case the agent was not available the first time it was ran. Even though
it should not happen it avoids a very rare race condition and really does not
add much complexity.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1945501 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
With this function we can decide whether to try running the polkit text agent
only if it is available, removing a potential needless error saying that the
agent binary does not exist, which is useful especially when running the agent
before knowing whether it is going to be needed.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Recently, FreeBSD has got sched_get/setaffinity(3) implementations and
the sched.h header as well [1]. To make these routines visible,
users have to define _WITH_CPU_SET_T.
This breaks current detection. Specifically, meson sees the
sched_getaffinity() symbol and defines WITH_SCHED_GETAFFINITY. This
define unlocks Linux implementation of virProcessSetAffinity() and other
functions, which fails to build on FreeBSD because cpu_set_t is not
visible as _WITH_CPU_SET_T is not defined.
For now, change detection to the following:
- Instead of checking sched_getaffinity(), check if 'cpu_set_t' is
available through sched.h
- Explicitly check the sched.h header instead of assuming its presence
if WITH_SCHED_SETSCHEDULER is defined
We've changed the behavior of this API that from now on it will always
restart the VM process and we are no longer able to revert to snapshots
created by libvirt older then 0.9.5.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Pavel Hrdina [Thu, 11 Nov 2021 16:34:46 +0000 (17:34 +0100)]
test: snapshot revert: always error out if VM XML is missing
We should have this check even if FORCE flag is used because later we
unconditionally copy the `snap->def->dom` and error out if there is no
copy created. The test driver will always save the VM XML when creating
new snapshot.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
projects / thirdparty / libvirt.git / log
