On November 14, 2023 3:02 pm, Fiona Ebner wrote:
> Currently, volume activation, PCI reservation and resetting systemd
> scope happen in between and the 5 second expiretime used for port
> reservation might not be enough.
>
> Still not ideal, because entering systemd scope and maybe starting
> swtpm still happen after reservation before the QEMU binary can be
> invoked and actually use the port, but the reservation needs to happen
> outside of the fork, because the result is used there too.
>
> Signed-off-by: Fiona Ebner <f.eb...@proxmox.com>
Acked-by: Fabian Grünbichler <f.gruenbich...@proxmox.com>
we could move the whole statefile handling further down, but then some
additional side-effects need to be taken care of/refactored, this seems
like a minimal-invasive version for the uncommon (insecure) case.
> ---
> PVE/QemuServer.pm | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index c465fb6f..aeaea8eb 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5697,6 +5697,9 @@ sub vm_start_nolock {
> return $migration_ip;
> };
>
> + # helper to move port reservation and usage closer together to avoid
> expiry (bug #4501)
> + my $append_tcp_migration_cmdline;
> +
> if ($statefile) {
> if ($statefile eq 'tcp') {
> my $migrate = $res->{migrate} = { proto => 'tcp' };
> @@ -5717,12 +5720,13 @@ sub vm_start_nolock {
> $migrate->{addr} = "[$migrate->{addr}]" if
> Net::IP::ip_is_ipv6($migrate->{addr});
> }
>
> - my $pfamily = PVE::Tools::get_host_address_family($nodename);
> - $migrate->{port} = PVE::Tools::next_migrate_port($pfamily);
> - $migrate->{uri} = "tcp:$migrate->{addr}:$migrate->{port}";
> - push @$cmd, '-incoming', $migrate->{uri};
> - push @$cmd, '-S';
> -
nit: I'd maybe add another comment here, maybe something like
# delay migration port reservation to prevent expiry before binding
> + $append_tcp_migration_cmdline = sub {
> + my $pfamily = PVE::Tools::get_host_address_family($nodename);
> + $migrate->{port} = PVE::Tools::next_migrate_port($pfamily);
> + $migrate->{uri} = "tcp:$migrate->{addr}:$migrate->{port}";
> + push @$cmd, '-incoming', $migrate->{uri};
> + push @$cmd, '-S';
> + };
> } elsif ($statefile eq 'unix') {
> # should be default for secure migrations as a ssh TCP forward
> # tunnel is not deterministic reliable ready and fails regurarly
> @@ -5840,6 +5844,10 @@ sub vm_start_nolock {
> $systemd_properties{timeout} = 10 if $statefile; # setting up the scope
> shoul be quick
>
> my $run_qemu = sub {
> + # sets the port+uri for $res->{migrate} which is printed below and part
> of the result, so
> + # needs to happen outside of the fork.
> + $append_tcp_migration_cmdline->() if $append_tcp_migration_cmdline;
> +
> PVE::Tools::run_fork sub {
> PVE::Systemd::enter_systemd_scope($vmid, "Proxmox VE VM $vmid",
> %systemd_properties);
>
> --
> 2.39.2
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
