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

[Fiona Ebner <f.ebner@proxmox.com>]

The virtual hardware is generated differently (at least for i440fx
machines) when host_mtu is set or not set on the netdev command line
[0]. When the MTU is the same value as the default 1500, Proxmox VE
did not add a host_mtu parameter. This is problematic for migration
where host_mtu is present on one end of the migration, but not on the
other [1]. Moreover, the effective setting in the guest (state) will
still be the host_mtu from the source side, even if a different value
is used for host_mtu on the target instance's commandline. This will
not lead to an error loading the migration stream in QEMU, but having
a larger host_mtu than the bridge MTU is still problematic for certain
network traffic like
> iperf3 -c 10.10.10.11 -u -l 2k
when host_mtu=9000 and bridge MTU=1500. Preventing migrations in this
case would be a breaking change, so for now, only a warning is added.

Add the necessary parameter for VM start to allow preserving the
values going forward.

[0]: https://bugzilla.redhat.com/show_bug.cgi?id=1449346
[1]: https://forum.proxmox.com/threads/live-vm-migration-fails.169537/post-796379

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/API2/Qemu.pm  | 11 ++++++++
 src/PVE/QemuServer.pm | 61 ++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 66 insertions(+), 6 deletions(-)

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.',
+            },
         },
     },
     returns => {
@@ -3414,6 +3423,7 @@ __PACKAGE__->register_method({
         my $targetstorage = $get_root_param->('targetstorage');
         my $force_cpu = $get_root_param->('force-cpu');
         my $with_conntrack_state = $get_root_param->('with-conntrack-state');
+        my $nets_host_mtu = $get_root_param->('nets-host-mtu');
 
         my $storagemap;
 
@@ -3501,6 +3511,7 @@ __PACKAGE__->register_method({
                     forcemachine => $machine,
                     timeout => $timeout,
                     forcecpu => $force_cpu,
+                    'nets-host-mtu' => $nets_host_mtu,
                 };
 
                 PVE::QemuServer::vm_start($storecfg, $vmid, $params, $migrate_opts);
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 5b7087dc..77b8e9c4 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1457,7 +1457,17 @@ sub print_pbs_blockdev {
 }
 
 sub print_netdevice_full {
-    my ($vmid, $conf, $net, $netid, $bridges, $use_old_bios_files, $arch, $machine_version) = @_;
+    my (
+        $vmid,
+        $conf,
+        $net,
+        $netid,
+        $bridges,
+        $use_old_bios_files,
+        $arch,
+        $machine_version,
+        $host_mtu_migration, # force this value for host_mtu, 0 means force absence of param
+    ) = @_;
 
     my $device = $net->{model};
     if ($net->{model} eq 'virtio') {
@@ -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
+            } 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 (!defined($mtu) || $mtu == 1) {
+        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) {
             $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";
         }
 
-        if (min_version($machine_version, 10, 0, 1)) {
+        if (min_version($machine_version, 10, 0, 1) || $host_mtu_migration) {
             # Always add host_mtu for migration compatibility, because the presence of host_mtu
             # means that the virtual hardware is generated differently (at least for i440fx)
             $tmpstr .= ",host_mtu=$mtu";
@@ -1503,8 +1540,14 @@ sub print_netdevice_full {
             $tmpstr .= ",host_mtu=$mtu" if $mtu != 1500;
         }
     } elsif (defined($mtu)) {
-        warn
-            "WARN: netdev $netid: ignoring MTU '$mtu', not using VirtIO or no bridge configured.\n";
+        my $msg_prefix = "netdev $netid: ignoring MTU '$mtu'";
+        if (defined($host_mtu_migration) && !$host_mtu_migration) {
+            log_warn("$msg_prefix, not used on the source side according to migration parameters");
+        } elsif (!$net->{bridge}) {
+            log_warn("$msg_prefix, no bridge configured");
+        } else {
+            log_warn("$msg_prefix, not using VirtIO");
+        }
     }
 
     if ($use_old_bios_files) {
@@ -3810,6 +3853,8 @@ sub config_to_command {
         },
     );
 
+    my $nets_host_mtu =
+        { map { split('=', $_) } PVE::Tools::split_list($options->{'nets-host-mtu'}) };
     for (my $i = 0; $i < $MAX_NETS; $i++) {
         my $netname = "net$i";
 
@@ -3836,6 +3881,7 @@ sub config_to_command {
             $use_old_bios_files,
             $arch,
             $machine_version,
+            $nets_host_mtu->{$netname},
         );
 
         push @$devices, '-device', $netdevicefull;
@@ -5566,6 +5612,8 @@ sub vm_start {
 #      },
 #      virtio2 => ...
 #   }
+#   nets-host-mtu => 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.
 # migrate_opts:
 #   nbd => volumes for NBD exports (vm_migrate_alloc_nbd_disks)
 #   migratedfrom => source node
@@ -5678,6 +5726,7 @@ sub vm_start_nolock {
                 'force-machine' => $forcemachine,
                 'force-cpu' => $forcecpu,
                 'live-restore-backing' => $params->{'live-restore-backing'},
+                'nets-host-mtu' => $params->{'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