[pve-devel] [PATCH_V4 pve-container 2/5] Add move_volume.

[Fabian Grünbichler]

comments inline

> Wolfgang Link <w.link at proxmox.com> hat am 1. Juni 2016 um 09:22 geschrieben:
> 
> 
> Now it is possible to move the volume to an other storage.
> This works only when the CT is off, to keep the volume consistent.
> ---
>  src/PVE/API2/LXC.pm | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/PVE/CLI/pct.pm  |   1 +
>  2 files changed, 117 insertions(+)
> 
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index 0bcadc3..71cf21d 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -1374,4 +1374,120 @@ __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 }),
> +	    storage => get_standard_option('pve-storage-id', {
> +		description => "Target Storage.",
> +		default => 'local',
> +		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.",
> +		optional => 1,
> +		default => 0,
> +	    },
> +	    volume => {
> +		type => 'string',
> +		description => "Volume which will move.\n Format: [rootfs|mp<x>]",
> +
> +	    },

this format is not enforced anywhere, see below

> +	    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 $code = sub {
> +
> +	    my $conf = PVE::LXC::Config->load_config($vmid);
> +	    PVE::LXC::Config->check_lock($conf);
> +
> +	    die "can't move volume: $volume if snapshot exists\n"
> +		if %{$conf->{snapshots}};
> +
> +	    PVE::Tools::assert_if_modified($digest, $conf->{digest});
> +
> +	    die "Move Volume can't be done online.\n" if PVE::LXC::check_running($vmid);
> +
> +	    PVE::Cluster::log_msg('info', $authuser, "move_volume CT:$vmid Volume:$volume to Storage:$storage");
> +	    my $realcmd = sub {
> +
> +		my $storage_cfg = PVE::Storage::config();
> +
> +		my $mp;
> +		if ($volume eq 'rootfs') {
> +		    $mp = PVE::LXC::Config->parse_ct_rootfs($conf->{$volume});
> +		} else {
> +		    $mp = PVE::LXC::Config->parse_ct_mountpoint($conf->{$volume});
> +		}
> +

no check for definedness of $conf->{$volume}, no check whether $volume is a valid mountpoint key other than rootfs (you can use PVE::LXC::Config->mountpoint_names() if you don't want to hardcode mp\d+)

> +		my $old_volid =  $mp->{volume};
> +
> +		eval {
> +		    $mp->{volume} = PVE::LXC::copy_volume($mp, $vmid, $vmid, $storage, $storage_cfg, $conf);
> +
> +		    $conf->{$volume} = PVE::LXC::Config->print_ct_mountpoint($mp, $volume eq 'rootfs');
> +		};
> +		if (my $err = $@) {
> +		    die $err;
> +		}
> +
> +		if ($delete) {
> +		    PVE::Storage::vdisk_free($storage_cfg, $old_volid);
> +		} else {
> +		    my $unused = PVE::LXC::Config->add_unused_volume($conf, $old_volid);
> +		    $conf->{$unused} = $old_volid;

add_unused_volume already modifies the config hash, so a simple 

PVE::LXC::Config->add_unused_volume($conf, $old_volid);

should be enough. also add_unused_volume returns nothing if the volume id is already marked as unused, which is not caught in your version version.

note that add_unused_volume might fail/die if the maximum amount of unused disks was already reached, in which case the config would not be written although the volumes were in fact copied. it might make sense to add the unused volume before copying in order to fail early in this case (as long as the config is not written, this does not matter for error recovery, and we hold a lock so nobody else should add unused volumes).

> +		}
> +		PVE::LXC::Config->write_config($vmid, $conf);
> +	    };
> +
> +	    return $rpcenv->fork_worker('move_volume', $vmid, $authuser, $realcmd);
> +	};
> +
> +	return PVE::LXC::Config->lock_config($vmid, $code);
> +  }});
>  1;
> diff --git a/src/PVE/CLI/pct.pm b/src/PVE/CLI/pct.pm
> index ca87229..b6b834d 100755
> --- a/src/PVE/CLI/pct.pm
> +++ b/src/PVE/CLI/pct.pm
> @@ -564,6 +564,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', 'storage', 'volume'], { node => $nodename }, $upid_exit ],
>      
>      console => [ __PACKAGE__, 'console', ['vmid']],
>      enter => [ __PACKAGE__, 'enter', ['vmid']],
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>