[pve-devel] [PATCH qemu-server v2 2/4] api: vm start: introduce nets-host-mtu parameter for migration compat

[Fabian Grünbichler]

On September 4, 2025 11:28 am, Fiona Ebner wrote:
> Am 04.09.25 um 11:11 AM schrieb Fabian Grünbichler:
>> On September 3, 2025 4:22 pm, Fiona Ebner wrote:
>>> diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
>>> index b571e6c1..95db271b 100644
>>> --- a/src/PVE/API2/Qemu.pm
>>> +++ b/src/PVE/API2/Qemu.pm
>>> @@ -1484,10 +1494,37 @@ sub print_netdevice_full {
>>>  
>>>      my $mtu = $net->{mtu};
>>>  
>>> -    if ($net->{model} eq 'virtio' && $net->{bridge}) {
>>> +    if (defined($host_mtu_migration)) {
>>> +        if ($host_mtu_migration) {
>>> +            if (defined($mtu) && $mtu != 1) {
>>> +                if ($mtu != $host_mtu_migration) {
>>> +                    log_warn(
>>> +                        "netdev $netid: using 'host_mtu=$host_mtu_migration' for migration compat,"
>>> +                            . " but value different from value in configuration '$mtu'");
>>> +                } # else avoid being overly verbose if there is an explicit setting
>> 
>> can this happen in practice (without manually editing the config or
>> manual QMP commands)?
> 
> I don't think so. But since we have the info, I though it'd be good to
> warn about it.
> 

I just figured it makes the code harder to parse ;)

>> 
>>> +            } else {
>>> +                print "netdev $netid: using 'host_mtu=$host_mtu_migration' for migration compat\n";
>>> +            }
>>> +        } else {
>>> +            print "netdev $netid: not adding 'host_mtu' parameter for migration compat\n";
>>> +        }
>>> +    }
>> 
>> this if here
>> 
>>> +
>>> +    if (
>>> +        $net->{model} eq 'virtio'
>>> +        && $net->{bridge}
>>> +        && (!defined($host_mtu_migration) || $host_mtu_migration)
>>> +    ) {
>>>          my $bridge_mtu = PVE::Network::read_bridge_mtu($net->{bridge});
>>>  
>>> -        if (!defined($mtu) || $mtu == 1) {
>>> +        if ($host_mtu_migration) {
>> 
>> and this if here could be combined?
> 
> How? You mean setting $mtu = $host_mtu_migration; early if there is a
> non-zero value?

something like this (untested):

----8<----
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 77b8e9c4..a76a134a 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1494,7 +1494,13 @@ sub print_netdevice_full {
 
     my $mtu = $net->{mtu};
 
-    if (defined($host_mtu_migration)) {
+    if (
+        $net->{model} eq 'virtio'
+        && $net->{bridge}
+        && (!defined($host_mtu_migration) || $host_mtu_migration)
+    ) {
+        my $bridge_mtu = PVE::Network::read_bridge_mtu($net->{bridge});
+
         if ($host_mtu_migration) {
             if (defined($mtu) && $mtu != 1) {
                 if ($mtu != $host_mtu_migration) {
@@ -1505,26 +1511,10 @@ sub print_netdevice_full {
             } else {
                 print "netdev $netid: using 'host_mtu=$host_mtu_migration' for migration compat\n";
             }
-        } else {
-            print "netdev $netid: not adding 'host_mtu' parameter for migration compat\n";
-        }
-    }
-
-    if (
-        $net->{model} eq 'virtio'
-        && $net->{bridge}
-        && (!defined($host_mtu_migration) || $host_mtu_migration)
-    ) {
-        my $bridge_mtu = PVE::Network::read_bridge_mtu($net->{bridge});
-
-        if ($host_mtu_migration) {
             $mtu = $host_mtu_migration;
-            # TODO PVE 10 - upgrade to failure? Certain network traffic can break like
-            # iperf3 -c 10.10.10.11 -u -l 2k when host_mtu=9000 and bridge MTU=1500
-            if ($mtu > $bridge_mtu) {
-                log_warn("netdev $netid: MTU '$mtu' is bigger than the bridge MTU '$bridge_mtu'");
-            }
-        } elsif (!defined($mtu) || $mtu == 1) {
+        }
+
+        if (!defined($mtu) || $mtu == 1) {
             $mtu = $bridge_mtu;
         } elsif ($mtu < 576) {
             die "netdev $netid: MTU '$mtu' is smaller than the IP minimum MTU '576'\n";
---->8----

I dropped the log statement about host_mtu_migration being an explicit
zero, since that is logged further below already if $mtu is defined, but
it could of course be added back there also for the $mtu == undef case
with a final

} elsif (defined($host_mtu_migration) && $host_mtu_migration == 0) {
    log_warn(..);
}

> 
>> 
>>> +            $mtu = $host_mtu_migration;
>>> +            # TODO PVE 10 - upgrade to failure? Certain network traffic can break like
>>> +            # iperf3 -c 10.10.10.11 -u -l 2k when host_mtu=9000 and bridge MTU=1500
>>> +            if ($mtu > $bridge_mtu) {
>>> +                log_warn("netdev $netid: MTU '$mtu' is bigger than the bridge MTU '$bridge_mtu'");
>>> +            }
>>> +        } elsif (!defined($mtu) || $mtu == 1) {
>> 
>> this could still be an if, then the newly added warning above can be
>> dropped
>> 
>>>              $mtu = $bridge_mtu;
>>>          } elsif ($mtu < 576) {
>>>              die "netdev $netid: MTU '$mtu' is smaller than the IP minimum MTU '576'\n";
>>> @@ -1495,7 +1532,7 @@ sub print_netdevice_full {
>>>              die "netdev $netid: MTU '$mtu' is bigger than the bridge MTU '$bridge_mtu'\n";
>> 
>> since it is covered by this one here
> 
> If we want to go for early failure when host_mtu > bridge MTU then yes.
> Because this is die, the above is warn ;) Doing that is fine by me, but
> I wanted to propose the non-breaking option first. If we consider that
> it's problematic in more cases than not, then I'll go for early failure.
> And maybe I'll add a little more context to the error message (at least
> when it happens for migration).

could still handle it in one place, and warn if $host_mtu_migration
(either inline, or via an elsif coming first that compares
$host_mtu_migration to $bridge_mtu), die otherwise - but I think if we
die on this condition when starting, then making it fatal for migration
starts might make sense as well, since the die would happen early on (so
no harm done).. might want to add a warning in that case as well that
tells the user what to do to make the VM migrateable though?