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

[Fiona Ebner]

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
>> @@ -3383,6 +3383,15 @@ __PACKAGE__->register_method({
>>                  default => 0,
>>                  description => 'Whether to migrate conntrack entries for running VMs.',
>>              },
>> +            'nets-host-mtu' => {
>> +                type => 'string',
>> +                pattern => 'net\d+=\d+(,net\d+=\d+)*',
>> +                optional => 1,
>> +                description =>
>> +                    'Used for migration compat. List of VirtIO network devices and their effective'
>> +                    . ' host_mtu setting according to the QEMU object model on the source side of'
>> +                    . ' the migration.',
> 
> should we note here as well that `0` means "explicitly don't set host_mtu"?

Will add that.

>> @@ -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.

> 
>> +            } 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?

> 
>> +            $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).