Re: [libvirt PATCH 0/7] docs: ci: Update the CI pages with fresh contents
On Wed, Sep 27, 2023 at 08:53:41PM +0200, Erik Skultety wrote: > While we have already descriptive articles on our GitLab CI, there's recently > been some work on the CI front where a few sections deserve some updates. > > Erik Skultety (7): > docs: ci-runners: Add a note on a new runner registration process > docs: ci: Update the description on the integration CI GitLab > variables > docs: testtck: Tweak the Avocado command to run TCK test suite > docs: testtck: Improve the documentation on how to get a VM from > lcitool > docs: testing: Adjust the docs on how to run container workloads > locally > docs: testtck: Add a clear note on libvirt + Perl bindings dependency > docs: testtck: Expand the 'Run TCK' section on making use of > ci/jobs.sh With the issues fixed: Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
Re: [libvirt PATCH 7/7] docs: testtck: Expand the 'Run TCK' section on making use of ci/jobs.sh
On Wed, Sep 27, 2023 at 08:53:48PM +0200, Erik Skultety wrote: > Ever since commit 6e9bd600 added a new GitLab job description function Same here, please use full commit hash. > handling the integration test suite process to ci/jobs.sh it should be > mentioned in the docs. > This patch splits the 'Run TCK' section in two, giving user the option > to run the integration test suite in their VM environment the same way > as we do in GitLab CI or execute everything manually. > This patch takes the opportunity to also link to the virtiofs kbase > article to give users a different option to get the local libvirt > repositories to be used in testing inside a VM. > > Signed-off-by: Erik Skultety Pavel signature.asc Description: PGP signature
Re: [libvirt PATCH 5/7] docs: testing: Adjust the docs on how to run container workloads locally
On Wed, Sep 27, 2023 at 08:53:46PM +0200, Erik Skultety wrote: > The fact that we need ci/helper script to run the workloads remains > true, but the invocation has changed as of commit eb41e456 . We also Use full commit hash as in the future the short one might not be unique. > extracted GitLab job specs into a standalone ci/jobs.sh script which > allows execution of any container job we run in upstream CI locally, > unlike the original functionality which only allowed builds, tests and > shell (although important to say it could be adjusted with the right > meson/ninja args). > lcitool also became mandatory as it enables the container execution > which replaced a Makefile we used to have for this purpose. > > Signed-off-by: Erik Skultety > --- > docs/testing.rst | 37 + > 1 file changed, 29 insertions(+), 8 deletions(-) > > diff --git a/docs/testing.rst b/docs/testing.rst > index a597c3ed07..9ca47072ba 100644 > --- a/docs/testing.rst > +++ b/docs/testing.rst > @@ -48,11 +48,17 @@ Running container builds with GitLab CI > As long as your GitLab account has CI minutes available, pipelines will run > automatically on every branch push to your fork. > > -Running container builds locally > - > +Running container jobs locally > +~~ > > -In order to run container builds locally, we have a ``helper`` script inside > -the ``ci`` directory that can pull, build, and test (if applicable) changes > on > +GitLab CI configuration file is the only source of truth when it comes to > +various job specifications we execute as part of the upstream pipeline. > +Luckily, all "script" (i.e. Bash scripts) were extracted to standalone Shell > +functions in ``ci/jobs.sh``. This allows users to run any of the container > +GitLab job specifications locally by just referencing the job name. > + > +When it comes to actually running the GitLab jobs locally, we have a > +``ci/helper`` script can pull, build, and test (if applicable) changes on /script/script that/ Pavel signature.asc Description: PGP signature
Re: [libvirt PATCH 26/42] systemd: Switch virtchd to common templates
On Mon, Sep 25, 2023 at 08:58:24PM +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani
> ---
> src/ch/meson.build| 27
> src/ch/virtchd.service.in | 44 ---
> 2 files changed, 23 insertions(+), 48 deletions(-)
> delete mode 100644 src/ch/virtchd.service.in
>
> diff --git a/src/ch/meson.build b/src/ch/meson.build
> index dc08069dcd..f6c443f3c6 100644
> --- a/src/ch/meson.build
> +++ b/src/ch/meson.build
> @@ -57,11 +57,30 @@ if conf.has('WITH_CH')
>
>virt_daemon_units += {
> 'service': 'virtchd',
> -'service_in': files('virtchd.service.in'),
> 'name': 'Libvirt ch',
> -'socket_in': libvirtd_socket_in,
> -'socket_ro_in': libvirtd_socket_ro_in,
> -'socket_admin_in': libvirtd_socket_admin_in,
> +'service_unit_extra': [
> + 'Wants=systemd-machined.service',
> + 'After=systemd-machined.service',
> + 'After=remote-fs.target',
> +],
> +'service_service_extra': [
> + 'KillMode=process',
> + '# Raise hard limits to match behaviour of systemd >= 240.',
> + '# During startup, daemon will set soft limit to match hard limit',
> + '# per systemd recommendations',
> + 'LimitNOFILE=1024:524288',
> + '# The cgroups pids controller can limit the number of tasks started
> by',
> + '# the daemon, which can limit the number of domains for some
> hypervisors.',
> + '# A conservative default of 8 tasks per guest results in a TasksMax
> of',
> + '# 32k to support 4096 guests.',
> + 'TasksMax=32768',
> + '# With cgroups v2 there is no devices controller anymore, we have to
> use',
> + '# eBPF to control access to devices. In order to do that we create a
> eBPF',
> + '# hash MAP which locks memory. The default map size for 64 devices
> together',
> + '# with program takes 12k per guest. After rounding up we will get
> 64M to',
> + '# support 4096 guests.',
> + 'LimitMEMLOCK=64M',
> +],
This feels wrong to have it in meson.build file. In addition it is the
same as for virtlxcd and virtqemud so we are basically duplicating the
data and which makes it easy to make inconsistent changes not affecting
all places.
IMHO it would be better to have additional file that will be included
into the template for services where we need it.
I'm not sure about the `service_unit_extra` as well if we want to have
it in meson.build files as it is not strictly related to the build
process and there is more data compared to the old `deps`.
Pavel
>}
>
>virt_install_dirs += [
> diff --git a/src/ch/virtchd.service.in b/src/ch/virtchd.service.in
> deleted file mode 100644
> index 351eee312b..00
> --- a/src/ch/virtchd.service.in
> +++ /dev/null
> @@ -1,44 +0,0 @@
> -[Unit]
> -Description=Virtualization Cloud-Hypervisor daemon
> -Conflicts=libvirtd.service
> -Requires=virtchd.socket
> -Requires=virtchd-ro.socket
> -Requires=virtchd-admin.socket
> -Wants=systemd-machined.service
> -After=network.target
> -After=dbus.service
> -After=apparmor.service
> -After=remote-fs.target
> -After=systemd-machined.service
> -Documentation=man:virtchd(8)
> -Documentation=https://libvirt.org
> -
> -[Service]
> -Type=notify
> -Environment=VIRTCHD_ARGS="--timeout 120"
> -EnvironmentFile=-@initconfdir@/virtchd
> -ExecStart=@sbindir@/virtchd $VIRTCHD_ARGS
> -ExecReload=/bin/kill -HUP $MAINPID
> -KillMode=process
> -Restart=on-failure
> -# Raise hard limits to match behaviour of systemd >= 240.
> -# During startup, daemon will set soft limit to match hard limit
> -# per systemd recommendations
> -LimitNOFILE=1024:524288
> -# The cgroups pids controller can limit the number of tasks started by
> -# the daemon, which can limit the number of domains for some hypervisors.
> -# A conservative default of 8 tasks per guest results in a TasksMax of
> -# 32k to support 4096 guests.
> -TasksMax=32768
> -# With cgroups v2 there is no devices controller anymore, we have to use
> -# eBPF to control access to devices. In order to do that we create a eBPF
> -# hash MAP which locks memory. The default map size for 64 devices together
> -# with program takes 12k per guest. After rounding up we will get 64M to
> -# support 4096 guests.
> -LimitMEMLOCK=64M
> -
> -[Install]
> -WantedBy=multi-user.target
> -Also=virtchd.socket
> -Also=virtchd-ro.socket
> -Also=virtchd-admin.socket
> --
> 2.41.0
>
signature.asc
Description: PGP signature
Re: [libvirt PATCH v8 10/37] qemu: add functions to start and stop nbdkit
On Thu, Aug 31, 2023 at 04:39:50PM -0500, Jonathon Jongsma wrote:
> Add some helper functions to build a virCommand object and run the
> nbdkit process for a given virStorageSource.
>
> Signed-off-by: Jonathon Jongsma
> Reviewed-by: Peter Krempa
> ---
> src/qemu/qemu_nbdkit.c | 250 +
> src/qemu/qemu_nbdkit.h | 10 ++
> 2 files changed, 260 insertions(+)
>
> diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
> index 9a2a89224d..6bf962d0f1 100644
> --- a/src/qemu/qemu_nbdkit.c
> +++ b/src/qemu/qemu_nbdkit.c
> @@ -24,6 +24,8 @@
> #include "virerror.h"
> #include "virlog.h"
> #include "virpidfile.h"
> +#include "virsecureerase.h"
> +#include "virtime.h"
> #include "virutil.h"
> #include "qemu_block.h"
> #include "qemu_conf.h"
> @@ -666,6 +668,168 @@ qemuNbdkitInitStorageSource(qemuNbdkitCaps *caps,
> }
>
>
> +static int
> +qemuNbdkitProcessBuildCommandCurl(qemuNbdkitProcess *proc,
> + virCommand *cmd)
> +{
> +g_autoptr(virURI) uri = qemuBlockStorageSourceGetURI(proc->source);
> +g_autofree char *uristring = virURIFormat(uri);
> +
> +/* nbdkit plugin name */
> +virCommandAddArg(cmd, "curl");
> +if (proc->source->protocol == VIR_STORAGE_NET_PROTOCOL_HTTP) {
> +/* allow http to be upgraded to https via e.g. redirect */
> +virCommandAddArgPair(cmd, "protocols", "http,https");
> +} else {
> +virCommandAddArgPair(cmd, "protocols",
> +
> virStorageNetProtocolTypeToString(proc->source->protocol));
> +}
> +virCommandAddArgPair(cmd, "url", uristring);
> +
> +if (proc->source->auth) {
> +g_autoptr(virConnect) conn = virGetConnectSecret();
> +g_autofree uint8_t *secret = NULL;
> +size_t secretlen = 0;
> +g_autofree char *password = NULL;
> +int secrettype;
> +virStorageAuthDef *authdef = proc->source->auth;
> +
> +virCommandAddArgPair(cmd, "user",
> + proc->source->auth->username);
> +
> +if ((secrettype =
> virSecretUsageTypeFromString(proc->source->auth->secrettype)) < 0) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("invalid secret type %1$s"),
> + proc->source->auth->secrettype);
> +return -1;
> +}
> +
> +if (virSecretGetSecretString(conn,
> + >seclookupdef,
> + secrettype,
> + ,
> + ) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("failed to get auth secret for storage"));
> +return -1;
> +}
> +
> +/* ensure that the secret is a NULL-terminated string */
> +password = g_strndup((char*)secret, secretlen);
> +virSecureErase(secret, secretlen);
> +
> +/* for now, just report an error rather than passing the password in
> + * cleartext on the commandline */
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Password not yet supported for nbdkit sources"));
> +
> +virSecureEraseString(password);
> +
> +return -1;
> +}
> +
> +if (proc->source->ncookies > 0) {
> +/* for now, just report an error rather than passing cookies in
> + * cleartext on the commandline */
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Cookies not yet supported for nbdkit sources"));
> +return -1;
> +}
> +
> +if (proc->source->sslverify == VIR_TRISTATE_BOOL_NO) {
> +virCommandAddArgPair(cmd, "sslverify", "false");
> +}
> +
> +if (proc->source->timeout > 0) {
> +g_autofree char *timeout = g_strdup_printf("%llu",
> proc->source->timeout);
> +virCommandAddArgPair(cmd, "timeout", timeout);
> +}
> +
> +return 0;
> +}
> +
> +
> +static int
> +qemuNbdkitProcessBuildCommandSSH(qemuNbdkitProcess *proc,
> + virCommand *cmd)
> +{
> +const char *user = NULL;
> +virStorageNetHostDef *host = >source->hosts[0];
> +g_autofree char *portstr = g_strdup_printf("%u", host->port);
> +
> +/* nbdkit plugin name */
> +virCommandAddArg(cmd, "ssh");
> +
> +virCommandAddArgPair(cmd, "host", host->name);
> +virCommandAddArgPair(cmd, "port", portstr);
> +virCommandAddArgPair(cmd, "path", proc->source->path);
> +
> +if (proc->source->auth)
> +user = proc->source->auth->username;
> +else if (proc->source->ssh_user)
> +user = proc->source->ssh_user;
> +
> +if (user)
> +virCommandAddArgPair(cmd, "user", user);
> +
> +if (proc->source->ssh_host_key_check_disabled)
> +virCommandAddArgPair(cmd, "verify-remote-host", "false");
> +
> +return 0;
> +}
> +
> +
> +static virCommand *
[libvirt PATCH] qemu_nbdkit: fix possible null dereference
Function virGetConnectSecret() can return NULL so we need to check it since in virSecretGetSecretString() it gets dereferenced. Reported-by: coverity Signed-off-by: Pavel Hrdina --- src/qemu/qemu_nbdkit.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c index 66b09cd240..17819ca992 100644 --- a/src/qemu/qemu_nbdkit.c +++ b/src/qemu/qemu_nbdkit.c @@ -971,6 +971,9 @@ qemuNbdkitProcessBuildCommandAuth(virStorageAuthDef *authdef, } conn = virGetConnectSecret(); +if (!conn) +return -1; + if (virSecretGetSecretString(conn, >seclookupdef, secrettype, -- 2.41.0
[libvirt PATCH v3 10/10] NEWS: document support for reverting external snapshots
Signed-off-by: Pavel Hrdina --- NEWS.rst | 8 1 file changed, 8 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 926620b03f..940e6e348a 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -23,6 +23,14 @@ v9.8.0 (unreleased) * **New features** + * QEMU: implement reverting external snapshots + +Reverting external snapshots is now possible using the existing API +``virDomainSnapshotRevert()``. Management application can check host +capabilities for element within the list of +guest features to see if the current libvirt supports both deleting +and reverting external snapshots. + * **Improvements** * **Bug fixes** -- 2.41.0
[libvirt PATCH v3 07/10] qemu_snapshot: fix reverting external snapshot when not all disks are included
We need to skip all disks that have snapshot type other than 'external'.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 12
1 file changed, 12 insertions(+)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index cdc8e12cff..44bd97e564 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2054,6 +2054,9 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm,
virDomainSnapshotDiskDef *snapdisk = >disks[i];
virDomainDiskDef *domdisk = domdef->disks[i];
+if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
+
if (qemuSnapshotPrepareDiskExternal(domdisk, snapdisk, active, false)
< 0)
return -1;
}
@@ -2094,6 +2097,9 @@ qemuSnapshotRevertExternalActive(virDomainObj *vm,
return -1;
for (i = 0; i < tmpsnapdef->ndisks; i++) {
+if (tmpsnapdef->disks[i].snapshot !=
VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
+
if (qemuSnapshotDiskPrepareOne(snapctxt,
vm->def->disks[i],
tmpsnapdef->disks + i,
@@ -2184,6 +2190,9 @@ qemuSnapshotRevertExternalFinish(virDomainObj *vm,
for (i = 0; i < curdef->nrevertdisks; i++) {
virDomainSnapshotDiskDef *snapdisk = &(curdef->revertdisks[i]);
+if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
+
if (virStorageSourceInit(snapdisk->src) < 0 ||
virStorageSourceUnlink(snapdisk->src) < 0) {
VIR_WARN("Failed to remove snapshot image '%s'",
@@ -2199,6 +2208,9 @@ qemuSnapshotRevertExternalFinish(virDomainObj *vm,
for (i = 0; i < curdef->ndisks; i++) {
virDomainSnapshotDiskDef *snapdisk = &(curdef->disks[i]);
+if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
+
if (virStorageSourceInit(snapdisk->src) < 0 ||
virStorageSourceUnlink(snapdisk->src) < 0) {
VIR_WARN("Failed to remove snapshot image '%s'",
--
2.41.0
[libvirt PATCH v3 09/10] capabilities: report full external snapshot support
Now that deleting and reverting external snapshots is implemented we can
report that in capabilities so management applications can use that
information and start using external snapshots.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
docs/formatcaps.rst| 7 +++
src/conf/capabilities.c| 1 +
src/conf/capabilities.h| 1 +
src/conf/schemas/capability.rng| 5 +
src/qemu/qemu_capabilities.c | 1 +
tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 +
tests/qemucaps2xmloutdata/caps.aarch64.xml | 1 +
tests/qemucaps2xmloutdata/caps.ppc.xml | 1 +
tests/qemucaps2xmloutdata/caps.ppc64.xml | 1 +
tests/qemucaps2xmloutdata/caps.riscv64.xml | 1 +
tests/qemucaps2xmloutdata/caps.s390x.xml | 1 +
tests/qemucaps2xmloutdata/caps.sparc.xml | 1 +
tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml | 1 +
tests/qemucaps2xmloutdata/caps.x86_64.xml | 1 +
14 files changed, 24 insertions(+)
diff --git a/docs/formatcaps.rst b/docs/formatcaps.rst
index bb8bc663d2..95502c511f 100644
--- a/docs/formatcaps.rst
+++ b/docs/formatcaps.rst
@@ -134,6 +134,12 @@ The element will typically wrap up the
following elements:
creating external disk snapshots is supported. If absent, creating
external
snapshots may still be supported, but it requires attempting the API and
checking for an error to find out for sure. :since:`Since 1.2.3`
+ ``externalSnapshot``
+ If this element is present, the hypervisor supports deleting and
+ reverting external snapshots including memory state. Support for creation
+ of external snapshots is reported via the ``disksnapshot`` feature flag.
+ Management applications can now switch from internal snapshots to
external
+ snapshots. :since:`Since 9.7.0`
Examples
@@ -318,6 +324,7 @@ capabilities enabled in the chip and BIOS you will see:
+
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 56768ce6e0..34f04cb7d3 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -508,6 +508,7 @@ static const struct virCapsGuestFeatureInfo
virCapsGuestFeatureInfos[VIR_CAPS_GU
[VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT] = { "deviceboot", false },
[VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT] = { "disksnapshot", true },
[VIR_CAPS_GUEST_FEATURE_TYPE_HAP] = { "hap", true },
+[VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT] = { "externalSnapshot",
false },
};
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index c78e3e52fa..9eaf6e2807 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -40,6 +40,7 @@ typedef enum {
VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT,
VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT,
VIR_CAPS_GUEST_FEATURE_TYPE_HAP,
+VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT,
VIR_CAPS_GUEST_FEATURE_TYPE_LAST
} virCapsGuestFeatureType;
diff --git a/src/conf/schemas/capability.rng b/src/conf/schemas/capability.rng
index 83b414961a..b1968df258 100644
--- a/src/conf/schemas/capability.rng
+++ b/src/conf/schemas/capability.rng
@@ -493,6 +493,11 @@
+
+
+
+
+
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 3a1bfbf74d..83119e871a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1122,6 +1122,7 @@ virQEMUCapsInitGuestFromBinary(virCaps *caps,
virCapabilitiesAddGuestFeature(guest,
VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT);
virCapabilitiesAddGuestFeatureWithToggle(guest,
VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT,
true, false);
+virCapabilitiesAddGuestFeature(guest,
VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT);
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_TCG)) {
virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_QEMU,
diff --git a/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
b/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
index b53a886b90..b9a5b5a1d6 100644
--- a/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
+++ b/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
@@ -21,6 +21,7 @@
+
diff --git a/tests/qemucaps2xmloutdata/caps.aarch64.xml
b/tests/qemucaps2xmloutdata/caps.aarch64.xml
index 5dca6d3102..61512ed740 100644
--- a/tests/qemucaps2xmloutdata/caps.aarch64.xml
+++ b/tests/qemucaps2xmloutdata/caps.aarch64.xml
@@ -21,6 +21,7 @@
+
diff --git a/tests/qemucaps2xmloutdata/caps.ppc.xml
b/tests/qemucaps2xmloutdata/caps.ppc.xml
index e7ba391795..86d6b85ae0 100644
--- a/tests/qemucaps2xmloutdata/caps.ppc.xml
+++ b/tests/qemucaps2xmloutdata/caps.ppc.xml
@@ -19,6 +19,7 @@
[libvirt PATCH v3 05/10] qemuProcessStartWithMemoryState: add snapshot argument
When called from snapshot code we will need to pass snapshot object in
order to make internal snapshots work correctly.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_process.c | 9 -
src/qemu/qemu_process.h | 1 +
src/qemu/qemu_saveimage.c | 2 +-
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d414a40fd5..c8430bf7b7 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8164,6 +8164,7 @@ qemuProcessStart(virConnectPtr conn,
* @vm: domain object
* @fd: FD pointer of memory state file
* @path: path to memory state file
+ * @snapshot: internal snapshot to load when starting QEMU process or NULL
* @data: data from memory state file
* @asyncJob: type of asynchronous job
* @start_flags: flags to start QEMU process with
@@ -8173,6 +8174,11 @@ qemuProcessStart(virConnectPtr conn,
* Start VM with existing memory state. Make sure that the stored memory state
* is correctly decompressed so it can be loaded by QEMU process.
*
+ * When reverting to internal snapshot callers needs to pass @snapshot as well
+ * correctly start QEMU process.
+ *
+ * When restoring VM from saved image @snapshot needs to be NULL.
+ *
* For audit purposes the expected @reason is one of `restored` or
`from-snapshot`.
*
* Returns 0 on success, -1 on error.
@@ -8183,6 +8189,7 @@ qemuProcessStartWithMemoryState(virConnectPtr conn,
virDomainObj *vm,
int *fd,
const char *path,
+virDomainMomentObj *snapshot,
virQEMUSaveData *data,
virDomainAsyncJob asyncJob,
unsigned int start_flags,
@@ -8214,7 +8221,7 @@ qemuProcessStartWithMemoryState(virConnectPtr conn,
priv->disableSlirp = true;
if (qemuProcessStart(conn, driver, vm, cookie ? cookie->cpu : NULL,
- asyncJob, "stdio", *fd, path, NULL,
+ asyncJob, "stdio", *fd, path, snapshot,
VIR_NETDEV_VPORT_PROFILE_OP_RESTORE,
start_flags) == 0)
*started = true;
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 26c9a04969..ce2f25a2e7 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -96,6 +96,7 @@ int qemuProcessStartWithMemoryState(virConnectPtr conn,
virDomainObj *vm,
int *fd,
const char *path,
+virDomainMomentObj *snapshot,
virQEMUSaveData *data,
virDomainAsyncJob asyncJob,
unsigned int start_flags,
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 92dcf4b616..89112e3e44 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -698,7 +698,7 @@ qemuSaveImageStartVM(virConnectPtr conn,
if (reset_nvram)
start_flags |= VIR_QEMU_PROCESS_START_RESET_NVRAM;
-if (qemuProcessStartWithMemoryState(conn, driver, vm, fd, path, data,
+if (qemuProcessStartWithMemoryState(conn, driver, vm, fd, path, NULL, data,
asyncJob, start_flags, "restored",
) < 0) {
goto cleanup;
--
2.41.0
[libvirt PATCH v3 08/10] qemu_snapshot: correctly load the saved memory state file
Original code assumed that the memory state file is only migration
stream but it has additional metadata stored by libvirt. To correctly
load the memory state file we need to reuse code that is used when
restoring domain from saved image.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 79
1 file changed, 47 insertions(+), 32 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 44bd97e564..e52d264826 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2000,6 +2000,21 @@ qemuSnapshotRevertWriteMetadata(virDomainObj *vm,
}
+typedef struct _qemuSnapshotRevertMemoryData {
+int fd;
+char *path;
+virQEMUSaveData *data;
+} qemuSnapshotRevertMemoryData;
+
+static void
+qemuSnapshotClearRevertMemoryData(qemuSnapshotRevertMemoryData *memdata)
+{
+VIR_FORCE_CLOSE(memdata->fd);
+virQEMUSaveDataFree(memdata->data);
+}
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(qemuSnapshotRevertMemoryData,
qemuSnapshotClearRevertMemoryData);
+
+
/**
* qemuSnapshotRevertExternalPrepare:
* @vm: domain object
@@ -2007,15 +2022,13 @@ qemuSnapshotRevertWriteMetadata(virDomainObj *vm,
* @snap: snapshot object we are reverting to
* @config: live domain definition
* @inactiveConfig: offline domain definition
- * memsnapFD: pointer to store memory state file FD or NULL
- * memsnapPath: pointer to store memory state file path or NULL
+ * @memdata: struct with data to load memory state
*
* Prepare new temporary snapshot definition @tmpsnapdef that will
* be used while creating new overlay files after reverting to snapshot
* @snap. In case we are reverting to snapshot with memory state it will
- * open it and pass FD via @memsnapFD and path to the file via
- * @memsnapPath, caller is responsible for freeing both @memsnapFD and
- * memsnapPath.
+ * open it and store necessary data in @memdata. Caller is responsible
+ * to clear the data by using qemuSnapshotClearRevertMemoryData().
*
* Returns 0 in success, -1 on error.
*/
@@ -2025,8 +2038,7 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm,
virDomainMomentObj *snap,
virDomainDef *config,
virDomainDef *inactiveConfig,
- int *memsnapFD,
- char **memsnapPath)
+ qemuSnapshotRevertMemoryData *memdata)
{
size_t i;
bool active = virDomainObjIsActive(vm);
@@ -2061,12 +2073,21 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm,
return -1;
}
-if (memsnapFD && memsnapPath && snapdef->memorysnapshotfile) {
+if (memdata && snapdef->memorysnapshotfile) {
virQEMUDriver *driver = ((qemuDomainObjPrivate *)
vm->privateData)->driver;
-g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virDomainDef) savedef = NULL;
-*memsnapPath = snapdef->memorysnapshotfile;
-*memsnapFD = qemuDomainOpenFile(cfg, NULL, *memsnapPath, O_RDONLY,
NULL);
+memdata->path = snapdef->memorysnapshotfile;
+memdata->fd = qemuSaveImageOpen(driver, NULL, memdata->path,
+, >data,
+false, NULL,
+false, false);
+
+if (memdata->fd < 0)
+return -1;
+
+if (!virDomainDefCheckABIStability(savedef, domdef, driver->xmlopt))
+return -1;
}
return 0;
@@ -2250,13 +2271,12 @@ qemuSnapshotRevertActive(virDomainObj *vm,
virObjectEvent *event = NULL;
virObjectEvent *event2 = NULL;
virDomainMomentObj *loadSnap = NULL;
-VIR_AUTOCLOSE memsnapFD = -1;
-char *memsnapPath = NULL;
int detail;
bool defined = false;
-qemuDomainSaveCookie *cookie = (qemuDomainSaveCookie *) snapdef->cookie;
int rc;
g_autoptr(virDomainSnapshotDef) tmpsnapdef = NULL;
+g_auto(qemuSnapshotRevertMemoryData) memdata = { -1, NULL, NULL };
+bool started = false;
start_flags |= VIR_QEMU_PROCESS_START_PAUSED;
@@ -2280,7 +2300,7 @@ qemuSnapshotRevertActive(virDomainObj *vm,
if (qemuSnapshotRevertExternalPrepare(vm, tmpsnapdef, snap,
*config, *inactiveConfig,
- , ) < 0) {
+ ) < 0) {
return -1;
}
} else {
@@ -2294,28 +2314,24 @@ qemuSnapshotRevertActive(virDomainObj *vm,
virDomainObjAssignDef(vm, config, true, NULL);
-/* No cookie means libvirt which saved the domain was too old to
- * mess up the CPU definitions.
- */
-if (cookie &&
-qemuDomainFixupCPUs(vm,
[libvirt PATCH v3 06/10] qemuProcessStartWithMemoryState: make it possible to use without data
When used with internal snapshots there is no memory state file so we
have no data to load and decompression is not needed.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_process.c | 23 +--
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c8430bf7b7..f96918073f 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8165,7 +8165,7 @@ qemuProcessStart(virConnectPtr conn,
* @fd: FD pointer of memory state file
* @path: path to memory state file
* @snapshot: internal snapshot to load when starting QEMU process or NULL
- * @data: data from memory state file
+ * @data: data from memory state file or NULL
* @asyncJob: type of asynchronous job
* @start_flags: flags to start QEMU process with
* @reason: audit log reason
@@ -8175,7 +8175,8 @@ qemuProcessStart(virConnectPtr conn,
* is correctly decompressed so it can be loaded by QEMU process.
*
* When reverting to internal snapshot callers needs to pass @snapshot as well
- * correctly start QEMU process.
+ * correctly start QEMU process. In addition there is no memory state file so
+ * it's safe to pass NULL as @data.
*
* When restoring VM from saved image @snapshot needs to be NULL.
*
@@ -8203,9 +8204,16 @@ qemuProcessStartWithMemoryState(virConnectPtr conn,
g_autofree char *errbuf = NULL;
int rc = 0;
-if (virSaveCookieParseString(data->cookie, (virObject **),
-
virDomainXMLOptionGetSaveCookie(driver->xmlopt)) < 0)
-return -1;
+if (data) {
+if (virSaveCookieParseString(data->cookie, (virObject **),
+
virDomainXMLOptionGetSaveCookie(driver->xmlopt)) < 0)
+return -1;
+
+if (qemuSaveImageDecompressionStart(data, fd, ,
+, ) < 0) {
+return -1;
+}
+}
if (qemuSaveImageDecompressionStart(data, fd, , ,
) < 0)
return -1;
@@ -8226,7 +8234,10 @@ qemuProcessStartWithMemoryState(virConnectPtr conn,
start_flags) == 0)
*started = true;
-rc = qemuSaveImageDecompressionStop(cmd, fd, , errbuf,
*started, path);
+if (data) {
+rc = qemuSaveImageDecompressionStop(cmd, fd, , errbuf,
+*started, path);
+}
virDomainAuditStart(vm, reason, *started);
if (!*started || rc < 0)
--
2.41.0
[libvirt PATCH v3 03/10] qemu_saveimage: move qemuSaveImageStartProcess to qemu_process
The function will no longer be used only when restoring VM as it will
be used when reverting snapshot as well so move it to qemu_process
and rename it accordingly.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_process.c | 73 +
src/qemu/qemu_process.h | 11 ++
src/qemu/qemu_saveimage.c | 75 ++-
src/qemu/qemu_saveimage.h | 11 --
4 files changed, 86 insertions(+), 84 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 42837c4a8a..0de8ceb244 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8157,6 +8157,79 @@ qemuProcessStart(virConnectPtr conn,
}
+/**
+ * qemuProcessStartWithMemoryState:
+ * @conn: connection object
+ * @driver: qemu driver object
+ * @vm: domain object
+ * @fd: FD pointer of memory state file
+ * @path: path to memory state file
+ * @data: data from memory state file
+ * @asyncJob: type of asynchronous job
+ * @start_flags: flags to start QEMU process with
+ * @started: boolean to store if QEMU process was started
+ *
+ * Start VM with existing memory state. Make sure that the stored memory state
+ * is correctly decompressed so it can be loaded by QEMU process.
+ *
+ * Returns 0 on success, -1 on error.
+ */
+int
+qemuProcessStartWithMemoryState(virConnectPtr conn,
+virQEMUDriver *driver,
+virDomainObj *vm,
+int *fd,
+const char *path,
+virQEMUSaveData *data,
+virDomainAsyncJob asyncJob,
+unsigned int start_flags,
+bool *started)
+{
+qemuDomainObjPrivate *priv = vm->privateData;
+g_autoptr(qemuDomainSaveCookie) cookie = NULL;
+VIR_AUTOCLOSE intermediatefd = -1;
+g_autoptr(virCommand) cmd = NULL;
+g_autofree char *errbuf = NULL;
+int rc = 0;
+
+if (virSaveCookieParseString(data->cookie, (virObject **),
+
virDomainXMLOptionGetSaveCookie(driver->xmlopt)) < 0)
+return -1;
+
+if (qemuSaveImageDecompressionStart(data, fd, , ,
) < 0)
+return -1;
+
+/* No cookie means libvirt which saved the domain was too old to mess up
+ * the CPU definitions.
+ */
+if (cookie &&
+qemuDomainFixupCPUs(vm, >cpu) < 0)
+return -1;
+
+if (cookie && !cookie->slirpHelper)
+priv->disableSlirp = true;
+
+if (qemuProcessStart(conn, driver, vm, cookie ? cookie->cpu : NULL,
+ asyncJob, "stdio", *fd, path, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_RESTORE,
+ start_flags) == 0)
+*started = true;
+
+rc = qemuSaveImageDecompressionStop(cmd, fd, , errbuf,
*started, path);
+
+virDomainAuditStart(vm, "restored", *started);
+if (!*started || rc < 0)
+return -1;
+
+/* qemuProcessStart doesn't unset the qemu error reporting infrastructure
+ * in case of migration (which is used in this case) so we need to reset it
+ * so that the handle to virtlogd is not held open unnecessarily */
+qemuMonitorSetDomainLog(qemuDomainGetMonitor(vm), NULL, NULL, NULL);
+
+return 0;
+}
+
+
int
qemuProcessCreatePretendCmdPrepare(virQEMUDriver *driver,
virDomainObj *vm,
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index cae1b49756..d758b4f51a 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -23,6 +23,7 @@
#include "qemu_conf.h"
#include "qemu_domain.h"
+#include "qemu_saveimage.h"
#include "vireventthread.h"
int qemuProcessPrepareMonitorChr(virDomainChrSourceDef *monConfig,
@@ -90,6 +91,16 @@ int qemuProcessStart(virConnectPtr conn,
virNetDevVPortProfileOp vmop,
unsigned int flags);
+int qemuProcessStartWithMemoryState(virConnectPtr conn,
+virQEMUDriver *driver,
+virDomainObj *vm,
+int *fd,
+const char *path,
+virQEMUSaveData *data,
+virDomainAsyncJob asyncJob,
+unsigned int start_flags,
+bool *started);
+
int qemuProcessCreatePretendCmdPrepare(virQEMUDriver *driver,
virDomainObj *vm,
const char *migrateURI,
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 95fabee907..1fbc7891b1 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -675,77 +675,6 @@ qemuSaveImageOpen(virQEMUDrive
[libvirt PATCH v3 04/10] qemuProcessStartWithMemoryState: allow setting reason for audit log
When called by snapshot code we will need to use different reason.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_process.c | 6 +-
src/qemu/qemu_process.h | 1 +
src/qemu/qemu_saveimage.c | 3 ++-
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0de8ceb244..d414a40fd5 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8167,11 +8167,14 @@ qemuProcessStart(virConnectPtr conn,
* @data: data from memory state file
* @asyncJob: type of asynchronous job
* @start_flags: flags to start QEMU process with
+ * @reason: audit log reason
* @started: boolean to store if QEMU process was started
*
* Start VM with existing memory state. Make sure that the stored memory state
* is correctly decompressed so it can be loaded by QEMU process.
*
+ * For audit purposes the expected @reason is one of `restored` or
`from-snapshot`.
+ *
* Returns 0 on success, -1 on error.
*/
int
@@ -8183,6 +8186,7 @@ qemuProcessStartWithMemoryState(virConnectPtr conn,
virQEMUSaveData *data,
virDomainAsyncJob asyncJob,
unsigned int start_flags,
+const char *reason,
bool *started)
{
qemuDomainObjPrivate *priv = vm->privateData;
@@ -8217,7 +8221,7 @@ qemuProcessStartWithMemoryState(virConnectPtr conn,
rc = qemuSaveImageDecompressionStop(cmd, fd, , errbuf,
*started, path);
-virDomainAuditStart(vm, "restored", *started);
+virDomainAuditStart(vm, reason, *started);
if (!*started || rc < 0)
return -1;
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index d758b4f51a..26c9a04969 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -99,6 +99,7 @@ int qemuProcessStartWithMemoryState(virConnectPtr conn,
virQEMUSaveData *data,
virDomainAsyncJob asyncJob,
unsigned int start_flags,
+const char *reason,
bool *started);
int qemuProcessCreatePretendCmdPrepare(virQEMUDriver *driver,
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 1fbc7891b1..92dcf4b616 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -699,7 +699,8 @@ qemuSaveImageStartVM(virConnectPtr conn,
start_flags |= VIR_QEMU_PROCESS_START_RESET_NVRAM;
if (qemuProcessStartWithMemoryState(conn, driver, vm, fd, path, data,
-asyncJob, start_flags, ) < 0) {
+asyncJob, start_flags, "restored",
+) < 0) {
goto cleanup;
}
--
2.41.0
[libvirt PATCH v3 01/10] qemu_saveimage: extract starting process to qemuSaveImageStartProcess
Part of qemuSaveImageStartVM() function will be used when reverting
external snapshots. To avoid duplicating code and logic extract the
shared bits into separate function.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_saveimage.c | 96 +++
src/qemu/qemu_saveimage.h | 11 +
2 files changed, 78 insertions(+), 29 deletions(-)
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 41310d6a9a..27b95b03ae 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -565,42 +565,50 @@ qemuSaveImageOpen(virQEMUDriver *driver,
return ret;
}
+/**
+ * qemuSaveImageStartProcess:
+ * @conn: connection object
+ * @driver: qemu driver object
+ * @vm: domain object
+ * @fd: FD pointer of memory state file
+ * @path: path to memory state file
+ * @data: data from memory state file
+ * @asyncJob: type of asynchronous job
+ * @start_flags: flags to start QEMU process with
+ * @started: boolean to store if QEMU process was started
+ *
+ * Start VM with existing memory state. Make sure that the stored memory state
+ * is correctly decompressed so it can be loaded by QEMU process.
+ *
+ * Returns 0 on success, -1 on error.
+ */
int
-qemuSaveImageStartVM(virConnectPtr conn,
- virQEMUDriver *driver,
- virDomainObj *vm,
- int *fd,
- virQEMUSaveData *data,
- const char *path,
- bool start_paused,
- bool reset_nvram,
- virDomainAsyncJob asyncJob)
+qemuSaveImageStartProcess(virConnectPtr conn,
+ virQEMUDriver *driver,
+ virDomainObj *vm,
+ int *fd,
+ const char *path,
+ virQEMUSaveData *data,
+ virDomainAsyncJob asyncJob,
+ unsigned int start_flags,
+ bool *started)
{
qemuDomainObjPrivate *priv = vm->privateData;
-int ret = -1;
-bool started = false;
-virObjectEvent *event;
+g_autoptr(qemuDomainSaveCookie) cookie = NULL;
+virQEMUSaveHeader *header = >header;
VIR_AUTOCLOSE intermediatefd = -1;
g_autoptr(virCommand) cmd = NULL;
g_autofree char *errbuf = NULL;
-g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-virQEMUSaveHeader *header = >header;
-g_autoptr(qemuDomainSaveCookie) cookie = NULL;
int rc = 0;
-unsigned int start_flags = VIR_QEMU_PROCESS_START_PAUSED |
-VIR_QEMU_PROCESS_START_GEN_VMID;
-
-if (reset_nvram)
-start_flags |= VIR_QEMU_PROCESS_START_RESET_NVRAM;
if (virSaveCookieParseString(data->cookie, (virObject **),
virDomainXMLOptionGetSaveCookie(driver->xmlopt)) < 0)
-goto cleanup;
+return -1;
if ((header->version == 2) &&
(header->compressed != QEMU_SAVE_FORMAT_RAW)) {
if (!(cmd = qemuSaveImageGetCompressionCommand(header->compressed)))
-goto cleanup;
+return -1;
intermediatefd = *fd;
*fd = -1;
@@ -613,7 +621,7 @@ qemuSaveImageStartVM(virConnectPtr conn,
if (virCommandRunAsync(cmd, NULL) < 0) {
*fd = intermediatefd;
intermediatefd = -1;
-goto cleanup;
+return -1;
}
}
@@ -622,7 +630,7 @@ qemuSaveImageStartVM(virConnectPtr conn,
*/
if (cookie &&
qemuDomainFixupCPUs(vm, >cpu) < 0)
-goto cleanup;
+return -1;
if (cookie && !cookie->slirpHelper)
priv->disableSlirp = true;
@@ -631,12 +639,12 @@ qemuSaveImageStartVM(virConnectPtr conn,
asyncJob, "stdio", *fd, path, NULL,
VIR_NETDEV_VPORT_PROFILE_OP_RESTORE,
start_flags) == 0)
-started = true;
+*started = true;
if (intermediatefd != -1) {
virErrorPtr orig_err = NULL;
-if (!started) {
+if (!*started) {
/* if there was an error setting up qemu, the intermediate
* process will wait forever to write to stdout, so we
* must manually kill it and ignore any error related to
@@ -656,15 +664,45 @@ qemuSaveImageStartVM(virConnectPtr conn,
rc = -1;
}
-virDomainAuditStart(vm, "restored", started);
-if (!started || rc < 0)
-goto cleanup;
+virDomainAuditStart(vm, "restored", *started);
+if (!*started || rc < 0)
+return -1;
/* qemuProcessStart doesn't unset the qemu error reporting infrastructure
* in case of migration (which is used in this case) so we need to reset it
* so that the handle to virtlogd is not held open unnecessarily */
qemuMonitorSetDomainLog(qemuDomain
[libvirt PATCH v3 00/10] external snapshot revert fixes
This fixes reverting external snapshots to not error out in cases where it should work and makes it correctly load the memory state when reverting to snapshot of running VM. This discards v2 completely and makes changes to v1: - moves qemuSaveImageStartProcess to qemu_process as qemuProcessStartWithMemoryState - change it to use cookie from memory state file instead of using cookie from snapshot xml - comments improvements - introduces new helpers to start and stop decompression process - reintroduces capability Pavel Hrdina (10): qemu_saveimage: extract starting process to qemuSaveImageStartProcess qemu_saveimage: introduce helpers to decompress memory state file qemu_saveimage: move qemuSaveImageStartProcess to qemu_process qemuProcessStartWithMemoryState: allow setting reason for audit log qemuProcessStartWithMemoryState: add snapshot argument qemuProcessStartWithMemoryState: make it possible to use without data qemu_snapshot: fix reverting external snapshot when not all disks are included qemu_snapshot: correctly load the saved memory state file capabilities: report full external snapshot support NEWS: document support for reverting external snapshots NEWS.rst | 8 + docs/formatcaps.rst | 7 + src/conf/capabilities.c | 1 + src/conf/capabilities.h | 1 + src/conf/schemas/capability.rng | 5 + src/qemu/qemu_capabilities.c | 1 + src/qemu/qemu_process.c | 95 + src/qemu/qemu_process.h | 13 ++ src/qemu/qemu_saveimage.c | 189 +++--- src/qemu/qemu_saveimage.h | 15 ++ src/qemu/qemu_snapshot.c | 91 ++--- .../qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 + tests/qemucaps2xmloutdata/caps.aarch64.xml| 1 + tests/qemucaps2xmloutdata/caps.ppc.xml| 1 + tests/qemucaps2xmloutdata/caps.ppc64.xml | 1 + tests/qemucaps2xmloutdata/caps.riscv64.xml| 1 + tests/qemucaps2xmloutdata/caps.s390x.xml | 1 + tests/qemucaps2xmloutdata/caps.sparc.xml | 1 + tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml | 1 + tests/qemucaps2xmloutdata/caps.x86_64.xml | 1 + 20 files changed, 328 insertions(+), 107 deletions(-) -- 2.41.0
[libvirt PATCH v3 02/10] qemu_saveimage: introduce helpers to decompress memory state file
These new helpers separates the code from the logic used to start new
QEMU process with memory state and will make it easier to move
qemuSaveImageStartProcess() into qemu_process.c file.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_saveimage.c | 155 +++---
src/qemu/qemu_saveimage.h | 15
2 files changed, 128 insertions(+), 42 deletions(-)
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 27b95b03ae..95fabee907 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -247,6 +247,116 @@ qemuSaveImageGetCompressionCommand(virQEMUSaveFormat
compression)
}
+/**
+ * qemuSaveImageDecompressionStart:
+ * @data: data from memory state file
+ * @fd: pointer to FD of memory state file
+ * @intermediatefd: pointer to FD to store original @fd
+ * @errbuf: error buffer for @retcmd
+ * @retcmd: new virCommand pointer
+ *
+ * Start process to decompress VM memory state from @fd. If decompression
+ * is needed the original FD is stored to @intermediatefd and new FD after
+ * decompression is stored to @fd so caller can use the same variable
+ * in both cases.
+ *
+ * Function qemuSaveImageDecompressionStop() needs to be used to correctly
+ * stop the process and swap FD to the original state.
+ *
+ * Caller is responsible for freeing @retcmd.
+ *
+ * Returns -1 on error, 0 on success.
+ */
+int
+qemuSaveImageDecompressionStart(virQEMUSaveData *data,
+int *fd,
+int *intermediatefd,
+char **errbuf,
+virCommand **retcmd)
+{
+virQEMUSaveHeader *header = >header;
+g_autoptr(virCommand) cmd = NULL;
+
+if (header->version != 2)
+return 0;
+
+if (header->compressed == QEMU_SAVE_FORMAT_RAW)
+return 0;
+
+if (!(cmd = qemuSaveImageGetCompressionCommand(header->compressed)))
+return -1;
+
+*intermediatefd = *fd;
+*fd = -1;
+
+virCommandSetInputFD(cmd, *intermediatefd);
+virCommandSetOutputFD(cmd, fd);
+virCommandSetErrorBuffer(cmd, errbuf);
+virCommandDoAsyncIO(cmd);
+
+if (virCommandRunAsync(cmd, NULL) < 0) {
+*fd = *intermediatefd;
+*intermediatefd = -1;
+return -1;
+}
+
+*retcmd = g_steal_pointer();
+return 0;
+}
+
+
+/**
+ * qemuSaveImageDecompressionStop:
+ * @cmd: virCommand pointer
+ * @fd: pointer to FD of memory state file
+ * @intermediatefd: pointer to FD to store original @fd
+ * @errbuf: error buffer for @cmd
+ * @started: boolean to indicate if QEMU process was started
+ * @path: path to the memory state file
+ *
+ * Stop decompression process and close both @fd and @intermediatefd if
+ * necessary.
+ *
+ * Returns -1 on errro, 0 on success.
+ */
+int
+qemuSaveImageDecompressionStop(virCommand *cmd,
+ int *fd,
+ int *intermediatefd,
+ char *errbuf,
+ bool started,
+ const char *path)
+{
+int rc = 0;
+virErrorPtr orig_err = NULL;
+
+if (*intermediatefd == -1)
+return rc;
+
+if (!started) {
+/* if there was an error setting up qemu, the intermediate
+ * process will wait forever to write to stdout, so we
+ * must manually kill it and ignore any error related to
+ * the process
+ */
+virErrorPreserveLast(_err);
+VIR_FORCE_CLOSE(*intermediatefd);
+VIR_FORCE_CLOSE(*fd);
+}
+
+rc = virCommandWait(cmd, NULL);
+VIR_DEBUG("Decompression binary stderr: %s", NULLSTR(errbuf));
+virErrorRestore(_err);
+
+if (VIR_CLOSE(*fd) < 0) {
+virReportSystemError(errno, _("cannot close file: %1$s"), path);
+rc = -1;
+}
+
+return rc;
+}
+
+
/* Helper function to execute a migration to file with a correct save header
* the caller needs to make sure that the processors are stopped and do all
other
* actions besides saving memory */
@@ -595,7 +705,6 @@ qemuSaveImageStartProcess(virConnectPtr conn,
{
qemuDomainObjPrivate *priv = vm->privateData;
g_autoptr(qemuDomainSaveCookie) cookie = NULL;
-virQEMUSaveHeader *header = >header;
VIR_AUTOCLOSE intermediatefd = -1;
g_autoptr(virCommand) cmd = NULL;
g_autofree char *errbuf = NULL;
@@ -605,25 +714,8 @@ qemuSaveImageStartProcess(virConnectPtr conn,
virDomainXMLOptionGetSaveCookie(driver->xmlopt)) < 0)
return -1;
-if ((header->version == 2) &&
-(header->compressed != QEMU_SAVE_FORMAT_RAW)) {
-if (!(cmd = qemuSaveImageGetCompressionCommand(header->compressed)))
-return -1;
-
-intermediatefd = *fd;
-*fd = -1;
-
-virCommandSetInputFD(cmd, intermediatefd);
-virCommandSetOutputFD(cmd, fd);
-
Re: [libvirt PATCH v2 1/6] qemu_snapshot: fix reverting external snapshot when not all disks are included
On Fri, Sep 01, 2023 at 11:10:43AM +0200, Peter Krempa wrote: > On Fri, Sep 01, 2023 at 10:32:12 +0200, Pavel Hrdina wrote: > > We need to skip all disks that have snapshot type other than 'external'. > > Since the commit message is vague on the specific problem details ... > > > Signed-off-by: Pavel Hrdina > > --- > > src/qemu/qemu_snapshot.c | 12 > > 1 file changed, 12 insertions(+) > > > > diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c > > index d943281e35..ff85d56572 100644 > > --- a/src/qemu/qemu_snapshot.c > > +++ b/src/qemu/qemu_snapshot.c > > @@ -2058,6 +2058,9 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm, > > virDomainSnapshotDiskDef *snapdisk = >disks[i]; > > virDomainDiskDef *domdisk = domdef->disks[i]; > > > > +if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) > > +continue; > > ... I remember you talking about the need to create overlays also for > images which might have been excluded in given snapshot (the one you are > reverting to) , but externally snapshotted in a later (still existing) > one. > > In such case not creating the overlay here would still invalidate the > overlay of the next snapshot. Is that right? > > I also remember that you mentioned that the actual problem is with empty > cdroms, thus, did you rather want to use 'virStorageSourceIsEmpty' here? That is done in virDomainSnapshotAlignDisks() called right before the for loop in qemuSnapshotRevertExternalPrepare(), there we fill in the temporary snapshot definition with new overlays. From that moment the temporary snapshot definition contains all disks based on the VM definition. Looking at the code, when reverting to a snapshot we will always create new overlays for all disks including readonly disks, not sure if that is what we should do or no. Pavel signature.asc Description: PGP signature
[libvirt PATCH v2 2/6] qemu_saveimage: export virQEMUSaveFormat enum
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_saveimage.c | 17 -
src/qemu/qemu_saveimage.h | 17 +
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 41310d6a9a..eca47171c2 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -37,23 +37,6 @@
VIR_LOG_INIT("qemu.qemu_saveimage");
-typedef enum {
-QEMU_SAVE_FORMAT_RAW = 0,
-QEMU_SAVE_FORMAT_GZIP = 1,
-QEMU_SAVE_FORMAT_BZIP2 = 2,
-/*
- * Deprecated by xz and never used as part of a release
- * QEMU_SAVE_FORMAT_LZMA
- */
-QEMU_SAVE_FORMAT_XZ = 3,
-QEMU_SAVE_FORMAT_LZOP = 4,
-/* Note: add new members only at the end.
- These values are used in the on-disk format.
- Do not change or re-use numbers. */
-
-QEMU_SAVE_FORMAT_LAST
-} virQEMUSaveFormat;
-
VIR_ENUM_DECL(qemuSaveCompression);
VIR_ENUM_IMPL(qemuSaveCompression,
QEMU_SAVE_FORMAT_LAST,
diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h
index 30cf4b1ee0..6892e6764f 100644
--- a/src/qemu/qemu_saveimage.h
+++ b/src/qemu/qemu_saveimage.h
@@ -32,6 +32,23 @@
G_STATIC_ASSERT(sizeof(QEMU_SAVE_MAGIC) == sizeof(QEMU_SAVE_PARTIAL));
+typedef enum {
+QEMU_SAVE_FORMAT_RAW = 0,
+QEMU_SAVE_FORMAT_GZIP = 1,
+QEMU_SAVE_FORMAT_BZIP2 = 2,
+/*
+ * Deprecated by xz and never used as part of a release
+ * QEMU_SAVE_FORMAT_LZMA
+ */
+QEMU_SAVE_FORMAT_XZ = 3,
+QEMU_SAVE_FORMAT_LZOP = 4,
+/* Note: add new members only at the end.
+ These values are used in the on-disk format.
+ Do not change or re-use numbers. */
+
+QEMU_SAVE_FORMAT_LAST
+} virQEMUSaveFormat;
+
typedef struct _virQEMUSaveHeader virQEMUSaveHeader;
struct _virQEMUSaveHeader {
char magic[sizeof(QEMU_SAVE_MAGIC)-1];
--
2.41.0
[libvirt PATCH v2 4/6] qemu_snapshot: correctly load the saved memory state file
Original code assumed that the memory state file is only migration
stream but it has additional metadata stored by libvirt. To correctly
load the memory state file we need to reuse code that is used when
restoring domain from saved image.
This duplicates some necessary parts of qemuSaveImageStartVM() because
the external snapshot memory state is done by qemuSaveImageCreate().
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 115 +--
1 file changed, 97 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index ff85d56572..538da6570a 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2004,6 +2004,22 @@ qemuSnapshotRevertWriteMetadata(virDomainObj *vm,
}
+typedef struct _qemuSnapshotRevertMemoryData {
+int fd;
+char *path;
+virQEMUSaveData *data;
+} qemuSnapshotRevertMemoryData;
+
+
+static void
+qemuSnapshotClearRevertMemoryData(qemuSnapshotRevertMemoryData *memdata)
+{
+VIR_FORCE_CLOSE(memdata->fd);
+virQEMUSaveDataFree(memdata->data);
+}
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(qemuSnapshotRevertMemoryData,
qemuSnapshotClearRevertMemoryData);
+
+
/**
* qemuSnapshotRevertExternalPrepare:
* @vm: domain object
@@ -2011,15 +2027,13 @@ qemuSnapshotRevertWriteMetadata(virDomainObj *vm,
* @snap: snapshot object we are reverting to
* @config: live domain definition
* @inactiveConfig: offline domain definition
- * memsnapFD: pointer to store memory state file FD or NULL
- * memsnapPath: pointer to store memory state file path or NULL
+ * @memdata: struct with data to load memory state
*
* Prepare new temporary snapshot definition @tmpsnapdef that will
* be used while creating new overlay files after reverting to snapshot
* @snap. In case we are reverting to snapshot with memory state it will
- * open it and pass FD via @memsnapFD and path to the file via
- * @memsnapPath, caller is responsible for freeing both @memsnapFD and
- * memsnapPath.
+ * open it and store necessary data in @memdata. Caller is responsible
+ * to clear the data by using qemuSnapshotClearRevertMemoryData().
*
* Returns 0 in success, -1 on error.
*/
@@ -2029,8 +2043,7 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm,
virDomainMomentObj *snap,
virDomainDef *config,
virDomainDef *inactiveConfig,
- int *memsnapFD,
- char **memsnapPath)
+ qemuSnapshotRevertMemoryData *memdata)
{
size_t i;
bool active = virDomainObjIsActive(vm);
@@ -2065,12 +2078,20 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm,
return -1;
}
-if (memsnapFD && memsnapPath && snapdef->memorysnapshotfile) {
+if (memdata && snapdef->memorysnapshotfile) {
virQEMUDriver *driver = ((qemuDomainObjPrivate *)
vm->privateData)->driver;
-g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virDomainDef) savedef = NULL;
-*memsnapPath = snapdef->memorysnapshotfile;
-*memsnapFD = qemuDomainOpenFile(cfg, NULL, *memsnapPath, O_RDONLY,
NULL);
+memdata->path = snapdef->memorysnapshotfile;
+memdata->fd = qemuSaveImageOpen(driver, NULL, memdata->path,
+, >data,
+false, NULL, false, false);
+
+if (memdata->fd < 0)
+return -1;
+
+if (!virDomainDefCheckABIStability(savedef, domdef, driver->xmlopt))
+return -1;
}
return 0;
@@ -2254,13 +2275,16 @@ qemuSnapshotRevertActive(virDomainObj *vm,
virObjectEvent *event = NULL;
virObjectEvent *event2 = NULL;
virDomainMomentObj *loadSnap = NULL;
-VIR_AUTOCLOSE memsnapFD = -1;
-char *memsnapPath = NULL;
int detail;
bool defined = false;
qemuDomainSaveCookie *cookie = (qemuDomainSaveCookie *) snapdef->cookie;
int rc;
g_autoptr(virDomainSnapshotDef) tmpsnapdef = NULL;
+g_auto(qemuSnapshotRevertMemoryData) memdata = { -1, NULL, NULL };
+g_autoptr(virCommand) cmd = NULL;
+g_autofree char *errbuf = NULL;
+VIR_AUTOCLOSE intermediatefd = -1;
+int memrc = 0;
start_flags |= VIR_QEMU_PROCESS_START_PAUSED;
@@ -2284,7 +2308,7 @@ qemuSnapshotRevertActive(virDomainObj *vm,
if (qemuSnapshotRevertExternalPrepare(vm, tmpsnapdef, snap,
*config, *inactiveConfig,
- , ) < 0) {
+ ) < 0) {
return -1;
}
} else {
@@ -2298,6 +2322,30 @@ qemuSnapshotRevertActive(virDomainObj *vm,
virDomainObjAssignDef(vm, config, true, NULL);
+if (v
[libvirt PATCH v2 5/6] NEWS: document support for reverting external snapshots
Signed-off-by: Pavel Hrdina --- NEWS.rst | 8 1 file changed, 8 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index e40c8ac259..a3be76d6cc 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -28,6 +28,14 @@ v9.7.0 (unreleased) 2) pre-binding the variant driver using the ``--driver`` option of ``virsh nodedev-detach``. + * QEMU: implement reverting external snapshots + +Reverting external snapshots is now possible using the existing API +``virDomainSnapshotRevert()``. Management application can check host +capabilities for element within the list of +guest features to see if the current libvirt supports both deleting +and reverting external snapshots. + * **Improvements** * **Bug fixes** -- 2.41.0
[libvirt PATCH v2 3/6] qemu_saveimage: export qemuSaveImageGetCompressionCommand
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_saveimage.c | 2 +-
src/qemu/qemu_saveimage.h | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index eca47171c2..44ab263144 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -207,7 +207,7 @@ virQEMUSaveDataFinish(virQEMUSaveData *data,
}
-static virCommand *
+virCommand *
qemuSaveImageGetCompressionCommand(virQEMUSaveFormat compression)
{
virCommand *ret = NULL;
diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h
index 6892e6764f..510576baa2 100644
--- a/src/qemu/qemu_saveimage.h
+++ b/src/qemu/qemu_saveimage.h
@@ -105,6 +105,9 @@ qemuSaveImageGetCompressionProgram(const char *imageFormat,
bool use_raw_on_fail)
ATTRIBUTE_NONNULL(2);
+virCommand *
+qemuSaveImageGetCompressionCommand(virQEMUSaveFormat compression);
+
int
qemuSaveImageCreate(virQEMUDriver *driver,
virDomainObj *vm,
--
2.41.0
[libvirt PATCH v2 6/6] Revert "capabilities: report full external snapshot support"
Reverting external snapshot for running VM doesn't work correctly so we
should not report this capability until it is fixed.
This reverts commit de71573bfec7f3acd22ec74794318de121716e21.
Signed-off-by: Pavel Hrdina
---
docs/formatcaps.rst| 7 ---
src/conf/capabilities.c| 1 -
src/conf/capabilities.h| 1 -
src/conf/schemas/capability.rng| 5 -
src/qemu/qemu_capabilities.c | 1 -
tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 -
tests/qemucaps2xmloutdata/caps.aarch64.xml | 1 -
tests/qemucaps2xmloutdata/caps.ppc.xml | 1 -
tests/qemucaps2xmloutdata/caps.ppc64.xml | 1 -
tests/qemucaps2xmloutdata/caps.riscv64.xml | 1 -
tests/qemucaps2xmloutdata/caps.s390x.xml | 1 -
tests/qemucaps2xmloutdata/caps.sparc.xml | 1 -
tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml | 1 -
tests/qemucaps2xmloutdata/caps.x86_64.xml | 1 -
14 files changed, 24 deletions(-)
diff --git a/docs/formatcaps.rst b/docs/formatcaps.rst
index 95502c511f..bb8bc663d2 100644
--- a/docs/formatcaps.rst
+++ b/docs/formatcaps.rst
@@ -134,12 +134,6 @@ The element will typically wrap up the
following elements:
creating external disk snapshots is supported. If absent, creating
external
snapshots may still be supported, but it requires attempting the API and
checking for an error to find out for sure. :since:`Since 1.2.3`
- ``externalSnapshot``
- If this element is present, the hypervisor supports deleting and
- reverting external snapshots including memory state. Support for creation
- of external snapshots is reported via the ``disksnapshot`` feature flag.
- Management applications can now switch from internal snapshots to
external
- snapshots. :since:`Since 9.7.0`
Examples
@@ -324,7 +318,6 @@ capabilities enabled in the chip and BIOS you will see:
-
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 34f04cb7d3..56768ce6e0 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -508,7 +508,6 @@ static const struct virCapsGuestFeatureInfo
virCapsGuestFeatureInfos[VIR_CAPS_GU
[VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT] = { "deviceboot", false },
[VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT] = { "disksnapshot", true },
[VIR_CAPS_GUEST_FEATURE_TYPE_HAP] = { "hap", true },
-[VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT] = { "externalSnapshot",
false },
};
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index 9eaf6e2807..c78e3e52fa 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -40,7 +40,6 @@ typedef enum {
VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT,
VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT,
VIR_CAPS_GUEST_FEATURE_TYPE_HAP,
-VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT,
VIR_CAPS_GUEST_FEATURE_TYPE_LAST
} virCapsGuestFeatureType;
diff --git a/src/conf/schemas/capability.rng b/src/conf/schemas/capability.rng
index b1968df258..83b414961a 100644
--- a/src/conf/schemas/capability.rng
+++ b/src/conf/schemas/capability.rng
@@ -493,11 +493,6 @@
-
-
-
-
-
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 05cc11218a..40eacf0f4d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1121,7 +1121,6 @@ virQEMUCapsInitGuestFromBinary(virCaps *caps,
virCapabilitiesAddGuestFeature(guest,
VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT);
virCapabilitiesAddGuestFeatureWithToggle(guest,
VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT,
true, false);
-virCapabilitiesAddGuestFeature(guest,
VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT);
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_TCG)) {
virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_QEMU,
diff --git a/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
b/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
index b9a5b5a1d6..b53a886b90 100644
--- a/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
+++ b/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
@@ -21,7 +21,6 @@
-
diff --git a/tests/qemucaps2xmloutdata/caps.aarch64.xml
b/tests/qemucaps2xmloutdata/caps.aarch64.xml
index 61512ed740..5dca6d3102 100644
--- a/tests/qemucaps2xmloutdata/caps.aarch64.xml
+++ b/tests/qemucaps2xmloutdata/caps.aarch64.xml
@@ -21,7 +21,6 @@
-
diff --git a/tests/qemucaps2xmloutdata/caps.ppc.xml
b/tests/qemucaps2xmloutdata/caps.ppc.xml
index 86d6b85ae0..e7ba391795 100644
--- a/tests/qemucaps2xmloutdata/caps.ppc.xml
+++ b/tests/qemucaps2xmloutdata/caps.ppc.xml
@@ -19,7 +19,6 @@
-
diff --gi
[libvirt PATCH v2 1/6] qemu_snapshot: fix reverting external snapshot when not all disks are included
We need to skip all disks that have snapshot type other than 'external'.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 12
1 file changed, 12 insertions(+)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index d943281e35..ff85d56572 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2058,6 +2058,9 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm,
virDomainSnapshotDiskDef *snapdisk = >disks[i];
virDomainDiskDef *domdisk = domdef->disks[i];
+if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
+
if (qemuSnapshotPrepareDiskExternal(domdisk, snapdisk, active, false)
< 0)
return -1;
}
@@ -2098,6 +2101,9 @@ qemuSnapshotRevertExternalActive(virDomainObj *vm,
return -1;
for (i = 0; i < tmpsnapdef->ndisks; i++) {
+if (tmpsnapdef->disks[i].snapshot !=
VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
+
if (qemuSnapshotDiskPrepareOne(snapctxt,
vm->def->disks[i],
tmpsnapdef->disks + i,
@@ -2188,6 +2194,9 @@ qemuSnapshotRevertExternalFinish(virDomainObj *vm,
for (i = 0; i < curdef->nrevertdisks; i++) {
virDomainSnapshotDiskDef *snapdisk = &(curdef->revertdisks[i]);
+if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
+
if (virStorageSourceInit(snapdisk->src) < 0 ||
virStorageSourceUnlink(snapdisk->src) < 0) {
VIR_WARN("Failed to remove snapshot image '%s'",
@@ -2203,6 +2212,9 @@ qemuSnapshotRevertExternalFinish(virDomainObj *vm,
for (i = 0; i < curdef->ndisks; i++) {
virDomainSnapshotDiskDef *snapdisk = &(curdef->disks[i]);
+if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
+
if (virStorageSourceInit(snapdisk->src) < 0 ||
virStorageSourceUnlink(snapdisk->src) < 0) {
VIR_WARN("Failed to remove snapshot image '%s'",
--
2.41.0
[libvirt PATCH v2 0/6] external snapshot revert fixes
This fixes reverting external snapshots to not error out in cases where it should work and makes it correctly load the memory state when reverting to snapshot of running VM. Last patch is not part of the fix, it is alternative solution to remove the capability as we are close to releasing libvirt 9.7.0 in case the code fixing reverting snapshots is considered too complex. This v2 limits the impact only to snapshot code and no longer changes anything in qemu_saveimage except for exporting one function and one enum. Pavel Hrdina (6): qemu_snapshot: fix reverting external snapshot when not all disks are included qemu_saveimage: export virQEMUSaveFormat enum qemu_saveimage: export qemuSaveImageGetCompressionCommand qemu_snapshot: correctly load the saved memory state file NEWS: document support for reverting external snapshots Revert "capabilities: report full external snapshot support" NEWS.rst | 8 ++ docs/formatcaps.rst | 7 - src/conf/capabilities.c | 1 - src/conf/capabilities.h | 1 - src/conf/schemas/capability.rng | 5 - src/qemu/qemu_capabilities.c | 1 - src/qemu/qemu_saveimage.c | 19 +-- src/qemu/qemu_saveimage.h | 20 +++ src/qemu/qemu_snapshot.c | 127 +++--- .../qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 - tests/qemucaps2xmloutdata/caps.aarch64.xml| 1 - tests/qemucaps2xmloutdata/caps.ppc.xml| 1 - tests/qemucaps2xmloutdata/caps.ppc64.xml | 1 - tests/qemucaps2xmloutdata/caps.riscv64.xml| 1 - tests/qemucaps2xmloutdata/caps.s390x.xml | 1 - tests/qemucaps2xmloutdata/caps.sparc.xml | 1 - tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml | 1 - tests/qemucaps2xmloutdata/caps.x86_64.xml | 1 - 18 files changed, 138 insertions(+), 60 deletions(-) -- 2.41.0
Re: [libvirt PATCH 4/7] qemuSaveImageStartProcess: make it possible to use without header
On Thu, Aug 31, 2023 at 05:43:35PM +0200, Peter Krempa wrote: > On Thu, Aug 31, 2023 at 16:55:03 +0200, Pavel Hrdina wrote: > > When used with internal snapshots there is no header to be used and no > > memory state to be decompressed. > > I didn't yet have a look at the rest, but this made me curious. What are > you actually doing with this with internal snapshots? > > There in fact isn't a save image at all with internal snapshots as the > memory image is stored in the qcow2 image (in a different section than > the data -> thus also the name internal) so I'm not exactly sure what > you are refering to here in the commit message. All of that is correct. In PATCH 6/7 this new function is called from qemuSnapshotRevertActive unconditionally. And it will handle reverting internal and external snapshots and do the correct thing based on what arguments are passed to it. In case of internal snapshots we were calling qemuProcessStart and passing virDomainMomentObj. To avoid code duplication this parameter was introduced in PATCH 3/7 to this new helper so it can start QEMU process when reverting to internal snapshot as well as when reverting to external snapshots. signature.asc Description: PGP signature
Re: [libvirt PATCH 0/7] external snapshot revert fixes
On Thu, Aug 31, 2023 at 04:54:59PM +0200, Pavel Hrdina wrote: > This fixes reverting external snapshots to not error out in cases where > it should work and makes it correctly load the memory state when > reverting to snapshot of running VM. Forget to mention that we need this to be in 9.7.0 otherwise I will have to revert commit introducing capability to indicate that reverting external snapshots is usable. Pavel > Pavel Hrdina (7): > qemu_saveimage: extract starting process to qemuSaveImageStartProcess > qemuSaveImageStartProcess: allow setting reason for audit log > qemuSaveImageStartProcess: add snapshot argument > qemuSaveImageStartProcess: make it possible to use without header > qemu_snapshot: fix reverting external snapshot when not all disks are > included > qemu_snapshot: correctly load the saved memory state file > NEWS: document support for reverting external snapshots > > NEWS.rst | 8 +++ > src/qemu/qemu_saveimage.c | 111 ++ > src/qemu/qemu_saveimage.h | 14 + > src/qemu/qemu_snapshot.c | 90 --- > 4 files changed, 158 insertions(+), 65 deletions(-) > > -- > 2.41.0 > signature.asc Description: PGP signature
[libvirt PATCH 2/7] qemuSaveImageStartProcess: allow setting reason for audit log
When called by snapshot code we will need to use different reason.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_saveimage.c | 6 --
src/qemu/qemu_saveimage.h | 1 +
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 86f31d1820..1eedc900b9 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -576,6 +576,7 @@ qemuSaveImageOpen(virQEMUDriver *driver,
* @cookie: cookie from memory state file
* @asyncJob: type of asynchronous job
* @start_flags: flags to start QEMU process with
+ * @reason: audit log reason
* @started: boolean to store if QEMU process was started
*
* Start VM with existing memory state. Make sure that the stored memory state
@@ -593,6 +594,7 @@ qemuSaveImageStartProcess(virConnectPtr conn,
qemuDomainSaveCookie *cookie,
virDomainAsyncJob asyncJob,
unsigned int start_flags,
+ const char *reason,
bool *started)
{
qemuDomainObjPrivate *priv = vm->privateData;
@@ -660,7 +662,7 @@ qemuSaveImageStartProcess(virConnectPtr conn,
rc = -1;
}
-virDomainAuditStart(vm, "restored", *started);
+virDomainAuditStart(vm, reason, started);
if (!*started || rc < 0)
return -1;
@@ -700,7 +702,7 @@ qemuSaveImageStartVM(virConnectPtr conn,
goto cleanup;
if (qemuSaveImageStartProcess(conn, driver, vm, fd, path, header, cookie,
- asyncJob, start_flags, ) < 0) {
+ asyncJob, start_flags, "restored", )
< 0) {
goto cleanup;
}
diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h
index af30b7f2ec..c6a701dcf5 100644
--- a/src/qemu/qemu_saveimage.h
+++ b/src/qemu/qemu_saveimage.h
@@ -67,6 +67,7 @@ qemuSaveImageStartProcess(virConnectPtr conn,
qemuDomainSaveCookie *cookie,
virDomainAsyncJob asyncJob,
unsigned int start_flags,
+ const char *reason,
bool *started);
int
--
2.41.0
[libvirt PATCH 0/7] external snapshot revert fixes
This fixes reverting external snapshots to not error out in cases where it should work and makes it correctly load the memory state when reverting to snapshot of running VM. Pavel Hrdina (7): qemu_saveimage: extract starting process to qemuSaveImageStartProcess qemuSaveImageStartProcess: allow setting reason for audit log qemuSaveImageStartProcess: add snapshot argument qemuSaveImageStartProcess: make it possible to use without header qemu_snapshot: fix reverting external snapshot when not all disks are included qemu_snapshot: correctly load the saved memory state file NEWS: document support for reverting external snapshots NEWS.rst | 8 +++ src/qemu/qemu_saveimage.c | 111 ++ src/qemu/qemu_saveimage.h | 14 + src/qemu/qemu_snapshot.c | 90 --- 4 files changed, 158 insertions(+), 65 deletions(-) -- 2.41.0
[libvirt PATCH 5/7] qemu_snapshot: fix reverting external snapshot when not all disks are included
We need to skip all disks that have snapshot type other than 'external'.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 12
1 file changed, 12 insertions(+)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index d943281e35..ff85d56572 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2058,6 +2058,9 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm,
virDomainSnapshotDiskDef *snapdisk = >disks[i];
virDomainDiskDef *domdisk = domdef->disks[i];
+if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
+
if (qemuSnapshotPrepareDiskExternal(domdisk, snapdisk, active, false)
< 0)
return -1;
}
@@ -2098,6 +2101,9 @@ qemuSnapshotRevertExternalActive(virDomainObj *vm,
return -1;
for (i = 0; i < tmpsnapdef->ndisks; i++) {
+if (tmpsnapdef->disks[i].snapshot !=
VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
+
if (qemuSnapshotDiskPrepareOne(snapctxt,
vm->def->disks[i],
tmpsnapdef->disks + i,
@@ -2188,6 +2194,9 @@ qemuSnapshotRevertExternalFinish(virDomainObj *vm,
for (i = 0; i < curdef->nrevertdisks; i++) {
virDomainSnapshotDiskDef *snapdisk = &(curdef->revertdisks[i]);
+if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
+
if (virStorageSourceInit(snapdisk->src) < 0 ||
virStorageSourceUnlink(snapdisk->src) < 0) {
VIR_WARN("Failed to remove snapshot image '%s'",
@@ -2203,6 +2212,9 @@ qemuSnapshotRevertExternalFinish(virDomainObj *vm,
for (i = 0; i < curdef->ndisks; i++) {
virDomainSnapshotDiskDef *snapdisk = &(curdef->disks[i]);
+if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
+
if (virStorageSourceInit(snapdisk->src) < 0 ||
virStorageSourceUnlink(snapdisk->src) < 0) {
VIR_WARN("Failed to remove snapshot image '%s'",
--
2.41.0
[libvirt PATCH 3/7] qemuSaveImageStartProcess: add snapshot argument
When called from snapshot code we will need to pass snapshot object in
order to make internal snapshots work correctly.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_saveimage.c | 6 --
src/qemu/qemu_saveimage.h | 1 +
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 1eedc900b9..73115af42d 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -572,6 +572,7 @@ qemuSaveImageOpen(virQEMUDriver *driver,
* @vm: domain object
* @fd: FD pointer of memory state file
* @path: path to memory state file
+ * @snapshot: snapshot to load when starting QEMU process or NULL
* @header: header from memory state file
* @cookie: cookie from memory state file
* @asyncJob: type of asynchronous job
@@ -590,6 +591,7 @@ qemuSaveImageStartProcess(virConnectPtr conn,
virDomainObj *vm,
int *fd,
const char *path,
+ virDomainMomentObj *snapshot,
virQEMUSaveHeader *header,
qemuDomainSaveCookie *cookie,
virDomainAsyncJob asyncJob,
@@ -634,7 +636,7 @@ qemuSaveImageStartProcess(virConnectPtr conn,
priv->disableSlirp = true;
if (qemuProcessStart(conn, driver, vm, cookie ? cookie->cpu : NULL,
- asyncJob, "stdio", *fd, path, NULL,
+ asyncJob, "stdio", *fd, path, snapshot,
VIR_NETDEV_VPORT_PROFILE_OP_RESTORE,
start_flags) == 0)
*started = true;
@@ -701,7 +703,7 @@ qemuSaveImageStartVM(virConnectPtr conn,
virDomainXMLOptionGetSaveCookie(driver->xmlopt)) < 0)
goto cleanup;
-if (qemuSaveImageStartProcess(conn, driver, vm, fd, path, header, cookie,
+if (qemuSaveImageStartProcess(conn, driver, vm, fd, path, NULL, header,
cookie,
asyncJob, start_flags, "restored", )
< 0) {
goto cleanup;
}
diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h
index c6a701dcf5..c5ad50558e 100644
--- a/src/qemu/qemu_saveimage.h
+++ b/src/qemu/qemu_saveimage.h
@@ -63,6 +63,7 @@ qemuSaveImageStartProcess(virConnectPtr conn,
virDomainObj *vm,
int *fd,
const char *path,
+ virDomainMomentObj *snapshot,
virQEMUSaveHeader *header,
qemuDomainSaveCookie *cookie,
virDomainAsyncJob asyncJob,
--
2.41.0
[libvirt PATCH 6/7] qemu_snapshot: correctly load the saved memory state file
Original code assumed that the memory state file is only migration
stream but it has additional metadata stored by libvirt. To correctly
load the memory state file we need to reuse code that is used when
restoring domain from saved image.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 78
1 file changed, 47 insertions(+), 31 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index ff85d56572..869b46a9a8 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2004,6 +2004,21 @@ qemuSnapshotRevertWriteMetadata(virDomainObj *vm,
}
+typedef struct _qemuSnapshotRevertMemoryData {
+int fd;
+char *path;
+virQEMUSaveData *data;
+} qemuSnapshotRevertMemoryData;
+
+static void
+qemuSnapshotClearRevertMemoryData(qemuSnapshotRevertMemoryData *memdata)
+{
+VIR_FORCE_CLOSE(memdata->fd);
+virQEMUSaveDataFree(memdata->data);
+}
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(qemuSnapshotRevertMemoryData,
qemuSnapshotClearRevertMemoryData);
+
+
/**
* qemuSnapshotRevertExternalPrepare:
* @vm: domain object
@@ -2011,15 +2026,13 @@ qemuSnapshotRevertWriteMetadata(virDomainObj *vm,
* @snap: snapshot object we are reverting to
* @config: live domain definition
* @inactiveConfig: offline domain definition
- * memsnapFD: pointer to store memory state file FD or NULL
- * memsnapPath: pointer to store memory state file path or NULL
+ * @memdata: struct with data to load memory state
*
* Prepare new temporary snapshot definition @tmpsnapdef that will
* be used while creating new overlay files after reverting to snapshot
* @snap. In case we are reverting to snapshot with memory state it will
- * open it and pass FD via @memsnapFD and path to the file via
- * @memsnapPath, caller is responsible for freeing both @memsnapFD and
- * memsnapPath.
+ * open it and store necessary data in @memdata. Caller is responsible
+ * to clear the data by using qemuSnapshotClearRevertMemoryData().
*
* Returns 0 in success, -1 on error.
*/
@@ -2029,8 +2042,7 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm,
virDomainMomentObj *snap,
virDomainDef *config,
virDomainDef *inactiveConfig,
- int *memsnapFD,
- char **memsnapPath)
+ qemuSnapshotRevertMemoryData *memdata)
{
size_t i;
bool active = virDomainObjIsActive(vm);
@@ -2065,12 +2077,21 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm,
return -1;
}
-if (memsnapFD && memsnapPath && snapdef->memorysnapshotfile) {
+if (memdata && snapdef->memorysnapshotfile) {
virQEMUDriver *driver = ((qemuDomainObjPrivate *)
vm->privateData)->driver;
-g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virDomainDef) savedef = NULL;
-*memsnapPath = snapdef->memorysnapshotfile;
-*memsnapFD = qemuDomainOpenFile(cfg, NULL, *memsnapPath, O_RDONLY,
NULL);
+memdata->path = snapdef->memorysnapshotfile;
+memdata->fd = qemuSaveImageOpen(driver, NULL, memdata->path,
+, >data,
+false, NULL,
+false, false);
+
+if (memdata->fd < 0)
+return -1;
+
+if (!virDomainDefCheckABIStability(savedef, domdef, driver->xmlopt))
+return -1;
}
return 0;
@@ -2254,13 +2275,13 @@ qemuSnapshotRevertActive(virDomainObj *vm,
virObjectEvent *event = NULL;
virObjectEvent *event2 = NULL;
virDomainMomentObj *loadSnap = NULL;
-VIR_AUTOCLOSE memsnapFD = -1;
-char *memsnapPath = NULL;
int detail;
bool defined = false;
qemuDomainSaveCookie *cookie = (qemuDomainSaveCookie *) snapdef->cookie;
int rc;
g_autoptr(virDomainSnapshotDef) tmpsnapdef = NULL;
+g_auto(qemuSnapshotRevertMemoryData) memdata = { -1, NULL, NULL };
+bool started = false;
start_flags |= VIR_QEMU_PROCESS_START_PAUSED;
@@ -2284,7 +2305,7 @@ qemuSnapshotRevertActive(virDomainObj *vm,
if (qemuSnapshotRevertExternalPrepare(vm, tmpsnapdef, snap,
*config, *inactiveConfig,
- , ) < 0) {
+ ) < 0) {
return -1;
}
} else {
@@ -2298,28 +2319,24 @@ qemuSnapshotRevertActive(virDomainObj *vm,
virDomainObjAssignDef(vm, config, true, NULL);
-/* No cookie means libvirt which saved the domain was too old to
- * mess up the CPU definitions.
- */
-if (cookie &&
-qemuDomainFixupCPUs(vm, >cpu) < 0)
+if (qemuSaveImageStartP
[libvirt PATCH 7/7] NEWS: document support for reverting external snapshots
Signed-off-by: Pavel Hrdina --- NEWS.rst | 8 1 file changed, 8 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index e40c8ac259..a3be76d6cc 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -28,6 +28,14 @@ v9.7.0 (unreleased) 2) pre-binding the variant driver using the ``--driver`` option of ``virsh nodedev-detach``. + * QEMU: implement reverting external snapshots + +Reverting external snapshots is now possible using the existing API +``virDomainSnapshotRevert()``. Management application can check host +capabilities for element within the list of +guest features to see if the current libvirt supports both deleting +and reverting external snapshots. + * **Improvements** * **Bug fixes** -- 2.41.0
[libvirt PATCH 4/7] qemuSaveImageStartProcess: make it possible to use without header
When used with internal snapshots there is no header to be used and no
memory state to be decompressed.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_saveimage.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 73115af42d..2538732901 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -573,7 +573,7 @@ qemuSaveImageOpen(virQEMUDriver *driver,
* @fd: FD pointer of memory state file
* @path: path to memory state file
* @snapshot: snapshot to load when starting QEMU process or NULL
- * @header: header from memory state file
+ * @header: header from memory state file or NULL
* @cookie: cookie from memory state file
* @asyncJob: type of asynchronous job
* @start_flags: flags to start QEMU process with
@@ -605,7 +605,7 @@ qemuSaveImageStartProcess(virConnectPtr conn,
g_autofree char *errbuf = NULL;
int rc = 0;
-if ((header->version == 2) &&
+if (header && (header->version == 2) &&
(header->compressed != QEMU_SAVE_FORMAT_RAW)) {
if (!(cmd = qemuSaveImageGetCompressionCommand(header->compressed)))
return -1;
--
2.41.0
[libvirt PATCH 1/7] qemu_saveimage: extract starting process to qemuSaveImageStartProcess
Part of qemuSaveImageStartVM() function will be used when reverting
external snapshots. To avoid duplicating code and logic extract the
shared bits into separate function.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_saveimage.c | 103 ++
src/qemu/qemu_saveimage.h | 12 +
2 files changed, 83 insertions(+), 32 deletions(-)
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 41310d6a9a..86f31d1820 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -565,42 +565,46 @@ qemuSaveImageOpen(virQEMUDriver *driver,
return ret;
}
+/**
+ * qemuSaveImageStartProcess:
+ * @conn: connection object
+ * @driver: qemu driver object
+ * @vm: domain object
+ * @fd: FD pointer of memory state file
+ * @path: path to memory state file
+ * @header: header from memory state file
+ * @cookie: cookie from memory state file
+ * @asyncJob: type of asynchronous job
+ * @start_flags: flags to start QEMU process with
+ * @started: boolean to store if QEMU process was started
+ *
+ * Start VM with existing memory state. Make sure that the stored memory state
+ * is correctly decompressed so it can be loaded by QEMU process.
+ *
+ * Returns 0 on success, -1 on error.
+ */
int
-qemuSaveImageStartVM(virConnectPtr conn,
- virQEMUDriver *driver,
- virDomainObj *vm,
- int *fd,
- virQEMUSaveData *data,
- const char *path,
- bool start_paused,
- bool reset_nvram,
- virDomainAsyncJob asyncJob)
+qemuSaveImageStartProcess(virConnectPtr conn,
+ virQEMUDriver *driver,
+ virDomainObj *vm,
+ int *fd,
+ const char *path,
+ virQEMUSaveHeader *header,
+ qemuDomainSaveCookie *cookie,
+ virDomainAsyncJob asyncJob,
+ unsigned int start_flags,
+ bool *started)
{
qemuDomainObjPrivate *priv = vm->privateData;
-int ret = -1;
-bool started = false;
-virObjectEvent *event;
VIR_AUTOCLOSE intermediatefd = -1;
g_autoptr(virCommand) cmd = NULL;
g_autofree char *errbuf = NULL;
-g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-virQEMUSaveHeader *header = >header;
-g_autoptr(qemuDomainSaveCookie) cookie = NULL;
int rc = 0;
-unsigned int start_flags = VIR_QEMU_PROCESS_START_PAUSED |
-VIR_QEMU_PROCESS_START_GEN_VMID;
-
-if (reset_nvram)
-start_flags |= VIR_QEMU_PROCESS_START_RESET_NVRAM;
-
-if (virSaveCookieParseString(data->cookie, (virObject **),
-
virDomainXMLOptionGetSaveCookie(driver->xmlopt)) < 0)
-goto cleanup;
if ((header->version == 2) &&
(header->compressed != QEMU_SAVE_FORMAT_RAW)) {
if (!(cmd = qemuSaveImageGetCompressionCommand(header->compressed)))
-goto cleanup;
+return -1;
intermediatefd = *fd;
*fd = -1;
@@ -613,7 +617,7 @@ qemuSaveImageStartVM(virConnectPtr conn,
if (virCommandRunAsync(cmd, NULL) < 0) {
*fd = intermediatefd;
intermediatefd = -1;
-goto cleanup;
+return -1;
}
}
@@ -622,7 +626,7 @@ qemuSaveImageStartVM(virConnectPtr conn,
*/
if (cookie &&
qemuDomainFixupCPUs(vm, >cpu) < 0)
-goto cleanup;
+return -1;
if (cookie && !cookie->slirpHelper)
priv->disableSlirp = true;
@@ -631,12 +635,12 @@ qemuSaveImageStartVM(virConnectPtr conn,
asyncJob, "stdio", *fd, path, NULL,
VIR_NETDEV_VPORT_PROFILE_OP_RESTORE,
start_flags) == 0)
-started = true;
+*started = true;
if (intermediatefd != -1) {
virErrorPtr orig_err = NULL;
-if (!started) {
+if (!*started) {
/* if there was an error setting up qemu, the intermediate
* process will wait forever to write to stdout, so we
* must manually kill it and ignore any error related to
@@ -656,15 +660,50 @@ qemuSaveImageStartVM(virConnectPtr conn,
rc = -1;
}
-virDomainAuditStart(vm, "restored", started);
-if (!started || rc < 0)
-goto cleanup;
+virDomainAuditStart(vm, "restored", *started);
+if (!*started || rc < 0)
+return -1;
/* qemuProcessStart doesn't unset the qemu error reporting infrastructure
* in case of migration (which is used in this case) so we need to reset it
* so that the handle to virtlogd is not held open unnecessarily */
qemuMonitorSetDomainLog(qemuDomainGet
[libvirt PATCH] NEWS: document support for reverting external snapshots
Signed-off-by: Pavel Hrdina --- NEWS.rst | 8 1 file changed, 8 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index e40c8ac259..a3be76d6cc 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -28,6 +28,14 @@ v9.7.0 (unreleased) 2) pre-binding the variant driver using the ``--driver`` option of ``virsh nodedev-detach``. + * QEMU: implement reverting external snapshots + +Reverting external snapshots is now possible using the existing API +``virDomainSnapshotRevert()``. Management application can check host +capabilities for element within the list of +guest features to see if the current libvirt supports both deleting +and reverting external snapshots. + * **Improvements** * **Bug fixes** -- 2.41.0
Re: [PATCH] docs: Improve documentation of
On Fri, Aug 25, 2023 at 02:34:04PM +0200, Peter Krempa wrote: > Note the implications and caveats of . > > Closes: https://gitlab.com/libvirt/libvirt/-/issues/519 > Signed-off-by: Peter Krempa > --- > docs/formatdomain.rst | 18 ++ > 1 file changed, 18 insertions(+) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
Re: [PATCH 00/27] Move error messages onto a single line
On Fri, Aug 25, 2023 at 09:39:57AM +0200, Michal Privoznik wrote: > This is inspired by the following discussion: > > https://listman.redhat.com/archives/libvir-list/2023-August/241361.html We have it covered by coding style document so it would be nice to make a syntax-check for it as well so there are no new multi-line errors introduced in the future. Not sure how difficult it would be so definitely as follow up. Otherwise looks good and compiles without issues on my machine. I would consider pushing it after release is done to make sure it will not break anything as it affects basically the whole libvirt code base. > And ideally I'd present a green pipeline but for some reason, I can't: > > https://gitlab.com/MichalPrivoznik/libvirt/-/pipelines/981105262 > > But the problem is not with my code rather than our CI. Unless there are objections from others Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
[libvirt PATCH v2 2/2] capabilities: report full external snapshot support
Now that deleting and reverting external snapshots is implemented we can
report that in capabilities so management applications can use that
information and start using external snapshots.
Signed-off-by: Pavel Hrdina
---
docs/formatcaps.rst| 6 ++
src/conf/capabilities.c| 1 +
src/conf/capabilities.h| 1 +
src/conf/schemas/capability.rng| 5 +
src/qemu/qemu_capabilities.c | 1 +
tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 +
tests/qemucaps2xmloutdata/caps.aarch64.xml | 1 +
tests/qemucaps2xmloutdata/caps.ppc.xml | 1 +
tests/qemucaps2xmloutdata/caps.ppc64.xml | 1 +
tests/qemucaps2xmloutdata/caps.riscv64.xml | 1 +
tests/qemucaps2xmloutdata/caps.s390x.xml | 1 +
tests/qemucaps2xmloutdata/caps.sparc.xml | 1 +
tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml | 1 +
tests/qemucaps2xmloutdata/caps.x86_64.xml | 1 +
14 files changed, 23 insertions(+)
diff --git a/docs/formatcaps.rst b/docs/formatcaps.rst
index bb8bc663d2..255180135d 100644
--- a/docs/formatcaps.rst
+++ b/docs/formatcaps.rst
@@ -134,6 +134,11 @@ The element will typically wrap up the
following elements:
creating external disk snapshots is supported. If absent, creating
external
snapshots may still be supported, but it requires attempting the API and
checking for an error to find out for sure. :since:`Since 1.2.3`
+ ``externalSnapshot``
+ If this element is present, the hypervisor supports deleting and
+ reverting external snapshots including memory state. Creating external
+ snapshots is supported for long time. Management applications can now
+ switch from internal snapshots to external snapshots. :since:`Since
9.7.0`
Examples
@@ -318,6 +323,7 @@ capabilities enabled in the chip and BIOS you will see:
+
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 56768ce6e0..34f04cb7d3 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -508,6 +508,7 @@ static const struct virCapsGuestFeatureInfo
virCapsGuestFeatureInfos[VIR_CAPS_GU
[VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT] = { "deviceboot", false },
[VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT] = { "disksnapshot", true },
[VIR_CAPS_GUEST_FEATURE_TYPE_HAP] = { "hap", true },
+[VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT] = { "externalSnapshot",
false },
};
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index c78e3e52fa..9eaf6e2807 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -40,6 +40,7 @@ typedef enum {
VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT,
VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT,
VIR_CAPS_GUEST_FEATURE_TYPE_HAP,
+VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT,
VIR_CAPS_GUEST_FEATURE_TYPE_LAST
} virCapsGuestFeatureType;
diff --git a/src/conf/schemas/capability.rng b/src/conf/schemas/capability.rng
index 83b414961a..b1968df258 100644
--- a/src/conf/schemas/capability.rng
+++ b/src/conf/schemas/capability.rng
@@ -493,6 +493,11 @@
+
+
+
+
+
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 40eacf0f4d..05cc11218a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1121,6 +1121,7 @@ virQEMUCapsInitGuestFromBinary(virCaps *caps,
virCapabilitiesAddGuestFeature(guest,
VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT);
virCapabilitiesAddGuestFeatureWithToggle(guest,
VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT,
true, false);
+virCapabilitiesAddGuestFeature(guest,
VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT);
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_TCG)) {
virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_QEMU,
diff --git a/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
b/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
index b53a886b90..b9a5b5a1d6 100644
--- a/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
+++ b/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
@@ -21,6 +21,7 @@
+
diff --git a/tests/qemucaps2xmloutdata/caps.aarch64.xml
b/tests/qemucaps2xmloutdata/caps.aarch64.xml
index 5dca6d3102..61512ed740 100644
--- a/tests/qemucaps2xmloutdata/caps.aarch64.xml
+++ b/tests/qemucaps2xmloutdata/caps.aarch64.xml
@@ -21,6 +21,7 @@
+
diff --git a/tests/qemucaps2xmloutdata/caps.ppc.xml
b/tests/qemucaps2xmloutdata/caps.ppc.xml
index e7ba391795..86d6b85ae0 100644
--- a/tests/qemucaps2xmloutdata/caps.ppc.xml
+++ b/tests/qemucaps2xmloutdata/caps.ppc.xml
@@ -19,6 +19,7 @@
+
diff --git a/tests/qemucaps2xmloutdata/caps.ppc64.xml
[libvirt PATCH v2 1/2] capabilities: reword disksnapshot feature to mention creating snapshots
Libvirt supports creating snapshots for a long time but the wording of the feature may imply that libvirt supports external snapshots in general but that is not true as users were not able to use APIs to delete or revert external snapshots. Signed-off-by: Pavel Hrdina --- docs/formatcaps.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/formatcaps.rst b/docs/formatcaps.rst index f7e5342654..bb8bc663d2 100644 --- a/docs/formatcaps.rst +++ b/docs/formatcaps.rst @@ -131,9 +131,9 @@ The element will typically wrap up the following elements: :since:`Since 0.8.8` ``disksnapshot`` If this element is present, the ``default`` attribute describes whether - external disk snapshots are supported. If absent, external snapshots may - still be supported, but it requires attempting the API and checking for an - error to find out for sure. :since:`Since 1.2.3` + creating external disk snapshots is supported. If absent, creating external + snapshots may still be supported, but it requires attempting the API and + checking for an error to find out for sure. :since:`Since 1.2.3` Examples -- 2.41.0
[libvirt PATCH v2 0/2] reword and add external snapshot related capabilities
Pavel Hrdina (2): capabilities: reword disksnapshot feature to mention creating snapshots capabilities: report full external snapshot support docs/formatcaps.rst| 12 +--- src/conf/capabilities.c| 1 + src/conf/capabilities.h| 1 + src/conf/schemas/capability.rng| 5 + src/qemu/qemu_capabilities.c | 1 + tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 + tests/qemucaps2xmloutdata/caps.aarch64.xml | 1 + tests/qemucaps2xmloutdata/caps.ppc.xml | 1 + tests/qemucaps2xmloutdata/caps.ppc64.xml | 1 + tests/qemucaps2xmloutdata/caps.riscv64.xml | 1 + tests/qemucaps2xmloutdata/caps.s390x.xml | 1 + tests/qemucaps2xmloutdata/caps.sparc.xml | 1 + tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml | 1 + tests/qemucaps2xmloutdata/caps.x86_64.xml | 1 + 14 files changed, 26 insertions(+), 3 deletions(-) -- 2.41.0
Re: [libvirt PATCH] capabilities: report full external snapshot support
On Thu, Aug 24, 2023 at 05:44:32PM +0200, Peter Krempa wrote: > On Thu, Aug 24, 2023 at 17:29:49 +0200, Pavel Hrdina wrote: > > Now that deleting and reverting external snapshots is implemented we can > > report that in capabilities so management applications can use that > > information and start using external snapshots. > > > > Signed-off-by: Pavel Hrdina > > --- > > docs/formatcaps.rst| 6 ++ > > src/conf/capabilities.c| 1 + > > src/conf/capabilities.h| 1 + > > src/conf/schemas/capability.rng| 5 + > > src/qemu/qemu_capabilities.c | 1 + > > tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 + > > tests/qemucaps2xmloutdata/caps.aarch64.xml | 1 + > > tests/qemucaps2xmloutdata/caps.ppc.xml | 1 + > > tests/qemucaps2xmloutdata/caps.ppc64.xml | 1 + > > tests/qemucaps2xmloutdata/caps.riscv64.xml | 1 + > > tests/qemucaps2xmloutdata/caps.s390x.xml | 1 + > > tests/qemucaps2xmloutdata/caps.sparc.xml | 1 + > > tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml | 1 + > > tests/qemucaps2xmloutdata/caps.x86_64.xml | 1 + > > 14 files changed, 23 insertions(+) > > > > diff --git a/docs/formatcaps.rst b/docs/formatcaps.rst > > index f7e5342654..0e056914b8 100644 > > --- a/docs/formatcaps.rst > > +++ b/docs/formatcaps.rst > > @@ -134,6 +134,11 @@ The element will typically wrap up the > > following elements: > >external disk snapshots are supported. If absent, external snapshots > > may > >still be supported, but it requires attempting the API and checking > > for an > >error to find out for sure. :since:`Since 1.2.3` > > + ``externalSnapshot`` > > + If this element is present, the hypervisor supports creating, > > deleting and > > + reverting external snapshots including memory state and management > > + applications can switch from internal snapshots to external > > snapshots. > > + :since:`Since 9.7.0` > > This implies that 'creation' of external snapshots is also included in > the "Sice 9.7.0". Reword it such that you explicitly mention that > creation of external snapshots is a long-existing feature. > > You can also entirely omit 'creation' from the text as it's covered by > 'disksnapshot' above. Yeah the fact the feature name is `disksnapshot` is a bit unfortunate and it doesn't mention that only creation is supported if that feature is available. I'll reword it and post v2. Thanks, Pavel signature.asc Description: PGP signature
[libvirt PATCH] capabilities: report full external snapshot support
Now that deleting and reverting external snapshots is implemented we can
report that in capabilities so management applications can use that
information and start using external snapshots.
Signed-off-by: Pavel Hrdina
---
docs/formatcaps.rst| 6 ++
src/conf/capabilities.c| 1 +
src/conf/capabilities.h| 1 +
src/conf/schemas/capability.rng| 5 +
src/qemu/qemu_capabilities.c | 1 +
tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 +
tests/qemucaps2xmloutdata/caps.aarch64.xml | 1 +
tests/qemucaps2xmloutdata/caps.ppc.xml | 1 +
tests/qemucaps2xmloutdata/caps.ppc64.xml | 1 +
tests/qemucaps2xmloutdata/caps.riscv64.xml | 1 +
tests/qemucaps2xmloutdata/caps.s390x.xml | 1 +
tests/qemucaps2xmloutdata/caps.sparc.xml | 1 +
tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml | 1 +
tests/qemucaps2xmloutdata/caps.x86_64.xml | 1 +
14 files changed, 23 insertions(+)
diff --git a/docs/formatcaps.rst b/docs/formatcaps.rst
index f7e5342654..0e056914b8 100644
--- a/docs/formatcaps.rst
+++ b/docs/formatcaps.rst
@@ -134,6 +134,11 @@ The element will typically wrap up the
following elements:
external disk snapshots are supported. If absent, external snapshots may
still be supported, but it requires attempting the API and checking for
an
error to find out for sure. :since:`Since 1.2.3`
+ ``externalSnapshot``
+ If this element is present, the hypervisor supports creating, deleting
and
+ reverting external snapshots including memory state and management
+ applications can switch from internal snapshots to external snapshots.
+ :since:`Since 9.7.0`
Examples
@@ -318,6 +323,7 @@ capabilities enabled in the chip and BIOS you will see:
+
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 56768ce6e0..34f04cb7d3 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -508,6 +508,7 @@ static const struct virCapsGuestFeatureInfo
virCapsGuestFeatureInfos[VIR_CAPS_GU
[VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT] = { "deviceboot", false },
[VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT] = { "disksnapshot", true },
[VIR_CAPS_GUEST_FEATURE_TYPE_HAP] = { "hap", true },
+[VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT] = { "externalSnapshot",
false },
};
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index c78e3e52fa..9eaf6e2807 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -40,6 +40,7 @@ typedef enum {
VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT,
VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT,
VIR_CAPS_GUEST_FEATURE_TYPE_HAP,
+VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT,
VIR_CAPS_GUEST_FEATURE_TYPE_LAST
} virCapsGuestFeatureType;
diff --git a/src/conf/schemas/capability.rng b/src/conf/schemas/capability.rng
index 83b414961a..b1968df258 100644
--- a/src/conf/schemas/capability.rng
+++ b/src/conf/schemas/capability.rng
@@ -493,6 +493,11 @@
+
+
+
+
+
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 40eacf0f4d..05cc11218a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1121,6 +1121,7 @@ virQEMUCapsInitGuestFromBinary(virCaps *caps,
virCapabilitiesAddGuestFeature(guest,
VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT);
virCapabilitiesAddGuestFeatureWithToggle(guest,
VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT,
true, false);
+virCapabilitiesAddGuestFeature(guest,
VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT);
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_TCG)) {
virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_QEMU,
diff --git a/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
b/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
index b53a886b90..b9a5b5a1d6 100644
--- a/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
+++ b/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml
@@ -21,6 +21,7 @@
+
diff --git a/tests/qemucaps2xmloutdata/caps.aarch64.xml
b/tests/qemucaps2xmloutdata/caps.aarch64.xml
index 5dca6d3102..61512ed740 100644
--- a/tests/qemucaps2xmloutdata/caps.aarch64.xml
+++ b/tests/qemucaps2xmloutdata/caps.aarch64.xml
@@ -21,6 +21,7 @@
+
diff --git a/tests/qemucaps2xmloutdata/caps.ppc.xml
b/tests/qemucaps2xmloutdata/caps.ppc.xml
index e7ba391795..86d6b85ae0 100644
--- a/tests/qemucaps2xmloutdata/caps.ppc.xml
+++ b/tests/qemucaps2xmloutdata/caps.ppc.xml
@@ -19,6 +19,7 @@
+
diff --git a/tests/qemucaps2xmloutdata/caps.ppc64.xml
b/tests/qemucaps2xmloutdata/caps.ppc64.xml
index 8
Re: [PATCH RESEND 0/2] qemu: generate shorter channel target paths
On Wed, Aug 16, 2023 at 09:15:20AM +0200, Michal Privoznik wrote: > This is a resend of: > > https://listman.redhat.com/archives/libvir-list/2023-July/240904.html > > Diff to previous version: > - s/9.6.0/9.7.0/ because of missed release > > Michal Prívozník (2): > qemu: Generate shorter channel target paths > qemu: Move channelTargetDir into stateDir Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
Re: [PATCH 3/3] meson: Drop python3_prog
On Wed, Aug 16, 2023 at 01:18:37PM +0200, Michal Prívozník wrote:
> On 8/16/23 12:25, Pavel Hrdina wrote:
> > On Tue, Aug 15, 2023 at 02:45:37PM +0200, Michal Privoznik wrote:
> >> Signed-off-by: Michal Privoznik
> >> ---
> >> docs/meson.build| 7 +++
> >> meson.build | 4 ++--
> >> src/access/meson.build | 6 +++---
> >> src/meson.build | 43 +++--
> >> src/network/meson.build | 2 +-
> >> src/qemu/meson.build| 2 +-
> >> tests/meson.build | 2 +-
> >> 7 files changed, 31 insertions(+), 35 deletions(-)
> >
> > Not sure we can do this. In meson.build we have the following:
> >
> > required_programs = [
> > 'perl',
> > 'python3',
> > 'xmllint',
> > 'xsltproc',
> > ]
> >
> > foreach name : required_programs
> > prog = find_program(name, dirs: libvirt_sbin_path)
> > varname = name.underscorify()
> > conf.set_quoted(varname.to_upper(), prog.full_path())
> > set_variable('@0@_prog'.format(varname), prog)
> > endforeach
> >
> > which will set the python3_prog variable and we use that as our python
> > executable.
> >
> > I did a quick testing using the following meson.build file:
> >
> > ---
> > project('mesonpy', 'c')
> >
> > required_programs = [
> > 'python3',
> > ]
> >
> > foreach name : required_programs
> > prog = find_program(name)
> > varname = name.underscorify()
> > set_variable('@0@_prog'.format(varname), prog)
> > endforeach
> >
> > res1 = run_command(python3_prog, 'script.py')
> > res2 = run_command('script.py')
> >
> > warning(res1.stdout())
> > warning(res2.stdout())
> > ---
> >
> > with the script.py having the following:
> >
> > ---
> > #!/usr/bin/env python3
> >
> > import sys
> >
> > print(sys.version)
> > ---
> >
> > and when I changed PATH to have python3 pointing to python3.12 but my
> > system python is python3.11 I've got the following resutl:
> >
> > meson.build:16: WARNING: 3.12.0b4 (main, Jul 12 2023, 00:00:00) [GCC 13.1.1
> > 20230614 (Red Hat 13.1.1-4)]
> >
> > meson.build:17: WARNING: 3.11.4 (main, Jun 7 2023, 00:00:00) [GCC 13.1.1
> > 20230511 (Red Hat 13.1.1-2)]
> >
> >
> > Don't remember the origin of the wrapper but my guess is that this was
> > the main reason why we have it.
>
> Right. But this is expected, isn't it? The same way, if you'd change any
> other binary invoked from meson (say compiler) and we don't hardcode
> paths for them either.
>
> IOW - why should this behavior be problematic?
After offlist discussion and explanation the patch will not be merged.
The main issue with this patch is that compiling libvirt would use
system python and not what user have in the current environment which is
not the same what happens with compiler or other binaries we are using
during compilation.
Pavel
> BTW: we already invoke python scripts WITHOUT pytho3_prog and it didn't
> bother us yet: apibuild_prog, hyperv_wmi_generator_prog,
> esx_vi_generator_prog to name a few.
Yeah, some of them where broken by commit
and that should be fixed.
Pavel
signature.asc
Description: PGP signature
Re: [PATCH 3/3] meson: Drop python3_prog
On Tue, Aug 15, 2023 at 02:45:37PM +0200, Michal Privoznik wrote:
> Signed-off-by: Michal Privoznik
> ---
> docs/meson.build| 7 +++
> meson.build | 4 ++--
> src/access/meson.build | 6 +++---
> src/meson.build | 43 +++--
> src/network/meson.build | 2 +-
> src/qemu/meson.build| 2 +-
> tests/meson.build | 2 +-
> 7 files changed, 31 insertions(+), 35 deletions(-)
Not sure we can do this. In meson.build we have the following:
required_programs = [
'perl',
'python3',
'xmllint',
'xsltproc',
]
foreach name : required_programs
prog = find_program(name, dirs: libvirt_sbin_path)
varname = name.underscorify()
conf.set_quoted(varname.to_upper(), prog.full_path())
set_variable('@0@_prog'.format(varname), prog)
endforeach
which will set the python3_prog variable and we use that as our python
executable.
I did a quick testing using the following meson.build file:
---
project('mesonpy', 'c')
required_programs = [
'python3',
]
foreach name : required_programs
prog = find_program(name)
varname = name.underscorify()
set_variable('@0@_prog'.format(varname), prog)
endforeach
res1 = run_command(python3_prog, 'script.py')
res2 = run_command('script.py')
warning(res1.stdout())
warning(res2.stdout())
---
with the script.py having the following:
---
#!/usr/bin/env python3
import sys
print(sys.version)
---
and when I changed PATH to have python3 pointing to python3.12 but my
system python is python3.11 I've got the following resutl:
meson.build:16: WARNING: 3.12.0b4 (main, Jul 12 2023, 00:00:00) [GCC 13.1.1
20230614 (Red Hat 13.1.1-4)]
meson.build:17: WARNING: 3.11.4 (main, Jun 7 2023, 00:00:00) [GCC 13.1.1
20230511 (Red Hat 13.1.1-2)]
Don't remember the origin of the wrapper but my guess is that this was
the main reason why we have it.
Pavel
> diff --git a/docs/meson.build b/docs/meson.build
> index b20ef1c926..2231d2da04 100644
> --- a/docs/meson.build
> +++ b/docs/meson.build
> @@ -126,7 +126,7 @@ aclperms_gen = custom_target(
> )
>
> docs_timestamp = run_command(
> - python3_prog, meson_timestamp_prog.full_path(), env: runutf8, check: true,
> + meson_timestamp_prog, env: runutf8, check: true,
> ).stdout().strip()
>
> site_xsl = files('site.xsl')
> @@ -345,7 +345,7 @@ endforeach
> run_target(
>'install-web',
>command: [
> -meson_python_prog, python3_prog.full_path(),
> meson_install_web_prog.full_path(),
> +meson_python_prog, meson_install_web_prog.full_path(),
> install_web_files,
>],
>depends: install_web_deps,
> @@ -353,9 +353,8 @@ run_target(
>
> test(
>'check-html-references',
> - python3_prog,
> + check_html_references_prog,
>args: [
> -check_html_references_prog.full_path(),
> '--webroot',
> meson.project_build_root() / 'docs'
>],
> diff --git a/meson.build b/meson.build
> index 965ada483b..2c7e9908db 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2097,7 +2097,7 @@ if git
> configuration: spec_conf,
>)
>
> - authors = run_command(python3_prog, meson_gen_authors_prog.full_path(),
> + authors = run_command(meson_gen_authors_prog,
> env: runutf8, check: true)
>authors_file = 'AUTHORS.rst.in'
>
> @@ -2119,7 +2119,7 @@ if git
>
>foreach file : dist_files
> meson.add_dist_script(
> - meson_python_prog.full_path(), python3_prog.full_path(),
> meson_dist_prog.full_path(),
> + meson_python_prog.full_path(), meson_dist_prog.full_path(),
>meson.project_build_root(), file
> )
>endforeach
> diff --git a/src/access/meson.build b/src/access/meson.build
> index e65f17c0a2..0ddfbd9d9e 100644
> --- a/src/access/meson.build
> +++ b/src/access/meson.build
> @@ -70,7 +70,7 @@ if conf.has('WITH_POLKIT')
>'org.libvirt.api.policy',
>input: access_perm_h,
>output: 'org.libvirt.api.policy',
> - command: [ meson_python_prog, python3_prog, genpolkit_prog, '@INPUT@'
> ],
> + command: [ meson_python_prog, genpolkit_prog, '@INPUT@' ],
>capture: true,
>install: true,
>install_dir: datadir / 'polkit-1' / 'actions',
> @@ -107,8 +107,8 @@ generated_sym_files += access_gen_sym
>
> test(
>'check-aclperms',
> - python3_prog,
> - args: [ check_aclperms_prog.full_path(), access_perm_h,
> files('viraccessperm.c') ],
> + check_aclperms_prog,
> + args: [ access_perm_h, files('viraccessperm.c') ],
>env: runutf8,
>suite: 'script'
> )
> diff --git a/src/meson.build b/src/meson.build
> index 28e4d0cc4e..606f238a95 100644
> --- a/src/meson.build
> +++ b/src/meson.build
> @@ -75,7 +75,7 @@ if conf.has('WITH_DTRACE_PROBES')
>
[libvirt PATCH v3 13/25] qemu_snapshot: rename qemuSnapshotDeleteExternalPrepare
The new name reflects that we prepare data for external snapshot
deletion and the old name will be used later for different part of code.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 0534989e37..dd88e478b8 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2623,9 +2623,9 @@ qemuSnapshotFindParentSnapForDisk(virDomainMomentObj
*snap,
static int
-qemuSnapshotDeleteExternalPrepare(virDomainObj *vm,
- virDomainMomentObj *snap,
- GSList **externalData)
+qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
+ virDomainMomentObj *snap,
+ GSList **externalData)
{
ssize_t i;
virDomainSnapshotDef *snapdef = virDomainSnapshotObjGetDef(snap);
@@ -3482,7 +3482,7 @@ qemuSnapshotDelete(virDomainObj *vm,
g_autoslist(qemuSnapshotDeleteExternalData) tmpData = NULL;
/* this also serves as validation whether the snapshot can be
deleted */
-if (qemuSnapshotDeleteExternalPrepare(vm, snap, ) < 0)
+if (qemuSnapshotDeleteExternalPrepareData(vm, snap, ) < 0)
goto endjob;
if (!virDomainObjIsActive(vm)) {
@@ -3497,7 +3497,7 @@ qemuSnapshotDelete(virDomainObj *vm,
/* Call the prepare again as some data require that the VM is
* running to get everything we need. */
-if (qemuSnapshotDeleteExternalPrepare(vm, snap, )
< 0)
+if (qemuSnapshotDeleteExternalPrepareData(vm, snap,
) < 0)
goto endjob;
} else {
qemuDomainJobPrivate *jobPriv = vm->job->privateData;
--
2.41.0
[libvirt PATCH v3 06/25] qemu_snapshot: introduce qemuSnapshotDomainDefUpdateDisk
Extract the code that updates disks in domain definition while creating
external snapshots. We will use it later in the external snapshot revert
code.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 63
1 file changed, 38 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 844b02d427..72a0f71d4f 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -142,6 +142,42 @@ qemuSnapshotFSThaw(virDomainObj *vm,
}
+static int
+qemuSnapshotDomainDefUpdateDisk(virDomainDef *domdef,
+virDomainSnapshotDef *snapdef,
+bool reuse)
+{
+size_t i;
+
+for (i = 0; i < snapdef->ndisks; i++) {
+g_autoptr(virStorageSource) newsrc = NULL;
+virDomainSnapshotDiskDef *snapdisk = &(snapdef->disks[i]);
+virDomainDiskDef *defdisk = domdef->disks[i];
+
+if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
+
+if (!(newsrc = virStorageSourceCopy(snapdisk->src, false)))
+return -1;
+
+if (virStorageSourceInitChainElement(newsrc, defdisk->src, false) < 0)
+return -1;
+
+if (!reuse &&
+virStorageSourceHasBacking(defdisk->src)) {
+defdisk->src->readonly = true;
+newsrc->backingStore = g_steal_pointer(>src);
+} else {
+virObjectUnref(defdisk->src);
+}
+
+defdisk->src = g_steal_pointer();
+}
+
+return 0;
+}
+
+
/* The domain is expected to be locked and inactive. */
static int
qemuSnapshotCreateInactiveInternal(virQEMUDriver *driver,
@@ -216,31 +252,8 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver,
}
/* update disk definitions */
-for (i = 0; i < snapdef->ndisks; i++) {
-g_autoptr(virStorageSource) newsrc = NULL;
-
-snapdisk = &(snapdef->disks[i]);
-defdisk = vm->def->disks[i];
-
-if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
-continue;
-
-if (!(newsrc = virStorageSourceCopy(snapdisk->src, false)))
-goto cleanup;
-
-if (virStorageSourceInitChainElement(newsrc, defdisk->src, false) < 0)
-goto cleanup;
-
-if (!reuse &&
-virStorageSourceHasBacking(defdisk->src)) {
-defdisk->src->readonly = true;
-newsrc->backingStore = g_steal_pointer(>src);
-} else {
-virObjectUnref(defdisk->src);
-}
-
-defdisk->src = g_steal_pointer();
-}
+if (qemuSnapshotDomainDefUpdateDisk(vm->def, snapdef, reuse) < 0)
+goto cleanup;
if (virDomainDefSave(vm->def, driver->xmlopt, cfg->configDir) < 0)
goto cleanup;
--
2.41.0
[libvirt PATCH v3 08/25] qemu_snapshot: introduce qemuSnapshotCreateQcow2Files
Extract creation of qcow2 files for external snapshots to separate
function as we will need it for external snapshot revert code.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 85 ++--
1 file changed, 56 insertions(+), 29 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 8e1eb21b5d..f43548e29f 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -181,43 +181,37 @@ qemuSnapshotDomainDefUpdateDisk(virDomainDef *domdef,
}
-/* The domain is expected to be locked and inactive. */
+/**
+ * qemuSnapshotCreateQcow2Files:
+ * @vm: domain object
+ * @snapdef: snapshot definition
+ * @created: bitmap to store which disks were created
+ *
+ * Create new qcow2 images based on snapshot definition @snapdef and use
+ * domain object @vm as source for backing images.
+ *
+ * Returns 0 on success, -1 on error.
+ */
static int
-qemuSnapshotCreateInactiveInternal(virQEMUDriver *driver,
- virDomainObj *vm,
- virDomainMomentObj *snap)
-{
-return qemuDomainSnapshotForEachQcow2(driver, vm->def, snap, "-c", false);
-}
-
-
-/* The domain is expected to be locked and inactive. */
-static int
-qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver,
- virDomainObj *vm,
- virDomainMomentObj *snap,
- bool reuse)
+qemuSnapshotCreateQcow2Files(virDomainObj *vm,
+ virDomainSnapshotDef *snapdef,
+ virBitmap *created)
{
size_t i;
-virDomainSnapshotDiskDef *snapdisk;
-virDomainDiskDef *defdisk;
const char *qemuImgPath;
-g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-int ret = -1;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
-virDomainSnapshotDef *snapdef = virDomainSnapshotObjGetDef(snap);
-g_autoptr(virBitmap) created = virBitmapNew(snapdef->ndisks);
+virQEMUDriver *driver = ((qemuDomainObjPrivate *) vm->privateData)->driver;
+virDomainSnapshotDiskDef *snapdisk = NULL;
+virDomainDiskDef *defdisk = NULL;
if (!(qemuImgPath = qemuFindQemuImgBinary(driver)))
-goto cleanup;
+return -1;
-/* If reuse is true, then qemuSnapshotPrepare already
- * ensured that the new files exist, and it was up to the user to
- * create them correctly. */
-for (i = 0; i < snapdef->ndisks && !reuse; i++) {
+for (i = 0; i < snapdef->ndisks; i++) {
g_autoptr(virCommand) cmd = NULL;
snapdisk = &(snapdef->disks[i]);
defdisk = vm->def->disks[i];
+
if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
continue;
@@ -225,7 +219,7 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver,
snapdisk->src->format = VIR_STORAGE_FILE_QCOW2;
if (qemuDomainStorageSourceValidateDepth(defdisk->src, 1,
defdisk->dst) < 0)
-goto cleanup;
+return -1;
/* creates cmd line args: qemu-img create -f qcow2 -o */
if (!(cmd = virCommandNewArgList(qemuImgPath,
@@ -234,7 +228,7 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver,
virStorageFileFormatTypeToString(snapdisk->src->format),
"-o",
NULL)))
-goto cleanup;
+return -1;
/* adds cmd line arg:
backing_fmt=format,backing_file=/path/to/backing/file */
virBufferAsprintf(, "backing_fmt=%s,backing_file=",
@@ -251,9 +245,42 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver,
ignore_value(virBitmapSetBit(created, i));
if (virCommandRun(cmd, NULL) < 0)
-goto cleanup;
+return -1;
}
+return 0;
+}
+
+
+/* The domain is expected to be locked and inactive. */
+static int
+qemuSnapshotCreateInactiveInternal(virQEMUDriver *driver,
+ virDomainObj *vm,
+ virDomainMomentObj *snap)
+{
+return qemuDomainSnapshotForEachQcow2(driver, vm->def, snap, "-c", false);
+}
+
+
+/* The domain is expected to be locked and inactive. */
+static int
+qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver,
+ virDomainObj *vm,
+ virDomainMomentObj *snap,
+ bool reuse)
+{
+virDomainSnapshotDiskDef *snapdisk;
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+int ret = -1;
+virDomainSnapshotDef *snapdef = virDomainSnapshotObjGetDef(snap);
+g_autoptr(virBitmap) created = virBitmapNew(snapdef->ndisks);
+
+/*
[libvirt PATCH v3 16/25] qemu_snapshot: prepare data for non-active leaf external snapshot deletion
In this case there is no need to run block commit and using qemu process
at all.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 55 +++-
1 file changed, 32 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index d47dc72709..8b005cae8f 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2759,34 +2759,43 @@ qemuSnapshotDeleteExternalPrepare(virDomainObj *vm,
return 0;
}
-/* this also serves as validation whether the snapshot can be deleted */
-if (qemuSnapshotDeleteExternalPrepareData(vm, snap, true, ) < 0)
-return -1;
-
-if (!virDomainObjIsActive(vm)) {
-if (qemuProcessStart(NULL, driver, vm, NULL, VIR_ASYNC_JOB_SNAPSHOT,
- NULL, -1, NULL, NULL,
- VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
- VIR_QEMU_PROCESS_START_PAUSED) < 0) {
-return -1;
-}
-
-*stop_qemu = true;
-
-/* Call the prepare again as some data require that the VM is
- * running to get everything we need. */
-if (qemuSnapshotDeleteExternalPrepareData(vm, snap, true,
externalData) < 0)
+/* Leaf non-active snapshot doesn't have overlay files for the disk images
+ * so there is no need to do any merge and we can just delete the files
+ * directly. */
+if (snap != virDomainSnapshotGetCurrent(vm->snapshots) &&
+snap->nchildren == 0) {
+if (qemuSnapshotDeleteExternalPrepareData(vm, snap, false,
externalData) < 0)
return -1;
} else {
-qemuDomainJobPrivate *jobPriv = vm->job->privateData;
+/* this also serves as validation whether the snapshot can be deleted
*/
+if (qemuSnapshotDeleteExternalPrepareData(vm, snap, true, ) <
0)
+return -1;
-*externalData = g_steal_pointer();
+if (!virDomainObjIsActive(vm)) {
+if (qemuProcessStart(NULL, driver, vm, NULL,
VIR_ASYNC_JOB_SNAPSHOT,
+ NULL, -1, NULL, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
+ VIR_QEMU_PROCESS_START_PAUSED) < 0) {
+return -1;
+}
-/* If the VM is running we need to indicate that the async snapshot
- * job is snapshot delete job. */
-jobPriv->snapshotDelete = true;
+*stop_qemu = true;
-qemuDomainSaveStatus(vm);
+/* Call the prepare again as some data require that the VM is
+ * running to get everything we need. */
+if (qemuSnapshotDeleteExternalPrepareData(vm, snap, true,
externalData) < 0)
+return -1;
+} else {
+qemuDomainJobPrivate *jobPriv = vm->job->privateData;
+
+*externalData = g_steal_pointer();
+
+/* If the VM is running we need to indicate that the async snapshot
+ * job is snapshot delete job. */
+jobPriv->snapshotDelete = true;
+
+qemuDomainSaveStatus(vm);
+}
}
return 0;
--
2.41.0
[libvirt PATCH v3 10/25] qemu_snapshot: move external disk prepare to single function
We will need to reuse the functionality when reverting external
snapshots.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 38 +++---
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 50f871baf3..a090ca13d4 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -527,6 +527,25 @@ qemuSnapshotPrepareDiskExternal(virDomainDiskDef *disk,
bool active,
bool reuse)
{
+if (!snapdisk->src->format) {
+snapdisk->src->format = VIR_STORAGE_FILE_QCOW2;
+} else if (snapdisk->src->format != VIR_STORAGE_FILE_QCOW2 &&
+ snapdisk->src->format != VIR_STORAGE_FILE_QED) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("external snapshot format for disk %1$s is
unsupported: %2$s"),
+ snapdisk->name,
+
virStorageFileFormatTypeToString(snapdisk->src->format));
+return -1;
+}
+
+if (snapdisk->src->metadataCacheMaxSize > 0) {
+if (snapdisk->src->format != VIR_STORAGE_FILE_QCOW2) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("metadata cache max size control is supported
only with qcow2 images"));
+return -1;
+}
+}
+
if (qemuTranslateSnapshotDiskSourcePool(snapdisk) < 0)
return -1;
@@ -707,25 +726,6 @@ qemuSnapshotPrepare(virDomainObj *vm,
break;
case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL:
-if (!disk->src->format) {
-disk->src->format = VIR_STORAGE_FILE_QCOW2;
-} else if (disk->src->format != VIR_STORAGE_FILE_QCOW2 &&
- disk->src->format != VIR_STORAGE_FILE_QED) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("external snapshot format for disk %1$s is
unsupported: %2$s"),
- disk->name,
-
virStorageFileFormatTypeToString(disk->src->format));
-return -1;
-}
-
-if (disk->src->metadataCacheMaxSize > 0) {
-if (disk->src->format != VIR_STORAGE_FILE_QCOW2) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("metadata cache max size control is
supported only with qcow2 images"));
-return -1;
-}
-}
-
if (qemuSnapshotPrepareDiskExternal(dom_disk, disk, active, reuse)
< 0)
return -1;
--
2.41.0
[libvirt PATCH v3 25/25] qemu_snapshot: allow snapshot revert for external snapshots
Now that the support to revert external snapshots is implemented we can
drop this check.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 692d88e24d..d943281e35 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1892,12 +1892,6 @@ qemuSnapshotRevertValidate(virDomainObj *vm,
return -1;
}
-if (virDomainSnapshotIsExternal(snap)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("revert to external snapshot not supported yet"));
-return -1;
-}
-
if (!snap->def->dom) {
virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY,
_("snapshot '%1$s' lacks domain '%2$s' rollback info"),
--
2.41.0
[libvirt PATCH v3 22/25] qemu_snapshot: update backing store after deleting external snapshot
With introduction of external snapshot revert we will have to update
backing store of qcow images not actively used be QEMU manually.
The need for this patch comes from the fact that we stop and start QEMU
process therefore after revert not all existing snapshots will be known
to that QEMU process due to reverting to non-leaf snapshot or having
multiple branches.
We need to loop over all existing snapshots and check all disks to see
if they happen to have the image we are deleting as backing store and
update them to point to the new image except for images currently used
by the running QEMU process doing the merge operation.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 122 +++
1 file changed, 122 insertions(+)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 0238ab2249..8d0581d33b 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2588,6 +2588,8 @@ typedef struct _qemuSnapshotDeleteExternalData {
virStorageSource *parentDiskSrc; /* backing disk source of the @diskSrc */
virStorageSource *prevDiskSrc; /* source of disk for which @diskSrc is
backing disk */
+GSList *disksWithBacking; /* list of storage source data for which the
+ deleted storage source is backing store */
qemuBlockJobData *job;
bool merge;
} qemuSnapshotDeleteExternalData;
@@ -2600,6 +2602,7 @@
qemuSnapshotDeleteExternalDataFree(qemuSnapshotDeleteExternalData *data)
return;
virObjectUnref(data->job);
+g_slist_free_full(data->disksWithBacking, g_free);
g_free(data);
}
@@ -2649,6 +2652,84 @@ qemuSnapshotFindParentSnapForDisk(virDomainMomentObj
*snap,
}
+struct _qemuSnapshotDisksWithBackingStoreData {
+virStorageSource *diskSrc;
+uid_t uid;
+gid_t gid;
+};
+
+
+struct _qemuSnapshotDisksWithBackingStoreIterData {
+virDomainMomentObj *current;
+virStorageSource *diskSrc;
+GSList **disksWithBacking;
+virQEMUDriverConfig *cfg;
+};
+
+
+static int
+qemuSnapshotDiskHasBackingDisk(void *payload,
+ const char *name G_GNUC_UNUSED,
+ void *opaque)
+{
+virDomainMomentObj *snap = payload;
+virDomainSnapshotDef *snapdef = virDomainSnapshotObjGetDef(snap);
+struct _qemuSnapshotDisksWithBackingStoreIterData *iterdata = opaque;
+ssize_t i;
+
+/* skip snapshots that are within the active snapshot tree as it will be
handled
+ * by qemu */
+if (virDomainMomentIsAncestor(iterdata->current, snap) ||
iterdata->current == snap)
+return 0;
+
+for (i = 0; i < snapdef->parent.dom->ndisks; i++) {
+virDomainDiskDef *disk = snapdef->parent.dom->disks[i];
+uid_t uid;
+gid_t gid;
+
+if (!virStorageSourceIsLocalStorage(disk->src))
+continue;
+
+qemuDomainGetImageIds(iterdata->cfg, snapdef->parent.dom, disk->src,
+ NULL, , );
+
+if (!disk->src->backingStore)
+ignore_value(virStorageSourceGetMetadata(disk->src, uid, gid, 1,
false));
+
+if (virStorageSourceIsSameLocation(disk->src->backingStore,
iterdata->diskSrc)) {
+struct _qemuSnapshotDisksWithBackingStoreData *data =
+g_new0(struct _qemuSnapshotDisksWithBackingStoreData, 1);
+
+data->diskSrc = disk->src;
+data->uid = uid;
+data->gid = gid;
+
+*iterdata->disksWithBacking =
g_slist_prepend(*iterdata->disksWithBacking, data);
+}
+}
+
+return 0;
+}
+
+
+static void
+qemuSnapshotGetDisksWithBackingStore(virDomainObj *vm,
+ virDomainMomentObj *snap,
+ qemuSnapshotDeleteExternalData *data)
+{
+struct _qemuSnapshotDisksWithBackingStoreIterData iterData;
+virQEMUDriver *driver = QEMU_DOMAIN_PRIVATE(vm)->driver;
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+
+iterData.current = virDomainSnapshotGetCurrent(vm->snapshots);
+iterData.diskSrc = data->diskSrc;
+iterData.disksWithBacking = >disksWithBacking;
+iterData.cfg = cfg;
+
+virDomainMomentForEachDescendant(snap, qemuSnapshotDiskHasBackingDisk,
);
+}
+
+
/**
* qemuSnapshotDeleteExternalPrepareData:
* @vm: domain object
@@ -2733,6 +2814,8 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
_("snapshot VM disk source and parent disk
source are not the same"));
return -1;
}
+
+qemuSnapshotGetDisksWithBackingStore(vm, snap, data);
}
data->parentSnap = qemuSnapshotFindParentSnapForDisk(snap,
data->snapDisk);
@@ -3129,6 +3212,43 @@ qemuSn
[libvirt PATCH v3 11/25] qemu_snapshot: use VIR_ASYNC_JOB_SNAPSHOT when reverting snapshot
Both creating and deleting snapshot are using VIR_ASYNC_JOB_SNAPSHOT but
reverting is using VIR_ASYNC_JOB_START. Let's unify it to make it
consistent for all snapshot operations.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index a090ca13d4..82fbb79ab0 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2008,7 +2008,7 @@ qemuSnapshotRevertActive(virDomainObj *vm,
/* Transitions 5, 6, 8, 9 */
qemuProcessStop(driver, vm,
VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT,
-VIR_ASYNC_JOB_START, 0);
+VIR_ASYNC_JOB_SNAPSHOT, 0);
virDomainAuditStop(vm, "from-snapshot");
detail = VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT;
event = virDomainEventLifecycleNewFromObj(vm,
@@ -2033,7 +2033,7 @@ qemuSnapshotRevertActive(virDomainObj *vm,
rc = qemuProcessStart(snapshot->domain->conn, driver, vm,
cookie ? cookie->cpu : NULL,
- VIR_ASYNC_JOB_START, NULL, -1, NULL, snap,
+ VIR_ASYNC_JOB_SNAPSHOT, NULL, -1, NULL, snap,
VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
start_flags);
virDomainAuditStart(vm, "from-snapshot", rc >= 0);
@@ -2066,7 +2066,7 @@ qemuSnapshotRevertActive(virDomainObj *vm,
}
rc = qemuProcessStartCPUs(driver, vm,
VIR_DOMAIN_RUNNING_FROM_SNAPSHOT,
- VIR_ASYNC_JOB_START);
+ VIR_ASYNC_JOB_SNAPSHOT);
if (rc < 0)
return -1;
}
@@ -2129,7 +2129,7 @@ qemuSnapshotRevertInactive(virDomainObj *vm,
if (virDomainObjIsActive(vm)) {
/* Transitions 4, 7 */
qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT,
-VIR_ASYNC_JOB_START, 0);
+VIR_ASYNC_JOB_SNAPSHOT, 0);
virDomainAuditStop(vm, "from-snapshot");
detail = VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT;
event = virDomainEventLifecycleNewFromObj(vm,
@@ -2156,7 +2156,7 @@ qemuSnapshotRevertInactive(virDomainObj *vm,
start_flags |= paused ? VIR_QEMU_PROCESS_START_PAUSED : 0;
rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL,
- VIR_ASYNC_JOB_START, NULL, -1, NULL, NULL,
+ VIR_ASYNC_JOB_SNAPSHOT, NULL, -1, NULL, NULL,
VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
start_flags);
virDomainAuditStart(vm, "from-snapshot", rc >= 0);
@@ -2223,10 +2223,11 @@ qemuSnapshotRevert(virDomainObj *vm,
return -1;
}
-if (qemuProcessBeginJob(vm,
-VIR_DOMAIN_JOB_OPERATION_SNAPSHOT_REVERT,
-flags) < 0)
+if (virDomainObjBeginAsyncJob(vm, VIR_ASYNC_JOB_SNAPSHOT,
+ VIR_DOMAIN_JOB_OPERATION_SNAPSHOT_REVERT,
+ flags) < 0) {
return -1;
+}
if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))
goto endjob;
@@ -2276,7 +2277,7 @@ qemuSnapshotRevert(virDomainObj *vm,
}
endjob:
-qemuProcessEndJob(vm);
+virDomainObjEndAsyncJob(vm);
return ret;
}
--
2.41.0
[libvirt PATCH v3 21/25] qemuDomainGetImageIds: pass domain definition directly
We only need the domain definition from domain object. This will allow
us to use it from snapshot code where we need to pass different domain
definition.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_backup.c | 2 +-
src/qemu/qemu_blockjob.c | 2 +-
src/qemu/qemu_domain.c | 8
src/qemu/qemu_domain.h | 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 3be33193f4..a94869522d 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -586,7 +586,7 @@ qemuBackupJobTerminate(virDomainObj *vm,
if (!cfg)
cfg = virQEMUDriverGetConfig(priv->driver);
-qemuDomainGetImageIds(cfg, vm, backupdisk->store, NULL, ,
);
+qemuDomainGetImageIds(cfg, vm->def, backupdisk->store, NULL, ,
);
if (virFileRemove(backupdisk->store->path, uid, gid) < 0)
VIR_WARN("failed to remove scratch file '%s'",
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 1cac17c5c0..f1d22df59f 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -932,7 +932,7 @@ qemuBlockJobDeleteImages(virQEMUDriver *driver,
for (; p != NULL; p = p->backingStore) {
if (virStorageSourceGetActualType(p) == VIR_STORAGE_TYPE_FILE) {
-qemuDomainGetImageIds(cfg, vm, p, disk->src, , );
+qemuDomainGetImageIds(cfg, vm->def, p, disk->src, , );
if (virFileRemove(p->path, uid, gid) < 0) {
VIR_WARN("Unable to remove snapshot image file '%s' (%s)",
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 029238a9d7..0bb1fafdec 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7719,7 +7719,7 @@ qemuDomainCleanupRun(virQEMUDriver *driver,
void
qemuDomainGetImageIds(virQEMUDriverConfig *cfg,
- virDomainObj *vm,
+ virDomainDef *def,
virStorageSource *src,
virStorageSource *parentSrc,
uid_t *uid, gid_t *gid)
@@ -7740,7 +7740,7 @@ qemuDomainGetImageIds(virQEMUDriverConfig *cfg,
*gid = cfg->group;
}
-if (vm && (vmlabel = virDomainDefGetSecurityLabelDef(vm->def, "dac")) &&
+if ((vmlabel = virDomainDefGetSecurityLabelDef(def, "dac")) &&
vmlabel->label)
virParseOwnershipIds(vmlabel->label, uid, gid);
@@ -7765,7 +7765,7 @@ qemuDomainStorageFileInit(virQEMUDriver *driver,
uid_t uid;
gid_t gid;
-qemuDomainGetImageIds(cfg, vm, src, parent, , );
+qemuDomainGetImageIds(cfg, vm->def, src, parent, , );
if (virStorageSourceInitAs(src, uid, gid) < 0)
return -1;
@@ -7958,7 +7958,7 @@ qemuDomainDetermineDiskChain(virQEMUDriver *driver,
return 0;
}
-qemuDomainGetImageIds(cfg, vm, src, disksrc, , );
+qemuDomainGetImageIds(cfg, vm->def, src, disksrc, , );
if (virStorageSourceGetMetadata(src, uid, gid,
QEMU_DOMAIN_STORAGE_SOURCE_CHAIN_MAX_DEPTH,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 999190e381..7ac1cdd6be 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -738,7 +738,7 @@ bool qemuDomainDiskChangeSupported(virDomainDiskDef *disk,
virDomainDiskDef *orig_disk);
void qemuDomainGetImageIds(virQEMUDriverConfig *cfg,
- virDomainObj *vm,
+ virDomainDef *def,
virStorageSource *src,
virStorageSource *parentSrc,
uid_t *uid,
--
2.41.0
[libvirt PATCH v3 09/25] qemuSnapshotCreateQcow2Files: use domain definition directly
To create new overlay files when external snapshot revert support is
introduced we will be using different domain definition than what is
currently used by the domain.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index f43548e29f..50f871baf3 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -183,24 +183,25 @@ qemuSnapshotDomainDefUpdateDisk(virDomainDef *domdef,
/**
* qemuSnapshotCreateQcow2Files:
- * @vm: domain object
+ * @driver: QEMU driver
+ * @def: domain definition
* @snapdef: snapshot definition
* @created: bitmap to store which disks were created
*
* Create new qcow2 images based on snapshot definition @snapdef and use
- * domain object @vm as source for backing images.
+ * domain definition @def as source for backing images.
*
* Returns 0 on success, -1 on error.
*/
static int
-qemuSnapshotCreateQcow2Files(virDomainObj *vm,
+qemuSnapshotCreateQcow2Files(virQEMUDriver *driver,
+ virDomainDef *def,
virDomainSnapshotDef *snapdef,
virBitmap *created)
{
size_t i;
const char *qemuImgPath;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
-virQEMUDriver *driver = ((qemuDomainObjPrivate *) vm->privateData)->driver;
virDomainSnapshotDiskDef *snapdisk = NULL;
virDomainDiskDef *defdisk = NULL;
@@ -210,7 +211,7 @@ qemuSnapshotCreateQcow2Files(virDomainObj *vm,
for (i = 0; i < snapdef->ndisks; i++) {
g_autoptr(virCommand) cmd = NULL;
snapdisk = &(snapdef->disks[i]);
-defdisk = vm->def->disks[i];
+defdisk = def->disks[i];
if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
continue;
@@ -278,7 +279,7 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver,
/* If reuse is true, then qemuSnapshotPrepare already
* ensured that the new files exist, and it was up to the user to
* create them correctly. */
-if (!reuse && qemuSnapshotCreateQcow2Files(vm, snapdef, created) < 0)
+if (!reuse && qemuSnapshotCreateQcow2Files(driver, vm->def, snapdef,
created) < 0)
goto cleanup;
/* update disk definitions */
--
2.41.0
[libvirt PATCH v3 17/25] qemu_snapshot: add support to delete external snapshot without block commit
When block commit is not needed we can just simply unlink the
disk files.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 56 ++--
1 file changed, 36 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 8b005cae8f..a67be1ae49 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2563,6 +2563,7 @@ typedef struct _qemuSnapshotDeleteExternalData {
virStorageSource *prevDiskSrc; /* source of disk for which @diskSrc is
backing disk */
qemuBlockJobData *job;
+bool merge;
} qemuSnapshotDeleteExternalData;
@@ -2664,6 +2665,7 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
data = g_new0(qemuSnapshotDeleteExternalData, 1);
data->snapDisk = snapDisk;
+data->merge = merge;
data->parentDomDisk = virDomainDiskByTarget(snapdef->parent.dom,
data->snapDisk->name);
@@ -2674,7 +2676,7 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
return -1;
}
-if (merge) {
+if (data->merge) {
data->domDisk = qemuDomainDiskByName(vm->def, snapDisk->name);
if (!data->domDisk)
return -1;
@@ -3114,31 +3116,42 @@ qemuSnapshotDiscardExternal(virDomainObj *vm,
virTristateBool autofinalize = VIR_TRISTATE_BOOL_NO;
unsigned int commitFlags = VIR_DOMAIN_BLOCK_COMMIT_DELETE;
-if (data->domDisk->src == data->diskSrc) {
-commitFlags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE;
-autofinalize = VIR_TRISTATE_BOOL_YES;
+if (data->merge) {
+if (data->domDisk->src == data->diskSrc) {
+commitFlags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE;
+autofinalize = VIR_TRISTATE_BOOL_YES;
+}
+
+if (qemuSnapshotSetInvalid(vm, data->parentSnap, data->snapDisk,
true) < 0)
+goto error;
+
+data->job = qemuBlockCommit(vm,
+data->domDisk,
+data->parentDiskSrc,
+data->diskSrc,
+data->prevDiskSrc,
+0,
+VIR_ASYNC_JOB_SNAPSHOT,
+autofinalize,
+commitFlags);
+
+if (!data->job)
+goto error;
+} else {
+if (virStorageSourceInit(data->parentDomDisk->src) < 0 ||
+virStorageSourceUnlink(data->parentDomDisk->src) < 0) {
+VIR_WARN("Failed to remove snapshot image '%s'",
+ data->snapDisk->name);
+}
}
-
-if (qemuSnapshotSetInvalid(vm, data->parentSnap, data->snapDisk, true)
< 0)
-goto error;
-
-data->job = qemuBlockCommit(vm,
-data->domDisk,
-data->parentDiskSrc,
-data->diskSrc,
-data->prevDiskSrc,
-0,
-VIR_ASYNC_JOB_SNAPSHOT,
-autofinalize,
-commitFlags);
-
-if (!data->job)
-goto error;
}
for (cur = externalData; cur; cur = g_slist_next(cur)) {
qemuSnapshotDeleteExternalData *data = cur->data;
+if (!data->merge)
+continue;
+
if (qemuSnapshotDeleteBlockJobRunning(vm, data->job) < 0)
goto error;
@@ -3153,6 +3166,9 @@ qemuSnapshotDiscardExternal(virDomainObj *vm,
for (cur = externalData; cur; cur = g_slist_next(cur)) {
qemuSnapshotDeleteExternalData *data = cur->data;
+if (!data->merge)
+continue;
+
if (data->job->state == QEMU_BLOCKJOB_STATE_READY) {
if (qemuBlockPivot(vm, data->job, VIR_ASYNC_JOB_SNAPSHOT, NULL) <
0)
goto error;
--
2.41.0
[libvirt PATCH v3 23/25] qemu_snapshot: check only once if snapshot is external
There will be more external snapshot checks introduced by following
patch so group them together.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 8d0581d33b..dca95b415b 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -3728,18 +3728,18 @@ qemuSnapshotDeleteValidate(virDomainObj *vm,
}
}
-if (virDomainSnapshotIsExternal(snap) &&
-qemuDomainHasBlockjob(vm, false)) {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("cannot delete external snapshots when there is
another active block job"));
-return -1;
-}
+if (virDomainSnapshotIsExternal(snap)) {
+if (qemuDomainHasBlockjob(vm, false)) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("cannot delete external snapshots when there is
another active block job"));
+return -1;
+}
-if (virDomainSnapshotIsExternal(snap) &&
-(flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("deletion of external disk snapshots with children
not supported"));
-return -1;
+if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("deletion of external disk snapshots with
children not supported"));
+return -1;
+}
}
return 0;
--
2.41.0
[libvirt PATCH v3 01/25] libvirt_private: list virDomainMomentDefPostParse
We will need to call this function from qemu_snapshot when introducing external snapshot revert support. Signed-off-by: Pavel Hrdina Reviewed-by: Peter Krempa --- src/libvirt_private.syms | 4 1 file changed, 4 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index da60c965dd..aeb54d8952 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -807,6 +807,10 @@ virInterfaceDefParseString; virInterfaceDefParseXML; +# conf/moment_conf.h +virDomainMomentDefPostParse; + + # conf/netdev_bandwidth_conf.h virDomainClearNetBandwidth; virNetDevBandwidthFormat; -- 2.41.0
[libvirt PATCH v3 12/25] qemu_snapshot: introduce external snapshot revert support
When reverting to external snapshot we need to create new overlay qcow2
files from the disk files the VM had when the snapshot was taken.
There are some specifics and limitations when reverting to a snapshot:
1) When reverting to last snapshot we need to first create new overlay
files before we can safely delete the old overlay files in case the
creation fails so we have still recovery option when we error out.
These new files will not have the suffix as when the snapshot was
created as renaming the original files in order to use the same file
names as when the snapshot was created would add unnecessary
complexity to the code.
2) When reverting to any snapshot we will always create overlay files
for every disk the VM had when the snapshot was done. Otherwise we
would have to figure out if there is any other qcow2 image already
using any of the VM disks as backing store and that itself might be
extremely complex and in some cases impossible.
3) When reverting from any state the current overlay files will be
always removed as that VM state is not meant to be saved. It's the
same as with internal snapshots. If user want's to keep the current
state before reverting they need to create a new snapshot. For now
this will only work if the current snapshot is the last.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 278 ++-
1 file changed, 274 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 82fbb79ab0..0534989e37 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -18,6 +18,8 @@
#include
+#include
+
#include "qemu_snapshot.h"
#include "qemu_monitor.h"
@@ -1982,6 +1984,229 @@ qemuSnapshotRevertWriteMetadata(virDomainObj *vm,
}
+/**
+ * qemuSnapshotRevertExternalPrepare:
+ * @vm: domain object
+ * @tmpsnapdef: temporary snapshot definition
+ * @snap: snapshot object we are reverting to
+ * @config: live domain definition
+ * @inactiveConfig: offline domain definition
+ * memsnapFD: pointer to store memory state file FD or NULL
+ * memsnapPath: pointer to store memory state file path or NULL
+ *
+ * Prepare new temporary snapshot definition @tmpsnapdef that will
+ * be used while creating new overlay files after reverting to snapshot
+ * @snap. In case we are reverting to snapshot with memory state it will
+ * open it and pass FD via @memsnapFD and path to the file via
+ * @memsnapPath, caller is responsible for freeing both @memsnapFD and
+ * memsnapPath.
+ *
+ * Returns 0 in success, -1 on error.
+ */
+static int
+qemuSnapshotRevertExternalPrepare(virDomainObj *vm,
+ virDomainSnapshotDef *tmpsnapdef,
+ virDomainMomentObj *snap,
+ virDomainDef *config,
+ virDomainDef *inactiveConfig,
+ int *memsnapFD,
+ char **memsnapPath)
+{
+size_t i;
+bool active = virDomainObjIsActive(vm);
+virDomainDef *domdef = NULL;
+virDomainSnapshotDef *snapdef = virDomainSnapshotObjGetDef(snap);
+
+if (config) {
+domdef = config;
+} else {
+domdef = inactiveConfig;
+}
+
+/* We need this to generate creation timestamp that is used as default
+ * snapshot name. */
+if (virDomainMomentDefPostParse(>parent) < 0)
+return -1;
+
+if (virDomainSnapshotAlignDisks(tmpsnapdef, domdef,
+VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL,
+false, true) < 0) {
+return -1;
+}
+
+for (i = 0; i < tmpsnapdef->ndisks; i++) {
+virDomainSnapshotDiskDef *snapdisk = >disks[i];
+virDomainDiskDef *domdisk = domdef->disks[i];
+
+if (qemuSnapshotPrepareDiskExternal(domdisk, snapdisk, active, false)
< 0)
+return -1;
+}
+
+if (memsnapFD && memsnapPath && snapdef->memorysnapshotfile) {
+virQEMUDriver *driver = ((qemuDomainObjPrivate *)
vm->privateData)->driver;
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+
+*memsnapPath = snapdef->memorysnapshotfile;
+*memsnapFD = qemuDomainOpenFile(cfg, NULL, *memsnapPath, O_RDONLY,
NULL);
+}
+
+return 0;
+}
+
+
+/**
+ * qemuSnapshotRevertExternalActive:
+ * @vm: domain object
+ * @tmpsnapdef: temporary snapshot definition
+ *
+ * Creates a new disk overlays using the temporary snapshot
+ * definition @tmpsnapdef for running VM by calling QMP APIs.
+ *
+ * Returns 0 on success, -1 on error.
+ */
+static int
+qemuSnapshotRevertExternalActive(virDomainObj *vm,
+ virDomainSnapshotDef *tmpsnapdef)
+{
+size_t i;
+g_autoptr(GHashTable) blockNamedNodeData = NULL;
[libvirt PATCH v3 20/25] virdomainmomentobjlist: introduce virDomainMomentIsAncestor
This new helper will allow us to check if we are able to delete external
snapshot after user did revert to non-leaf snapshot.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/conf/virdomainmomentobjlist.c | 17 +
src/conf/virdomainmomentobjlist.h | 4
src/libvirt_private.syms | 1 +
3 files changed, 22 insertions(+)
diff --git a/src/conf/virdomainmomentobjlist.c
b/src/conf/virdomainmomentobjlist.c
index e5cdd9a141..ea9850df8c 100644
--- a/src/conf/virdomainmomentobjlist.c
+++ b/src/conf/virdomainmomentobjlist.c
@@ -582,3 +582,20 @@ virDomainMomentFindLeaf(virDomainMomentObjList *list)
return moment;
return NULL;
}
+
+
+/* Check if @moment is descendant of @ancestor. */
+bool
+virDomainMomentIsAncestor(virDomainMomentObj *moment,
+ virDomainMomentObj *ancestor)
+{
+if (moment == ancestor)
+return false;
+
+for (moment = moment->parent; moment; moment = moment->parent) {
+if (moment == ancestor)
+return true;
+}
+
+return false;
+}
diff --git a/src/conf/virdomainmomentobjlist.h
b/src/conf/virdomainmomentobjlist.h
index d2ab3b46b1..2ea6b181c0 100644
--- a/src/conf/virdomainmomentobjlist.h
+++ b/src/conf/virdomainmomentobjlist.h
@@ -157,3 +157,7 @@ virDomainMomentCheckCycles(virDomainMomentObjList *list,
virDomainMomentObj *
virDomainMomentFindLeaf(virDomainMomentObjList *list);
+
+bool
+virDomainMomentIsAncestor(virDomainMomentObj *moment,
+ virDomainMomentObj *ancestor);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a75156f097..1cf40ba863 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1221,6 +1221,7 @@ virDomainMomentDropChildren;
virDomainMomentDropParent;
virDomainMomentForEachChild;
virDomainMomentForEachDescendant;
+virDomainMomentIsAncestor;
virDomainMomentMoveChildren;
virDomainMomentObjFree;
virDomainMomentObjNew;
--
2.41.0
[libvirt PATCH v3 02/25] snapshot_conf: export virDomainSnapshotDiskDefClear
We will need to call this function from qemu_snapshot when introducing
external snapshot revert support.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/conf/snapshot_conf.c | 2 +-
src/conf/snapshot_conf.h | 3 +++
src/libvirt_private.syms | 1 +
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 0acba95d7f..cc59bddbc8 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -74,7 +74,7 @@ VIR_ENUM_IMPL(virDomainSnapshotState,
);
/* Snapshot Def functions */
-static void
+void
virDomainSnapshotDiskDefClear(virDomainSnapshotDiskDef *disk)
{
VIR_FREE(disk->name);
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 96c77ef42b..ad49990a1e 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -59,6 +59,9 @@ struct _virDomainSnapshotDiskDef {
virStorageSource *src;
};
+void
+virDomainSnapshotDiskDefClear(virDomainSnapshotDiskDef *disk);
+
void
virDomainSnapshotDiskDefFree(virDomainSnapshotDiskDef *disk);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index aeb54d8952..a75156f097 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1037,6 +1037,7 @@ virDomainSnapshotDefFormat;
virDomainSnapshotDefIsExternal;
virDomainSnapshotDefNew;
virDomainSnapshotDefParseString;
+virDomainSnapshotDiskDefClear;
virDomainSnapshotDiskDefFree;
virDomainSnapshotDiskDefParseXML;
virDomainSnapshotFormatConvertXMLFlags;
--
2.41.0
[libvirt PATCH v3 19/25] qemu_snapshot: remove revertdisks when creating new snapshot
When user creates a new snapshot after reverting to non-leaf snapshot we
no longer need to store the temporary overlays as they will be part of
the VM XMLs stored in the newly created snapshot.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 28 +++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 6dd420a53b..0238ab2249 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1659,6 +1659,28 @@ qemuSnapshotCreateWriteMetadata(virDomainObj *vm,
}
+static void
+qemuSnapshotClearRevertdisks(virDomainMomentObj *current)
+{
+virDomainSnapshotDef *curdef = NULL;
+
+if (!current)
+return;
+
+curdef = virDomainSnapshotObjGetDef(current);
+
+if (curdef->revertdisks) {
+size_t i;
+
+for (i = 0; i < curdef->nrevertdisks; i++)
+virDomainSnapshotDiskDefClear(>revertdisks[i]);
+
+g_clear_pointer(>revertdisks, g_free);
+curdef->nrevertdisks = 0;
+}
+}
+
+
static virDomainSnapshotPtr
qemuSnapshotRedefine(virDomainObj *vm,
virDomainPtr domain,
@@ -1668,6 +1690,7 @@ qemuSnapshotRedefine(virDomainObj *vm,
unsigned int flags)
{
virDomainMomentObj *snap = NULL;
+virDomainMomentObj *current = virDomainSnapshotGetCurrent(vm->snapshots);
virDomainSnapshotPtr ret = NULL;
g_autoptr(virDomainSnapshotDef) snapdef = virObjectRef(snapdeftmp);
@@ -1685,8 +1708,10 @@ qemuSnapshotRedefine(virDomainObj *vm,
* makes sense, such as checking that qemu-img recognizes the
* snapshot name in at least one of the domain's disks? */
-if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)
+if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT) {
+qemuSnapshotClearRevertdisks(current);
qemuSnapshotSetCurrent(vm, snap);
+}
if (qemuSnapshotCreateWriteMetadata(vm, snap, driver, cfg) < 0)
goto error;
@@ -1765,6 +1790,7 @@ qemuSnapshotCreate(virDomainObj *vm,
}
if (!tmpsnap) {
+qemuSnapshotClearRevertdisks(current);
qemuSnapshotSetCurrent(vm, snap);
if (qemuSnapshotCreateWriteMetadata(vm, snap, driver, cfg) < 0)
--
2.41.0
[libvirt PATCH v3 00/25] introduce external snapshot revert support
This implements virDomainRevertToSnapshot to work with external snapshots. In addition it modifies virDomainSnapshotDelete to work correctly when we revert to non-leaf snapshot or when there is non-linear snapshot tree with multiple branches. Gitlab repo with the patches: https://gitlab.com/hrdina/libvirt/-/tree/snapshot-revert-external changes in v3: - `revertdisks` is properly freed in virDomainSnapshotDefDispose() - qemuSnapshotCreateQcow2Files() no longer takes `reuse` as argument and was changed to take `virDomainDef *` instead of `virDomainObj *` - proper commit message for `qemu_snapshot: use VIR_ASYNC_JOB_SNAPSHOT when reverting snapshot` - fixed incorrect usage of `ssize_t i` - dropped the weird logic from qemuSnapshotRevertExternalInactive() as we only need offline VM definition and preserve correct error message if creating qcow files fails - qemuSnapshotClearRevertdisks() correctly frees `revertdisks` - added new patches 'qemuDomainGetImageIds: pass domain definition directly` as we need to modify the function to take `virDomainDef *` directly - qemuSnapshotDiskHasBackingDisk() now uses qemuDomainGetImageIds() to get correct UID and GID for virStorageSourceGetMetadata() and also for virCommandRun() as well by storing it in `struct _qemuSnapshotDisksWithBackingStoreData` Pavel Hrdina (25): libvirt_private: list virDomainMomentDefPostParse snapshot_conf: export virDomainSnapshotDiskDefClear snapshot_conf: use alternate domain definition in virDomainSnapshotDefAssignExternalNames snapshot_conf: introduce metadata element virDomainSnapshotAlignDisks: Allow overriding user-configured snapshot default qemu_snapshot: introduce qemuSnapshotDomainDefUpdateDisk qemu_snapshot: use virDomainDiskByName while updating domain def qemu_snapshot: introduce qemuSnapshotCreateQcow2Files qemuSnapshotCreateQcow2Files: use domain definition directly qemu_snapshot: move external disk prepare to single function qemu_snapshot: use VIR_ASYNC_JOB_SNAPSHOT when reverting snapshot qemu_snapshot: introduce external snapshot revert support qemu_snapshot: rename qemuSnapshotDeleteExternalPrepare qemu_snapshot: extract external snapshot delete prepare to function qemu_snapshot: add merge to external snapshot delete prepare data qemu_snapshot: prepare data for non-active leaf external snapshot deletion qemu_snapshot: add support to delete external snapshot without block commit qemu_snapshot: delete: properly update parent snapshot with revert data qemu_snapshot: remove revertdisks when creating new snapshot virdomainmomentobjlist: introduce virDomainMomentIsAncestor qemuDomainGetImageIds: pass domain definition directly qemu_snapshot: update backing store after deleting external snapshot qemu_snapshot: check only once if snapshot is external qemu_snapshot: add checks for external snapshot deletion qemu_snapshot: allow snapshot revert for external snapshots src/conf/schemas/domainsnapshot.rng | 7 + src/conf/snapshot_conf.c| 55 +- src/conf/snapshot_conf.h| 11 +- src/conf/virdomainmomentobjlist.c | 17 + src/conf/virdomainmomentobjlist.h | 4 + src/libvirt_private.syms| 6 + src/qemu/qemu_backup.c | 2 +- src/qemu/qemu_blockjob.c| 2 +- src/qemu/qemu_domain.c | 8 +- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_snapshot.c| 973 ++-- src/test/test_driver.c | 2 +- 12 files changed, 886 insertions(+), 203 deletions(-) -- 2.41.0
[libvirt PATCH v3 03/25] snapshot_conf: use alternate domain definition in virDomainSnapshotDefAssignExternalNames
Commit introduced new
argument for virDomainSnapshotAlignDisks() that allows passing alternate
domain definition in case the snapshot parent.dom is NULL.
In case of redefining snapshot it will not hit the part of code that
unconditionally uses parent.dom as there will not be need to generate
default external file names.
It should be still fixed to make it safe. Future external snapshot
revert code will use this to generate default file names and in this
case it would crash.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/conf/snapshot_conf.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index cc59bddbc8..ac5aba1753 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -486,12 +486,14 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDef
*def,
/**
* virDomainSnapshotDefAssignExternalNames:
* @def: snapshot def object
+ * @domdef: domain def object
*
* Generate default external file names for snapshot targets. Returns 0 on
* success, -1 on error.
*/
static int
-virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDef *def)
+virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDef *def,
+virDomainDef *domdef)
{
const char *origpath;
char *tmppath;
@@ -514,7 +516,7 @@
virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDef *def)
return -1;
}
-if (!(origpath = virDomainDiskGetSource(def->parent.dom->disks[i]))) {
+if (!(origpath = virDomainDiskGetSource(domdef->disks[i]))) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("cannot generate external snapshot name for disk
'%1$s' without source"),
disk->name);
@@ -702,7 +704,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
}
/* Generate default external file names for external snapshot locations */
-if (virDomainSnapshotDefAssignExternalNames(snapdef) < 0)
+if (virDomainSnapshotDefAssignExternalNames(snapdef, domdef) < 0)
return -1;
return 0;
--
2.41.0
[libvirt PATCH v3 05/25] virDomainSnapshotAlignDisks: Allow overriding user-configured snapshot default
This new option will be used by external snapshot revert code.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/conf/snapshot_conf.c | 15 ---
src/conf/snapshot_conf.h | 3 ++-
src/qemu/qemu_snapshot.c | 2 +-
src/test/test_driver.c | 2 +-
4 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 8ecf311a17..67cec34920 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -581,6 +581,8 @@
virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDef *def,
* @default_snapshot: snapshot location to assign to disks which don't have any
* @uniform_internal_snapshot: Require that for an internal snapshot all disks
* take part in the internal snapshot
+ * @force_default_location: Always use @default_snapshot even if domain def
+ * has different default value
*
* Align snapdef->disks to domain definition, filling in any missing disks or
* snapshot state defaults given by the domain, with a fallback to
@@ -598,6 +600,10 @@
virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDef *def,
* in the internal snapshot. This is for hypervisors where granularity of an
* internal snapshot can't be controlled.
*
+ * When @force_default_location is true we will always use @default_snapshot
+ * even if domain definition has different default set. This is required to
+ * create new snapshot definition when reverting external snapshots.
+ *
* Convert paths to disk targets for uniformity.
*
* On error -1 is returned and a libvirt error is reported.
@@ -606,7 +612,8 @@ int
virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
virDomainDef *existingDomainDef,
virDomainSnapshotLocation default_snapshot,
-bool uniform_internal_snapshot)
+bool uniform_internal_snapshot,
+bool force_default_location)
{
virDomainDef *domdef = snapdef->parent.dom;
g_autoptr(GHashTable) map = virHashNew(NULL);
@@ -714,7 +721,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
/* Don't snapshot empty drives */
if (virStorageSourceIsEmpty(domdef->disks[i]->src))
snapdisk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NO;
-else
+else if (!force_default_location)
snapdisk->snapshot = domdef->disks[i]->snapshot;
snapdisk->src->type = VIR_STORAGE_TYPE_FILE;
@@ -970,8 +977,10 @@ virDomainSnapshotRedefinePrep(virDomainObj *vm,
virDomainSnapshotDefIsExternal(snapdef))
align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
-if (virDomainSnapshotAlignDisks(snapdef, otherDomDef, align_location,
true) < 0)
+if (virDomainSnapshotAlignDisks(snapdef, otherDomDef, align_location,
+true, false) < 0) {
return -1;
+}
return 0;
}
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index ab76af604a..14254d1c86 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -126,7 +126,8 @@ char *virDomainSnapshotDefFormat(const char *uuidstr,
int virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapshot,
virDomainDef *existingDomainDef,
virDomainSnapshotLocation default_snapshot,
-bool uniform_internal_snapshot);
+bool uniform_internal_snapshot,
+bool force_default_location);
bool virDomainSnapshotDefIsExternal(virDomainSnapshotDef *def);
bool virDomainSnapshotIsExternal(virDomainMomentObj *snap);
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 91de8b0c31..844b02d427 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1585,7 +1585,7 @@ qemuSnapshotCreateAlignDisks(virDomainObj *vm,
else
def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
}
-if (virDomainSnapshotAlignDisks(def, NULL, align_location, true) < 0)
+if (virDomainSnapshotAlignDisks(def, NULL, align_location, true, false) <
0)
return -1;
return 0;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 4b8e02c684..905be3853b 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -8724,7 +8724,7 @@ testDomainSnapshotAlignDisks(virDomainObj *vm,
VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
}
-return virDomainSnapshotAlignDisks(def, NULL, align_location, true);
+return virDomainSnapshotAlignDisks(def, NULL, align_location, true, false);
}
--
2.41.0
[libvirt PATCH v3 04/25] snapshot_conf: introduce metadata element
This new element will hold the new disk overlay created when reverting
to non-leaf snapshot in order to remember the files libvirt created.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/conf/schemas/domainsnapshot.rng | 7 +++
src/conf/snapshot_conf.c| 30 +
src/conf/snapshot_conf.h| 5 +
3 files changed, 42 insertions(+)
diff --git a/src/conf/schemas/domainsnapshot.rng
b/src/conf/schemas/domainsnapshot.rng
index 45f01b96cd..2549c47b22 100644
--- a/src/conf/schemas/domainsnapshot.rng
+++ b/src/conf/schemas/domainsnapshot.rng
@@ -60,6 +60,13 @@
+
+
+
+
+
+
+
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index ac5aba1753..8ecf311a17 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -112,6 +112,9 @@ virDomainSnapshotDefDispose(void *obj)
for (i = 0; i < def->ndisks; i++)
virDomainSnapshotDiskDefClear(>disks[i]);
g_free(def->disks);
+for (i = 0; i < def->nrevertdisks; i++)
+virDomainSnapshotDiskDefClear(>revertdisks[i]);
+g_free(def->revertdisks);
virObjectUnref(def->cookie);
}
@@ -376,6 +379,22 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
return NULL;
}
+if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) {
+g_autofree xmlNodePtr *revertDiskNodes = NULL;
+
+if ((n = virXPathNodeSet("./revertDisks/*", ctxt, )) <
0)
+return NULL;
+if (n)
+def->revertdisks = g_new0(virDomainSnapshotDiskDef, n);
+def->nrevertdisks = n;
+for (i = 0; i < def->nrevertdisks; i++) {
+if (virDomainSnapshotDiskDefParseXML(revertDiskNodes[i], ctxt,
+ >revertdisks[i],
+ flags, xmlopt) < 0)
+return NULL;
+}
+}
+
if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL) {
int active;
@@ -834,6 +853,17 @@ virDomainSnapshotDefFormatInternal(virBuffer *buf,
virBufferAddLit(buf, "\n");
}
+if (def->nrevertdisks > 0) {
+g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
+
+for (i = 0; i < def->nrevertdisks; i++) {
+if (virDomainSnapshotDiskDefFormat(,
>revertdisks[i], xmlopt) < 0)
+return -1;
+}
+
+virXMLFormatElement(buf, "revertDisks", NULL, );
+}
+
if (def->parent.dom) {
if (virDomainDefFormatInternal(def->parent.dom, xmlopt,
buf, domainflags) < 0)
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index ad49990a1e..ab76af604a 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -80,6 +80,11 @@ struct _virDomainSnapshotDef {
size_t ndisks; /* should not exceed dom->ndisks */
virDomainSnapshotDiskDef *disks;
+/* When we revert to non-leaf snapshot we need to
+ * store the new overlay disks. */
+size_t nrevertdisks;
+virDomainSnapshotDiskDef *revertdisks;
+
virObject *cookie;
};
--
2.41.0
[libvirt PATCH v3 24/25] qemu_snapshot: add checks for external snapshot deletion
With the introduction of external snapshot revert support we need to
error out in some cases when trying to delete some snapshots.
If users reverts to non-leaf snapshots and would try to delete it after
the revert is done it would not work currently as this operation would
require using block-stream which is not implemented for now as in this
case the snapshot has two children so the disk files have multiple
overlays.
Similarly if user reverts to non-leaf snapshot and would try to delete
snapshot that is non-leaf but not in currently active snapshot chain we
would still need to use block-commit operation. The issue here is that
in order to do that we would have to start new qemu process with
different domain definition than what is currently used by the domain.
If the current domain would be running it would complicate things even
more so this operation is not yet supported.
If user creates new snapshot after reverting to non-leaf snapshot it
creates a new branch. Deleting snapshot with multiple children will
require block-stream which is not implemented for now.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index dca95b415b..692d88e24d 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -3729,6 +3729,8 @@ qemuSnapshotDeleteValidate(virDomainObj *vm,
}
if (virDomainSnapshotIsExternal(snap)) {
+virDomainMomentObj *current =
virDomainSnapshotGetCurrent(vm->snapshots);
+
if (qemuDomainHasBlockjob(vm, false)) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot delete external snapshots when there is
another active block job"));
@@ -3740,6 +3742,25 @@ qemuSnapshotDeleteValidate(virDomainObj *vm,
_("deletion of external disk snapshots with
children not supported"));
return -1;
}
+
+if (snap == current && snap->nchildren != 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("deletion of active external snapshot that is not
a leaf snapshot is not supported"));
+return -1;
+}
+
+if (snap != current && snap->nchildren != 0 &&
+virDomainMomentIsAncestor(snap, current)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("deletion of non-leaf external snapshot that is
not in active chain is not supported"));
+return -1;
+}
+
+if (snap->nchildren > 1) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("deletion of external disk snapshot with multiple
children snapshots not supported"));
+return -1;
+}
}
return 0;
--
2.41.0
[libvirt PATCH v3 18/25] qemu_snapshot: delete: properly update parent snapshot with revert data
When deleting external snapshot and parent snapshot is the currently
active snapshot as user reverted to it we need to properly update the
parent snapshot metadata.
After the delete is done the new overlay files will be the currently
used files created when snapshot revert was done, replacing the original
overlay files.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 40
1 file changed, 40 insertions(+)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index a67be1ae49..6dd420a53b 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -3229,6 +3229,41 @@ qemuSnapshotDiscardExternal(virDomainObj *vm,
}
+static int
+qemuSnapshotDeleteUpdateParent(virDomainObj *vm,
+ virDomainMomentObj *parent)
+{
+size_t i;
+virQEMUDriver *driver = QEMU_DOMAIN_PRIVATE(vm)->driver;
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+virDomainSnapshotDef *parentDef = virDomainSnapshotObjGetDef(parent);
+
+if (!parentDef)
+return 0;
+
+if (!parentDef->revertdisks)
+return 0;
+
+for (i = 0; i < parentDef->ndisks; i++) {
+virDomainSnapshotDiskDefClear(>disks[i]);
+}
+g_free(parentDef->disks);
+
+parentDef->disks = g_steal_pointer(>revertdisks);
+parentDef->ndisks = parentDef->nrevertdisks;
+parentDef->nrevertdisks = 0;
+
+if (qemuDomainSnapshotWriteMetadata(vm,
+parent,
+driver->xmlopt,
+cfg->snapshotDir) < 0) {
+return -1;
+}
+
+return 0;
+}
+
+
static int
qemuSnapshotDiscardMetadata(virDomainObj *vm,
virDomainMomentObj *snap,
@@ -3268,6 +3303,11 @@ qemuSnapshotDiscardMetadata(virDomainObj *vm,
virDomainMomentMoveChildren(snap, snap->parent);
}
+if (update_parent && snap->parent) {
+if (qemuSnapshotDeleteUpdateParent(vm, snap->parent) < 0)
+ret = -1;
+}
+
snapFile = g_strdup_printf("%s/%s/%s.xml", cfg->snapshotDir, vm->def->name,
snap->def->name);
--
2.41.0
[libvirt PATCH v3 15/25] qemu_snapshot: add merge to external snapshot delete prepare data
Before external snapshot revert every delete operation did block commit
in order to delete a snapshot. But now when user reverts to non-leaf
snapshot deleting leaf snapshot will not have any overlay files so we
can just simply delete the snapshot images.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 84 +---
1 file changed, 52 insertions(+), 32 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index c3d17b4cee..d47dc72709 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2622,9 +2622,26 @@ qemuSnapshotFindParentSnapForDisk(virDomainMomentObj
*snap,
}
+/**
+ * qemuSnapshotDeleteExternalPrepareData:
+ * @vm: domain object
+ * @snap: snapshot object
+ * @merge: whether we are just deleting image or not
+ * @externalData: prepared data to delete external snapshot
+ *
+ * Validate if we can delete selected snapshot @snap and prepare all necessary
+ * data that will be used when deleting snapshot as @externalData.
+ *
+ * If @merge is set to true we will merge the deleted snapshot into parent one
+ * instead of just deleting it. This is necessary when operating on snapshot
+ * that has existing overlay files.
+ *
+ * Returns -1 on error, 0 on success.
+ */
static int
qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
virDomainMomentObj *snap,
+ bool merge,
GSList **externalData)
{
ssize_t i;
@@ -2648,10 +2665,6 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
data = g_new0(qemuSnapshotDeleteExternalData, 1);
data->snapDisk = snapDisk;
-data->domDisk = qemuDomainDiskByName(vm->def, snapDisk->name);
-if (!data->domDisk)
-return -1;
-
data->parentDomDisk = virDomainDiskByTarget(snapdef->parent.dom,
data->snapDisk->name);
if (!data->parentDomDisk) {
@@ -2661,39 +2674,46 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
return -1;
}
-if (virDomainObjIsActive(vm)) {
-data->diskSrc =
virStorageSourceChainLookupBySource(data->domDisk->src,
-
data->snapDisk->src,
-
>prevDiskSrc);
-if (!data->diskSrc)
+if (merge) {
+data->domDisk = qemuDomainDiskByName(vm->def, snapDisk->name);
+if (!data->domDisk)
return -1;
-if (!virStorageSourceIsSameLocation(data->diskSrc,
data->snapDisk->src)) {
-virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("VM disk source and snapshot disk source are
not the same"));
-return -1;
-}
+if (virDomainObjIsActive(vm)) {
+data->diskSrc =
virStorageSourceChainLookupBySource(data->domDisk->src,
+
data->snapDisk->src,
+
>prevDiskSrc);
+if (!data->diskSrc)
+return -1;
-data->parentDiskSrc = data->diskSrc->backingStore;
-if (!virStorageSourceIsBacking(data->parentDiskSrc)) {
-virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("failed to find parent disk source in backing
chain"));
-return -1;
-}
+if (!virStorageSourceIsSameLocation(data->diskSrc,
data->snapDisk->src)) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("VM disk source and snapshot disk source
are not the same"));
+return -1;
+}
-if (!virStorageSourceIsSameLocation(data->parentDiskSrc,
data->parentDomDisk->src)) {
-virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("snapshot VM disk source and parent disk
source are not the same"));
-return -1;
+data->parentDiskSrc = data->diskSrc->backingStore;
+if (!virStorageSourceIsBacking(data->parentDiskSrc)) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("failed to find parent disk source in
backing chain"));
+return -1;
+}
+
+if (!virStorageSourceIsSameLocation(data->parentDiskSrc,
+
[libvirt PATCH v3 14/25] qemu_snapshot: extract external snapshot delete prepare to function
This part of code is about to grow to make deletion work when user
reverts to non-leaf snapshot.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 104 ++-
1 file changed, 70 insertions(+), 34 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index dd88e478b8..c3d17b4cee 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2706,6 +2706,73 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
}
+/**
+ * qemuSnapshotDeleteExternalPrepare:
+ * @vm: domain object
+ * @snap: snapshot object we are deleting
+ * @flags: flags passed to virDomainSnapshotDelete
+ * @externalData: pointer to GSList of qemuSnapshotDeleteExternalData
+ * @stop_qemu: pointer to boolean indicating QEMU process was started
+ *
+ * Validates and prepares data for snapshot @snap we are deleting and
+ * store it in @externalData. For offline VMs we need to start QEMU
+ * process in order to delete external snapshots and caller will need
+ * to stop that process, @stop_qemu will be set to True.
+ *
+ * Return 0 on success, -1 on error.
+ */
+static int
+qemuSnapshotDeleteExternalPrepare(virDomainObj *vm,
+ virDomainMomentObj *snap,
+ unsigned int flags,
+ GSList **externalData,
+ bool *stop_qemu)
+{
+virQEMUDriver *driver = ((qemuDomainObjPrivate *) vm->privateData)->driver;
+g_autoslist(qemuSnapshotDeleteExternalData) tmpData = NULL;
+
+if (!virDomainSnapshotIsExternal(snap))
+return 0;
+
+if (flags & (VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN |
+ VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY)) {
+return 0;
+}
+
+/* this also serves as validation whether the snapshot can be deleted */
+if (qemuSnapshotDeleteExternalPrepareData(vm, snap, ) < 0)
+return -1;
+
+if (!virDomainObjIsActive(vm)) {
+if (qemuProcessStart(NULL, driver, vm, NULL, VIR_ASYNC_JOB_SNAPSHOT,
+ NULL, -1, NULL, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
+ VIR_QEMU_PROCESS_START_PAUSED) < 0) {
+return -1;
+}
+
+*stop_qemu = true;
+
+/* Call the prepare again as some data require that the VM is
+ * running to get everything we need. */
+if (qemuSnapshotDeleteExternalPrepareData(vm, snap, externalData) < 0)
+return -1;
+} else {
+qemuDomainJobPrivate *jobPriv = vm->job->privateData;
+
+*externalData = g_steal_pointer();
+
+/* If the VM is running we need to indicate that the async snapshot
+ * job is snapshot delete job. */
+jobPriv->snapshotDelete = true;
+
+qemuDomainSaveStatus(vm);
+}
+
+return 0;
+}
+
+
typedef struct _virQEMUMomentReparent virQEMUMomentReparent;
struct _virQEMUMomentReparent {
const char *dir;
@@ -3476,40 +3543,9 @@ qemuSnapshotDelete(virDomainObj *vm,
if (qemuSnapshotDeleteValidate(vm, snap, flags) < 0)
goto endjob;
-if (virDomainSnapshotIsExternal(snap) &&
-!(flags & (VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN |
- VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY))) {
-g_autoslist(qemuSnapshotDeleteExternalData) tmpData = NULL;
-
-/* this also serves as validation whether the snapshot can be
deleted */
-if (qemuSnapshotDeleteExternalPrepareData(vm, snap, ) < 0)
-goto endjob;
-
-if (!virDomainObjIsActive(vm)) {
-if (qemuProcessStart(NULL, driver, vm, NULL,
VIR_ASYNC_JOB_SNAPSHOT,
- NULL, -1, NULL, NULL,
- VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
- VIR_QEMU_PROCESS_START_PAUSED) < 0) {
-goto endjob;
-}
-
-stop_qemu = true;
-
-/* Call the prepare again as some data require that the VM is
- * running to get everything we need. */
-if (qemuSnapshotDeleteExternalPrepareData(vm, snap,
) < 0)
-goto endjob;
-} else {
-qemuDomainJobPrivate *jobPriv = vm->job->privateData;
-
-externalData = g_steal_pointer();
-
-/* If the VM is running we need to indicate that the async
snapshot
- * job is snapshot delete job. */
-jobPriv->snapshotDelete = true;
-
-qemuDomainSaveStatus(vm);
-}
+if (qemuSnapshotDeleteExternalPrepare(vm, snap, flags,
+ , _qemu) < 0) {
+goto endjob;
}
}
--
2.41.0
[libvirt PATCH v3 07/25] qemu_snapshot: use virDomainDiskByName while updating domain def
When creating external snapshot this function is called only when the VM
is not running so there is only one definition to care about. However,
it will be used by external snapshot revert code for active and inactive
definition and they may be different if a disk was (un)plugged only for
the active or inactive definition.
The current code would crash so use virDomainDiskByName() to get the
correct disk from the domain definition based on the disk name and make
sure it exists.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 72a0f71d4f..8e1eb21b5d 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -152,11 +152,14 @@ qemuSnapshotDomainDefUpdateDisk(virDomainDef *domdef,
for (i = 0; i < snapdef->ndisks; i++) {
g_autoptr(virStorageSource) newsrc = NULL;
virDomainSnapshotDiskDef *snapdisk = &(snapdef->disks[i]);
-virDomainDiskDef *defdisk = domdef->disks[i];
+virDomainDiskDef *defdisk = virDomainDiskByName(domdef,
snapdisk->name, false);
if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
continue;
+if (!defdisk)
+continue;
+
if (!(newsrc = virStorageSourceCopy(snapdisk->src, false)))
return -1;
--
2.41.0
Re: [libvirt PATCH v2 11/24] qemu_snapshot: use VIR_ASYNC_JOB_SNAPSHOT when reverting snapshot
On Tue, Jul 25, 2023 at 03:54:54PM +0200, Peter Krempa wrote:
> On Tue, Jun 27, 2023 at 17:07:14 +0200, Pavel Hrdina wrote:
>
> So what is this actually doing?
>
>
> > Signed-off-by: Pavel Hrdina
> > ---
> > src/qemu/qemu_snapshot.c | 19 ++-
> > 1 file changed, 10 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> > index 5d2ffdeee6..1cb0ea55de 100644
> > --- a/src/qemu/qemu_snapshot.c
> > +++ b/src/qemu/qemu_snapshot.c
> > @@ -2001,7 +2001,7 @@ qemuSnapshotRevertActive(virDomainObj *vm,
> > /* Transitions 5, 6, 8, 9 */
> > qemuProcessStop(driver, vm,
> > VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT,
> > -VIR_ASYNC_JOB_START, 0);
> > +VIR_ASYNC_JOB_SNAPSHOT, 0);
>
> It looks like we are declaring that an async job is used here ...
>
> > virDomainAuditStop(vm, "from-snapshot");
> > detail = VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT;
> > event = virDomainEventLifecycleNewFromObj(vm,
> > @@ -2026,7 +2026,7 @@ qemuSnapshotRevertActive(virDomainObj *vm,
> >
> > rc = qemuProcessStart(snapshot->domain->conn, driver, vm,
> >cookie ? cookie->cpu : NULL,
> > - VIR_ASYNC_JOB_START, NULL, -1, NULL, snap,
> > + VIR_ASYNC_JOB_SNAPSHOT, NULL, -1, NULL, snap,
> >VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
> >start_flags);
> > virDomainAuditStart(vm, "from-snapshot", rc >= 0);
> > @@ -2059,7 +2059,7 @@ qemuSnapshotRevertActive(virDomainObj *vm,
> > }
> > rc = qemuProcessStartCPUs(driver, vm,
> >VIR_DOMAIN_RUNNING_FROM_SNAPSHOT,
> > - VIR_ASYNC_JOB_START);
> > + VIR_ASYNC_JOB_SNAPSHOT);
> > if (rc < 0)
> > return -1;
> > }
> > @@ -2122,7 +2122,7 @@ qemuSnapshotRevertInactive(virDomainObj *vm,
> > if (virDomainObjIsActive(vm)) {
> > /* Transitions 4, 7 */
> > qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT,
> > -VIR_ASYNC_JOB_START, 0);
> > +VIR_ASYNC_JOB_SNAPSHOT, 0);
> > virDomainAuditStop(vm, "from-snapshot");
> > detail = VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT;
> > event = virDomainEventLifecycleNewFromObj(vm,
> > @@ -2149,7 +2149,7 @@ qemuSnapshotRevertInactive(virDomainObj *vm,
> > start_flags |= paused ? VIR_QEMU_PROCESS_START_PAUSED : 0;
> >
> > rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL,
> > - VIR_ASYNC_JOB_START, NULL, -1, NULL, NULL,
> > + VIR_ASYNC_JOB_SNAPSHOT, NULL, -1, NULL, NULL,
> >VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
> >start_flags);
> > virDomainAuditStart(vm, "from-snapshot", rc >= 0);
> > @@ -2216,10 +2216,11 @@ qemuSnapshotRevert(virDomainObj *vm,
> > return -1;
> > }
> >
> > -if (qemuProcessBeginJob(vm,
> > -VIR_DOMAIN_JOB_OPERATION_SNAPSHOT_REVERT,
> > -flags) < 0)
>
> ... but it was never initiated as async.
>
> It is unclear if you are fixing a bug, or doing a change for consistency
> or what is happening here actually.
qemuProcessBeginJob is just a wrapper for:
if (virDomainObjBeginAsyncJob(vm, VIR_ASYNC_JOB_START,
operation, apiFlags) < 0)
return -1;
qemuDomainObjSetAsyncJobMask(vm, VIR_JOB_NONE);
return 0;
so we actually did start the job as async.
Yeah missing details in commit message bites me now as I don't remember
if this was only cosmetic change as CREATE and DELETE both use
VIR_ASYNC_JOB_SNAPSHOT instead of VIR_ASYNC_JOB_START or if there was
any other reason for this patch.
The only change this introduces is that now when revert job is running
query jobs are allowed as previously no other jobs were allowed.
Pavel
signature.asc
Description: PGP signature
[libvirt PATCH v2 11/24] qemu_snapshot: use VIR_ASYNC_JOB_SNAPSHOT when reverting snapshot
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 5d2ffdeee6..1cb0ea55de 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2001,7 +2001,7 @@ qemuSnapshotRevertActive(virDomainObj *vm,
/* Transitions 5, 6, 8, 9 */
qemuProcessStop(driver, vm,
VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT,
-VIR_ASYNC_JOB_START, 0);
+VIR_ASYNC_JOB_SNAPSHOT, 0);
virDomainAuditStop(vm, "from-snapshot");
detail = VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT;
event = virDomainEventLifecycleNewFromObj(vm,
@@ -2026,7 +2026,7 @@ qemuSnapshotRevertActive(virDomainObj *vm,
rc = qemuProcessStart(snapshot->domain->conn, driver, vm,
cookie ? cookie->cpu : NULL,
- VIR_ASYNC_JOB_START, NULL, -1, NULL, snap,
+ VIR_ASYNC_JOB_SNAPSHOT, NULL, -1, NULL, snap,
VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
start_flags);
virDomainAuditStart(vm, "from-snapshot", rc >= 0);
@@ -2059,7 +2059,7 @@ qemuSnapshotRevertActive(virDomainObj *vm,
}
rc = qemuProcessStartCPUs(driver, vm,
VIR_DOMAIN_RUNNING_FROM_SNAPSHOT,
- VIR_ASYNC_JOB_START);
+ VIR_ASYNC_JOB_SNAPSHOT);
if (rc < 0)
return -1;
}
@@ -2122,7 +2122,7 @@ qemuSnapshotRevertInactive(virDomainObj *vm,
if (virDomainObjIsActive(vm)) {
/* Transitions 4, 7 */
qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT,
-VIR_ASYNC_JOB_START, 0);
+VIR_ASYNC_JOB_SNAPSHOT, 0);
virDomainAuditStop(vm, "from-snapshot");
detail = VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT;
event = virDomainEventLifecycleNewFromObj(vm,
@@ -2149,7 +2149,7 @@ qemuSnapshotRevertInactive(virDomainObj *vm,
start_flags |= paused ? VIR_QEMU_PROCESS_START_PAUSED : 0;
rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL,
- VIR_ASYNC_JOB_START, NULL, -1, NULL, NULL,
+ VIR_ASYNC_JOB_SNAPSHOT, NULL, -1, NULL, NULL,
VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
start_flags);
virDomainAuditStart(vm, "from-snapshot", rc >= 0);
@@ -2216,10 +2216,11 @@ qemuSnapshotRevert(virDomainObj *vm,
return -1;
}
-if (qemuProcessBeginJob(vm,
-VIR_DOMAIN_JOB_OPERATION_SNAPSHOT_REVERT,
-flags) < 0)
+if (virDomainObjBeginAsyncJob(vm, VIR_ASYNC_JOB_SNAPSHOT,
+ VIR_DOMAIN_JOB_OPERATION_SNAPSHOT_REVERT,
+ flags) < 0) {
return -1;
+}
if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))
goto endjob;
@@ -2269,7 +2270,7 @@ qemuSnapshotRevert(virDomainObj *vm,
}
endjob:
-qemuProcessEndJob(vm);
+virDomainObjEndAsyncJob(vm);
return ret;
}
--
2.41.0
[libvirt PATCH v2 02/24] snapshot_conf: export virDomainSnapshotDiskDefClear
We will need to call this function from qemu_snapshot when introducing
external snapshot revert support.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/conf/snapshot_conf.c | 2 +-
src/conf/snapshot_conf.h | 3 +++
src/libvirt_private.syms | 1 +
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 0acba95d7f..cc59bddbc8 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -74,7 +74,7 @@ VIR_ENUM_IMPL(virDomainSnapshotState,
);
/* Snapshot Def functions */
-static void
+void
virDomainSnapshotDiskDefClear(virDomainSnapshotDiskDef *disk)
{
VIR_FREE(disk->name);
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 96c77ef42b..ad49990a1e 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -59,6 +59,9 @@ struct _virDomainSnapshotDiskDef {
virStorageSource *src;
};
+void
+virDomainSnapshotDiskDefClear(virDomainSnapshotDiskDef *disk);
+
void
virDomainSnapshotDiskDefFree(virDomainSnapshotDiskDef *disk);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index eed4c17d49..2b1d4e4512 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1037,6 +1037,7 @@ virDomainSnapshotDefFormat;
virDomainSnapshotDefIsExternal;
virDomainSnapshotDefNew;
virDomainSnapshotDefParseString;
+virDomainSnapshotDiskDefClear;
virDomainSnapshotDiskDefFree;
virDomainSnapshotDiskDefParseXML;
virDomainSnapshotFormatConvertXMLFlags;
--
2.41.0
[libvirt PATCH v2 20/24] virdomainmomentobjlist: introduce virDomainMomentIsAncestor
This new helper will allow us to check if we are able to delete external
snapshot after user did revert to non-leaf snapshot.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/conf/virdomainmomentobjlist.c | 17 +
src/conf/virdomainmomentobjlist.h | 4
src/libvirt_private.syms | 1 +
3 files changed, 22 insertions(+)
diff --git a/src/conf/virdomainmomentobjlist.c
b/src/conf/virdomainmomentobjlist.c
index e5cdd9a141..ea9850df8c 100644
--- a/src/conf/virdomainmomentobjlist.c
+++ b/src/conf/virdomainmomentobjlist.c
@@ -582,3 +582,20 @@ virDomainMomentFindLeaf(virDomainMomentObjList *list)
return moment;
return NULL;
}
+
+
+/* Check if @moment is descendant of @ancestor. */
+bool
+virDomainMomentIsAncestor(virDomainMomentObj *moment,
+ virDomainMomentObj *ancestor)
+{
+if (moment == ancestor)
+return false;
+
+for (moment = moment->parent; moment; moment = moment->parent) {
+if (moment == ancestor)
+return true;
+}
+
+return false;
+}
diff --git a/src/conf/virdomainmomentobjlist.h
b/src/conf/virdomainmomentobjlist.h
index d2ab3b46b1..2ea6b181c0 100644
--- a/src/conf/virdomainmomentobjlist.h
+++ b/src/conf/virdomainmomentobjlist.h
@@ -157,3 +157,7 @@ virDomainMomentCheckCycles(virDomainMomentObjList *list,
virDomainMomentObj *
virDomainMomentFindLeaf(virDomainMomentObjList *list);
+
+bool
+virDomainMomentIsAncestor(virDomainMomentObj *moment,
+ virDomainMomentObj *ancestor);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2b1d4e4512..2f4ab607e5 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1221,6 +1221,7 @@ virDomainMomentDropChildren;
virDomainMomentDropParent;
virDomainMomentForEachChild;
virDomainMomentForEachDescendant;
+virDomainMomentIsAncestor;
virDomainMomentMoveChildren;
virDomainMomentObjFree;
virDomainMomentObjNew;
--
2.41.0
[libvirt PATCH v2 14/24] qemu_snapshot: extract external snapshot delete prepare to function
This part of code is about to grow to make deletion work when user
reverts to non-leaf snapshot.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 89 +---
1 file changed, 55 insertions(+), 34 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index c5de24917b..08cff2a9a2 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2653,6 +2653,58 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
}
+static int
+qemuSnapshotDeleteExternalPrepare(virDomainObj *vm,
+ virDomainMomentObj *snap,
+ unsigned int flags,
+ GSList **externalData,
+ bool *stop_qemu)
+{
+virQEMUDriver *driver = ((qemuDomainObjPrivate *) vm->privateData)->driver;
+g_autoslist(qemuSnapshotDeleteExternalData) tmpData = NULL;
+
+if (!virDomainSnapshotIsExternal(snap))
+return 0;
+
+if (flags & (VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN |
+ VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY)) {
+return 0;
+}
+
+/* this also serves as validation whether the snapshot can be deleted */
+if (qemuSnapshotDeleteExternalPrepareData(vm, snap, ) < 0)
+return -1;
+
+if (!virDomainObjIsActive(vm)) {
+if (qemuProcessStart(NULL, driver, vm, NULL, VIR_ASYNC_JOB_SNAPSHOT,
+ NULL, -1, NULL, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
+ VIR_QEMU_PROCESS_START_PAUSED) < 0) {
+return -1;
+}
+
+*stop_qemu = true;
+
+/* Call the prepare again as some data require that the VM is
+ * running to get everything we need. */
+if (qemuSnapshotDeleteExternalPrepareData(vm, snap, externalData) < 0)
+return -1;
+} else {
+qemuDomainJobPrivate *jobPriv = vm->job->privateData;
+
+*externalData = g_steal_pointer();
+
+/* If the VM is running we need to indicate that the async snapshot
+ * job is snapshot delete job. */
+jobPriv->snapshotDelete = true;
+
+qemuDomainSaveStatus(vm);
+}
+
+return 0;
+}
+
+
typedef struct _virQEMUMomentReparent virQEMUMomentReparent;
struct _virQEMUMomentReparent {
const char *dir;
@@ -3423,40 +3475,9 @@ qemuSnapshotDelete(virDomainObj *vm,
if (qemuSnapshotDeleteValidate(vm, snap, flags) < 0)
goto endjob;
-if (virDomainSnapshotIsExternal(snap) &&
-!(flags & (VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN |
- VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY))) {
-g_autoslist(qemuSnapshotDeleteExternalData) tmpData = NULL;
-
-/* this also serves as validation whether the snapshot can be
deleted */
-if (qemuSnapshotDeleteExternalPrepareData(vm, snap, ) < 0)
-goto endjob;
-
-if (!virDomainObjIsActive(vm)) {
-if (qemuProcessStart(NULL, driver, vm, NULL,
VIR_ASYNC_JOB_SNAPSHOT,
- NULL, -1, NULL, NULL,
- VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
- VIR_QEMU_PROCESS_START_PAUSED) < 0) {
-goto endjob;
-}
-
-stop_qemu = true;
-
-/* Call the prepare again as some data require that the VM is
- * running to get everything we need. */
-if (qemuSnapshotDeleteExternalPrepareData(vm, snap,
) < 0)
-goto endjob;
-} else {
-qemuDomainJobPrivate *jobPriv = vm->job->privateData;
-
-externalData = g_steal_pointer();
-
-/* If the VM is running we need to indicate that the async
snapshot
- * job is snapshot delete job. */
-jobPriv->snapshotDelete = true;
-
-qemuDomainSaveStatus(vm);
-}
+if (qemuSnapshotDeleteExternalPrepare(vm, snap, flags,
+ , _qemu) < 0) {
+goto endjob;
}
}
--
2.41.0
[libvirt PATCH v2 00/24] introduce external snapshot revert support
This implements virDomainRevertToSnapshot to work with external snapshots. In addition it modifies virDomainSnapshotDelete to work correctly when we revert to non-leaf snapshot or when there is non-linear snapshot tree with multiple branches. Gitlab repo with the patches: https://gitlab.com/phrdina/libvirt/-/tree/snapshot-revert-external Pavel Hrdina (24): libvirt_private: list virDomainMomentDefPostParse snapshot_conf: export virDomainSnapshotDiskDefClear snapshot_conf: use alternate domain definition in virDomainSnapshotDefAssignExternalNames snapshot_conf: introduce metadata element snapshot_conf: add new argument to virDomainSnapshotAlignDisks qemu_snapshot: introduce qemuSnapshotDomainDefUpdateDisk qemu_snapshot: use virDomainDiskByName while updating domain def qemu_snapshot: introduce qemuSnapshotCreateQcow2Files qemu_snapshot: allow using alternate domain definition when creating qcow2 files qemu_snapshot: move external disk prepare to single function qemu_snapshot: use VIR_ASYNC_JOB_SNAPSHOT when reverting snapshot qemu_snapshot: introduce external snapshot revert support qemu_snapshot: rename qemuSnapshotDeleteExternalPrepare qemu_snapshot: extract external snapshot delete prepare to function qemu_snapshot: add merge to external snapshot delete prepare data qemu_snapshot: prepare data for non-active leaf external snapshot deletion qemu_snapshot: add support to delete external snapshot without block commit qemu_snapshot: delete: properly update parent snapshot with revert data qemu_snapshot: remove revertdisks when creating new snapshot virdomainmomentobjlist: introduce virDomainMomentIsAncestor qemu_snapshot: update backing store after deleting external snapshot qemu_snapshot: check only once if snapshot is external qemu_snapshot: add checks for external snapshot deletion qemu_snapshot: allow snapshot revert for external snapshots src/conf/schemas/domainsnapshot.rng | 7 + src/conf/snapshot_conf.c| 52 +- src/conf/snapshot_conf.h| 11 +- src/conf/virdomainmomentobjlist.c | 17 + src/conf/virdomainmomentobjlist.h | 4 + src/libvirt_private.syms| 6 + src/qemu/qemu_snapshot.c| 874 ++-- src/test/test_driver.c | 2 +- 8 files changed, 780 insertions(+), 193 deletions(-) -- 2.41.0
[libvirt PATCH v2 07/24] qemu_snapshot: use virDomainDiskByName while updating domain def
When creating external snapshot this function is called only when the VM
is not running so there is only one definition to care about. However,
it will be used by external snapshot revert code for active and inactive
definition and they may be different if a disk was (un)plugged only for
the active or inactive definition.
The current code would crash so use virDomainDiskByName() to get the
correct disk from the domain definition based on the disk name and make
sure it exists.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 72a0f71d4f..8e1eb21b5d 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -152,11 +152,14 @@ qemuSnapshotDomainDefUpdateDisk(virDomainDef *domdef,
for (i = 0; i < snapdef->ndisks; i++) {
g_autoptr(virStorageSource) newsrc = NULL;
virDomainSnapshotDiskDef *snapdisk = &(snapdef->disks[i]);
-virDomainDiskDef *defdisk = domdef->disks[i];
+virDomainDiskDef *defdisk = virDomainDiskByName(domdef,
snapdisk->name, false);
if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
continue;
+if (!defdisk)
+continue;
+
if (!(newsrc = virStorageSourceCopy(snapdisk->src, false)))
return -1;
--
2.41.0
[libvirt PATCH v2 21/24] qemu_snapshot: update backing store after deleting external snapshot
With introduction of external snapshot revert we will have to update
backing store of qcow images not actively used be QEMU manually.
The need for this patch comes from the fact that we stop and start QEMU
process therefore after revert not all existing snapshots will be known
to that QEMU process due to reverting to non-leaf snapshot or having
multiple branches.
We need to loop over all existing snapshots and check all disks to see
if they happen to have the image we are deleting as backing store and
update them to point to the new image except for images currently used
by the running QEMU process doing the merge operation.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 95
1 file changed, 95 insertions(+)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 2950ad7d77..337c83f151 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2530,6 +2530,8 @@ typedef struct _qemuSnapshotDeleteExternalData {
virStorageSource *parentDiskSrc; /* backing disk source of the @diskSrc */
virStorageSource *prevDiskSrc; /* source of disk for which @diskSrc is
backing disk */
+GSList *disksWithBacking; /* list of storage source for which the
+ deleted storage source is backing store */
qemuBlockJobData *job;
bool merge;
} qemuSnapshotDeleteExternalData;
@@ -2542,6 +2544,7 @@
qemuSnapshotDeleteExternalDataFree(qemuSnapshotDeleteExternalData *data)
return;
virObjectUnref(data->job);
+g_slist_free(data->disksWithBacking);
g_free(data);
}
@@ -2591,6 +2594,60 @@ qemuSnapshotFindParentSnapForDisk(virDomainMomentObj
*snap,
}
+struct _qemuSnapshotDisksWithBackingStoreData {
+virDomainMomentObj *current;
+virStorageSource *diskSrc;
+GSList **disksWithBacking;
+};
+
+
+static int
+qemuSnapshotDiskHasBackingDisk(void *payload,
+ const char *name G_GNUC_UNUSED,
+ void *opaque)
+{
+virDomainMomentObj *snap = payload;
+virDomainSnapshotDef *snapdef = virDomainSnapshotObjGetDef(snap);
+struct _qemuSnapshotDisksWithBackingStoreData *data = opaque;
+ssize_t i;
+
+/* skip snapshots that are within the active snapshot tree as it will be
handled
+ * by qemu */
+if (virDomainMomentIsAncestor(data->current, snap) || data->current ==
snap)
+return 0;
+
+for (i = 0; i < snapdef->parent.dom->ndisks; i++) {
+virDomainDiskDef *disk = snapdef->parent.dom->disks[i];
+
+if (!virStorageSourceIsLocalStorage(disk->src))
+continue;
+
+if (!disk->src->backingStore)
+ignore_value(virStorageSourceGetMetadata(disk->src, -1, -1, 1,
false));
+
+if (virStorageSourceIsSameLocation(disk->src->backingStore,
data->diskSrc))
+*data->disksWithBacking = g_slist_prepend(*data->disksWithBacking,
disk->src);
+}
+
+return 0;
+}
+
+
+static void
+qemuSnapshotGetDisksWithBackingStore(virDomainObj *vm,
+ virDomainMomentObj *snap,
+ qemuSnapshotDeleteExternalData *data)
+{
+struct _qemuSnapshotDisksWithBackingStoreData iterData;
+
+iterData.current = virDomainSnapshotGetCurrent(vm->snapshots);
+iterData.diskSrc = data->diskSrc;
+iterData.disksWithBacking = >disksWithBacking;
+
+virDomainMomentForEachDescendant(snap, qemuSnapshotDiskHasBackingDisk,
);
+}
+
+
/**
* qemuSnapshotDeleteExternalPrepareData:
* @vm: domain object
@@ -2682,6 +2739,8 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
_("snapshot VM disk source and parent disk
source are not the same"));
return -1;
}
+
+qemuSnapshotGetDisksWithBackingStore(vm, snap, data);
}
data->parentSnap = qemuSnapshotFindParentSnapForDisk(snap,
data->snapDisk);
@@ -3063,6 +3122,40 @@ qemuSnapshotSetInvalid(virDomainObj *vm,
}
+static void
+qemuSnapshotUpdateBackingStore(virDomainObj *vm,
+ qemuSnapshotDeleteExternalData *data)
+{
+GSList *cur = NULL;
+const char *qemuImgPath;
+virQEMUDriver *driver = QEMU_DOMAIN_PRIVATE(vm)->driver;
+
+if (!(qemuImgPath = qemuFindQemuImgBinary(driver)))
+return;
+
+for (cur = data->disksWithBacking; cur; cur = g_slist_next(cur)) {
+virStorageSource *diskSrc = cur->data;
+g_autoptr(virCommand) cmd = NULL;
+
+/* creates cmd line args: qemu-img create -f qcow2 -o */
+if (!(cmd = virCommandNewArgList(qemuImgPath,
+ "rebase",
+ "-u",
+
[libvirt PATCH v2 19/24] qemu_snapshot: remove revertdisks when creating new snapshot
When user creates a new snapshot after reverting to non-leaf snapshot we
no longer need to store the temporary overlays as they will be part of
the VM XMLs stored in the newly created snapshot.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index a206f015c4..2950ad7d77 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1652,6 +1652,23 @@ qemuSnapshotCreateWriteMetadata(virDomainObj *vm,
}
+static void
+qemuSnapshotClearRevertdisks(virDomainMomentObj *current)
+{
+virDomainSnapshotDef *curdef = NULL;
+
+if (!current)
+return;
+
+curdef = virDomainSnapshotObjGetDef(current);
+
+if (curdef->revertdisks) {
+g_clear_pointer(>revertdisks, g_free);
+curdef->nrevertdisks = 0;
+}
+}
+
+
static virDomainSnapshotPtr
qemuSnapshotRedefine(virDomainObj *vm,
virDomainPtr domain,
@@ -1661,6 +1678,7 @@ qemuSnapshotRedefine(virDomainObj *vm,
unsigned int flags)
{
virDomainMomentObj *snap = NULL;
+virDomainMomentObj *current = virDomainSnapshotGetCurrent(vm->snapshots);
virDomainSnapshotPtr ret = NULL;
g_autoptr(virDomainSnapshotDef) snapdef = virObjectRef(snapdeftmp);
@@ -1678,8 +1696,10 @@ qemuSnapshotRedefine(virDomainObj *vm,
* makes sense, such as checking that qemu-img recognizes the
* snapshot name in at least one of the domain's disks? */
-if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)
+if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT) {
+qemuSnapshotClearRevertdisks(current);
qemuSnapshotSetCurrent(vm, snap);
+}
if (qemuSnapshotCreateWriteMetadata(vm, snap, driver, cfg) < 0)
goto error;
@@ -1758,6 +1778,7 @@ qemuSnapshotCreate(virDomainObj *vm,
}
if (!tmpsnap) {
+qemuSnapshotClearRevertdisks(current);
qemuSnapshotSetCurrent(vm, snap);
if (qemuSnapshotCreateWriteMetadata(vm, snap, driver, cfg) < 0)
--
2.41.0
[libvirt PATCH v2 05/24] snapshot_conf: add new argument to virDomainSnapshotAlignDisks
This new option will be used by external snapshot revert code.
Signed-off-by: Pavel Hrdina
---
src/conf/snapshot_conf.c | 15 ---
src/conf/snapshot_conf.h | 3 ++-
src/qemu/qemu_snapshot.c | 2 +-
src/test/test_driver.c | 2 +-
4 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index f6725c0e7b..d8ebf786cf 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -578,6 +578,8 @@
virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDef *def,
* @default_snapshot: snapshot location to assign to disks which don't have any
* @uniform_internal_snapshot: Require that for an internal snapshot all disks
* take part in the internal snapshot
+ * @force_default_location: Always use @default_snapshot even if domain def
+ * has different default value
*
* Align snapdef->disks to domain definition, filling in any missing disks or
* snapshot state defaults given by the domain, with a fallback to
@@ -595,6 +597,10 @@
virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDef *def,
* in the internal snapshot. This is for hypervisors where granularity of an
* internal snapshot can't be controlled.
*
+ * When @force_default_location is true we will always use @default_snapshot
+ * even if domain definition has different default set. This is required to
+ * create new snapshot definition when reverting external snapshots.
+ *
* Convert paths to disk targets for uniformity.
*
* On error -1 is returned and a libvirt error is reported.
@@ -603,7 +609,8 @@ int
virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
virDomainDef *existingDomainDef,
virDomainSnapshotLocation default_snapshot,
-bool uniform_internal_snapshot)
+bool uniform_internal_snapshot,
+bool force_default_location)
{
virDomainDef *domdef = snapdef->parent.dom;
g_autoptr(GHashTable) map = virHashNew(NULL);
@@ -711,7 +718,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
/* Don't snapshot empty drives */
if (virStorageSourceIsEmpty(domdef->disks[i]->src))
snapdisk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NO;
-else
+else if (!force_default_location)
snapdisk->snapshot = domdef->disks[i]->snapshot;
snapdisk->src->type = VIR_STORAGE_TYPE_FILE;
@@ -967,8 +974,10 @@ virDomainSnapshotRedefinePrep(virDomainObj *vm,
virDomainSnapshotDefIsExternal(snapdef))
align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
-if (virDomainSnapshotAlignDisks(snapdef, otherDomDef, align_location,
true) < 0)
+if (virDomainSnapshotAlignDisks(snapdef, otherDomDef, align_location,
+true, false) < 0) {
return -1;
+}
return 0;
}
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index ab76af604a..14254d1c86 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -126,7 +126,8 @@ char *virDomainSnapshotDefFormat(const char *uuidstr,
int virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapshot,
virDomainDef *existingDomainDef,
virDomainSnapshotLocation default_snapshot,
-bool uniform_internal_snapshot);
+bool uniform_internal_snapshot,
+bool force_default_location);
bool virDomainSnapshotDefIsExternal(virDomainSnapshotDef *def);
bool virDomainSnapshotIsExternal(virDomainMomentObj *snap);
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 91de8b0c31..844b02d427 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1585,7 +1585,7 @@ qemuSnapshotCreateAlignDisks(virDomainObj *vm,
else
def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
}
-if (virDomainSnapshotAlignDisks(def, NULL, align_location, true) < 0)
+if (virDomainSnapshotAlignDisks(def, NULL, align_location, true, false) <
0)
return -1;
return 0;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index e7fce053b4..83d97e629c 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -8721,7 +8721,7 @@ testDomainSnapshotAlignDisks(virDomainObj *vm,
VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
}
-return virDomainSnapshotAlignDisks(def, NULL, align_location, true);
+return virDomainSnapshotAlignDisks(def, NULL, align_location, true, false);
}
--
2.41.0
[libvirt PATCH v2 18/24] qemu_snapshot: delete: properly update parent snapshot with revert data
When deleting external snapshot and parent snapshot is the currently
active snapshot as user reverted to it we need to properly update the
parent snapshot metadata.
After the delete is done the new overlay files will be the currently
used files created when snapshot revert was done, replacing the original
overlay files.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 40
1 file changed, 40 insertions(+)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index b08e06d312..a206f015c4 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -3168,6 +3168,41 @@ qemuSnapshotDiscardExternal(virDomainObj *vm,
}
+static int
+qemuSnapshotDeleteUpdateParent(virDomainObj *vm,
+ virDomainMomentObj *parent)
+{
+size_t i;
+virQEMUDriver *driver = QEMU_DOMAIN_PRIVATE(vm)->driver;
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+virDomainSnapshotDef *parentDef = virDomainSnapshotObjGetDef(parent);
+
+if (!parentDef)
+return 0;
+
+if (!parentDef->revertdisks)
+return 0;
+
+for (i = 0; i < parentDef->ndisks; i++) {
+virDomainSnapshotDiskDefClear(>disks[i]);
+}
+g_free(parentDef->disks);
+
+parentDef->disks = g_steal_pointer(>revertdisks);
+parentDef->ndisks = parentDef->nrevertdisks;
+parentDef->nrevertdisks = 0;
+
+if (qemuDomainSnapshotWriteMetadata(vm,
+parent,
+driver->xmlopt,
+cfg->snapshotDir) < 0) {
+return -1;
+}
+
+return 0;
+}
+
+
static int
qemuSnapshotDiscardMetadata(virDomainObj *vm,
virDomainMomentObj *snap,
@@ -3207,6 +3242,11 @@ qemuSnapshotDiscardMetadata(virDomainObj *vm,
virDomainMomentMoveChildren(snap, snap->parent);
}
+if (update_parent && snap->parent) {
+if (qemuSnapshotDeleteUpdateParent(vm, snap->parent) < 0)
+ret = -1;
+}
+
snapFile = g_strdup_printf("%s/%s/%s.xml", cfg->snapshotDir, vm->def->name,
snap->def->name);
--
2.41.0
[libvirt PATCH v2 06/24] qemu_snapshot: introduce qemuSnapshotDomainDefUpdateDisk
Extract the code that updates disks in domain definition while creating
external snapshots. We will use it later in the external snapshot revert
code.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 63
1 file changed, 38 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 844b02d427..72a0f71d4f 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -142,6 +142,42 @@ qemuSnapshotFSThaw(virDomainObj *vm,
}
+static int
+qemuSnapshotDomainDefUpdateDisk(virDomainDef *domdef,
+virDomainSnapshotDef *snapdef,
+bool reuse)
+{
+size_t i;
+
+for (i = 0; i < snapdef->ndisks; i++) {
+g_autoptr(virStorageSource) newsrc = NULL;
+virDomainSnapshotDiskDef *snapdisk = &(snapdef->disks[i]);
+virDomainDiskDef *defdisk = domdef->disks[i];
+
+if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
+
+if (!(newsrc = virStorageSourceCopy(snapdisk->src, false)))
+return -1;
+
+if (virStorageSourceInitChainElement(newsrc, defdisk->src, false) < 0)
+return -1;
+
+if (!reuse &&
+virStorageSourceHasBacking(defdisk->src)) {
+defdisk->src->readonly = true;
+newsrc->backingStore = g_steal_pointer(>src);
+} else {
+virObjectUnref(defdisk->src);
+}
+
+defdisk->src = g_steal_pointer();
+}
+
+return 0;
+}
+
+
/* The domain is expected to be locked and inactive. */
static int
qemuSnapshotCreateInactiveInternal(virQEMUDriver *driver,
@@ -216,31 +252,8 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver,
}
/* update disk definitions */
-for (i = 0; i < snapdef->ndisks; i++) {
-g_autoptr(virStorageSource) newsrc = NULL;
-
-snapdisk = &(snapdef->disks[i]);
-defdisk = vm->def->disks[i];
-
-if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
-continue;
-
-if (!(newsrc = virStorageSourceCopy(snapdisk->src, false)))
-goto cleanup;
-
-if (virStorageSourceInitChainElement(newsrc, defdisk->src, false) < 0)
-goto cleanup;
-
-if (!reuse &&
-virStorageSourceHasBacking(defdisk->src)) {
-defdisk->src->readonly = true;
-newsrc->backingStore = g_steal_pointer(>src);
-} else {
-virObjectUnref(defdisk->src);
-}
-
-defdisk->src = g_steal_pointer();
-}
+if (qemuSnapshotDomainDefUpdateDisk(vm->def, snapdef, reuse) < 0)
+goto cleanup;
if (virDomainDefSave(vm->def, driver->xmlopt, cfg->configDir) < 0)
goto cleanup;
--
2.41.0
[libvirt PATCH v2 23/24] qemu_snapshot: add checks for external snapshot deletion
With the introduction of external snapshot revert support we need to
error out in some cases when trying to delete some snapshots.
If users reverts to non-leaf snapshots and would try to delete it after
the revert is done it would not work currently as this operation would
require using block-stream which is not implemented for now as in this
case the snapshot has two children so the disk files have multiple
overlays.
Similarly if user reverts to non-leaf snapshot and would try to delete
snapshot that is non-leaf but not in currently active snapshot chain we
would still need to use block-commit operation. The issue here is that
in order to do that we would have to start new qemu process with
different domain definition than what is currently used by the domain.
If the current domain would be running it would complicate things even
more so this operation is not yet supported.
If user creates new snapshot after reverting to non-leaf snapshot it
creates a new branch. Deleting snapshot with multiple children will
require block-stream which is not implemented for now.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 9246c02f12..9e8a7f2f9f 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -3636,6 +3636,8 @@ qemuSnapshotDeleteValidate(virDomainObj *vm,
}
if (virDomainSnapshotIsExternal(snap)) {
+virDomainMomentObj *current =
virDomainSnapshotGetCurrent(vm->snapshots);
+
if (qemuDomainHasBlockjob(vm, false)) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("cannot delete external snapshots when there is
another active block job"));
@@ -3647,6 +3649,25 @@ qemuSnapshotDeleteValidate(virDomainObj *vm,
_("deletion of external disk snapshots with
children not supported"));
return -1;
}
+
+if (snap == current && snap->nchildren != 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("deletion of active external snapshot that is not
a leaf snapshot is not supported"));
+return -1;
+}
+
+if (snap != current && snap->nchildren != 0 &&
+virDomainMomentIsAncestor(snap, current)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("deletion of non-leaf external snapshot that is
not in active chain is not supported"));
+return -1;
+}
+
+if (snap->nchildren > 1) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("deletion of external disk snapshot with multiple
children snapshots not supported"));
+return -1;
+}
}
return 0;
--
2.41.0
[libvirt PATCH v2 24/24] qemu_snapshot: allow snapshot revert for external snapshots
Now that the support to revert external snapshots is implemented we can
drop this check.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 9e8a7f2f9f..5150e8685a 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1880,12 +1880,6 @@ qemuSnapshotRevertValidate(virDomainObj *vm,
return -1;
}
-if (virDomainSnapshotIsExternal(snap)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("revert to external snapshot not supported yet"));
-return -1;
-}
-
if (!snap->def->dom) {
virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY,
_("snapshot '%1$s' lacks domain '%2$s' rollback info"),
--
2.41.0
[libvirt PATCH v2 10/24] qemu_snapshot: move external disk prepare to single function
We will need to reuse the functionality when reverting external
snapshots.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 38 +++---
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 069c4c8ba7..5d2ffdeee6 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -520,6 +520,25 @@ qemuSnapshotPrepareDiskExternal(virDomainDiskDef *disk,
bool active,
bool reuse)
{
+if (!snapdisk->src->format) {
+snapdisk->src->format = VIR_STORAGE_FILE_QCOW2;
+} else if (snapdisk->src->format != VIR_STORAGE_FILE_QCOW2 &&
+ snapdisk->src->format != VIR_STORAGE_FILE_QED) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("external snapshot format for disk %1$s is
unsupported: %2$s"),
+ snapdisk->name,
+
virStorageFileFormatTypeToString(snapdisk->src->format));
+return -1;
+}
+
+if (snapdisk->src->metadataCacheMaxSize > 0) {
+if (snapdisk->src->format != VIR_STORAGE_FILE_QCOW2) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("metadata cache max size control is supported
only with qcow2 images"));
+return -1;
+}
+}
+
if (qemuTranslateSnapshotDiskSourcePool(snapdisk) < 0)
return -1;
@@ -700,25 +719,6 @@ qemuSnapshotPrepare(virDomainObj *vm,
break;
case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL:
-if (!disk->src->format) {
-disk->src->format = VIR_STORAGE_FILE_QCOW2;
-} else if (disk->src->format != VIR_STORAGE_FILE_QCOW2 &&
- disk->src->format != VIR_STORAGE_FILE_QED) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("external snapshot format for disk %1$s is
unsupported: %2$s"),
- disk->name,
-
virStorageFileFormatTypeToString(disk->src->format));
-return -1;
-}
-
-if (disk->src->metadataCacheMaxSize > 0) {
-if (disk->src->format != VIR_STORAGE_FILE_QCOW2) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("metadata cache max size control is
supported only with qcow2 images"));
-return -1;
-}
-}
-
if (qemuSnapshotPrepareDiskExternal(dom_disk, disk, active, reuse)
< 0)
return -1;
--
2.41.0
[libvirt PATCH v2 22/24] qemu_snapshot: check only once if snapshot is external
There will be more external snapshot checks introduced by following
patch so group them together.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 337c83f151..9246c02f12 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -3635,18 +3635,18 @@ qemuSnapshotDeleteValidate(virDomainObj *vm,
}
}
-if (virDomainSnapshotIsExternal(snap) &&
-qemuDomainHasBlockjob(vm, false)) {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("cannot delete external snapshots when there is
another active block job"));
-return -1;
-}
+if (virDomainSnapshotIsExternal(snap)) {
+if (qemuDomainHasBlockjob(vm, false)) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("cannot delete external snapshots when there is
another active block job"));
+return -1;
+}
-if (virDomainSnapshotIsExternal(snap) &&
-(flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("deletion of external disk snapshots with children
not supported"));
-return -1;
+if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("deletion of external disk snapshots with
children not supported"));
+return -1;
+}
}
return 0;
--
2.41.0
[libvirt PATCH v2 12/24] qemu_snapshot: introduce external snapshot revert support
When reverting to external snapshot we need to create new overlay qcow2
files from the disk files the VM had when the snapshot was taken.
There are some specifics and limitations when reverting to a snapshot:
1) When reverting to last snapshot we need to first create new overlay
files before we can safely delete the old overlay files in case the
creation fails so we have still recovery option when we error out.
These new files will not have the suffix as when the snapshot was
created as renaming the original files in order to use the same file
names as when the snapshot was created would add unnecessary
complexity to the code.
2) When reverting to any snapshot we will always create overlay files
for every disk the VM had when the snapshot was done. Otherwise we
would have to figure out if there is any other qcow2 image already
using any of the VM disks as backing store and that itself might be
extremely complex and in some cases impossible.
3) When reverting from any state the current overlay files will be
always removed as that VM state is not meant to be saved. It's the
same as with internal snapshots. If user want's to keep the current
state before reverting they need to create a new snapshot. For now
this will only work if the current snapshot is the last.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 232 ++-
1 file changed, 228 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 1cb0ea55de..dbf2cdd5db 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -18,6 +18,8 @@
#include
+#include
+
#include "qemu_snapshot.h"
#include "qemu_monitor.h"
@@ -1975,6 +1977,183 @@ qemuSnapshotRevertWriteMetadata(virDomainObj *vm,
}
+static int
+qemuSnapshotRevertExternalPrepare(virDomainObj *vm,
+ virDomainSnapshotDef *tmpsnapdef,
+ virDomainMomentObj *snap,
+ virDomainDef *config,
+ virDomainDef *inactiveConfig,
+ int *memsnapFD,
+ char **memsnapPath)
+{
+ssize_t i;
+bool active = virDomainObjIsActive(vm);
+virDomainDef *domdef = NULL;
+virDomainSnapshotLocation location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
+virDomainSnapshotDef *snapdef = virDomainSnapshotObjGetDef(snap);
+
+if (config) {
+domdef = config;
+} else {
+domdef = inactiveConfig;
+}
+
+/* We need this to generate creation timestamp that is used as default
+ * snapshot name. */
+if (virDomainMomentDefPostParse(>parent) < 0)
+return -1;
+
+if (virDomainSnapshotAlignDisks(tmpsnapdef, domdef, location, false, true)
< 0)
+return -1;
+
+for (i = 0; i < tmpsnapdef->ndisks; i++) {
+virDomainSnapshotDiskDef *snapdisk = >disks[i];
+virDomainDiskDef *domdisk = domdef->disks[i];
+
+if (qemuSnapshotPrepareDiskExternal(domdisk, snapdisk, active, false)
< 0)
+return -1;
+}
+
+if (memsnapFD && memsnapPath && snapdef->memorysnapshotfile) {
+virQEMUDriver *driver = ((qemuDomainObjPrivate *)
vm->privateData)->driver;
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+
+*memsnapPath = snapdef->memorysnapshotfile;
+*memsnapFD = qemuDomainOpenFile(cfg, NULL, *memsnapPath, O_RDONLY,
NULL);
+}
+
+return 0;
+}
+
+
+static int
+qemuSnapshotRevertExternalActive(virDomainObj *vm,
+ virDomainSnapshotDef *tmpsnapdef)
+{
+ssize_t i;
+g_autoptr(GHashTable) blockNamedNodeData = NULL;
+g_autoptr(qemuSnapshotDiskContext) snapctxt = NULL;
+
+snapctxt = qemuSnapshotDiskContextNew(tmpsnapdef->ndisks, vm,
VIR_ASYNC_JOB_SNAPSHOT);
+
+if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm,
VIR_ASYNC_JOB_SNAPSHOT)))
+return -1;
+
+for (i = 0; i < tmpsnapdef->ndisks; i++) {
+if (qemuSnapshotDiskPrepareOne(snapctxt,
+ vm->def->disks[i],
+ tmpsnapdef->disks + i,
+ blockNamedNodeData,
+ false,
+ true) < 0)
+return -1;
+}
+
+if (qemuSnapshotDiskCreate(snapctxt) < 0)
+return -1;
+
+return 0;
+}
+
+
+static int
+qemuSnapshotRevertExternalInactive(virDomainObj *vm,
+ virDomainSnapshotDef *tmpsnapdef,
+ virDomainDef *config,
+ virDomainDef *inactiveConfig)
+{
+virDomainDef *domdef = NULL;
+g_autoptr(virBitmap) created = NULL;
+
+create
[libvirt PATCH v2 17/24] qemu_snapshot: add support to delete external snapshot without block commit
When block commit is not needed we can just simply unlink the
disk files.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 56 ++--
1 file changed, 36 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index be94e97340..b08e06d312 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2510,6 +2510,7 @@ typedef struct _qemuSnapshotDeleteExternalData {
virStorageSource *prevDiskSrc; /* source of disk for which @diskSrc is
backing disk */
qemuBlockJobData *job;
+bool merge;
} qemuSnapshotDeleteExternalData;
@@ -2618,6 +2619,7 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
data = g_new0(qemuSnapshotDeleteExternalData, 1);
data->snapDisk = snapDisk;
+data->merge = merge;
data->parentDomDisk = virDomainDiskByTarget(snapdef->parent.dom,
data->snapDisk->name);
@@ -2628,7 +2630,7 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
return -1;
}
-if (merge) {
+if (data->merge) {
data->domDisk = qemuDomainDiskByName(vm->def, snapDisk->name);
if (!data->domDisk)
return -1;
@@ -3053,31 +3055,42 @@ qemuSnapshotDiscardExternal(virDomainObj *vm,
virTristateBool autofinalize = VIR_TRISTATE_BOOL_NO;
unsigned int commitFlags = VIR_DOMAIN_BLOCK_COMMIT_DELETE;
-if (data->domDisk->src == data->diskSrc) {
-commitFlags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE;
-autofinalize = VIR_TRISTATE_BOOL_YES;
+if (data->merge) {
+if (data->domDisk->src == data->diskSrc) {
+commitFlags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE;
+autofinalize = VIR_TRISTATE_BOOL_YES;
+}
+
+if (qemuSnapshotSetInvalid(vm, data->parentSnap, data->snapDisk,
true) < 0)
+goto error;
+
+data->job = qemuBlockCommit(vm,
+data->domDisk,
+data->parentDiskSrc,
+data->diskSrc,
+data->prevDiskSrc,
+0,
+VIR_ASYNC_JOB_SNAPSHOT,
+autofinalize,
+commitFlags);
+
+if (!data->job)
+goto error;
+} else {
+if (virStorageSourceInit(data->parentDomDisk->src) < 0 ||
+virStorageSourceUnlink(data->parentDomDisk->src) < 0) {
+VIR_WARN("Failed to remove snapshot image '%s'",
+ data->snapDisk->name);
+}
}
-
-if (qemuSnapshotSetInvalid(vm, data->parentSnap, data->snapDisk, true)
< 0)
-goto error;
-
-data->job = qemuBlockCommit(vm,
-data->domDisk,
-data->parentDiskSrc,
-data->diskSrc,
-data->prevDiskSrc,
-0,
-VIR_ASYNC_JOB_SNAPSHOT,
-autofinalize,
-commitFlags);
-
-if (!data->job)
-goto error;
}
for (cur = externalData; cur; cur = g_slist_next(cur)) {
qemuSnapshotDeleteExternalData *data = cur->data;
+if (!data->merge)
+continue;
+
if (qemuSnapshotDeleteBlockJobRunning(vm, data->job) < 0)
goto error;
@@ -3092,6 +3105,9 @@ qemuSnapshotDiscardExternal(virDomainObj *vm,
for (cur = externalData; cur; cur = g_slist_next(cur)) {
qemuSnapshotDeleteExternalData *data = cur->data;
+if (!data->merge)
+continue;
+
if (data->job->state == QEMU_BLOCKJOB_STATE_READY) {
if (qemuBlockPivot(vm, data->job, VIR_ASYNC_JOB_SNAPSHOT, NULL) <
0)
goto error;
--
2.41.0
[libvirt PATCH v2 16/24] qemu_snapshot: prepare data for non-active leaf external snapshot deletion
In this case there is no need to run block commit and using qemu process
at all.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 55 +++-
1 file changed, 32 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 9c4d26bad5..be94e97340 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2698,34 +2698,43 @@ qemuSnapshotDeleteExternalPrepare(virDomainObj *vm,
return 0;
}
-/* this also serves as validation whether the snapshot can be deleted */
-if (qemuSnapshotDeleteExternalPrepareData(vm, snap, true, ) < 0)
-return -1;
-
-if (!virDomainObjIsActive(vm)) {
-if (qemuProcessStart(NULL, driver, vm, NULL, VIR_ASYNC_JOB_SNAPSHOT,
- NULL, -1, NULL, NULL,
- VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
- VIR_QEMU_PROCESS_START_PAUSED) < 0) {
-return -1;
-}
-
-*stop_qemu = true;
-
-/* Call the prepare again as some data require that the VM is
- * running to get everything we need. */
-if (qemuSnapshotDeleteExternalPrepareData(vm, snap, true,
externalData) < 0)
+/* Leaf non-active snapshot doesn't have overlay files for the disk images
+ * so there is no need to do any merge and we can just delete the files
+ * directly. */
+if (snap != virDomainSnapshotGetCurrent(vm->snapshots) &&
+snap->nchildren == 0) {
+if (qemuSnapshotDeleteExternalPrepareData(vm, snap, false,
externalData) < 0)
return -1;
} else {
-qemuDomainJobPrivate *jobPriv = vm->job->privateData;
+/* this also serves as validation whether the snapshot can be deleted
*/
+if (qemuSnapshotDeleteExternalPrepareData(vm, snap, true, ) <
0)
+return -1;
-*externalData = g_steal_pointer();
+if (!virDomainObjIsActive(vm)) {
+if (qemuProcessStart(NULL, driver, vm, NULL,
VIR_ASYNC_JOB_SNAPSHOT,
+ NULL, -1, NULL, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
+ VIR_QEMU_PROCESS_START_PAUSED) < 0) {
+return -1;
+}
-/* If the VM is running we need to indicate that the async snapshot
- * job is snapshot delete job. */
-jobPriv->snapshotDelete = true;
+*stop_qemu = true;
-qemuDomainSaveStatus(vm);
+/* Call the prepare again as some data require that the VM is
+ * running to get everything we need. */
+if (qemuSnapshotDeleteExternalPrepareData(vm, snap, true,
externalData) < 0)
+return -1;
+} else {
+qemuDomainJobPrivate *jobPriv = vm->job->privateData;
+
+*externalData = g_steal_pointer();
+
+/* If the VM is running we need to indicate that the async snapshot
+ * job is snapshot delete job. */
+jobPriv->snapshotDelete = true;
+
+qemuDomainSaveStatus(vm);
+}
}
return 0;
--
2.41.0
[libvirt PATCH v2 08/24] qemu_snapshot: introduce qemuSnapshotCreateQcow2Files
Extract creation of qcow2 files for external snapshots to separate
function as we will need it for external snapshot revert code.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 67 +---
1 file changed, 42 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 8e1eb21b5d..227c201195 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -181,35 +181,21 @@ qemuSnapshotDomainDefUpdateDisk(virDomainDef *domdef,
}
-/* The domain is expected to be locked and inactive. */
static int
-qemuSnapshotCreateInactiveInternal(virQEMUDriver *driver,
- virDomainObj *vm,
- virDomainMomentObj *snap)
-{
-return qemuDomainSnapshotForEachQcow2(driver, vm->def, snap, "-c", false);
-}
-
-
-/* The domain is expected to be locked and inactive. */
-static int
-qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver,
- virDomainObj *vm,
- virDomainMomentObj *snap,
- bool reuse)
+qemuSnapshotCreateQcow2Files(virDomainObj *vm,
+ virDomainSnapshotDef *snapdef,
+ virBitmap *created,
+ bool reuse)
{
size_t i;
-virDomainSnapshotDiskDef *snapdisk;
-virDomainDiskDef *defdisk;
const char *qemuImgPath;
-g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-int ret = -1;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
-virDomainSnapshotDef *snapdef = virDomainSnapshotObjGetDef(snap);
-g_autoptr(virBitmap) created = virBitmapNew(snapdef->ndisks);
+virQEMUDriver *driver = ((qemuDomainObjPrivate *) vm->privateData)->driver;
+virDomainSnapshotDiskDef *snapdisk = NULL;
+virDomainDiskDef *defdisk = NULL;
if (!(qemuImgPath = qemuFindQemuImgBinary(driver)))
-goto cleanup;
+return -1;
/* If reuse is true, then qemuSnapshotPrepare already
* ensured that the new files exist, and it was up to the user to
@@ -218,6 +204,7 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver,
g_autoptr(virCommand) cmd = NULL;
snapdisk = &(snapdef->disks[i]);
defdisk = vm->def->disks[i];
+
if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
continue;
@@ -225,7 +212,7 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver,
snapdisk->src->format = VIR_STORAGE_FILE_QCOW2;
if (qemuDomainStorageSourceValidateDepth(defdisk->src, 1,
defdisk->dst) < 0)
-goto cleanup;
+return -1;
/* creates cmd line args: qemu-img create -f qcow2 -o */
if (!(cmd = virCommandNewArgList(qemuImgPath,
@@ -234,7 +221,7 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver,
virStorageFileFormatTypeToString(snapdisk->src->format),
"-o",
NULL)))
-goto cleanup;
+return -1;
/* adds cmd line arg:
backing_fmt=format,backing_file=/path/to/backing/file */
virBufferAsprintf(, "backing_fmt=%s,backing_file=",
@@ -251,9 +238,39 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver,
ignore_value(virBitmapSetBit(created, i));
if (virCommandRun(cmd, NULL) < 0)
-goto cleanup;
+return -1;
}
+return 0;
+}
+
+
+/* The domain is expected to be locked and inactive. */
+static int
+qemuSnapshotCreateInactiveInternal(virQEMUDriver *driver,
+ virDomainObj *vm,
+ virDomainMomentObj *snap)
+{
+return qemuDomainSnapshotForEachQcow2(driver, vm->def, snap, "-c", false);
+}
+
+
+/* The domain is expected to be locked and inactive. */
+static int
+qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver,
+ virDomainObj *vm,
+ virDomainMomentObj *snap,
+ bool reuse)
+{
+virDomainSnapshotDiskDef *snapdisk;
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+int ret = -1;
+virDomainSnapshotDef *snapdef = virDomainSnapshotObjGetDef(snap);
+g_autoptr(virBitmap) created = virBitmapNew(snapdef->ndisks);
+
+if (qemuSnapshotCreateQcow2Files(vm, snapdef, created, reuse) < 0)
+goto cleanup;
+
/* update disk definitions */
if (qemuSnapshotDomainDefUpdateDisk(vm->def, snapdef, reuse) < 0)
goto cleanup;
--
2.41.0
[libvirt PATCH v2 09/24] qemu_snapshot: allow using alternate domain definition when creating qcow2 files
To create new overlay files when external snapshot revert support is
introduced we will be using different domain definition than what is
currently used by the domain.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 227c201195..069c4c8ba7 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -183,6 +183,7 @@ qemuSnapshotDomainDefUpdateDisk(virDomainDef *domdef,
static int
qemuSnapshotCreateQcow2Files(virDomainObj *vm,
+ virDomainDef *def,
virDomainSnapshotDef *snapdef,
virBitmap *created,
bool reuse)
@@ -194,6 +195,9 @@ qemuSnapshotCreateQcow2Files(virDomainObj *vm,
virDomainSnapshotDiskDef *snapdisk = NULL;
virDomainDiskDef *defdisk = NULL;
+if (!def)
+def = vm->def;
+
if (!(qemuImgPath = qemuFindQemuImgBinary(driver)))
return -1;
@@ -203,7 +207,7 @@ qemuSnapshotCreateQcow2Files(virDomainObj *vm,
for (i = 0; i < snapdef->ndisks && !reuse; i++) {
g_autoptr(virCommand) cmd = NULL;
snapdisk = &(snapdef->disks[i]);
-defdisk = vm->def->disks[i];
+defdisk = def->disks[i];
if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
continue;
@@ -268,7 +272,7 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriver *driver,
virDomainSnapshotDef *snapdef = virDomainSnapshotObjGetDef(snap);
g_autoptr(virBitmap) created = virBitmapNew(snapdef->ndisks);
-if (qemuSnapshotCreateQcow2Files(vm, snapdef, created, reuse) < 0)
+if (qemuSnapshotCreateQcow2Files(vm, NULL, snapdef, created, reuse) < 0)
goto cleanup;
/* update disk definitions */
--
2.41.0
[libvirt PATCH v2 15/24] qemu_snapshot: add merge to external snapshot delete prepare data
Before external snapshot revert every delete operation did block commit
in order to delete a snapshot. But now when user reverts to non-leaf
snapshot deleting leaf snapshot will not have any overlay files so we
can just simply delete the snapshot images.
Signed-off-by: Pavel Hrdina
---
src/qemu/qemu_snapshot.c | 93 ++--
1 file changed, 60 insertions(+), 33 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 08cff2a9a2..9c4d26bad5 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2569,9 +2569,26 @@ qemuSnapshotFindParentSnapForDisk(virDomainMomentObj
*snap,
}
+/**
+ * qemuSnapshotDeleteExternalPrepareData:
+ * @vm: domain object
+ * @snap: snapshot object
+ * @merge: whether we are just deleting image or not
+ * @externalData: prepared data to delete external snapshot
+ *
+ * Validate if we can delete selected snapshot @snap and prepare all necessary
+ * data that will be used when deleting snapshot as @externalData.
+ *
+ * If @merge is set to true we will merge the deleted snapshot into parent one
+ * instead of just deleting it. This is necessary when operating on snapshot
+ * that has existing overlay files.
+ *
+ * Returns -1 on error, 0 on success.
+ */
static int
qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
virDomainMomentObj *snap,
+ bool merge,
GSList **externalData)
{
ssize_t i;
@@ -2579,7 +2596,6 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
g_autoslist(qemuSnapshotDeleteExternalData) ret = NULL;
for (i = 0; i < snapdef->ndisks; i++) {
-g_autofree qemuSnapshotDeleteExternalData *data = NULL;
virDomainSnapshotDiskDef *snapDisk = &(snapdef->disks[i]);
if (snapDisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
@@ -2591,14 +2607,18 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
snapDisk->name);
return -1;
}
+}
+
+for (i = 0; i < snapdef->ndisks; i++) {
+virDomainSnapshotDiskDef *snapDisk = &(snapdef->disks[i]);
+g_autofree qemuSnapshotDeleteExternalData *data = NULL;
+
+if (snapDisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+continue;
data = g_new0(qemuSnapshotDeleteExternalData, 1);
data->snapDisk = snapDisk;
-data->domDisk = qemuDomainDiskByName(vm->def, snapDisk->name);
-if (!data->domDisk)
-return -1;
-
data->parentDomDisk = virDomainDiskByTarget(snapdef->parent.dom,
data->snapDisk->name);
if (!data->parentDomDisk) {
@@ -2608,39 +2628,46 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
return -1;
}
-if (virDomainObjIsActive(vm)) {
-data->diskSrc =
virStorageSourceChainLookupBySource(data->domDisk->src,
-
data->snapDisk->src,
-
>prevDiskSrc);
-if (!data->diskSrc)
+if (merge) {
+data->domDisk = qemuDomainDiskByName(vm->def, snapDisk->name);
+if (!data->domDisk)
return -1;
-if (!virStorageSourceIsSameLocation(data->diskSrc,
data->snapDisk->src)) {
-virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("VM disk source and snapshot disk source are
not the same"));
-return -1;
-}
+if (virDomainObjIsActive(vm)) {
+data->diskSrc =
virStorageSourceChainLookupBySource(data->domDisk->src,
+
data->snapDisk->src,
+
>prevDiskSrc);
+if (!data->diskSrc)
+return -1;
-data->parentDiskSrc = data->diskSrc->backingStore;
-if (!virStorageSourceIsBacking(data->parentDiskSrc)) {
-virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("failed to find parent disk source in backing
chain"));
-return -1;
-}
+if (!virStorageSourceIsSameLocation(data->diskSrc,
data->snapDisk->src)) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("VM disk source and snapshot disk source
are not the same"));
+return -1;
+}
-if (!virStorageSourceIsSameLocation(data-
[libvirt PATCH v2 01/24] libvirt_private: list virDomainMomentDefPostParse
We will need to call this function from qemu_snapshot when introducing external snapshot revert support. Signed-off-by: Pavel Hrdina Reviewed-by: Peter Krempa --- src/libvirt_private.syms | 4 1 file changed, 4 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fb7ad9c855..eed4c17d49 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -807,6 +807,10 @@ virInterfaceDefParseString; virInterfaceDefParseXML; +# conf/moment_conf.h +virDomainMomentDefPostParse; + + # conf/netdev_bandwidth_conf.h virDomainClearNetBandwidth; virNetDevBandwidthFormat; -- 2.41.0
[libvirt PATCH v2 13/24] qemu_snapshot: rename qemuSnapshotDeleteExternalPrepare
The new name reflects that we prepare data for external snapshot
deletion and the old name will be used later for different part of code.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/qemu/qemu_snapshot.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index dbf2cdd5db..c5de24917b 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2570,9 +2570,9 @@ qemuSnapshotFindParentSnapForDisk(virDomainMomentObj
*snap,
static int
-qemuSnapshotDeleteExternalPrepare(virDomainObj *vm,
- virDomainMomentObj *snap,
- GSList **externalData)
+qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
+ virDomainMomentObj *snap,
+ GSList **externalData)
{
ssize_t i;
virDomainSnapshotDef *snapdef = virDomainSnapshotObjGetDef(snap);
@@ -3429,7 +3429,7 @@ qemuSnapshotDelete(virDomainObj *vm,
g_autoslist(qemuSnapshotDeleteExternalData) tmpData = NULL;
/* this also serves as validation whether the snapshot can be
deleted */
-if (qemuSnapshotDeleteExternalPrepare(vm, snap, ) < 0)
+if (qemuSnapshotDeleteExternalPrepareData(vm, snap, ) < 0)
goto endjob;
if (!virDomainObjIsActive(vm)) {
@@ -3444,7 +3444,7 @@ qemuSnapshotDelete(virDomainObj *vm,
/* Call the prepare again as some data require that the VM is
* running to get everything we need. */
-if (qemuSnapshotDeleteExternalPrepare(vm, snap, )
< 0)
+if (qemuSnapshotDeleteExternalPrepareData(vm, snap,
) < 0)
goto endjob;
} else {
qemuDomainJobPrivate *jobPriv = vm->job->privateData;
--
2.41.0
[libvirt PATCH v2 03/24] snapshot_conf: use alternate domain definition in virDomainSnapshotDefAssignExternalNames
Commit introduced new
argument for virDomainSnapshotAlignDisks() that allows passing alternate
domain definition in case the snapshot parent.dom is NULL.
In case of redefining snapshot it will not hit the part of code that
unconditionally uses parent.dom as there will not be need to generate
default external file names.
It should be still fixed to make it safe. Future external snapshot
revert code will use this to generate default file names and in this
case it would crash.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/conf/snapshot_conf.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index cc59bddbc8..ac5aba1753 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -486,12 +486,14 @@ virDomainSnapshotRedefineValidate(virDomainSnapshotDef
*def,
/**
* virDomainSnapshotDefAssignExternalNames:
* @def: snapshot def object
+ * @domdef: domain def object
*
* Generate default external file names for snapshot targets. Returns 0 on
* success, -1 on error.
*/
static int
-virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDef *def)
+virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDef *def,
+virDomainDef *domdef)
{
const char *origpath;
char *tmppath;
@@ -514,7 +516,7 @@
virDomainSnapshotDefAssignExternalNames(virDomainSnapshotDef *def)
return -1;
}
-if (!(origpath = virDomainDiskGetSource(def->parent.dom->disks[i]))) {
+if (!(origpath = virDomainDiskGetSource(domdef->disks[i]))) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("cannot generate external snapshot name for disk
'%1$s' without source"),
disk->name);
@@ -702,7 +704,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
}
/* Generate default external file names for external snapshot locations */
-if (virDomainSnapshotDefAssignExternalNames(snapdef) < 0)
+if (virDomainSnapshotDefAssignExternalNames(snapdef, domdef) < 0)
return -1;
return 0;
--
2.41.0
[libvirt PATCH v2 04/24] snapshot_conf: introduce metadata element
This new element will hold the new disk overlay created when reverting
to non-leaf snapshot in order to remember the files libvirt created.
Signed-off-by: Pavel Hrdina
Reviewed-by: Peter Krempa
---
src/conf/schemas/domainsnapshot.rng | 7 +++
src/conf/snapshot_conf.c| 27 +++
src/conf/snapshot_conf.h| 5 +
3 files changed, 39 insertions(+)
diff --git a/src/conf/schemas/domainsnapshot.rng
b/src/conf/schemas/domainsnapshot.rng
index 45f01b96cd..2549c47b22 100644
--- a/src/conf/schemas/domainsnapshot.rng
+++ b/src/conf/schemas/domainsnapshot.rng
@@ -60,6 +60,13 @@
+
+
+
+
+
+
+
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index ac5aba1753..f6725c0e7b 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -376,6 +376,22 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
return NULL;
}
+if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) {
+g_autofree xmlNodePtr *revertDiskNodes = NULL;
+
+if ((n = virXPathNodeSet("./revertDisks/*", ctxt, )) <
0)
+return NULL;
+if (n)
+def->revertdisks = g_new0(virDomainSnapshotDiskDef, n);
+def->nrevertdisks = n;
+for (i = 0; i < def->nrevertdisks; i++) {
+if (virDomainSnapshotDiskDefParseXML(revertDiskNodes[i], ctxt,
+ >revertdisks[i],
+ flags, xmlopt) < 0)
+return NULL;
+}
+}
+
if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL) {
int active;
@@ -834,6 +850,17 @@ virDomainSnapshotDefFormatInternal(virBuffer *buf,
virBufferAddLit(buf, "\n");
}
+if (def->nrevertdisks > 0) {
+g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
+
+for (i = 0; i < def->nrevertdisks; i++) {
+if (virDomainSnapshotDiskDefFormat(,
>revertdisks[i], xmlopt) < 0)
+return -1;
+}
+
+virXMLFormatElement(buf, "revertDisks", NULL, );
+}
+
if (def->parent.dom) {
if (virDomainDefFormatInternal(def->parent.dom, xmlopt,
buf, domainflags) < 0)
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index ad49990a1e..ab76af604a 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -80,6 +80,11 @@ struct _virDomainSnapshotDef {
size_t ndisks; /* should not exceed dom->ndisks */
virDomainSnapshotDiskDef *disks;
+/* When we revert to non-leaf snapshot we need to
+ * store the new overlay disks. */
+size_t nrevertdisks;
+virDomainSnapshotDiskDef *revertdisks;
+
virObject *cookie;
};
--
2.41.0
Re: [PATCH] ci: Regenerate files
On Thu, Apr 27, 2023 at 11:44:30AM +0200, Michal Privoznik wrote:
> This removes minor version number from OpenSUSE LEAP target names
> and on CentOS Stream 9 installs flake8 from repositories, instead
> of pip.
>
> Signed-off-by: Michal Privoznik
> ---
> ci/buildenv/centos-stream-9.sh | 5 +
> .../{opensuse-leap-154.sh => opensuse-leap-15.sh} | 0
> ci/containers/centos-stream-9.Dockerfile | 6 +-
> ...leap-154.Dockerfile => opensuse-leap-15.Dockerfile} | 0
> ci/gitlab/builds.yml | 10 +-
> ci/gitlab/containers.yml | 4 ++--
> ci/manifest.yml| 2 +-
> 7 files changed, 10 insertions(+), 17 deletions(-)
> rename ci/buildenv/{opensuse-leap-154.sh => opensuse-leap-15.sh} (100%)
> rename ci/containers/{opensuse-leap-154.Dockerfile =>
> opensuse-leap-15.Dockerfile} (100%)
Reviewed-by: Pavel Hrdina
signature.asc
Description: PGP signature
Re: [libvirt PATCH 0/4] Various cleanups
On Thu, Apr 20, 2023 at 04:02:12PM +0200, Ján Tomko wrote: > Found by cppcheck. > > Ján Tomko (4): > conf: domain: remove unreachable break > qemu: command: join two adjacent conditions > ch: pinVcpuLive: remove unused variable > conf: storage: remove redundant condition Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
