The most important bit is that the caller is expected to pass
locked monitor.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Using the job owner API name directly works fine as long as it is a
static string or the owner's thread is still running. However, this is
not always the case. For example, when the owner API name is filled in a
job when we're reconnecting to existing domains after daemon restart,
the dynamically allocated owner name will disappear with the
reconnecting thread. Any follow up usage of the pointer will read random
memory.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Commit 010ed0856b and commit db64acfbda introduced the ability to use
the <teaming> element in a generic <hostdev> (previously it could only
be used with <interface type='hostdev'>). However, the patch omitted
one crucial detail - along with parsing the <teaming> element in
<hostdev>, and adding the necessary info to the qemu commandline, we
also need to modify qemuMigrationSrcIsAllowedHostdev() to allow
migration when the generic <hostdev> has a <teaming> element.
https://bugzilla.redhat.com/1927984
Fixes: 010ed0856bb06f439e6fdf44e4f529f53441c398
Reported-by: Yalan Zhang <yalzhang@redhat.com>
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemuMonitorUnregister will be called in multiple threads (e.g. threads
in rpc worker pool and the vm event thread). In some cases, it isn't
protected by the monitor lock, which may lead to call g_source_unref
more than one time and a use-after-free problem eventually.
Add the missing lock in qemuProcessHandleMonitorEOF (which is the only
position missing lock of monitor I found).
Suggested-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Peng Liang <liangpeng10@huawei.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The libvirt libxl driver has no access to FDs associated with VM disks.
The disks are opened by libxl.so and any related FDs are not exposed to
applications. The prevents using virtlockd's auto-release feature to
release locks when the FD is closed. Acquiring and releasing locks is
explicitly handled by the libxl driver.
The current logic is structured such that locks are acquired in
libxlDomainStart and released in libxlDomainCleanup. This works well
except for migration, where the locks must be released on the source
host before the domain can be started on the destination host, but the
domain cannot be cleaned up until the migration confirmation stage.
When libxlDomainCleanup if finally called in the confirm stage, locks
are again released resulting in confusing errors from virtlockd and
libvirtd
virtlockd[8095]: resource busy: Lockspace resource 'xxxxxx' is not locked
libvirtd[8050]: resource busy: Lockspace resource 'xxxxxx' is not locked
libvirtd[8050]: Unable to release lease on testvm
The error is also encountered in some error cases, e.g. when
libxlDomainStart fails before acquiring locks and libxlDomainCleanup
is still used for cleanup.
In lieu of a mechanism to check if a lock has been acquired, this patch
takes an easy approach to fixing the unnecessary lock releases by adding
an indicator to the libxlDomainPrivate object that can be set when the
lock is acquired and cleared when the lock is released. libxlDomainCleanup
can then skip releasing the lock in cases where it was previously released
or never acquired in the first place.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit fa30ee04a2 caused a regression in normal domain shutown.
Initiating a shutdown from within the domain or via 'virsh shutdown'
does cause the guest OS running in the domain to shutdown, but libvirt
never reaps the domain so it is always shown in a running state until
calling 'virsh destroy'.
The shutdown thread is also an internal user of the driver shutdown
machinery and eventually calls libxlDomainDestroyInternal where
the ignoreDeathEvent inhibitor is set, but running in a thread
introduces the possibility of racing with the death event from
libxl. This can be prevented by setting ignoreDeathEvent before
running the shutdown thread.
An additional improvement is to handle the destroy event synchronously
instead of spawning a thread. The time consuming aspects of destroying
a domain have been completed when the destroy event is delivered.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commints <bc760f4d7c4f964fadcb2a73e126b0053e7a9b06> and
<98a09ca48ed4fc011abf2aa290e02ce1b8f1bb5f> fixed the code to use
defines instead of magic numbers but missed this place.
Following commit <ed1ba69f5a8132f8c1e73d2a1f142d70de0b564a> changed
the cpu quota limit to reflect what kernel actually allows so using
the defines fixes XML validations as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The function calls virJSONValueObjectAppend/virJSONValueArrayAppend, so
by taking a double pointer we can drop the pointer clearing from
callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Avoid pointless copies of temporary strings when constructing number
JSON objects.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
virJSONValueNew* won't return error nowadays so NULL checks are not
necessary. The memory can be cleared via g_autoptr.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The parent array takes ownership of the inserted value once all checks
pass. Don't make the callers second-guess when that happens and modify
the function to take a double pointer so that it can be cleared once the
ownership is taken.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use automatic memory freeing and don't check return value of
virJSONValueNewString as it can't fail.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The parent object takes ownership of the inserted value once all checks
pass. Don't make the callers second-guess when that happens and modify
the function to take a double pointer so that it can be cleared once the
ownership is taken.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr for the JSON value objects and remove the cleanup label
and inline freeing of objects.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autofree for the pointer of the added object and remove the NULL
checks for values returned by virJSONValueNew* (except
virJSONValueNewNumberDouble) since they can't fail nowadays.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We know the exact number of keys or array members for the copied objects
so we can pre-allocate the arrays rather than inserting into them in a
loop incurring realloc copy penalty.
Also virJSONValueCopy now can't fail since all of the functions
allocating the different cases use just g_new/g_strdup internally so we
can remove the NULL checks from the recursive calls.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The function takes ownership of @value on success so the proper
semantics will be to clear out the @value pointer. Convert @value to a
double pointer to do this.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The functions report errors already and the error can nowadays only
happen on programmer errors (if the passed virJSONValue isn't an
object), which won't happen. Remove the reporting.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
For incremental backup we need QEMU_CAPS_BLOCKDEV,
QEMU_CAPS_BLOCKDEV_REOPEN, QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Preserve block dirty bitmaps after migration with
QEMU_MONITOR_MIGRATE_NON_SHARED_(DISK|INC).
This patch implements functions which offer the bitmaps to the
destination, check for eligibility on destination and then configure
source for the migration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
In case when the block migration job required temporary bitmaps for
merging the appropriate checkpoints we need to clean them up when
cancelling the job. On success we don't need to do that though as the
bitmaps are just temporary thus are not written to disk.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Add status XML infrastructure for storing a list of block dirty bitmaps
which are temporarily used when migrating a VM with
VIR_MIGRATE_NON_SHARED_DISK for cleanup after a libvirtd restart during
migration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
'qemuMigrationCookieBlockDirtyBitmapsMatchDisks' maps the bitmaps from
the migration cookie to actual disk objects definition pointers.
'qemuMigrationCookieBlockDirtyBitmapsToParams' converts the bitmap
definitions from the migration cookie into parameters for the
'block-bitmap-mapping' migration parameter.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
In cases where we are copying the storage we need to ensure that also
bitmaps are copied properly. This patch adds migration cookie XML
infrastructure which will allow the migration sides reach consensus on
which bitmaps to migrate.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Add the migration capability flag and the propagation of the
corresponding mapping configuration. The mapping will be produced from
the bitmaps on disk depending on both sides of the migration and the
necessity to perform merges.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
There's no need in the cleanup steps to invoke a transaction to delete a
single bitmap.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The non-transaction wrapper is useful for code paths which want to
delete individual bitmaps or for cleanup after a failed job where we
want to attempt to delete every bitmap individually to prevent a failure
from cleaning up the rest.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Use the new format when pre-creating the image for the user. Users
wishing to use the legacy format can always provide their own images or
use shared storage.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Such images don't support stuff like dirty bitmaps. Note that the
synthetic test for detecting bitmaps is used as an example to prevent
adding additional test cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The capability represents qemu's ability to setup mappings for migrating
block dirty bitmaps and is based on presence of the 'transform' property
of the 'block-bitmap-mapping' property of 'migrate-set-parameters' QMP
command.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
This patch partially reverts commit 5cde9dee where the qemuExtDevicesStop()
was moved to a location before the QEMU process is stopped. It may be
alright to tear down some devices before QEMU is stopped, but it doesn't work
for the external TPM (swtpm) which assumes that QEMU sends it a signal to stop
it before libvirt may try to clean it up. So this patch moves the
virFileDeleteTree() calls after the call to qemuExtDevicesStop() so that the
pid file of virtiofsd is not deleted before that call.
Afftected libvirt versions are 6.10 and 7.0.
Fixes: 5cde9dee8c70b17c458d031ab6cf71dce476eea2
Cc: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
A VM defined similar to:
...
<features><kvm><hint-dedicated state='on'/></kvm></features>
<cpu mode="host-model"/>
...
is currently invalid, as hint-dedicated is only allowed if cpu mode
is host-passthrough or maximum. This restriction is unnecessary, see
https://bugzilla.redhat.com/show_bug.cgi?id=1857671
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Stay true to the name of the function and clear the pointer
after freeing it.
This also silences a bogus Coverity report about a double
free in qemuMonitorGetCPUInfo where qemuMonitorCPUInfoClear
is called right after allocating a new qemuMonitorCPUInfo
to fill out the non-zero defaults.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The comment and the caller assume virQEMUSaveDataNew only steals
domXML on success, but it is copied even on failure.
Also remove the misleading g_steal_pointer call on a local variable.
Reported by coverity.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
The switch to g_auto left this one call behind.
Reported by Coverity.
Fixes: 4ab0d1844a1e60def576086edc8b2c3775e7c10d
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Commit 76f47889326c4 made qemuNodeDeviceDetachFlags() unusable due to an
'if then else if' chain that will always results in a 'return -1',
regardless of 'driverName' input.
Found by Coverity.
Fixes: 76f47889326c45d2732711bc6dd5751aaf6e5194
Reported-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
