[pve-devel] [PATCH v4 qemu-server 1/3] api2 : add migrate_vm_external

[David Limbeck]

The patches look good to me, some non-blockers inline.

On 1/8/19 2:00 AM, Alexandre Derumier wrote:
> qm migrate_external <vmid> <targetremotenode_fqdn_or_ip> [--targetstorage otherstorage] [--targetbridge otherbridge] [--targetvmid] [--targetkey]
>
>
> targetvmid is optionnal, if not specified, the next vmid available will be used.
>
> targetkey is optionnal, if not specified, the ssh private key should be
> /etc/pve/priv/external_migration/id_rsa_targetremotenode_fqdn_or_ip
>
>
> ---
>   PVE/API2/Qemu.pm | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   PVE/CLI/qm.pm    |  2 ++
>   2 files changed, 98 insertions(+)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index b55fd13..b74f111 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -21,6 +21,7 @@ use PVE::GuestHelpers;
>   use PVE::QemuConfig;
>   use PVE::QemuServer;
>   use PVE::QemuMigrate;
> +use PVE::QemuMigrateExternal;
>   use PVE::RPCEnvironment;
>   use PVE::AccessControl;
>   use PVE::INotify;
> @@ -3164,6 +3165,101 @@ __PACKAGE__->register_method({
>       }});
>   
>   __PACKAGE__->register_method({
> +    name => 'migrate_vm_external',
> +    path => '{vmid}/migrate_external',
> +    method => 'POST',
> +    protected => 1,
> +    proxyto => 'node',
> +    description => "Migrate virtual machine to an external cluster. Creates a new migration task.",
The description should perhaps contain something like 'Experimental! Use 
at your own risk'.
> +    permissions => {
> +	check => ['perm', '/vms/{vmid}', [ 'VM.Migrate' ]],
> +    },
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	    vmid => get_standard_option('pve-vmid', { completion => \&PVE::QemuServer::complete_vmid }),
> +	    target => {
> +		type => 'string',
> +		description => "Target node fqdn address.",
description should mention target ip and fqdn, basically anything that 
can be resolved by get_ip_from_hostname is allowed.
> +            },
> +	    targetstorage => get_standard_option('pve-storage-id', {
> +		description => "Target remote storage.",
> +		optional => 1,
> +	    }),
> +	    targetbridge => {
> +		type => 'string',
> +		description => "Target remote bridge.",
> +		format_description => 'bridge',
> +		optional => 1,
> +	    },
This would probably be more useful if it supported multiple different 
bridges instead of just a single one.
> +	    targetvmid => get_standard_option('pve-vmid', {
> +		description => "Target vmid. If not specified the next available vmid will be used.",
> +		optional => 1,
> +	    }),
> +	    targetkey => {
> +		type => 'string',
> +		description => "Ssh private key file located in /etc/pve/priv/migrate_external/.",
> +		optional => 1,
> +	    },
> +	},
> +    },
> +    returns => {
> +	type => 'string',
> +	description => "the task ID.",
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $rpcenv = PVE::RPCEnvironment::get();
> +
> +	my $authuser = $rpcenv->get_user();
> +
> +	die "Only root can do external migration." if $authuser ne 'root at pam';
> +
> +	my $target = extract_param($param, 'target');
> +
> +	my $vmid = extract_param($param, 'vmid');
> +
> +	my $targetkey = extract_param($param, 'targetkey');
> +
> +	PVE::Cluster::check_cfs_quorum();
> +
> +	raise_param_exc({ target => "target is member of local cluster."}) if PVE::Cluster::check_node_exists($target, 1);
> +
> +        die "HA must be disable for external migration." if PVE::HA::Config::vm_is_ha_managed($vmid);
> +
> +	my $migration_external_sshkey = $targetkey ? "/etc/pve/priv/migrate_external/$targetkey" : "/etc/pve/priv/migrate_external/id_rsa_$target";
> +
> +	die "ssh privatekey is missing for $target" if !-e $migration_external_sshkey;
> +
> +	my $targetip = PVE::Network::get_ip_from_hostname($target, 1);

This should probably be 0 instead of 1 ('noerr') as we don't have any 
check for validity before using $target. Just use it to error out with a 
nice message if $target can't be resolved.

> +
> +	# test if VM exists
> +	my $conf = PVE::QemuConfig->load_config($vmid);
> +
> +	# try to detect errors early
> +
> +	PVE::QemuConfig->check_lock($conf);
> +
> +	die "VM need to be online for external migration" if !PVE::QemuServer::check_running($vmid);
> +
> +	$param->{online} = 1;
> +	$param->{migration_external_sshkey} = $migration_external_sshkey;
> +
> +	my $realcmd = sub {
> +	    PVE::QemuMigrateExternal->migrate($target, $targetip, $vmid, $param);
> +	};
> +
> +	my $worker = sub {
> +	    return PVE::GuestHelpers::guest_migration_lock($vmid, 10, $realcmd);
> +	};
> +
> +	return $rpcenv->fork_worker('qmigrate', $vmid, $authuser, $worker);
> +
> +    }});
> +
> +__PACKAGE__->register_method({
>       name => 'monitor',
>       path => '{vmid}/monitor',
>       method => 'POST',
> diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
> index 26d4217..c271964 100755
> --- a/PVE/CLI/qm.pm
> +++ b/PVE/CLI/qm.pm
> @@ -853,6 +853,8 @@ our $cmddef = {
>   
>       migrate => [ "PVE::API2::Qemu", 'migrate_vm', ['vmid', 'target'], { node => $nodename }, $upid_exit ],
>   
> +    migrate_external => [ "PVE::API2::Qemu", 'migrate_vm_external', ['vmid', 'target'], { node => $nodename }, $upid_exit ],
> +
>       set => [ "PVE::API2::Qemu", 'update_vm', ['vmid'], { node => $nodename } ],
>   
>       resize => [ "PVE::API2::Qemu", 'resize_vm', ['vmid', 'disk', 'size'], { node => $nodename } ],