[pve-devel] [PATCH v7 pve-storage 08/10] Implement support for snapshots

[Fabian Grünbichler]

On Tue, Jun 20, 2017 at 10:40:00PM +0200, mir at datanom.net wrote:
> From: Michael Rasmussen <mir at datanom.net>
> 
> Signed-off-by: Michael Rasmussen <mir at datanom.net>
> ---
>  PVE/Storage/FreeNASPlugin.pm | 72 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 71 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/Storage/FreeNASPlugin.pm b/PVE/Storage/FreeNASPlugin.pm
> index 9c0136d..f954dc8 100644
> --- a/PVE/Storage/FreeNASPlugin.pm
> +++ b/PVE/Storage/FreeNASPlugin.pm
> @@ -848,6 +848,24 @@ my $rescan_session = sub {
>      $delete_session->($scfg, $sid) unless %$luns_to_keep;
>  };
>  

whitespace

> +my $freenas_get_latest_snapshot = sub {
> +    my ($class, $scfg, $volname) = @_;
> +
> +    my (undef, $vname) = $class->parse_volname($volname);
> +
> +    # abort rollback if snapshot is not the latest
> +    my $snapshots = $freenas_request->($scfg, 'GET', "storage/snapshot/");
> +    

whitespace

> +    my $recentsnap;
> +    foreach my $snapshot (@$snapshots) {
> +        next unless $snapshot->{filesystem} =~ /$scfg->{pool}\/$vname/ && $snapshot->{mostrecent};
> +        $recentsnap = $snapshot->{name};
> +        last;
> +    }
> +
> +    return $recentsnap;

foreach
  return $snapshot->{name} if .... ;

return undef;

> +};
> +
>  # Storage implementation
>  
>  sub volume_size_info {
> @@ -1113,7 +1131,7 @@ sub volume_snapshot {
>          dataset => "$scfg->{pool}/$vname",
>          name => $snap,
>      };
> -    $freenas_request->($scfg, 'POST', "storage/snapshot/", encode_json($data));    
> +    $freenas_request->($scfg, 'POST', "storage/snapshot/", encode_json($data));
>  }
>  

whitespace

>  sub volume_snapshot_delete {
> @@ -1121,12 +1139,58 @@ sub volume_snapshot_delete {
>      

whitespace

>      my (undef, $vname, $vmid) = $class->parse_volname($volname);
>  

whitespace

> +    if ($snap eq 'vzdump') {
> +        eval {
> +            my $target = $freenas_get_target->($scfg, $vmid);
> +            die "volume_snapshot_delete-> missing target\n" unless $target;
> +            my $extent = $freenas_get_extent->($scfg, "$vname\@$snap");
> +            die "volume_snapshot_delete-> missing extent\n" unless $extent;
> +            my $tg2exent = $freenas_get_target_to_exent->($scfg, $extent, $target);
> +            die "volume_snapshot_delete-> missing target to extent\n" unless $tg2exent;
> +            my $lun = $freenas_get_lun_number->($scfg, "$vname\@$snap");
> +            die "volume_snapshot_delete-> missing LUN\n" unless defined $lun;

$lun is not used?

> +            $freenas_delete_target_to_exent->($scfg, $tg2exent);
> +            $freenas_delete_extent->($scfg, $extent);
> +            $class->deactivate_volume($storeid, $scfg, "$vname\@$snap");
> +        };
> +        warn "$@ - unable to deactivate snapshot from remote FreeNAS storage" if $@;

we usually do warnings the other way round (with $@ at the end).

> +    }
> +
> +    $freenas_request->($scfg, 'DELETE', "storage/snapshot/$scfg->{pool}/$vname\@$snap/");
>  }
>  

whitespace

>  sub volume_snapshot_rollback {
>      my ($class, $scfg, $storeid, $volname, $snap) = @_;
>  

whitespace

>      my ($vtype, $name, $vmid) = $class->parse_volname($volname);
> +
> +    my $target = $freenas_get_target->($scfg, $vmid);
> +    die "volume_resize-> Missing target\n" unless $target;
> +    my $extent = $freenas_get_extent->($scfg, $name);
> +    die "volume_resize-> Missing extent\n" unless $extent;
> +    my $tg2exent = $freenas_get_target_to_exent->($scfg, $extent, $target);
> +    die "volume_resize-> Missing target to extent\n" unless $tg2exent;
> +    my $lunid = $freenas_get_lun_number->($scfg, $name);
> +    die "volume_resize-> Missing LUN\n" unless defined $lunid;

these all contain volume_resize? maybe it would make sense to factor
this sequence of events into its own helper, you use it a lot..

> +
> +    eval {
> +        $freenas_delete_target_to_exent->($scfg, $tg2exent);
> +        $freenas_delete_extent->($scfg, $extent);
> +    };
> +    warn "Failed to remove current extent. Trying to proceed anyway: $@\n" if $@;
> +    

whitespace

> +    my $data = {
> +        force => JSON::false,
> +    };
> +    $freenas_request->(
> +        $scfg, 'POST', "storage/snapshot/$scfg->{pool}/$name\@$snap/rollback/", encode_json($data));
> +
> +    eval {
> +        $extent = $freenas_create_extent->($scfg, $name);
> +        $freenas_create_target_to_exent->($scfg, $target, $extent, $lunid);
> +        $rescan_session->($class, $storeid, $scfg, $name);
> +    };
> +    die "Rollback failed: $@\n" if $@;
>  }
>  
>  sub volume_rollback_is_possible {
> @@ -1134,6 +1198,12 @@ sub volume_rollback_is_possible {
>      

whitespace

>      my (undef, $name) = $class->parse_volname($volname);
>  
> +    my $recentsnap = $freenas_get_latest_snapshot->($class, $scfg, $name);
> +    if ($snap ne $recentsnap) {
> +        die "can't rollback, more recent snapshots exist\n";
> +    }
> +
> +    return 1; 

whitespace

>  }
>  
>  sub volume_snapshot_list {
> -- 
> 2.11.0
> 
> 
> ----
> 
> This mail was virus scanned and spam checked before delivery.
> This mail is also DKIM signed. See header dkim-signature.
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel