[pve-devel] [PATCH v7 pve-storage 09/10] Implement support for linked clones.

[Fabian Grünbichler]

this patch has a wrong title? it implements create_base, not linked
clones.. and should therefor be before patch #7, which implements linked
clones..

On Tue, Jun 20, 2017 at 10:40:01PM +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 | 56 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/PVE/Storage/FreeNASPlugin.pm b/PVE/Storage/FreeNASPlugin.pm
> index f954dc8..bfd3ee8 100644
> --- a/PVE/Storage/FreeNASPlugin.pm
> +++ b/PVE/Storage/FreeNASPlugin.pm
> @@ -976,6 +976,62 @@ sub create_base {
>  
>      die "create_base not possible with base image\n" if $isBase;
>  
> +    my $newname = $name;
> +    $newname =~ s/^vm-/base-/;
> +    # Check for existing base
> +    my $base;
> +    eval {
> +        $base = $freenas_get_extent->($scfg, $newname);
> +    };
> +    warn "Check for existing base failed: $@\n" if $@;
> +    die "$newname: Base already exists\n" if $base;
> +    

whitespace

> +    my $target = $freenas_get_target->($scfg, $vmid);
> +    die "create_base-> missing target\n" unless $target;
> +    my $extent = $freenas_get_extent->($scfg, $name);
> +    die "create_base-> missing extent\n" unless $extent;
> +    my $tg2exent = $freenas_get_target_to_exent->($scfg, $extent, $target);
> +    die "create_base-> missing target to extent\n" unless $tg2exent;
> +    my $lun = $freenas_get_lun_number->($scfg, $name);
> +    die "create_base-> missing LUN\n" unless defined $lun;
> +
> +    # if disable access to base vm fails bail

s/disable/disabling/

and maybe moe the bail to the beginning of the comment?

> +    eval {
> +        $freenas_delete_target_to_exent->($scfg, $tg2exent);
> +        $freenas_delete_extent->($scfg, $extent);
> +    };
> +    die "Could not convert '$name' to '$newname': $@\n" if $@;
> +
> +    my $sid = $get_sid->($scfg, $name);
> +    if ($sid >= 0) {
> +        my $lid = $build_lun_list->($scfg, $sid, $lun);

what is lid supposed to mean? please use consistent naming..

> +        if ($lid && $lid->{$lun}) {
> +            $remove_local_lun->($lid->{$lun});
> +        }
> +    }
> +
> +    eval {
> +        # FreeNAS API does not support renaming a zvol so create a snapshot
> +        # and make a clone of the snapshot instead
> +        $class->volume_snapshot($scfg, $storeid, $name, $snap);
> +    

whitespace

> +        my $data = {
> +            name => "$scfg->{pool}/$newname"
> +        };
> +        $freenas_request->(
> +            $scfg, 'POST', "storage/snapshot/$scfg->{pool}/$name\@$snap/clone/", encode_json($data));
> +
> +        $freenas_create_lun->($scfg, $vmid, $newname);
> +    };
> +    if ($@) {
> +        # if creating base fails restore previous state

but wouldn't that also include removing the LUN, clone and snapshot?

> +        $extent = $freenas_create_extent->($scfg, $name);
> +        die "create_base-> Could not create extent for VM '$vmid'\n" unless $extent;
> +        $tg2exent = $freenas_create_target_to_exent->($scfg, $target, $extent, $lun);
> +        die "create_base-> Could not create target to extend for VM '$vmid'\n" unless defined $tg2exent;

aren't extents per volume, not per VM (this is in other places as well
btw..)?

also, unless ;)

> +    }
> +
> +    return $newname;
>  }
>  
>  sub clone_image {
> -- 
> 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