[PATCH qemu-server v3 17/21] migration: intra-cluster: check config can be parsed on target node

[Fiona Ebner <f.ebner@proxmox.com>]

For remote migration, we already check that the config can be parsed
on the target. Do the same for intra-cluster migration, to avoid
issues like [0] for future new settings, with lines being unexpectedly
and relatively silently dropped (there are warnings in the target's
system logs, but nothing else).

Unfortunately, before commit "qm: mtunnel: reply when a command is
unknown", which is part of the same patch series, when a command is
unknown, mtunnel did not reply at all. Therefore, check that the
target node is recent enough to support the new mtunnel command.

[0]: https://bugzilla.proxmox.com/show_bug.cgi?id=7341

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

The node version might need to be adapted when applying!

Changes in v3:
* add skip-config-check flag rather than re-using force.
* check node version to decide if too old or not.

 src/PVE/API2/Qemu.pm                      | 10 ++++++++++
 src/PVE/QemuMigrate.pm                    | 24 +++++++++++++++++++++++
 src/test/MigrationTest/QemuMigrateMock.pm | 11 ++++++++++-
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index 56b6c500..937f8cae 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -138,6 +138,15 @@ my $migrate_json_properties = {
         default => 0,
         description => 'Whether to migrate conntrack entries for running VMs.',
     },
+    'skip-config-check' => {
+        type => 'boolean',
+        optional => 1,
+        default => 0,
+        description =>
+            'For intra-cluster migration. Skip checking if the configuration can be parsed'
+            . ' successfully by the target. Using this flag can lead to dropped'
+            . ' configuration options if the target is too old to parse them.',
+    },
 };
 
 # Properties that will be forwarded via the HA stack to the LRM, which will then use them for its
@@ -145,6 +154,7 @@ my $migrate_json_properties = {
 # HA migrations with many being hard-coded for the LRM invocation.
 my $ha_migrate_props = {
     'with-conntrack-state' => 1,
+    'skip-config-check' => 1,
 };
 
 sub ha_migrate_json_properties {
diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm
index 080490f0..fff57eab 100644
--- a/src/PVE/QemuMigrate.pm
+++ b/src/PVE/QemuMigrate.pm
@@ -359,6 +359,30 @@ sub prepare {
         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}->{'skip-config-check'}) {
+            if (PVE::Cluster::node_pvecfg_version_at_least($self->{node}, 9, 2, 21)) {
+                # Fork a short-lived tunnel for checking the config. Later, the proper tunnel with
+                # SSH forwarding info is forked.
+                my $tunnel = $self->fork_tunnel();
+                # Compared to remote migration, which also does volume activation, this only
+                # strictly parses the config, so no too large timeout is needed.
+                eval {
+                    my $nodename = PVE::INotify::nodename();
+                    PVE::Tunnel::write_tunnel($tunnel, 30, "config $vmid $nodename");
+                };
+                my $err = $@;
+
+                eval { PVE::Tunnel::finish_tunnel($tunnel); };
+                $self->log('warn', "failed to finish tunnel in prepare() - $@") if $@;
+
+                die "$err - use 'skip-config-check' flag to migrate regardless\n" if $err;
+            } else {
+                $self->log('info', "skipping config check - target pve-manager version < 9.2.21");
+            }
+        } else {
+            $self->log('info', "skipping config check - override option is set");
+        }
     }
 
     return $running;
diff --git a/src/test/MigrationTest/QemuMigrateMock.pm b/src/test/MigrationTest/QemuMigrateMock.pm
index 3a483817..ac198acb 100644
--- a/src/test/MigrationTest/QemuMigrateMock.pm
+++ b/src/test/MigrationTest/QemuMigrateMock.pm
@@ -65,6 +65,10 @@ $tunnel_module->mock(
             my $vmid = $1;
             die "resuming wrong VM '$vmid'\n" if $vmid ne $test_vmid;
             return;
+        } elsif ($command =~ m/^config (\d+) (\S+)$/) {
+            my ($vmid, $node) = ($1, $2);
+            die "check config for wrong VM '$vmid'\n" if $vmid ne $test_vmid;
+            return;
         }
         die "write_tunnel (mocked) - implement me: $command\n";
     },
@@ -73,7 +77,12 @@ $tunnel_module->mock(
 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
+        return {
+            writer => "mocked",
+            reader => "mocked",
+            pid => 123456,
+            version => 1,
+        };
     },
     start_remote_tunnel => sub {
         my ($self, $raddr, $rport, $ruri, $unix_socket_info) = @_;
-- 
2.47.3