public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH-SERIES 0/21] remote migration
@ 2022-02-09 13:07 Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 common 1/1] add 'map_id' helper for ID maps Fabian Grünbichler
                   ` (23 more replies)
  0 siblings, 24 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

this series adds remote migration for VMs (and if the last pve-container
patch is applied, CTs ;)).

both live and offline migration of VMs including NBD and storage-migrated disks
should work, containers don't have any live migration so both offline
and restart mode work identical except for the restart part.

groundwork for extending to pvesr already laid.

uncovered (but not fixed) https://bugzilla.proxmox.com/show_bug.cgi?id=3873
(migration btrfs -> btrfs with snapshots)

new in v5: lots of edge cases fixed, PoC for pve-container, some more
helper moving for re-use in pve-container without duplication

new in v4: lots of small fixes, improved bwlimit handling, `qm` command
(thanks Fabian Ebner and Dominik Csapak for the feedback on v3!)

new in v3: lots of refactoring and edge-case handling

new in v2: dropped parts already applied, incorporated Fabian's and
Dominik's feedback (thanks!)

new in v1: explicit remote endpoint specified as part of API call
instead of
remote.cfg

overview over affected repos and changes, see individual patches for
more details.

pve-guest-common:

new/refactored generic WS/SSH tunnel fork/read/.. helpers
new storage migration over WS tunnel helpers

pve-storage:

refactor storage_migrate to make code-reuse possible
use raw+size for btrfs when no snapshots exist

qemu-server:

some refactoring, new mtunnel endpoints, new remote_migration endpoints,
new `qm` command
TODO: handle pending changes and snapshots
TODO: improve handling of C^c
TODO: strict parser for FW config as well?

as usual, some of the patches are best viewed with '-w', especially in
qemu-server..

pve-container:

--target-storage for regular migration
remote migration PoC
TODO: handle pending changes and snapshots
TODO: strict parser for FW config as well?

pve-common:

map_id helper moved from qemu-server's map_storage

required dependencies are noted, patch overview follows..

pve-common:
  add 'map_id' helper for ID maps

pve-container:
  fix #1532: add target-storage support to migration
  config: add strict parser
  migration: add remote migration

qemu-server:
  move map_storage to PVE::JSONSchema::map_id
  schema: use pve-bridge-id
  parse_config: optional strict mode
  update_vm: allow simultaneous setting of boot-order and dev
  nbd alloc helper: allow passing in explicit format
  migrate: move tunnel-helpers to pve-guest-common
  mtunnel: add API endpoints
  migrate: refactor remote VM/tunnel start
  migrate: add remote migration handling
  api: add remote migrate endpoint
  qm: add remote-migrate command

pve-storage:
  storage_migrate_snapshot: skip for btrfs without snapshots
  storage_migrate: pull out import/export_prepare
  add volume_import/export_start helpers

pve-guest-common:
  migrate: add get_bwlimit helper
  add tunnel helper module
  add storage tunnel module




^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 common 1/1] add 'map_id' helper for ID maps
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 container 1/3] fix #1532: add target-storage support to migration Fabian Grünbichler
                   ` (22 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

currently these are used by qemu-server for mapping source and target
storages, but this mechanism will be extended to network bridge maps and
re-used in pve-container as well, so let's put it next to the schema
definitions/helpers.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    new in v5, required by pve-container and qemu-server

 src/PVE/JSONSchema.pm | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index 38be3f8..65055e0 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -249,6 +249,21 @@ sub pve_verify_node_name {
     return $node;
 }
 
+# maps source to target ID using an ID map
+sub map_id {
+    my ($map, $source) = @_;
+
+    return $source if !defined($map);
+
+    return $map->{entries}->{$source}
+	if $map->{entries} && defined($map->{entries}->{$source});
+
+    return $map->{default} if $map->{default};
+
+    # identity (fallback)
+    return $source;
+}
+
 sub parse_idmap {
     my ($idmap, $idformat) = @_;
 
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 container 1/3] fix #1532: add target-storage support to migration
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 common 1/1] add 'map_id' helper for ID maps Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-10 11:52   ` Fabian Ebner
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 container 2/3] config: add strict parser Fabian Grünbichler
                   ` (21 subsequent siblings)
  23 siblings, 1 reply; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

re-using helpers that already exist for qemu-server. this is a
pre-requisite for extending remote migration support to containers.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    new in v5, no GUI yet until possible wrinkles are ironed out
    
    requires pve-common/pve-guest-common with changes from this series

 src/PVE/API2/LXC.pm    | 32 ++++++++++++++++++++++
 src/PVE/LXC/Migrate.pm | 62 ++++++++++++++++++++++++++++++++----------
 2 files changed, 79 insertions(+), 15 deletions(-)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 7573814..61eaaf7 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -36,6 +36,18 @@ BEGIN {
     }
 }
 
