Re: [D] VM HA not working when there's an issue with vm restart [cloudstack]

["gusmef (via GitHub)" <gi...@apache.org>]

GitHub user gusmef added a comment to the discussion: VM HA not working when there's an issue with vm restart

Hello everyone, with the help of Claude i got this summary:
<html>
<body>
<!--StartFragment--><html><head></head><body>
<h2>Observed behavior</h2>
<p>When VM HA triggers a restart attempt that fails with a <code>CloudRuntimeException</code> (e.g. QEMU failing to acquire a qcow2 write lock on shared NFS storage), the HA manager incorrectly marks the work as <code>Done</code> at attempt 2/5, logging:</p>
<pre><code>VM i-2-1483-VM has been changed. Current State = Stopped Previous State = Stopped
last updated = 20 previous updated = 17
Completed work HAWork[17-HA-1483-Stopped-Scheduled]. Took 2/5 attempts.
</code></pre>
<p>The VM never restarts despite remaining attempts being available.</p>
<hr>
<h2>Root cause</h2>
<h3>Step 1 — <code>processWork()</code> fetches the VM once and never refreshes it</h3>
<p><code>HighAvailabilityManagerImpl.processWork()</code> fetches the VM at the top of the method before calling <code>restart()</code>:</p>
<pre><code class="language-java">final VMInstanceVO vm = _instanceDao.findById(work.getInstanceId()); // update_count = 17
</code></pre>
<p>This reference is never refreshed inside <code>processWork()</code>.</p>
<h3>Step 2 — a failed start attempt increments <code>update_count</code> three times</h3>
<p><code>restart()</code> → <code>startVm()</code> → <code>startVirtualMachineForHA()</code> → <code>advanceStart()</code> → <code>orchestrateStart()</code></p>
<p>Inside <code>orchestrateStart()</code>, a single failed attempt produces exactly three <code>stateTransitTo()</code> calls, each invoking <code>VMInstanceDaoImpl.updateState()</code> → <code>incrUpdated()</code>:</p>

Transition | Event | Counter
-- | -- | --
Stopped → Starting | StartRequested | 17 → 18
Starting → Starting | OperationRetry (per-host loop) | 18 → 19
Starting → Stopped | OperationFailed (outer finally) | 19 → 20


<p>All three increments are written to the DB. The <code>vm</code> object held by <code>processWork()</code> sees none of them.</p>
<h3>Step 3 — <code>CloudRuntimeException</code> bypasses the typed catch blocks in <code>restart()</code></h3>
<p>The QEMU write-lock failure surfaces as a <code>CloudRuntimeException</code>. This is <strong>not</strong> caught by any of the typed catch blocks inside <code>restart()</code>:</p>
<pre><code class="language-java">} catch (final InsufficientCapacityException e) { ... }
} catch (final ResourceUnavailableException e) { ... }
} catch (ConcurrentOperationException e) { ... }
} catch (OperationTimedoutException e) { ... }
</code></pre>
<p>It also bypasses the bottom of <code>restart()</code>, which would have correctly re-fetched the VM:</p>
<pre><code class="language-java">// never reached in this path:
vm = _itMgr.findById(vm.getId());
work.setUpdateTime(vm.getUpdated());  // would have captured 20
</code></pre>
<p>Instead it propagates up to <code>processWork()</code>'s generic <code>catch (Exception e)</code> block:</p>
<pre><code class="language-java">} catch (Exception e) {
    long nextTime = getRescheduleTime(wt);
    rescheduleWork(work, nextTime);
    if (vm != null) {
        work.setUpdateTime(vm.getUpdated());  // stale top-of-method object → 17
        work.setPreviousState(vm.getState()); // Stopped
    }
}
</code></pre>
<p><code>work.setUpdateTime(17)</code> is written to the HA work record and the work is rescheduled.</p>
<h3>Step 4 — the guard at attempt 2 fires on the stale snapshot</h3>
<p>At attempt 2, <code>restart()</code> fetches a fresh VM from the DB, getting <code>update_count = 20</code>, then immediately hits:</p>
<pre><code class="language-java">if (vm.getState() != work.getPreviousState() || vm.getUpdated() != work.getUpdateTime()) {
    logger.info("VM {} has been changed. ...", vm, vm.getState(), work.getPreviousState(),
            vm.getUpdated(), work.getUpdateTime());
    return null;
}
</code></pre>
<ul>
<li>States match: <code>Stopped == Stopped</code> ✓</li>
<li>Counters do not: <code>20 != 17</code> → condition is true → <code>return null</code></li>
</ul>
<p>Back in <code>processWork()</code>, a <code>null</code> return means "work completed":</p>
<pre><code class="language-java">if (nextTime == null) {
    logger.info("Completed work {}. Took {}/{} attempts.", work, work.getTimesTried() + 1, _maxRetries);
    work.setStep(Step.Done);
}
</code></pre>
<p>All remaining attempts are silently abandoned.</p>
<hr>
<h2>Why the normal retry path does not have this problem</h2>
<p>When <code>startVm()</code> fails with one of the <strong>typed</strong> declared exceptions, the exception is caught inside <code>restart()</code> and execution falls through to the bottom:</p>
<pre><code class="language-java">vm = _itMgr.findById(vm.getId());       // fresh DB fetch → update_count = 20
work.setUpdateTime(vm.getUpdated());    // correctly snapshots 20
work.setPreviousState(vm.getState());
return (System.currentTimeMillis() &gt;&gt; 10) + _restartRetryInterval;
</code></pre>
<p>This correctly re-fetches the VM before snapshotting the counter, so attempt 2's guard sees <code>20 == 20</code> and passes. The bug is specific to any unchecked exception not in the typed catch list of <code>restart()</code>.</p>
<hr>
<h2>Proposed fix</h2>
<p>The <code>catch (Exception e)</code> block in <code>processWork()</code> should re-fetch the VM from the DB before snapshotting the counter, mirroring exactly what the bottom of <code>restart()</code> already does correctly:</p>
<pre><code class="language-java">} catch (Exception e) {
    logger.warn("Encountered unhandled exception during HA process, reschedule work {}", work, e);
    long nextTime = getRescheduleTime(wt);
    rescheduleWork(work, nextTime);
    if (vm != null) {
        VMInstanceVO reloaded = _instanceDao.findById(vm.getId());
        if (reloaded != null) {
            work.setUpdateTime(reloaded.getUpdated());
            work.setPreviousState(reloaded.getState());
        }
    }
}
</code></pre>
<hr>
<h2>Affected scenario</h2>
<p>Any start failure that surfaces as <code>CloudRuntimeException</code> rather than one of the four typed exceptions declared in <code>startVm()</code>'s signature. Confirmed trigger: QEMU write-lock contention on shared NFS/SAN storage when the source host has not been fenced and its QEMU processes still hold the disk image open.</p></body></html><!--EndFragment-->
</body>
</html>

### Final consideration
Obviously you can implement whatever fix you think is best (or not), i'm just pointing out what i've found and why the vm stays stopped instead of starting. If you see something that does not add up feel free to tell me i'm completely/partially wrong. I'd just like to help, best regards

GitHub link: https://github.com/apache/cloudstack/discussions/13729#discussioncomment-17973863

----
This is an automatically sent email for users@cloudstack.apache.org.
To unsubscribe, please send an email to: users-unsubscribe@cloudstack.apache.org