Re: [PATCH qemu-server v2 06/14] migration: intra-cluster: check config can be parsed on target node

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On February 25, 2026 4:18 pm, Fiona Ebner wrote:
> 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).
> 
> 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, this delays
> backwards migrations to qemu-server versions less than the next bumped
> version (at the time of this writing expected to be 9.1.5) by 3
> seconds.
> 
> [0]: https://bugzilla.proxmox.com/show_bug.cgi?id=7341
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> 
> Changes in v2:
> * log when skipping config check
> 
> For easier testing:
> 
> # commented out so 'git am' does not apply it O;)
> # diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
> # index 545758dc..b914314e 100644
> # --- a/src/PVE/QemuServer.pm
> # +++ b/src/PVE/QemuServer.pm
> # @@ -236,6 +236,13 @@ my $spice_enhancements_fmt = {
> # };
> # 
> # my $confdesc = {
> #+    'shiny-new' => {
> #+        type => 'string',
> #+        enum => ['shiny', 'new'],
> #+        default => 'shiny',
> #+        optional => 1,
> #+        description => "you know you want it",
> #+    },
> #     onboot => {
> #         optional => 1,
> #         type => 'boolean',
> 
>  src/PVE/API2/Qemu.pm                      |  4 +++-
>  src/PVE/QemuMigrate.pm                    | 27 +++++++++++++++++++++++
>  src/test/MigrationTest/QemuMigrateMock.pm | 11 ++++++++-
>  3 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
> index 1f0864f5..47466513 100644
> --- a/src/PVE/API2/Qemu.pm
> +++ b/src/PVE/API2/Qemu.pm
> @@ -5399,7 +5399,9 @@ __PACKAGE__->register_method({
>              force => {
>                  type => 'boolean',
>                  description =>
> -                    "Allow to migrate VMs which use local devices. Only root may use this option.",
> +                    "Allow to migrate VMs which use local devices and for intra-cluster migration,"
> +                    . " configuration options not understood by the target. Only root may use this"
> +                    . " option.",
>                  optional => 1,
>              },
>              migration_type => {
> diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm
> index f7ec3227..901fe96d 100644
> --- a/src/PVE/QemuMigrate.pm
> +++ b/src/PVE/QemuMigrate.pm
> @@ -355,6 +355,33 @@ 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}->{force}) {
> +            # Fork a short-lived tunnel for checking the config. Later, the proper tunnel with SSH
> +            # forwaring info is forked.

typo: forwarding

> +            my $tunnel = $self->fork_tunnel();
> +            # Compared to remote migration, which also does volume activation, this only strictly
> +            # parses the config, so no large timeout is needed. Unfortunately, mtunnel did not
> +            # indicate that a command is unknown, but not reply at all, so the timeout must be very
> +            # low right now.
> +            # FIXME PVE 10 - bump timeout, the trade-off between delaying backwards migration and
> +            # giving config check more time should now be in favor of config checking
> +            eval {
> +                my $nodename = PVE::INotify::nodename();
> +                PVE::Tunnel::write_tunnel($tunnel, 3, "config $vmid $nodename");
> +            };
> +            if (my $err = $@) {
> +                chomp($err);
> +                # if there is no reply, assume target did not know the command yet
> +                if ($err =~ m/^no reply to command/) {
> +                    $self->log('info', "skipping strict configuration check (target too old?)");
> +                } else {
> +                    die "$err - use --force to migrate regardless\n";

if we ever have a bug in our config handling that causes the writer to
emit something that the parser would skip, affected VMs are effectively
only migratable as root.. which in turn means that a lot of new->old
migrations would become root only? maybe it would make sense to split
this from the existing force logic to make it available for regular
users?

this also applies to the matching pve-container patch.

> +                }
> +            }
> +            eval { PVE::Tunnel::finish_tunnel($tunnel); };
> +            $self->log('warn', "failed to finish tunnel in prepare() - $@") if $@;
> +        }
>      }
>  
>      return $running;
> diff --git a/src/test/MigrationTest/QemuMigrateMock.pm b/src/test/MigrationTest/QemuMigrateMock.pm
> index df8b575a..170634de 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
> 
> 
> 
> 
> 
>