[pve-devel] [PATCH 2/7] Storage Plugins: extend clone_image by snap parameter and add support to RBDPlugin

[Stefan Priebe - Profihost AG]

Am 07.02.2014 12:59, schrieb Alexandre DERUMIER:
> why use 
> 
> if length $snapname;
> 
> for tests ?
> 
> 
> "if $snapname" should be enough.  

If $snapname is "0mysnap" it isn't as perl would interpret this as false

> 
> 
> 
> ----- Mail original ----- 
> 
> De: "Stefan Priebe" <s.priebe at profihost.ag> 
> À: pve-devel at pve.proxmox.com 
> Envoyé: Vendredi 7 Février 2014 12:16:33 
> Objet: [pve-devel] [PATCH 2/7] Storage Plugins: extend clone_image by snap parameter and add support to RBDPlugin 
> 
> 
> Signed-off-by: Stefan Priebe <s.priebe at profihost.ag> 
> --- 
> PVE/Storage/ISCSIDirectPlugin.pm | 2 +- 
> PVE/Storage/ISCSIPlugin.pm | 2 +- 
> PVE/Storage/LVMPlugin.pm | 2 +- 
> PVE/Storage/Plugin.pm | 6 ++++-- 
> PVE/Storage/RBDPlugin.pm | 17 ++++++++++++++--- 
> PVE/Storage/SheepdogPlugin.pm | 4 ++-- 
> PVE/Storage/ZFSPlugin.pm | 4 ++-- 
> 7 files changed, 25 insertions(+), 12 deletions(-) 
> 
> diff --git a/PVE/Storage/ISCSIDirectPlugin.pm b/PVE/Storage/ISCSIDirectPlugin.pm 
> index cc6a8c3..c957ade 100644 
> --- a/PVE/Storage/ISCSIDirectPlugin.pm 
> +++ b/PVE/Storage/ISCSIDirectPlugin.pm 
> @@ -104,7 +104,7 @@ sub create_base { 
> } 
> 
> sub clone_image { 
> - my ($class, $scfg, $storeid, $volname, $vmid) = @_; 
> + my ($class, $scfg, $storeid, $volname, $vmid, $snap) = @_; 
> 
> die "can't clone images in iscsi storage\n"; 
> } 
> diff --git a/PVE/Storage/ISCSIPlugin.pm b/PVE/Storage/ISCSIPlugin.pm 
> index 0fe8d69..e5bb7bb 100644 
> --- a/PVE/Storage/ISCSIPlugin.pm 
> +++ b/PVE/Storage/ISCSIPlugin.pm 
> @@ -294,7 +294,7 @@ sub create_base { 
> } 
> 
> sub clone_image { 
> - my ($class, $scfg, $storeid, $volname, $vmid) = @_; 
> + my ($class, $scfg, $storeid, $volname, $vmid, $snap) = @_; 
> 
> die "can't clone images in iscsi storage\n"; 
> } 
> diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm 
> index ff2a6ec..cbd4039 100644 
> --- a/PVE/Storage/LVMPlugin.pm 
> +++ b/PVE/Storage/LVMPlugin.pm 
> @@ -241,7 +241,7 @@ sub create_base { 
> } 
> 
> sub clone_image { 
> - my ($class, $scfg, $storeid, $volname, $vmid) = @_; 
> + my ($class, $scfg, $storeid, $volname, $vmid, $snap) = @_; 
> 
> die "can't clone images in lvm storage\n"; 
> } 
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm 
> index a80aba4..15c23d4 100644 
> --- a/PVE/Storage/Plugin.pm 
> +++ b/PVE/Storage/Plugin.pm 
> @@ -487,7 +487,7 @@ my $find_free_diskname = sub { 
> }; 
> 
> sub clone_image { 
> - my ($class, $scfg, $storeid, $volname, $vmid) = @_; 
> + my ($class, $scfg, $storeid, $volname, $vmid, $snap) = @_; 
> 
> # this only works for file based storage types 
> die "storage definintion has no path\n" if !$scfg->{path}; 
> @@ -497,7 +497,9 @@ sub clone_image { 
> 
> die "clone_image on wrong vtype '$vtype'\n" if $vtype ne 'images'; 
> 
> - die "clone_image onyl works on base images\n" if !$isBase; 
> + die "this storage type does not support clone_image on snapshot\n" if $snap; 
> + 
> + die "clone_image only works on base images\n" if !$isBase; 
> 
> my $imagedir = $class->get_subdir($scfg, 'images'); 
> $imagedir .= "/$vmid"; 
> diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm 
> index f15f1fc..7e67fd8 100644 
> --- a/PVE/Storage/RBDPlugin.pm 
> +++ b/PVE/Storage/RBDPlugin.pm 
> @@ -311,20 +311,31 @@ sub create_base { 
> } 
> 
> sub clone_image { 
> - my ($class, $scfg, $storeid, $volname, $vmid) = @_; 
> + my ($class, $scfg, $storeid, $volname, $vmid, $snapname) = @_; 
> 
> my $snap = '__base__'; 
> + $snap = $snapname if length $snapname; 
> 
> my ($vtype, $basename, $basevmid, undef, undef, $isBase) = 
> $class->parse_volname($volname); 
> 
> - die "clone_image onyl works on base images\n" if !$isBase; 
> + die "$volname is not a base image and snapname is not provided\n" if !$isBase && !length $snapname; 
> 
> my $name = &$find_free_diskname($storeid, $scfg, $vmid); 
> 
> - warn "clone $volname: $basename to $name\n"; 
> + warn "clone $volname: $basename snapname $snap to $name\n"; 
> + 
> + if (length $snapname) { 
> + my (undef, undef, undef, $protected) = rbd_volume_info($scfg, $storeid, $volname, $snapname); 
> + 
> + if (!$protected){ 
> + my $cmd = &$rbd_cmd($scfg, $storeid, 'snap', 'protect', $volname, '--snap', $snapname); 
> + run_command($cmd, errmsg => "rbd protect $volname snap $snapname error"); 
> + } 
> + } 
> 
> my $newvol = "$basename/$name"; 
> + $newvol = $name if length $snapname; 
> 
> my $cmd = &$rbd_cmd($scfg, $storeid, 'clone', &$add_pool_to_disk($scfg, $basename), '--snap', $snap, &$add_pool_to_disk($scfg, $name)); 
> run_command($cmd, errmsg => "rbd clone $basename' error"); 
> diff --git a/PVE/Storage/SheepdogPlugin.pm b/PVE/Storage/SheepdogPlugin.pm 
> index 3bef2a8..8e1ca0c 100644 
> --- a/PVE/Storage/SheepdogPlugin.pm 
> +++ b/PVE/Storage/SheepdogPlugin.pm 
> @@ -215,9 +215,9 @@ sub create_base { 
> } 
> 
> sub clone_image { 
> - my ($class, $scfg, $storeid, $volname, $vmid) = @_; 
> + my ($class, $scfg, $storeid, $volname, $vmid, $snap) = @_; 
> 
> - my $snap = '__base__'; 
> + $snap ||= '__base__'; 
> 
> my ($vtype, $basename, $basevmid, undef, undef, $isBase) = 
> $class->parse_volname($volname); 
> diff --git a/PVE/Storage/ZFSPlugin.pm b/PVE/Storage/ZFSPlugin.pm 
> index d077cfb..e25c4db 100644 
> --- a/PVE/Storage/ZFSPlugin.pm 
> +++ b/PVE/Storage/ZFSPlugin.pm 
> @@ -422,9 +422,9 @@ sub create_base { 
> } 
> 
> sub clone_image { 
> - my ($class, $scfg, $storeid, $volname, $vmid) = @_; 
> + my ($class, $scfg, $storeid, $volname, $vmid, $snap) = @_; 
> 
> - my $snap = '__base__'; 
> + $snap ||= '__base__'; 
> 
> my ($vtype, $basename, $basevmid, undef, undef, $isBase) = 
> $class->parse_volname($volname); 
>