[RFC PATCH-SERIES qemu-server 0/1] fix #7053: allow setting additional HA migration parameters

[RFC PATCH-SERIES qemu-server 0/1] fix #7053: allow setting additional HA migration parameters

[Daniel Kral]

Bugzilla #7053 reports that even though 'with-conntrack-state' is
checked, the VM will always migrate without conntrack state in the end.

In fact, any parameters from the migrate_vm API endpoint but the $vmid
and $node are not passed on to the HA stack at all. This was likely
caught now, because the conntrack state is the only optional parameter
visible in the web interface and set by default.



Currently, the resource motion crm command is matched from ^ to $:

    if ($cmd =~ m/^(migrate|relocate)\s+(\S+)\s+(\S+)$/) {

We could extend that crm command to something like:

    if ($cmd =~ m/^(migrate|relocate)\s+(\S+)\s+(\S+)(?:\s+(\S.*))?$/) {

but this would need the newer `ha-manager {migrate,relocate} ...`
API/CLI endpoint to append both the standard and extended version for
some period as older HA Manager versions wouldn't be able to parse the
extended version but only the standard versions. Newer HA Manager
versions would be fine though, as first the standard version would be
parsed and afterwards the extended version would overwrite the request
from the standard version.

The downside from this though is that the migration parameters are not
the same for VMs and CTs (and possible future resource types) and would
therefore expose quite a lot of resource-specific data structures to the
more generic HA Manager code.

Additionally, both the node with the active HA Manager as well as the
node's LRM where the to-be-moved HA resource is on need to have the
newer pve-ha-manager version to correctly relay the migration
parameters.



As the migrate_vm API request is proxied to the node where the HA
resource is assigned to, this RFC patch series puts the responsibility
to handle the additional migration parameters at the caller's side,
where these are saved while the request is relayed through the HA stack
until the LRM on the node calls migrate_vm again.

The implementation is not fully fleshed out (e.g. cleaning up the
migration params file on a crashed/stopped VM or rejected migration
requests, etc.), but I wanted to get more feedback whether this solution
has any merit and if not decide on another possible solution.

If it does have merit, this could be generalized for both qemu-server
and pve-container if it useful for containers as well.


qemu-server:

Daniel Kral (1):
  fix #7053: api: migrate: save and restore migration params for HA
    managed VMs

 src/PVE/API2/Qemu.pm | 54 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)


Summary over all repositories:
  1 files changed, 54 insertions(+), 0 deletions(-)

-- 
Generated by murpp 0.9.0

[RFC qemu-server 1/1] fix #7053: api: migrate: save and restore migration params for HA managed VMs

[Daniel Kral]

If a HA-managed VM's migrate API endpoint is called from a web API or
CLI environment, it is first relayed to the HA Manager by queueing a
'migrate' CRM command with `ha-manager migrate vm:$vmid $target_node`.
This command doesn't take any additional migration parameters though.

As soon as the HA Manager reads the CRM command in the next HA Manager
round, it passes the migration request - if valid - to the HA resource
state. This migration request is then picked up by the LRM, where the HA
resource is assigned to and calls the migrate_vm API endpoint, which
will then initial the VM migration.

As the migrate_vm API request is proxied to the node, where the HA
resource is assigned to, this allows the migrate parameters to stored
locally while the migration request is passed through the HA stack.

Signed-off-by: Daniel Kral <d.kral@proxmox.com>
---
 src/PVE/API2/Qemu.pm | 54 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index 1f0864f5..027896a3 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -3,6 +3,7 @@ package PVE::API2::Qemu;
 use strict;
 use warnings;
 use Cwd 'abs_path';
+use File::stat qw();
 use Net::SSLeay;
 use IO::Socket::IP;
 use IO::Socket::UNIX;
@@ -5367,6 +5368,52 @@ __PACKAGE__->register_method({
     },
 });
 
