From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <d.csapak@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) server-digest SHA256)
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id DBD1289FB
 for <pve-devel@lists.proxmox.com>; Thu, 22 Jun 2023 14:15:47 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 4E29325A8A
 for <pve-devel@lists.proxmox.com>; Thu, 22 Jun 2023 14:15:17 +0200 (CEST)
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) server-digest SHA256)
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS
 for <pve-devel@lists.proxmox.com>; Thu, 22 Jun 2023 14:15:15 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id A0C9C42996
 for <pve-devel@lists.proxmox.com>; Thu, 22 Jun 2023 14:15:13 +0200 (CEST)
From: Dominik Csapak <d.csapak@proxmox.com>
To: pve-devel@lists.proxmox.com
Date: Thu, 22 Jun 2023 14:15:12 +0200
Message-Id: <20230622121512.1283861-1-d.csapak@proxmox.com>
X-Mailer: git-send-email 2.30.2
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.016 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 -
Subject: [pve-devel] [PATCH manager] ui: migrate: fix disabled migrate
 button glitch
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: Thu, 22 Jun 2023 12:15:47 -0000

under certain circumstances, the migrate button stays disabled, even
when a valid target node was selected:
* the first node that gets autoselected (most likely the second)
  is not a valid migration target
* the user changes to a migration target that is a valid one

if that happens, the migration button would stay disabled.
switching once to a non valid target and would enable the button.

To fix it, we have to do two things here:

'checkQemuPreconditions' is actually an async function that awaits an
api call and uses the result to set the 'migration.allowedNodes'
property

'checkMigratePreconditions' calls 'checkQemuPreconditions' and uses the
'migration.allowedNodes' property afterwards.

but since 'checkMigratePreconditions' is not async, that happens before
the api call can return the valid data and thus leaves it empty, making
all nodes valid in the selector. (thus the initial selected node is
valid)

instead make 'checkMigratePreconditions' also async and await the result
of 'checkQemuPreconditions'

this unearthed another issue, namely we access an object that is
possibly undefined (worked out before due to race conditions) so
fallback to an empty object.

and lastly, since we want the 'disallowedNodes' set before actually
checking the qemu preconditions, we move the setting of that on
the node selector above the qemu preconditions check
(this is the only place where we set it anyway, and the source does not
change, we probably could move that out of that function altogether)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 www/manager6/window/Migrate.js | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/www/manager6/window/Migrate.js b/www/manager6/window/Migrate.js
index c310342d..5473821b 100644
--- a/www/manager6/window/Migrate.js
+++ b/www/manager6/window/Migrate.js
@@ -155,7 +155,7 @@ Ext.define('PVE.window.Migrate', {
 	    });
 	},
 
-	checkMigratePreconditions: function(resetMigrationPossible) {
+	checkMigratePreconditions: async function(resetMigrationPossible) {
 	    var me = this,
 		vm = me.getViewModel();
 
@@ -165,12 +165,13 @@ Ext.define('PVE.window.Migrate', {
 		vm.set('running', true);
 	    }
 
+	    me.lookup('pveNodeSelector').disallowedNodes = [vm.get('nodename')];
+
 	    if (vm.get('vmtype') === 'qemu') {
-		me.checkQemuPreconditions(resetMigrationPossible);
+		await me.checkQemuPreconditions(resetMigrationPossible);
 	    } else {
 		me.checkLxcPreconditions(resetMigrationPossible);
 	    }
-	    me.lookup('pveNodeSelector').disallowedNodes = [vm.get('nodename')];
 
 	    // Only allow nodes where the local storage is available in case of offline migration
 	    // where storage migration is not possible
@@ -218,7 +219,7 @@ Ext.define('PVE.window.Migrate', {
 		migration.allowedNodes = migrateStats.allowed_nodes;
 		let target = me.lookup('pveNodeSelector').value;
 		if (target.length && !migrateStats.allowed_nodes.includes(target)) {
-		    let disallowed = migrateStats.not_allowed_nodes[target];
+		    let disallowed = migrateStats.not_allowed_nodes[target] ?? {};
 		    if (disallowed.unavailable_storages !== undefined) {
 			let missingStorages = disallowed.unavailable_storages.join(', ');
 
-- 
2.30.2