* [pve-devel] [PATCH qemu-server master+stable-bookworm] migration: check if target supports nets-host-mtu parameter up front @ 2025-09-08 13:47 Fiona Ebner 2025-09-08 14:49 ` Thomas Lamprecht 0 siblings, 1 reply; 3+ messages in thread From: Fiona Ebner @ 2025-09-08 13:47 UTC (permalink / raw) To: pve-devel 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pve-devel] [PATCH qemu-server master+stable-bookworm] migration: check if target supports nets-host-mtu parameter up front 2025-09-08 13:47 [pve-devel] [PATCH qemu-server master+stable-bookworm] migration: check if target supports nets-host-mtu parameter up front Fiona Ebner @ 2025-09-08 14:49 ` Thomas Lamprecht 2025-09-09 8:01 ` Fiona Ebner 0 siblings, 1 reply; 3+ messages in thread From: Thomas Lamprecht @ 2025-09-08 14:49 UTC (permalink / raw) To: Proxmox VE development discussion, Fiona Ebner 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pve-devel] [PATCH qemu-server master+stable-bookworm] migration: check if target supports nets-host-mtu parameter up front 2025-09-08 14:49 ` Thomas Lamprecht @ 2025-09-09 8:01 ` Fiona Ebner 0 siblings, 0 replies; 3+ messages in thread From: Fiona Ebner @ 2025-09-09 8:01 UTC (permalink / raw) To: Thomas Lamprecht, Proxmox VE development discussion 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-09-09 8:02 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-09-08 13:47 [pve-devel] [PATCH qemu-server master+stable-bookworm] migration: check if target supports nets-host-mtu parameter up front Fiona Ebner 2025-09-08 14:49 ` Thomas Lamprecht 2025-09-09 8:01 ` Fiona Ebner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox