Re: [pve-devel] [PATCH_V4 pve-container 5/5] Add full clone with running CT

2016-06-01 Thread Fabian Grünbichler
comments inline

> Wolfgang Link  hat am 1. Juni 2016 um 09:22 geschrieben:
> 
> 
> With this patch it is possible to make a full clone from an running container,
> if the underlying Storage provides snapshots.
> ---
>  src/PVE/API2/LXC.pm | 42 +-
>  1 file changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index 6fa1da0..c3b02c9 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -1087,7 +1087,8 @@ __PACKAGE__->register_method({
>  
>   my $clonefn = sub {
>  
> - die "Clone can't be done online\n." if  
> PVE::LXC::check_running($vmid);
> + my $running = PVE::LXC::check_running($vmid);
> +
>   # do all tests after lock
>   # we also try to do all tests before we fork the worker
>   my $conf = PVE::LXC::Config->load_config($vmid);
> @@ -1107,6 +1108,32 @@ __PACKAGE__->register_method({
>   my $mountpoints = {};
>   my $fullclone = {};
>   my $vollist = [];
> + my $clone_volumes = [];
> +
> + #create snapshot for full clone to minimize downtime
> + if ($running && !$snapname) {
> + $snapname = 'cloneCT';
> +
> + PVE::Tools::run_command(['/usr/bin/lxc-freeze', '-n', $vmid]);
> +
> + foreach my $opt (keys %$conf) {
> + my $value = $conf->{$opt};
> +
> + next if $opt !~ /rootfs|mp\d+/;
> +
> +  my $mp = $opt eq 'rootfs' ?
> + PVE::LXC::Config->parse_ct_rootfs($value) :
> + PVE::LXC::Config->parse_ct_mountpoint($value);
> +
> + die "For online copy a CT storage must provide snapshots!\n"
> + if !PVE::Storage::volume_has_feature($storecfg, 
> 'snapshot', $mp->{volume}, $snapname, 0);;
> +
> + PVE::Storage::volume_snapshot($storecfg, $mp->{volume}, 
> $snapname);
> + push @$clone_volumes, $mp->{volume};
> + }

why don't you use PVE::LXC::Config->snapshot_create() here instead of 
replicating its functionality? it does freezing and syncing and all the 
snapshot related checks..

> +
> + PVE::Tools::run_command(['/usr/bin/lxc-unfreeze', '-n', $vmid]);
> + }
>  
>   foreach my $opt (keys %$oldconf) {
>   my $value = $oldconf->{$opt};
> @@ -1189,6 +1216,19 @@ __PACKAGE__->register_method({
>  
>   PVE::AccessControl::add_vm_to_pool($newid, $pool) if $pool;
>   };
> +
> + if (defined($snapname) && $snapname eq 'cloneCT') {
> + foreach my $vol (@$clone_volumes) {
> +
> + eval {
> + PVE::Storage::volume_snapshot_delete($storecfg, 
> $vol, $snapname);
> + };
> + #warn only and remove all snapshots
> + if (my $err = $@) {
> + warn $err;
> + }
> + }
> + }

same as above, why don't you simply use PVE::LXC::Config->snapshot_delete() 
(with force)?

>   if (my $err = $@) {
>   unlink $conffile;
>  
> -- 
> 2.1.4
> 
> 
> ___
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>

___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


[pve-devel] [PATCH_V4 pve-container 5/5] Add full clone with running CT

2016-06-01 Thread Wolfgang Link
With this patch it is possible to make a full clone from an running container,
if the underlying Storage provides snapshots.
---
 src/PVE/API2/LXC.pm | 42 +-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 6fa1da0..c3b02c9 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -1087,7 +1087,8 @@ __PACKAGE__->register_method({
 
my $clonefn = sub {
 
-   die "Clone can't be done online\n." if  
PVE::LXC::check_running($vmid);
+   my $running = PVE::LXC::check_running($vmid);
+
# do all tests after lock
# we also try to do all tests before we fork the worker
my $conf = PVE::LXC::Config->load_config($vmid);
@@ -1107,6 +1108,32 @@ __PACKAGE__->register_method({
my $mountpoints = {};
my $fullclone = {};
my $vollist = [];
+   my $clone_volumes = [];
+
+   #create snapshot for full clone to minimize downtime
+   if ($running && !$snapname) {
+   $snapname = 'cloneCT';
+
+   PVE::Tools::run_command(['/usr/bin/lxc-freeze', '-n', $vmid]);
+
+   foreach my $opt (keys %$conf) {
+   my $value = $conf->{$opt};
+
+   next if $opt !~ /rootfs|mp\d+/;
+
+my $mp = $opt eq 'rootfs' ?
+   PVE::LXC::Config->parse_ct_rootfs($value) :
+   PVE::LXC::Config->parse_ct_mountpoint($value);
+
+   die "For online copy a CT storage must provide snapshots!\n"
+   if !PVE::Storage::volume_has_feature($storecfg, 
'snapshot', $mp->{volume}, $snapname, 0);;
+
+   PVE::Storage::volume_snapshot($storecfg, $mp->{volume}, 
$snapname);
+   push @$clone_volumes, $mp->{volume};
+   }
+
+   PVE::Tools::run_command(['/usr/bin/lxc-unfreeze', '-n', $vmid]);
+   }
 
foreach my $opt (keys %$oldconf) {
my $value = $oldconf->{$opt};
@@ -1189,6 +1216,19 @@ __PACKAGE__->register_method({
 
PVE::AccessControl::add_vm_to_pool($newid, $pool) if $pool;
};
+
+   if (defined($snapname) && $snapname eq 'cloneCT') {
+   foreach my $vol (@$clone_volumes) {
+
+   eval {
+   PVE::Storage::volume_snapshot_delete($storecfg, 
$vol, $snapname);
+   };
+   #warn only and remove all snapshots
+   if (my $err = $@) {
+   warn $err;
+   }
+   }
+   }
if (my $err = $@) {
unlink $conffile;
 
-- 
2.1.4


___
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel