[pve-devel] [PATCH v3 container guest-common 0/2] snapshot parent checks for containers

[pve-devel] [PATCH v3 container guest-common 0/2] snapshot parent checks for containers

[Oguz Bektas]

v2->v3:
* automatically delete the 'parent' property for an existing snapshot
(instead of aborting) if its the same as the new snapshot name (and the
snapshot referenced by 'parent' is not used)

pve-container:
Oguz Bektas (1):
  api: clone_vm: don't include snapshot properties

 src/PVE/API2/LXC.pm | 5 +++++
 1 file changed, 5 insertions(+)

pve-guest-common:
Oguz Bektas (1):
  snapshots: delete parent property if new snapshot name is already a
    parent to existing one

 src/PVE/AbstractConfig.pm | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH v3 container 1/2] api: clone_vm: don't include snapshot properties

[Oguz Bektas]

apparently this caused a weird[0] bug... when a container with a snapshot was
cloned, it would take 'parent: foo' from the original container. if you
add a new snapshot 'bar', and then another one 'foo', this causes the
snapshots to become parents of each other (thus not parsed correctly in
the tree view of GUI nor with 'pct listsnapshot CTID')

we also drop these properties for VMs, so it makes sense to do the same
here as well.

[0]: https://forum.proxmox.com/threads/snapshots-of-one-lxc-disappeared.97711/

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
v2->v3:
* no changes


 src/PVE/API2/LXC.pm | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 1f2f1f0..05cfbad 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -1512,7 +1512,12 @@ __PACKAGE__->register_method({
 	    # Replace the 'disk' lock with a 'create' lock.
 	    $newconf->{lock} = 'create';
 
+	    # delete all snapshot related config options
 	    delete $newconf->{snapshots};
+	    delete $newconf->{parent};
+	    delete $newconf->{snaptime};
+	    delete $newconf->{snapstate};
+
 	    delete $newconf->{pending};
 	    delete $newconf->{template};
 	    if ($param->{hostname}) {
-- 
2.30.2

[pve-devel] [PATCH v3 guest-common 2/2] snapshots: delete parent property if new snapshot name is already a parent to existing one

[Oguz Bektas]

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
v2->v3:
* automatically delete the 'parent' property for an existing snapshot
(instead of aborting) if its the same as the new snapshot name (and the
snapshot referenced by 'parent' is not used)



 src/PVE/AbstractConfig.pm | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/PVE/AbstractConfig.pm b/src/PVE/AbstractConfig.pm
index 3348d8a..e4ddeab 100644
--- a/src/PVE/AbstractConfig.pm
+++ b/src/PVE/AbstractConfig.pm
@@ -721,14 +721,21 @@ sub __snapshot_prepare {
 
 	$conf->{lock} = 'snapshot';
 
+	my $snapshots = $conf->{snapshots};
+
 	die "snapshot name '$snapname' already used\n"
-	    if defined($conf->{snapshots}->{$snapname});
+	    if defined($snapshots->{$snapname});
 
 	my $storecfg = PVE::Storage::config();
 	die "snapshot feature is not available\n"
 	    if !$class->has_feature('snapshot', $conf, $storecfg, undef, undef, $snapname eq 'vzdump');
 
-	$snap = $conf->{snapshots}->{$snapname} = {};
+	foreach my $existing_snapshot (keys %$snapshots) {
+	    my $parent_name = $snapshots->{$existing_snapshot}->{parent} // '';
+	    delete $snapshots->{$existing_snapshot}->{parent} if $snapname eq $parent_name;
+	}
+
+	$snap = $snapshots->{$snapname} = {};
 
 	if ($save_vmstate && $class->__snapshot_check_running($vmid)) {
 	    $class->__snapshot_save_vmstate($vmid, $conf, $snapname, $storecfg);
-- 
2.30.2

Re: [pve-devel] [PATCH v3 guest-common 2/2] snapshots: delete parent property if new snapshot name is already a parent to existing one

[Oguz Bektas]

hi,

was this patch missed?
or is there a problem with it?
the fix for containers was applied but without this one.

thanks

On Thu, Oct 14, 2021 at 11:29:57AM +0200, Oguz Bektas wrote:
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
> v2->v3:
> * automatically delete the 'parent' property for an existing snapshot
> (instead of aborting) if its the same as the new snapshot name (and the
> snapshot referenced by 'parent' is not used)
> 
> 
> 
>  src/PVE/AbstractConfig.pm | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/AbstractConfig.pm b/src/PVE/AbstractConfig.pm
> index 3348d8a..e4ddeab 100644
> --- a/src/PVE/AbstractConfig.pm
> +++ b/src/PVE/AbstractConfig.pm
> @@ -721,14 +721,21 @@ sub __snapshot_prepare {
>  
>  	$conf->{lock} = 'snapshot';
>  
> +	my $snapshots = $conf->{snapshots};
> +
>  	die "snapshot name '$snapname' already used\n"
> -	    if defined($conf->{snapshots}->{$snapname});
> +	    if defined($snapshots->{$snapname});
>  
>  	my $storecfg = PVE::Storage::config();
>  	die "snapshot feature is not available\n"
>  	    if !$class->has_feature('snapshot', $conf, $storecfg, undef, undef, $snapname eq 'vzdump');
>  
> -	$snap = $conf->{snapshots}->{$snapname} = {};
> +	foreach my $existing_snapshot (keys %$snapshots) {
> +	    my $parent_name = $snapshots->{$existing_snapshot}->{parent} // '';
> +	    delete $snapshots->{$existing_snapshot}->{parent} if $snapname eq $parent_name;
> +	}
> +
> +	$snap = $snapshots->{$snapname} = {};
>  
>  	if ($save_vmstate && $class->__snapshot_check_running($vmid)) {
>  	    $class->__snapshot_save_vmstate($vmid, $conf, $snapname, $storecfg);
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 

Re: [pve-devel] [PATCH v3 guest-common 2/2] snapshots: delete parent property if new snapshot name is already a parent to existing one

[Hannes Laimer]

Tested-by: Hannes Laimer <h.laimer@proxmox.com>

Works as advertised, only affects old configs created before the first
patch was applied.

Am 14.10.21 um 11:29 schrieb Oguz Bektas:
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
> v2->v3:
> * automatically delete the 'parent' property for an existing snapshot
> (instead of aborting) if its the same as the new snapshot name (and the
> snapshot referenced by 'parent' is not used)
> 
> 
> 
>  src/PVE/AbstractConfig.pm | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/AbstractConfig.pm b/src/PVE/AbstractConfig.pm
> index 3348d8a..e4ddeab 100644
> --- a/src/PVE/AbstractConfig.pm
> +++ b/src/PVE/AbstractConfig.pm
> @@ -721,14 +721,21 @@ sub __snapshot_prepare {
>  
>  	$conf->{lock} = 'snapshot';
>  
> +	my $snapshots = $conf->{snapshots};
> +
>  	die "snapshot name '$snapname' already used\n"
> -	    if defined($conf->{snapshots}->{$snapname});
> +	    if defined($snapshots->{$snapname});
>  
>  	my $storecfg = PVE::Storage::config();
>  	die "snapshot feature is not available\n"
>  	    if !$class->has_feature('snapshot', $conf, $storecfg, undef, undef, $snapname eq 'vzdump');
>  
> -	$snap = $conf->{snapshots}->{$snapname} = {};
> +	foreach my $existing_snapshot (keys %$snapshots) {
> +	    my $parent_name = $snapshots->{$existing_snapshot}->{parent} // '';
> +	    delete $snapshots->{$existing_snapshot}->{parent} if $snapname eq $parent_name;
> +	}
> +
> +	$snap = $snapshots->{$snapname} = {};
>  
>  	if ($save_vmstate && $class->__snapshot_check_running($vmid)) {
>  	    $class->__snapshot_save_vmstate($vmid, $conf, $snapname, $storecfg);
> 

[pve-devel] applied: [PATCH v3 guest-common 2/2] snapshots: delete parent property if new snapshot name is already a parent to existing one

[Thomas Lamprecht]

On 29.11.21 13:31, Hannes Laimer wrote:
> Tested-by: Hannes Laimer <h.laimer@proxmox.com>
> 

with above applied, thanks!