+my $check_storage_access_migrate = sub {
+    my ($rpcenv, $authuser, $storecfg, $storage, $node) = @_;
+
+    PVE::Storage::storage_check_enabled($storecfg, $storage, $node);
+
+    $rpcenv->check($authuser, "/storage/$storage", ['Datastore.AllocateSpace']);
+
+    my $scfg = PVE::Storage::storage_config($storecfg, $storage);
+    die "storage '$storage' does not support CT rootdirs\n"
+	if !$scfg->{content}->{rootdir};
+};
+
 __PACKAGE__->register_method ({
     subclass => "PVE::API2::LXC::Config",
     path => '{vmid}/config',
@@ -1091,6 +1103,7 @@ __PACKAGE__->register_method({
 		description => "Target node.",
 		completion => \&PVE::Cluster::complete_migration_target,
 	    }),
+	    'target-storage' => get_standard_option('pve-targetstorage'),
 	    online => {
 		type => 'boolean',
 		description => "Use online/live migration.",
@@ -1149,6 +1162,25 @@ __PACKAGE__->register_method({
 		if !$param->{online} && !$param->{restart};
 	}
 
+	if (my $targetstorage = delete $param->{'target-storage'}) {
+	    my $storecfg = PVE::Storage::config();
+	    my $storagemap = eval { PVE::JSONSchema::parse_idmap($targetstorage, 'pve-storage-id') };
+	    raise_param_exc({ targetstorage => "failed to parse storage map: $@" })
+		if $@;
+
+	    $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.Disk'])
+		if !defined($storagemap->{identity});
+
+	    foreach my $target_sid (values %{$storagemap->{entries}}) {
+		$check_storage_access_migrate->($rpcenv, $authuser, $storecfg, $target_sid, $target);
+	    }
+
+	    $check_storage_access_migrate->($rpcenv, $authuser, $storecfg, $storagemap->{default}, $target)
+		if $storagemap->{default};
+
+	    $param->{storagemap} = $storagemap;
+	}
+
 	if (PVE::HA::Config::vm_is_ha_managed($vmid) && $rpcenv->{type} ne 'ha') {
 
 	    my $hacmd = sub {
diff --git a/src/PVE/LXC/Migrate.pm b/src/PVE/LXC/Migrate.pm
index 95562e4..c85a09c 100644
--- a/src/PVE/LXC/Migrate.pm
+++ b/src/PVE/LXC/Migrate.pm
@@ -64,7 +64,8 @@ sub prepare {
 
 	# check if storage is available on both nodes
 	my $scfg = PVE::Storage::storage_check_enabled($self->{storecfg}, $storage);
-	PVE::Storage::storage_check_enabled($self->{storecfg}, $storage, $self->{node});
+
+	my $targetsid = $storage;
 
 	die "content type 'rootdir' is not available on storage '$storage'\n"
 	    if !$scfg->{content}->{rootdir};
@@ -78,8 +79,14 @@ sub prepare {
 	    # unless in restart mode because we shut the container down
 	    die "unable to migrate local mount point '$volid' while CT is running"
 		if $running && !$restart;
+
+	    $targetsid = PVE::JSONSchema::map_id($self->{opts}->{storagemap}, $storage);
 	}
 
+	my $target_scfg = PVE::Storage::storage_check_enabled($self->{storecfg}, $targetsid, $self->{node});
+
+	die "$volid: content type 'rootdir' is not available on storage '$targetsid'\n"
+	    if !$target_scfg->{content}->{rootdir};
     });
 
     # todo: test if VM uses local resources
@@ -135,18 +142,27 @@ sub phase1 {
 
 	my ($sid, $volname) = PVE::Storage::parse_volume_id($volid);
 
-	# check if storage is available on both nodes
+	# check if storage is available on source node
 	my $scfg = PVE::Storage::storage_check_enabled($self->{storecfg}, $sid);
-	PVE::Storage::storage_check_enabled($self->{storecfg}, $sid, $self->{node});
+
+	my $targetsid = $sid;
 
 	if ($scfg->{shared}) {
 	    $self->log('info', "volume '$volid' is on shared storage '$sid'")
 		if !$snapname;
 	    return;
+	} else {
+	    $targetsid = PVE::JSONSchema::map_id($self->{opts}->{storagemap}, $sid);
 	}
 
+	PVE::Storage::storage_check_enabled($self->{storecfg}, $targetsid, $self->{node});
+
+	my $bwlimit = $self->get_bwlimit($sid, $targetsid);
+
 	$volhash->{$volid}->{ref} = defined($snapname) ? 'snapshot' : 'config';
 	$volhash->{$volid}->{snapshots} = 1 if defined($snapname);
+	$volhash->{$volid}->{targetsid} = $targetsid;
+	$volhash->{$volid}->{bwlimit} = $bwlimit;
 
 	my ($path, $owner) = PVE::Storage::path($self->{storecfg}, $volid);
 
@@ -194,7 +210,8 @@ sub phase1 {
 	next if @{$dl->{$storeid}} == 0;
 
 	# check if storage is available on target node
-	PVE::Storage::storage_check_enabled($self->{storecfg}, $storeid, $self->{node});
+	my $targetsid = PVE::JSONSchema::map_id($self->{opts}->{storagemap}, $storeid);
+	PVE::Storage::storage_check_enabled($self->{storecfg}, $targetsid, $self->{node});
 
 	die "content type 'rootdir' is not available on storage '$storeid'\n"
 	    if !$scfg->{content}->{rootdir};
@@ -275,25 +292,38 @@ sub phase1 {
 	next if $rep_volumes->{$volid};
 	my ($sid, $volname) = PVE::Storage::parse_volume_id($volid);
 	push @{$self->{volumes}}, $volid;
-	my $bwlimit = PVE::Storage::get_bandwidth_limit('migration', [$sid], $opts->{bwlimit});
+
 	# JSONSchema and get_bandwidth_limit use kbps - storage_migrate bps
+	my $bwlimit = $volhash->{$volid}->{bwlimit};
 	$bwlimit = $bwlimit * 1024 if defined($bwlimit);
 
-	my $storage_migrate_opts = {
-	    'ratelimit_bps' => $bwlimit,
-	    'insecure' => $opts->{migration_type} eq 'insecure',
-	    'with_snapshots' => $volhash->{$volid}->{snapshots},
+	my $targetsid = $volhash->{$volid}->{targetsid};
+
+	my $new_volid = eval {
+	    my $storage_migrate_opts = {
+		'ratelimit_bps' => $bwlimit,
+		'insecure' => $opts->{migration_type} eq 'insecure',
+		'with_snapshots' => $volhash->{$volid}->{snapshots},
+	    };
+
+	    my $logfunc = sub { $self->log('info', $_[0]); };
+	    return PVE::Storage::storage_migrate(
+		$self->{storecfg},
+		$volid,
+		$self->{ssh_info},
+		$targetsid,
+		$storage_migrate_opts,
+		$logfunc,
+	    );
 	};
 
-	my $logfunc = sub { $self->log('info', $_[0]); };
-	eval {
-	    PVE::Storage::storage_migrate($self->{storecfg}, $volid, $self->{ssh_info},
-					  $sid, $storage_migrate_opts, $logfunc);
-	};
 	if (my $err = $@) {
-	    die "storage migration for '$volid' to storage '$sid' failed - $err\n";
+	    die "storage migration for '$volid' to storage '$targetsid' failed - $err\n";
 	}
 
+	$self->{volume_map}->{$volid} = $new_volid;
+	$self->log('info', "volume '$volid' is '$new_volid' on the target\n");
+
 	eval { PVE::Storage::deactivate_volumes($self->{storecfg}, [$volid]); };
 	if (my $err = $@) {
 	    $self->log('warn', $err);
@@ -316,6 +346,8 @@ sub phase1 {
 
     # transfer replication state before moving config
     $self->transfer_replication_state() if $rep_volumes;
+    PVE::LXC::Config->update_volume_ids($conf, $self->{volume_map});
+    PVE::LXC::Config->write_config($vmid, $conf);
     PVE::LXC::Config->move_config_to_node($vmid, $self->{node});
     $self->{conf_migrated} = 1;
     $self->switch_replication_job_target() if $rep_volumes;
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 container 2/3] config: add strict parser
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 common 1/1] add 'map_id' helper for ID maps Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 container 1/3] fix #1532: add target-storage support to migration Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH PoC v5 container 3/3] migration: add remote migration Fabian Grünbichler
                   ` (20 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

as safeguard when migrating across clusters, which might have different
versions installed.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    new in v5

 src/PVE/LXC/Config.pm | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index 6c2acd6..16739a8 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -877,7 +877,7 @@ for (my $i = 0; $i < $MAX_UNUSED_DISKS; $i++) {
 }
 
 sub parse_pct_config {
-    my ($filename, $raw) = @_;
+    my ($filename, $raw, $strict) = @_;
 
     return undef if !defined($raw);
 
@@ -887,6 +887,16 @@ sub parse_pct_config {
 	pending => {},
     };
 
+    my $handle_error = sub {
+	my ($msg) = @_;
+
+	if ($strict) {
+	    die $msg;
+	} else {
+	    warn $msg;
+	}
+    };
+
     $filename =~ m|/lxc/(\d+).conf$|
 	|| die "got strange filename '$filename'";
 
@@ -926,9 +936,9 @@ sub parse_pct_config {
 	    if ($validity eq 1) {
 		push @{$conf->{lxc}}, [$key, $value];
 	    } elsif (my $errmsg = $validity) {
-		warn "vm $vmid - $key: $errmsg\n";
+		$handle_error->("vm $vmid - $key: $errmsg\n");
 	    } else {
-		warn "vm $vmid - unable to parse config: $line\n";
+		$handle_error->("vm $vmid - unable to parse config: $line\n");
 	    }
 	} elsif ($line =~ m/^(description):\s*(.*\S)\s*$/) {
 	    $descr .= PVE::Tools::decode_text($2);
@@ -939,16 +949,16 @@ sub parse_pct_config {
 	    if ($section eq 'pending') {
 		$conf->{delete} = $value;
 	    } else {
-		warn "vm $vmid - property 'delete' is only allowed in [pve:pending]\n";
+		$handle_error->("vm $vmid - property 'delete' is only allowed in [pve:pending]\n");
 	    }
 	} elsif ($line =~ m/^([a-z][a-z_]*\d*):\s*(.+?)\s*$/) {
 	    my $key = $1;
 	    my $value = $2;
 	    eval { $value = PVE::LXC::Config->check_type($key, $value); };
-	    warn "vm $vmid - unable to parse value of '$key' - $@" if $@;
+	    $handle_error->("vm $vmid - unable to parse value of '$key' - $@") if $@;
 	    $conf->{$key} = $value;
 	} else {
-	    warn "vm $vmid - unable to parse config: $line\n";
+	    $handle_error->("vm $vmid - unable to parse config: $line\n");
 	}
     }
 
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH PoC v5 container 3/3] migration: add remote migration
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (2 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 container 2/3] config: add strict parser Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 guest-common 1/3] migrate: add get_bwlimit helper Fabian Grünbichler
                   ` (19 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    new in v5 - PoC to ensure helpers and abstractions are re-usable

 src/PVE/API2/LXC.pm    | 630 +++++++++++++++++++++++++++++++++++++++++
 src/PVE/LXC/Migrate.pm | 237 +++++++++++++---
 2 files changed, 828 insertions(+), 39 deletions(-)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 61eaaf7..32ae465 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -3,6 +3,8 @@ package PVE::API2::LXC;
 use strict;
 use warnings;
 
+use Socket qw(SOCK_STREAM);
+
 use PVE::SafeSyslog;
 use PVE::Tools qw(extract_param run_command);
 use PVE::Exception qw(raise raise_param_exc raise_perm_exc);
@@ -1084,6 +1086,174 @@ __PACKAGE__->register_method ({
     }});
 
 
+__PACKAGE__->register_method({
+    name => 'remote_migrate_vm',
+    path => '{vmid}/remote_migrate',
+    method => 'POST',
+    protected => 1,
+    proxyto => 'node',
+    description => "Migrate the container to another cluster. Creates a new migration task.",
+    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::LXC::complete_ctid }),
+	    'target-vmid' => get_standard_option('pve-vmid', { optional => 1 }),
+	    'target-endpoint' => get_standard_option('proxmox-remote', {
+		description => "Remote target endpoint",
+	    }),
+	    online => {
+		type => 'boolean',
+		description => "Use online/live migration.",
+		optional => 1,
+	    },
+	    restart => {
+		type => 'boolean',
+		description => "Use restart migration",
+		optional => 1,
+	    },
+	    timeout => {
+		type => 'integer',
+		description => "Timeout in seconds for shutdown for restart migration",
+		optional => 1,
+		default => 180,
+	    },
+	    delete => {
+		type => 'boolean',
+		description => "Delete the original VM and related data after successful migration. By default the original VM is kept on the source cluster in a stopped state.",
+		optional => 1,
+		default => 0,
+	    },
+	    'target-storage' => get_standard_option('pve-targetstorage', {
+		optional => 0,
+	    }),
+	    'target-bridge' => {
+		type => 'string',
+		description => "Mapping from source to target bridges. Providing only a single bridge ID maps all source bridges to that bridge. Providing the special value '1' will map each source bridge to itself.",
+		format => 'bridge-pair-list',
+	    },
+	    bwlimit => {
+		description => "Override I/O bandwidth limit (in KiB/s).",
+		optional => 1,
+		type => 'number',
+		minimum => '0',
+		default => 'migrate limit from datacenter or storage config',
+	    },
+	},
+    },
+    returns => {
+	type => 'string',
+	description => "the task ID.",
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+
+	my $source_vmid = extract_param($param, 'vmid');
+	my $target_endpoint = extract_param($param, 'target-endpoint');
+	my $target_vmid = extract_param($param, 'target-vmid') // $source_vmid;
+
+	my $delete = extract_param($param, 'delete') // 0;
+
+	PVE::Cluster::check_cfs_quorum();
+
+	# test if CT exists
+	my $conf = PVE::LXC::Config->load_config($source_vmid);
+	PVE::LXC::Config->check_lock($conf);
+
+	# try to detect errors early
+	if (PVE::LXC::check_running($source_vmid)) {
+	    die "can't migrate running container without --online or --restart\n"
+		if !$param->{online} && !$param->{restart};
+	}
+
+	raise_param_exc({ vmid => "cannot migrate HA-managed CT to remote cluster" })
+	    if PVE::HA::Config::vm_is_ha_managed($source_vmid);
+
+	my $remote = PVE::JSONSchema::parse_property_string('proxmox-remote', $target_endpoint);
+
+	# TODO: move this as helper somewhere appropriate?
+	my $conn_args = {
+	    protocol => 'https',
+	    host => $remote->{host},
+	    port => $remote->{port} // 8006,
+	    apitoken => $remote->{apitoken},
+	};
+
+	my $fp;
+	if ($fp = $remote->{fingerprint}) {
+	    $conn_args->{cached_fingerprints} = { uc($fp) => 1 };
+	}
+
+	print "Establishing API connection with remote at '$remote->{host}'\n";
+
+	my $api_client = PVE::APIClient::LWP->new(%$conn_args);
+
+	if (!defined($fp)) {
+	    my $cert_info = $api_client->get("/nodes/localhost/certificates/info");
+	    foreach my $cert (@$cert_info) {
+		my $filename = $cert->{filename};
+		next if $filename ne 'pveproxy-ssl.pem' && $filename ne 'pve-ssl.pem';
+		$fp = $cert->{fingerprint} if !$fp || $filename eq 'pveproxy-ssl.pem';
+	    }
+	    $conn_args->{cached_fingerprints} = { uc($fp) => 1 }
+		if defined($fp);
+	}
+
+	my $storecfg = PVE::Storage::config();
+	my $target_storage = extract_param($param, 'target-storage');
+	my $storagemap = eval { PVE::JSONSchema::parse_idmap($target_storage, 'pve-storage-id') };
+	raise_param_exc({ 'target-storage' => "failed to parse storage map: $@" })
+	    if $@;
+
+	my $target_bridge = extract_param($param, 'target-bridge');
+	my $bridgemap = eval { PVE::JSONSchema::parse_idmap($target_bridge, 'pve-bridge-id') };
+	raise_param_exc({ 'target-bridge' => "failed to parse bridge map: $@" })
+	    if $@;
+
+	die "remote migration requires explicit storage mapping!\n"
+	    if $storagemap->{identity};
+
+	$param->{storagemap} = $storagemap;
+	$param->{bridgemap} = $bridgemap;
+	$param->{remote} = {
+	    conn => $conn_args, # re-use fingerprint for tunnel
+	    client => $api_client,
+	    vmid => $target_vmid,
+	};
+	$param->{migration_type} = 'websocket';
+	$param->{delete} = $delete if $delete;
+
+	my $cluster_status = $api_client->get("/cluster/status");
+	my $target_node;
+	foreach my $entry (@$cluster_status) {
+	    next if $entry->{type} ne 'node';
+	    if ($entry->{local}) {
+		$target_node = $entry->{name};
+		last;
+	    }
+	}
+
+	die "couldn't determine endpoint's node name\n"
+	    if !defined($target_node);
+
+	my $realcmd = sub {
+	    PVE::LXC::Migrate->migrate($target_node, $remote->{host}, $source_vmid, $param);
+	};
+
+	my $worker = sub {
+	    return PVE::GuestHelpers::guest_migration_lock($source_vmid, 10, $realcmd);
+	};
+
+	return $rpcenv->fork_worker('vzmigrate', $source_vmid, $authuser, $worker);
+    }});
+
+
 __PACKAGE__->register_method({
     name => 'migrate_vm',
     path => '{vmid}/migrate',
@@ -2308,4 +2478,464 @@ __PACKAGE__->register_method({
 	return PVE::GuestHelpers::config_with_pending_array($conf, $pending_delete_hash);
     }});
 
+__PACKAGE__->register_method({
+    name => 'mtunnel',
+    path => '{vmid}/mtunnel',
+    method => 'POST',
+    protected => 1,
+    description => 'Migration tunnel endpoint - only for internal use by CT migration.',
+    permissions => {
+	check => ['perm', '/vms/{vmid}', [ 'VM.Allocate' ]],
+	description => "You need 'VM.Allocate' permissions on /vms/{vmid}. Further permission checks happen during the actual migration.",
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    vmid => get_standard_option('pve-vmid'),
+	    storages => {
+		type => 'string',
+		format => 'pve-storage-id-list',
+		optional => 1,
+		description => 'List of storages to check permission and availability. Will be checked again for all actually used storages during migration.',
+	    },
+	    bridges => {
+		type => 'string',
+		format => 'pve-bridge-id-list',
+		optional => 1,
+		description => 'List of network bridges to check availability. Will be checked again for actually used bridges during migration.',
+	    },
+	},
+    },
+    returns => {
+	additionalProperties => 0,
+	properties => {
+	    upid => { type => 'string' },
+	    ticket => { type => 'string' },
+	    socket => { 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 $storages = extract_param($param, 'storages');
+	my $bridges = extract_param($param, 'bridges');
+
+	my $nodename = PVE::INotify::nodename();
+
+	raise_param_exc({ node => "node needs to be 'localhost' or local hostname '$nodename'" })
+	    if $node ne 'localhost' && $node ne $nodename;
+
+	$node = $nodename;
+
+	my $storecfg = PVE::Storage::config();
+	foreach my $storeid (PVE::Tools::split_list($storages)) {
+	    $check_storage_access_migrate->($rpcenv, $authuser, $storecfg, $storeid, $node);
+	}
+
+	foreach my $bridge (PVE::Tools::split_list($bridges)) {
+	    PVE::Network::read_bridge_mtu($bridge);
+	}
+
+	PVE::Cluster::check_cfs_quorum();
+
+	my $socket_addr = "/run/pve/ct-$vmid.mtunnel";
+
+	my $lock = 'create';
+	eval { PVE::LXC::Config->create_and_lock_config($vmid, 0, $lock); };
+
+	raise_param_exc({ vmid => "unable to create empty CT config - $@"})
+	    if $@;
+
+	my $realcmd = sub {
+	    my $state = {
+		storecfg => PVE::Storage::config(),
+		lock => $lock,
+		vmid => $vmid,
+	    };
+
+	    my $run_locked = sub {
+		my ($code, $params) = @_;
+		return PVE::LXC::Config->lock_config($state->{vmid}, sub {
+		    my $conf = PVE::LXC::Config->load_config($state->{vmid});
+
+		    $state->{conf} = $conf;
+
+		    die "Encountered wrong lock - aborting mtunnel command handling.\n"
+			if $state->{lock} && !PVE::LXC::Config->has_lock($conf, $state->{lock});
+
+		    return $code->($params);
+		});
+	    };
+
+	    my $cmd_desc = {
+		config => {
+		    conf => {
+			type => 'string',
+			description => 'Full CT config, adapted for target cluster/node',
+		    },
+		    'firewall-config' => {
+			type => 'string',
+			description => 'CT firewall config',
+			optional => 1,
+		    },
+		},
+		ticket => {
+		    path => {
+			type => 'string',
+			description => 'socket path for which the ticket should be valid. must be known to current mtunnel instance.',
+		    },
+		},
+		quit => {
+		    cleanup => {
+			type => 'boolean',
+			description => 'remove CT config and volumes, aborting migration',
+			default => 0,
+		    },
+		},
+		'disk-import' => $PVE::StorageTunnel::cmd_schema->{'disk-import'},
+		'query-disk-import' => $PVE::StorageTunnel::cmd_schema->{'query-disk-import'},
+		bwlimit => $PVE::StorageTunnel::cmd_schema->{bwlimit},
+	    };
+
+	    my $cmd_handlers = {
+		'version' => sub {
+		    # compared against other end's version
+		    # bump/reset for breaking changes
+		    # bump/bump for opt-in changes
+		    return {
+			api => $PVE::LXC::Migrate::WS_TUNNEL_VERSION,
+			age => 0,
+		    };
+		},
+		'config' => sub {
+		    my ($params) = @_;
+
+		    # parse and write out VM FW config if given
+		    if (my $fw_conf = $params->{'firewall-config'}) {
+			my ($path, $fh) = PVE::Tools::tempfile_contents($fw_conf, 700);
+
+			my $empty_conf = {
+			    rules => [],
+			    options => {},
+			    aliases => {},
+			    ipset => {} ,
+			    ipset_comments => {},
+			};
+			my $cluster_fw_conf = PVE::Firewall::load_clusterfw_conf();
+
+			# TODO: add flag for strict parsing?
+			# TODO: add import sub that does all this given raw content?
+			my $vmfw_conf = PVE::Firewall::generic_fw_config_parser($path, $cluster_fw_conf, $empty_conf, 'vm');
+			$vmfw_conf->{vmid} = $state->{vmid};
+			PVE::Firewall::save_vmfw_conf($state->{vmid}, $vmfw_conf);
+
+			$state->{cleanup}->{fw} = 1;
+		    }
+
+		    my $conf_fn = "incoming/lxc/$state->{vmid}.conf";
+		    my $new_conf = PVE::LXC::Config::parse_pct_config($conf_fn, $params->{conf}, 1);
+		    delete $new_conf->{lock};
+		    delete $new_conf->{digest};
+
+		    my $unprivileged = delete $new_conf->{unprivileged};
+		    my $arch = delete $new_conf->{arch};
+
+		    # TODO handle properly?
+		    delete $new_conf->{snapshots};
+		    delete $new_conf->{parent};
+		    delete $new_conf->{pending};
+		    delete $new_conf->{lxc};
+
+		    PVE::LXC::Config->remove_lock($state->{vmid}, 'create');
+
+		    eval {
+			my $conf = {
+			    unprivileged => $unprivileged,
+			    arch => $arch,
+			};
+			PVE::LXC::check_ct_modify_config_perm(
+			    $rpcenv,
+			    $authuser,
+			    $state->{vmid},
+			    undef,
+			    $conf,
+			    $new_conf,
+			    undef,
+			    $unprivileged,
+			);
+			my $errors = PVE::LXC::Config->update_pct_config(
+			    $state->{vmid},
+			    $conf,
+			    0,
+			    $new_conf,
+			    [],
+			    [],
+			);
+			raise_param_exc($errors) if scalar(keys %$errors);
+			PVE::LXC::Config->write_config($state->{vmid}, $conf);
+			PVE::LXC::update_lxc_config($vmid, $conf);
+		    };
+		    if (my $err = $@) {
+			# revert to locked previous config
+			my $conf = PVE::LXC::Config->load_config($state->{vmid});
+			$conf->{lock} = 'create';
+			PVE::LXC::Config->write_config($state->{vmid}, $conf);
+
+			die $err;
+		    }
+
+		    my $conf = PVE::LXC::Config->load_config($state->{vmid});
+		    $conf->{lock} = 'migrate';
+		    PVE::LXC::Config->write_config($state->{vmid}, $conf);
+
+		    $state->{lock} = 'migrate';
+
+		    return;
+		},
+		'bwlimit' => sub {
+		    my ($params) = @_;
+		    return PVE::StorageTunnel::handle_bwlimit($params);
+		},
+		'disk-import' => sub {
+		    my ($params) = @_;
+
+		    $check_storage_access_migrate->(
+			$rpcenv,
+			$authuser,
+			$state->{storecfg},
+			$params->{storage},
+			$node
+		    );
+
+		    $params->{unix} = "/run/pve/ct-$state->{vmid}.storage";
+
+		    return PVE::StorageTunnel::handle_disk_import($state, $params);
+		},
+		'query-disk-import' => sub {
+		    my ($params) = @_;
+
+		    return PVE::StorageTunnel::handle_query_disk_import($state, $params);
+		},
+		'unlock' => sub {
+		    PVE::LXC::Config->remove_lock($state->{vmid}, $state->{lock});
+		    delete $state->{lock};
+		    return;
+		},
+		'start' => sub {
+		    PVE::LXC::vm_start(
+			$state->{vmid},
+			$state->{conf},
+			0
+		    );
+
+		    return;
+		},
+		'stop' => sub {
+		    PVE::LXC::vm_stop($state->{vmid}, 1, 10, 1);
+		    return;
+		},
+		'ticket' => sub {
+		    my ($params) = @_;
+
+		    my $path = $params->{path};
+
+		    die "Not allowed to generate ticket for unknown socket '$path'\n"
+			if !defined($state->{sockets}->{$path});
+
+		    return { ticket => PVE::AccessControl::assemble_tunnel_ticket($authuser, "/socket/$path") };
+		},
+		'quit' => sub {
+		    my ($params) = @_;
+
+		    if ($params->{cleanup}) {
+			if ($state->{cleanup}->{fw}) {
+			    PVE::Firewall::remove_vmfw_conf($state->{vmid});
+			}
+
+			for my $volid (keys $state->{cleanup}->{volumes}->%*) {
+			    print "freeing volume '$volid' as part of cleanup\n";
+			    eval { PVE::Storage::vdisk_free($state->{storecfg}, $volid) };
+			    warn $@ if $@;
+			}
+
+			PVE::LXC::destroy_lxc_container(
+			    $state->{storecfg},
+			    $state->{vmid},
+			    $state->{conf},
+			    undef,
+			    0,
+			);
+		    }
+
+		    print "switching to exit-mode, waiting for client to disconnect\n";
+		    $state->{exit} = 1;
+		    return;
+		},
+	    };
+
+	    $run_locked->(sub {
+		my $socket_addr = "/run/pve/ct-$state->{vmid}.mtunnel";
+		unlink $socket_addr;
+
+		$state->{socket} = IO::Socket::UNIX->new(
+	            Type => SOCK_STREAM(),
+		    Local => $socket_addr,
+		    Listen => 1,
+		);
+
+		$state->{socket_uid} = getpwnam('www-data')
+		    or die "Failed to resolve user 'www-data' to numeric UID\n";
+		chown $state->{socket_uid}, -1, $socket_addr;
+	    });
+
+	    print "mtunnel started\n";
+
+	    my $conn = eval { PVE::Tools::run_with_timeout(300, sub { $state->{socket}->accept() }) };
+	    if ($@) {
+		warn "Failed to accept tunnel connection - $@\n";
+
+		warn "Removing tunnel socket..\n";
+		unlink $state->{socket};
+
+		warn "Removing temporary VM config..\n";
+		$run_locked->(sub {
+		    PVE::LXC::destroy_config($state->{vmid});
+		});
+
+		die "Exiting mtunnel\n";
+	    }
+
+	    $state->{conn} = $conn;
+
+	    my $reply_err = sub {
+		my ($msg) = @_;
+
+		my $reply = JSON::encode_json({
+		    success => JSON::false,
+		    msg => $msg,
+		});
+		$conn->print("$reply\n");
+		$conn->flush();
+	    };
+
+	    my $reply_ok = sub {
+		my ($res) = @_;
+
+		$res->{success} = JSON::true;
+		my $reply = JSON::encode_json($res);
+		$conn->print("$reply\n");
+		$conn->flush();
+	    };
+
+	    while (my $line = <$conn>) {
+		chomp $line;
+
+		# untaint, we validate below if needed
+		($line) = $line =~ /^(.*)$/;
+		my $parsed = eval { JSON::decode_json($line) };
+		if ($@) {
+		    $reply_err->("failed to parse command - $@");
+		    next;
+		}
+
+		my $cmd = delete $parsed->{cmd};
+		if (!defined($cmd)) {
+		    $reply_err->("'cmd' missing");
+		} elsif ($state->{exit}) {
+		    $reply_err->("tunnel is in exit-mode, processing '$cmd' cmd not possible");
+		    next;
+		} elsif (my $handler = $cmd_handlers->{$cmd}) {
+		    print "received command '$cmd'\n";
+		    eval {
+			if ($cmd_desc->{$cmd}) {
+			    PVE::JSONSchema::validate($cmd_desc->{$cmd}, $parsed);
+			} else {
+			    $parsed = {};
+			}
+			my $res = $run_locked->($handler, $parsed);
+			$reply_ok->($res);
+		    };
+		    $reply_err->("failed to handle '$cmd' command - $@")
+			if $@;
+		} else {
+		    $reply_err->("unknown command '$cmd' given");
+		}
+	    }
+
+	    if ($state->{exit}) {
+		print "mtunnel exited\n";
+	    } else {
+		die "mtunnel exited unexpectedly\n";
+	    }
+	};
+
+	my $ticket = PVE::AccessControl::assemble_tunnel_ticket($authuser, "/socket/$socket_addr");
+	my $upid = $rpcenv->fork_worker('vzmtunnel', $vmid, $authuser, $realcmd);
+
+	return {
+	    ticket => $ticket,
+	    upid => $upid,
+	    socket => $socket_addr,
+	};
+    }});
+
+__PACKAGE__->register_method({
+    name => 'mtunnelwebsocket',
+    path => '{vmid}/mtunnelwebsocket',
+    method => 'GET',
+    permissions => {
+	description => "You need to pass a ticket valid for the selected socket. Tickets can be created via the mtunnel API call, which will check permissions accordingly.",
+        user => 'all', # check inside
+    },
+    description => 'Migration tunnel endpoint for websocket upgrade - only for internal use by VM migration.',
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    vmid => get_standard_option('pve-vmid'),
+	    socket => {
+		type => "string",
+		description => "unix socket to forward to",
+	    },
+	    ticket => {
+		type => "string",
+		description => "ticket return by initial 'mtunnel' API call, or retrieved via 'ticket' tunnel command",
+	    },
+	},
+    },
+    returns => {
+	type => "object",
+	properties => {
+	    port => { type => 'string', optional => 1 },
+	    socket => { type => 'string', optional => 1 },
+	},
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+
+	my $nodename = PVE::INotify::nodename();
+	my $node = extract_param($param, 'node');
+
+	raise_param_exc({ node => "node needs to be 'localhost' or local hostname '$nodename'" })
+	    if $node ne 'localhost' && $node ne $nodename;
+
+	my $vmid = $param->{vmid};
+	# check VM exists
+	PVE::LXC::Config->load_config($vmid);
+
+	my $socket = $param->{socket};
+	PVE::AccessControl::verify_tunnel_ticket($param->{ticket}, $authuser, "/socket/$socket");
+
+	return { socket => $socket };
+    }});
 1;
diff --git a/src/PVE/LXC/Migrate.pm b/src/PVE/LXC/Migrate.pm
index c85a09c..84911e3 100644
--- a/src/PVE/LXC/Migrate.pm
+++ b/src/PVE/LXC/Migrate.pm
@@ -17,6 +17,9 @@ use PVE::Replication;
 
 use base qw(PVE::AbstractMigrate);
 
+# compared against remote end's minimum version
+our $WS_TUNNEL_VERSION = 2;
+
 sub lock_vm {
     my ($self, $vmid, $code, @param) = @_;
 
@@ -28,6 +31,7 @@ sub prepare {
 
     my $online = $self->{opts}->{online};
     my $restart= $self->{opts}->{restart};
+    my $remote = $self->{opts}->{remote};
 
     $self->{storecfg} = PVE::Storage::config();
 
@@ -44,6 +48,7 @@ sub prepare {
     }
     $self->{was_running} = $running;
 
+    my $storages = {};
     PVE::LXC::Config->foreach_volume_full($conf, { include_unused => 1 }, sub {
 	my ($ms, $mountpoint) = @_;
 
@@ -70,7 +75,7 @@ sub prepare {
 	die "content type 'rootdir' is not available on storage '$storage'\n"
 	    if !$scfg->{content}->{rootdir};
 
-	if ($scfg->{shared}) {
+	if ($scfg->{shared} && !$remote) {
 	    # PVE::Storage::activate_storage checks this for non-shared storages
 	    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
 	    warn "Used shared storage '$storage' is not online on source node!\n"
@@ -83,18 +88,63 @@ sub prepare {
 	    $targetsid = PVE::JSONSchema::map_id($self->{opts}->{storagemap}, $storage);
 	}
 
-	my $target_scfg = PVE::Storage::storage_check_enabled($self->{storecfg}, $targetsid, $self->{node});
+	if (!$remote) {
+	    my $target_scfg = PVE::Storage::storage_check_enabled($self->{storecfg}, $targetsid, $self->{node});
+
+	    die "$volid: content type 'rootdir' is not available on storage '$targetsid'\n"
+		if !$target_scfg->{content}->{rootdir};
+	}
 
-	die "$volid: content type 'rootdir' is not available on storage '$targetsid'\n"
-	    if !$target_scfg->{content}->{rootdir};
+	$storages->{$targetsid} = 1;
     });
 
     # todo: test if VM uses local resources
 
-    # test ssh connection
-    my $cmd = [ @{$self->{rem_ssh}}, '/bin/true' ];
-    eval { $self->cmd_quiet($cmd); };
-    die "Can't connect to destination address using public key\n" if $@;
+    if ($remote) {
+	# test & establish websocket connection
+	my $bridges = map_bridges($conf, $self->{opts}->{bridgemap}, 1);
+
+	my $remote = $self->{opts}->{remote};
+	my $conn = $remote->{conn};
+
+	my $log = sub {
+	    my ($level, $msg) = @_;
+	    $self->log($level, $msg);
+	};
+
+	my $websocket_url = "https://$conn->{host}:$conn->{port}/api2/json/nodes/$self->{node}/lxc/$remote->{vmid}/mtunnelwebsocket";
+	my $url = "/nodes/$self->{node}/lxc/$remote->{vmid}/mtunnel";
+
+	my $tunnel_params = {
+	    url => $websocket_url,
+	};
+
+	my $storage_list = join(',', keys %$storages);
+	my $bridge_list = join(',', keys %$bridges);
+
+	my $req_params = {
+	    storages => $storage_list,
+	    bridges => $bridge_list,
+	};
+
+	my $tunnel = PVE::Tunnel::fork_websocket_tunnel($conn, $url, $req_params, $tunnel_params, $log);
+	my $min_version = $tunnel->{version} - $tunnel->{age};
+	$self->log('info', "local WS tunnel version: $WS_TUNNEL_VERSION");
+	$self->log('info', "remote WS tunnel version: $tunnel->{version}");
+	$self->log('info', "minimum required WS tunnel version: $min_version");
+	die "Remote tunnel endpoint not compatible, upgrade required\n"
+	    if $WS_TUNNEL_VERSION < $min_version;
+	 die "Remote tunnel endpoint too old, upgrade required\n"
+	    if $WS_TUNNEL_VERSION > $tunnel->{version};
+
+	$self->log('info', "websocket tunnel started\n");
+	$self->{tunnel} = $tunnel;
+    } else {
+	# test ssh connection
+	my $cmd = [ @{$self->{rem_ssh}}, '/bin/true' ];
+	eval { $self->cmd_quiet($cmd); };
+	die "Can't connect to destination address using public key\n" if $@;
+    }
 
     # in restart mode, we shutdown the container before migrating
     if ($restart && $running) {
@@ -113,6 +163,8 @@ sub prepare {
 sub phase1 {
     my ($self, $vmid) = @_;
 
+    my $remote = $self->{opts}->{remote};
+
     $self->log('info', "starting migration of CT $self->{vmid} to node '$self->{node}' ($self->{nodeip})");
 
     my $conf = $self->{vmconf};
@@ -147,7 +199,7 @@ sub phase1 {
 
 	my $targetsid = $sid;
 
-	if ($scfg->{shared}) {
+	if ($scfg->{shared} && !$remote) {
 	    $self->log('info', "volume '$volid' is on shared storage '$sid'")
 		if !$snapname;
 	    return;
@@ -155,7 +207,8 @@ sub phase1 {
 	    $targetsid = PVE::JSONSchema::map_id($self->{opts}->{storagemap}, $sid);
 	}
 
-	PVE::Storage::storage_check_enabled($self->{storecfg}, $targetsid, $self->{node});
+	PVE::Storage::storage_check_enabled($self->{storecfg}, $targetsid, $self->{node})
+	    if !$remote;
 
 	my $bwlimit = $self->get_bwlimit($sid, $targetsid);
 
@@ -192,6 +245,8 @@ sub phase1 {
 
 	eval {
 	    &$test_volid($volid, $snapname);
+
+	    die "remote migration with snapshots not supported yet\n" if $remote;
 	};
 
 	&$log_error($@, $volid) if $@;
@@ -201,7 +256,7 @@ sub phase1 {
     my @sids = PVE::Storage::storage_ids($self->{storecfg});
     foreach my $storeid (@sids) {
 	my $scfg = PVE::Storage::storage_config($self->{storecfg}, $storeid);
-	next if $scfg->{shared};
+	next if $scfg->{shared} && !$remote;
 	next if !PVE::Storage::storage_check_enabled($self->{storecfg}, $storeid, undef, 1);
 
 	# get list from PVE::Storage (for unreferenced volumes)
@@ -211,7 +266,8 @@ sub phase1 {
 
 	# check if storage is available on target node
 	my $targetsid = PVE::JSONSchema::map_id($self->{opts}->{storagemap}, $storeid);
-	PVE::Storage::storage_check_enabled($self->{storecfg}, $targetsid, $self->{node});
+	PVE::Storage::storage_check_enabled($self->{storecfg}, $targetsid, $self->{node})
+	    if !$remote;
 
 	die "content type 'rootdir' is not available on storage '$storeid'\n"
 	    if !$scfg->{content}->{rootdir};
@@ -239,12 +295,21 @@ sub phase1 {
 	    my ($sid, $volname) = PVE::Storage::parse_volume_id($volid);
 	    my $scfg =  PVE::Storage::storage_config($self->{storecfg}, $sid);
 
-	    my $migratable = ($scfg->{type} eq 'dir') || ($scfg->{type} eq 'zfspool')
-		|| ($scfg->{type} eq 'lvmthin') || ($scfg->{type} eq 'lvm')
-		|| ($scfg->{type} eq 'btrfs');
+	    # TODO move to storage plugin layer?
+	    my $migratable_storages = [
+		'dir',
+		'zfspool',
+		'lvmthin',
+		'lvm',
+		'btrfs',
+	    ];
+	    if ($remote) {
+		push @$migratable_storages, 'cifs';
+		push @$migratable_storages, 'nfs';
+	    }
 
 	    die "storage type '$scfg->{type}' not supported\n"
-		if !$migratable;
+		if !grep { $_ eq $scfg->{type} } @$migratable_storages;
 
 	    # image is a linked clone on local storage, se we can't migrate.
 	    if (my $basename = (PVE::Storage::parse_volname($self->{storecfg}, $volid))[3]) {
@@ -279,7 +344,10 @@ sub phase1 {
 
     my $rep_cfg = PVE::ReplicationConfig->new();
 
-    if (my $jobcfg = $rep_cfg->find_local_replication_job($vmid, $self->{node})) {
+    if ($remote) {
+	die "cannot remote-migrate replicated VM\n"
+	    if $rep_cfg->check_for_existing_jobs($vmid, 1);
+    } elsif (my $jobcfg = $rep_cfg->find_local_replication_job($vmid, $self->{node})) {
 	die "can't live migrate VM with replicated volumes\n" if $self->{running};
 	my $start_time = time();
 	my $logfunc = sub { my ($msg) = @_;  $self->log('info', $msg); };
@@ -290,7 +358,6 @@ sub phase1 {
     my $opts = $self->{opts};
     foreach my $volid (keys %$volhash) {
 	next if $rep_volumes->{$volid};
-	my ($sid, $volname) = PVE::Storage::parse_volume_id($volid);
 	push @{$self->{volumes}}, $volid;
 
 	# JSONSchema and get_bandwidth_limit use kbps - storage_migrate bps
@@ -300,21 +367,38 @@ sub phase1 {
 	my $targetsid = $volhash->{$volid}->{targetsid};
 
 	my $new_volid = eval {
-	    my $storage_migrate_opts = {
-		'ratelimit_bps' => $bwlimit,
-		'insecure' => $opts->{migration_type} eq 'insecure',
-		'with_snapshots' => $volhash->{$volid}->{snapshots},
-	    };
-
-	    my $logfunc = sub { $self->log('info', $_[0]); };
-	    return PVE::Storage::storage_migrate(
-		$self->{storecfg},
-		$volid,
-		$self->{ssh_info},
-		$targetsid,
-		$storage_migrate_opts,
-		$logfunc,
-	    );
+	    if ($remote) {
+		my $log = sub {
+		    my ($level, $msg) = @_;
+		    $self->log($level, $msg);
+		};
+
+		return PVE::StorageTunnel::storage_migrate(
+		    $self->{tunnel},
+		    $self->{storecfg},
+		    $volid,
+		    $self->{vmid},
+		    $remote->{vmid},
+		    $volhash->{$volid},
+		    $log,
+		);
+	    } else {
+		my $storage_migrate_opts = {
+		    'ratelimit_bps' => $bwlimit,
+		    'insecure' => $opts->{migration_type} eq 'insecure',
+		    'with_snapshots' => $volhash->{$volid}->{snapshots},
+		};
+
+		my $logfunc = sub { $self->log('info', $_[0]); };
+		return PVE::Storage::storage_migrate(
+		    $self->{storecfg},
+		    $volid,
+		    $self->{ssh_info},
+		    $targetsid,
+		    $storage_migrate_opts,
+		    $logfunc,
+		);
+	    }
 	};
 
 	if (my $err = $@) {
@@ -344,13 +428,38 @@ sub phase1 {
     my $vollist = PVE::LXC::Config->get_vm_volumes($conf);
     PVE::Storage::deactivate_volumes($self->{storecfg}, $vollist);
 
-    # transfer replication state before moving config
-    $self->transfer_replication_state() if $rep_volumes;
-    PVE::LXC::Config->update_volume_ids($conf, $self->{volume_map});
-    PVE::LXC::Config->write_config($vmid, $conf);
-    PVE::LXC::Config->move_config_to_node($vmid, $self->{node});
+    if ($remote) {
+	my $remote_conf = PVE::LXC::Config->load_config($vmid);
+	PVE::LXC::Config->update_volume_ids($remote_conf, $self->{volume_map});
+
+	my $bridges = map_bridges($remote_conf, $self->{opts}->{bridgemap});
+	for my $target (keys $bridges->%*) {
+	    for my $nic (keys $bridges->{$target}->%*) {
+		$self->log('info', "mapped: $nic from $bridges->{$target}->{$nic} to $target");
+	    }
+	}
+	my $conf_str = PVE::LXC::Config::write_pct_config("remote", $remote_conf);
+
+	# TODO expose in PVE::Firewall?
+	my $vm_fw_conf_path = "/etc/pve/firewall/$vmid.fw";
+	my $fw_conf_str;
+	$fw_conf_str = PVE::Tools::file_get_contents($vm_fw_conf_path)
+	    if -e $vm_fw_conf_path;
+	my $params = {
+	    conf => $conf_str,
+	    'firewall-config' => $fw_conf_str,
+	};
+
+	PVE::Tunnel::write_tunnel($self->{tunnel}, 10, 'config', $params);
+    } else {
+	# transfer replication state before moving config
+	$self->transfer_replication_state() if $rep_volumes;
+	PVE::LXC::Config->update_volume_ids($conf, $self->{volume_map});
+	PVE::LXC::Config->write_config($vmid, $conf);
+	PVE::LXC::Config->move_config_to_node($vmid, $self->{node});
+	$self->switch_replication_job_target() if $rep_volumes;
+    }
     $self->{conf_migrated} = 1;
-    $self->switch_replication_job_target() if $rep_volumes;
 }
 
 sub phase1_cleanup {
@@ -364,6 +473,12 @@ sub phase1_cleanup {
 	    # fixme: try to remove ?
 	}
     }
+
+    if ($self->{opts}->{remote}) {
+	# cleans up remote volumes
+	PVE::Tunnel::finish_tunnel($self->{tunnel}, 1);
+	delete $self->{tunnel};
+    }
 }
 
 sub phase3 {
@@ -371,6 +486,9 @@ sub phase3 {
 
     my $volids = $self->{volumes};
 
+    # handled below in final_cleanup
+    return if $self->{opts}->{remote};
+
     # destroy local copies
     foreach my $volid (@$volids) {
 	eval { PVE::Storage::vdisk_free($self->{storecfg}, $volid); };
@@ -399,6 +517,24 @@ sub final_cleanup {
 	    my $skiplock = 1;
 	    PVE::LXC::vm_start($vmid, $self->{vmconf}, $skiplock);
 	}
+    } elsif ($self->{opts}->{remote}) {
+	eval { PVE::Tunnel::write_tunnel($self->{tunnel}, 10, 'unlock') };
+	$self->log('err', "Failed to clear migrate lock - $@\n") if $@;
+
+	if ($self->{opts}->{restart} && $self->{was_running}) {
+	    $self->log('info', "start container on target node");
+	    PVE::Tunnel::write_tunnel($self->{tunnel}, 60, 'start');
+	}
+	if ($self->{opts}->{delete}) {
+	    PVE::LXC::destroy_lxc_container(
+		PVE::Storage::config(),
+		$vmid,
+		PVE::LXC::Config->load_config($vmid),
+		undef,
+		0,
+	    );
+	}
+	PVE::Tunnel::finish_tunnel($self->{tunnel});
     } else {
 	my $cmd = [ @{$self->{rem_ssh}}, 'pct', 'unlock', $vmid ];
 	$self->cmd_logerr($cmd, errmsg => "failed to clear migrate lock");
@@ -411,7 +547,30 @@ sub final_cleanup {
 	    $self->cmd($cmd);
 	}
     }
+}
+
+sub map_bridges {
+    my ($conf, $map, $scan_only) = @_;
+
+    my $bridges = {};
+
+    foreach my $opt (keys %$conf) {
+	next if $opt !~ m/^net\d+$/;
+
+	next if !$conf->{$opt};
+	my $d = PVE::LXC::Config->parse_lxc_network($conf->{$opt});
+	next if !$d || !$d->{bridge};
+
+	my $target_bridge = PVE::JSONSchema::map_id($map, $d->{bridge});
+	$bridges->{$target_bridge}->{$opt} = $d->{bridge};
+
+	next if $scan_only;
+
+	$d->{bridge} = $target_bridge;
+	$conf->{$opt} = PVE::LXC::Config->print_lxc_network($d);
+    }
 
+    return $bridges;
 }
 
 1;
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 guest-common 1/3] migrate: add get_bwlimit helper
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (3 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH PoC v5 container 3/3] migration: add remote migration Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 guest-common 2/3] add tunnel helper module Fabian Grünbichler
                   ` (18 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

given a source and target storage query either locally or both locally
and remotely and merge the result.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    new in v5, replaces merge_bwlimits previously in PVE::QemuMigrate

 src/PVE/AbstractMigrate.pm | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/src/PVE/AbstractMigrate.pm b/src/PVE/AbstractMigrate.pm
index a20b213..d90e5b7 100644
--- a/src/PVE/AbstractMigrate.pm
+++ b/src/PVE/AbstractMigrate.pm
@@ -342,4 +342,30 @@ sub switch_replication_job_target {
     }
 }
 
+# merges local limit '$bwlimit' and a possible remote limit
+sub get_bwlimit {
+    my ($self, $source, $target) = @_;
+
+    my $local_sids = $self->{opts}->{remote} ? [$source] : [$source, $target];
+
+    my $bwlimit = PVE::Storage::get_bandwidth_limit('migration', $local_sids, $self->{opts}->{bwlimit});
+
+    if ($self->{opts}->{remote}) {
+	my $bwlimit_opts = {
+	    operation => 'migration',
+	    storages => [$target],
+	    bwlimit => $self->{opts}->{bwlimit},
+	};
+	my $remote_bwlimit = PVE::Tunnel::write_tunnel($self->{tunnel}, 10, 'bwlimit', $bwlimit_opts);
+	if ($remote_bwlimit && $remote_bwlimit->{bwlimit}) {
+	    $remote_bwlimit = $remote_bwlimit->{bwlimit};
+
+	    $bwlimit = $remote_bwlimit
+		if (!$bwlimit || $bwlimit > $remote_bwlimit);
+	}
+    }
+
+    return $bwlimit;
+}
+
 1;
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 guest-common 2/3] add tunnel helper module
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (4 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 guest-common 1/3] migrate: add get_bwlimit helper Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 guest-common 3/3] add storage tunnel module Fabian Grünbichler
                   ` (17 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

lifted from PVE::QemuMigrate, abstracting away use-case specific data.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    v5: nits
    v4:
    - remove remaining 'migration' mentions
    - fix wrong 'log' call
    - set 'log' during WS tunnel setup already so it's available for error handling
    - switch 'log' back to two-arg variant used in migration

 src/Makefile      |   1 +
 debian/control    |   1 +
 src/PVE/Tunnel.pm | 360 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 362 insertions(+)
 create mode 100644 src/PVE/Tunnel.pm

diff --git a/src/Makefile b/src/Makefile
index 0298d3f..d82162c 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -12,6 +12,7 @@ install: PVE
 	install -m 0644 PVE/ReplicationConfig.pm ${PERL5DIR}/PVE/
 	install -m 0644 PVE/ReplicationState.pm ${PERL5DIR}/PVE/
 	install -m 0644 PVE/Replication.pm ${PERL5DIR}/PVE/
+	install -m 0644 PVE/Tunnel.pm ${PERL5DIR}/PVE/
 	install -d ${PERL5DIR}/PVE/VZDump
 	install -m 0644 PVE/VZDump/Plugin.pm ${PERL5DIR}/PVE/VZDump/
 	install -m 0644 PVE/VZDump/Common.pm ${PERL5DIR}/PVE/VZDump/
diff --git a/debian/control b/debian/control
index 4c246d0..73c28bd 100644
--- a/debian/control
+++ b/debian/control
@@ -16,6 +16,7 @@ Depends: libpve-cluster-perl,
          libpve-common-perl (>= 4.0-89),
          libpve-storage-perl (>= 7.0-14),
          pve-cluster,
+         proxmox-websocket-tunnel,
          ${misc:Depends},
          ${perl:Depends},
 Breaks: libpve-common-perl (<< 4.0-89),
diff --git a/src/PVE/Tunnel.pm b/src/PVE/Tunnel.pm
new file mode 100644
index 0000000..4f8df8e
--- /dev/null
+++ b/src/PVE/Tunnel.pm
@@ -0,0 +1,360 @@
+package PVE::Tunnel;
+
+use strict;
+use warnings;
+
+use IO::Pipe;
+use IPC::Open2;
+use JSON qw(encode_json decode_json);
+use POSIX qw( WNOHANG );
+use Storable qw(dclone);
+use URI::Escape;
+
+use PVE::APIClient::LWP;
+use PVE::Tools;
+
+my $finish_command_pipe = sub {
+    my ($cmdpipe, $timeout) = @_;
+
+    my $cpid = $cmdpipe->{pid};
+    return if !defined($cpid);
+
+    my $writer = $cmdpipe->{writer};
+    my $reader = $cmdpipe->{reader};
+
+    $writer->close();
+    $reader->close();
+
+    my $collect_child_process = sub {
+	my $res = waitpid($cpid, WNOHANG);
+	if (defined($res) && ($res == $cpid)) {
+	    delete $cmdpipe->{cpid};
+	    return 1;
+	} else {
+	    return 0;
+	}
+    };
+
+    if ($timeout) {
+	for (my $i = 0; $i < $timeout; $i++) {
+	    return if &$collect_child_process();
+	    sleep(1);
+	}
+    }
+
+    $cmdpipe->{log}->('info', "tunnel still running - terminating now with SIGTERM\n");
+    kill(15, $cpid);
+
+    # wait again
+    for (my $i = 0; $i < 10; $i++) {
+	return if &$collect_child_process();
+	sleep(1);
+    }
+
+    $cmdpipe->{log}->('info', "tunnel still running - terminating now with SIGKILL\n");
+    kill 9, $cpid;
+    sleep 1;
+
+    $cmdpipe->{log}->('err', "tunnel child process (PID $cpid) couldn't be collected\n")
+	if !&$collect_child_process();
+};
+
+sub read_tunnel {
+    my ($tunnel, $timeout) = @_;
+
+    $timeout = 60 if !defined($timeout);
+
+    my $reader = $tunnel->{reader};
+
+    my $output;
+    eval {
+	PVE::Tools::run_with_timeout($timeout, sub { $output = <$reader>; });
+    };
+    die "reading from tunnel failed: $@\n" if $@;
+
+    chomp $output if defined($output);
+
+    return $output;
+}
+
+sub write_tunnel {
+    my ($tunnel, $timeout, $command, $params) = @_;
+
+    $timeout = 60 if !defined($timeout);
+
+    my $writer = $tunnel->{writer};
+
+    if ($tunnel->{version} && $tunnel->{version} >= 2) {
+	my $object = defined($params) ? dclone($params) : {};
+	$object->{cmd} = $command;
+
+	$command = eval { JSON::encode_json($object) };
+
+	die "failed to encode command as JSON - $@\n"
+	    if $@;
+    }
+
+    eval {
+	PVE::Tools::run_with_timeout($timeout, sub {
+	    print $writer "$command\n";
+	    $writer->flush();
+	});
+    };
+    die "writing to tunnel failed: $@\n" if $@;
+
+    if ($tunnel->{version} && $tunnel->{version} >= 1) {
+	my $res = eval { read_tunnel($tunnel, $timeout); };
+	die "no reply to command '$command': $@\n" if $@;
+
+	if ($tunnel->{version} == 1) {
+	    if ($res eq 'OK') {
+		return;
+	    } else {
+		die "tunnel replied '$res' to command '$command'\n";
+	    }
+	} else {
+	    my $parsed = eval { JSON::decode_json($res) };
+	    die "failed to decode tunnel reply '$res' (command '$command') - $@\n"
+		if $@;
+
+	    if (!$parsed->{success}) {
+		if (defined($parsed->{msg})) {
+		    die "error - tunnel command '$command' failed - $parsed->{msg}\n";
+		} else {
+		    die "error - tunnel command '$command' failed\n";
+		}
+	    }
+
+	    return $parsed;
+	}
+    }
+}
+
+sub fork_ssh_tunnel {
+    my ($rem_ssh, $cmd, $ssh_forward_info, $log) = @_;
+
+    my @localtunnelinfo = ();
+    foreach my $addr (@$ssh_forward_info) {
+	push @localtunnelinfo, '-L', $addr;
+    }
+
+    my $full_cmd = [@$rem_ssh, '-o ExitOnForwardFailure=yes', @localtunnelinfo, @$cmd];
+
+    my $reader = IO::File->new();
+    my $writer = IO::File->new();
+
+    my $orig_pid = $$;
+
+    my $cpid;
+
+    eval { $cpid = open2($reader, $writer, @$full_cmd); };
+
+    my $err = $@;
+
+    # catch exec errors
+    if ($orig_pid != $$) {
+	$log->('err', "can't fork command pipe, aborting\n");
+	POSIX::_exit(1);
+	kill('KILL', $$);
+    }
+
+    die $err if $err;
+
+    my $tunnel = {
+	writer => $writer,
+	reader => $reader,
+	pid => $cpid,
+	rem_ssh => $rem_ssh,
+	log => $log,
+    };
+
+    eval {
+	my $helo = read_tunnel($tunnel, 60);
+	die "no reply\n" if !$helo;
+	die "no quorum on target node\n" if $helo =~ m/^no quorum$/;
+	die "got strange reply from tunnel ('$helo')\n"
+	    if $helo !~ m/^tunnel online$/;
+    };
+    $err = $@;
+
+    eval {
+	my $ver = read_tunnel($tunnel, 10);
+	if ($ver =~ /^ver (\d+)$/) {
+	    $tunnel->{version} = $1;
+	    $log->('info', "ssh tunnel $ver\n");
+	} else {
+	    $err = "received invalid tunnel version string '$ver'\n" if !$err;
+	}
+    };
+
+    if ($err) {
+	$finish_command_pipe->($tunnel);
+	die "can't open tunnel - $err";
+    }
+    return $tunnel;
+}
+
+sub forward_unix_socket {
+    my ($tunnel, $local, $remote) = @_;
+
+    my $params = dclone($tunnel->{params});
+    $params->{unix} = $local;
+    $params->{url} = $params->{url} ."socket=".uri_escape($remote)."&";
+    $params->{ticket} = { path => $remote };
+
+    my $cmd = encode_json({
+	control => JSON::true,
+	cmd => 'forward',
+	data => $params,
+    });
+
+    my $writer = $tunnel->{writer};
+    $tunnel->{forwarded}->{$local} = $remote;
+    eval {
+	unlink $local;
+	PVE::Tools::run_with_timeout(15, sub {
+	    print $writer "$cmd\n";
+	    $writer->flush();
+	});
+    };
+    die "failed to write forwarding command - $@\n" if $@;
+
+    read_tunnel($tunnel);
+}
+
+sub fork_websocket_tunnel {
+    my ($conn, $url, $req_params, $tunnel_params, $log) = @_;
+
+    if (my $apitoken = $conn->{apitoken}) {
+	$tunnel_params->{headers} = [["Authorization", "$apitoken"]];
+    } else {
+	die "can't connect to remote host without credentials\n";
+    }
+
+    if (my $fps = $conn->{cached_fingerprints}) {
+	$tunnel_params->{fingerprint} = (keys %$fps)[0];
+    }
+
+    my $api_client = PVE::APIClient::LWP->new(%$conn);
+
+    my $res = $api_client->post(
+	$url,
+	$req_params,
+    );
+
+    $log->('info', "remote: started tunnel worker '$res->{upid}'");
+
+    my $websocket_url = $tunnel_params->{url};
+
+    $tunnel_params->{url} .= "?ticket=".uri_escape($res->{ticket});
+    $tunnel_params->{url} .= "&socket=".uri_escape($res->{socket});
+
+    my $reader = IO::Pipe->new();
+    my $writer = IO::Pipe->new();
+
+    my $cpid = fork();
+    if ($cpid) {
+	$writer->writer();
+	$reader->reader();
+	my $tunnel = {
+	    writer => $writer,
+	    reader => $reader,
+	    pid => $cpid,
+	    log => $log,
+	};
+
+	eval {
+	    my $writer = $tunnel->{writer};
+	    my $cmd = encode_json({
+		control => JSON::true,
+		cmd => 'connect',
+		data => $tunnel_params,
+	    });
+
+	    eval {
+		PVE::Tools::run_with_timeout(15, sub {
+		    print {$writer} "$cmd\n";
+		    $writer->flush();
+		});
+	    };
+	    die "failed to write tunnel connect command - $@\n" if $@;
+	};
+	die "failed to connect via WS: $@\n" if $@;
+
+	my $err;
+        eval {
+	    my $writer = $tunnel->{writer};
+	    my $cmd = encode_json({
+		cmd => 'version',
+	    });
+
+	    eval {
+		PVE::Tools::run_with_timeout(15, sub {
+		    print {$writer} "$cmd\n";
+		    $writer->flush();
+		});
+	    };
+	    $err = "failed to write tunnel version command - $@\n" if $@;
+	    my $res = read_tunnel($tunnel, 10);
+	    $res = JSON::decode_json($res);
+	    my $version = $res->{api};
+
+	    if ($version =~ /^(\d+)$/) {
+		$tunnel->{version} = $1;
+		$tunnel->{age} = $res->{age};
+	    } else {
+		$err = "received invalid tunnel version string '$version'\n" if !$err;
+	    }
+	};
+	$err = $@ if !$err;
+
+	if ($err) {
+	    $finish_command_pipe->($tunnel);
+	    die "can't open tunnel - $err";
+	}
+
+	$tunnel_params->{url} = "$websocket_url?"; # reset ticket and socket
+
+	$tunnel->{params} = $tunnel_params; # for forwarding
+
+	return $tunnel;
+    } else {
+	eval {
+	    $writer->reader();
+	    $reader->writer();
+	    PVE::Tools::run_command(
+		['proxmox-websocket-tunnel'],
+		input => "<&".fileno($writer),
+		output => ">&".fileno($reader),
+		errfunc => sub { my $line = shift; print "tunnel: $line\n"; },
+	    );
+	};
+	warn "CMD websocket tunnel died: $@\n" if $@;
+	exit 0;
+    }
+}
+
+sub finish_tunnel {
+    my ($tunnel, $cleanup) = @_;
+
+    $cleanup = $cleanup ? 1 : 0;
+
+    eval { write_tunnel($tunnel, 30, 'quit', { cleanup => $cleanup }); };
+    my $err = $@;
+
+    $finish_command_pipe->($tunnel, 30);
+
+    if (my $unix_sockets = $tunnel->{unix_sockets}) {
+	# ssh does not clean up on local host
+	my $cmd = ['rm', '-f', @$unix_sockets];
+	PVE::Tools::run_command($cmd);
+
+	# .. and just to be sure check on remote side
+	if ($tunnel->{rem_ssh}) {
+	    unshift @{$cmd}, @{$tunnel->{rem_ssh}};
+	    PVE::Tools::run_command($cmd);
+	}
+    }
+
+    die $err if $err;
+}
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 guest-common 3/3] add storage tunnel module
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (5 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 guest-common 2/3] add tunnel helper module Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 01/11] move map_storage to PVE::JSONSchema::map_id Fabian Grünbichler
                   ` (16 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

encapsulating storage-related tunnel methods, currently
- source-side storage-migrate helper
- target-side disk-import handler
- target-side query-disk-import handler
- target-side bwlimit handler

to be extended further with replication-related handlers and helpers.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    v5:
    - fix socket path to no longer reference qemu-server
    - fix accidental setting of $base_snapshot when determining export formats
    - pass $migration_snapshot to storage_migrate_snapshot
    - delete snapshot based on $migration_snapshot, not $snapshot
    v4:
    - add 'bwlimit' command, extended to support multiple storages and operation override
    - fix option/parameter names ('-' vs '_')
    - storage_migrate: move bwlimit decision to caller
    - storage_migrate: handle volids owned by third VMID
    - storage_migrate: pass snapshot to volume_export_formats
    - storage_migrate: pass log function to export helper
    - disk-import: add missing parameters to schema
    - query-disk-import: fix race when querying import result
    
    new in v3, includes code previously in qemu-server
    
    requires bumped pve-storage with new export/import helpers

 src/Makefile             |   1 +
 src/PVE/StorageTunnel.pm | 296 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 297 insertions(+)
 create mode 100644 src/PVE/StorageTunnel.pm

diff --git a/src/Makefile b/src/Makefile
index d82162c..baa2688 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -12,6 +12,7 @@ install: PVE
 	install -m 0644 PVE/ReplicationConfig.pm ${PERL5DIR}/PVE/
 	install -m 0644 PVE/ReplicationState.pm ${PERL5DIR}/PVE/
 	install -m 0644 PVE/Replication.pm ${PERL5DIR}/PVE/
+	install -m 0644 PVE/StorageTunnel.pm ${PERL5DIR}/PVE/
 	install -m 0644 PVE/Tunnel.pm ${PERL5DIR}/PVE/
 	install -d ${PERL5DIR}/PVE/VZDump
 	install -m 0644 PVE/VZDump/Plugin.pm ${PERL5DIR}/PVE/VZDump/
diff --git a/src/PVE/StorageTunnel.pm b/src/PVE/StorageTunnel.pm
new file mode 100644
index 0000000..41147dd
--- /dev/null
+++ b/src/PVE/StorageTunnel.pm
@@ -0,0 +1,296 @@
+package PVE::StorageTunnel;
+
+use strict;
+use warnings;
+
+use IO::Socket::UNIX;
+use POSIX qw(WNOHANG);
+use Socket qw(SOCK_STREAM);
+
+use PVE::Storage;
+use PVE::Tools;
+use PVE::Tunnel;
+
+sub storage_migrate {
+    my ($tunnel, $storecfg, $volid, $local_vmid, $remote_vmid, $opts, $log) = @_;
+
+    my $targetsid = $opts->{targetsid};
+    my $bwlimit = $opts->{bwlimit};
+
+    # JSONSchema and get_bandwidth_limit use kbps - storage_migrate bps
+    $bwlimit = $bwlimit * 1024 if defined($bwlimit);
+
+    # adapt volume name for import call
+    my ($sid, undef) = PVE::Storage::parse_volume_id($volid);
+    my (undef, $name, $owner, undef, undef, undef, $format) = PVE::Storage::parse_volname($storecfg, $volid);
+    my $scfg = PVE::Storage::storage_config($storecfg, $sid);
+    PVE::Storage::activate_volumes($storecfg, [$volid]);
+
+    die "failed to determine owner of volume '$volid'\n" if !defined($owner);
+    $log->('warn', "volume '$volid' owner by VM/CT '$owner', not '$local_vmid'\n")
+	if $owner != $local_vmid;
+
+    if ($owner != $remote_vmid) {
+	$name =~ s/-$owner-/-$remote_vmid-/g;
+	$name =~ s/^$owner\///; # re-added on target if dir-based storage
+    }
+
+    my $with_snapshots = $opts->{snapshots} ? 1 : 0;
+    my $snapshot;
+    my $migration_snapshot = PVE::Storage::storage_migrate_snapshot($storecfg, $sid, $with_snapshots);
+    if ($migration_snapshot) {
+	$snapshot = '__migration__';
+	$with_snapshots = 1;
+    }
+
+    my @export_formats = PVE::Storage::volume_export_formats($storecfg, $volid, $snapshot, undef, $with_snapshots);
+    die "no export formats for '$volid' - check storage plugin support!\n"
+	if !@export_formats;
+
+    my $disk_import_opts = {
+	format => $format,
+	storage => $targetsid,
+	snapshot => $snapshot,
+	migration_snapshot => $migration_snapshot,
+	with_snapshots => $with_snapshots,
+	allow_rename => !$opts->{is_vmstate},
+	export_formats => join(",", @export_formats),
+	volname => $name,
+    };
+    my $res = PVE::Tunnel::write_tunnel($tunnel, 600, 'disk-import', $disk_import_opts);
+    my $local = "/run/pve/$local_vmid.storage";
+    if (!$tunnel->{forwarded}->{$local}) {
+	PVE::Tunnel::forward_unix_socket($tunnel, $local, $res->{socket});
+    }
+    my $socket = IO::Socket::UNIX->new(Peer => $local, Type => SOCK_STREAM())
+	or die "failed to connect to websocket tunnel at $local\n";
+    # we won't be reading from the socket
+    shutdown($socket, 0);
+
+    my $disk_export_opts = {
+	snapshot => $snapshot,
+	migration_snapshot => $migration_snapshot,
+	with_snapshots => $with_snapshots,
+	ratelimit_bps => $bwlimit,
+	cmd => {
+	    output => '>&'.fileno($socket),
+	},
+    };
+
+    eval {
+	PVE::Storage::volume_export_start(
+	    $storecfg,
+	    $volid,
+	    $res->{format},
+	    sub { $log->('info', shift) },
+	    $disk_export_opts,
+	);
+    };
+    my $send_error = $@;
+    warn "$send_error\n" if $send_error;
+
+    # don't close the connection entirely otherwise the
+    # receiving end might not get all buffered data (and
+    # fails with 'connection reset by peer')
+    shutdown($socket, 1);
+
+    # wait for the remote process to finish
+    my $new_volid;
+    while ($res = PVE::Tunnel::write_tunnel($tunnel, 10, 'query-disk-import')) {
+	if ($res->{status} eq 'pending') {
+	    if (my $msg = $res->{msg}) {
+		$log->('info', "disk-import: $msg\n");
+	    } else {
+		$log->('info', "waiting for disk import to finish..\n");
+	    }
+	    sleep(1)
+	} elsif ($res->{status} eq 'complete') {
+	    $new_volid = $res->{volid};
+	    last;
+	} else {
+	    $log->('err', "unknown query-disk-import result: $res->{status}\n");
+	    last;
+	}
+    }
+
+    # now close the socket
+    close($socket);
+    if ($migration_snapshot) {
+	eval { PVE::Storage::volume_snapshot_delete($storecfg, $volid, $snapshot, 0) };
+	warn "could not remove source snapshot: $@\n" if $@;
+    }
+    die $send_error if $send_error;
+    die "disk import failed - see log above\n" if !$new_volid;
+
+    return $new_volid;
+}
+
+our $cmd_schema = {
+    bwlimit => {
+	storages => {
+	    type => 'string',
+	    format => 'pve-storage-id-list',
+	    description => "Storage for which bwlimit is queried",
+	},
+	bwlimit => {
+	    description => "Override I/O bandwidth limit (in KiB/s).",
+	    optional => 1,
+	    type => 'integer',
+	    minimum => '0',
+	},
+	operation => {
+	    description => 'Operation for which bwlimit is queried ("restore", "migration", "clone", "move")',
+	    type => 'string',
+	    default => 'migration',
+	    optional => 1,
+	},
+    },
+    'disk-import' => {
+	volname => {
+	    type => 'string',
+	    description => 'volume name to use as preferred target volume name',
+	},
+	format => PVE::JSONSchema::get_standard_option('pve-qm-image-format'),
+	export_formats => {
+	    type => 'string',
+	    description => 'list of supported export formats',
+	},
+	storage => {
+	    type => 'string',
+	    format => 'pve-storage-id',
+	},
+	snapshot => {
+	    description => "The current-state snapshot if the stream contains snapshots",
+	    type => 'string',
+	    pattern => qr/[a-z0-9_\-]{1,40}/i,
+	    optional => 1,
+	},
+	migration_snapshot => {
+	    type => 'boolean',
+	    optional => 1,
+	    description => '`snapshot` was created for migration and will be removed after import',
+	},
+	with_snapshots => {
+	    description => 'Whether the stream includes intermediate snapshots',
+	    type => 'boolean',
+	    optional => 1,
+	    default => 0,
+	},
+	allow_rename => {
+	    description => "Choose a new volume ID if the requested " .
+		"volume ID already exists, instead of throwing an error.",
+	    type => 'boolean',
+	    optional => 1,
+	    default => 0,
+	},
+    },
+    'query-disk-import' => {},
+};
+
+sub handle_disk_import {
+    my ($state, $params) = @_;
+
+    die "disk import already running as PID '$state->{disk_import}->{pid}'\n"
+	if $state->{disk_import}->{pid};
+
+    my $storage = delete $params->{storage};
+    my $format = delete $params->{format};
+    my $volname = delete $params->{volname};
+
+    my $import = PVE::Storage::volume_import_start($state->{storecfg}, $storage, $volname, $format, $state->{vmid}, $params);
+
+    my $socket = $import->{socket};
+    $format = delete $import->{format};
+
+    $state->{sockets}->{$socket} = 1;
+    $state->{disk_import} = $import;
+
+    chown $state->{socket_uid}, -1, $socket;
+
+    return {
+	socket => $socket,
+	format => $format,
+    };
+}
+
+sub handle_query_disk_import {
+    my ($state, $params) = @_;
+
+    die "no disk import running\n"
+	if !$state->{disk_import}->{pid};
+
+    my $pattern = PVE::Storage::volume_imported_message(undef, 1);
+
+    my $read_output = sub {
+	my ($timeout) = @_;
+
+	my $line;
+
+	eval {
+	    my $fh = $state->{disk_import}->{fh};
+	    PVE::Tools::run_with_timeout($timeout, sub { $line = <$fh>; });
+	    print "disk-import: $line\n" if $line;
+        };
+
+	return $line;
+    };
+
+    my $result = $read_output->(5);
+
+    # attempted read empty or timeout, and process has exited already
+    if (!$result && waitpid($state->{disk_import}->{pid}, WNOHANG)) {
+	my $msg = '';
+
+	# read any missed output
+	while (my $line = $read_output->(1)) {
+	    if ($line =~ $pattern) {
+		$result = $line;
+	    } else {
+		$msg .= "$line\n";
+	    }
+	}
+
+	my $unix = $state->{disk_import}->{socket};
+	unlink $unix;
+	delete $state->{sockets}->{$unix};
+	delete $state->{disk_import};
+
+	if (!$result) {
+	    $msg = "import process failed\n" if !$msg;
+	    return {
+		status => "error",
+		msg => $msg,
+	    };
+	}
+    }
+
+    if ($result && $result =~ $pattern) {
+	my $volid = $1;
+	waitpid($state->{disk_import}->{pid}, 0);
+
+	my $unix = $state->{disk_import}->{socket};
+	unlink $unix;
+	delete $state->{sockets}->{$unix};
+	delete $state->{disk_import};
+	$state->{cleanup}->{volumes}->{$volid} = 1;
+	return {
+	    status => "complete",
+	    volid => $volid,
+	};
+    } else {
+	return {
+	    status => "pending",
+	    msg => $result,
+	};
+    }
+}
+
+sub handle_bwlimit {
+    my ($params) = @_;
+
+    my $op = $params->{operation} // "migration";
+    my $storages = $params->{storages};
+    my $override = $params->{bwlimit};
+
+    return { bwlimit => PVE::Storage::get_bandwidth_limit($op, $storages, $override) };
+}
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 qemu-server 01/11] move map_storage to PVE::JSONSchema::map_id
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (6 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 guest-common 3/3] add storage tunnel module Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 02/11] schema: use pve-bridge-id Fabian Grünbichler
                   ` (15 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

since we are going to reuse the same mechanism/code for network bridge
mapping and pve-container.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    v5: move to pve-common/PVE::JSONSchema
    
    requires bumped pve-common with map_id (build and regular depends)

 PVE/QemuMigrate.pm |  6 +++---
 PVE/QemuServer.pm  | 17 +----------------
 2 files changed, 4 insertions(+), 19 deletions(-)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index c9bc39d2..a00c2459 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -342,7 +342,7 @@ sub prepare {
 	my $targetsid = $sid;
 	# NOTE: we currently ignore shared source storages in mappings so skip here too for now
 	if (!$scfg->{shared}) {
-	    $targetsid = PVE::QemuServer::map_storage($self->{opts}->{storagemap}, $sid);
+	    $targetsid = PVE::JSONSchema::map_id($self->{opts}->{storagemap}, $sid);
 	}
 
 	my $target_scfg = PVE::Storage::storage_check_enabled($storecfg, $targetsid, $self->{node});
@@ -408,7 +408,7 @@ sub scan_local_volumes {
 
 	    next if @{$dl->{$storeid}} == 0;
 
-	    my $targetsid = PVE::QemuServer::map_storage($self->{opts}->{storagemap}, $storeid);
+	    my $targetsid = PVE::JSONSchema::map_id($self->{opts}->{storagemap}, $storeid);
 	    # check if storage is available on target node
 	    my $target_scfg = PVE::Storage::storage_check_enabled(
 		$storecfg,
@@ -479,7 +479,7 @@ sub scan_local_volumes {
 	    my $targetsid = $sid;
 	    # NOTE: we currently ignore shared source storages in mappings so skip here too for now
 	    if (!$scfg->{shared}) {
-		$targetsid = PVE::QemuServer::map_storage($self->{opts}->{storagemap}, $sid);
+		$targetsid = PVE::JSONSchema::map_id($self->{opts}->{storagemap}, $sid);
 	    }
 
 	    PVE::Storage::storage_check_enabled($storecfg, $targetsid, $self->{node});
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 8aa19469..8c1aa30a 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -119,21 +119,6 @@ PVE::JSONSchema::register_standard_option('pve-qemu-machine', {
 	optional => 1,
 });
 
-
-sub map_storage {
-    my ($map, $source) = @_;
-
-    return $source if !defined($map);
-
-    return $map->{entries}->{$source}
-	if $map->{entries} && defined($map->{entries}->{$source});
-
-    return $map->{default} if $map->{default};
-
-    # identity (fallback)
-    return $source;
-}
-
 PVE::JSONSchema::register_standard_option('pve-targetstorage', {
     description => "Mapping from source to target storages. Providing only a single storage ID maps all source storages to that storage. Providing the special value '1' will map each source storage to itself.",
     type => 'string',
@@ -5279,7 +5264,7 @@ sub vm_migrate_alloc_nbd_disks {
 	# volume is not available there, fall back to the default format.
 	# Otherwise use the same format as the original.
 	if (!$storagemap->{identity}) {
-	    $storeid = map_storage($storagemap, $storeid);
+	    $storeid = PVE::JSONSchema::map_id($storagemap, $storeid);
 	    my ($defFormat, $validFormats) = PVE::Storage::storage_default_format($storecfg, $storeid);
 	    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
 	    my $fileFormat = qemu_img_format($scfg, $volname);
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 qemu-server 02/11] schema: use pve-bridge-id
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (7 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 01/11] move map_storage to PVE::JSONSchema::map_id Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 03/11] parse_config: optional strict mode Fabian Grünbichler
                   ` (14 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    requires pve-common with pve-bridge-id

 PVE/QemuServer.pm | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 8c1aa30a..8893f86e 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -914,13 +914,10 @@ my $net_fmt = {
         default_key => 1,
     },
     (map { $_ => { keyAlias => 'model', alias => 'macaddr' }} @$nic_model_list),
-    bridge => {
-	type => 'string',
+    bridge => get_standard_option('pve-bridge-id', {
 	description => $net_fmt_bridge_descr,
-	format_description => 'bridge',
-	pattern => '[-_.\w\d]+',
 	optional => 1,
-    },
+    }),
     queues => {
 	type => 'integer',
 	minimum => 0, maximum => 16,
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 qemu-server 03/11] parse_config: optional strict mode
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (8 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 02/11] schema: use pve-bridge-id Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 04/11] update_vm: allow simultaneous setting of boot-order and dev Fabian Grünbichler
                   ` (13 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

when passing a config from one cluster to another, we want to be strict
when parsing - it's better to fail the migration early and upgrade the
target node instead of failing the migration later (when significant
work for transferring disks and/or state has already been done) or not
at all, but silently lose config settings that the target doesn't
understand.

this also might be helpful in other cases - e.g. when restoring from a
backup.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    v4: fix bool inversion
    v3: new

 PVE/QemuServer.pm | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 8893f86e..2b7ac404 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -2312,7 +2312,7 @@ sub destroy_vm {
 }
 
 sub parse_vm_config {
-    my ($filename, $raw) = @_;
+    my ($filename, $raw, $strict) = @_;
 
     return if !defined($raw);
 
@@ -2322,6 +2322,16 @@ sub parse_vm_config {
 	pending => {},
     };
 
+    my $handle_error = sub {
+	my ($msg) = @_;
+
+	if ($strict) {
+	    die $msg;
+	} else {
+	    warn $msg;
+	}
+    };
+
     $filename =~ m|/qemu-server/(\d+)\.conf$|
 	|| die "got strange filename '$filename'";
 
@@ -2376,14 +2386,14 @@ sub parse_vm_config {
 	    if ($section eq 'pending') {
 		$conf->{delete} = $value; # we parse this later
 	    } else {
-		warn "vm $vmid - propertry 'delete' is only allowed in [PENDING]\n";
+		$handle_error->("vm $vmid - property 'delete' is only allowed in [PENDING]\n");
 	    }
 	} elsif ($line =~ m/^([a-z][a-z_]*\d*):\s*(.+?)\s*$/) {
 	    my $key = $1;
 	    my $value = $2;
 	    eval { $value = check_type($key, $value); };
 	    if ($@) {
-		warn "vm $vmid - unable to parse value of '$key' - $@";
+		$handle_error->("vm $vmid - unable to parse value of '$key' - $@");
 	    } else {
 		$key = 'ide2' if $key eq 'cdrom';
 		my $fmt = $confdesc->{$key}->{format};
@@ -2393,7 +2403,7 @@ sub parse_vm_config {
 			$v->{file} = $volid;
 			$value = print_drive($v);
 		    } else {
-			warn "vm $vmid - unable to parse value of '$key'\n";
+			$handle_error->("vm $vmid - unable to parse value of '$key'\n");
 			next;
 		    }
 		}
@@ -2401,7 +2411,7 @@ sub parse_vm_config {
 		$conf->{$key} = $value;
 	    }
 	} else {
-	    warn "vm $vmid - unable to parse config: $line\n";
+	    $handle_error->("vm $vmid - unable to parse config: $line\n");
 	}
     }
 
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 qemu-server 04/11] update_vm: allow simultaneous setting of boot-order and dev
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (9 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 03/11] parse_config: optional strict mode Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 05/11] nbd alloc helper: allow passing in explicit format Fabian Grünbichler
                   ` (12 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

else this fails if we check 'boot' before the device was put into
the config or pending section.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    as happens when doing a remote migration and the full config is passed through
    update_vm_api

 PVE/API2/Qemu.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index a359d096..9be1caf3 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -1411,7 +1411,7 @@ my $update_vm_api  = sub {
 			if ($new_bootcfg->{order}) {
 			    my @devs = PVE::Tools::split_list($new_bootcfg->{order});
 			    for my $dev (@devs) {
-				my $exists = $conf->{$dev} || $conf->{pending}->{$dev};
+				my $exists = $conf->{$dev} || $conf->{pending}->{$dev} || $param->{$dev};
 				my $deleted = grep {$_ eq $dev} @delete;
 				die "invalid bootorder: device '$dev' does not exist'\n"
 				    if !$exists || $deleted;
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 qemu-server 05/11] nbd alloc helper: allow passing in explicit format
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (10 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 04/11] update_vm: allow simultaneous setting of boot-order and dev Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 06/11] migrate: move tunnel-helpers to pve-guest-common Fabian Grünbichler
                   ` (11 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

and make $volname optional, to support remote migration usage without
code duplication.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    v2: new

 PVE/QemuServer.pm | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 2b7ac404..a4d6487f 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5254,11 +5254,9 @@ sub vm_migrate_get_nbd_disks {
 sub vm_migrate_alloc_nbd_disks {
     my ($storecfg, $vmid, $source_volumes, $storagemap) = @_;
 
-    my $format = undef;
-
     my $nbd = {};
     foreach my $opt (sort keys %$source_volumes) {
-	my ($volid, $storeid, $volname, $drive, $use_existing) = @{$source_volumes->{$opt}};
+	my ($volid, $storeid, $volname, $drive, $use_existing, $format) = @{$source_volumes->{$opt}};
 
 	if ($use_existing) {
 	    $nbd->{$opt}->{drivestr} = print_drive($drive);
@@ -5267,16 +5265,26 @@ sub vm_migrate_alloc_nbd_disks {
 	    next;
 	}
 
-	# If a remote storage is specified and the format of the original
-	# volume is not available there, fall back to the default format.
-	# Otherwise use the same format as the original.
+	# storage mapping + volname = regular migration
+	# storage mapping + format = remote migration
+	# order of precedence, filtered by whether storage supports it:
+	# 1. explicit requested format
+	# 2. format of current volume
+	# 3. default format of storage
 	if (!$storagemap->{identity}) {
 	    $storeid = PVE::JSONSchema::map_id($storagemap, $storeid);
 	    my ($defFormat, $validFormats) = PVE::Storage::storage_default_format($storecfg, $storeid);
-	    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
-	    my $fileFormat = qemu_img_format($scfg, $volname);
-	    $format = (grep {$fileFormat eq $_} @{$validFormats}) ? $fileFormat : $defFormat;
+	    if (!$format || !grep { $format eq $_ } @$validFormats) {
+		if ($volname) {
+		    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
+		    my $fileFormat = qemu_img_format($scfg, $volname);
+		    $format = $fileFormat
+			if grep { $fileFormat eq $_ } @$validFormats;
+		}
+		$format //= $defFormat;
+	    }
 	} else {
+	    # can't happen for remote migration, so $volname is always defined
 	    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
 	    $format = qemu_img_format($scfg, $volname);
 	}
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 qemu-server 06/11] migrate: move tunnel-helpers to pve-guest-common
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (11 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 05/11] nbd alloc helper: allow passing in explicit format Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 07/11] mtunnel: add API endpoints Fabian Grünbichler
                   ` (10 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

besides the log calls these don't need any parts of the migration state,
so let's make them generic and re-use them for container migration and
replication in the future.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    v4: switch log to use two-parameter logging sub like migration
    new in v3, requires bumped libpve-guest-common-perl

 PVE/QemuMigrate.pm                    | 183 ++------------------------
 test/MigrationTest/QemuMigrateMock.pm |  28 ++--
 2 files changed, 26 insertions(+), 185 deletions(-)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index a00c2459..104e62ce 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -18,6 +18,7 @@ use PVE::ReplicationConfig;
 use PVE::ReplicationState;
 use PVE::Storage;
 use PVE::Tools;
+use PVE::Tunnel;
 
 use PVE::QemuConfig;
 use PVE::QemuServer::CPUConfig;
@@ -30,180 +31,16 @@ use PVE::QemuServer;
 use PVE::AbstractMigrate;
 use base qw(PVE::AbstractMigrate);
 
-sub fork_command_pipe {
-    my ($self, $cmd) = @_;
-
-    my $reader = IO::File->new();
-    my $writer = IO::File->new();
-
-    my $orig_pid = $$;
-
-    my $cpid;
-
-    eval { $cpid = open2($reader, $writer, @$cmd); };
-
-    my $err = $@;
-
-    # catch exec errors
-    if ($orig_pid != $$) {
-	$self->log('err', "can't fork command pipe\n");
-	POSIX::_exit(1);
-	kill('KILL', $$);
-    }
-
-    die $err if $err;
-
-    return { writer => $writer, reader => $reader, pid => $cpid };
-}
-
-sub finish_command_pipe {
-    my ($self, $cmdpipe, $timeout) = @_;
-
-    my $cpid = $cmdpipe->{pid};
-    return if !defined($cpid);
-
-    my $writer = $cmdpipe->{writer};
-    my $reader = $cmdpipe->{reader};
-
-    $writer->close();
-    $reader->close();
-
-    my $collect_child_process = sub {
-	my $res = waitpid($cpid, WNOHANG);
-	if (defined($res) && ($res == $cpid)) {
-	    delete $cmdpipe->{cpid};
-	    return 1;
-	} else {
-	    return 0;
-	}
-     };
-
-    if ($timeout) {
-	for (my $i = 0; $i < $timeout; $i++) {
-	    return if &$collect_child_process();
-	    sleep(1);
-	}
-    }
-
-    $self->log('info', "ssh tunnel still running - terminating now with SIGTERM\n");
-    kill(15, $cpid);
-
-    # wait again
-    for (my $i = 0; $i < 10; $i++) {
-	return if &$collect_child_process();
-	sleep(1);
-    }
-
-    $self->log('info', "ssh tunnel still running - terminating now with SIGKILL\n");
-    kill 9, $cpid;
-    sleep 1;
-
-    $self->log('err', "ssh tunnel child process (PID $cpid) couldn't be collected\n")
-	if !&$collect_child_process();
-}
-
-sub read_tunnel {
-    my ($self, $tunnel, $timeout) = @_;
-
-    $timeout = 60 if !defined($timeout);
-
-    my $reader = $tunnel->{reader};
-
-    my $output;
-    eval {
-	PVE::Tools::run_with_timeout($timeout, sub { $output = <$reader>; });
-    };
-    die "reading from tunnel failed: $@\n" if $@;
-
-    chomp $output;
-
-    return $output;
-}
-
-sub write_tunnel {
-    my ($self, $tunnel, $timeout, $command) = @_;
-
-    $timeout = 60 if !defined($timeout);
-
-    my $writer = $tunnel->{writer};
-
-    eval {
-	PVE::Tools::run_with_timeout($timeout, sub {
-	    print $writer "$command\n";
-	    $writer->flush();
-	});
-    };
-    die "writing to tunnel failed: $@\n" if $@;
-
-    if ($tunnel->{version} && $tunnel->{version} >= 1) {
-	my $res = eval { $self->read_tunnel($tunnel, 10); };
-	die "no reply to command '$command': $@\n" if $@;
-
-	if ($res eq 'OK') {
-	    return;
-	} else {
-	    die "tunnel replied '$res' to command '$command'\n";
-	}
-    }
-}
-
 sub fork_tunnel {
     my ($self, $ssh_forward_info) = @_;
 
-    my @localtunnelinfo = ();
-    foreach my $addr (@$ssh_forward_info) {
-	push @localtunnelinfo, '-L', $addr;
-    }
-
-    my $cmd = [@{$self->{rem_ssh}}, '-o ExitOnForwardFailure=yes', @localtunnelinfo, '/usr/sbin/qm', 'mtunnel' ];
-
-    my $tunnel = $self->fork_command_pipe($cmd);
-
-    eval {
-	my $helo = $self->read_tunnel($tunnel, 60);
-	die "no reply\n" if !$helo;
-	die "no quorum on target node\n" if $helo =~ m/^no quorum$/;
-	die "got strange reply from mtunnel ('$helo')\n"
-	    if $helo !~ m/^tunnel online$/;
+    my $cmd = ['/usr/sbin/qm', 'mtunnel'];
+    my $log = sub {
+	my ($level, $msg) = @_;
+	$self->log($level, $msg);
     };
-    my $err = $@;
-
-    eval {
-	my $ver = $self->read_tunnel($tunnel, 10);
-	if ($ver =~ /^ver (\d+)$/) {
-	    $tunnel->{version} = $1;
-	    $self->log('info', "ssh tunnel $ver\n");
-	} else {
-	    $err = "received invalid tunnel version string '$ver'\n" if !$err;
-	}
-    };
-
-    if ($err) {
-	$self->finish_command_pipe($tunnel);
-	die "can't open migration tunnel - $err";
-    }
-    return $tunnel;
-}
-
-sub finish_tunnel {
-    my ($self, $tunnel) = @_;
-
-    eval { $self->write_tunnel($tunnel, 30, 'quit'); };
-    my $err = $@;
-
-    $self->finish_command_pipe($tunnel, 30);
-
-    if (my $unix_sockets = $tunnel->{unix_sockets}) {
-	# ssh does not clean up on local host
-	my $cmd = ['rm', '-f', @$unix_sockets];
-	PVE::Tools::run_command($cmd);
-
-	# .. and just to be sure check on remote side
-	unshift @{$cmd}, @{$self->{rem_ssh}};
-	PVE::Tools::run_command($cmd);
-    }
 
-    die $err if $err;
+    return PVE::Tunnel::fork_ssh_tunnel($self->{rem_ssh}, $cmd, $ssh_forward_info, $log);
 }
 
 sub start_remote_tunnel {
@@ -244,7 +81,7 @@ sub start_remote_tunnel {
 	    }
 	    if ($unix_socket_try > 100) {
 		$self->{errors} = 1;
-		$self->finish_tunnel($self->{tunnel});
+		PVE::Tunnel::finish_tunnel($self->{tunnel});
 		die "Timeout, migration socket $ruri did not get ready";
 	    }
 	    $self->{tunnel}->{unix_sockets} = $unix_sockets if (@$unix_sockets);
@@ -1254,7 +1091,7 @@ sub phase2_cleanup {
 
 
     if ($self->{tunnel}) {
-	eval { finish_tunnel($self, $self->{tunnel});  };
+	eval { PVE::Tunnel::finish_tunnel($self->{tunnel});  };
 	if (my $err = $@) {
 	    $self->log('err', $err);
 	    $self->{errors} = 1;
@@ -1312,7 +1149,7 @@ sub phase3_cleanup {
 	# config moved and nbd server stopped - now we can resume vm on target
 	if ($tunnel && $tunnel->{version} && $tunnel->{version} >= 1) {
 	    eval {
-		$self->write_tunnel($tunnel, 30, "resume $vmid");
+		PVE::Tunnel::write_tunnel($tunnel, 30, "resume $vmid");
 	    };
 	    if (my $err = $@) {
 		$self->log('err', $err);
@@ -1339,7 +1176,7 @@ sub phase3_cleanup {
 
     # close tunnel on successful migration, on error phase2_cleanup closed it
     if ($tunnel) {
-	eval { finish_tunnel($self, $tunnel);  };
+	eval { PVE::Tunnel::finish_tunnel($tunnel); };
 	if (my $err = $@) {
 	    $self->log('err', $err);
 	    $self->{errors} = 1;
diff --git a/test/MigrationTest/QemuMigrateMock.pm b/test/MigrationTest/QemuMigrateMock.pm
index 8e0b7d09..f2c02819 100644
--- a/test/MigrationTest/QemuMigrateMock.pm
+++ b/test/MigrationTest/QemuMigrateMock.pm
@@ -51,12 +51,26 @@ $MigrationTest::Shared::qemu_config_module->mock(
     },
 );
 
-my $qemu_migrate_module = Test::MockModule->new("PVE::QemuMigrate");
-$qemu_migrate_module->mock(
+my $tunnel_module = Test::MockModule->new("PVE::Tunnel");
+$tunnel_module->mock(
     finish_tunnel => sub {
 	delete $expected_calls->{'finish_tunnel'};
 	return;
     },
+    write_tunnel => sub {
+	my ($tunnel, $timeout, $command) = @_;
+
+	if ($command =~ m/^resume (\d+)$/) {
+	    my $vmid = $1;
+	    die "resuming wrong VM '$vmid'\n" if $vmid ne $test_vmid;
+	    return;
+	}
+	die "write_tunnel (mocked) - implement me: $command\n";
+    },
+);
+
+my $qemu_migrate_module = Test::MockModule->new("PVE::QemuMigrate");
+$qemu_migrate_module->mock(
     fork_tunnel => sub {
 	die "fork_tunnel (mocked) - implement me\n"; # currently no call should lead here
     },
@@ -73,16 +87,6 @@ $qemu_migrate_module->mock(
 	    version => 1,
 	};
     },
-    write_tunnel => sub {
-	my ($self, $tunnel, $timeout, $command) = @_;
-
-	if ($command =~ m/^resume (\d+)$/) {
-	    my $vmid = $1;
-	    die "resuming wrong VM '$vmid'\n" if $vmid ne $test_vmid;
-	    return;
-	}
-	die "write_tunnel (mocked) - implement me: $command\n";
-    },
     log => sub {
 	my ($self, $level, $message) = @_;
 	$current_log .= "$level: $message\n";
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 qemu-server 07/11] mtunnel: add API endpoints
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (12 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 06/11] migrate: move tunnel-helpers to pve-guest-common Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-11 13:01   ` Fabian Ebner
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 08/11] migrate: refactor remote VM/tunnel start Fabian Grünbichler
                   ` (9 subsequent siblings)
  23 siblings, 1 reply; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

the following two endpoints are used for migration on the remote side

POST /nodes/NODE/qemu/VMID/mtunnel

which creates and locks an empty VM config, and spawns the main qmtunnel
worker which binds to a VM-specific UNIX socket.

this worker handles JSON-encoded migration commands coming in via this
UNIX socket:
- config (set target VM config)
-- checks permissions for updating config
-- strips pending changes and snapshots
-- sets (optional) firewall config
- disk (allocate disk for NBD migration)
-- checks permission for target storage
-- returns drive string for allocated volume
- disk-import, query-disk-import, bwlimit
-- handled by PVE::StorageTunnel
- start (returning migration info)
- fstrim (via agent)
- ticket (creates a ticket for a WS connection to a specific socket)
- resume
- stop
- nbdstop
- unlock
- quit (+ cleanup)

this worker serves as a replacement for both 'qm mtunnel' and various
manual calls via SSH. the API call will return a ticket valid for
connecting to the worker's UNIX socket via a websocket connection.

GET+WebSocket upgrade /nodes/NODE/qemu/VMID/mtunnelwebsocket

gets called for connecting to a UNIX socket via websocket forwarding,
i.e. once for the main command mtunnel, and once each for the memory
migration and each NBD drive-mirror/storage migration.

access is guarded by a short-lived ticket binding the authenticated user
to the socket path. such tickets can be requested over the main mtunnel,
which keeps track of socket paths currently used by that
mtunnel/migration instance.

each command handler should check privileges for the requested action if
necessary.

both mtunnel and mtunnelwebsocket endpoints are not proxied, the
client/caller is responsible for ensuring the passed 'node' parameter
and the endpoint handling the call are matching.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    v4:
    - add timeout to accept()
    - move 'bwlimit' to PVE::StorageTunnel and extend it
    - mark mtunnel(websocket) as non-proxied, and check $node accordingly
    v3:
    - handle meta and vmgenid better
    - handle failure of 'config' updating
    - move 'disk-import' and 'query-disk-import' handlers to pve-guest-common
    - improve tunnel exit by letting client close the connection
    - use strict VM config parser
    v2: incorporated Fabian Ebner's feedback, mainly:
    - use modified nbd alloc helper instead of duplicating
    - fix disk cleanup, also cleanup imported disks
    - fix firewall-conf vs firewall-config mismatch
    
    requires
    - pve-access-control with tunnel ticket support (already marked in d/control)
    - pve-http-server with websocket fixes (could be done via breaks? or bumped in
      pve-manager..)
    - pve-guest-common with PVE::StorageTunnel (not yet marked - depends on when
      this series gets applied)
    
    new in v4

 PVE/API2/Qemu.pm | 523 ++++++++++++++++++++++++++++++++++++++++++++++-
 debian/control   |   2 +-
 2 files changed, 523 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 9be1caf3..78313a28 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4,10 +4,13 @@ use strict;
 use warnings;
 use Cwd 'abs_path';
 use Net::SSLeay;
-use POSIX;
 use IO::Socket::IP;
+use IO::Socket::UNIX;
+use IPC::Open3;
+use JSON;
 use URI::Escape;
 use Crypt::OpenSSL::Random;
+use Socket qw(SOCK_STREAM);
 
 use PVE::Cluster qw (cfs_read_file cfs_write_file);;
 use PVE::RRD;
@@ -37,6 +40,7 @@ use PVE::VZDump::Plugin;
 use PVE::DataCenterConfig;
 use PVE::SSHInfo;
 use PVE::Replication;
+use PVE::StorageTunnel;
 
 BEGIN {
     if (!$ENV{PVE_GENERATING_DOCS}) {
@@ -857,6 +861,7 @@ __PACKAGE__->register_method({
 	    { subdir => 'spiceproxy' },
 	    { subdir => 'sendkey' },
 	    { subdir => 'firewall' },
+	    { subdir => 'mtunnel' },
 	    ];
 
 	return $res;
@@ -4667,4 +4672,520 @@ __PACKAGE__->register_method({
 	return PVE::QemuServer::Cloudinit::dump_cloudinit_config($conf, $param->{vmid}, $param->{type});
     }});
 
+__PACKAGE__->register_method({
+    name => 'mtunnel',
+    path => '{vmid}/mtunnel',
+    method => 'POST',
+    protected => 1,
+    description => 'Migration tunnel endpoint - only for internal use by VM migration.',
+    permissions => {
+	check => ['perm', '/vms/{vmid}', [ 'VM.Allocate' ]],
+	description => "You need 'VM.Allocate' permissions on /vms/{vmid}. Further permission checks happen during the actual migration.",
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    vmid => get_standard_option('pve-vmid'),
+	    storages => {
+		type => 'string',
+		format => 'pve-storage-id-list',
+		optional => 1,
+		description => 'List of storages to check permission and availability. Will be checked again for all actually used storages during migration.',
+	    },
+	},
+    },
+    returns => {
+	additionalProperties => 0,
+	properties => {
+	    upid => { type => 'string' },
+	    ticket => { type => 'string' },
+	    socket => { 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 $storages = extract_param($param, 'storages');
+
+	my $nodename = PVE::INotify::nodename();
+
+	raise_param_exc({ node => "node needs to be 'localhost' or local hostname '$nodename'" })
+	    if $node ne 'localhost' && $node ne $nodename;
+
+	$node = $nodename;
+
+	my $storecfg = PVE::Storage::config();
+	foreach my $storeid (PVE::Tools::split_list($storages)) {
+	    $check_storage_access_migrate->($rpcenv, $authuser, $storecfg, $storeid, $node);
+	}
+
+	PVE::Cluster::check_cfs_quorum();
+
+	my $socket_addr = "/run/qemu-server/$vmid.mtunnel";
+
+	my $lock = 'create';
+	eval { PVE::QemuConfig->create_and_lock_config($vmid, 0, $lock); };
+
+	raise_param_exc({ vmid => "unable to create empty VM config - $@"})
+	    if $@;
+
+	my $realcmd = sub {
+	    my $state = {
+		storecfg => PVE::Storage::config(),
+		lock => $lock,
+		vmid => $vmid,
+	    };
+
+	    my $run_locked = sub {
+		my ($code, $params) = @_;
+		return PVE::QemuConfig->lock_config($state->{vmid}, sub {
+		    my $conf = PVE::QemuConfig->load_config($state->{vmid});
+
+		    $state->{conf} = $conf;
+
+		    die "Encountered wrong lock - aborting mtunnel command handling.\n"
+			if $state->{lock} && !PVE::QemuConfig->has_lock($conf, $state->{lock});
+
+		    return $code->($params);
+		});
+	    };
+
+	    my $cmd_desc = {
+		config => {
+		    conf => {
+			type => 'string',
+			description => 'Full VM config, adapted for target cluster/node',
+		    },
+		    'firewall-config' => {
+			type => 'string',
+			description => 'VM firewall config',
+			optional => 1,
+		    },
+		},
+		disk => {
+		    format => PVE::JSONSchema::get_standard_option('pve-qm-image-format'),
+		    storage => {
+			type => 'string',
+			format => 'pve-storage-id',
+		    },
+		    drive => {
+			type => 'object',
+			description => 'parsed drive information without volid and format',
+		    },
+		},
+		start => {
+		    start_params => {
+			type => 'object',
+			description => 'params passed to vm_start_nolock',
+		    },
+		    migrate_opts => {
+			type => 'object',
+			description => 'migrate_opts passed to vm_start_nolock',
+		    },
+		},
+		ticket => {
+		    path => {
+			type => 'string',
+			description => 'socket path for which the ticket should be valid. must be known to current mtunnel instance.',
+		    },
+		},
+		quit => {
+		    cleanup => {
+			type => 'boolean',
+			description => 'remove VM config and disks, aborting migration',
+			default => 0,
+		    },
+		},
+		'disk-import' => $PVE::StorageTunnel::cmd_schema->{'disk-import'},
+		'query-disk-import' => $PVE::StorageTunnel::cmd_schema->{'query-disk-import'},
+		bwlimit => $PVE::StorageTunnel::cmd_schema->{bwlimit},
+	    };
+
+	    my $cmd_handlers = {
+		'version' => sub {
+		    # compared against other end's version
+		    # bump/reset for breaking changes
+		    # bump/bump for opt-in changes
+		    return {
+			api => 2,
+			age => 0,
+		    };
+		},
+		'config' => sub {
+		    my ($params) = @_;
+
+		    # parse and write out VM FW config if given
+		    if (my $fw_conf = $params->{'firewall-config'}) {
+			my ($path, $fh) = PVE::Tools::tempfile_contents($fw_conf, 700);
+
+			my $empty_conf = {
+			    rules => [],
+			    options => {},
+			    aliases => {},
+			    ipset => {} ,
+			    ipset_comments => {},
+			};
+			my $cluster_fw_conf = PVE::Firewall::load_clusterfw_conf();
+
+			# TODO: add flag for strict parsing?
+			# TODO: add import sub that does all this given raw content?
+			my $vmfw_conf = PVE::Firewall::generic_fw_config_parser($path, $cluster_fw_conf, $empty_conf, 'vm');
+			$vmfw_conf->{vmid} = $state->{vmid};
+			PVE::Firewall::save_vmfw_conf($state->{vmid}, $vmfw_conf);
+
+			$state->{cleanup}->{fw} = 1;
+		    }
+
+		    my $conf_fn = "incoming/qemu-server/$state->{vmid}.conf";
+		    my $new_conf = PVE::QemuServer::parse_vm_config($conf_fn, $params->{conf}, 1);
+		    delete $new_conf->{lock};
+		    delete $new_conf->{digest};
+
+		    # TODO handle properly?
+		    delete $new_conf->{snapshots};
+		    delete $new_conf->{parent};
+		    delete $new_conf->{pending};
+
+		    # not handled by update_vm_api
+		    my $vmgenid = delete $new_conf->{vmgenid};
+		    my $meta = delete $new_conf->{meta};
+
+		    $new_conf->{vmid} = $state->{vmid};
+		    $new_conf->{node} = $node;
+
+		    PVE::QemuConfig->remove_lock($state->{vmid}, 'create');
+
+		    eval {
+			$update_vm_api->($new_conf, 1);
+		    };
+		    if (my $err = $@) {
+			# revert to locked previous config
+			my $conf = PVE::QemuConfig->load_config($state->{vmid});
+			$conf->{lock} = 'create';
+			PVE::QemuConfig->write_config($state->{vmid}, $conf);
+
+			die $err;
+		    }
+
+		    my $conf = PVE::QemuConfig->load_config($state->{vmid});
+		    $conf->{lock} = 'migrate';
+		    $conf->{vmgenid} = $vmgenid if $vmgenid;
+		    $conf->{meta} = $meta if $meta;
+		    PVE::QemuConfig->write_config($state->{vmid}, $conf);
+
+		    $state->{lock} = 'migrate';
+
+		    return;
+		},
+		'bwlimit' => sub {
+		    my ($params) = @_;
+		    return PVE::StorageTunnel::handle_bwlimit($params);
+		},
+		'disk' => sub {
+		    my ($params) = @_;
+
+		    my $format = $params->{format};
+		    my $storeid = $params->{storage};
+		    my $drive = $params->{drive};
+
+		    $check_storage_access_migrate->($rpcenv, $authuser, $state->{storecfg}, $storeid, $node);
+
+		    my $storagemap = {
+			default => $storeid,
+		    };
+
+		    my $source_volumes = {
+			'disk' => [
+			    undef,
+			    $storeid,
+			    undef,
+			    $drive,
+			    0,
+			    $format,
+			],
+		    };
+
+		    my $res = PVE::QemuServer::vm_migrate_alloc_nbd_disks($state->{storecfg}, $state->{vmid}, $source_volumes, $storagemap);
+		    if (defined($res->{disk})) {
+			$state->{cleanup}->{volumes}->{$res->{disk}->{volid}} = 1;
+			return $res->{disk};
+		    } else {
+			die "failed to allocate NBD disk..\n";
+		    }
+		},
+		'disk-import' => sub {
+		    my ($params) = @_;
+
+		    $check_storage_access_migrate->(
+			$rpcenv,
+			$authuser,
+			$state->{storecfg},
+			$params->{storage},
+			$node
+		    );
+
+		    $params->{unix} = "/run/qemu-server/$state->{vmid}.storage";
+
+		    return PVE::StorageTunnel::handle_disk_import($state, $params);
+		},
+		'query-disk-import' => sub {
+		    my ($params) = @_;
+
+		    return PVE::StorageTunnel::handle_query_disk_import($state, $params);
+		},
+		'start' => sub {
+		    my ($params) = @_;
+
+		    my $info = PVE::QemuServer::vm_start_nolock(
+			$state->{storecfg},
+			$state->{vmid},
+			$state->{conf},
+			$params->{start_params},
+			$params->{migrate_opts},
+		    );
+
+
+		    if ($info->{migrate}->{proto} ne 'unix') {
+			PVE::QemuServer::vm_stop(undef, $state->{vmid}, 1, 1);
+			die "migration over non-UNIX sockets not possible\n";
+		    }
+
+		    my $socket = $info->{migrate}->{addr};
+		    chown $state->{socket_uid}, -1, $socket;
+		    $state->{sockets}->{$socket} = 1;
+
+		    my $unix_sockets = $info->{migrate}->{unix_sockets};
+		    foreach my $socket (@$unix_sockets) {
+			chown $state->{socket_uid}, -1, $socket;
+			$state->{sockets}->{$socket} = 1;
+		    }
+		    return $info;
+		},
+		'fstrim' => sub {
+		    if (PVE::QemuServer::qga_check_running($state->{vmid})) {
+			eval { mon_cmd($state->{vmid}, "guest-fstrim") };
+			warn "fstrim failed: $@\n" if $@;
+		    }
+		    return;
+		},
+		'stop' => sub {
+		    PVE::QemuServer::vm_stop(undef, $state->{vmid}, 1, 1);
+		    return;
+		},
+		'nbdstop' => sub {
+		    PVE::QemuServer::nbd_stop($state->{vmid});
+		    return;
+		},
+		'resume' => sub {
+		    if (PVE::QemuServer::check_running($state->{vmid}, 1)) {
+			PVE::QemuServer::vm_resume($state->{vmid}, 1, 1);
+		    } else {
+			die "VM $state->{vmid} not running\n";
+		    }
+		    return;
+		},
+		'unlock' => sub {
+		    PVE::QemuConfig->remove_lock($state->{vmid}, $state->{lock});
+		    delete $state->{lock};
+		    return;
+		},
+		'ticket' => sub {
+		    my ($params) = @_;
+
+		    my $path = $params->{path};
+
+		    die "Not allowed to generate ticket for unknown socket '$path'\n"
+			if !defined($state->{sockets}->{$path});
+
+		    return { ticket => PVE::AccessControl::assemble_tunnel_ticket($authuser, "/socket/$path") };
+		},
+		'quit' => sub {
+		    my ($params) = @_;
+
+		    if ($params->{cleanup}) {
+			if ($state->{cleanup}->{fw}) {
+			    PVE::Firewall::remove_vmfw_conf($state->{vmid});
+			}
+
+			for my $volid (keys $state->{cleanup}->{volumes}->%*) {
+			    print "freeing volume '$volid' as part of cleanup\n";
+			    eval { PVE::Storage::vdisk_free($state->{storecfg}, $volid) };
+			    warn $@ if $@;
+			}
+
+			PVE::QemuServer::destroy_vm($state->{storecfg}, $state->{vmid}, 1);
+		    }
+
+		    print "switching to exit-mode, waiting for client to disconnect\n";
+		    $state->{exit} = 1;
+		    return;
+		},
+	    };
+
+	    $run_locked->(sub {
+		my $socket_addr = "/run/qemu-server/$state->{vmid}.mtunnel";
+		unlink $socket_addr;
+
+		$state->{socket} = IO::Socket::UNIX->new(
+	            Type => SOCK_STREAM(),
+		    Local => $socket_addr,
+		    Listen => 1,
+		);
+
+		$state->{socket_uid} = getpwnam('www-data')
+		    or die "Failed to resolve user 'www-data' to numeric UID\n";
+		chown $state->{socket_uid}, -1, $socket_addr;
+	    });
+
+	    print "mtunnel started\n";
+
+	    my $conn = eval { PVE::Tools::run_with_timeout(300, sub { $state->{socket}->accept() }) };
+	    if ($@) {
+		warn "Failed to accept tunnel connection - $@\n";
+
+		warn "Removing tunnel socket..\n";
+		unlink $state->{socket};
+
+		warn "Removing temporary VM config..\n";
+		$run_locked->(sub {
+		    PVE::QemuServer::destroy_vm($state->{storecfg}, $state->{vmid}, 1);
+		});
+
+		die "Exiting mtunnel\n";
+	    }
+
+	    $state->{conn} = $conn;
+
+	    my $reply_err = sub {
+		my ($msg) = @_;
+
+		my $reply = JSON::encode_json({
+		    success => JSON::false,
+		    msg => $msg,
+		});
+		$conn->print("$reply\n");
+		$conn->flush();
+	    };
+
+	    my $reply_ok = sub {
+		my ($res) = @_;
+
+		$res->{success} = JSON::true;
+		my $reply = JSON::encode_json($res);
+		$conn->print("$reply\n");
+		$conn->flush();
+	    };
+
+	    while (my $line = <$conn>) {
+		chomp $line;
+
+		# untaint, we validate below if needed
+		($line) = $line =~ /^(.*)$/;
+		my $parsed = eval { JSON::decode_json($line) };
+		if ($@) {
+		    $reply_err->("failed to parse command - $@");
+		    next;
+		}
+
+		my $cmd = delete $parsed->{cmd};
+		if (!defined($cmd)) {
+		    $reply_err->("'cmd' missing");
+		} elsif ($state->{exit}) {
+		    $reply_err->("tunnel is in exit-mode, processing '$cmd' cmd not possible");
+		    next;
+		} elsif (my $handler = $cmd_handlers->{$cmd}) {
+		    print "received command '$cmd'\n";
+		    eval {
+			if ($cmd_desc->{$cmd}) {
+			    PVE::JSONSchema::validate($cmd_desc->{$cmd}, $parsed);
+			} else {
+			    $parsed = {};
+			}
+			my $res = $run_locked->($handler, $parsed);
+			$reply_ok->($res);
+		    };
+		    $reply_err->("failed to handle '$cmd' command - $@")
+			if $@;
+		} else {
+		    $reply_err->("unknown command '$cmd' given");
+		}
+	    }
+
+	    if ($state->{exit}) {
+		print "mtunnel exited\n";
+	    } else {
+		die "mtunnel exited unexpectedly\n";
+	    }
+	};
+
+	my $ticket = PVE::AccessControl::assemble_tunnel_ticket($authuser, "/socket/$socket_addr");
+	my $upid = $rpcenv->fork_worker('qmtunnel', $vmid, $authuser, $realcmd);
+
+	return {
+	    ticket => $ticket,
+	    upid => $upid,
+	    socket => $socket_addr,
+	};
+    }});
+
+__PACKAGE__->register_method({
+    name => 'mtunnelwebsocket',
+    path => '{vmid}/mtunnelwebsocket',
+    method => 'GET',
+    permissions => {
+	description => "You need to pass a ticket valid for the selected socket. Tickets can be created via the mtunnel API call, which will check permissions accordingly.",
+        user => 'all', # check inside
+    },
+    description => 'Migration tunnel endpoint for websocket upgrade - only for internal use by VM migration.',
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    vmid => get_standard_option('pve-vmid'),
+	    socket => {
+		type => "string",
+		description => "unix socket to forward to",
+	    },
+	    ticket => {
+		type => "string",
+		description => "ticket return by initial 'mtunnel' API call, or retrieved via 'ticket' tunnel command",
+	    },
+	},
+    },
+    returns => {
+	type => "object",
+	properties => {
+	    port => { type => 'string', optional => 1 },
+	    socket => { type => 'string', optional => 1 },
+	},
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+
+	my $nodename = PVE::INotify::nodename();
+	my $node = extract_param($param, 'node');
+
+	raise_param_exc({ node => "node needs to be 'localhost' or local hostname '$nodename'" })
+	    if $node ne 'localhost' && $node ne $nodename;
+
+	my $vmid = $param->{vmid};
+	# check VM exists
+	PVE::QemuConfig->load_config($vmid);
+
+	my $socket = $param->{socket};
+	PVE::AccessControl::verify_tunnel_ticket($param->{ticket}, $authuser, "/socket/$socket");
+
+	return { socket => $socket };
+    }});
+
 1;
diff --git a/debian/control b/debian/control
index 6cf471a6..8d0b856e 100644
--- a/debian/control
+++ b/debian/control
@@ -33,7 +33,7 @@ Depends: dbus,
          libjson-perl,
          libjson-xs-perl,
          libnet-ssleay-perl,
-         libpve-access-control (>= 5.0-7),
+         libpve-access-control (>= 7.0-7),
          libpve-cluster-perl,
          libpve-common-perl (>= 7.0-14),
          libpve-guest-common-perl (>= 3.1-3),
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 qemu-server 08/11] migrate: refactor remote VM/tunnel start
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (13 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 07/11] mtunnel: add API endpoints Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-11 13:01   ` Fabian Ebner
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 09/11] migrate: add remote migration handling Fabian Grünbichler
                   ` (8 subsequent siblings)
  23 siblings, 1 reply; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

no semantic changes intended, except for:
- no longer passing the main migration UNIX socket to SSH twice for
forwarding
- dropping the 'unix:' prefix in start_remote_tunnel's timeout error message

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 PVE/QemuMigrate.pm | 158 ++++++++++++++++++++++++++++-----------------
 PVE/QemuServer.pm  |  34 +++++-----
 2 files changed, 113 insertions(+), 79 deletions(-)

diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index 104e62ce..e6cb7e79 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -43,19 +43,24 @@ sub fork_tunnel {
     return PVE::Tunnel::fork_ssh_tunnel($self->{rem_ssh}, $cmd, $ssh_forward_info, $log);
 }
 
+# tunnel_info:
+#   proto: unix (secure) or tcp (insecure/legacy compat)
+#   addr: IP or UNIX socket path
+#   port: optional TCP port
+#   unix_sockets: additional UNIX socket paths to forward
 sub start_remote_tunnel {
-    my ($self, $raddr, $rport, $ruri, $unix_socket_info) = @_;
+    my ($self, $tunnel_info) = @_;
 
     my $nodename = PVE::INotify::nodename();
     my $migration_type = $self->{opts}->{migration_type};
 
     if ($migration_type eq 'secure') {
 
-	if ($ruri =~ /^unix:/) {
-	    my $ssh_forward_info = ["$raddr:$raddr"];
-	    $unix_socket_info->{$raddr} = 1;
+	if ($tunnel_info->{proto} eq 'unix') {
+	    my $ssh_forward_info = [];
 
-	    my $unix_sockets = [ keys %$unix_socket_info ];
+	    my $unix_sockets = [ keys %{$tunnel_info->{unix_sockets}} ];
+	    push @$unix_sockets, $tunnel_info->{addr};
 	    for my $sock (@$unix_sockets) {
 		push @$ssh_forward_info, "$sock:$sock";
 		unlink $sock;
@@ -82,23 +87,23 @@ sub start_remote_tunnel {
 	    if ($unix_socket_try > 100) {
 		$self->{errors} = 1;
 		PVE::Tunnel::finish_tunnel($self->{tunnel});
-		die "Timeout, migration socket $ruri did not get ready";
+		die "Timeout, migration socket $tunnel_info->{addr} did not get ready";
 	    }
 	    $self->{tunnel}->{unix_sockets} = $unix_sockets if (@$unix_sockets);
 
-	} elsif ($ruri =~ /^tcp:/) {
+	} elsif ($tunnel_info->{proto} eq 'tcp') {
 	    my $ssh_forward_info = [];
-	    if ($raddr eq "localhost") {
+	    if ($tunnel_info->{addr} eq "localhost") {
 		# for backwards compatibility with older qemu-server versions
 		my $pfamily = PVE::Tools::get_host_address_family($nodename);
 		my $lport = PVE::Tools::next_migrate_port($pfamily);
-		push @$ssh_forward_info, "$lport:localhost:$rport";
+		push @$ssh_forward_info, "$lport:localhost:$tunnel_info->{rport}";
 	    }
 
 	    $self->{tunnel} = $self->fork_tunnel($ssh_forward_info);
 
 	} else {
-	    die "unsupported protocol in migration URI: $ruri\n";
+	    die "unsupported protocol in migration URI: $tunnel_info->{proto}\n";
 	}
     } else {
 	#fork tunnel for insecure migration, to send faster commands like resume
@@ -650,52 +655,40 @@ sub phase1_cleanup {
     }
 }
 
-sub phase2 {
-    my ($self, $vmid) = @_;
+sub phase2_start_local_cluster {
+    my ($self, $vmid, $params) = @_;
 
     my $conf = $self->{vmconf};
     my $local_volumes = $self->{local_volumes};
     my @online_local_volumes = $self->filter_local_volumes('online');
 
     $self->{storage_migration} = 1 if scalar(@online_local_volumes);
+    my $start = $params->{start_params};
+    my $migrate = $params->{migrate_opts};
 
     $self->log('info', "starting VM $vmid on remote node '$self->{node}'");
 
-    my $raddr;
-    my $rport;
-    my $ruri; # the whole migration dst. URI (protocol:address[:port])
-    my $nodename = PVE::INotify::nodename();
+    my $tunnel_info = {};
 
     ## start on remote node
     my $cmd = [@{$self->{rem_ssh}}];
 
-    my $spice_ticket;
-    if (PVE::QemuServer::vga_conf_has_spice($conf->{vga})) {
-	my $res = mon_cmd($vmid, 'query-spice');
-	$spice_ticket = $res->{ticket};
-    }
+    push @$cmd, 'qm', 'start', $vmid, '--skiplock';
+    push @$cmd, '--migratedfrom', $migrate->{migratedfrom};
 
-    push @$cmd , 'qm', 'start', $vmid, '--skiplock', '--migratedfrom', $nodename;
+    push @$cmd, '--migration_type', $migrate->{type};
 
-    my $migration_type = $self->{opts}->{migration_type};
+    push @$cmd, '--migration_network', $migrate->{network}
+      if $migrate->{network};
 
-    push @$cmd, '--migration_type', $migration_type;
+    push @$cmd, '--stateuri', $start->{statefile};
 
-    push @$cmd, '--migration_network', $self->{opts}->{migration_network}
-      if $self->{opts}->{migration_network};
-
-    if ($migration_type eq 'insecure') {
-	push @$cmd, '--stateuri', 'tcp';
-    } else {
-	push @$cmd, '--stateuri', 'unix';
+    if ($start->{forcemachine}) {
+	push @$cmd, '--machine', $start->{forcemachine};
     }
 
-    if ($self->{forcemachine}) {
-	push @$cmd, '--machine', $self->{forcemachine};
-    }
-
-    if ($self->{forcecpu}) {
-	push @$cmd, '--force-cpu', $self->{forcecpu};
+    if ($start->{forcecpu}) {
+	push @$cmd, '--force-cpu', $start->{forcecpu};
     }
 
     if ($self->{storage_migration}) {
@@ -703,10 +696,7 @@ sub phase2 {
     }
 
     my $spice_port;
-    my $unix_socket_info = {};
-    # version > 0 for unix socket support
-    my $nbd_protocol_version = 1;
-    my $input = "nbd_protocol_version: $nbd_protocol_version\n";
+    my $input = "nbd_protocol_version: $migrate->{nbd_proto_version}\n";
 
     if ($conf->{tpmstate0}) {
 	my $tpmdrive = PVE::QemuServer::parse_drive('tpmstate0', $conf->{tpmstate0});
@@ -715,7 +705,7 @@ sub phase2 {
 	    if $self->{volume_map}->{$tpmvol} && $tpmvol ne $self->{volume_map}->{$tpmvol};
     }
 
-    $input .= "spice_ticket: $spice_ticket\n" if $spice_ticket;
+    $input .= "spice_ticket: $migrate->{spice_ticket}\n" if $migrate->{spice_ticket};
 
     my @online_replicated_volumes = $self->filter_local_volumes('online', 1);
     foreach my $volid (@online_replicated_volumes) {
@@ -745,20 +735,20 @@ sub phase2 {
     my $exitcode = PVE::Tools::run_command($cmd, input => $input, outfunc => sub {
 	my $line = shift;
 
-	if ($line =~ m/^migration listens on tcp:(localhost|[\d\.]+|\[[\d\.:a-fA-F]+\]):(\d+)$/) {
-	    $raddr = $1;
-	    $rport = int($2);
-	    $ruri = "tcp:$raddr:$rport";
+	if ($line =~ m/^migration listens on (tcp):(localhost|[\d\.]+|\[[\d\.:a-fA-F]+\]):(\d+)$/) {
+	    $tunnel_info->{addr} = $2;
+	    $tunnel_info->{port} = int($3);
+	    $tunnel_info->{proto} = $1;
 	}
-	elsif ($line =~ m!^migration listens on unix:(/run/qemu-server/(\d+)\.migrate)$!) {
-	    $raddr = $1;
-	    die "Destination UNIX sockets VMID does not match source VMID" if $vmid ne $2;
-	    $ruri = "unix:$raddr";
+	elsif ($line =~ m!^migration listens on (unix):(/run/qemu-server/(\d+)\.migrate)$!) {
+	    $tunnel_info->{addr} = $2;
+	    die "Destination UNIX sockets VMID does not match source VMID" if $vmid ne $3;
+	    $tunnel_info->{proto} = $1;
 	}
 	elsif ($line =~ m/^migration listens on port (\d+)$/) {
-	    $raddr = "localhost";
-	    $rport = int($1);
-	    $ruri = "tcp:$raddr:$rport";
+	    $tunnel_info->{addr} = "localhost";
+	    $tunnel_info->{port} = int($1);
+	    $tunnel_info->{proto} = "tcp";
 	}
 	elsif ($line =~ m/^spice listens on port (\d+)$/) {
 	    $spice_port = int($1);
@@ -779,7 +769,7 @@ sub phase2 {
 	    $targetdrive =~ s/drive-//g;
 
 	    $handle_storage_migration_listens->($targetdrive, $drivestr, $nbd_uri);
-	    $unix_socket_info->{$nbd_unix_addr} = 1;
+	    $tunnel_info->{unix_sockets}->{$nbd_unix_addr} = 1;
 	} elsif ($line =~ m/^re-using replicated volume: (\S+) - (.*)$/) {
 	    my $drive = $1;
 	    my $volid = $2;
@@ -794,19 +784,65 @@ sub phase2 {
 
     die "remote command failed with exit code $exitcode\n" if $exitcode;
 
-    die "unable to detect remote migration address\n" if !$raddr;
+    die "unable to detect remote migration address\n" if !$tunnel_info->{addr} || !$tunnel_info->{proto};
 
     if (scalar(keys %$target_replicated_volumes) != scalar(@online_replicated_volumes)) {
 	die "number of replicated disks on source and target node do not match - target node too old?\n"
     }
 
+    return ($tunnel_info, $spice_port);
+}
+
+sub phase2 {
+    my ($self, $vmid) = @_;
+
+    my $conf = $self->{vmconf};
+
+    # version > 0 for unix socket support
+    my $nbd_protocol_version = 1;
+
+    my $spice_ticket;
+    if (PVE::QemuServer::vga_conf_has_spice($conf->{vga})) {
+	my $res = mon_cmd($vmid, 'query-spice');
+	$spice_ticket = $res->{ticket};
+    }
+
+    my $migration_type = $self->{opts}->{migration_type};
+    my $state_uri = $migration_type eq 'insecure' ? 'tcp' : 'unix';
+
+    my $params = {
+	start_params => {
+	    statefile => $state_uri,
+	    forcemachine => $self->{forcemachine},
+	    forcecpu => $self->{forcecpu},
+	    skiplock => 1,
+	},
+	migrate_opts => {
+	    spice_ticket => $spice_ticket,
+	    type => $migration_type,
+	    network => $self->{opts}->{migration_network},
+	    storagemap => $self->{opts}->{storagemap},
+	    migratedfrom => PVE::INotify::nodename(),
+	    nbd_proto_version => $nbd_protocol_version,
+	    nbd => $self->{nbd},
+	},
+    };
+
+    my ($tunnel_info, $spice_port) = $self->phase2_start_local_cluster($vmid, $params);
+
     $self->log('info', "start remote tunnel");
-    $self->start_remote_tunnel($raddr, $rport, $ruri, $unix_socket_info);
+    $self->start_remote_tunnel($tunnel_info);
+
+    my $migrate_uri = "$tunnel_info->{proto}:$tunnel_info->{addr}";
+    $migrate_uri .= ":$tunnel_info->{port}"
+	if defined($tunnel_info->{port});
 
     if ($self->{storage_migration}) {
 	$self->{storage_migration_jobs} = {};
 	$self->log('info', "starting storage migration");
 
+	my @online_local_volumes = $self->filter_local_volumes('online');
+
 	die "The number of local disks does not match between the source and the destination.\n"
 	    if (scalar(keys %{$self->{target_drive}}) != scalar(@online_local_volumes));
 	foreach my $drive (keys %{$self->{target_drive}}){
@@ -816,7 +852,7 @@ sub phase2 {
 	    my $source_drive = PVE::QemuServer::parse_drive($drive, $conf->{$drive});
 	    my $source_volid = $source_drive->{file};
 
-	    my $bwlimit = $local_volumes->{$source_volid}->{bwlimit};
+	    my $bwlimit = $self->{local_volumes}->{$source_volid}->{bwlimit};
 	    my $bitmap = $target->{bitmap};
 
 	    $self->log('info', "$drive: start migration to $nbd_uri");
@@ -824,7 +860,7 @@ sub phase2 {
 	}
     }
 
-    $self->log('info', "starting online/live migration on $ruri");
+    $self->log('info', "starting online/live migration on $migrate_uri");
     $self->{livemigration} = 1;
 
     # load_defaults
@@ -901,12 +937,12 @@ sub phase2 {
 
     my $start = time();
 
-    $self->log('info', "start migrate command to $ruri");
+    $self->log('info', "start migrate command to $migrate_uri");
     eval {
-	mon_cmd($vmid, "migrate", uri => $ruri);
+	mon_cmd($vmid, "migrate", uri => $migrate_uri);
     };
     my $merr = $@;
-    $self->log('info', "migrate uri => $ruri failed: $merr") if $merr;
+    $self->log('info', "migrate uri => $migrate_uri failed: $merr") if $merr;
 
     my $last_mem_transferred = 0;
     my $usleep = 1000000;
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index a4d6487f..8af6e7ae 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5455,10 +5455,10 @@ sub vm_start_nolock {
 	return $migration_ip;
     };
 
-    my $migrate_uri;
     if ($statefile) {
 	if ($statefile eq 'tcp') {
-	    my $localip = "localhost";
+	    my $migrate = $res->{migrate} = { proto => 'tcp' };
+	    $migrate->{addr} = "localhost";
 	    my $datacenterconf = PVE::Cluster::cfs_read_file('datacenter.cfg');
 	    my $nodename = nodename();
 
@@ -5471,26 +5471,26 @@ sub vm_start_nolock {
 	    }
 
 	    if ($migration_type eq 'insecure') {
-		$localip = $get_migration_ip->($nodename);
-		$localip = "[$localip]" if Net::IP::ip_is_ipv6($localip);
+		$migrate->{addr} = $get_migration_ip->($nodename);
+		$migrate->{addr} = "[$migrate->{addr}]" if Net::IP::ip_is_ipv6($migrate->{addr});
 	    }
 
 	    my $pfamily = PVE::Tools::get_host_address_family($nodename);
-	    my $migrate_port = PVE::Tools::next_migrate_port($pfamily);
-	    $migrate_uri = "tcp:${localip}:${migrate_port}";
-	    push @$cmd, '-incoming', $migrate_uri;
+	    $migrate->{port} = PVE::Tools::next_migrate_port($pfamily);
+	    $migrate->{uri} = "tcp:$migrate->{addr}:$migrate->{port}";
+	    push @$cmd, '-incoming', $migrate->{uri};
 	    push @$cmd, '-S';
 
 	} elsif ($statefile eq 'unix') {
 	    # should be default for secure migrations as a ssh TCP forward
 	    # tunnel is not deterministic reliable ready and fails regurarly
 	    # to set up in time, so use UNIX socket forwards
-	    my $socket_addr = "/run/qemu-server/$vmid.migrate";
-	    unlink $socket_addr;
+	    my $migrate = $res->{migrate} = { proto => 'unix' };
+	    $migrate->{addr} = "/run/qemu-server/$vmid.migrate";
+	    unlink $migrate->{addr};
 
-	    $migrate_uri = "unix:$socket_addr";
-
-	    push @$cmd, '-incoming', $migrate_uri;
+	    $migrate->{uri} = "unix:$migrate->{addr}";
+	    push @$cmd, '-incoming', $migrate->{uri};
 	    push @$cmd, '-S';
 
 	} elsif (-e $statefile) {
@@ -5637,10 +5637,9 @@ sub vm_start_nolock {
     eval { PVE::QemuServer::PCI::reserve_pci_usage($pci_id_list, $vmid, undef, $pid) };
     warn $@ if $@;
 
-    print "migration listens on $migrate_uri\n" if $migrate_uri;
-    $res->{migrate_uri} = $migrate_uri;
-
-    if ($statefile && $statefile ne 'tcp' && $statefile ne 'unix')  {
+    if (defined($res->{migrate})) {
+	print "migration listens on $res->{migrate}->{uri}\n";
+    } elsif ($statefile) {
 	eval { mon_cmd($vmid, "cont"); };
 	warn $@ if $@;
     }
@@ -5655,6 +5654,7 @@ sub vm_start_nolock {
 	    my $socket_path = "/run/qemu-server/$vmid\_nbd.migrate";
 	    mon_cmd($vmid, "nbd-server-start", addr => { type => 'unix', data => { path => $socket_path } } );
 	    $migrate_storage_uri = "nbd:unix:$socket_path";
+	    $res->{migrate}->{unix_sockets} = [$socket_path];
 	} else {
 	    my $nodename = nodename();
 	    my $localip = $get_migration_ip->($nodename);
@@ -5672,8 +5672,6 @@ sub vm_start_nolock {
 	    $migrate_storage_uri = "nbd:${localip}:${storage_migrate_port}";
 	}
 
-	$res->{migrate_storage_uri} = $migrate_storage_uri;
-
 	foreach my $opt (sort keys %$nbd) {
 	    my $drivestr = $nbd->{$opt}->{drivestr};
 	    my $volid = $nbd->{$opt}->{volid};
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 qemu-server 09/11] migrate: add remote migration handling
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (14 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 08/11] migrate: refactor remote VM/tunnel start Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 10/11] api: add remote migrate endpoint Fabian Grünbichler
                   ` (7 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

remote migration uses a websocket connection to a task worker running on
the target node instead of commands via SSH to control the migration.
this websocket tunnel is started earlier than the SSH tunnel, and allows
adding UNIX-socket forwarding over additional websocket connections
on-demand.

the main differences to regular intra-cluster migration are:
- source VM config and disks are only removed upon request via --delete
- shared storages are treated like local storages, since we can't
assume they are shared across clusters (with potentical to extend this
by marking storages as shared)
- NBD migrated disks are explicitly pre-allocated on the target node via
tunnel command before starting the target VM instance
- in addition to storages, network bridges and the VMID itself is
transformed via a user defined mapping
- all commands and migration data streams are sent via a WS tunnel proxy

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    v5:
    - move merge_bwlimits helper to PVE::AbstractMigrate and extend it
    - adapt to map_id move
    - add check on source side for VM snapshots (not yet supported/implemented)
    
    v4:
    - new merge_bwlimits helper, improved bwlimit handling
    - use config-aware remote start timeout
    - switch tunnel log to match migration log sub
    
    v3:
    - move WS tunnel helpers to pve-guest-common-perl
    - check bridge mapping early
    - fix misplaced parentheses
    
    v2:
    - improve tunnel version info printing and error handling
    - don't cleanup unix sockets twice
    - url escape remote socket path
    - cleanup nits and small issues
    
    requires proxmox-websocket-tunnel, bumped pve-guest-common

 PVE/API2/Qemu.pm   |   2 +-
 PVE/QemuMigrate.pm | 433 +++++++++++++++++++++++++++++++++++++--------
 PVE/QemuServer.pm  |   7 +-
 3 files changed, 363 insertions(+), 79 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 78313a28..ca691e20 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4814,7 +4814,7 @@ __PACKAGE__->register_method({
 		    # bump/reset for breaking changes
 		    # bump/bump for opt-in changes
 		    return {
-			api => 2,
+			api => $PVE::QemuMigrate::WS_TUNNEL_VERSION,
 			age => 0,
 		    };
 		},
diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
index e6cb7e79..e3f417f3 100644
--- a/PVE/QemuMigrate.pm
+++ b/PVE/QemuMigrate.pm
@@ -5,11 +5,10 @@ use warnings;
 
 use IO::File;
 use IPC::Open2;
-use POSIX qw( WNOHANG );
 use Time::HiRes qw( usleep );
 
-use PVE::Format qw(render_bytes);
 use PVE::Cluster;
+use PVE::Format qw(render_bytes);
 use PVE::GuestHelpers qw(safe_boolean_ne safe_string_ne);
 use PVE::INotify;
 use PVE::RPCEnvironment;
@@ -17,6 +16,7 @@ use PVE::Replication;
 use PVE::ReplicationConfig;
 use PVE::ReplicationState;
 use PVE::Storage;
+use PVE::StorageTunnel;
 use PVE::Tools;
 use PVE::Tunnel;
 
@@ -31,6 +31,9 @@ use PVE::QemuServer;
 use PVE::AbstractMigrate;
 use base qw(PVE::AbstractMigrate);
 
+# compared against remote end's minimum version
+our $WS_TUNNEL_VERSION = 2;
+
 sub fork_tunnel {
     my ($self, $ssh_forward_info) = @_;
 
@@ -43,6 +46,35 @@ sub fork_tunnel {
     return PVE::Tunnel::fork_ssh_tunnel($self->{rem_ssh}, $cmd, $ssh_forward_info, $log);
 }
 
+sub fork_websocket_tunnel {
+    my ($self, $storages, $bridges) = @_;
+
+    my $remote = $self->{opts}->{remote};
+    my $conn = $remote->{conn};
+
+    my $log = sub {
+	my ($level, $msg) = @_;
+	$self->log($level, $msg);
+    };
+
+    my $websocket_url = "https://$conn->{host}:$conn->{port}/api2/json/nodes/$self->{node}/qemu/$remote->{vmid}/mtunnelwebsocket";
+    my $url = "/nodes/$self->{node}/qemu/$remote->{vmid}/mtunnel";
+
+    my $tunnel_params = {
+	url => $websocket_url,
+    };
+
+    my $storage_list = join(',', keys %$storages);
+    my $bridge_list = join(',', keys %$bridges);
+
+    my $req_params = {
+	storages => $storage_list,
+	bridges => $bridge_list,
+    };
+
+    return PVE::Tunnel::fork_websocket_tunnel($conn, $url, $req_params, $tunnel_params, $log);
+}
+
 # tunnel_info:
 #   proto: unix (secure) or tcp (insecure/legacy compat)
 #   addr: IP or UNIX socket path
@@ -175,23 +207,34 @@ sub prepare {
     }
 
     my $vollist = PVE::QemuServer::get_vm_volumes($conf);
+
+    my $storages = {};
     foreach my $volid (@$vollist) {
 	my ($sid, $volname) = PVE::Storage::parse_volume_id($volid, 1);
 
-	# check if storage is available on both nodes
+	# check if storage is available on source node
 	my $scfg = PVE::Storage::storage_check_enabled($storecfg, $sid);
 
 	my $targetsid = $sid;
-	# NOTE: we currently ignore shared source storages in mappings so skip here too for now
-	if (!$scfg->{shared}) {
+	# NOTE: local ignores shared mappings, remote maps them
+	if (!$scfg->{shared} || $self->{opts}->{remote}) {
 	    $targetsid = PVE::JSONSchema::map_id($self->{opts}->{storagemap}, $sid);
 	}
 
-	my $target_scfg = PVE::Storage::storage_check_enabled($storecfg, $targetsid, $self->{node});
-	my ($vtype) = PVE::Storage::parse_volname($storecfg, $volid);
+	$storages->{$targetsid} = 1;
 
-	die "$volid: content type '$vtype' is not available on storage '$targetsid'\n"
-	    if !$target_scfg->{content}->{$vtype};
+	if (!$self->{opts}->{remote}) {
+	    # check if storage is available on target node
+	    my $target_scfg = PVE::Storage::storage_check_enabled(
+		$storecfg,
+		$targetsid,
+		$self->{node},
+	    );
+	    my ($vtype) = PVE::Storage::parse_volname($storecfg, $volid);
+
+	    die "$volid: content type '$vtype' is not available on storage '$targetsid'\n"
+		if !$target_scfg->{content}->{$vtype};
+	}
 
 	if ($scfg->{shared}) {
 	    # PVE::Storage::activate_storage checks this for non-shared storages
@@ -201,10 +244,27 @@ sub prepare {
 	}
     }
 
-    # test ssh connection
-    my $cmd = [ @{$self->{rem_ssh}}, '/bin/true' ];
-    eval { $self->cmd_quiet($cmd); };
-    die "Can't connect to destination address using public key\n" if $@;
+    if ($self->{opts}->{remote}) {
+	# test & establish websocket connection
+	my $bridges = map_bridges($conf, $self->{opts}->{bridgemap}, 1);
+	my $tunnel = $self->fork_websocket_tunnel($storages, $bridges);
+	my $min_version = $tunnel->{version} - $tunnel->{age};
+	$self->log('info', "local WS tunnel version: $WS_TUNNEL_VERSION");
+	$self->log('info', "remote WS tunnel version: $tunnel->{version}");
+	$self->log('info', "minimum required WS tunnel version: $min_version");
+	die "Remote tunnel endpoint not compatible, upgrade required\n"
+	    if $WS_TUNNEL_VERSION < $min_version;
+	 die "Remote tunnel endpoint too old, upgrade required\n"
+	    if $WS_TUNNEL_VERSION > $tunnel->{version};
+
+	print "websocket tunnel started\n";
+	$self->{tunnel} = $tunnel;
+    } else {
+	# test ssh connection
+	my $cmd = [ @{$self->{rem_ssh}}, '/bin/true' ];
+	eval { $self->cmd_quiet($cmd); };
+	die "Can't connect to destination address using public key\n" if $@;
+    }
 
     return $running;
 }
@@ -242,7 +302,7 @@ sub scan_local_volumes {
 	my @sids = PVE::Storage::storage_ids($storecfg);
 	foreach my $storeid (@sids) {
 	    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
-	    next if $scfg->{shared};
+	    next if $scfg->{shared} && !$self->{opts}->{remote};
 	    next if !PVE::Storage::storage_check_enabled($storecfg, $storeid, undef, 1);
 
 	    # get list from PVE::Storage (for unused volumes)
@@ -251,21 +311,20 @@ sub scan_local_volumes {
 	    next if @{$dl->{$storeid}} == 0;
 
 	    my $targetsid = PVE::JSONSchema::map_id($self->{opts}->{storagemap}, $storeid);
-	    # check if storage is available on target node
-	    my $target_scfg = PVE::Storage::storage_check_enabled(
-		$storecfg,
-		$targetsid,
-		$self->{node},
-	    );
+	    if (!$self->{opts}->{remote}) {
+		# check if storage is available on target node
+		my $target_scfg = PVE::Storage::storage_check_enabled(
+		    $storecfg,
+		    $targetsid,
+		    $self->{node},
+		);
 
-	    die "content type 'images' is not available on storage '$targetsid'\n"
-		if !$target_scfg->{content}->{images};
+		die "content type 'images' is not available on storage '$targetsid'\n"
+		    if !$target_scfg->{content}->{images};
 
-	    my $bwlimit = PVE::Storage::get_bandwidth_limit(
-		'migration',
-		[$targetsid, $storeid],
-		$self->{opts}->{bwlimit},
-	    );
+	    }
+
+	    my $bwlimit = $self->get_bwlimit($storeid, $targetsid);
 
 	    PVE::Storage::foreach_volid($dl, sub {
 		my ($volid, $sid, $volinfo) = @_;
@@ -319,14 +378,17 @@ sub scan_local_volumes {
 	    my $scfg = PVE::Storage::storage_check_enabled($storecfg, $sid);
 
 	    my $targetsid = $sid;
-	    # NOTE: we currently ignore shared source storages in mappings so skip here too for now
-	    if (!$scfg->{shared}) {
+	    # NOTE: local ignores shared mappings, remote maps them
+	    if (!$scfg->{shared} || $self->{opts}->{remote}) {
 		$targetsid = PVE::JSONSchema::map_id($self->{opts}->{storagemap}, $sid);
 	    }
 
-	    PVE::Storage::storage_check_enabled($storecfg, $targetsid, $self->{node});
+	    # check target storage on target node if intra-cluster migration
+	    if (!$self->{opts}->{remote}) {
+		PVE::Storage::storage_check_enabled($storecfg, $targetsid, $self->{node});
 
-	    return if $scfg->{shared};
+		return if $scfg->{shared};
+	    }
 
 	    $local_volumes->{$volid}->{ref} = $attr->{referenced_in_config} ? 'config' : 'snapshot';
 	    $local_volumes->{$volid}->{ref} = 'storage' if $attr->{is_unused};
@@ -359,6 +421,8 @@ sub scan_local_volumes {
 		# exceptions: 'zfspool' or 'qcow2' files (on directory storage)
 
 		die "online storage migration not possible if snapshot exists\n" if $self->{running};
+		die "remote migration with snapshots not supported yet\n" if $self->{opts}->{remote};
+
 		if (!($scfg->{type} eq 'zfspool'
 		    || ($scfg->{type} eq 'btrfs' && $local_volumes->{$volid}->{format} eq 'raw')
 		    || $local_volumes->{$volid}->{format} eq 'qcow2'
@@ -415,6 +479,9 @@ sub scan_local_volumes {
 
 	    my $migratable = $scfg->{type} =~ /^(?:dir|btrfs|zfspool|lvmthin|lvm)$/;
 
+	    # TODO: what is this even here for?
+	    $migratable = 1 if $self->{opts}->{remote};
+
 	    die "can't migrate '$volid' - storage type '$scfg->{type}' not supported\n"
 		if !$migratable;
 
@@ -449,6 +516,10 @@ sub handle_replication {
     my $local_volumes = $self->{local_volumes};
 
     return if !$self->{replication_jobcfg};
+
+    die "can't migrate VM with replicated volumes to remote cluster/node\n"
+	if $self->{opts}->{remote};
+
     if ($self->{running}) {
 
 	my $version = PVE::QemuServer::kvm_user_version();
@@ -548,24 +619,51 @@ sub sync_offline_local_volumes {
     $self->log('info', "copying local disk images") if scalar(@volids);
 
     foreach my $volid (@volids) {
-	my $targetsid = $local_volumes->{$volid}->{targetsid};
-	my $bwlimit = $local_volumes->{$volid}->{bwlimit};
-	$bwlimit = $bwlimit * 1024 if defined($bwlimit); # storage_migrate uses bps
-
-	my $storage_migrate_opts = {
-	    'ratelimit_bps' => $bwlimit,
-	    'insecure' => $opts->{migration_type} eq 'insecure',
-	    'with_snapshots' => $local_volumes->{$volid}->{snapshots},
-	    'allow_rename' => !$local_volumes->{$volid}->{is_vmstate},
-	};
+	my $new_volid;
 
-	my $logfunc = sub { $self->log('info', $_[0]); };
-	my $new_volid = eval {
-	    PVE::Storage::storage_migrate($storecfg, $volid, $self->{ssh_info},
-					  $targetsid, $storage_migrate_opts, $logfunc);
-	};
-	if (my $err = $@) {
-	    die "storage migration for '$volid' to storage '$targetsid' failed - $err\n";
+	my $opts = $self->{opts};
+	if ($opts->{remote}) {
+	    my $log = sub {
+		my ($level, $msg) = @_;
+		$self->log($level, $msg);
+	    };
+
+	    $new_volid = PVE::StorageTunnel::storage_migrate(
+		$self->{tunnel},
+		$storecfg,
+		$volid,
+		$self->{vmid},
+		$opts->{remote}->{vmid},
+		$local_volumes->{$volid},
+		$log,
+	    );
+	} else {
+	    my $targetsid = $local_volumes->{$volid}->{targetsid};
+
+	    my $bwlimit = $local_volumes->{$volid}->{bwlimit};
+	    $bwlimit = $bwlimit * 1024 if defined($bwlimit); # storage_migrate uses bps
+
+	    my $storage_migrate_opts = {
+		'ratelimit_bps' => $bwlimit,
+		'insecure' => $opts->{migration_type} eq 'insecure',
+		'with_snapshots' => $local_volumes->{$volid}->{snapshots},
+		'allow_rename' => !$local_volumes->{$volid}->{is_vmstate},
+	    };
+
+	    my $logfunc = sub { $self->log('info', $_[0]); };
+	    $new_volid = eval {
+		PVE::Storage::storage_migrate(
+		    $storecfg,
+		    $volid,
+		    $self->{ssh_info},
+		    $targetsid,
+		    $storage_migrate_opts,
+		    $logfunc,
+		);
+	    };
+	    if (my $err = $@) {
+		die "storage migration for '$volid' to storage '$targetsid' failed - $err\n";
+	    }
 	}
 
 	$self->{volume_map}->{$volid} = $new_volid;
@@ -581,6 +679,12 @@ sub sync_offline_local_volumes {
 sub cleanup_remotedisks {
     my ($self) = @_;
 
+    if ($self->{opts}->{remote}) {
+	PVE::Tunnel::finish_tunnel($self->{tunnel}, 1);
+	delete $self->{tunnel};
+	return;
+    }
+
     my $local_volumes = $self->{local_volumes};
 
     foreach my $volid (values %{$self->{volume_map}}) {
@@ -630,8 +734,100 @@ sub phase1 {
     $self->handle_replication($vmid);
 
     $self->sync_offline_local_volumes();
+    $self->phase1_remote($vmid) if $self->{opts}->{remote};
 };
 
+sub map_bridges {
+    my ($conf, $map, $scan_only) = @_;
+
+    my $bridges = {};
+
+    foreach my $opt (keys %$conf) {
+	next if $opt !~ m/^net\d+$/;
+
+	next if !$conf->{$opt};
+	my $d = PVE::QemuServer::parse_net($conf->{$opt});
+	next if !$d || !$d->{bridge};
+
+	my $target_bridge = PVE::JSONSchema::map_id($map, $d->{bridge});
+	$bridges->{$target_bridge}->{$opt} = $d->{bridge};
+
+	next if $scan_only;
+
+	$d->{bridge} = $target_bridge;
+	$conf->{$opt} = PVE::QemuServer::print_net($d);
+    }
+
+    return $bridges;
+}
+
+sub phase1_remote {
+    my ($self, $vmid) = @_;
+
+    my $remote_conf = PVE::QemuConfig->load_config($vmid);
+    PVE::QemuConfig->update_volume_ids($remote_conf, $self->{volume_map});
+
+    my $bridges = map_bridges($remote_conf, $self->{opts}->{bridgemap});
+    for my $target (keys $bridges->%*) {
+	for my $nic (keys $bridges->{$target}->%*) {
+	    $self->log('info', "mapped: $nic from $bridges->{$target}->{$nic} to $target");
+	}
+    }
+
+    my @online_local_volumes = $self->filter_local_volumes('online');
+
+    my $storage_map = $self->{opts}->{storagemap};
+    $self->{nbd} = {};
+    PVE::QemuConfig->foreach_volume($remote_conf, sub {
+	my ($ds, $drive) = @_;
+
+	# TODO eject CDROM?
+	return if PVE::QemuServer::drive_is_cdrom($drive);
+
+	my $volid = $drive->{file};
+	return if !$volid;
+
+	return if !grep { $_ eq $volid} @online_local_volumes;
+
+	my ($storeid, $volname) = PVE::Storage::parse_volume_id($volid);
+	my $scfg = PVE::Storage::storage_config($self->{storecfg}, $storeid);
+	my $source_format = PVE::QemuServer::qemu_img_format($scfg, $volname);
+
+	# set by target cluster
+	my $oldvolid = delete $drive->{file};
+	delete $drive->{format};
+
+	my $targetsid = PVE::JSONSchema::map_id($storage_map, $storeid);
+
+	my $params = {
+	    format => $source_format,
+	    storage => $targetsid,
+	    drive => $drive,
+	};
+
+	$self->log('info', "Allocating volume for drive '$ds' on remote storage '$targetsid'..");
+	my $res = PVE::Tunnel::write_tunnel($self->{tunnel}, 600, 'disk', $params);
+
+	$self->log('info', "volume '$oldvolid' is '$res->{volid}' on the target\n");
+	$remote_conf->{$ds} = $res->{drivestr};
+	$self->{nbd}->{$ds} = $res;
+    });
+
+    my $conf_str = PVE::QemuServer::write_vm_config("remote", $remote_conf);
+
+    # TODO expose in PVE::Firewall?
+    my $vm_fw_conf_path = "/etc/pve/firewall/$vmid.fw";
+    my $fw_conf_str;
+    $fw_conf_str = PVE::Tools::file_get_contents($vm_fw_conf_path)
+	if -e $vm_fw_conf_path;
+    my $params = {
+	conf => $conf_str,
+	'firewall-config' => $fw_conf_str,
+    };
+
+    PVE::Tunnel::write_tunnel($self->{tunnel}, 10, 'config', $params);
+}
+
 sub phase1_cleanup {
     my ($self, $vmid, $err) = @_;
 
@@ -662,7 +858,6 @@ sub phase2_start_local_cluster {
     my $local_volumes = $self->{local_volumes};
     my @online_local_volumes = $self->filter_local_volumes('online');
 
-    $self->{storage_migration} = 1 if scalar(@online_local_volumes);
     my $start = $params->{start_params};
     my $migrate = $params->{migrate_opts};
 
@@ -793,10 +988,37 @@ sub phase2_start_local_cluster {
     return ($tunnel_info, $spice_port);
 }
 
+sub phase2_start_remote_cluster {
+    my ($self, $vmid, $params) = @_;
+
+    die "insecure migration to remote cluster not implemented\n"
+	if $params->{migrate_opts}->{type} ne 'websocket';
+
+    my $remote_vmid = $self->{opts}->{remote}->{vmid};
+
+    # like regular start but with some overhead accounted for
+    my $timeout = PVE::QemuServer::Helpers::config_aware_timeout($self->{vmconf}) + 10;
+
+    my $res = PVE::Tunnel::write_tunnel($self->{tunnel}, $timeout, "start", $params);
+
+    foreach my $drive (keys %{$res->{drives}}) {
+	$self->{stopnbd} = 1;
+	$self->{target_drive}->{$drive}->{drivestr} = $res->{drives}->{$drive}->{drivestr};
+	my $nbd_uri = $res->{drives}->{$drive}->{nbd_uri};
+	die "unexpected NBD uri for '$drive': $nbd_uri\n"
+	    if $nbd_uri !~ s!/run/qemu-server/$remote_vmid\_!/run/qemu-server/$vmid\_!;
+
+	$self->{target_drive}->{$drive}->{nbd_uri} = $nbd_uri;
+    }
+
+    return ($res->{migrate}, $res->{spice_port});
+}
+
 sub phase2 {
     my ($self, $vmid) = @_;
 
     my $conf = $self->{vmconf};
+    my $local_volumes = $self->{local_volumes};
 
     # version > 0 for unix socket support
     my $nbd_protocol_version = 1;
@@ -828,10 +1050,39 @@ sub phase2 {
 	},
     };
 
-    my ($tunnel_info, $spice_port) = $self->phase2_start_local_cluster($vmid, $params);
+    my ($tunnel_info, $spice_port);
+
+    my @online_local_volumes = $self->filter_local_volumes('online');
+    $self->{storage_migration} = 1 if scalar(@online_local_volumes);
+
+    if (my $remote = $self->{opts}->{remote}) {
+	my $remote_vmid = $remote->{vmid};
+	$params->{migrate_opts}->{remote_node} = $self->{node};
+	($tunnel_info, $spice_port) = $self->phase2_start_remote_cluster($vmid, $params);
+	die "only UNIX sockets are supported for remote migration\n"
+	    if $tunnel_info->{proto} ne 'unix';
+
+	my $remote_socket = $tunnel_info->{addr};
+	my $local_socket = $remote_socket;
+	$local_socket =~ s/$remote_vmid/$vmid/g;
+	$tunnel_info->{addr} = $local_socket;
+
+	$self->log('info', "Setting up tunnel for '$local_socket'");
+	PVE::Tunnel::forward_unix_socket($self->{tunnel}, $local_socket, $remote_socket);
+
+	foreach my $remote_socket (@{$tunnel_info->{unix_sockets}}) {
+	    my $local_socket = $remote_socket;
+	    $local_socket =~ s/$remote_vmid/$vmid/g;
+	    next if $self->{tunnel}->{forwarded}->{$local_socket};
+	    $self->log('info', "Setting up tunnel for '$local_socket'");
+	    PVE::Tunnel::forward_unix_socket($self->{tunnel}, $local_socket, $remote_socket);
+	}
+    } else {
+	($tunnel_info, $spice_port) = $self->phase2_start_local_cluster($vmid, $params);
 
-    $self->log('info', "start remote tunnel");
-    $self->start_remote_tunnel($tunnel_info);
+	$self->log('info', "start remote tunnel");
+	$self->start_remote_tunnel($tunnel_info);
+    }
 
     my $migrate_uri = "$tunnel_info->{proto}:$tunnel_info->{addr}";
     $migrate_uri .= ":$tunnel_info->{port}"
@@ -841,8 +1092,6 @@ sub phase2 {
 	$self->{storage_migration_jobs} = {};
 	$self->log('info', "starting storage migration");
 
-	my @online_local_volumes = $self->filter_local_volumes('online');
-
 	die "The number of local disks does not match between the source and the destination.\n"
 	    if (scalar(keys %{$self->{target_drive}}) != scalar(@online_local_volumes));
 	foreach my $drive (keys %{$self->{target_drive}}){
@@ -874,7 +1123,8 @@ sub phase2 {
 
     # migrate speed can be set via bwlimit (datacenter.cfg and API) and via the
     # migrate_speed parameter in qm.conf - take the lower of the two.
-    my $bwlimit = PVE::Storage::get_bandwidth_limit('migration', undef, $self->{opts}->{bwlimit}) // 0;
+    my $bwlimit = $self->get_bwlimit();
+
     my $migrate_speed = $conf->{migrate_speed} // 0;
     $migrate_speed *= 1024; # migrate_speed is in MB/s, bwlimit in KB/s
 
@@ -915,7 +1165,7 @@ sub phase2 {
     };
     $self->log('info', "migrate-set-parameters error: $@") if $@;
 
-    if (PVE::QemuServer::vga_conf_has_spice($conf->{vga})) {
+    if (PVE::QemuServer::vga_conf_has_spice($conf->{vga}) && !$self->{opts}->{remote}) {
 	my $rpcenv = PVE::RPCEnvironment::get();
 	my $authuser = $rpcenv->get_user();
 
@@ -1112,11 +1362,15 @@ sub phase2_cleanup {
 
     my $nodename = PVE::INotify::nodename();
 
-    my $cmd = [@{$self->{rem_ssh}}, 'qm', 'stop', $vmid, '--skiplock', '--migratedfrom', $nodename];
-    eval{ PVE::Tools::run_command($cmd, outfunc => sub {}, errfunc => sub {}) };
-    if (my $err = $@) {
-        $self->log('err', $err);
-        $self->{errors} = 1;
+    if ($self->{tunnel} && $self->{tunnel}->{version} >= 2) {
+	PVE::Tunnel::write_tunnel($self->{tunnel}, 10, 'stop');
+    } else {
+	my $cmd = [@{$self->{rem_ssh}}, 'qm', 'stop', $vmid, '--skiplock', '--migratedfrom', $nodename];
+	eval{ PVE::Tools::run_command($cmd, outfunc => sub {}, errfunc => sub {}) };
+	if (my $err = $@) {
+	    $self->log('err', $err);
+	    $self->{errors} = 1;
+	}
     }
 
     # cleanup after stopping, otherwise disks might be in-use by target VM!
@@ -1149,7 +1403,7 @@ sub phase3_cleanup {
 
     my $tunnel = $self->{tunnel};
 
-    if ($self->{volume_map}) {
+    if ($self->{volume_map} && !$self->{opts}->{remote}) {
 	my $target_drives = $self->{target_drive};
 
 	# FIXME: for NBD storage migration we now only update the volid, and
@@ -1165,27 +1419,34 @@ sub phase3_cleanup {
     }
 
     # transfer replication state before move config
-    $self->transfer_replication_state() if $self->{is_replicated};
-    PVE::QemuConfig->move_config_to_node($vmid, $self->{node});
-    $self->switch_replication_job_target() if $self->{is_replicated};
+    if (!$self->{opts}->{remote}) {
+	$self->transfer_replication_state() if $self->{is_replicated};
+	PVE::QemuConfig->move_config_to_node($vmid, $self->{node});
+	$self->switch_replication_job_target() if $self->{is_replicated};
+    }
 
     if ($self->{livemigration}) {
 	if ($self->{stopnbd}) {
 	    $self->log('info', "stopping NBD storage migration server on target.");
 	    # stop nbd server on remote vm - requirement for resume since 2.9
-	    my $cmd = [@{$self->{rem_ssh}}, 'qm', 'nbdstop', $vmid];
+	    if ($tunnel && $tunnel->{version} && $tunnel->{version} >= 2) {
+		PVE::Tunnel::write_tunnel($tunnel, 30, 'nbdstop');
+	    } else {
+		my $cmd = [@{$self->{rem_ssh}}, 'qm', 'nbdstop', $vmid];
 
-	    eval{ PVE::Tools::run_command($cmd, outfunc => sub {}, errfunc => sub {}) };
-	    if (my $err = $@) {
-		$self->log('err', $err);
-		$self->{errors} = 1;
+		eval{ PVE::Tools::run_command($cmd, outfunc => sub {}, errfunc => sub {}) };
+		if (my $err = $@) {
+		    $self->log('err', $err);
+		    $self->{errors} = 1;
+		}
 	    }
 	}
 
 	# config moved and nbd server stopped - now we can resume vm on target
 	if ($tunnel && $tunnel->{version} && $tunnel->{version} >= 1) {
+	    my $cmd = $tunnel->{version} == 1 ? "resume $vmid" : "resume";
 	    eval {
-		PVE::Tunnel::write_tunnel($tunnel, 30, "resume $vmid");
+		PVE::Tunnel::write_tunnel($tunnel, 30, $cmd);
 	    };
 	    if (my $err = $@) {
 		$self->log('err', $err);
@@ -1205,18 +1466,24 @@ sub phase3_cleanup {
 	}
 
 	if ($self->{storage_migration} && PVE::QemuServer::parse_guest_agent($conf)->{fstrim_cloned_disks} && $self->{running}) {
-	    my $cmd = [@{$self->{rem_ssh}}, 'qm', 'guest', 'cmd', $vmid, 'fstrim'];
-	    eval{ PVE::Tools::run_command($cmd, outfunc => sub {}, errfunc => sub {}) };
+	    if ($self->{opts}->{remote}) {
+		PVE::Tunnel::write_tunnel($self->{tunnel}, 600, 'fstrim');
+	    } else {
+		my $cmd = [@{$self->{rem_ssh}}, 'qm', 'guest', 'cmd', $vmid, 'fstrim'];
+		eval{ PVE::Tools::run_command($cmd, outfunc => sub {}, errfunc => sub {}) };
+	    }
 	}
     }
 
     # close tunnel on successful migration, on error phase2_cleanup closed it
-    if ($tunnel) {
+    if ($tunnel && $tunnel->{version} == 1) {
 	eval { PVE::Tunnel::finish_tunnel($tunnel); };
 	if (my $err = $@) {
 	    $self->log('err', $err);
 	    $self->{errors} = 1;
 	}
+	$tunnel = undef;
+	delete $self->{tunnel};
     }
 
     eval {
@@ -1254,6 +1521,9 @@ sub phase3_cleanup {
 
     # destroy local copies
     foreach my $volid (@not_replicated_volumes) {
+	# remote is cleaned up below
+	next if $self->{opts}->{remote};
+
 	eval { PVE::Storage::vdisk_free($self->{storecfg}, $volid); };
 	if (my $err = $@) {
 	    $self->log('err', "removing local copy of '$volid' failed - $err");
@@ -1263,8 +1533,19 @@ sub phase3_cleanup {
     }
 
     # clear migrate lock
-    my $cmd = [ @{$self->{rem_ssh}}, 'qm', 'unlock', $vmid ];
-    $self->cmd_logerr($cmd, errmsg => "failed to clear migrate lock");
+    if ($tunnel && $tunnel->{version} >= 2) {
+	PVE::Tunnel::write_tunnel($tunnel, 10, "unlock");
+
+	PVE::Tunnel::finish_tunnel($tunnel);
+    } else {
+	my $cmd = [ @{$self->{rem_ssh}}, 'qm', 'unlock', $vmid ];
+	$self->cmd_logerr($cmd, errmsg => "failed to clear migrate lock");
+    }
+
+    if ($self->{opts}->{remote} && $self->{opts}->{delete}) {
+	eval { PVE::QemuServer::destroy_vm($self->{storecfg}, $vmid, 1, undef, 0) };
+	warn "Failed to remove source VM - $@\n" if $@;
+    }
 }
 
 sub final_cleanup {
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 8af6e7ae..1b603915 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5408,7 +5408,10 @@ sub vm_start_nolock {
     my $defaults = load_defaults();
 
     # set environment variable useful inside network script
-    $ENV{PVE_MIGRATED_FROM} = $migratedfrom if $migratedfrom;
+    # for remote migration the config is available on the target node!
+    if (!$migrate_opts->{remote_node}) {
+	$ENV{PVE_MIGRATED_FROM} = $migratedfrom;
+    }
 
     PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'pre-start', 1);
 
@@ -5650,7 +5653,7 @@ sub vm_start_nolock {
 
 	my $migrate_storage_uri;
 	# nbd_protocol_version > 0 for unix socket support
-	if ($nbd_protocol_version > 0 && $migration_type eq 'secure') {
+	if ($nbd_protocol_version > 0 && ($migration_type eq 'secure' || $migration_type eq 'websocket')) {
 	    my $socket_path = "/run/qemu-server/$vmid\_nbd.migrate";
 	    mon_cmd($vmid, "nbd-server-start", addr => { type => 'unix', data => { path => $socket_path } } );
 	    $migrate_storage_uri = "nbd:unix:$socket_path";
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 qemu-server 10/11] api: add remote migrate endpoint
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (15 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 09/11] migrate: add remote migration handling Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-11 13:01   ` Fabian Ebner
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 11/11] qm: add remote-migrate command Fabian Grünbichler
                   ` (6 subsequent siblings)
  23 siblings, 1 reply; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

entry point for the remote migration on the source side, mainly
preparing the API client that gets passed to the actual migration code
and doing some parameter parsing.

querying of the remote sides resources (like available storages, free
VMIDs, lookup of endpoint details for specific nodes, ...) should be
done by the client - see next commit with CLI example.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    v5:
    - add to API index
    v4:
    - removed target_node parameter, now determined by querying /cluster/status on the remote
    - moved checks to CLI
    
    requires libpve-common-perl with proxmox-remote (already bumped in d/control)

 PVE/API2/Qemu.pm | 217 ++++++++++++++++++++++++++++++++++++++++++++++-
 debian/control   |   6 +-
 2 files changed, 218 insertions(+), 5 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index ca691e20..b2f9b1c0 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -12,6 +12,7 @@ use URI::Escape;
 use Crypt::OpenSSL::Random;
 use Socket qw(SOCK_STREAM);
 
+use PVE::APIClient::LWP;
 use PVE::Cluster qw (cfs_read_file cfs_write_file);;
 use PVE::RRD;
 use PVE::SafeSyslog;
@@ -51,8 +52,6 @@ BEGIN {
     }
 }
 
-use Data::Dumper; # fixme: remove
-
 use base qw(PVE::RESTHandler);
 
 my $opt_force_description = "Force physical removal. Without this, we simple remove the disk from the config file and create an additional configuration entry called 'unused[n]', which contains the volume ID. Unlink of unused[n] always cause physical removal.";
@@ -862,7 +861,8 @@ __PACKAGE__->register_method({
 	    { subdir => 'sendkey' },
 	    { subdir => 'firewall' },
 	    { subdir => 'mtunnel' },
-	    ];
+	    { subdir => 'remote_migrate' },
+	];
 
 	return $res;
     }});
@@ -4016,6 +4016,206 @@ __PACKAGE__->register_method({
 
     }});
 
+__PACKAGE__->register_method({
+    name => 'remote_migrate_vm',
+    path => '{vmid}/remote_migrate',
+    method => 'POST',
+    protected => 1,
+    proxyto => 'node',
+    description => "Migrate virtual machine to a remote cluster. Creates a new migration task.",
+    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-vmid' => get_standard_option('pve-vmid', { optional => 1 }),
+	    'target-endpoint' => get_standard_option('proxmox-remote', {
+		description => "Remote target endpoint",
+	    }),
+	    online => {
+		type => 'boolean',
+		description => "Use online/live migration if VM is running. Ignored if VM is stopped.",
+		optional => 1,
+	    },
+	    'with-local-disks' => {
+		type => 'boolean',
+		description => "Enable live storage migration for local disk",
+		optional => 1,
+	    },
+	    delete => {
+		type => 'boolean',
+		description => "Delete the original VM and related data after successful migration. By default the original VM is kept on the source cluster in a stopped state.",
+		optional => 1,
+		default => 0,
+	    },
+            'target-storage' => get_standard_option('pve-targetstorage', {
+		completion => \&PVE::QemuServer::complete_migration_storage,
+		optional => 0,
+            }),
+	    'target-bridge' => {
+		type => 'string',
+		description => "Mapping from source to target bridges. Providing only a single bridge ID maps all source bridges to that bridge. Providing the special value '1' will map each source bridge to itself.",
+		format => 'bridge-pair-list',
+	    },
+	    bwlimit => {
+		description => "Override I/O bandwidth limit (in KiB/s).",
+		optional => 1,
+		type => 'integer',
+		minimum => '0',
+		default => 'migrate limit from datacenter or storage config',
+	    },
+	},
+    },
+    returns => {
+	type => 'string',
+	description => "the task ID.",
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+
+	my $source_vmid = extract_param($param, 'vmid');
+	my $target_endpoint = extract_param($param, 'target-endpoint');
+	my $target_vmid = extract_param($param, 'target-vmid') // $source_vmid;
+
+	my $delete = extract_param($param, 'delete') // 0;
+
+	PVE::Cluster::check_cfs_quorum();
+
+	# test if VM exists
+	my $conf = PVE::QemuConfig->load_config($source_vmid);
+
+	PVE::QemuConfig->check_lock($conf);
+
+	raise_param_exc({ vmid => "cannot migrate HA-managed VM to remote cluster" })
+	    if PVE::HA::Config::vm_is_ha_managed($source_vmid);
+
+	my $remote = PVE::JSONSchema::parse_property_string('proxmox-remote', $target_endpoint);
+
+	# TODO: move this as helper somewhere appropriate?
+	my $conn_args = {
+	    protocol => 'https',
+	    host => $remote->{host},
+	    port => $remote->{port} // 8006,
+	    apitoken => $remote->{apitoken},
+	};
+
+	my $fp;
+	if ($fp = $remote->{fingerprint}) {
+	    $conn_args->{cached_fingerprints} = { uc($fp) => 1 };
+	}
+
+	print "Establishing API connection with remote at '$remote->{host}'\n";
+
+	my $api_client = PVE::APIClient::LWP->new(%$conn_args);
+
+	if (!defined($fp)) {
+	    my $cert_info = $api_client->get("/nodes/localhost/certificates/info");
+	    foreach my $cert (@$cert_info) {
+		my $filename = $cert->{filename};
+		next if $filename ne 'pveproxy-ssl.pem' && $filename ne 'pve-ssl.pem';
+		$fp = $cert->{fingerprint} if !$fp || $filename eq 'pveproxy-ssl.pem';
+	    }
+	    $conn_args->{cached_fingerprints} = { uc($fp) => 1 }
+		if defined($fp);
+	}
+
+	my $repl_conf = PVE::ReplicationConfig->new();
+	my $is_replicated = $repl_conf->check_for_existing_jobs($source_vmid, 1);
+	die "cannot remote-migrate replicated VM\n" if $is_replicated;
+
+	if (PVE::QemuServer::check_running($source_vmid)) {
+	    die "can't migrate running VM without --online\n" if !$param->{online};
+
+	} else {
+	    warn "VM isn't running. Doing offline migration instead.\n" if $param->{online};
+	    $param->{online} = 0;
+	}
+
+	# FIXME: fork worker hear to avoid timeout? or poll these periodically
+	# in pvestatd and access cached info here? all of the below is actually
+	# checked at the remote end anyway once we call the mtunnel endpoint,
+	# we could also punt it to the client and not do it here at all..
+	my $resources = $api_client->get("/cluster/resources", { type => 'vm' });
+	if (grep { defined($_->{vmid}) && $_->{vmid} eq $target_vmid } @$resources) {
+	    raise_param_exc({ target_vmid => "Guest with ID '$target_vmid' already exists on remote cluster" });
+	}
+
+	my $storages = $api_client->get("/nodes/localhost/storage", { enabled => 1 });
+
+	my $storecfg = PVE::Storage::config();
+	my $target_storage = extract_param($param, 'target-storage');
+	my $storagemap = eval { PVE::JSONSchema::parse_idmap($target_storage, 'pve-storage-id') };
+	raise_param_exc({ 'target-storage' => "failed to parse storage map: $@" })
+	    if $@;
+
+	my $target_bridge = extract_param($param, 'target-bridge');
+	my $bridgemap = eval { PVE::JSONSchema::parse_idmap($target_bridge, 'pve-bridge-id') };
+	raise_param_exc({ 'target-bridge' => "failed to parse bridge map: $@" })
+	    if $@;
+
+	my $check_remote_storage = sub {
+	    my ($storage) = @_;
+	    my $found = [ grep { $_->{storage} eq $storage } @$storages ];
+	    die "remote: storage '$storage' does not exist!\n"
+		if !@$found;
+
+	    $found = @$found[0];
+
+	    my $content_types = [ PVE::Tools::split_list($found->{content}) ];
+	    die "remote: storage '$storage' cannot store images\n"
+		if !grep { $_ eq 'images' } @$content_types;
+	};
+
+	foreach my $target_sid (values %{$storagemap->{entries}}) {
+	    $check_remote_storage->($target_sid);
+	}
+
+	$check_remote_storage->($storagemap->{default})
+	    if $storagemap->{default};
+
+	die "remote migration requires explicit storage mapping!\n"
+	    if $storagemap->{identity};
+
+	$param->{storagemap} = $storagemap;
+	$param->{bridgemap} = $bridgemap;
+	$param->{remote} = {
+	    conn => $conn_args, # re-use fingerprint for tunnel
+	    client => $api_client,
+	    vmid => $target_vmid,
+	};
+	$param->{migration_type} = 'websocket';
+	$param->{delete} = $delete if $delete;
+
+	my $cluster_status = $api_client->get("/cluster/status");
+	my $target_node;
+	foreach my $entry (@$cluster_status) {
+	    next if $entry->{type} ne 'node';
+	    if ($entry->{local}) {
+		$target_node = $entry->{name};
+		last;
+	    }
+	}
+
+	die "couldn't determine endpoint's node name\n"
+	    if !defined($target_node);
+
+	my $realcmd = sub {
+	    PVE::QemuMigrate->migrate($target_node, $remote->{host}, $source_vmid, $param);
+	};
+
+	my $worker = sub {
+	    return PVE::GuestHelpers::guest_migration_lock($source_vmid, 10, $realcmd);
+	};
+
+	return $rpcenv->fork_worker('qmigrate', $source_vmid, $authuser, $worker);
+    }});
+
 __PACKAGE__->register_method({
     name => 'monitor',
     path => '{vmid}/monitor',
@@ -4693,6 +4893,12 @@ __PACKAGE__->register_method({
 		optional => 1,
 		description => 'List of storages to check permission and availability. Will be checked again for all actually used storages during migration.',
 	    },
+	    bridges => {
+		type => 'string',
+		format => 'pve-bridge-id-list',
+		optional => 1,
+		description => 'List of network bridges to check availability. Will be checked again for actually used bridges during migration.',
+	    },
 	},
     },
     returns => {
@@ -4713,6 +4919,7 @@ __PACKAGE__->register_method({
 	my $vmid = extract_param($param, 'vmid');
 
 	my $storages = extract_param($param, 'storages');
+	my $bridges = extract_param($param, 'bridges');
 
 	my $nodename = PVE::INotify::nodename();
 
@@ -4726,6 +4933,10 @@ __PACKAGE__->register_method({
 	    $check_storage_access_migrate->($rpcenv, $authuser, $storecfg, $storeid, $node);
 	}
 
+	foreach my $bridge (PVE::Tools::split_list($bridges)) {
+	    PVE::Network::read_bridge_mtu($bridge);
+	}
+
 	PVE::Cluster::check_cfs_quorum();
 
 	my $socket_addr = "/run/qemu-server/$vmid.mtunnel";
diff --git a/debian/control b/debian/control
index 8d0b856e..c2395ec6 100644
--- a/debian/control
+++ b/debian/control
@@ -6,8 +6,9 @@ Build-Depends: debhelper (>= 12~),
                libglib2.0-dev,
                libio-multiplex-perl,
                libjson-c-dev,
+               libpve-apiclient-perl,
                libpve-cluster-perl,
-               libpve-common-perl (>= 7.0-14),
+               libpve-common-perl (>= 7.0-19),
                libpve-guest-common-perl (>= 3.1-3),
                libpve-storage-perl (>= 6.1-7),
                libtest-mockmodule-perl,
@@ -34,8 +35,9 @@ Depends: dbus,
          libjson-xs-perl,
          libnet-ssleay-perl,
          libpve-access-control (>= 7.0-7),
+         libpve-apiclient-perl,
          libpve-cluster-perl,
-         libpve-common-perl (>= 7.0-14),
+         libpve-common-perl (>= 7.0-19),
          libpve-guest-common-perl (>= 3.1-3),
          libpve-storage-perl (>= 6.3-8),
          libterm-readline-gnu-perl,
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 qemu-server 11/11] qm: add remote-migrate command
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (16 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 10/11] api: add remote migrate endpoint Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 storage 1/3] storage_migrate_snapshot: skip for btrfs without snapshots Fabian Grünbichler
                   ` (5 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

which wraps the remote_migrate_vm API endpoint, but does the
precondition checks that can be done up front itself.

this now just leaves the FP retrieval and target node name lookup to the
sync part of the API endpoint, which should be do-able in <30s ..

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    v5: rename to 'remote-migrate'

 PVE/API2/Qemu.pm |  31 -------------
 PVE/CLI/qm.pm    | 118 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 118 insertions(+), 31 deletions(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index b2f9b1c0..d2ba28e1 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4137,17 +4137,6 @@ __PACKAGE__->register_method({
 	    $param->{online} = 0;
 	}
 
-	# FIXME: fork worker hear to avoid timeout? or poll these periodically
-	# in pvestatd and access cached info here? all of the below is actually
-	# checked at the remote end anyway once we call the mtunnel endpoint,
-	# we could also punt it to the client and not do it here at all..
-	my $resources = $api_client->get("/cluster/resources", { type => 'vm' });
-	if (grep { defined($_->{vmid}) && $_->{vmid} eq $target_vmid } @$resources) {
-	    raise_param_exc({ target_vmid => "Guest with ID '$target_vmid' already exists on remote cluster" });
-	}
-
-	my $storages = $api_client->get("/nodes/localhost/storage", { enabled => 1 });
-
 	my $storecfg = PVE::Storage::config();
 	my $target_storage = extract_param($param, 'target-storage');
 	my $storagemap = eval { PVE::JSONSchema::parse_idmap($target_storage, 'pve-storage-id') };
@@ -4159,26 +4148,6 @@ __PACKAGE__->register_method({
 	raise_param_exc({ 'target-bridge' => "failed to parse bridge map: $@" })
 	    if $@;
 
-	my $check_remote_storage = sub {
-	    my ($storage) = @_;
-	    my $found = [ grep { $_->{storage} eq $storage } @$storages ];
-	    die "remote: storage '$storage' does not exist!\n"
-		if !@$found;
-
-	    $found = @$found[0];
-
-	    my $content_types = [ PVE::Tools::split_list($found->{content}) ];
-	    die "remote: storage '$storage' cannot store images\n"
-		if !grep { $_ eq 'images' } @$content_types;
-	};
-
-	foreach my $target_sid (values %{$storagemap->{entries}}) {
-	    $check_remote_storage->($target_sid);
-	}
-
-	$check_remote_storage->($storagemap->{default})
-	    if $storagemap->{default};
-
 	die "remote migration requires explicit storage mapping!\n"
 	    if $storagemap->{identity};
 
diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
index cf0d6f3d..d2ac6812 100755
--- a/PVE/CLI/qm.pm
+++ b/PVE/CLI/qm.pm
@@ -15,6 +15,7 @@ use POSIX qw(strftime);
 use Term::ReadLine;
 use URI::Escape;
 
+use PVE::APIClient::LWP;
 use PVE::Cluster;
 use PVE::Exception qw(raise_param_exc);
 use PVE::GuestHelpers;
@@ -158,6 +159,122 @@ __PACKAGE__->register_method ({
 	return;
     }});
 
+
+__PACKAGE__->register_method({
+    name => 'remote_migrate_vm',
+    path => 'remote_migrate_vm',
+    method => 'POST',
+    description => "Migrate virtual machine to a remote cluster. Creates a new migration task.",
+    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-vmid' => get_standard_option('pve-vmid', { optional => 1 }),
+	    'target-endpoint' => get_standard_option('proxmox-remote', {
+		description => "Remote target endpoint",
+	    }),
+	    online => {
+		type => 'boolean',
+		description => "Use online/live migration if VM is running. Ignored if VM is stopped.",
+		optional => 1,
+	    },
+	    'with-local-disks' => {
+		type => 'boolean',
+		description => "Enable live storage migration for local disk",
+		optional => 1,
+	    },
+	    delete => {
+		type => 'boolean',
+		description => "Delete the original VM and related data after successful migration. By default the original VM is kept on the source cluster in a stopped state.",
+		optional => 1,
+		default => 0,
+	    },
+            'target-storage' => get_standard_option('pve-targetstorage', {
+		completion => \&PVE::QemuServer::complete_migration_storage,
+		optional => 0,
+            }),
+	    'target-bridge' => {
+		type => 'string',
+		description => "Mapping from source to target bridges. Providing only a single bridge ID maps all source bridges to that bridge. Providing the special value '1' will map each source bridge to itself.",
+		format => 'bridge-pair-list',
+	    },
+	    bwlimit => {
+		description => "Override I/O bandwidth limit (in KiB/s).",
+		optional => 1,
+		type => 'integer',
+		minimum => '0',
+		default => 'migrate limit from datacenter or storage config',
+	    },
+	},
+    },
+    returns => {
+	type => 'string',
+	description => "the task ID.",
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+
+	my $source_vmid = $param->{vmid};
+	my $target_endpoint = $param->{'target-endpoint'};
+	my $target_vmid = $param->{'target-vmid'} // $source_vmid;
+
+	my $remote = PVE::JSONSchema::parse_property_string('proxmox-remote', $target_endpoint);
+
+	# TODO: move this as helper somewhere appropriate?
+	my $conn_args = {
+	    protocol => 'https',
+	    host => $remote->{host},
+	    port => $remote->{port} // 8006,
+	    apitoken => $remote->{apitoken},
+	};
+
+	$conn_args->{cached_fingerprints} = { uc($remote->{fingerprint}) => 1 }
+	    if defined($remote->{fingerprint});
+
+	my $api_client = PVE::APIClient::LWP->new(%$conn_args);
+	my $resources = $api_client->get("/cluster/resources", { type => 'vm' });
+	if (grep { defined($_->{vmid}) && $_->{vmid} eq $target_vmid } @$resources) {
+	    raise_param_exc({ target_vmid => "Guest with ID '$target_vmid' already exists on remote cluster" });
+	}
+
+	my $storages = $api_client->get("/nodes/localhost/storage", { enabled => 1 });
+
+	my $storecfg = PVE::Storage::config();
+	my $target_storage = $param->{'target-storage'};
+	my $storagemap = eval { PVE::JSONSchema::parse_idmap($target_storage, 'pve-storage-id') };
+	raise_param_exc({ 'target-storage' => "failed to parse storage map: $@" })
+	    if $@;
+
+	my $check_remote_storage = sub {
+	    my ($storage) = @_;
+	    my $found = [ grep { $_->{storage} eq $storage } @$storages ];
+	    die "remote: storage '$storage' does not exist!\n"
+		if !@$found;
+
+	    $found = @$found[0];
+
+	    my $content_types = [ PVE::Tools::split_list($found->{content}) ];
+	    die "remote: storage '$storage' cannot store images\n"
+		if !grep { $_ eq 'images' } @$content_types;
+	};
+
+	foreach my $target_sid (values %{$storagemap->{entries}}) {
+	    $check_remote_storage->($target_sid);
+	}
+
+	$check_remote_storage->($storagemap->{default})
+	    if $storagemap->{default};
+
+	return PVE::API2::Qemu->remote_migrate_vm($param);
+    }});
+
 __PACKAGE__->register_method ({
     name => 'status',
     path => 'status',
@@ -900,6 +1017,7 @@ our $cmddef = {
     clone => [ "PVE::API2::Qemu", 'clone_vm', ['vmid', 'newid'], { node => $nodename }, $upid_exit ],
 
     migrate => [ "PVE::API2::Qemu", 'migrate_vm', ['vmid', 'target'], { node => $nodename }, $upid_exit ],
+    'remote-migrate' => [ __PACKAGE__, 'remote_migrate_vm', ['vmid', 'target-vmid', 'target-endpoint'], { node => $nodename }, $upid_exit ],
 
     set => [ "PVE::API2::Qemu", 'update_vm', ['vmid'], { node => $nodename } ],
 
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 storage 1/3] storage_migrate_snapshot: skip for btrfs without snapshots
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (17 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 11/11] qm: add remote-migrate command Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 storage 2/3] storage_migrate: pull out import/export_prepare Fabian Grünbichler
                   ` (4 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

this allows migrating from btrfs to other raw+size accepting storages,
provided no snapshots exist.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    new in v5

 PVE/Storage.pm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 93ae3ac..866b5cd 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -645,10 +645,10 @@ my $volname_for_storage = sub {
 
 # whether a migration snapshot is needed for a given storage
 sub storage_migrate_snapshot {
-    my ($cfg, $storeid) = @_;
+    my ($cfg, $storeid, $existing_snapshots) = @_;
     my $scfg = storage_config($cfg, $storeid);
 
-    return $scfg->{type} eq 'zfspool' || $scfg->{type} eq 'btrfs';
+    return $scfg->{type} eq 'zfspool' || ($scfg->{type} eq 'btrfs' && $existing_snapshots);
 }
 
 sub storage_migrate {
@@ -696,7 +696,7 @@ sub storage_migrate {
 
     my $migration_snapshot;
     if (!defined($snapshot)) {
-	$migration_snapshot = storage_migrate_snapshot($cfg, $storeid);
+	$migration_snapshot = storage_migrate_snapshot($cfg, $storeid, $opts->{with_snapshots});
 	$snapshot = '__migration__' if $migration_snapshot;
     }
 
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 storage 2/3] storage_migrate: pull out import/export_prepare
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (18 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 storage 1/3] storage_migrate_snapshot: skip for btrfs without snapshots Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 storage 3/3] add volume_import/export_start helpers Fabian Grünbichler
                   ` (3 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

for re-use with remote migration, where import and export happen on
different clusters connected via a websocket instead of SSH tunnel.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    v4:
    - unify array refs
    - small fixups
    new in v3

 PVE/Storage.pm | 113 ++++++++++++++++++++++++++++++-------------------
 1 file changed, 70 insertions(+), 43 deletions(-)

diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 866b5cd..5a5874a 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -651,16 +651,70 @@ sub storage_migrate_snapshot {
     return $scfg->{type} eq 'zfspool' || ($scfg->{type} eq 'btrfs' && $existing_snapshots);
 }
 
-sub storage_migrate {
-    my ($cfg, $volid, $target_sshinfo, $target_storeid, $opts, $logfunc) = @_;
+my $volume_import_prepare = sub {
+    my ($volid, $format, $path, $apiver, $opts) = @_;
 
     my $base_snapshot = $opts->{base_snapshot};
     my $snapshot = $opts->{snapshot};
-    my $ratelimit_bps = $opts->{ratelimit_bps};
-    my $insecure = $opts->{insecure};
     my $with_snapshots = $opts->{with_snapshots} ? 1 : 0;
+    my $migration_snapshot = $opts->{migration_snapshot} ? 1 : 0;
     my $allow_rename = $opts->{allow_rename} ? 1 : 0;
 
+    my $recv = ['pvesm', 'import', $volid, $format, $path, '-with-snapshots', $with_snapshots];
+    if (defined($snapshot)) {
+	push @$recv, '-snapshot', $snapshot;
+    }
+    if ($migration_snapshot) {
+	push @$recv, '-delete-snapshot', $snapshot;
+    }
+    push @$recv, '-allow-rename', $allow_rename if $apiver >= 5;
+
+    if (defined($base_snapshot)) {
+	# Check if the snapshot exists on the remote side:
+	push @$recv, '-base', $base_snapshot if $apiver >= 9;
+    }
+
+    return $recv;
+};
+
+my $volume_export_prepare = sub {
+    my ($cfg, $volid, $format, $logfunc, $opts) = @_;
+    my $base_snapshot = $opts->{base_snapshot};
+    my $snapshot = $opts->{snapshot};
+    my $with_snapshots = $opts->{with_snapshots} ? 1 : 0;
+    my $migration_snapshot = $opts->{migration_snapshot} ? 1 : 0;
+    my $ratelimit_bps = $opts->{ratelimit_bps};
+
+    my $send = ['pvesm', 'export', $volid, $format, '-', '-with-snapshots', $with_snapshots];
+    if (defined($snapshot)) {
+	push @$send, '-snapshot', $snapshot;
+    }
+    if (defined($base_snapshot)) {
+	push @$send, '-base', $base_snapshot;
+    }
+
+    my $cstream;
+    if (defined($ratelimit_bps)) {
+	$cstream = [ '/usr/bin/cstream', '-t', $ratelimit_bps ];
+	$logfunc->("using a bandwidth limit of $ratelimit_bps bps for transferring '$volid'") if $logfunc;
+    }
+
+    volume_snapshot($cfg, $volid, $snapshot) if $migration_snapshot;
+
+    if (defined($snapshot)) {
+	activate_volumes($cfg, [$volid], $snapshot);
+    } else {
+	activate_volumes($cfg, [$volid]);
+    }
+
+    return $cstream ? [ $send, $cstream ] : [ $send ];
+};
+
+sub storage_migrate {
+    my ($cfg, $volid, $target_sshinfo, $target_storeid, $opts, $logfunc) = @_;
+
+    my $insecure = $opts->{insecure};
+
     my ($storeid, $volname) = parse_volume_id($volid);
 
     my $scfg = storage_config($cfg, $storeid);
@@ -688,19 +742,12 @@ sub storage_migrate {
     my $ssh_base = PVE::SSHInfo::ssh_info_to_command_base($target_sshinfo);
     local $ENV{RSYNC_RSH} = PVE::Tools::cmd2string($ssh_base);
 
-    my @cstream;
-    if (defined($ratelimit_bps)) {
-	@cstream = ([ '/usr/bin/cstream', '-t', $ratelimit_bps ]);
-	$logfunc->("using a bandwidth limit of $ratelimit_bps bps for transferring '$volid'") if $logfunc;
+    if (!defined($opts->{snapshot})) {
+	$opts->{migration_snapshot} = storage_migrate_snapshot($cfg, $storeid, $opts->{with_snapshots});
+	$opts->{snapshot} = '__migration__' if $opts->{migration_snapshot};
     }
 
-    my $migration_snapshot;
-    if (!defined($snapshot)) {
-	$migration_snapshot = storage_migrate_snapshot($cfg, $storeid, $opts->{with_snapshots});
-	$snapshot = '__migration__' if $migration_snapshot;
-    }
-
-    my @formats = volume_transfer_formats($cfg, $volid, $target_volid, $snapshot, $base_snapshot, $with_snapshots);
+    my @formats = volume_transfer_formats($cfg, $volid, $target_volid, $opts->{snapshot}, $opts->{base_snapshot}, $opts->{with_snapshots});
     die "cannot migrate from storage type '$scfg->{type}' to '$tcfg->{type}'\n" if !@formats;
     my $format = $formats[0];
 
@@ -715,22 +762,7 @@ sub storage_migrate {
     my $match_api_version = sub { $target_apiver = $1 if $_[0] =~ m!^APIVER (\d+)$!; };
     eval { run_command($get_api_version, logfunc => $match_api_version); };
 
-    my $send = ['pvesm', 'export', $volid, $format, '-', '-with-snapshots', $with_snapshots];
-    my $recv = [@$ssh, '--', 'pvesm', 'import', $target_volid, $format, $import_fn, '-with-snapshots', $with_snapshots];
-    if (defined($snapshot)) {
-	push @$send, '-snapshot', $snapshot;
-	push @$recv, '-snapshot', $snapshot;
-    }
-    if ($migration_snapshot) {
-	push @$recv, '-delete-snapshot', $snapshot;
-    }
-    push @$recv, '-allow-rename', $allow_rename if $target_apiver >= 5;
-
-    if (defined($base_snapshot)) {
-	# Check if the snapshot exists on the remote side:
-	push @$send, '-base', $base_snapshot;
-	push @$recv, '-base', $base_snapshot if $target_apiver >= 9;
-    }
+    my $recv = [ @$ssh, '--', $volume_import_prepare->($target_volid, $format, $import_fn, $target_apiver, $opts)->@* ];
 
     my $new_volid;
     my $pattern = volume_imported_message(undef, 1);
@@ -745,19 +777,13 @@ sub storage_migrate {
 	}
     };
 
-    volume_snapshot($cfg, $volid, $snapshot) if $migration_snapshot;
-
-    if (defined($snapshot)) {
-	activate_volumes($cfg, [$volid], $snapshot);
-    } else {
-	activate_volumes($cfg, [$volid]);
-    }
+    my $cmds = $volume_export_prepare->($cfg, $volid, $format, $logfunc, $opts);
 
     eval {
 	if ($insecure) {
 	    my $input = IO::File->new();
 	    my $info = IO::File->new();
-	    open3($input, $info, $info, @{$recv})
+	    open3($input, $info, $info, @$recv)
 		or die "receive command failed: $!\n";
 	    close($input);
 
@@ -774,7 +800,7 @@ sub storage_migrate {
 	    # we won't be reading from the socket
 	    shutdown($socket, 0);
 
-	    eval { run_command([$send, @cstream], output => '>&'.fileno($socket), errfunc => $logfunc); };
+	    eval { run_command($cmds, output => '>&'.fileno($socket), errfunc => $logfunc); };
 	    my $send_error = $@;
 
 	    # don't close the connection entirely otherwise the receiving end
@@ -795,7 +821,8 @@ sub storage_migrate {
 
 	    die $send_error if $send_error;
 	} else {
-	    run_command([$send, @cstream, $recv], logfunc => $match_volid_and_log);
+	    push @$cmds, $recv;
+	    run_command($cmds, logfunc => $match_volid_and_log);
 	}
 
 	die "unable to get ID of the migrated volume\n"
@@ -803,8 +830,8 @@ sub storage_migrate {
     };
     my $err = $@;
     warn "send/receive failed, cleaning up snapshot(s)..\n" if $err;
-    if ($migration_snapshot) {
-	eval { volume_snapshot_delete($cfg, $volid, $snapshot, 0) };
+    if ($opts->{migration_snapshot}) {
+	eval { volume_snapshot_delete($cfg, $volid, $opts->{snapshot}, 0) };
 	warn "could not remove source snapshot: $@\n" if $@;
     }
     die $err if $err;
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH v5 storage 3/3] add volume_import/export_start helpers
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (19 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 storage 2/3] storage_migrate: pull out import/export_prepare Fabian Grünbichler
@ 2022-02-09 13:07 ` Fabian Grünbichler
  2022-02-09 17:56 ` [pve-devel] [PATCH-SERIES 0/21] remote migration Thomas Lamprecht
                   ` (2 subsequent siblings)
  23 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-09 13:07 UTC (permalink / raw)
  To: pve-devel

exposing the two halves of a storage migration for usage across
cluster boundaries.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    v5:
    - re-use migration snapshot info from source in volume_import_start
      (fixes migration from non-btrfs to btrfs)
    - correctly pass snapshot (name), not migration_snapshot (bool) to
      volume_import_formats
    v4:
    - add log parameter
    - unify array-refs
    
    new in v3

 PVE/Storage.pm | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 5a5874a..b1d31bb 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -1833,6 +1833,67 @@ sub volume_imported_message {
     }
 }
 
+# $format and $volname are requests and might be overruled depending on $opts
+# $opts:
+# - with_snapshots: passed to `pvesm import` and used to select import format
+# - allow_rename: passed to `pvesm import`
+# - export_formats: used to select common transport format
+# - unix: unix socket path
+sub volume_import_start {
+    my ($cfg, $storeid, $volname, $format, $vmid, $opts) = @_;
+
+    my $with_snapshots = $opts->{'with_snapshots'} ? 1 : 0;
+
+    $volname = $volname_for_storage->($cfg, $storeid, $volname, $vmid, $format);
+
+    my $volid = "$storeid:$volname";
+
+    # find common import/export format, like volume_transfer_formats
+    my @import_formats = PVE::Storage::volume_import_formats($cfg, $volid, $opts->{snapshot}, undef, $with_snapshots);
+    my @export_formats = PVE::Tools::split_list($opts->{export_formats});
+    my %import_hash = map { $_ => 1 } @import_formats;
+    my @common = grep { $import_hash{$_} } @export_formats;
+    die "no matching import/export format found for storage '$storeid'\n"
+	if !@common;
+    $format = $common[0];
+
+    my $input = IO::File->new();
+    my $info = IO::File->new();
+
+    my $unix = $opts->{unix} // "/run/pve/storage-migrate-$vmid.$$.unix";
+    my $import = $volume_import_prepare->($volid, $format, "unix://$unix", APIVER, $opts);
+
+    unlink $unix;
+    my $cpid = open3($input, $info, $info, @$import)
+	or die "failed to spawn disk-import child - $!\n";
+
+    my $ready;
+    eval {
+	PVE::Tools::run_with_timeout(5, sub { $ready = <$info>; });
+    };
+
+    die "failed to read readyness from disk import child: $@\n" if $@;
+
+    print "$ready\n";
+
+    return {
+	fh => $info,
+	pid => $cpid,
+	socket => $unix,
+	format => $format,
+    };
+}
+
+sub volume_export_start {
+    my ($cfg, $volid, $format, $log, $opts) = @_;
+
+    my $run_command_params = delete $opts->{cmd} // {};
+
+    my $cmds = $volume_export_prepare->($cfg, $volid, $format, $log, $opts);
+
+    PVE::Tools::run_command($cmds, %$run_command_params);
+}
+
 # bash completion helper
 
 sub complete_storage {
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [pve-devel] [PATCH-SERIES 0/21] remote migration
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (20 preceding siblings ...)
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 storage 3/3] add volume_import/export_start helpers Fabian Grünbichler
@ 2022-02-09 17:56 ` Thomas Lamprecht
  2022-02-11 10:38 ` [pve-devel] [PATCH qemu-server follow-up] schema: move 'pve-targetstorage' to pve-common Fabian Grünbichler
  2022-02-11 13:08 ` [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Ebner
  23 siblings, 0 replies; 35+ messages in thread
From: Thomas Lamprecht @ 2022-02-09 17:56 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Grünbichler

On 09.02.22 14:07, Fabian Grünbichler wrote:
> this series adds remote migration for VMs (and if the last pve-container
> patch is applied, CTs ;)).
> 
> both live and offline migration of VMs including NBD and storage-migrated disks
> should work, containers don't have any live migration so both offline
> and restart mode work identical except for the restart part.
> 

applied all but those marked explicit below.

> pve-common:
>   add 'map_id' helper for ID maps
> 
> pve-container:
>   fix #1532: add target-storage support to migration
>   config: add strict parser
>   migration: add remote migration

skipped the POC above for now, had not really time to check it out closely but
wanted to already reduce the rats tail of patches we need to pull along, so
trusting that we do not need to break much now anymore :-)

> 
> qemu-server:
>   move map_storage to PVE::JSONSchema::map_id
>   schema: use pve-bridge-id
>   parse_config: optional strict mode
>   update_vm: allow simultaneous setting of boot-order and dev
>   nbd alloc helper: allow passing in explicit format
>   migrate: move tunnel-helpers to pve-guest-common
>   mtunnel: add API endpoints

skipped the four below for now, need to take a bit closer look and
some testing.

>   migrate: refactor remote VM/tunnel start
>   migrate: add remote migration handling
>   api: add remote migrate endpoint
>   qm: add remote-migrate command
> 
> pve-storage:
>   storage_migrate_snapshot: skip for btrfs without snapshots
>   storage_migrate: pull out import/export_prepare
>   add volume_import/export_start helpers
> 
> pve-guest-common:
>   migrate: add get_bwlimit helper
>   add tunnel helper module
>   add storage tunnel module

many thanks!




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [pve-devel] [PATCH v5 container 1/3] fix #1532: add target-storage support to migration
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 container 1/3] fix #1532: add target-storage support to migration Fabian Grünbichler
@ 2022-02-10 11:52   ` Fabian Ebner
  2022-02-11  8:33     ` Fabian Grünbichler
  0 siblings, 1 reply; 35+ messages in thread
From: Fabian Ebner @ 2022-02-10 11:52 UTC (permalink / raw)
  To: pve-devel, Fabian Grünbichler

Am 09.02.22 um 14:07 schrieb Fabian Grünbichler:
> re-using helpers that already exist for qemu-server. this is a
> pre-requisite for extending remote migration support to containers.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> 
> Notes:
>     new in v5, no GUI yet until possible wrinkles are ironed out
>     
>     requires pve-common/pve-guest-common with changes from this series
> 
>  src/PVE/API2/LXC.pm    | 32 ++++++++++++++++++++++
>  src/PVE/LXC/Migrate.pm | 62 ++++++++++++++++++++++++++++++++----------
>  2 files changed, 79 insertions(+), 15 deletions(-)
> 
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index 7573814..61eaaf7 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -36,6 +36,18 @@ BEGIN {
>      }
>  }
>  
> +my $check_storage_access_migrate = sub {
> +    my ($rpcenv, $authuser, $storecfg, $storage, $node) = @_;
> +
> +    PVE::Storage::storage_check_enabled($storecfg, $storage, $node);
> +
> +    $rpcenv->check($authuser, "/storage/$storage", ['Datastore.AllocateSpace']);
> +
> +    my $scfg = PVE::Storage::storage_config($storecfg, $storage);
> +    die "storage '$storage' does not support CT rootdirs\n"
> +	if !$scfg->{content}->{rootdir};
> +};
> +
>  __PACKAGE__->register_method ({
>      subclass => "PVE::API2::LXC::Config",
>      path => '{vmid}/config',
> @@ -1091,6 +1103,7 @@ __PACKAGE__->register_method({
>  		description => "Target node.",
>  		completion => \&PVE::Cluster::complete_migration_target,
>  	    }),
> +	    'target-storage' => get_standard_option('pve-targetstorage'),

This option is currently registered in PVE/QemuServer.pm and I don't
think we want to depend on that ;)

Also, the parameter is named 'targetstorage' for VMs. I do agree this
one is nicer, but it might be confusing.

>  	    online => {
>  		type => 'boolean',
>  		description => "Use online/live migration.",
> @@ -1149,6 +1162,25 @@ __PACKAGE__->register_method({
>  		if !$param->{online} && !$param->{restart};
>  	}
>  
> +	if (my $targetstorage = delete $param->{'target-storage'}) {
> +	    my $storecfg = PVE::Storage::config();
> +	    my $storagemap = eval { PVE::JSONSchema::parse_idmap($targetstorage, 'pve-storage-id') };
> +	    raise_param_exc({ targetstorage => "failed to parse storage map: $@" })

Should be 'target-storage' if we go with that.

> +		if $@;
> +
> +	    $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.Disk'])
> +		if !defined($storagemap->{identity});
> +
> +	    foreach my $target_sid (values %{$storagemap->{entries}}) {
> +		$check_storage_access_migrate->($rpcenv, $authuser, $storecfg, $target_sid, $target);
> +	    }
> +
> +	    $check_storage_access_migrate->($rpcenv, $authuser, $storecfg, $storagemap->{default}, $target)
> +		if $storagemap->{default};
> +
> +	    $param->{storagemap} = $storagemap;
> +	}
> +
>  	if (PVE::HA::Config::vm_is_ha_managed($vmid) && $rpcenv->{type} ne 'ha') {
>  
>  	    my $hacmd = sub {
> diff --git a/src/PVE/LXC/Migrate.pm b/src/PVE/LXC/Migrate.pm
> index 95562e4..c85a09c 100644
> --- a/src/PVE/LXC/Migrate.pm
> +++ b/src/PVE/LXC/Migrate.pm

----8<----

> @@ -194,7 +210,8 @@ sub phase1 {
>  	next if @{$dl->{$storeid}} == 0;
>  
>  	# check if storage is available on target node
> -	PVE::Storage::storage_check_enabled($self->{storecfg}, $storeid, $self->{node});
> +	my $targetsid = PVE::JSONSchema::map_id($self->{opts}->{storagemap}, $storeid);
> +	PVE::Storage::storage_check_enabled($self->{storecfg}, $targetsid, $self->{node});
>  
>  	die "content type 'rootdir' is not available on storage '$storeid'\n"
>  	    if !$scfg->{content}->{rootdir};

Should use target's scfg.

> @@ -275,25 +292,38 @@ sub phase1 {
>  	next if $rep_volumes->{$volid};
>  	my ($sid, $volname) = PVE::Storage::parse_volume_id($volid);
>  	push @{$self->{volumes}}, $volid;
> -	my $bwlimit = PVE::Storage::get_bandwidth_limit('migration', [$sid], $opts->{bwlimit});
> +
>  	# JSONSchema and get_bandwidth_limit use kbps - storage_migrate bps
> +	my $bwlimit = $volhash->{$volid}->{bwlimit};
>  	$bwlimit = $bwlimit * 1024 if defined($bwlimit);
>  
> -	my $storage_migrate_opts = {
> -	    'ratelimit_bps' => $bwlimit,
> -	    'insecure' => $opts->{migration_type} eq 'insecure',
> -	    'with_snapshots' => $volhash->{$volid}->{snapshots},
> +	my $targetsid = $volhash->{$volid}->{targetsid};
> +
> +	my $new_volid = eval {
> +	    my $storage_migrate_opts = {
> +		'ratelimit_bps' => $bwlimit,
> +		'insecure' => $opts->{migration_type} eq 'insecure',
> +		'with_snapshots' => $volhash->{$volid}->{snapshots},

Since we update the config below, I think we can now also enable
allow_rename here. Otherwise, it's rather easy to run into conflicts
when mapping two storages on the source side to the same one on the target.

> +	    };
> +
> +	    my $logfunc = sub { $self->log('info', $_[0]); };
> +	    return PVE::Storage::storage_migrate(
> +		$self->{storecfg},
> +		$volid,
> +		$self->{ssh_info},
> +		$targetsid,
> +		$storage_migrate_opts,
> +		$logfunc,
> +	    );
>  	};
>  
> -	my $logfunc = sub { $self->log('info', $_[0]); };
> -	eval {
> -	    PVE::Storage::storage_migrate($self->{storecfg}, $volid, $self->{ssh_info},
> -					  $sid, $storage_migrate_opts, $logfunc);
> -	};
>  	if (my $err = $@) {
> -	    die "storage migration for '$volid' to storage '$sid' failed - $err\n";
> +	    die "storage migration for '$volid' to storage '$targetsid' failed - $err\n";
>  	}
>  
> +	$self->{volume_map}->{$volid} = $new_volid;
> +	$self->log('info', "volume '$volid' is '$new_volid' on the target\n");
> +
>  	eval { PVE::Storage::deactivate_volumes($self->{storecfg}, [$volid]); };
>  	if (my $err = $@) {
>  	    $self->log('warn', $err);
> @@ -316,6 +346,8 @@ sub phase1 {
>  
>      # transfer replication state before moving config
>      $self->transfer_replication_state() if $rep_volumes;
> +    PVE::LXC::Config->update_volume_ids($conf, $self->{volume_map});
> +    PVE::LXC::Config->write_config($vmid, $conf);
>      PVE::LXC::Config->move_config_to_node($vmid, $self->{node});
>      $self->{conf_migrated} = 1;
>      $self->switch_replication_job_target() if $rep_volumes;




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [pve-devel] [PATCH v5 container 1/3] fix #1532: add target-storage support to migration
  2022-02-10 11:52   ` Fabian Ebner
@ 2022-02-11  8:33     ` Fabian Grünbichler
  0 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-11  8:33 UTC (permalink / raw)
  To: Fabian Ebner, pve-devel

On February 10, 2022 12:52 pm, Fabian Ebner wrote:
> Am 09.02.22 um 14:07 schrieb Fabian Grünbichler:
>> re-using helpers that already exist for qemu-server. this is a
>> pre-requisite for extending remote migration support to containers.
>> 
>> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
>> ---
>> 
>> Notes:
>>     new in v5, no GUI yet until possible wrinkles are ironed out
>>     
>>     requires pve-common/pve-guest-common with changes from this series
>> 
>>  src/PVE/API2/LXC.pm    | 32 ++++++++++++++++++++++
>>  src/PVE/LXC/Migrate.pm | 62 ++++++++++++++++++++++++++++++++----------
>>  2 files changed, 79 insertions(+), 15 deletions(-)
>> 
>> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
>> index 7573814..61eaaf7 100644
>> --- a/src/PVE/API2/LXC.pm
>> +++ b/src/PVE/API2/LXC.pm
>> @@ -36,6 +36,18 @@ BEGIN {
>>      }
>>  }
>>  
>> +my $check_storage_access_migrate = sub {
>> +    my ($rpcenv, $authuser, $storecfg, $storage, $node) = @_;
>> +
>> +    PVE::Storage::storage_check_enabled($storecfg, $storage, $node);
>> +
>> +    $rpcenv->check($authuser, "/storage/$storage", ['Datastore.AllocateSpace']);
>> +
>> +    my $scfg = PVE::Storage::storage_config($storecfg, $storage);
>> +    die "storage '$storage' does not support CT rootdirs\n"
>> +	if !$scfg->{content}->{rootdir};
>> +};
>> +
>>  __PACKAGE__->register_method ({
>>      subclass => "PVE::API2::LXC::Config",
>>      path => '{vmid}/config',
>> @@ -1091,6 +1103,7 @@ __PACKAGE__->register_method({
>>  		description => "Target node.",
>>  		completion => \&PVE::Cluster::complete_migration_target,
>>  	    }),
>> +	    'target-storage' => get_standard_option('pve-targetstorage'),
> 
> This option is currently registered in PVE/QemuServer.pm and I don't
> think we want to depend on that ;)

ugh, yes.

> Also, the parameter is named 'targetstorage' for VMs. I do agree this
> one is nicer, but it might be confusing.

I opted for 'target-storage' since it's how we'd name the parameter now 
(same for qemu-server's remote migration API with its multiple 
target-foo parameters ;)), and would like to phase out the 
'targetstorage' in the regular VM migration via an alias..

> 
>>  	    online => {
>>  		type => 'boolean',
>>  		description => "Use online/live migration.",
>> @@ -1149,6 +1162,25 @@ __PACKAGE__->register_method({
>>  		if !$param->{online} && !$param->{restart};
>>  	}
>>  
>> +	if (my $targetstorage = delete $param->{'target-storage'}) {
>> +	    my $storecfg = PVE::Storage::config();
>> +	    my $storagemap = eval { PVE::JSONSchema::parse_idmap($targetstorage, 'pve-storage-id') };
>> +	    raise_param_exc({ targetstorage => "failed to parse storage map: $@" })
> 
> Should be 'target-storage' if we go with that.

ack.

> 
>> +		if $@;
>> +
>> +	    $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.Disk'])
>> +		if !defined($storagemap->{identity});
>> +
>> +	    foreach my $target_sid (values %{$storagemap->{entries}}) {
>> +		$check_storage_access_migrate->($rpcenv, $authuser, $storecfg, $target_sid, $target);
>> +	    }
>> +
>> +	    $check_storage_access_migrate->($rpcenv, $authuser, $storecfg, $storagemap->{default}, $target)
>> +		if $storagemap->{default};
>> +
>> +	    $param->{storagemap} = $storagemap;
>> +	}
>> +
>>  	if (PVE::HA::Config::vm_is_ha_managed($vmid) && $rpcenv->{type} ne 'ha') {
>>  
>>  	    my $hacmd = sub {
>> diff --git a/src/PVE/LXC/Migrate.pm b/src/PVE/LXC/Migrate.pm
>> index 95562e4..c85a09c 100644
>> --- a/src/PVE/LXC/Migrate.pm
>> +++ b/src/PVE/LXC/Migrate.pm
> 
> ----8<----
> 
>> @@ -194,7 +210,8 @@ sub phase1 {
>>  	next if @{$dl->{$storeid}} == 0;
>>  
>>  	# check if storage is available on target node
>> -	PVE::Storage::storage_check_enabled($self->{storecfg}, $storeid, $self->{node});
>> +	my $targetsid = PVE::JSONSchema::map_id($self->{opts}->{storagemap}, $storeid);
>> +	PVE::Storage::storage_check_enabled($self->{storecfg}, $targetsid, $self->{node});
>>  
>>  	die "content type 'rootdir' is not available on storage '$storeid'\n"
>>  	    if !$scfg->{content}->{rootdir};
> 
> Should use target's scfg.

ack - had it in prepare (for the config-referenced volumes), but missed 
the second one here.

> 
>> @@ -275,25 +292,38 @@ sub phase1 {
>>  	next if $rep_volumes->{$volid};
>>  	my ($sid, $volname) = PVE::Storage::parse_volume_id($volid);
>>  	push @{$self->{volumes}}, $volid;
>> -	my $bwlimit = PVE::Storage::get_bandwidth_limit('migration', [$sid], $opts->{bwlimit});
>> +
>>  	# JSONSchema and get_bandwidth_limit use kbps - storage_migrate bps
>> +	my $bwlimit = $volhash->{$volid}->{bwlimit};
>>  	$bwlimit = $bwlimit * 1024 if defined($bwlimit);
>>  
>> -	my $storage_migrate_opts = {
>> -	    'ratelimit_bps' => $bwlimit,
>> -	    'insecure' => $opts->{migration_type} eq 'insecure',
>> -	    'with_snapshots' => $volhash->{$volid}->{snapshots},
>> +	my $targetsid = $volhash->{$volid}->{targetsid};
>> +
>> +	my $new_volid = eval {
>> +	    my $storage_migrate_opts = {
>> +		'ratelimit_bps' => $bwlimit,
>> +		'insecure' => $opts->{migration_type} eq 'insecure',
>> +		'with_snapshots' => $volhash->{$volid}->{snapshots},
> 
> Since we update the config below, I think we can now also enable
> allow_rename here. Otherwise, it's rather easy to run into conflicts
> when mapping two storages on the source side to the same one on the target.

true!

> 
>> +	    };
>> +
>> +	    my $logfunc = sub { $self->log('info', $_[0]); };
>> +	    return PVE::Storage::storage_migrate(
>> +		$self->{storecfg},
>> +		$volid,
>> +		$self->{ssh_info},
>> +		$targetsid,
>> +		$storage_migrate_opts,
>> +		$logfunc,
>> +	    );
>>  	};
>>  
>> -	my $logfunc = sub { $self->log('info', $_[0]); };
>> -	eval {
>> -	    PVE::Storage::storage_migrate($self->{storecfg}, $volid, $self->{ssh_info},
>> -					  $sid, $storage_migrate_opts, $logfunc);
>> -	};
>>  	if (my $err = $@) {
>> -	    die "storage migration for '$volid' to storage '$sid' failed - $err\n";
>> +	    die "storage migration for '$volid' to storage '$targetsid' failed - $err\n";
>>  	}
>>  
>> +	$self->{volume_map}->{$volid} = $new_volid;
>> +	$self->log('info', "volume '$volid' is '$new_volid' on the target\n");
>> +
>>  	eval { PVE::Storage::deactivate_volumes($self->{storecfg}, [$volid]); };
>>  	if (my $err = $@) {
>>  	    $self->log('warn', $err);
>> @@ -316,6 +346,8 @@ sub phase1 {
>>  
>>      # transfer replication state before moving config
>>      $self->transfer_replication_state() if $rep_volumes;
>> +    PVE::LXC::Config->update_volume_ids($conf, $self->{volume_map});
>> +    PVE::LXC::Config->write_config($vmid, $conf);
>>      PVE::LXC::Config->move_config_to_node($vmid, $self->{node});
>>      $self->{conf_migrated} = 1;
>>      $self->switch_replication_job_target() if $rep_volumes;
> 




^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH qemu-server follow-up] schema: move 'pve-targetstorage' to pve-common
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (21 preceding siblings ...)
  2022-02-09 17:56 ` [pve-devel] [PATCH-SERIES 0/21] remote migration Thomas Lamprecht
@ 2022-02-11 10:38 ` Fabian Grünbichler
  2022-02-11 10:38   ` [pve-devel] [PATCH common follow-up] schema: take over 'pve-targetstorage' option Fabian Grünbichler
  2022-02-11 11:31   ` [pve-devel] [PATCH qemu-server follow-up] schema: move 'pve-targetstorage' to pve-common Fabian Ebner
  2022-02-11 13:08 ` [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Ebner
  23 siblings, 2 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-11 10:38 UTC (permalink / raw)
  To: pve-devel

for proper re-use in pve-container.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    requires dependency on bumped libpve-common-perl

 PVE/QemuServer.pm | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index e703cee9..2228fe87 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -119,13 +119,6 @@ PVE::JSONSchema::register_standard_option('pve-qemu-machine', {
 	optional => 1,
 });
 
-PVE::JSONSchema::register_standard_option('pve-targetstorage', {
-    description => "Mapping from source to target storages. Providing only a single storage ID maps all source storages to that storage. Providing the special value '1' will map each source storage to itself.",
-    type => 'string',
-    format => 'storage-pair-list',
-    optional => 1,
-});
-
 #no warnings 'redefine';
 
 my $nodename_cache;
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* [pve-devel] [PATCH common follow-up] schema: take over 'pve-targetstorage' option
  2022-02-11 10:38 ` [pve-devel] [PATCH qemu-server follow-up] schema: move 'pve-targetstorage' to pve-common Fabian Grünbichler
@ 2022-02-11 10:38   ` Fabian Grünbichler
  2022-02-11 11:31   ` [pve-devel] [PATCH qemu-server follow-up] schema: move 'pve-targetstorage' to pve-common Fabian Ebner
  1 sibling, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-11 10:38 UTC (permalink / raw)
  To: pve-devel

from qemu-server, for re-use in pve-container.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    requires versioned breaks on old qemu-server containing the option, to avoid
    registering twice

 src/PVE/JSONSchema.pm | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
index 65055e0..ec53428 100644
--- a/src/PVE/JSONSchema.pm
+++ b/src/PVE/JSONSchema.pm
@@ -317,6 +317,13 @@ my $verify_idpair = sub {
     return $input;
 };
 
+PVE::JSONSchema::register_standard_option('pve-targetstorage', {
+    description => "Mapping from source to target storages. Providing only a single storage ID maps all source storages to that storage. Providing the special value '1' will map each source storage to itself.",
+    type => 'string',
+    format => 'storage-pair-list',
+    optional => 1,
+});
+
 # note: this only checks a single list entry
 # when using a storage-pair-list map, you need to pass the full parameter to
 # parse_idmap
-- 
2.30.2





^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [pve-devel] [PATCH qemu-server follow-up] schema: move 'pve-targetstorage' to pve-common
  2022-02-11 10:38 ` [pve-devel] [PATCH qemu-server follow-up] schema: move 'pve-targetstorage' to pve-common Fabian Grünbichler
  2022-02-11 10:38   ` [pve-devel] [PATCH common follow-up] schema: take over 'pve-targetstorage' option Fabian Grünbichler
@ 2022-02-11 11:31   ` Fabian Ebner
  1 sibling, 0 replies; 35+ messages in thread
From: Fabian Ebner @ 2022-02-11 11:31 UTC (permalink / raw)
  To: pve-devel, Fabian Grünbichler

Am 11.02.22 um 11:38 schrieb Fabian Grünbichler:
> for proper re-use in pve-container.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> 
> Notes:
>     requires dependency on bumped libpve-common-perl
>

Tricky to build, but FWIW, both patches:

Reviewed-by: Fabian Ebner <f.ebner@proxmox.com>

>  PVE/QemuServer.pm | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index e703cee9..2228fe87 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -119,13 +119,6 @@ PVE::JSONSchema::register_standard_option('pve-qemu-machine', {
>  	optional => 1,
>  });
>  
> -PVE::JSONSchema::register_standard_option('pve-targetstorage', {
> -    description => "Mapping from source to target storages. Providing only a single storage ID maps all source storages to that storage. Providing the special value '1' will map each source storage to itself.",
> -    type => 'string',
> -    format => 'storage-pair-list',
> -    optional => 1,
> -});
> -
>  #no warnings 'redefine';
>  
>  my $nodename_cache;




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [pve-devel] [PATCH v5 qemu-server 07/11] mtunnel: add API endpoints
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 07/11] mtunnel: add API endpoints Fabian Grünbichler
@ 2022-02-11 13:01   ` Fabian Ebner
       [not found]     ` <<0b8626f8-df25-05a6-3db3-698591688eab@proxmox.com>
  0 siblings, 1 reply; 35+ messages in thread
From: Fabian Ebner @ 2022-02-11 13:01 UTC (permalink / raw)
  To: pve-devel, Fabian Grünbichler

Am 09.02.22 um 14:07 schrieb Fabian Grünbichler:
> +	PVE::Cluster::check_cfs_quorum();
> +
> +	my $socket_addr = "/run/qemu-server/$vmid.mtunnel";

Nit: since there is another variable with the same name inside $realcmd
below, and this one is not used until the end, it could be moved further
down.

> +
> +	my $lock = 'create';
> +	eval { PVE::QemuConfig->create_and_lock_config($vmid, 0, $lock); };
> +
> +	raise_param_exc({ vmid => "unable to create empty VM config - $@"})
> +	    if $@;
> +
> +	my $realcmd = sub {

----8<----

> +		'config' => sub {
> +		    my ($params) = @_;
> +

----8<----

> +		    # not handled by update_vm_api
> +		    my $vmgenid = delete $new_conf->{vmgenid};
> +		    my $meta = delete $new_conf->{meta};
> +
> +		    $new_conf->{vmid} = $state->{vmid};
> +		    $new_conf->{node} = $node;
> +
> +		    PVE::QemuConfig->remove_lock($state->{vmid}, 'create');
> +
> +		    eval {
> +			$update_vm_api->($new_conf, 1);
> +		    };
> +		    if (my $err = $@) {
> +			# revert to locked previous config
> +			my $conf = PVE::QemuConfig->load_config($state->{vmid});
> +			$conf->{lock} = 'create';
> +			PVE::QemuConfig->write_config($state->{vmid}, $conf);
> +
> +			die $err;
> +		    }
> +
> +		    my $conf = PVE::QemuConfig->load_config($state->{vmid});
> +		    $conf->{lock} = 'migrate';
> +		    $conf->{vmgenid} = $vmgenid if $vmgenid;
> +		    $conf->{meta} = $meta if $meta;

Nit: shouldn't matter for these two, but 'if defined(...)' feels safer

> +		    PVE::QemuConfig->write_config($state->{vmid}, $conf);
> +
> +		    $state->{lock} = 'migrate';
> +
> +		    return;
> +		},

----8<----

> +		'resume' => sub {
> +		    if (PVE::QemuServer::check_running($state->{vmid}, 1)) {

Nit: use PVE::QemuServer::Helpers::vm_running_locally instead, as the
comment above PVE::QemuServer::check_running suggests

> +			PVE::QemuServer::vm_resume($state->{vmid}, 1, 1);
> +		    } else {
> +			die "VM $state->{vmid} not running\n";
> +		    }
> +		    return;
> +		},




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [pve-devel] [PATCH v5 qemu-server 08/11] migrate: refactor remote VM/tunnel start
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 08/11] migrate: refactor remote VM/tunnel start Fabian Grünbichler
@ 2022-02-11 13:01   ` Fabian Ebner
       [not found]     ` <<ce49d9a8-03b6-01ed-ad01-5cc500bfba19@proxmox.com>
  0 siblings, 1 reply; 35+ messages in thread
From: Fabian Ebner @ 2022-02-11 13:01 UTC (permalink / raw)
  To: pve-devel, Fabian Grünbichler

Am 09.02.22 um 14:07 schrieb Fabian Grünbichler:
> no semantic changes intended, except for:
> - no longer passing the main migration UNIX socket to SSH twice for
> forwarding
> - dropping the 'unix:' prefix in start_remote_tunnel's timeout error message
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>  PVE/QemuMigrate.pm | 158 ++++++++++++++++++++++++++++-----------------
>  PVE/QemuServer.pm  |  34 +++++-----
>  2 files changed, 113 insertions(+), 79 deletions(-)
> 
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index 104e62ce..e6cb7e79 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -43,19 +43,24 @@ sub fork_tunnel {
>      return PVE::Tunnel::fork_ssh_tunnel($self->{rem_ssh}, $cmd, $ssh_forward_info, $log);
>  }
>  
> +# tunnel_info:
> +#   proto: unix (secure) or tcp (insecure/legacy compat)
> +#   addr: IP or UNIX socket path
> +#   port: optional TCP port
> +#   unix_sockets: additional UNIX socket paths to forward
>  sub start_remote_tunnel {
> -    my ($self, $raddr, $rport, $ruri, $unix_socket_info) = @_;
> +    my ($self, $tunnel_info) = @_;
>  
>      my $nodename = PVE::INotify::nodename();
>      my $migration_type = $self->{opts}->{migration_type};
>  
>      if ($migration_type eq 'secure') {
>  
> -	if ($ruri =~ /^unix:/) {
> -	    my $ssh_forward_info = ["$raddr:$raddr"];
> -	    $unix_socket_info->{$raddr} = 1;
> +	if ($tunnel_info->{proto} eq 'unix') {
> +	    my $ssh_forward_info = [];
>  
> -	    my $unix_sockets = [ keys %$unix_socket_info ];
> +	    my $unix_sockets = [ keys %{$tunnel_info->{unix_sockets}} ];
> +	    push @$unix_sockets, $tunnel_info->{addr};
>  	    for my $sock (@$unix_sockets) {
>  		push @$ssh_forward_info, "$sock:$sock";
>  		unlink $sock;
> @@ -82,23 +87,23 @@ sub start_remote_tunnel {
>  	    if ($unix_socket_try > 100) {
>  		$self->{errors} = 1;
>  		PVE::Tunnel::finish_tunnel($self->{tunnel});
> -		die "Timeout, migration socket $ruri did not get ready";
> +		die "Timeout, migration socket $tunnel_info->{addr} did not get ready";
>  	    }
>  	    $self->{tunnel}->{unix_sockets} = $unix_sockets if (@$unix_sockets);
>  
> -	} elsif ($ruri =~ /^tcp:/) {
> +	} elsif ($tunnel_info->{proto} eq 'tcp') {
>  	    my $ssh_forward_info = [];
> -	    if ($raddr eq "localhost") {
> +	    if ($tunnel_info->{addr} eq "localhost") {
>  		# for backwards compatibility with older qemu-server versions
>  		my $pfamily = PVE::Tools::get_host_address_family($nodename);
>  		my $lport = PVE::Tools::next_migrate_port($pfamily);
> -		push @$ssh_forward_info, "$lport:localhost:$rport";
> +		push @$ssh_forward_info, "$lport:localhost:$tunnel_info->{rport}";

Should be $tunnel_info->{port}

>  	    }
>  
>  	    $self->{tunnel} = $self->fork_tunnel($ssh_forward_info);
>  
>  	} else {
> -	    die "unsupported protocol in migration URI: $ruri\n";
> +	    die "unsupported protocol in migration URI: $tunnel_info->{proto}\n";
>  	}
>      } else {
>  	#fork tunnel for insecure migration, to send faster commands like resume
> @@ -650,52 +655,40 @@ sub phase1_cleanup {
>      }
>  }
>  
> -sub phase2 {
> -    my ($self, $vmid) = @_;
> +sub phase2_start_local_cluster {
> +    my ($self, $vmid, $params) = @_;
>  
>      my $conf = $self->{vmconf};
>      my $local_volumes = $self->{local_volumes};
>      my @online_local_volumes = $self->filter_local_volumes('online');
>  
>      $self->{storage_migration} = 1 if scalar(@online_local_volumes);
> +    my $start = $params->{start_params};
> +    my $migrate = $params->{migrate_opts};
>  
>      $self->log('info', "starting VM $vmid on remote node '$self->{node}'");
>  
> -    my $raddr;
> -    my $rport;
> -    my $ruri; # the whole migration dst. URI (protocol:address[:port])
> -    my $nodename = PVE::INotify::nodename();
> +    my $tunnel_info = {};
>  
>      ## start on remote node
>      my $cmd = [@{$self->{rem_ssh}}];
>  
> -    my $spice_ticket;
> -    if (PVE::QemuServer::vga_conf_has_spice($conf->{vga})) {
> -	my $res = mon_cmd($vmid, 'query-spice');
> -	$spice_ticket = $res->{ticket};
> -    }
> +    push @$cmd, 'qm', 'start', $vmid, '--skiplock';

Nit: the parameter $start->{skiplock} that's passed in is ignored
(although it is always 1 currently)

> +    push @$cmd, '--migratedfrom', $migrate->{migratedfrom};
>  
> -    push @$cmd , 'qm', 'start', $vmid, '--skiplock', '--migratedfrom', $nodename;
> +    push @$cmd, '--migration_type', $migrate->{type};
>  
> -    my $migration_type = $self->{opts}->{migration_type};
> +    push @$cmd, '--migration_network', $migrate->{network}
> +      if $migrate->{network};
>  
> -    push @$cmd, '--migration_type', $migration_type;
> +    push @$cmd, '--stateuri', $start->{statefile};
>  
> -    push @$cmd, '--migration_network', $self->{opts}->{migration_network}
> -      if $self->{opts}->{migration_network};
> -
> -    if ($migration_type eq 'insecure') {
> -	push @$cmd, '--stateuri', 'tcp';
> -    } else {
> -	push @$cmd, '--stateuri', 'unix';
> +    if ($start->{forcemachine}) {
> +	push @$cmd, '--machine', $start->{forcemachine};
>      }
>  
> -    if ($self->{forcemachine}) {
> -	push @$cmd, '--machine', $self->{forcemachine};
> -    }
> -
> -    if ($self->{forcecpu}) {
> -	push @$cmd, '--force-cpu', $self->{forcecpu};
> +    if ($start->{forcecpu}) {
> +	push @$cmd, '--force-cpu', $start->{forcecpu};
>      }
>  
>      if ($self->{storage_migration}) {




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [pve-devel] [PATCH v5 qemu-server 10/11] api: add remote migrate endpoint
  2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 10/11] api: add remote migrate endpoint Fabian Grünbichler
@ 2022-02-11 13:01   ` Fabian Ebner
       [not found]     ` <<e5069cdd-7a84-9664-2dea-1ac3e68e339c@proxmox.com>
  0 siblings, 1 reply; 35+ messages in thread
From: Fabian Ebner @ 2022-02-11 13:01 UTC (permalink / raw)
  To: pve-devel, Fabian Grünbichler

Am 09.02.22 um 14:07 schrieb Fabian Grünbichler:
> @@ -4016,6 +4016,206 @@ __PACKAGE__->register_method({
>  
>      }});
>  
> +__PACKAGE__->register_method({
> +    name => 'remote_migrate_vm',
> +    path => '{vmid}/remote_migrate',
> +    method => 'POST',
> +    protected => 1,
> +    proxyto => 'node',
> +    description => "Migrate virtual machine to a remote cluster. Creates a new migration task.",
> +    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-vmid' => get_standard_option('pve-vmid', { optional => 1 }),
> +	    'target-endpoint' => get_standard_option('proxmox-remote', {
> +		description => "Remote target endpoint",
> +	    }),
> +	    online => {
> +		type => 'boolean',
> +		description => "Use online/live migration if VM is running. Ignored if VM is stopped.",
> +		optional => 1,
> +	    },
> +	    'with-local-disks' => {
> +		type => 'boolean',
> +		description => "Enable live storage migration for local disk",
> +		optional => 1,
> +	    },

Shouldn't this simply always be true, rather than an optional parameter?

> +	    delete => {
> +		type => 'boolean',
> +		description => "Delete the original VM and related data after successful migration. By default the original VM is kept on the source cluster in a stopped state.",
> +		optional => 1,
> +		default => 0,
> +	    },
> +            'target-storage' => get_standard_option('pve-targetstorage', {
> +		completion => \&PVE::QemuServer::complete_migration_storage,
> +		optional => 0,
> +            }),

Style nit: indentation is wrong

> +	    'target-bridge' => {
> +		type => 'string',
> +		description => "Mapping from source to target bridges. Providing only a single bridge ID maps all source bridges to that bridge. Providing the special value '1' will map each source bridge to itself.",
> +		format => 'bridge-pair-list',
> +	    },

Same two comments apply to the next patch.




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [pve-devel] [PATCH-SERIES 0/21] remote migration
  2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
                   ` (22 preceding siblings ...)
  2022-02-11 10:38 ` [pve-devel] [PATCH qemu-server follow-up] schema: move 'pve-targetstorage' to pve-common Fabian Grünbichler
@ 2022-02-11 13:08 ` Fabian Ebner
  23 siblings, 0 replies; 35+ messages in thread
From: Fabian Ebner @ 2022-02-11 13:08 UTC (permalink / raw)
  To: pve-devel, Fabian Grünbichler

Am 09.02.22 um 14:07 schrieb Fabian Grünbichler:
> qemu-server:
>   move map_storage to PVE::JSONSchema::map_id
>   schema: use pve-bridge-id
>   parse_config: optional strict mode
>   update_vm: allow simultaneous setting of boot-order and dev
>   nbd alloc helper: allow passing in explicit format
>   migrate: move tunnel-helpers to pve-guest-common
>   mtunnel: add API endpoints
>   migrate: refactor remote VM/tunnel start
>   migrate: add remote migration handling
>   api: add remote migrate endpoint
>   qm: add remote-migrate command
> 

Already needs a rebase, because of conflicts in debian/control ;)

For the remaining qemu-server patches, once the $tunnel_info->{rport}
typo (and any subset of my other nits) is addressed:

Reviewed-by: Fabian Ebner <f.ebner@proxmox.com>




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [pve-devel] [PATCH v5 qemu-server 07/11] mtunnel: add API endpoints
       [not found]     ` <<0b8626f8-df25-05a6-3db3-698591688eab@proxmox.com>
@ 2022-02-16 12:57       ` Fabian Grünbichler
  0 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-16 12:57 UTC (permalink / raw)
  To: Fabian Ebner, pve-devel

On February 11, 2022 2:01 pm, Fabian Ebner wrote:
> Am 09.02.22 um 14:07 schrieb Fabian Grünbichler:
>> +	PVE::Cluster::check_cfs_quorum();
>> +
>> +	my $socket_addr = "/run/qemu-server/$vmid.mtunnel";
> 
> Nit: since there is another variable with the same name inside $realcmd
> below, and this one is not used until the end, it could be moved further
> down.

ack

> 
>> +
>> +	my $lock = 'create';
>> +	eval { PVE::QemuConfig->create_and_lock_config($vmid, 0, $lock); };
>> +
>> +	raise_param_exc({ vmid => "unable to create empty VM config - $@"})
>> +	    if $@;
>> +
>> +	my $realcmd = sub {
> 
> ----8<----
> 
>> +		'config' => sub {
>> +		    my ($params) = @_;
>> +
> 
> ----8<----
> 
>> +		    # not handled by update_vm_api
>> +		    my $vmgenid = delete $new_conf->{vmgenid};
>> +		    my $meta = delete $new_conf->{meta};
>> +
>> +		    $new_conf->{vmid} = $state->{vmid};
>> +		    $new_conf->{node} = $node;
>> +
>> +		    PVE::QemuConfig->remove_lock($state->{vmid}, 'create');
>> +
>> +		    eval {
>> +			$update_vm_api->($new_conf, 1);
>> +		    };
>> +		    if (my $err = $@) {
>> +			# revert to locked previous config
>> +			my $conf = PVE::QemuConfig->load_config($state->{vmid});
>> +			$conf->{lock} = 'create';
>> +			PVE::QemuConfig->write_config($state->{vmid}, $conf);
>> +
>> +			die $err;
>> +		    }
>> +
>> +		    my $conf = PVE::QemuConfig->load_config($state->{vmid});
>> +		    $conf->{lock} = 'migrate';
>> +		    $conf->{vmgenid} = $vmgenid if $vmgenid;
>> +		    $conf->{meta} = $meta if $meta;
> 
> Nit: shouldn't matter for these two, but 'if defined(...)' feels safer
> 
>> +		    PVE::QemuConfig->write_config($state->{vmid}, $conf);
>> +
>> +		    $state->{lock} = 'migrate';
>> +
>> +		    return;
>> +		},
> 
> ----8<----
> 
>> +		'resume' => sub {
>> +		    if (PVE::QemuServer::check_running($state->{vmid}, 1)) {
> 
> Nit: use PVE::QemuServer::Helpers::vm_running_locally instead, as the
> comment above PVE::QemuServer::check_running suggests

done. this one might be worthy of a repo-wide cleanup at some point?

> 
>> +			PVE::QemuServer::vm_resume($state->{vmid}, 1, 1);
>> +		    } else {
>> +			die "VM $state->{vmid} not running\n";
>> +		    }
>> +		    return;
>> +		},
> 




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [pve-devel] [PATCH v5 qemu-server 08/11] migrate: refactor remote VM/tunnel start
       [not found]     ` <<ce49d9a8-03b6-01ed-ad01-5cc500bfba19@proxmox.com>
@ 2022-02-16 12:58       ` Fabian Grünbichler
  0 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-16 12:58 UTC (permalink / raw)
  To: Fabian Ebner, pve-devel

On February 11, 2022 2:01 pm, Fabian Ebner wrote:
> Am 09.02.22 um 14:07 schrieb Fabian Grünbichler:
>> no semantic changes intended, except for:
>> - no longer passing the main migration UNIX socket to SSH twice for
>> forwarding
>> - dropping the 'unix:' prefix in start_remote_tunnel's timeout error message
>> 
>> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
>>
>> [..]
>>
>> @@ -82,23 +87,23 @@ sub start_remote_tunnel {
>>  	    if ($unix_socket_try > 100) {
>>  		$self->{errors} = 1;
>>  		PVE::Tunnel::finish_tunnel($self->{tunnel});
>> -		die "Timeout, migration socket $ruri did not get ready";
>> +		die "Timeout, migration socket $tunnel_info->{addr} did not get ready";
>>  	    }
>>  	    $self->{tunnel}->{unix_sockets} = $unix_sockets if (@$unix_sockets);
>>  
>> -	} elsif ($ruri =~ /^tcp:/) {
>> +	} elsif ($tunnel_info->{proto} eq 'tcp') {
>>  	    my $ssh_forward_info = [];
>> -	    if ($raddr eq "localhost") {
>> +	    if ($tunnel_info->{addr} eq "localhost") {
>>  		# for backwards compatibility with older qemu-server versions
>>  		my $pfamily = PVE::Tools::get_host_address_family($nodename);
>>  		my $lport = PVE::Tools::next_migrate_port($pfamily);
>> -		push @$ssh_forward_info, "$lport:localhost:$rport";
>> +		push @$ssh_forward_info, "$lport:localhost:$tunnel_info->{rport}";
> 
> Should be $tunnel_info->{port}

right! never triggered since AFAICT this is dead code. we switched to 
using unix sockets for SSH migration in 2016/PVE 4[0], and this was just 
the fallback for compat reasons. unless I am missing something this can 
probably just be dropped altogether/replaced with a

die "secure TCP migration not supported\n";

0: https://git.proxmox.com/?p=qemu-server.git;a=commitdiff;h=1c9d54bfd05e0d017a6e2ac5524d75466b1a4455

source node will always use unix for secure and tcp for insecure
target node will only use 'localhost' as addr for tcp + secure, which no 
PVE 5/6/7 source node will ever set

> 
>>  	    }
>>  
>>  	    $self->{tunnel} = $self->fork_tunnel($ssh_forward_info);
>>  
>>  	} else {
>> -	    die "unsupported protocol in migration URI: $ruri\n";
>> +	    die "unsupported protocol in migration URI: $tunnel_info->{proto}\n";
>>  	}
>>      } else {
>>  	#fork tunnel for insecure migration, to send faster commands like resume
>> @@ -650,52 +655,40 @@ sub phase1_cleanup {
>>      }
>>  }
>>  
>> -sub phase2 {
>> -    my ($self, $vmid) = @_;
>> +sub phase2_start_local_cluster {
>> +    my ($self, $vmid, $params) = @_;
>>  
>>      my $conf = $self->{vmconf};
>>      my $local_volumes = $self->{local_volumes};
>>      my @online_local_volumes = $self->filter_local_volumes('online');
>>  
>>      $self->{storage_migration} = 1 if scalar(@online_local_volumes);
>> +    my $start = $params->{start_params};
>> +    my $migrate = $params->{migrate_opts};
>>  
>>      $self->log('info', "starting VM $vmid on remote node '$self->{node}'");
>>  
>> -    my $raddr;
>> -    my $rport;
>> -    my $ruri; # the whole migration dst. URI (protocol:address[:port])
>> -    my $nodename = PVE::INotify::nodename();
>> +    my $tunnel_info = {};
>>  
>>      ## start on remote node
>>      my $cmd = [@{$self->{rem_ssh}}];
>>  
>> -    my $spice_ticket;
>> -    if (PVE::QemuServer::vga_conf_has_spice($conf->{vga})) {
>> -	my $res = mon_cmd($vmid, 'query-spice');
>> -	$spice_ticket = $res->{ticket};
>> -    }
>> +    push @$cmd, 'qm', 'start', $vmid, '--skiplock';
> 
> Nit: the parameter $start->{skiplock} that's passed in is ignored
> (although it is always 1 currently)

fixed.

> 
>> +    push @$cmd, '--migratedfrom', $migrate->{migratedfrom};
>>  
>> -    push @$cmd , 'qm', 'start', $vmid, '--skiplock', '--migratedfrom', $nodename;
>> +    push @$cmd, '--migration_type', $migrate->{type};
>>  
>> -    my $migration_type = $self->{opts}->{migration_type};
>> +    push @$cmd, '--migration_network', $migrate->{network}
>> +      if $migrate->{network};
>>  
>> -    push @$cmd, '--migration_type', $migration_type;
>> +    push @$cmd, '--stateuri', $start->{statefile};
>>  
>> -    push @$cmd, '--migration_network', $self->{opts}->{migration_network}
>> -      if $self->{opts}->{migration_network};
>> -
>> -    if ($migration_type eq 'insecure') {
>> -	push @$cmd, '--stateuri', 'tcp';
>> -    } else {
>> -	push @$cmd, '--stateuri', 'unix';
>> +    if ($start->{forcemachine}) {
>> +	push @$cmd, '--machine', $start->{forcemachine};
>>      }
>>  
>> -    if ($self->{forcemachine}) {
>> -	push @$cmd, '--machine', $self->{forcemachine};
>> -    }
>> -
>> -    if ($self->{forcecpu}) {
>> -	push @$cmd, '--force-cpu', $self->{forcecpu};
>> +    if ($start->{forcecpu}) {
>> +	push @$cmd, '--force-cpu', $start->{forcecpu};
>>      }
>>  
>>      if ($self->{storage_migration}) {
> 




^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [pve-devel] [PATCH v5 qemu-server 10/11] api: add remote migrate endpoint
       [not found]     ` <<e5069cdd-7a84-9664-2dea-1ac3e68e339c@proxmox.com>
@ 2022-02-16 12:58       ` Fabian Grünbichler
  0 siblings, 0 replies; 35+ messages in thread
From: Fabian Grünbichler @ 2022-02-16 12:58 UTC (permalink / raw)
  To: Fabian Ebner, pve-devel

On February 11, 2022 2:01 pm, Fabian Ebner wrote:
> Am 09.02.22 um 14:07 schrieb Fabian Grünbichler:
>> @@ -4016,6 +4016,206 @@ __PACKAGE__->register_method({
>>  
>>      }});
>>  
>> +__PACKAGE__->register_method({
>> +    name => 'remote_migrate_vm',
>> +    path => '{vmid}/remote_migrate',
>> +    method => 'POST',
>> +    protected => 1,
>> +    proxyto => 'node',
>> +    description => "Migrate virtual machine to a remote cluster. Creates a new migration task.",
>> +    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-vmid' => get_standard_option('pve-vmid', { optional => 1 }),
>> +	    'target-endpoint' => get_standard_option('proxmox-remote', {
>> +		description => "Remote target endpoint",
>> +	    }),
>> +	    online => {
>> +		type => 'boolean',
>> +		description => "Use online/live migration if VM is running. Ignored if VM is stopped.",
>> +		optional => 1,
>> +	    },
>> +	    'with-local-disks' => {
>> +		type => 'boolean',
>> +		description => "Enable live storage migration for local disk",
>> +		optional => 1,
>> +	    },
> 
> Shouldn't this simply always be true, rather than an optional parameter?

depends on what our plans for cross-cluster shared storages are ;) but 
yeah, can be left out for now - this is still experimental anyway (also 
added a marker for that for the next revision), so if we re-introduce it 
it's a fairly small breaking change anyway, or we could default to on so 
that only those rare setups with actually shared storages opt out.

> 
>> +	    delete => {
>> +		type => 'boolean',
>> +		description => "Delete the original VM and related data after successful migration. By default the original VM is kept on the source cluster in a stopped state.",
>> +		optional => 1,
>> +		default => 0,
>> +	    },
>> +            'target-storage' => get_standard_option('pve-targetstorage', {
>> +		completion => \&PVE::QemuServer::complete_migration_storage,
>> +		optional => 0,
>> +            }),
> 
> Style nit: indentation is wrong
> 
>> +	    'target-bridge' => {
>> +		type => 'string',
>> +		description => "Mapping from source to target bridges. Providing only a single bridge ID maps all source bridges to that bridge. Providing the special value '1' will map each source bridge to itself.",
>> +		format => 'bridge-pair-list',
>> +	    },
> 
> Same two comments apply to the next patch.
> 




^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2022-02-16 12:58 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-09 13:07 [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 common 1/1] add 'map_id' helper for ID maps Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 container 1/3] fix #1532: add target-storage support to migration Fabian Grünbichler
2022-02-10 11:52   ` Fabian Ebner
2022-02-11  8:33     ` Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 container 2/3] config: add strict parser Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH PoC v5 container 3/3] migration: add remote migration Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 guest-common 1/3] migrate: add get_bwlimit helper Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 guest-common 2/3] add tunnel helper module Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 guest-common 3/3] add storage tunnel module Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 01/11] move map_storage to PVE::JSONSchema::map_id Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 02/11] schema: use pve-bridge-id Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 03/11] parse_config: optional strict mode Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 04/11] update_vm: allow simultaneous setting of boot-order and dev Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 05/11] nbd alloc helper: allow passing in explicit format Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 06/11] migrate: move tunnel-helpers to pve-guest-common Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 07/11] mtunnel: add API endpoints Fabian Grünbichler
2022-02-11 13:01   ` Fabian Ebner
     [not found]     ` <<0b8626f8-df25-05a6-3db3-698591688eab@proxmox.com>
2022-02-16 12:57       ` Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 08/11] migrate: refactor remote VM/tunnel start Fabian Grünbichler
2022-02-11 13:01   ` Fabian Ebner
     [not found]     ` <<ce49d9a8-03b6-01ed-ad01-5cc500bfba19@proxmox.com>
2022-02-16 12:58       ` Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 09/11] migrate: add remote migration handling Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 10/11] api: add remote migrate endpoint Fabian Grünbichler
2022-02-11 13:01   ` Fabian Ebner
     [not found]     ` <<e5069cdd-7a84-9664-2dea-1ac3e68e339c@proxmox.com>
2022-02-16 12:58       ` Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 qemu-server 11/11] qm: add remote-migrate command Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 storage 1/3] storage_migrate_snapshot: skip for btrfs without snapshots Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 storage 2/3] storage_migrate: pull out import/export_prepare Fabian Grünbichler
2022-02-09 13:07 ` [pve-devel] [PATCH v5 storage 3/3] add volume_import/export_start helpers Fabian Grünbichler
2022-02-09 17:56 ` [pve-devel] [PATCH-SERIES 0/21] remote migration Thomas Lamprecht
2022-02-11 10:38 ` [pve-devel] [PATCH qemu-server follow-up] schema: move 'pve-targetstorage' to pve-common Fabian Grünbichler
2022-02-11 10:38   ` [pve-devel] [PATCH common follow-up] schema: take over 'pve-targetstorage' option Fabian Grünbichler
2022-02-11 11:31   ` [pve-devel] [PATCH qemu-server follow-up] schema: move 'pve-targetstorage' to pve-common Fabian Ebner
2022-02-11 13:08 ` [pve-devel] [PATCH-SERIES 0/21] remote migration Fabian Ebner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal