The function never returns anything but zero.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The events would normally be triggered only if we're changing domain
state. But most of the time the domain is already in the right state and
we're just changing its substate from {PAUSED,RUNNING}_POSTCOPY to
*_POSTCOPY_FAILED. Let's emit lifecycle events explicitly when post-copy
migration fails to make the failure visible without polling.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
There's no need to artificially pause a domain when post-copy fails
from our point of view unless QEMU connection is broken too as migration
may still be progressing well.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The virStorageSourceGetActualType() function returns either
virStorageSource->type (which is of type virStorageType), or
virStorageSourcePoolDef->type, which really stores a value of the
same enum. Thus, the latter struct can be changed so that the
virStorageSourceGetActualType() function can return correct type
instead of generic int.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
There are three places (two in domain_conf.c and one in
qemu_migration.c) where a virStorageSource->type is typecasted to
virStorageType (for the purpose of catching missing enum member
in a switch() statement at compile time). This is needless,
because as of v8.2.0-rc1~120 the struct member is of proper type.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
My recent commit v8.3.0-201-gc500955e95 tried to fix a regression which
would cause the function to return success even if virCloseCallbacksSet
failed. But due to a strange code flow in the function introduced an
opposite regression. The function would return NULL on success when
called without VIR_MIGRATE_CHANGE_PROTECTION flag.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Commit v8.3.0-152-g49ef0f95c6 removed explicit VIR_FREE from
qemuMigrationBegin, effectively reverting v1.2.14-57-g77ddd0bba2
The xml variable was used to hold the return value and thus had to be
unset when an error happened after xml was already non-NULL. Such code
may be quite confusing though and we usually avoid it by not storing
anything to a return variable until everything succeeded.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Remove the argument from the function prototypes and the callback
handler.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Access the 'driver' struct from the private data rather than the passed
opaque pointer in preparation to remove the opaque pointer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
All QEMU versions we care about already support migration events.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Acquiring job introduced in commit [1] to fix a race described in the
commit. Actually it does not help because we get domain in create API
before acuiring job. Then [2] fixed the race but [1] was not reverted even
it is does not required by [2] to work properly.
[1] commit b629c64e5e0a32ef439b8eeb3a697e2cd76f3248
Author: Martin Kletzander <mkletzan@redhat.com>
Date: Thu Oct 30 14:38:35 2014 +0100
qemu: avoid rare race when undefining domain
[2] commit c7d1c139ca3402e875002753952e80ce8054374e
Author: Martin Kletzander <mkletzan@redhat.com>
Date: Thu Dec 11 11:14:08 2014 +0100
qemu: avoid rare race when undefining domain
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@openvz.org>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
All QEMU releases currently supported by libvirt already understand
"-incoming defer". We can drop the code handling "-incoming URI".
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Recent refactor (v8.1.0-217-ga193f4bef6) generalized job related enums
and functions by changing "qemu" prefix to "vir" and moving them to
src/hypervisor/domain_job.[ch]. This was in most cases a good thing, but
async job phases are driver specific and the corresponding functions
remained in src/qemu/qemu_domainjob.[ch], but still their prefix was
changed to "vir". Let's change it back to "qemu".
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
It makes sense to have these in the same file as the definitions
of enums.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
These enums are essentially the same and always sorted in the
same order in every hypervisor with jobs. They can be generalized
by using the qemu enums as the main ones as they are the most
extensive.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
I think the code looks cleaner without else branches.
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>
It does not make sense to have both of these, since one of them
is only a wrapper for the other one. I decided to preserve the
more general one, which requires only virDomainObj and rewrote it
a bit, so that it pulls the qemu driver from privateData.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Autofree the temporary string and shuffle around the success path to
avoid the 'cleanup' label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The NBD connection for non-shared storage migration can have the same
issue regarding TLS certificate name match as the migration connection
itself.
Propagate the configured name also for the NBD connections.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1901394
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We do support non-shared storage migration with TLS now. Fix the comment
claiming otherwise.
Fixes: a8dc146a4d14b6f169f27b6d238452beb254a637
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Now that the virNWFilterBinding APIs are using the nwfilter
update lock directly, there is no need for the virt drivers
to do it themselves.
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The destination daemon would crash in Finish phase due to NULL
dereference which I missed in my review of commit
v8.0.0-428-g0301db44e2
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We only need to set statsType in almost every case of setting
something from private data, so it seems unnecessary to pull
privateData out of current / completed job for just this one
thing every time. I think this patch keeps the code cleaner
without variables used just once.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
This patch includes:
* introducing new files: src/hypervisor/domain_job.c and src/hypervisor/domain_job.h
* new struct virDomainJobData, which is almost the same as
qemuDomainJobInfo - the only differences are moving qemu specific
job stats into the qemuDomainJobDataPrivate and adding jobType
(possibly more attributes in the future if needed).
* moving qemuDomainJobStatus to the domain_job.h and renaming it
as virDomainJobStatus
* moving and renaming qemuDomainJobStatusToType
* adding callback struct virDomainJobDataPrivateDataCallbacks
taking care of allocation, copying and freeing of private data
of virDomainJobData
* adding functions for virDomainJobDataPrivateDataCallbacks for
qemu hypervisor
* adding 'public' (public between the different hypervisors) functions
taking care of init, copy, free of virDomainJobData
* renaming every occurrence of qemuDomainJobInfo *info to
virDomainJobData *data
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
This change was generated using the following spatch:
@ rule1 @
expression a;
identifier f;
@@
<...
- f(*a);
... when != a;
- *a = NULL;
+ g_clear_pointer(a, f);
...>
@ rule2 @
expression a;
identifier f;
@@
<...
- f(a);
... when != a;
- a = NULL;
+ g_clear_pointer(&a, f);
...>
Then, I left some of the changes out, like tools/nss/ (which
doesn't link with glib) and put back a comment in
qemuBlockJobProcessEventCompletedActiveCommit() which coccinelle
decided to remove (I have no idea why).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There are few places where the g_steal_pointer() is open coded.
Switch them to calling the g_steal_pointer() function instead.
Generated by the following spatch:
@ rule1 @
expression a, b;
@@
<...
- b = a;
... when != b
- a = NULL;
+ b = g_steal_pointer(&a);
...>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
virConnectOpenAuth() calls virConnectOpenInternal(). This later function
generates fine grained errors arising from various failure conditions that are
more accurate than a "catch all" broader VIR_ERR_OPERATION_FAILED error that
the callers of this function generates. Remove the broader error so that more
specific errors can be caught and processed.
Signed-off-by: Ani Sinha <ani@anisinha.ca>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
It is a nice wrapper around virDomainObjSave which logs a warning, but
otherwise ignores the error. Let's use it where appropriate.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use it to enable the 'write-blocking' mode of 'blockdev-mirror'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Forces the data to be written synchronously to both the original and the
mirrored images which ensures that the job will reach synchronized
phase.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@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>
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>
We did not set priv->migMaxBandwidth if '--bandwidth' was
specified as an option in the 'migrate' virsh command. This
caused in printing the wrong value if virsh command
'migrate-getspeed' was called during the migration. This patch
first sets the value to the given bandwidth (if one was
specified) and restores the previous value after the migration.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1806856
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
The original idea was to ensure that the destination has the same
original state of the '-no-reboot' flag to ensure identical behaviour of
the 'vidDomainModifyLifecycleAction' API.
With newer qemu's we'll be able to modify the behaviour using the
monitor so old daemons won't be able to keep up anyways.
Remove this feature as it's not very useful and will be replaced by a
proper solution.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When we try to migrate vm, we check if it contains only devices
that are able to migrate. If a hostdev device is not able to
migrate we raise an error with <hostdev/>, but it can actually be
<interface/>, so we need to check if hostdev device was created
by us from interface and show the right error message.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1942315
Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
When doing a peer-to-peer migration it may happen that the
connection to the destination disappears. If that happens,
there's no point in trying to unregister the close callback
because the connection is closed already. It results only in
polluting logs with this message:
error : virNetSocketReadWire:1814 : End of file while reading data: : Input/output error
and the reason for that is unregistering a connection callback
results in RPC (among other things).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1918211
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
qemuMigrationSrcRunPrepareBlockDirtyBitmaps receives the flags parameter
from qemuMigrationSrcRun, where flags are based on the main API enum
values. Similar to commit f58349c9c6, use the main API enum instead of
internal driver enum when checking flags in
qemuMigrationSrcRunPrepareBlockDirtyBitmaps.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
The 'storageMigration' flag is supposed to be true if storage migration
is requested, which is based on VIR_MIGRATE_NON_SHARED_DISK or
VIR_MIGRATE_NON_SHARED_INC flags. The assignment to the variable used
QEMU_MONITOR_MIGRATE_NON_SHARED_INC (0x04) instead of
VIR_MIGRATE_NON_SHARED_INC (0x80), caused libvirtd to skip the actual
copy of data.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1978526
Fixes: da69f4b2084bff140238e450e264d6036ebef898
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
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>
