[PATCH manager/storage 0/2] fix #6202: warn if sockets exceed target node

[PATCH manager/storage 0/2] fix #6202: warn if sockets exceed target node

[Nicolas Frey]

and guard against division by zero in the import.

The latter came up in enterprise support, which is only tangentially
related to the original issue (but got me into fixing this issue in
the first place).

Tested using an ESXi VM with the entry 'cpuid.coresPerSocket'
manually set to '0', which can normally only happen when it is set to
"Assigned at power on" [0], i.e. the number of cores per socket is
"optimally selected" by ESXi.

Feedback on what to use as the nodeSocket heuristic would be great,
the initial bug report suggests the # of sockets on the node, but if
e.g. NUMA is enabled, then the # of CPU cores is the actual max for VM
sockets. Or do we want to generally warn about the differences between
how PVE and ESXi model CPU topology, regardless of the node's sockets?

[0] https://techdocs.broadcom.com/us/en/vmware-cis/vsphere/vsphere/9-0/vsphere-resource-management/using-numa-systems-with-esxi/virtual-numa-controls.html

pve-manager:

Nicolas Frey (1):
  fix #6202: ui: guest import: warn if sockets exceed target node

 www/manager6/window/GuestImport.js | 33 ++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)


pve-storage:

Nicolas Frey (1):
  esxi: guard against zero 'cpuid.coresPerSocket'

 src/PVE/Storage/ESXiPlugin.pm | 8 ++++++++
 1 file changed, 8 insertions(+)


Summary over all repositories:
  2 files changed, 41 insertions(+), 0 deletions(-)

--
Generated by murpp 0.12.1

[PATCH pve-manager 1/2] fix #6202: ui: guest import: warn if sockets exceed target node

[Nicolas Frey]

ESXi and PVE model CPU topology differently, so the socket count
taken from the source VM can exceed the number of sockets the target
node actually has. The import then succeeds but yields a guest whose
topology does not match the hardware.

Query the node status and show a hint next to the socket field when
the configured count is higher, suggesting cores instead.

Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
 www/manager6/window/GuestImport.js | 33 ++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/www/manager6/window/GuestImport.js b/www/manager6/window/GuestImport.js
index 1013fe14..896cd9ac 100644
--- a/www/manager6/window/GuestImport.js
+++ b/www/manager6/window/GuestImport.js
@@ -310,6 +310,7 @@ Ext.define('PVE.window.GuestImport', {
         data: {
             coreCount: 1,
             socketCount: 1,
+            nodeSockets: 0,
             liveImport: false,
             os: 'l26',
             maxCdDrives: false,
@@ -320,6 +321,20 @@ Ext.define('PVE.window.GuestImport', {
 
         formulas: {
             totalCoreCount: (get) => get('socketCount') * get('coreCount'),
+            // ESXi and PVE model CPU topology differently, so an imported socket count can
+            // exceed what the target node physically has.
+            socketCountHint: (get) => {
+                let nodeSockets = get('nodeSockets');
+                if (!nodeSockets || get('socketCount') <= nodeSockets) {
+                    return '';
+                }
+                return Ext.String.format(
+                    gettext(
+                        'Target node has only {0} CPU socket(s), consider using cores instead.',
+                    ),
+                    nodeSockets,
+                );
+            },
             hideWarnings: (get) => get('warnings').length === 0,
             warningsText: (get) =>
                 '<ul style="margin: 0; padding-left: 20px;">' +
@@ -500,6 +515,15 @@ Ext.define('PVE.window.GuestImport', {
                                 value: '{socketCount}',
                             },
                         },
+                        {
+                            xtype: 'displayfield',
+                            userCls: 'pmx-hint',
+                            hidden: true,
+                            bind: {
+                                value: '{socketCountHint}',
+                                hidden: '{!socketCountHint}',
+                            },
+                        },
                         {
                             xtype: 'proxmoxintegerfield',
                             fieldLabel: gettext('Cores'),
@@ -1010,6 +1034,15 @@ Ext.define('PVE.window.GuestImport', {
         me.lookup('defaultBridge').setNodename(me.nodename);
         me.lookup('extractionStorage').setNodename(me.nodename);
 
+        Proxmox.Utils.API2Request({
+            url: `/nodes/${me.nodename}/status`,
+            method: 'GET',
+            autoErrorAlert: false,
+            success: function (response) {
+                me.getViewModel().set('nodeSockets', response.result.data.cpuinfo?.sockets ?? 0);
+            },
+        });
+
         let renderWarning = (w) => {
             const warningsCatalogue = {
                 'cdrom-image-ignored': gettext(
-- 
2.47.3

[PATCH pve-storage 2/2] esxi: guard against zero 'cpuid.coresPerSocket'

[Nicolas Frey]

ESXi sets 'cpuid.coresPerSocket' to 0 when the CPU topology is set to
'Assigned at power on'. cpu_info() divided by it unconditionally, so
such a VM aborted the import with "Illegal division by zero".

Fall back to one core per socket, matching the default already used
for a missing key, and log a warning so the resulting topology can be
checked.

Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---

Notes:
    The warning could also be emitted in case 'cpuid.coresPerSocket' is not
    present at all (right above)

 src/PVE/Storage/ESXiPlugin.pm | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/PVE/Storage/ESXiPlugin.pm b/src/PVE/Storage/ESXiPlugin.pm
index 19f23bb..4d27bff 100644
--- a/src/PVE/Storage/ESXiPlugin.pm
+++ b/src/PVE/Storage/ESXiPlugin.pm
@@ -746,6 +746,8 @@ use strict;
 use warnings;
 use feature 'fc';
 
+use PVE::RESTEnvironment qw(log_warn);
+
 # FIXME: see if vmx files can actually have escape sequences in their quoted values?
 my sub unquote : prototype($) {
     my ($value) = @_;
@@ -890,6 +892,12 @@ sub cpu_info {
     my ($self) = @_;
 
     my $cps = int($self->{'cpuid.coresPerSocket'} // 1);
+    # ESXi reports 0 when the CPU topology is 'Assigned at power on'.
+    if ($cps == 0) {
+        log_warn("'cpuid.coresPerSocket' is set to 0, assuming 1 core per socket\n");
+        $cps = 1;
+    }
+
     my $max = int($self->{numvcpus} // $cps);
 
     return ($cps, ($max / $cps));
-- 
2.47.3