[pve-devel] [PATCH qemu-server master+stable-bookworm] migration: check if target supports nets-host-mtu parameter up front

[pve-devel] [PATCH qemu-server master+stable-bookworm] migration: check if target supports nets-host-mtu parameter up front

[Fiona Ebner]

Avoid blocking backwards migration to nodes that do not support the
new 'nets-host-mtu' parameter yet for local cluster migrations.

Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuMigrate.pm | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm
index 4381b542..f01c7907 100644
--- a/src/PVE/QemuMigrate.pm
+++ b/src/PVE/QemuMigrate.pm
@@ -958,6 +958,18 @@ sub phase1_cleanup {
     }
 }
 
+my sub target_supports_nets_host_mtu {
+    my ($self, $forcemachine) = @_;
+
+    return 1 if PVE::QemuServer::Machine::is_machine_version_at_least($forcemachine, 10, 0, 1);
+
+    my $cmd = [$self->{rem_ssh}->@*, 'qm', 'start', 0, '--nets-host-mtu'];
+
+    my $err = '';
+    eval { PVE::Tools::run_command($cmd, outfunc => sub { }, errfunc => sub { $err .= shift }) };
+    return $err =~ m/^Unknown option: nets-host-mtu/ ? 0 : 1;
+}
+
 sub phase2_start_local_cluster {
     my ($self, $vmid, $params) = @_;
 
@@ -998,7 +1010,7 @@ sub phase2_start_local_cluster {
         push @$cmd, '--force-cpu', $start->{forcecpu};
     }
 
-    if ($start->{'nets-host-mtu'}) {
+    if ($start->{'nets-host-mtu'} && target_supports_nets_host_mtu($self, $start->{forcemachine})) {
         push @$cmd, '--nets-host-mtu', $start->{'nets-host-mtu'};
     }
 
-- 
2.47.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu-server master+stable-bookworm] migration: check if target supports nets-host-mtu parameter up front

[Thomas Lamprecht]

Am 08.09.25 um 15:48 schrieb Fiona Ebner:
> Avoid blocking backwards migration to nodes that do not support the
> new 'nets-host-mtu' parameter yet for local cluster migrations.0

Please add a short rationale about the approach used to detect that.

Further, for 8.X <-> 8.Y this is now fine, but for 8.X to 9.Y (with 9.Y
being to old to know about the new parameter) it still can cause a VM
crash, or?

> Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  src/PVE/QemuMigrate.pm | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm
> index 4381b542..f01c7907 100644
> --- a/src/PVE/QemuMigrate.pm
> +++ b/src/PVE/QemuMigrate.pm
> @@ -958,6 +958,18 @@ sub phase1_cleanup {
>      }
>  }
>  
> +my sub target_supports_nets_host_mtu {
> +    my ($self, $forcemachine) = @_;
> +
> +    return 1 if PVE::QemuServer::Machine::is_machine_version_at_least($forcemachine, 10, 0, 1);
> +
> +    my $cmd = [$self->{rem_ssh}->@*, 'qm', 'start', 0, '--nets-host-mtu'];

Could be nice to have a comment that this depends on the unknown parameter
error getting checked earlier compared to the fixed 0 parameter not being a
valid VMID.

> +
> +    my $err = '';
> +    eval { PVE::Tools::run_command($cmd, outfunc => sub { }, errfunc => sub { $err .= shift }) };
> +    return $err =~ m/^Unknown option: nets-host-mtu/ ? 0 : 1;
> +}
> +
>  sub phase2_start_local_cluster {
>      my ($self, $vmid, $params) = @_;
>  
> @@ -998,7 +1010,7 @@ sub phase2_start_local_cluster {
>          push @$cmd, '--force-cpu', $start->{forcecpu};
>      }
>  
> -    if ($start->{'nets-host-mtu'}) {
> +    if ($start->{'nets-host-mtu'} && target_supports_nets_host_mtu($self, $start->{forcemachine})) {
>          push @$cmd, '--nets-host-mtu', $start->{'nets-host-mtu'};
>      }
>  



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH qemu-server master+stable-bookworm] migration: check if target supports nets-host-mtu parameter up front

[Fiona Ebner]

Am 08.09.25 um 4:49 PM schrieb Thomas Lamprecht:
> Am 08.09.25 um 15:48 schrieb Fiona Ebner:
>> Avoid blocking backwards migration to nodes that do not support the
>> new 'nets-host-mtu' parameter yet for local cluster migrations.0
> 
> Please add a short rationale about the approach used to detect that.
> 
> Further, for 8.X <-> 8.Y this is now fine, but for 8.X to 9.Y (with 9.Y
> being to old to know about the new parameter) it still can cause a VM
> crash, or?

Yeah. So for a 9.x source or 9.x target, we might not even want to avoid
the parameter at all, but just match the error for the real command and
tell users to upgrade like you initially suggested. Because most
migrations will have a NIC with MTU set to "inherit from bridge" and
thus be potentially problematic if not passing 'nets-host-mtu'. I mean,
we could still avoid setting the parameter if all MTUs are set
explicitly to a value, but it doesn't seem worth checking for that, as
it would reduce friction only in a very small number of cases.

In case it's a 8.x source to 8.x target migration, we could check if any
MTU is set to "inherit from bridge" and set and require the presence of
the parameter only in that case, because there, it is not the (vast)
majority of migrations.

> 
>> Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
>> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
>> ---
>>  src/PVE/QemuMigrate.pm | 14 +++++++++++++-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm
>> index 4381b542..f01c7907 100644
>> --- a/src/PVE/QemuMigrate.pm
>> +++ b/src/PVE/QemuMigrate.pm
>> @@ -958,6 +958,18 @@ sub phase1_cleanup {
>>      }
>>  }
>>  
>> +my sub target_supports_nets_host_mtu {
>> +    my ($self, $forcemachine) = @_;
>> +
>> +    return 1 if PVE::QemuServer::Machine::is_machine_version_at_least($forcemachine, 10, 0, 1);
>> +
>> +    my $cmd = [$self->{rem_ssh}->@*, 'qm', 'start', 0, '--nets-host-mtu'];
> 
> Could be nice to have a comment that this depends on the unknown parameter
> error getting checked earlier compared to the fixed 0 parameter not being a
> valid VMID.
> 
>> +
>> +    my $err = '';
>> +    eval { PVE::Tools::run_command($cmd, outfunc => sub { }, errfunc => sub { $err .= shift }) };
>> +    return $err =~ m/^Unknown option: nets-host-mtu/ ? 0 : 1;
>> +}
>> +
>>  sub phase2_start_local_cluster {
>>      my ($self, $vmid, $params) = @_;
>>  
>> @@ -998,7 +1010,7 @@ sub phase2_start_local_cluster {
>>          push @$cmd, '--force-cpu', $start->{forcecpu};
>>      }
>>  
>> -    if ($start->{'nets-host-mtu'}) {
>> +    if ($start->{'nets-host-mtu'} && target_supports_nets_host_mtu($self, $start->{forcemachine})) {
>>          push @$cmd, '--nets-host-mtu', $start->{'nets-host-mtu'};
>>      }
>>  
> 



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel