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>
We can now replace the existing NVRAM file on startup when
the API requests this.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
'virDomainSnapshotRedefinePrep' does everything needed for a redefine
when the snapshot exists but not when we are defining metadata for a new
snapshot. This gives us weird semantics.
Extract the code for replacing the definition of an existing snapshot
into a new helper 'virDomainSnapshotReplaceDef' and refactor all
callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Rather than callers second-guessing when the snapshot definition is
assigned turn it into a double pointer and clear it on success.
Fix callers to work with the new semantics.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As with qemuSnapshotRedefine, make an extra reference in a temporary
autocleaned variable and use that instead of refing the definition after
it's stolen.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Due to historical reasons we allow users to redefine an existing
snapshot without providing the domain definition which would correspond
to it. In such case we'd use the domain definition from the snapshot
that is being redefined.
To prevent callers from doing complex moving of the domain definition
object back and forth between the snapshot definitions we can add an
argument to virDomainSnapshotAlignDisks which will allow us to pass in
the alternate definition if the one from the snapshot is missing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'require_match' set to true is only needed for internal snapshots taken
by hypervisors (qemu) which don't have a way to control which disks take
part in the snapshot (savevm).
To de-clutter callers we can change the argument to mean 'this code path
requires uniform snapshot for internal snapshots'.
Change the argument and fix the callers. For now all callers pass 'true'
but any new hypervisor or even usage in qemu is not going to share the
limitation.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use the appropriate type for the variable and fix all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit f4aae9726d factored out the snapshot redefinition code into a
separate function, but didn't account for the fact that the code is
consuming the reference to the snapshot definition and by moving the
code away the caller (qemuSnapshotCreateXML) now frees the definition
which didn't happen before as we cleared the pointer.
Fix it by increasing the reference locally. Later patches will refactor
the code so that it's more obvious what's happening.
Fixes: f4aae9726d
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2039651
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'def' is commonly used to refer to domain definition. Most of the
snapshot code uses 'snapdef' for the snapshot definition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Our approach to snapshots without metadata was to insert them to the
snapshot list and then later remove them from the list when the flag is
present.
This quirky logic was broken in a recent refactor of the snapshot code
causing that the snapshot stayed inserted in the snapshot list.
Recent refactor of the snapshot code didn't faithfully relocate this
logic to the new function.
Rather than attempting to restore the quirky logic of adding and then
removing the object, don't add the snapshot into the list at all when
the user doesn't want metadata.
We achieve this by creating a temporary 'virDomainMomentObj' wrapper
which is not inserted into the list and using that instead of calling
virDomainSnapshotAssignDef.
Fixes: 9bad0fb809
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2039131
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Automatically free 'cmd' and 'created' by moving them to the appropriate
scopes.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit <f33ce12e9cd9cab7e6022e91d3765c33d99bf777> dropped unused code
but missed one variable.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@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>
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>
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>
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>
