From 75f4bbfeed82bf30beac00bef79ecb25801eefa5 Mon Sep 17 00:00:00 2001 From: "goodolclint-claude[bot]" <323206664+goodolclint-claude[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 16:41:30 +0000 Subject: [PATCH] fix: plumb skiplock parameter for VMs, force for containers VM removal: add skiplock=1 parameter when -Force is specified. PVE honors it for root@pam only; non-root callers receive 403 errors. Updated help text. Container removal: map -Force to force=1 (LXC-specific parameter for forcing removal of running containers). Containers do not support skiplock. Also: clarified class and parameter documentation to remove false claims about -Force suppressing confirmation (it does not). Fixes #136. Addresses correctness reviewer findings. --- src/PSProxmoxVE.Core/Services/ContainerService.cs | 6 +++--- src/PSProxmoxVE.Core/Services/VmService.cs | 12 ++++++------ .../Cmdlets/Containers/RemovePveContainerCmdlet.cs | 6 +++--- src/PSProxmoxVE/Cmdlets/Vms/RemovePveVmCmdlet.cs | 6 +++--- .../Services/VmServiceTests.cs | 2 +- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/PSProxmoxVE.Core/Services/ContainerService.cs b/src/PSProxmoxVE.Core/Services/ContainerService.cs index e57af92..c4cd6f7 100644 --- a/src/PSProxmoxVE.Core/Services/ContainerService.cs +++ b/src/PSProxmoxVE.Core/Services/ContainerService.cs @@ -330,15 +330,15 @@ namespace PSProxmoxVE.Core.Services string node, int vmid, bool purge = false, - bool skipLock = false) + bool force = false) { if (session == null) throw new ArgumentNullException(nameof(session)); if (string.IsNullOrWhiteSpace(node)) throw new ArgumentNullException(nameof(node)); var queryParams = new List(); queryParams.Add(purge ? "purge=1" : "purge=0"); - if (skipLock) - queryParams.Add("skiplock=1"); + if (force) + queryParams.Add("force=1"); var queryString = "?" + string.Join("&", queryParams); IPveHttpClient client = _injectedClient ?? new PveHttpClient(session); diff --git a/src/PSProxmoxVE.Core/Services/VmService.cs b/src/PSProxmoxVE.Core/Services/VmService.cs index d350088..c2ef198 100644 --- a/src/PSProxmoxVE.Core/Services/VmService.cs +++ b/src/PSProxmoxVE.Core/Services/VmService.cs @@ -208,9 +208,9 @@ namespace PSProxmoxVE.Core.Services /// /// The import source in PVE format. Examples: /// - /// "local:iso/image.img" — import from a file already on storage - /// "local:import/myvm.ova/disk.vmdk" — import a disk from within an OVA - /// "/var/lib/vz/images/disk.qcow2" — import from an absolute path on the node + /// "local:iso/image.img" - import from a file already on storage + /// "local:import/myvm.ova/disk.vmdk" - import a disk from within an OVA + /// "/var/lib/vz/images/disk.qcow2" - import from an absolute path on the node /// /// /// Optional target format (e.g. "qcow2", "raw"). Defaults to storage default. @@ -334,7 +334,7 @@ namespace PSProxmoxVE.Core.Services /// its own post-stop cleanup, so nothing can interleave between the two halves. Composing /// a reboot client-side as shutdown + start instead races that cleanup: the start wins the /// lock, cleanup then holds it for 30 s waiting on the newly started process, and the next - /// call fails with "can't lock file '/var/lock/qemu-server/lock-<vmid>.conf' - got timeout". + /// call fails with "can't lock file '/var/lock/qemu-server/lock-.conf' - got timeout". /// /// The authenticated PVE session. /// The cluster node name. @@ -644,7 +644,7 @@ namespace PSProxmoxVE.Core.Services // PVE's agent/exec "command" is an array: element 0 is the executable and // each subsequent element is one argv entry. It is sent as repeated form // keys (command=&command=&...). Do NOT use "input-data" for - // arguments — that is the process's STDIN, not argv. + // arguments - that is the process's STDIN, not argv. var data = new List> { new KeyValuePair("command", command) @@ -757,7 +757,7 @@ namespace PSProxmoxVE.Core.Services } // ------------------------------------------------------------------------- - // Guest agent — extended operations + // Guest agent - extended operations // ------------------------------------------------------------------------- /// diff --git a/src/PSProxmoxVE/Cmdlets/Containers/RemovePveContainerCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Containers/RemovePveContainerCmdlet.cs index bdadf5d..7c6e5a3 100644 --- a/src/PSProxmoxVE/Cmdlets/Containers/RemovePveContainerCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Containers/RemovePveContainerCmdlet.cs @@ -8,7 +8,7 @@ namespace PSProxmoxVE.Cmdlets.Containers /// Removes an LXC container from a Proxmox VE node. /// /// Deletes an LXC container and, optionally, all associated storage. - /// This operation is destructive and requires confirmation unless -Force is specified. + /// This operation is destructive and requires confirmation. /// /// [Cmdlet(VerbsCommon.Remove, "PveContainer", @@ -42,10 +42,10 @@ namespace PSProxmoxVE.Cmdlets.Containers /// /// - /// When specified, sends skiplock=1 to PVE, which bypasses locks. PVE honours this for root@pam only. + /// When specified, sends force=1 to PVE, allowing removal of running containers. /// /// - [Parameter(Mandatory = false, HelpMessage = "Bypass locks (root@pam only); sends skiplock=1 to PVE.")] + [Parameter(Mandatory = false, HelpMessage = "Force destroy, even if running.")] public SwitchParameter Force { get; set; } /// diff --git a/src/PSProxmoxVE/Cmdlets/Vms/RemovePveVmCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Vms/RemovePveVmCmdlet.cs index b047c25..e5901ed 100644 --- a/src/PSProxmoxVE/Cmdlets/Vms/RemovePveVmCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Vms/RemovePveVmCmdlet.cs @@ -8,7 +8,7 @@ namespace PSProxmoxVE.Cmdlets.Vms /// Removes a QEMU/KVM virtual machine from a Proxmox VE node. /// /// Deletes a virtual machine and, optionally, all associated disk images. - /// This operation is destructive and requires confirmation unless -Force is specified. + /// This operation is destructive and requires confirmation. /// /// [Cmdlet(VerbsCommon.Remove, "PveVm", @@ -42,10 +42,10 @@ namespace PSProxmoxVE.Cmdlets.Vms /// /// - /// When specified, sends skiplock=1 to PVE, which bypasses locks. PVE honours this for root@pam only. + /// When specified, sends skiplock=1 to PVE to bypass locks. PVE honours this parameter for root@pam only; non-root callers will receive a 403 permission error. /// /// - [Parameter(Mandatory = false, HelpMessage = "Bypass locks (root@pam only); sends skiplock=1 to PVE.")] + [Parameter(Mandatory = false, HelpMessage = "Bypass locks via skiplock=1 (root@pam only).")] public SwitchParameter Force { get; set; } /// diff --git a/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs b/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs index 2c0147d..85f68f5 100644 --- a/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs +++ b/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs @@ -56,7 +56,7 @@ namespace PSProxmoxVE.Core.Tests.Services "powershell.exe", new[] { "-NoProfile", "-Command", "echo hi" }); Assert.NotNull(captured); - // Args are argv, not STDIN — "input-data" must never be emitted. + // Args are argv, not STDIN - "input-data" must never be emitted. Assert.DoesNotContain(captured!, kvp => kvp.Key == "input-data"); }