The "spawnDaemon" and "binary" parameters are co-dependant, with the
latter non-NULL, if-and-only-if the former is true. Getting rid of the
"spawnDaemon" parameter simplifies life for the callers and eliminates
an error checking scenario.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Jumping back in the code is an anti-pattern that should be avoided if
possible.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
We don't require that the data is consistent on the destination if
aborting the migration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Rename the parameter so that it's more clear what state we are in and
fix all callees.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In certain cases such as when aborting migration we don't really care
for completion of the blockjob. Add 'force' as parameter of
'block-job-cancel'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Don't try to setup disk migration and the NBD stuff if we end up
migrating nothing.
The destination side has luckily no setup for the non-NBD cases so
omitting the element fully is okay.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Don't even try to setup storage migration if there are no eligible
disks.
This also fixes migration from older libvirts which didn't format an
empty <nbd/> element in the migration cookie if there weren't any disks
to migrate.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Base the decision on the main API flags (VIR_MIGRATE_NON_SHARED_DISK,
QEMU_MONITOR_MIGRATE_NON_SHARED_INC) via a boolean 'storageMigration'
rather than juggling everything trhough 'migration_flags'.
After this patch 'migration_flags' is updated to contain the legacy
storage migration flags only when we'll be about to use it rather than
setting it and then resetting it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
'migrate_flags' can be updated in the only caller and since
qemuMigrationSrcNBDStorageCopy already takes @flags which contains
VIR_MIGRATE_NON_SHARED_INC (used to set
QEMU_MONITOR_MIGRATE_NON_SHARED_INC) we can completely remove the
parameter.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In case the 'nbdURI' schema is not known the code would report an error
but wouldn't return failure.
Fixes: 49186372dbe
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Historically, we declared pointer type to our types:
typedef struct _virXXX virXXX;
typedef virXXX *virXXXPtr;
But usefulness of such declaration is questionable, at best.
Unfortunately, we can't drop every such declaration - we have to
carry some over, because they are part of public API (e.g.
virDomainPtr). But for internal types - we can do drop them and
use what every other C project uses 'virXXX *'.
This change was generated by a very ugly shell script that
generated sed script which was then called over each file in the
repository. For the shell script refer to the cover letter:
https://listman.redhat.com/archives/libvir-list/2021-March/msg00537.html
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Failure of 'qemuMigrationSetDBusVMState' would jump to 'exit_monitor'
but the function isn't called inside of the monitor context.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The current code is written under the assumption that, for all
limits except the core size, asking for the limit to be set to
zero is a no-op, and so the operation is performed
unconditionally.
While this is the behavior we want for the QEMU driver, the
virCommand and virProcess facilities are generic, and should not
implement this kind of policy: asking for a limit to be set to
zero should result in that limit being set to zero every single
time.
Add some checks in the QEMU driver, effectively moving the
policy where it belongs.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@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>
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>
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>
Use g_autofree for 'dom_xml' to free it on some of the (unlikely) code
paths jumping to cleanup prior to the deallocation which is done right
after it's not needed any more since it's a big string.
Noticed when running under valgrind:
==2204780== 8,192 bytes in 1 blocks are definitely lost in loss record 2,539 of 2,551
==2204780== at 0x483BCE8: realloc (vg_replace_malloc.c:834)
==2204780== by 0x4D890DF: g_realloc (in /usr/lib64/libglib-2.0.so.0.6600.4)
==2204780== by 0x4DA3AF0: g_string_append_vprintf (in /usr/lib64/libglib-2.0.so.0.6600.4)
==2204780== by 0x4917293: virBufferAsprintf (virbuffer.c:307)
==2204780== by 0x49B0B75: virDomainChrDefFormat (domain_conf.c:26109)
==2204780== by 0x49E25EF: virDomainDefFormatInternalSetRootName (domain_conf.c:28956)
==2204780== by 0x15F81D24: qemuDomainDefFormatBufInternal (qemu_domain.c:6204)
==2204780== by 0x15F8270D: qemuDomainDefFormatXMLInternal (qemu_domain.c:6229)
==2204780== by 0x15F8270D: qemuDomainDefFormatLive (qemu_domain.c:6279)
==2204780== by 0x15FD8100: qemuMigrationSrcBeginPhase (qemu_migration.c:2395)
==2204780== by 0x15FE0F0D: qemuMigrationSrcPerformPeer2Peer3 (qemu_migration.c:4640)
==2204780== by 0x15FE0F0D: qemuMigrationSrcPerformPeer2Peer (qemu_migration.c:5093)
==2204780== by 0x15FE0F0D: qemuMigrationSrcPerformJob (qemu_migration.c:5168)
==2204780== by 0x15FE280E: qemuMigrationSrcPerform (qemu_migration.c:5372)
==2204780== by 0x15F9BA3D: qemuDomainMigratePerform3Params (qemu_driver.c:11841)
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
To make it easier to split out the parsing/formatting of the <teaming>
element into separate functions (so we can more easily add the
<teaming> element to <hostdev>, change its virDomainNetDef so that it
points to a virDomainNetTeamingInfo rather than containing one.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The conversion removes the use of virStringListAdd/virStringListRemove
which try to add dynamic properties to a string list which is really
inefficient.
Storing the dbus VMState ids in a GSList is pretty straightforward and
the slightly increased complexity of the code will be paid back by
removing the string list helpers later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The code handles XML bits and internal definition and should be
in conf directory.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Use the per-job state to determine when the non-shared-storage mirror is
complete rather than the per-disk definition one. The qemuBlockJobData
is a newer approach and is always cleared after the blockjob is
terminated while the 'mirrorState' variable in the definition of the
disk may be left over. In such case the disk mirror would be considered
complete prematurely.
https://bugzilla.redhat.com/show_bug.cgi?id=1889131
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Fixes compiler error:
src/qemu/qemu_migration.c:4814:20: error: ‘dstOffline’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
4814 | if (offline && !dstOffline) {
The commit that introduced the error:
910b94df: qemu: adopt to VIR_DRV_SUPPORTS_FEATURE return -1
Signed-off-by: Nick Shyrokovskiy <nshyrokovskiy@gmail.com>
Otherwise in some places we can mistakenly report 'unsupported' error instead
of root cause. So let's handle root cause explicitly from the macro.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Even though it is technically possible, when running the migrations QEMU's
nbd-server-start errors out with:
"TLS is only supported with IPv4/IPv6"
We can always enable it when QEMU adds this feature, but for now it is safer to
show our error message rather than rely on QEMU to error out properly.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Forgetting to use the VIR_MIGRATE_TLS flag with migration can lead to
leak of sensitive information. Add an administrative knob to force use
of the flag.
Note that without VIR_MIGRATE_PEER2PEER, the migration is driven by an
instance of the client library which doesn't necessarily run on either
of the hosts so the flag can't be used to assume VIR_MIGRATE_TLS even
if it wasn't provided by the user instead of rejecting if it's not.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/67
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemu's internals were not prepared for switching to -blockdev for the
legacy storage migration. Add a proper error message since qemu is
unlikely to attempt fixing the old protocol.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/65
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move and aggregate all the logic which is switched based on whether the
migration is tunnelled or not before other checks. Further checks will
be added later.
While the code is being moved the error message is put on a single line
per new coding style.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Our streams are not the best transport for migration data and we support
TLS for security now. It's unlikely that there will be enough motivation
to add a new migration protocol to tunnel NBD too.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Due to failures to unlink on previous rename/undefine we can already have
autolink etc files for the domain to be defined. Remove them.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
qemuMigrationSrcCleanup uses qemuDomainObjDiscardAsyncJob currently. But
discard does not reduce jobs_queued counter so it leaks. Also discard does not
notify other threads that job condition is available. Discard does reset nested
job but nested job is not possible in this conditions.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Don't hide our use of GHashTable behind our typedef. This will also
promote the use of glibs hash function directly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Matt Coleman <matt@datto.com>
In one of recent patches the way that we start NBD server for
incoming migration was reworked (v6.8.0-rc1~298). A new boolean
was introduced that tracks whether the NBD server was started so
that we don't start it twice nor record in the port in the port
allocator twice. Well, this idea is good, but in the
implementation the boolean is never set, so we are reserving the
port twice and would be starting the NBD server twice too if it
wasn't for port reservation fail.
Fixes: e74d627bb3bc2684cbe3edc1e2f7cc745b4e1ff3
Reported-by: Vjaceslavs Klimovs <vklimovs@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Enable <interface type='vdpa'> for qemu domains. This provides basic
support and does not support hotplug or migration.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Centralize the logic deciding which arguments to use when exporting a
block backend via NBD to a single place so that it can be centrally
fixed in upcoming commits to support the new export method via
'block-export-add'.
Additionally this allows simplification of the caller from migration as
the logic deciding which arguments to use is extracted too.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
If storage migration is requested, and the destination storage does
not exist on the remote host, qemu's migration support will call
into the libvirt storage driver to precreate the destination storage.
The storage driver virConnectPtr is opened too early though, adding
an unnecessary dependency on the storage driver for several cases
that don't require it. This currently requires kubevirt to install
the storage driver even though they aren't actually using it.
Push the virGetConnectStorage calls to right before the cases they are
actually needed.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
Use a more descriptive name and move the verb to the end so that the
functions conform with the naming policy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We need an empty cookie, so use qemuMigrationCookieNew instead of
qemuMigrationEatCookie with NULL/0 arguments.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use a more descriptive name and move the verb to the end so that the
functions conform with the naming policy.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Block migration when transient disk option is enabled to simplify the
handling of the overlay files.
Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
Add an abort() on the class/object allocation failures so that
virStorageSourceNew() always returns a virStorageSource and remove
checks from all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In the past we had to declare @cfg and then explicitly unref it.
But now, with glib we can use g_autoptr() which will do the unref
automatically and thus is more bulletproof.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
