Re: [pve-devel] [zsync] fix: check for incremental sync snapshot.
Hi,
this does fix an issue when the receiving side has the most recent
snapshot, but not the 'old_snap' one. And of course testing for
'last_snap' is correct, since that one is the relevant one for the
incremental sync. With that:
Reviewed-By: Fabian Ebner
Tested-By: Fabian Ebner
Some ideas for further improvements:
* If on the destination there are older snapshots but not most recent
one, pve-zsync tries to do a full sync and fails with: "destination has
snapshots". One could get the list of snapshots from the destination,
the list of snapshots from the source and use the most recent common one
as the starting point for an incremental sync (and fail if both sides do
have snapshots but no match).
* Knowing the list of snapshots for the destination could also be used
to prevent issuing a useless remote 'zfs destroy' when the snapshot to
be deleted does not exist for the destination.
On 18.03.20 07:51, Wolfgang Link wrote:
For an incremental sync you need the last_snap on both sides.
---
pve-zsync | 13 -
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/pve-zsync b/pve-zsync
index ea3178e..893baf0 100755
--- a/pve-zsync
+++ b/pve-zsync
@@ -931,6 +931,7 @@ sub snapshot_destroy {
}
}
+# check if snapshot for incremental sync exist on dest side
sub snapshot_exist {
my ($source , $dest, $method, $dest_user) = @_;
@@ -940,22 +941,16 @@ sub snapshot_exist {
my $path = $dest->{all};
$path .= "/$source->{last_part}" if $source->{last_part};
-$path .= "\@$source->{old_snap}";
+$path .= "\@$source->{last_snap}";
push @$cmd, $path;
-
-my $text = "";
-eval {$text =run_cmd($cmd);};
+eval {run_cmd($cmd)};
if (my $erro =$@) {
warn "WARN: $erro";
return undef;
}
-
-while ($text && $text =~ s/^(.*?)(\n|$)//) {
- my $line =$1;
- return 1 if $line =~ m/^.*$source->{old_snap}$/;
-}
+return 1;
}
sub send_image {
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [zsync] fix: check for incremental sync snapshot.
The call to the zfs list contains the snapshot.
If the snapshot does not exist, the command is returned with an error that we
are catching.
> On March 18, 2020 8:02 AM Dietmar Maurer wrote:
>
>
> Why does the patch ignore the output from the command?
>
> > On March 18, 2020 7:51 AM Wolfgang Link wrote:
> >
> >
> > For an incremental sync you need the last_snap on both sides.
> > ---
> > pve-zsync | 13 -
> > 1 file changed, 4 insertions(+), 9 deletions(-)
> >
> > diff --git a/pve-zsync b/pve-zsync
> > index ea3178e..893baf0 100755
> > --- a/pve-zsync
> > +++ b/pve-zsync
> > @@ -931,6 +931,7 @@ sub snapshot_destroy {
> > }
> > }
> >
> > +# check if snapshot for incremental sync exist on dest side
> > sub snapshot_exist {
> > my ($source , $dest, $method, $dest_user) = @_;
> >
> > @@ -940,22 +941,16 @@ sub snapshot_exist {
> >
> > my $path = $dest->{all};
> > $path .= "/$source->{last_part}" if $source->{last_part};
> > -$path .= "\@$source->{old_snap}";
> > +$path .= "\@$source->{last_snap}";
> >
> > push @$cmd, $path;
> >
> > -
> > -my $text = "";
> > -eval {$text =run_cmd($cmd);};
> > +eval {run_cmd($cmd)};
> > if (my $erro =$@) {
> > warn "WARN: $erro";
> > return undef;
> > }
> > -
> > -while ($text && $text =~ s/^(.*?)(\n|$)//) {
> > - my $line =$1;
> > - return 1 if $line =~ m/^.*$source->{old_snap}$/;
> > -}
> > +return 1;
> > }
> >
> > sub send_image {
> > --
> > 2.20.1
> >
> >
> > ___
> > pve-devel mailing list
> > pve-devel@pve.proxmox.com
> > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [zsync] fix: check for incremental sync snapshot.
Why does the patch ignore the output from the command?
> On March 18, 2020 7:51 AM Wolfgang Link wrote:
>
>
> For an incremental sync you need the last_snap on both sides.
> ---
> pve-zsync | 13 -
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/pve-zsync b/pve-zsync
> index ea3178e..893baf0 100755
> --- a/pve-zsync
> +++ b/pve-zsync
> @@ -931,6 +931,7 @@ sub snapshot_destroy {
> }
> }
>
> +# check if snapshot for incremental sync exist on dest side
> sub snapshot_exist {
> my ($source , $dest, $method, $dest_user) = @_;
>
> @@ -940,22 +941,16 @@ sub snapshot_exist {
>
> my $path = $dest->{all};
> $path .= "/$source->{last_part}" if $source->{last_part};
> -$path .= "\@$source->{old_snap}";
> +$path .= "\@$source->{last_snap}";
>
> push @$cmd, $path;
>
> -
> -my $text = "";
> -eval {$text =run_cmd($cmd);};
> +eval {run_cmd($cmd)};
> if (my $erro =$@) {
> warn "WARN: $erro";
> return undef;
> }
> -
> -while ($text && $text =~ s/^(.*?)(\n|$)//) {
> - my $line =$1;
> - return 1 if $line =~ m/^.*$source->{old_snap}$/;
> -}
> +return 1;
> }
>
> sub send_image {
> --
> 2.20.1
>
>
> ___
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
