The libxl driver will handle all domain destruction and cleanup
when receiving a domain shutdown event from libxl. Commit fa30ee04a2
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 fa30ee04a2 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>
Since f29d7c3e69 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>
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>
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>
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>
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>
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>
We pass through to glib's hash table functions so we can also use glibs
function prototype definition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The code was converted to stop using this function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Use 'g_clear_pointer(&ptr, g_hash_table_unref)' instead.
In few instances it allows us to also remove explicit clearing of
pointers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
'blockNamedNodeData' is declared for automatic freeing but we also free
it manually and reuse which is a code pattern we don't normally allow.
Rewrite the code to have actually two separate hash tables.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
No need for the cleanup section once we switch to g_autoptr.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Allocate the hash table only after the log file is opened so that we
don't need to deallocate it on failure.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
After the conversion to g_autofree, the cleanup label is no longer
needed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
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>
It makes it easier to see what's going on when trying to figure out why
the disk definition was not updated on a finalized blockjob.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
In other places we use strspn to validate a character subset. Convert
the in-place loop and simplify the error message.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the new STRLIM macro and unify it with the empty string check.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
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>
Reduce the extent of custom logic and custom error messages by using
virXMLPropTristateBool.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The helpers will update the passed boolean if the tristate's value is
not _ABSENT.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This reports what TPM features QEMU supports, provided that swtpm is
installed in the host.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This adds reporting of available TPM models and backends to the domain
capabilities schema
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The QEMU domain capabilities code wants to quietly know whether swtpm is
available on the host.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
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>
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>
Every other exported API from virtpm.h will internally call
virTPMEmulatorInit, so there is no reason for this initializer
to be exported on its own.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
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>
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>
Best practice is to have all types use a naming convention based on the
filename.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
I think it makes more sense for the variable about jobs to be in
the job object. I also renamed it to be consistent with the rest
of the struct.
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>
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>
It is useful to be able to deeply check them for equality.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The logic of saving metadata doesn't have to be in endjob section as
it will only happen if we have successfully created snapshot.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There is no need for the extra variable as it is used only at one place.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This makes it obvious that the function is called for creating new
snapshot and redefining old snapshot as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Function qemuDomainJobAllowed() is never used -> remove it.
The last use was removed in commit 3f2fa8f303
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemuDomainObjExitMonitor is just an alias for it at this point.
This also removes the incomplete ATTRIBUTE_NONNULL(1) annotation.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This reverts my
commit dc2fd51fd7
Check for domain liveness in qemuDomainObjExitMonitor
which fixed the symptoms of the bug later fixed by
commit 81f50cb92d
qemu: Avoid calling qemuProcessStop without a job
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Remove the check from conditions where it's coupled with some other
checks.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This wrapper for qemuDomainObjExitMonitorInternal was
extended by my commit dc2fd51fd7
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 81f50cb92d
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>
There is a stray 'return -1' executed on all code paths.
Fixes: c18d9e23fa
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
macOS on Apple silicon reports 'arm64' as the architecture from uname,
which we need to canonicalize to VIR_ARCH_AARCH64 / 'aarch64'.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
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:
<interface type='network'>
<source network='default'/>
<model type='virtio'/>
<rom enabled='no'/>
</interface>
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: 24be92b8e3
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>
Commit bc24810c2c 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>
These functions always return success so it seems logical to not
return anything and remove unnecessary checks.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This patch rewrites the pattern using early return where it is
not needed and changes the return type of the functions to 'void'
if possible.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
I have seen this pattern a lot in the project, so I decided to
rewrite code I stumbled upon to the same pattern as well.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The libxlDomainObjPrivate object is never locked and hence does not need to
be a virObjectLockable object.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
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).
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
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>
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>
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>
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>
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.
Fixes:
commit 01cf7a1bb9
Author: Stefan Berger <stefanb@us.ibm.com>
Date: Thu Jul 25 14:22:04 2019 -0400
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 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>
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>
The function produces an error which is ignored in this code path.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virXPathStringLimit doesn't give callers a way to differentiate between
the queried XPath being empty and the length limit being exceeded.
This means that the callers is completely ignoring the error.
Move the length check into the caller.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Apart from code simplification the refactor of 'model' fixes an unlikely
memory leak of the string if a duplicate model is found.
While the coversion of 'label' variable may seem unnecessary it will
come in handy in the next patch.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virXPathStringLimit doesn't give callers a way to differentiate between
the queried XPath being empty and the length limit being exceeded.
This means that callers are either overwriting the error message or
ignoring it altogether.
Move the length checks into the caller.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virXPathStringLimit doesn't give callers a way to differentiate between
the queried XPath being empty and the length limit being exceeded.
This means that callers are overwriting the error message.
Move the length checks into the caller.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use separate variables for 'model' and 'relabel' properties.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'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>
Use the appropriate enum type instead of an int and fix the XML parser
and one missing fully populated switch.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@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>
There is a leftover 'ptys' variable, which we only assign
to and one assignment to 'content', where we add an empty
'pty' object.
Remove 'ptys'.
Fixes: 93accefd9e
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Kristína Hanicová <khanicov@redhat.com>
There is a stray mis-indented 'return NULL' left after a recent
refactor.
Fixes: c18d9e23fa
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Kristína Hanicová <khanicov@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>
Recent cleanup of snapshot revert code made these function unused.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It does not need a tty to work, it opens its controlling terminal for user
interaction and with this patch even crazy things like this work:
echo 'list --name' | virsh -q >/dev/null
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>
Automatic "Ptr " -> " *" also wreaked havoc in comments. Fix it and while at it
reword the sentence so it is clear that the object is newly allocated.
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
1:
https://cgit.freebsd.org/src/commit/?id=43736b71dd051212d5c55be9fa21c45993017fbbhttps://cgit.freebsd.org/src/commit/?id=160b4b922b6021848b6b48afc894d16b879b7af2https://cgit.freebsd.org/src/commit/?id=90fa9705d5cd29cf11c5dc7319299788dec2546a
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
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>
This will always happen so there is no need to error out and require
usage of FORCE flag.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Now that we always emulate restarting the VM process events are emitted
differently so we need to update the code and the comment as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Now that we always emulate VM process stop we can drop the unused code
and simply the logic.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reflect the same change in test driver as in QEMU driver because the
compatibility check code isn't perfect.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When active snapshot is reverted we stop CPUs in order to load the
snapshot but we never start the CPUs again.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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>
This will always happen so there is no need to error out and require
usage of FORCE flag.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Now that we always restart the QEMU process events are emitted
differently so we need to update the code and the comment as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Now that we always restart QEMU process the loadvm code is unused and
can be dropped.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Our compatibility check code isn't complete and there are cases where it
fails to detect incompatible configuration and the revert fails. In
addition future support for external snapshot will always require
restarting the QEMU process.
To unify the behavior drop the compatibility check code and always
restart the QEMU process.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The support to revert snapshots was introduced in libvirt 0.8.0 but
saving the whole VM XML was implemented later in libvirt 0.9.5.
That is more then 10 years ago so we can safely assume that nobody will
try reverting to snapshot created by that old libvirt. In the unlikely
scenario where someone would actually did it we would simply error out.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Git bisect took me to commit where incorrect usage of ATTRIBUTE_NONNULL
was introduced and caused coverity scan to fail. This patch fixes the
issue where the index starts from 1 and not 0 and two other different
cases.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We currently use -machine accel=XXX which is just a syntax sugar
for -accel XXX. The former doesn't allow specifying arguments for
accelerator, because all arguments passed to -machine are
treated as arguments of machine itself.
The -accel argument was introduced in QEMU commit
v2.9.0-rc0~70^2~19 and since our minimum required version is
newer (2.11.0) we can safely assume its existence and use it
without any capability.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/233
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Kashyap Chamarthy <kchamart@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The domain accelerator was validated in qemuValidateDomainDef()
which calls virQEMUCapsIsVirtTypeSupported() which reports proper
error if QEMU is not capable of KVM/TCG. There is no point in
doing the validation again when building command line.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Kashyap Chamarthy <kchamart@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Use 'virJSONValueObjectAdd' instead of the step-by-step manual JSON
object building.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When formatting the commandline we explicitly set the reconnect timeout
to 0 when it's disabled even when that's the default. Do the same in
the monitor/hotplug code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rename 'data' to 'backendData' so that it's more clear what the object
represents and 'backend_type' to 'backendType' to go with the common
camel case notation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In some cases we have a label that contains nothing but a return
statement. The amount of such labels rises as we use automagic
cleanup. Anyway, such labels are pointless and can be dropped.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
When guest has NUMA nodes and QEMU is new enough to report
default RAM ID then ideally we would use -numa memdev= combined
with memory-backend-* combo becasue -mem-path/-mem-prealloc/-numa
mem are deprecated. Well, there is one problem - the .memdev=
attribute is machine type dependent (just look at arguments of
virQEMUCapsGetMachineNumaMemSupported()) and to ensure backwards
compatibility we prefer -numa mem= over -numa memdev=.
But there was one corner case when -mem-prealloc was requested
but not generated on the cmd line. It all starts with
qemuBuildMemCommandLine() which generates just '-m XXX' and
because it sees defaultRAMid and guest NUMA nodes greater than
zero it does nothing more.
Then, qemuBuildNumaCommandLine() sees that -numa mem= is still
supported for given machine type and nothing else set
@needBackend thus qemuBuildMemPathStr() is called which output
-mem-prealloc only in a few cases assuming it was outputted
earlier.
Reported-by: Jing Qi <jinqi@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Create extra g_autofree strings and use them in an adequate scope to
eliminate the need for VIR_FREE() calls and the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
ppc64Compute() is used only once, by virCPUppc64Compare(), which
doesn't use the 'guest' parameter. It was last used by an API
called 'cpuGuestData' that was dropped by commit 03fa904c0c.
Removing the 'guest' parameter will not only remove unused code from
ppc64Compute() but also remove the ppc64MakeCPUData() entirely.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
'str' is no longer being used and the 'cleanup' label can be removed
together with the 'ret' variable.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Create a new helper to remove the arch validation logic from the
body of virQEMUCapsLoadCache().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Create a new helper to remove the emulator validation logic from the
body of virQEMUCapsLoadCache().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Create a new helper to remove the GIC parse logic from the body of
virQEMUCapsLoadCache().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Create a new helper to remove the parse capabilities flag logic from the
body of virQEMUCapsLoadCache().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
The content of /sys/module/kvm_amd/parameters/sev may vary depending on
the kernel version. Check also for 'Y' and 'y' in addition to '1' to
cover several possible variants. The fix is similar to the one
introduced in commit 3f9c1a4bb8
Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit 114e3b4232 added matching by
aliases for interfaces with same MAC address.
( https://bugzilla.redhat.com/show_bug.cgi?id=1926190 )
However, unless the domain is using user aliases (prefixed ua-),
there are no aliases in the persistent definition.
Only match by MAC/PCI/other addresses in that case, to fix update-device
with --persistent flag (AFFECT_LIVE | AFFECT_CONFIG).
https://bugzilla.redhat.com/show_bug.cgi?id=2024098
Fixes: 114e3b4232
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Originally the default encryption engine is populated in the disk
post-parse callback code. This works for disks but for any additional
images introduced either via the block copy API or via the backup API we
don't populate the default.
In case when the backup or block copy is requested on an encrypted image
this would then lead to an error:
error: internal error: Unexpected enum value 0 for virStorageEncryptionEngine
This patch adds another point where we populate the default which is
when setting up a virStorageSource for actual usage.
We keep the original setting in the post-parse callback as that's the
only point that is recorded in the XML file after definition.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2023674
Fixes: ab1d46d612
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Before commit 73c352ab8c which converted the hostdev commandline
formatter to JSON the 'ramfb' property was formatted only if it was
enabled.
The main reason for that is that enabling 'ramfb' switches the device
model to 'vfio-pci-nohotplug' which actually has the property, while
'vfio-pci' (used when 'ramfb' is disabled or absent) doesn't have it.
Restore the logic to format 'ramfb' only when it's enabled and add a
comment that it's deliberate.
Fixes: 73c352ab8c
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2024435
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'virDomainChrSourceDef' contains private data so 'virDomainChrSourceDefNew'
must be used to allocate it. 'virDomainTPMDef' was using it directly
which won't work with the chardev helper functions.
Convert it to a pointer to properly allocate private data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It's referenced only once and it's a shortcut to the chardev source thus
can be used directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rather than returning a different error code if the device address needs
to be released pass in the 'need_release' flag via a pointer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function is already getting 'virDomainObj' which has already the
driver pointer present.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Switch to automatic memory clearing for the two virJSONValues and remove
the 'cleanup' label and 'ret' variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The main reason is to ensure that the private data are properly
allocated for every instance.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The function is already returning JSON properties, rename it
accordingly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use 'virReportEnumRangeError' for the invalid cases and keep the
original error for known but unsupported chardevs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the addition of the command wrapper to qemuMonitorJSONAttachCharDev
and rename the function to qemuMonitorJSONAttachCharDevGetProps.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use 'qemuMonitorJSONMakeCommandInternal' instead of
'qemuMonitorJSONMakeCommand' + 'virJSONValueObjectAppend'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Add a simple wrapper for 'qemuMonitorAttachCharDev' named
'qemuHotplugChardevAttach' which will simplify the moving of the
character device property generator out of the monitor code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The 'xmlopt' parameter can be auto-unref by using g_autoptr().
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
This will enable code cleanups on code that still relies on
virObjectUnref() this pointer manually.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
x86Compute() is a static function called only by virCPUx86Compare()
which passes NULL to the 'guest' parameter of x86Compute().
The last caller of x86Compute() that used it with 'guest' != NULL
was an API called 'cpuGuestData'. This API was dropped by commit
03fa904c0c a few years ago. Since then all callers of x86Compute()
uses it with 'guest' = NULL.
Removing the 'guest' parameter allow us to remove a good chunk of
logic that isn't being used for awhile.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
There is no guarantee that QEMU and libvirt have been configured
with the same prefix.
In particular, Homebrew on macOS will pass a different, private
prefix for each package version and then use symlinks to make
the files for a specific version appear in the usual locations.
This works perfectly fine as long as one package doesn't try to
go poking around another package's data - which is exactly what
libvirt needs to do in order to read and parse the QEMU interop
data.
qemu_datadir can now be explicitly provided to make this and
other uncommon scenarios work. The common scenario, where QEMU
and libvirt both use the same prefix, is unaffected.
https://gitlab.com/libvirt/libvirt/-/issues/168
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use abstract names that more closely match the Meson
nomenclature.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
While datadir must live under prefix, there is no requirement
that its name must necessarily be "share": a different,
arbitrary name could have been provided by the user.
Fixes: 3c876d2428
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'-audiodev' as a modern implementation based on QAPI already takes JSON
as the argument. Convert our code to use it directly.
The declaration of the QAPI types can be found in
'qemu.git/qapi/audio.json'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The functions were obsoleted by virJSONValueObjectAdd(VArgs)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virJSONValueObjectAdd now works identically to virJSONValueObjectCreate
when used with a NULL argument. Replace all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virJSONValueObjectAdd now works identically to virJSONValueObjectCreate
when used with a NULL argument. Replace all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Upcomming patch will convert virJSONValueObjectCreate which didn't check
the value of the passed argument to virJSONValueObjectAdd which will and
this would lead to crashes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virJSONValueObjectAdd now works identically to virJSONValueObjectCreate
when used with a NULL argument. Replace all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virJSONValueObjectAdd now works identically to virJSONValueObjectCreate
when used with a NULL argument. Replace all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virJSONValueObjectAdd now works identically to virJSONValueObjectCreate
when used with a NULL argument. Replace all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virJSONValueObjectAdd now works identically to virJSONValueObjectCreate
when used with a NULL argument. Replace all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virJSONValueObjectAdd now works identically to virJSONValueObjectCreate
when used with a NULL argument. Replace all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Until now the code would crash if virJSONValueObjectAdd is used without
a valid object. Adding the functionality of allocating it if it's NULL
will allow us to replace all uses of virJSONValueObjectCreate with this
single function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Pass in the double pointer from the wrappers directly to
virJSONValueObjectAddVArgs, which will allow us to directly allocate the
new objects inside the function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Change the first argument to a double pointer so that later the function
can be unified with virJSONValueObjectCreate and fix all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit 3210c8b04 introduced a separate field 'diskbus' for
internal use by the commandline formatter.
Adjust the check for address equality to compare the relevant
fields separately instead of the whole struct.
This means that the changes of the 'diskbus' field added
in commit 03ed14d2d4 no longer affect the outcome of the address
equality check.
https://bugzilla.redhat.com/show_bug.cgi?id=2023605
Fixes: 3210c8b047
Fixes: 03ed14d2d4
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The upcoming QEMU 6.2.0 implements a new event called
DEVICE_UNPLUG_GUEST_ERROR, a new event that reports generic device
unplug errors that were detected by the guest and reported back to QEMU.
This new event is going to be specially useful for pseries guests that
uses newer kernels (must have kernel commit 29c9a2699e71), which is the
case for Fedora 34 at this moment. These guests have the capability of
reporting CPU removal errors back to QEMU which, starting in 6.2.0, will
emit the DEVICE_UNPLUG_GUEST_ERROR event. Libvirt can use this event to
abort the device removal immediately instead of waiting for 'setvcpus'
timeout.
QEMU 6.2.0 is also going to emit DEVICE_UNPLUG_GUEST_ERROR for memory
hotunplug errors, both in pseries and ACPI guests. QEMU 6.1.0 reports
memory removal errors using the MEM_UNPLUG_ERROR event, which is going to
be deprecated by DEVICE_UNPLUG_GUEST_ERROR in 6.2.0. Given that
Libvirt wasn't handling the MEM_UNPLUG_ERROR event we don't need to
worry about it - adding support to DEVICE_UNPLUG_GUEST_ERROR will be
enough to cover all future cases.
This patch adds support to DEVICE_UNPLUG_GUEST_ERROR by adding the
minimal wiring required for Libvirt to be aware of it. The monitor
callback for this event will abort the pending removal operation of the
device reported by the "device" property of the event. Most of the heavy
lifting is already done by existing code that handles
QEMU_DOMAIN_UNPLUGGING_DEVICE_STATUS_GUEST_REJECTED, making our life
easier to abort the pending removal operation.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Explicit calls to VIR_FREE() can be dropped then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Kristína Hanicová <khanicov@redhat.com>
Similarly to one of previous commits, there's no need to
increment domain object refcounter before unlocking it. Any
number of lock and unlock calls over domain object has no effect
on the refcounter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After previous cleanups this callback is unused. Remove it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Currently, when opening an agent socket the qemuConnectAgent()
increments domain object refcounter and calls qemuAgentOpen()
where the domain object pointer is simply stored inside
_qemuAgent struct. If qemuAgentOpen() fails, then it clears @cb
member only to avoid qemuProcessHandleAgentDestroy() being called
(which decrements the domain object refcounter) and the domain
object refcounter is then decreased explicitly in
qemuConnectAgent().
The same result can be achieved with much cleaner code: increment
the refcounter inside qemuAgentOpen() and drop the dance around
@cb.
Also, the comment in qemuConnectAgent() about holding an extra
reference is not correct. The thread that called
qemuConnectAgent() already holds a reference to the domain
object. No matter how many time the object is locked and unlocked
the reference counter can't be decreased.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Just like qemuMonitorOpen(), hold the domain object locked
throughout the whole time of qemuConnectAgent() and unlock it
only for a brief time of actual connect() (because this is the
only part that has a potential of blocking).
The reason is that qemuAgentOpen() does access domain object
(well, its privateData) AND also at least one argument (@context)
depends on domain object. Accessing these without the lock is
potentially dangerous.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1845468#c12
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
To make it easier for users to figure out how the DN should be formatted.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This removes a dead link, the need for users to understand a glib function and a
improper reference to fnmatch (as we only expand asterisks to any string).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
And to make that easier, allocate it on the heap.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Previous commit rendered 'cleanup' label and @ret variable
redundant. The same result can be achieved by returning 0/-1
directly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
There are two instances of an explicit call to
qemuMonitorCPUModelInfoFree() which in fact can be turned into
g_auto().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
There are a few cases where a string list is freed by an explicit
call of g_strfreev(), but the same result can be achieved by
g_atuo(GStrv).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
Instead of explicit virObjectUnlock(obj) + virObjectUnref(obj)
combo the virSecretObjEndAPI() can be used.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Instead of explicit virObjectUnlock(obj) + virObjectUnref(obj)
combo the virNodeDeviceObjEndAPI() can be used.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Instead of explicit virObjectUnlock(obj) + virObjectUnref(obj)
combo the virNWFilterBindingObjEndAPI() can be used.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Instead of explicit virObjectUnlock(obj) + virObjectUnref(obj)
combo the virInterfaceObjEndAPI() can be used.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Instead of explicit virObjectUnlock(obj) + virObjectUnref(obj)
combo the virDomainObjEndAPI() can be used.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Found via valgrind
==15016== 3,701 bytes in 2 blocks are definitely lost in loss record 975 of 1,009
==15016== at 0x4C2A2AF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==15016== by 0x1FCD30CB: libxl_read_file_contents (in /usr/lib64/libxenlight.so.4.12.0)
==15016== by 0x1FCCA58A: ??? (in /usr/lib64/libxenlight.so.4.12.0)
==15016== by 0x1FCCA6C2: libxl_userdata_retrieve (in /usr/lib64/libxenlight.so.4.12.0)
==15016== by 0x1FA42A5A: libxlReconnectDomain (libxl_driver.c:394)
==15016== by 0x53BAC99: virDomainObjListHelper (virdomainobjlist.c:802)
==15016== by 0x530842F: virHashForEach (virhash.c:575)
==15016== by 0x53BC0E0: virDomainObjListForEach (virdomainobjlist.c:817)
==15016== by 0x1FA423C4: libxlReconnectDomains (libxl_driver.c:468)
==15016== by 0x1FA423C4: libxlStateInitialize (libxl_driver.c:778)
==15016== by 0x54E8E9E: virStateInitialize (libvirt.c:657)
==15016== by 0x12DBFA: daemonRunStateInit (remote_daemon.c:797)
==15016== by 0x535BF79: virThreadHelper (virthread.c:206)
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
During the vm rebooting, the vm could be paused if the libvirtd is
restarted for some reason, which is not expected. We need continue
fakereboot process if fakereboot flags is true and the vm is in
paused-user status.
Signed-off-by: Bihong Yu <yubihong@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
During the vm rebooting, the vm could be shut down if the libvirtd is
restarted for some reason, which is not expected. We move set
fakereboot flags false after processing fakereboot over, so we can
ensure that fakereboot process have been executed.
Signed-off-by: Bihong Yu <yubihong@huawei.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This patch improves readability of the function and makes the
code look cleaner by removing the 'else' branches after return
and reordering of the 'if' branches.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit 88957116c9 switched to use
memory-backend-* for regular VM memory as well. That change indirectly
started using 'host-nodes' for system memory which results in QEMU
calling mbind() to bind the system memory to specific NUMA node if the
VM XML contains the configuration similar to this:
...
<numatune>
<memory mode='strict' nodeset='0'/>
</numatune>
...
Once the VM was started with that configuration it was no longer
possible to change the memory NUMA nodeset.
Fixes: 677c90cc1d
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
openssh supports environment variable expansion in its ssh_config
file[1]. These two environment variables can be used to
expand paths for ssh sockets and other files.
Ex.
```
Host *
ControlMaster auto
ControlPath ${XDG_RUNTIME_DIR}/ssh-%C.ctl
IdentityAgent ${XDG_RUNTIME_DIR}/ssh-agent.socket
```
see also:
[1] https://man7.org/linux/man-pages/man5/ssh_config.5.html#ENVIRONMENT_VARIABLES
[2] https://gitlab.com/libvirt/libvirt/-/issues/232
Signed-off-by: Дамјан Георгиевски <gdamjan@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Remove unnecessary label, goto, and closing of not-open file descriptor.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
