[pve-devel] [PATCH v2 container 4/4] add move_volume api call

[Fabian Grünbichler]

On Mon, Dec 05, 2016 at 04:06:27PM +0100, Wolfgang Bumiller wrote:
> ---
> Changes to v1:
> Run activate_volumes() on the source before copy_volume()
> 
>  src/PVE/API2/LXC.pm | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/PVE/CLI/pct.pm  |   1 +
>  2 files changed, 150 insertions(+)
> 
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index 1316efe..6d8b829 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -1458,4 +1458,153 @@ __PACKAGE__->register_method({
>  	return PVE::LXC::Config->lock_config($vmid, $code);;
>      }});
>  
> +__PACKAGE__->register_method({
> +    name => 'move_ct_volume',
> +    path => '{vmid}/move_volume',
> +    method => 'POST',
> +    protected => 1,
> +    proxyto => 'node',
> +    description => "Move a rootfs-/mp-volume to a different storage",
> +    permissions => {
> +	description => "You need 'VM.Config.Disk' permissions on /vms/{vmid}, " .
> +	    "and 'Datastore.AllocateSpace' permissions on the storage.",
> +	check =>
> +	[ 'and',
> +	  ['perm', '/vms/{vmid}', [ 'VM.Config.Disk' ]],
> +	  ['perm', '/storage/{storage}', [ 'Datastore.AllocateSpace' ]],
> +	],
> +    },
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	    vmid => get_standard_option('pve-vmid', { completion => \&PVE::LXC::complete_ctid }),
> +	    volume => {
> +		type => 'string',
> +		enum => [ PVE::LXC::Config->mountpoint_names() ],
> +		description => "Volume which will move.",

I am rather unhappy about this name - "volume" refers to the actual
volume in our terminology, and not the config key rootfs/mpN, and IMHO
this gets rather confusing down below.. the config keys are for "mount
points", so maybe call this "mountpoint" or "mount_point"?

> +	    },
> +	    storage => get_standard_option('pve-storage-id', {
> +		description => "Target Storage.",
> +		completion => \&PVE::Storage::complete_storage_enabled,
> +	    }),
> +	    delete => {
> +		type => 'boolean',
> +		description => "Delete the original volume after successful copy. By default the original disk is kept as unused disk.",

volume or disk?

> +		optional => 1,
> +		default => 0,
> +	    },
> +	    digest => {
> +		type => 'string',
> +		description => 'Prevent changes if current configuration file has different SHA1 digest. This can be used to prevent concurrent modifications.',
> +		maxLength => 40,
> +		optional => 1,
> +	    }
> +	},
> +    },
> +    returns => {
> +	type => 'string',
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $rpcenv = PVE::RPCEnvironment::get();
> +
> +	my $authuser = $rpcenv->get_user();
> +
> +	my $node = extract_param($param, 'node');
> +
> +	my $vmid = extract_param($param, 'vmid');
> +
> +	my $storage = extract_param($param, 'storage');
> +
> +	my $volume = extract_param($param, 'volume');
> +
> +	my $delete = extract_param($param, 'delete');
> +
> +	my $digest = extract_param($param, 'digest');
> +
> +	my $lockname = "move-disk";
> +
> +	PVE::LXC::Config->lock_config($vmid, sub {
> +	    my $conf = PVE::LXC::Config->load_config($vmid);
> +	    PVE::LXC::Config->check_lock($conf);
> +
> +	    die "cannot move volume $volume due to existing snapshots\n"
> +		if %{$conf->{snapshots}};

that's not true - we should check for a snapshot referencing this volume
in particular.

> +
> +	    PVE::Tools::assert_if_modified($digest, $conf->{digest});
> +
> +	    die "cannot move volumes of a running container\n" if PVE::LXC::check_running($vmid);
> +
> +	    PVE::LXC::Config->set_lock($vmid, $lockname);
> +	});
> +
> +	my $realcmd = sub {
> +	    eval {
> +		PVE::Cluster::log_msg('info', $authuser, "move_volume CT:$vmid Volume:$volume to Storage:$storage");
> +
> +		my $conf = PVE::LXC::Config->load_config($vmid);
> +		my $storage_cfg = PVE::Storage::config();
> +
> +		my $mp;
> +		if ($volume eq 'rootfs') {
> +		    $mp = PVE::LXC::Config->parse_ct_rootfs($conf->{$volume});
> +		} elsif ($volume =~ m/mp\d+/) {
> +		    $mp = PVE::LXC::Config->parse_ct_mountpoint($conf->{$volume});
> +		} else {
> +		    die "Can't parse $volume\n";
> +		}
> +
> +		my $old_volid = $mp->{volume};
> +		my $new_volid;
> +
> +		eval {
> +		    PVE::Storage::activate_volumes($storage_cfg, [ $old_volid ]);
> +		    $new_volid = PVE::LXC::copy_volume($mp, $vmid, $storage, $storage_cfg, $conf);
> +		    $mp->{volume} = $new_volid;
> +
> +		    $conf->{$volume} = PVE::LXC::Config->print_ct_mountpoint($mp, $volume eq 'rootfs');

this is very the naming scheme looks really confusing - what is a mp,
what is a volume? ;)

> +
> +		    PVE::LXC::Config->add_unused_volume($conf, $old_volid) if !$delete;
> +
> +		    PVE::LXC::Config->write_config($vmid, $conf);
> +
> +		    eval {
> +			# try to deactivate volumes - avoid lvm LVs to be active on several nodes
> +			PVE::Storage::deactivate_volumes($storage_cfg, [ $new_volid ])
> +		    };
> +		    warn $@ if $@;
> +		};
> +		if (my $err = $@) {
> +		    eval {
> +			PVE::Storage::vdisk_free($storage_cfg, $new_volid)
> +			    if defined($new_volid);
> +		    };
> +		    warn $@ if $@;
> +		    die $err;
> +		}
> +
> +		if ($delete) {
> +		    eval {
> +			PVE::Storage::deactivate_volumes($storage_cfg, [ $old_volid ]);
> +			PVE::Storage::vdisk_free($storage_cfg, $old_volid);
> +		    };
> +		    warn $@ if $@;
> +		}
> +	    };
> +	    my $err = $@;
> +	    PVE::LXC::Config->remove_lock($vmid, $lockname);
> +	    die $err if $err;
> +	};
> +	my $task = eval {
> +	    $rpcenv->fork_worker('move_volume', $vmid, $authuser, $realcmd);
> +	};
> +	if (my $err = $@) {
> +	    PVE::LXC::Config->remove_lock($vmid, $lockname);
> +	    die $err;
> +	}
> +	return $task;
> +  }});
> +
>  1;
> diff --git a/src/PVE/CLI/pct.pm b/src/PVE/CLI/pct.pm
> index 2d3a7a1..6de7eaf 100755
> --- a/src/PVE/CLI/pct.pm
> +++ b/src/PVE/CLI/pct.pm
> @@ -755,6 +755,7 @@ our $cmddef = {
>      
>      clone => [ "PVE::API2::LXC", 'clone_vm', ['vmid', 'newid'], { node => $nodename }, $upid_exit ],
>      migrate => [ "PVE::API2::LXC", 'migrate_vm', ['vmid', 'target'], { node => $nodename }, $upid_exit],
> +    move_volume => [ "PVE::API2::LXC", 'move_ct_volume', ['vmid', 'volume', 'storage'], { node => $nodename }, $upid_exit ],
>      
>      status => [ __PACKAGE__, 'status', ['vmid']],
>      console => [ __PACKAGE__, 'console', ['vmid']],
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel