subject:"\[libvirt\] \[PATCH 2\/7\] qemu\: Use active and inactive snapshot configuration on restore"

Re: [libvirt] [PATCH 2/7] qemu: Use active and inactive snapshot configuration on restore

2017-12-07 Thread John Ferlan



On 10/30/2017 04:51 AM, Kothapally Madhu Pavan wrote:
> By default, active and inactive XMl snapshot configurations are

*XML

> assigned to domain definition. This will make sure that all the
> non-persistent configurations of the snapshot are restored back
> as it is. This patch will also make sure that user has a choice

s/it is/they were/

??

> to choose of using active XML configuration of snapshot as both
> active and inactive XML configurations of the restoring domain.
> 
> Signed-off-by: Kothapally Madhu Pavan 
> ---
>  include/libvirt/libvirt-domain-snapshot.h | 10 +++---
>  src/qemu/qemu_driver.c| 20 +++-
>  2 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/include/libvirt/libvirt-domain-snapshot.h 
> b/include/libvirt/libvirt-domain-snapshot.h
> index 0f73f24..67ccb59 100644
> --- a/include/libvirt/libvirt-domain-snapshot.h
> +++ b/include/libvirt/libvirt-domain-snapshot.h
> @@ -184,9 +184,13 @@ int virDomainSnapshotHasMetadata(virDomainSnapshotPtr 
> snapshot,
>   unsigned int flags);
>  
>  typedef enum {
> -VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING = 1 << 0, /* Run after revert */
> -VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED  = 1 << 1, /* Pause after revert */
> -VIR_DOMAIN_SNAPSHOT_REVERT_FORCE   = 1 << 2, /* Allow risky reverts */
> +VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING   = 1 << 0, /* Run after revert */
> +VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED= 1 << 1, /* Pause after revert 
> */
> +VIR_DOMAIN_SNAPSHOT_REVERT_FORCE = 1 << 2, /* Allow risky 
> reverts */
> +VIR_DOMAIN_SNAPSHOT_REVERT_ACTIVE_ONLY   = 1 << 3, /* Use active snapshot
> +  configurations as 
> both
> +  active and inactive
> +  domain 
> configurations*/

Previous def only had 1 space between longest line of "RUNNING" and "=";
this has 2 spaces between "ONLY" and "=".  Just go with 1.

Also the tailing "*/" should gain a space.

The comment doesn't quite make sense though related to the name used for
the enum. I'm having trouble understanding the meaning of "as both
active and inactive" as it relates to REVERT_ACTIVE_ONLY.

I think you're trying to indicate that by setting this flag, when
reverting from the snapshot, that only the "saved" active definition
would be restored and if there was a saved persistent one, then it would
be skipped.  IOW: "VIR_DOMAIN_SNAPSHOT_REVERT_SKIP_PERSISTENT"


>  } virDomainSnapshotRevertFlags;
>  
>  /* Revert the domain to a point-in-time snapshot.  The
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 4ffec70..aecfcff 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -15577,6 +15577,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
> snapshot,
>  qemuDomainObjPrivatePtr priv;
>  int rc;
>  virDomainDefPtr config = NULL;
> +virDomainDefPtr newConfig = NULL;

persistentConfig?

>  virQEMUDriverConfigPtr cfg = NULL;
>  virCapsPtr caps = NULL;
>  bool was_running = false;
> @@ -15586,7 +15587,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
> snapshot,
>  
>  virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
>VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED |
> -  VIR_DOMAIN_SNAPSHOT_REVERT_FORCE, -1);
> +  VIR_DOMAIN_SNAPSHOT_REVERT_FORCE |
> +  VIR_DOMAIN_SNAPSHOT_REVERT_ACTIVE_ONLY, -1);
>  
>  /* We have the following transitions, which create the following events:
>   * 1. inactive -> inactive: none
> @@ -15688,6 +15690,16 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
> snapshot,
>  goto endjob;
>  }
>  

This is why I ended up writing that long comment in patch 1.

> +/* Prepare to copy snapshot inactive xml as inactive configuration
> + * of this domain unless user exclusively specify not to copy it */
> +if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_ACTIVE_ONLY) &&
> +snap->def->newDom) {
> +newConfig = virDomainDefCopy(snap->def->newDom, caps,
> + driver->xmlopt, NULL, true);
> +if (!newConfig)
> +goto endjob;
> +}
> +
>  cookie = (qemuDomainSaveCookiePtr) snap->def->cookie;
>  
>  switch ((virDomainState) snap->def->state) {
> @@ -15785,12 +15797,16 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
> snapshot,
>  virCPUDefFree(priv->origCPU);
>  VIR_STEAL_PTR(priv->origCPU, origCPU);
>  }
> +if (newConfig)
> +vm->newDef = newConfig;
>  } else {
>  /* Transitions 2, 3 */
>  load:
>  was_stopped = true;
>  if (config)
>  virDomainObjAssignDef(vm, config, false, NULL);
> +if

[libvirt] [PATCH 2/7] qemu: Use active and inactive snapshot configuration on restore

2017-10-30 Thread Kothapally Madhu Pavan

By default, active and inactive XMl snapshot configurations are
assigned to domain definition. This will make sure that all the
non-persistent configurations of the snapshot are restored back
as it is. This patch will also make sure that user has a choice
to choose of using active XML configuration of snapshot as both
active and inactive XML configurations of the restoring domain.

Signed-off-by: Kothapally Madhu Pavan 
---
 include/libvirt/libvirt-domain-snapshot.h | 10 +++---
 src/qemu/qemu_driver.c| 20 +++-
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/include/libvirt/libvirt-domain-snapshot.h 
b/include/libvirt/libvirt-domain-snapshot.h
index 0f73f24..67ccb59 100644
--- a/include/libvirt/libvirt-domain-snapshot.h
+++ b/include/libvirt/libvirt-domain-snapshot.h
@@ -184,9 +184,13 @@ int virDomainSnapshotHasMetadata(virDomainSnapshotPtr 
snapshot,
  unsigned int flags);
 
 typedef enum {
-VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING = 1 << 0, /* Run after revert */
-VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED  = 1 << 1, /* Pause after revert */
-VIR_DOMAIN_SNAPSHOT_REVERT_FORCE   = 1 << 2, /* Allow risky reverts */
+VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING   = 1 << 0, /* Run after revert */
+VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED= 1 << 1, /* Pause after revert */
+VIR_DOMAIN_SNAPSHOT_REVERT_FORCE = 1 << 2, /* Allow risky reverts 
*/
+VIR_DOMAIN_SNAPSHOT_REVERT_ACTIVE_ONLY   = 1 << 3, /* Use active snapshot
+  configurations as 
both
+  active and inactive
+  domain 
configurations*/
 } virDomainSnapshotRevertFlags;
 
 /* Revert the domain to a point-in-time snapshot.  The
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4ffec70..aecfcff 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15577,6 +15577,7 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 qemuDomainObjPrivatePtr priv;
 int rc;
 virDomainDefPtr config = NULL;
+virDomainDefPtr newConfig = NULL;
 virQEMUDriverConfigPtr cfg = NULL;
 virCapsPtr caps = NULL;
 bool was_running = false;
@@ -15586,7 +15587,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 
 virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
   VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED |
-  VIR_DOMAIN_SNAPSHOT_REVERT_FORCE, -1);
+  VIR_DOMAIN_SNAPSHOT_REVERT_FORCE |
+  VIR_DOMAIN_SNAPSHOT_REVERT_ACTIVE_ONLY, -1);
 
 /* We have the following transitions, which create the following events:
  * 1. inactive -> inactive: none
@@ -15688,6 +15690,16 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 goto endjob;
 }
 
+/* Prepare to copy snapshot inactive xml as inactive configuration
+ * of this domain unless user exclusively specify not to copy it */
+if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_ACTIVE_ONLY) &&
+snap->def->newDom) {
+newConfig = virDomainDefCopy(snap->def->newDom, caps,
+ driver->xmlopt, NULL, true);
+if (!newConfig)
+goto endjob;
+}
+
 cookie = (qemuDomainSaveCookiePtr) snap->def->cookie;
 
 switch ((virDomainState) snap->def->state) {
@@ -15785,12 +15797,16 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 virCPUDefFree(priv->origCPU);
 VIR_STEAL_PTR(priv->origCPU, origCPU);
 }
+if (newConfig)
+vm->newDef = newConfig;
 } else {
 /* Transitions 2, 3 */
 load:
 was_stopped = true;
 if (config)
 virDomainObjAssignDef(vm, config, false, NULL);
+if (newConfig)
+vm->newDef = newConfig;
 
 /* No cookie means libvirt which saved the domain was too old to
  * mess up the CPU definitions.
@@ -15884,6 +15900,8 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 }
 if (config)
 virDomainObjAssignDef(vm, config, false, NULL);
+if (newConfig)
+vm->newDef = newConfig;
 
 if (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
  VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/7] qemu: Use active and inactive snapshot configuration on restore

[libvirt] [PATCH 2/7] qemu: Use active and inactive snapshot configuration on restore

2 matches

Site Navigation

Mail list logo

Footer information