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: a8dc146a4d
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: da69f4b208
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>
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: 49186372db
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: 010ed0856b
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: e74d627bb3
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>
This allows:
a) migration without access to network
b) complete control of the migration stream
c) easy migration between containerised libvirt daemons on the same host
Resolves: https://bugzilla.redhat.com/1638889
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Adds new typed param for migration and uses this as a UNIX socket path that
should be used for the NBD part of migration. And also adds virsh support.
Partially resolves: https://bugzilla.redhat.com/1638889
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Clean up the semantics by using one extra self-describing variable.
This also fixes the port allocation when the port is specified.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Instead of saving some data from a union up front and changing an overlayed
struct before using said data, let's just set the new values after they are
decided. This will increase the readability of future commit(s).
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
GCC 10 complains about "well_formed_uri" may be used uninitialzed.
Even though it is a false positive, we can easily avoid it.
Avoiding
../src/qemu/qemu_migration.c: In function ‘qemuMigrationDstPrepareDirect’:
../src/qemu/qemu_migration.c:2920:16: error: ‘well_formed_uri’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
2920 | if (well_formed_uri) {
| ^
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Role(master or peer) controls how the domain behaves on migration.
For more details about migration with ivshmem, see
https://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/system/ivshmem.rst;hb=HEAD
It's a optional attribute in libvirt, and qemu will choose default
role for ivshmem device if the user is not specified.
With device property 'role', the value can be 'master' or 'peer'.
- 'master' (means 'master=on' in qemu), the guest will copy
the shared memory on migration to the destination host.
- 'peer' (means 'master=off' in qemu), the migration is disabled.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Yang Hang <yanghang44@huawei.com>
Signed-off-by: Wang Xin <wangxinxin.wang@huawei.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
To remove dependecy of `qemuDomainJob` on job specific
paramters, a `privateData` pointer is introduced.
To handle it, structure of callback functions is
also introduced.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove both 'error' and 'cleanup' labels.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove the unneeded 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr() and remove the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autofree and remove the 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The same functionality can be achieved using migrate-set-parameters QMP
command with max-bandwidth parameter.
https://bugzilla.redhat.com/show_bug.cgi?id=1829545
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Upon migration with disks, libvirt determines if each disk exists
on the destination and tries to pre-create missing ones. Well,
NVMe disks can't be pre-created, but they can be checked for
presence.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1823639
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
In order to add a string to qemuDomainJobInfo we must ensure that it's
freed and copied properly. Add helpers to copy and free the structure
and adjust the code to use them properly for the new semantics.
Additionally also allocation is changed to g_new0 as it includes the
type and thus it's very easy to grep for all the allocations of a given
type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Upcoming patch will remove it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Mores <pmores@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Helper processes may have their state migrated with QEMU data stream
thanks to the QEMU "dbus-vmstate".
libvirt maintains the list of helpers to be migrated. The
"dbus-vmstate" is added when required, and given the list of helper
Ids that must be migrated, on save & load sides.
See also:
https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/dbus-vmstate.rst
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This code was based on a per-helper instance and peer-to-peer
connections. The code that landed in qemu master for v5.0 is relying
on a single instance and DBus bus.
Instead of trying to adapt the existing dbus-vmstate code, let's
remove it and resubmit. That should make reviewing easier.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Historically threads are given a name based on the C function,
and this name is just used inside libvirt. With OS level thread
naming this name is now visible to debuggers, but also has to
fit in 15 characters on Linux, so function names are too long
in some cases.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is not yet supported.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tested-by: Andrea Bolognani <abologna@redhat.com>
Include virutil.h in all files that use it,
instead of relying on it being pulled in somehow.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Firstly, the check for disk I/O error can be moved into 'if
(!offline)' section a few lines below.
Secondly, checks for vmstate and slirp should be moved under the
same section because they reflect live state of a domain. For
offline migration no QEMU is involved and thus these restrictions
are not valid.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This addreses portability to Windows and standardizes
error reporting. This fixes a number of places which
failed to set O_CLOEXEC or failed to report errors.
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Aside from itinerant error (actually warning) messages due to an
unrecognized response from qemu, this isn't even necessary - the
migration proceeds successfully to completion anyway.
(I'm not sure where to see this status reported in the API though - do
we need to add an extra state, or recognition of a new event somewhere?)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Normally a PCI hostdev can't be migrated, so
qemuMigrationSrcIsAllowedHostdev() won't permit it. In the case of a a
hostdev network interface that has <teaming type='transient'/> set,
QEMU will automatically unplug the device prior to migration, and
re-plug a corresponding device on the destination. This patch modifies
qemuMigrationSrcIsAllowedHostdev() to allow domains with those devices
to be migrated.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
There are a large number of different header files that
are related to the sockets APIs. The virsocket.h header
includes all of the relevant headers for Windows and UNIX
in one convenient place. If virsocketaddr.h is already
included, then there's no need for virsocket.h
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When using blockdev configurations the 'device' argument of
'blockdev-mirror' must correspond to the topmost node in the block node
graph. Libvirt didn't do this properly in case when 'copy_on_read'
option was enabled on the disk.
Use qemuDomainDiskGetTopNodename to fix it for the blockdev-mirror calls
in qemuDomainBlockCopy and the non-shared-storage migration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
To simplify implementation, some restrictions are added. For
instance, an NVMe disk can't go to any bus but virtio and has to
be type of 'disk' and can't have startupPolicy set.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
There are going to be more disk types that are considered unsafe
with respect to migration. Therefore, move the error reporting
call outside of if() body and rework if-else combo to switch().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
We will want to use the async job infrastructure along with all the APIs
and event for the backup job so add the backup job as a new async job
type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The NUMA cells are stored directly in the virCapsHostPtr
struct. This moves them into their own struct allowing
them to be stored independantly of the rest of the host
capabilities. The change is used as an excuse to switch
the representation to use a GPtrArray too.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Our normal practice is for the object type to be the name prefix, and
the object instance be the first parameter passed in.
Rename these to virDomainObjSave and virDomainDefSave moving their
primary parameter to be the first one. Ensure that the xml options
are passed into both functions in prep for future work.
Finally enforce checking of the return type and mark all parameters
as non-NULL.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
With blockdev we need to refer to the nodename of the disk source image
as the source argument for the blockdev-mirror operation while still
keeping the old job name. With blockdev we must also persist the job in
qemu.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Separate out allocation of the virStorageSource corresponding to the
target NBD export of the migration.
As part of the splitout we allocate the export name explicitly as that
one must not change regardless whether blockdev is used or not to
provide compatibility.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Now that the cleanup section does not exist remove the label.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
qemuMigrationSrcNBDCopyCancelOne uses the block job data structure but
generated it's own job name rather than taking it from the block job
data.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
With -blockdev we must use the nodename as the export but we must keep
the name of the export as it was before to ensure compatiblity.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Declare the variable inside the loop with automatic clearing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
For managed save we can choose between various compression
methods. I randomly tested the 'xz' program on a 8 GB guest
and was surprised to have to wait > 50 minutes for it to
finish compressing, with 'xz' burning 100% cpu for the
entire time. Despite the impressive compression, this is
completely useless in the real world as it is far too long
to wait to save the VM.
The 'xz' binary defaults to '-6' optimization level which
aims for high compression, with moderate memory usage,
at the expense of speed.
This change switches it to use the '-3' optimization level
which is documented as being the one that optimizes speed
at expense of compression. Even with this, it will still
outperform all the other options in terms of compression
level. It is a little less than x4 faster than '-6' which
means it starts to be a viable choice to use 'xz' for
people who really want best compression.
The test results on a 1 GB, fairly freshly booted VM are
as follows
format | save | restore size
=======+=======+=============
raw | 05s | 1s | 428 MB
lzop | 05s | 3s | 160 MB
gzip | 29s | 5s | 118 MB
bz2 | 54s | 22s | 114 MB
xz | 4m37s | 13s | 86 MB
xz -3 | 1m20s | 12s | 95 MB
Based on this we can say
* For moderate compression with no noticable loss in speed
=> use lzop
* For high compression with moderate loss in speed
=> use gzip
* For best compression with significant loss in speed
=> use xz
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This previous commit introduced a simpler free callback for
hash data with only 1 arg, the value to free:
commit 49288fac96
Author: Peter Krempa <pkrempa@redhat.com>
Date: Wed Oct 9 15:26:37 2019 +0200
util: hash: Add possibility to use simpler data free function in virHash
It missed two functions in the hash table code which need
to call the alternate data free function, virHashRemoveEntry
and virHashRemoveSet.
After the previous patch though, there is no code that
makes functional use of the 2nd key arg in the data
free function. There is merely one log message that can
be dropped.
We can thus purge the current virHashDataFree callback
entirely, and rename virHashDataFreeSimple to replace
it.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
In many cases we used virDomainDiskByName to solely look up disk by
target. We have a new helper now so we can replace it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Replace all occurrences of
if (VIR_STRDUP(a, b) < 0)
/* effectively dead code */
with:
a = g_strdup(b);
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Replace all the occurrences of
ignore_value(VIR_STRDUP(a, b));
with
a = g_strdup(b);
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Provide some consistency over error message variable name and usage
when saving error messages across possible other errors or possibility
of resetting of the last error.
Instead of virSaveLastError paired up with virSetError and virFreeError,
we should use the newer virErrorPreserveLast and virRestoreError.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Now that all the types using VIR_AUTOUNREF have a cleanup func defined
to virObjectUnref, use g_autoptr instead of VIR_AUTOUNREF.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since commit 44e7f02915
util: rewrite auto cleanup macros to use glib's equivalent
VIR_AUTOPTR aliases to g_autoptr. Replace all of its use by the GLib
macro version.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Since commit 44e7f02915
util: rewrite auto cleanup macros to use glib's equivalent
VIR_AUTOFREE is just an alias for g_autofree. Use the GLib macros
directly instead of our custom aliases.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Also define the macro for building with GLib older than 2.60
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use G_GNUC_UNUSED from GLib instead of ATTRIBUTE_UNUSED.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Semantically we can't guarantee that we'll be able to destroy the VM on
the remote host, thus we can't allow remote migration. All other forms
of migration (e.g. saving to file) are okay though as they don't clash
with semantics of the flag.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
When blockdev is used we always should use the blockdev mode for
non-shared storage migration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Kristina Hanicova