+my sub migrate_params_filename {
+    my ($vmid) = @_;
+    return "/run/qemu-server/$vmid.migrate_params";
+}
+
+my sub save_migrate_params {
+    my ($vmid, $params) = @_;
+
+    my $migrate_params_file = migrate_params_filename($vmid);
+
+    warn "existing migration parameters file for '$vmid' will be overwritten\n"
+        if -f $migrate_params_file;
+
+    PVE::Tools::file_set_contents($migrate_params_file, encode_json($params), 0640);
+}
+
+my sub try_to_restore_migrate_params {
+    my ($vmid, $params) = @_;
+
+    my $migrate_params_file = migrate_params_filename($vmid);
+    my @migrate_params_denylist = qw(node vmid target online force with-local-disks targetstorage);
+
+    if (-f $migrate_params_file) {
+        my $stat = File::stat::lstat($migrate_params_file);
+        # prevent that non-root users could write the migrate parameters file
+        my $has_correct_perms = $stat->uid == 0 && ($stat->mode & 037) == 0;
+
+        if (PVE::HA::Config::vm_is_ha_managed($vmid) && $has_correct_perms) {
+            my $migration_params = {};
+            eval {
+                my $data = PVE::Tools::file_get_contents($migrate_params_file);
+                $migration_params = decode_json($data) // {};
+            };
+            for my $key (keys %$migration_params) {
+                next if grep { $key eq $_ } @migrate_params_denylist;
+
+                $params->{$key} = $migration_params->{$key};
+            }
+        } else {
+            warn "remove orphan migration parameters file\n";
+        }
+
+        unlink $migrate_params_file;
+    }
+}
+
 __PACKAGE__->register_method({
     name => 'migrate_vm',
     path => '{vmid}/migrate',
@@ -5464,6 +5511,13 @@ __PACKAGE__->register_method({
 
         my $vmid = extract_param($param, 'vmid');
 
+        if (PVE::HA::Config::vm_is_ha_managed($vmid) && $rpcenv->{type} ne 'ha') {
+            save_migrate_params($vmid, $param);
+        } else {
+            # always try to restore to remove oprhaned migration parameters files
+            try_to_restore_migrate_params($vmid, $param);
+        }
+
         raise_param_exc({ force => "Only root may use this option." })
             if $param->{force} && $authuser ne 'root@pam';
 
-- 
2.47.3

Re: [RFC qemu-server 1/1] fix #7053: api: migrate: save and restore migration params for HA managed VMs

[Fiona Ebner]

Am 25.02.26 um 3:35 PM schrieb Daniel Kral:
> If a HA-managed VM's migrate API endpoint is called from a web API or
> CLI environment, it is first relayed to the HA Manager by queueing a
> 'migrate' CRM command with `ha-manager migrate vm:$vmid $target_node`.
> This command doesn't take any additional migration parameters though.
> 
> As soon as the HA Manager reads the CRM command in the next HA Manager
> round, it passes the migration request - if valid - to the HA resource
> state. This migration request is then picked up by the LRM, where the HA
> resource is assigned to and calls the migrate_vm API endpoint, which
> will then initial the VM migration.

Nit: according to Wiktionary, 'initial' can't be used as a verb in this
context [0]

> 
> As the migrate_vm API request is proxied to the node, where the HA
> resource is assigned to, this allows the migrate parameters to stored
> locally while the migration request is passed through the HA stack.
> 

I'm fine with this approach :)

> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
> ---
>  src/PVE/API2/Qemu.pm | 54 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
> index 1f0864f5..027896a3 100644
> --- a/src/PVE/API2/Qemu.pm
> +++ b/src/PVE/API2/Qemu.pm
> @@ -3,6 +3,7 @@ package PVE::API2::Qemu;
>  use strict;
>  use warnings;
>  use Cwd 'abs_path';
> +use File::stat qw();
>  use Net::SSLeay;
>  use IO::Socket::IP;
>  use IO::Socket::UNIX;
> @@ -5367,6 +5368,52 @@ __PACKAGE__->register_method({
>      },
>  });
>  
> +my sub migrate_params_filename {

These helpers are closely connected to the API call, but I'm still
wondering if it's not better to put them in the
PVE::QemuMigrate::Helpers module.

> +    my ($vmid) = @_;
> +    return "/run/qemu-server/$vmid.migrate_params";
> +}
> +
> +my sub save_migrate_params {
> +    my ($vmid, $params) = @_;
> +
> +    my $migrate_params_file = migrate_params_filename($vmid);
> +
> +    warn "existing migration parameters file for '$vmid' will be overwritten\n"
> +        if -f $migrate_params_file;
> +
> +    PVE::Tools::file_set_contents($migrate_params_file, encode_json($params), 0640);

Please use PVE::File for new usages

> +}
> +
> +my sub try_to_restore_migrate_params {
> +    my ($vmid, $params) = @_;
> +
> +    my $migrate_params_file = migrate_params_filename($vmid);
> +    my @migrate_params_denylist = qw(node vmid target online force with-local-disks targetstorage);

Nit: it's rather 'skip' than 'deny' and using a hash and avoiding the
grep would be slightly simpler

> +
> +    if (-f $migrate_params_file) {
> +        my $stat = File::stat::lstat($migrate_params_file);
> +        # prevent that non-root users could write the migrate parameters file
> +        my $has_correct_perms = $stat->uid == 0 && ($stat->mode & 037) == 0;

Usually, we rely on the fact that the containing directory already has
the correct permissions to prevent unprivileged users from writing files
there and when you create the file, you also don't allow writes from
other users, so it should already be fine. If we do go for this, then
not having correct permissions should produce a dedicated warning.

> +
> +        if (PVE::HA::Config::vm_is_ha_managed($vmid) && $has_correct_perms) {

I'd prefer if we could check for the rpcenv type here too

> +            my $migration_params = {};
> +            eval {
> +                my $data = PVE::Tools::file_get_contents($migrate_params_file);

Please use PVE::File for new usages

> +                $migration_params = decode_json($data) // {};
> +            };

Missing error handling here.

> +            for my $key (keys %$migration_params) {
> +                next if grep { $key eq $_ } @migrate_params_denylist;
> +
> +                $params->{$key} = $migration_params->{$key};
> +            }
> +        } else {
> +            warn "remove orphan migration parameters file\n";
> +        }
> +
> +        unlink $migrate_params_file;

Nit: the return value could be checked while ignoring ENOENT, we have
quite a few examples of this

> +    }
> +}
> +
>  __PACKAGE__->register_method({
>      name => 'migrate_vm',
>      path => '{vmid}/migrate',
> @@ -5464,6 +5511,13 @@ __PACKAGE__->register_method({
>  
>          my $vmid = extract_param($param, 'vmid');
>  
> +        if (PVE::HA::Config::vm_is_ha_managed($vmid) && $rpcenv->{type} ne 'ha') {
> +            save_migrate_params($vmid, $param);
> +        } else {
> +            # always try to restore to remove oprhaned migration parameters files

Typo: 'oprhaned'

> +            try_to_restore_migrate_params($vmid, $param);

I'd prefer if the naming would mention that it can only have an effect
for HA, maybe restore_migrate_params_for_ha()?

Not sure if a dedicated helper for removing makes sense to better
organize it? Then we could call the restore one only if ha_managed and
rpcenv type eq 'ha'

> +        }
> +
>          raise_param_exc({ force => "Only root may use this option." })
>              if $param->{force} && $authuser ne 'root@pam';
>  

[0]: https://en.wiktionary.org/wiki/initial#Verb

Re: [RFC qemu-server 1/1] fix #7053: api: migrate: save and restore migration params for HA managed VMs

[Daniel Kral]

Thanks for the feedback, Fiona!

On Mon Mar 2, 2026 at 5:06 PM CET, Fiona Ebner wrote:
> Am 25.02.26 um 3:35 PM schrieb Daniel Kral:
>> If a HA-managed VM's migrate API endpoint is called from a web API or
>> CLI environment, it is first relayed to the HA Manager by queueing a
>> 'migrate' CRM command with `ha-manager migrate vm:$vmid $target_node`.
>> This command doesn't take any additional migration parameters though.
>> 
>> As soon as the HA Manager reads the CRM command in the next HA Manager
>> round, it passes the migration request - if valid - to the HA resource
>> state. This migration request is then picked up by the LRM, where the HA
>> resource is assigned to and calls the migrate_vm API endpoint, which
>> will then initial the VM migration.
>
> Nit: according to Wiktionary, 'initial' can't be used as a verb in this
> context [0]

Sorry, only a typo, should have been 'initiate' here ^^. Will fix that
for the v2.

>
>> 
>> As the migrate_vm API request is proxied to the node, where the HA
>> resource is assigned to, this allows the migrate parameters to stored
>> locally while the migration request is passed through the HA stack.
>> 
>
> I'm fine with this approach :)

Nice!

One design issue here I overlooked at first though is that if a HA
resource has dependent HA resources from a positive resource affinity
rule (which are shown to users either through the migration
preconditions and after initiating the migration), those will not have
the migration parameters passed down... I'm not sure yet if that is
wanted or not, as migrating them is only a side-effect and not all
parameters (e.g. `--with-conntrack-state`) are applicable to both guest
types of course.

If we want to go for passing them to the dependent HA resources, we
might as well fix this together with #6220 [0] as indexing the migration
parameters by the task rather than vmid would make it possible to store
and retrieve this information in/from a single place.

But it feels like this is more of a opt-in feature that can be added
later on rather than now as both behaviors seem valid for different use
cases.

>
>> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
>> ---
>>  src/PVE/API2/Qemu.pm | 54 ++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 54 insertions(+)
>> 
>> diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
>> index 1f0864f5..027896a3 100644
>> --- a/src/PVE/API2/Qemu.pm
>> +++ b/src/PVE/API2/Qemu.pm
>> @@ -3,6 +3,7 @@ package PVE::API2::Qemu;
>>  use strict;
>>  use warnings;
>>  use Cwd 'abs_path';
>> +use File::stat qw();
>>  use Net::SSLeay;
>>  use IO::Socket::IP;
>>  use IO::Socket::UNIX;
>> @@ -5367,6 +5368,52 @@ __PACKAGE__->register_method({
>>      },
>>  });
>>  
>> +my sub migrate_params_filename {
>
> These helpers are closely connected to the API call, but I'm still
> wondering if it's not better to put them in the
> PVE::QemuMigrate::Helpers module.

Hm, even though they are likely only used for the API calls, I'd be
happy to move them there in a v2!

>
>> +    my ($vmid) = @_;
>> +    return "/run/qemu-server/$vmid.migrate_params";
>> +}
>> +
>> +my sub save_migrate_params {
>> +    my ($vmid, $params) = @_;
>> +
>> +    my $migrate_params_file = migrate_params_filename($vmid);
>> +
>> +    warn "existing migration parameters file for '$vmid' will be overwritten\n"
>> +        if -f $migrate_params_file;
>> +
>> +    PVE::Tools::file_set_contents($migrate_params_file, encode_json($params), 0640);
>
> Please use PVE::File for new usages

ACK, will do!

>
>> +}
>> +
>> +my sub try_to_restore_migrate_params {
>> +    my ($vmid, $params) = @_;
>> +
>> +    my $migrate_params_file = migrate_params_filename($vmid);
>> +    my @migrate_params_denylist = qw(node vmid target online force with-local-disks targetstorage);
>
> Nit: it's rather 'skip' than 'deny' and using a hash and avoiding the
> grep would be slightly simpler

Thanks, that sounds good, will do that!

>
>> +
>> +    if (-f $migrate_params_file) {
>> +        my $stat = File::stat::lstat($migrate_params_file);
>> +        # prevent that non-root users could write the migrate parameters file
>> +        my $has_correct_perms = $stat->uid == 0 && ($stat->mode & 037) == 0;
>
> Usually, we rely on the fact that the containing directory already has
> the correct permissions to prevent unprivileged users from writing files
> there and when you create the file, you also don't allow writes from
> other users, so it should already be fine. If we do go for this, then
> not having correct permissions should produce a dedicated warning.

I see, then it seems alright to not check the permissions here rather
pedantically after all as that would mean an already privileged user has
tampered with a rather short-lived file.

>
>> +
>> +        if (PVE::HA::Config::vm_is_ha_managed($vmid) && $has_correct_perms) {
>
> I'd prefer if we could check for the rpcenv type here too

ACK

>
>> +            my $migration_params = {};
>> +            eval {
>> +                my $data = PVE::Tools::file_get_contents($migrate_params_file);
>
> Please use PVE::File for new usages

ACK

>
>> +                $migration_params = decode_json($data) // {};
>> +            };
>
> Missing error handling here.

ACK

>
>> +            for my $key (keys %$migration_params) {
>> +                next if grep { $key eq $_ } @migrate_params_denylist;
>> +
>> +                $params->{$key} = $migration_params->{$key};
>> +            }
>> +        } else {
>> +            warn "remove orphan migration parameters file\n";
>> +        }
>> +
>> +        unlink $migrate_params_file;
>
> Nit: the return value could be checked while ignoring ENOENT, we have
> quite a few examples of this

Good catch, I'll do that in a v2!

>
>> +    }
>> +}
>> +
>>  __PACKAGE__->register_method({
>>      name => 'migrate_vm',
>>      path => '{vmid}/migrate',
>> @@ -5464,6 +5511,13 @@ __PACKAGE__->register_method({
>>  
>>          my $vmid = extract_param($param, 'vmid');
>>  
>> +        if (PVE::HA::Config::vm_is_ha_managed($vmid) && $rpcenv->{type} ne 'ha') {
>> +            save_migrate_params($vmid, $param);
>> +        } else {
>> +            # always try to restore to remove oprhaned migration parameters files
>
> Typo: 'oprhaned'

ACK

>
>> +            try_to_restore_migrate_params($vmid, $param);
>
> I'd prefer if the naming would mention that it can only have an effect
> for HA, maybe restore_migrate_params_for_ha()?
>
> Not sure if a dedicated helper for removing makes sense to better
> organize it? Then we could call the restore one only if ha_managed and
> rpcenv type eq 'ha'

I'll clean that up in a v2!

>
>> +        }
>> +
>>          raise_param_exc({ force => "Only root may use this option." })
>>              if $param->{force} && $authuser ne 'root@pam';
>>  

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