From a3c4154fb2b97e52d9dab87616a6e8a76b943582 Mon Sep 17 00:00:00 2001
From: Prophet731 <Prophet731@users.noreply.github.com>
Date: Sun, 26 Apr 2026 03:38:33 +0000
Subject: [PATCH] fix(StockControl): match storageType code instead of pool id
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The package field exposed by VirtFusion as `primaryStorageProfile` is a
storage *type code* (mirrors `server_packages.storage_type` in the VF
database), not a profile id. It's meant to filter to any pool whose
`storageType` matches — multiple pools across the fleet can carry the
same code, which is exactly how multi-hypervisor placement works for
mountpoint/datastore storage.

`capForStorage()` was checking `pool.id` against this code. Pool ids are
unique per hypervisor (e.g. for the same logical mountpoint on three
hypervisors, ids 23/28/30) and almost never match the type-code domain
(0=local default, 4=mountpoint, etc.). The mismatch silently returned 0
for every hypervisor, zeroing qty fleet-wide whenever the package's
type code didn't accidentally collide with some pool id.

Symptoms in the wild: every stock-controlled VPS product showed qty=0
in WHMCS even with abundant memory/CPU/IPv4 capacity. Disabling
`stockcontrol` on the product or removing `primaryStorageProfile` from
the package were the only known workarounds; both lose the actual stock
gating this module is meant to provide.

Fix:
- Match `pool.storageType` instead of `pool.id`.
- Walk all pools that match (a hypervisor may have multiple pools of
  the same type) and use the one that fits the most VMs, instead of
  short-circuiting on the first match. A disabled pool no longer kills
  the whole hypervisor's capacity for that type — we just skip it and
  keep looking for an enabled peer.
- Rename the parameter from `$profileId` to `$storageTypeId` so future
  readers don't fall into the same naming trap. Updated the docblock
  with a NOTE explaining the VirtFusion-side naming inconsistency.

Verified on a 3-hypervisor cluster with `storageType=4` (mountpoint)
packages: qty went from 0/0/0/0/0/0/0/0 to 66/32/15/7/3/1/32/15 across
the VPS-1 through VPS-32 + storage products without any other config
change.
---
 .../VirtFusionDirect/lib/StockControl.php     | 42 ++++++++++++++-----
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/modules/servers/VirtFusionDirect/lib/StockControl.php b/modules/servers/VirtFusionDirect/lib/StockControl.php
index f5df2d8..1b612cf 100644
--- a/modules/servers/VirtFusionDirect/lib/StockControl.php
+++ b/modules/servers/VirtFusionDirect/lib/StockControl.php
@@ -365,36 +365,56 @@ class StockControl
     /**
      * Storage variant of capFor() that respects the package's primaryStorageProfile.
      *
+     * NOTE on naming: VirtFusion exposes two confusingly-named fields with the
+     * same numeric domain. `package.primaryStorageProfile` (mirrors the DB column
+     * `server_packages.storage_type`) is a **storage type code** — a filter,
+     * not an ID — and matches `otherStorage[].storageType` on each hypervisor.
+     * The pool's own `id` is unique per hypervisor and is never what the package
+     * targets. Treating $storageTypeId as `pool.id` (as this method previously
+     * did) returned 0 for every package whose type code didn't happen to also
+     * exist as a pool id, silently zeroing qty fleet-wide.
+     *
      * Rules:
-     *   - profileId > 0  → must match an otherStorage[].id on the hypervisor; if the
-     *                      matched pool is disabled or missing, this hypervisor has
-     *                      zero storage capacity for this product (can't place there).
-     *   - profileId <= 0 → fall back to localStorage. If local is disabled, 0.
+     *   - storageTypeId > 0  → match any enabled otherStorage[] whose storageType
+     *                          equals this code. If multiple match (e.g. several
+     *                          mountpoint pools on one hypervisor), pick the one
+     *                          that fits the most VMs.
+     *   - storageTypeId <= 0 → fall back to localStorage. If local is disabled, 0.
      */
-    private static function capForStorage(array $res, int $profileId, int $needGb, float $bufferPct): int
+    private static function capForStorage(array $res, int $storageTypeId, int $needGb, float $bufferPct): int
     {
         if ($needGb <= 0) {
             return PHP_INT_MAX;
         }
 
-        if ($profileId > 0) {
+        if ($storageTypeId > 0) {
+            $best = 0;
+            $matched = false;
             foreach ($res['otherStorage'] ?? [] as $pool) {
-                if ((int) ($pool['id'] ?? 0) !== $profileId) {
+                if ((int) ($pool['storageType'] ?? 0) !== $storageTypeId) {
                     continue;
                 }
+                $matched = true;
                 if (empty($pool['enabled'])) {
-                    return 0;
+                    continue;
                 }
 
-                return self::capFor(
+                $cap = self::capFor(
                     ['max' => (int) ($pool['max'] ?? 0), 'free' => (int) ($pool['free'] ?? 0)],
                     $needGb,
                     $bufferPct,
                 );
+                if ($cap > $best) {
+                    $best = $cap;
+                }
             }
 
-            // Storage profile not present on this hypervisor — cannot place the VM.
-            return 0;
+            if (! $matched) {
+                // No pool of this storage type on this hypervisor — cannot place the VM.
+                return 0;
+            }
+
+            return $best;
         }
 
         $local = $res['localStorage'] ?? null;