fix: lifecycle -Wait blocks until the guest config lock clears

PVE publishes a guest's new status while the operation still holds /var/lock/qemu-server/lock-<vmid>.conf, so WaitForStatusTransition could return while the guest was still locked and the caller's next request would fail with "can't lock file ... got timeout". Integration run 183 failed four tests from this one cause: Restart-PveVm -Wait returned after 4.1s having seen "running", the following Stop-PveVm spent exactly 10.0s failing to take the lock, and that cascaded into the template convert, clone, and remove tests. Run 184 - same commit, re-run - passed because its status poll happened to take 10.1s, by which point the lock had cleared. The same settling happens either way; the only variable is whether the wait absorbs it or the next caller does. The check goes in WaitForStatusTransition because all nine lifecycle call sites (Start/Stop/Restart/Reset/Resume across VMs and containers) route through it. `lock` comes from the status/current response the poll already fetches - present on both qemu and lxc since PVE 5.4, below the module's 7.0 floor - so it costs no extra request. If the status is reached but the lock outlasts -Timeout the cmdlet still returns success, so a call that succeeded before this change cannot become an exception after it. Recorded as D015, the guest-lock sibling of D014. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-03 18:55:33 +00:00 · 2026-09-01 16:33:47 +00:00
parent 845628711b
commit 3f63ddcad8
2 changed files with 110 additions and 0 deletions
@@ -0,0 +1,40 @@
+using System;
+using Newtonsoft.Json.Linq;
+
+namespace PSProxmoxVE.Core.Utilities
+{
+    /// <summary>
+    /// Reads the fields of a guest status/current response that decide whether a
+    /// lifecycle wait (-Wait on Start, Stop, Restart, Reset, Resume) is finished.
+    /// </summary>
+    public static class GuestStatusSnapshot
+    {
+        /// <summary>
+        /// Evaluates a status/current response body against the status a caller is waiting for.
+        /// </summary>
+        /// <param name="json">Raw status/current response body.</param>
+        /// <param name="expectedStatus">The status being waited for (e.g. "running", "stopped", "paused").</param>
+        /// <returns>
+        /// StatusMatched: the guest reports <paramref name="expectedStatus"/>. qmpstatus is
+        /// preferred over status when present, because PVE reports status=running with
+        /// qmpstatus=paused for a suspended VM.
+        /// Locked: the guest config still carries a lock, so the next API call against it
+        /// would fail to acquire the lock file.
+        /// </returns>
+        public static (bool StatusMatched, bool Locked) Evaluate(string json, string expectedStatus)
+        {
+            if (string.IsNullOrEmpty(json))
+                return (false, false);
+
+            var data = JObject.Parse(json)["data"];
+            var status = data?["status"]?.ToString();
+            var qmpStatus = data?["qmpstatus"]?.ToString();
+            var effectiveStatus = qmpStatus ?? status;
+
+            var matched = string.Equals(effectiveStatus, expectedStatus, StringComparison.OrdinalIgnoreCase);
+            var locked = !string.IsNullOrEmpty(data?["lock"]?.ToString());
+
+            return (matched, locked);
+        }
+    }
+}
@@ -0,0 +1,70 @@
+using PSProxmoxVE.Core.Utilities;
+using Xunit;
+
+namespace PSProxmoxVE.Core.Tests.Utilities
+{
+    public class GuestStatusSnapshotTests
+    {
+        [Fact]
+        public void Evaluate_RunningAndUnlocked_MatchesAndIsNotLocked()
+        {
+            var json = @"{""data"": {""status"": ""running"", ""qmpstatus"": ""running""}}";
+
+            var result = GuestStatusSnapshot.Evaluate(json, "running");
+
+            Assert.True(result.StatusMatched);
+            Assert.False(result.Locked);
+        }
+
+        [Fact]
+        public void Evaluate_RunningButStillLocked_MatchesAndIsLocked()
+        {
+            var json = @"{""data"": {""status"": ""running"", ""qmpstatus"": ""running"", ""lock"": ""clone""}}";
+
+            var result = GuestStatusSnapshot.Evaluate(json, "running");
+
+            Assert.True(result.StatusMatched);
+            Assert.True(result.Locked);
+        }
+
+        [Fact]
+        public void Evaluate_PrefersQmpStatusOverStatus()
+        {
+            // PVE reports status=running with qmpstatus=paused for a suspended VM.
+            var json = @"{""data"": {""status"": ""running"", ""qmpstatus"": ""paused""}}";
+
+            Assert.False(GuestStatusSnapshot.Evaluate(json, "running").StatusMatched);
+            Assert.True(GuestStatusSnapshot.Evaluate(json, "paused").StatusMatched);
+        }
+
+        [Fact]
+        public void Evaluate_ContainerResponseWithoutQmpStatus_FallsBackToStatus()
+        {
+            var json = @"{""data"": {""status"": ""stopped""}}";
+
+            var result = GuestStatusSnapshot.Evaluate(json, "stopped");
+
+            Assert.True(result.StatusMatched);
+            Assert.False(result.Locked);
+        }
+
+        [Fact]
+        public void Evaluate_EmptyLockValue_IsNotLocked()
+        {
+            var json = @"{""data"": {""status"": ""stopped"", ""lock"": """"}}";
+
+            Assert.False(GuestStatusSnapshot.Evaluate(json, "stopped").Locked);
+        }
+
+        [Theory]
+        [InlineData("")]
+        [InlineData(null)]
+        public void Evaluate_EmptyBody_DoesNotMatch(string? json)
+        {
+            var result = GuestStatusSnapshot.Evaluate(json!, "running");
+
+            Assert.False(result.StatusMatched);
+            Assert.False(result.Locked);
+        }
+    }
+}