From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <f.ebner@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id 1762FBBCE6
 for <pve-devel@lists.proxmox.com>; Tue, 19 Dec 2023 14:45:04 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id EF1D51EE
 for <pve-devel@lists.proxmox.com>; Tue, 19 Dec 2023 14:45:03 +0100 (CET)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS
 for <pve-devel@lists.proxmox.com>; Tue, 19 Dec 2023 14:45:02 +0100 (CET)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 6C4624854B
 for <pve-devel@lists.proxmox.com>; Tue, 19 Dec 2023 14:45:02 +0100 (CET)
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Tue, 19 Dec 2023 14:44:59 +0100
Message-Id: <20231219134459.49187-1-f.ebner@proxmox.com>
X-Mailer: git-send-email 2.39.2
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.076 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 T_SCC_BODY_TEXT_LINE    -0.01 -
 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
 http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
 information. [proxmox.com, qemuserver.pm]
Subject: [pve-devel] [PATCH v2 qemu-server] fix #4501: TCP migration: start
 vm: move port reservation and usage closer together
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Tue, 19 Dec 2023 13:45:04 -0000

Currently, volume activation, PCI reservation and resetting systemd
scope happen in between, so the 5 second expiretime used for port
reservation is not always enough.

It's possible to defer telling QEMU where it should listen for
migration and do so after it has been started via QMP. Therefore, the
port reservation can be moved very close to the actual usage.

Mentioned here for completeness and can still be done as an additional
change later if desired: next_migrate_port could be modified to
optionally return the open socket and it should be possible to pass
the file descriptor directly to QEMU, but that would require accepting
the connection before on the Perl side (otherwise leads to ENOTCONN
107). While it would avoid any races, it's not the most elegant
and the change at hand should be enough in all practical situations.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Discussion for v1:
https://lists.proxmox.com/pipermail/pve-devel/2023-November/060149.html

Changes in v2:
    * move reservation+usage much closer together than was done in v1
      of the qemu-server patch
    * drop other partial fix attempts for pve-common

 PVE/QemuServer.pm | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 2063e663..3b1540b6 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -5804,10 +5804,9 @@ 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};
+	    # see #4501: port reservation should be done close to usage - tell QEMU where to listen
+	    # via QMP later
+	    push @$cmd, '-incoming', 'defer';
 	    push @$cmd, '-S';
 
 	} elsif ($statefile eq 'unix') {
@@ -5991,8 +5990,15 @@ sub vm_start_nolock {
     eval { PVE::QemuServer::PCI::reserve_pci_usage($pci_reserve_list, $vmid, undef, $pid) };
     warn $@ if $@;
 
-    if (defined($res->{migrate})) {
-	print "migration listens on $res->{migrate}->{uri}\n";
+    if (defined(my $migrate = $res->{migrate})) {
+	if ($migrate->{proto} eq 'tcp') {
+	    my $nodename = nodename();
+	    my $pfamily = PVE::Tools::get_host_address_family($nodename);
+	    $migrate->{port} = PVE::Tools::next_migrate_port($pfamily);
+	    $migrate->{uri} = "tcp:$migrate->{addr}:$migrate->{port}";
+	    mon_cmd($vmid, "migrate-incoming", uri => $migrate->{uri});
+	}
+	print "migration listens on $migrate->{uri}\n";
     } elsif ($statefile) {
 	eval { mon_cmd($vmid, "cont"); };
 	warn $@ if $@;
-- 
2.39.2