From 80b70cdaf6f0e46963d4583bfec5182b404383ed Mon Sep 17 00:00:00 2001
From: Clint Branham <goodolclint@gmail.com>
Date: Fri, 22 May 2026 14:18:32 -0500
Subject: [PATCH] fix: address PR #67 review feedback

- HasDiskOptions(): include -ScsiHardware so the "disk options ignored"
  warning fires when -ScsiHardware is passed without -DiskStorage/-DiskSize.
- PveVmConfig.AdditionalProperties: lazy-init a backing field so the native
  dictionary is built once rather than reallocated on every property access
  (matters when iterating many configs in a pipeline). Safe because the model
  is effectively immutable after deserialization.
- New-PveVm.Tests.ps1: add a case asserting -DiskIoThread on scsi with a
  wrong -ScsiHardware (virtio-scsi-pci) is rejected, covering the validator's
  "!= virtio-scsi-single" branch (not just the null case).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/PSProxmoxVE.Core/Models/Vms/PveVmConfig.cs  | 7 ++++++-
 src/PSProxmoxVE/Cmdlets/Vms/NewPveVmCmdlet.cs   | 1 +
 tests/PSProxmoxVE.Tests/Vms/New-PveVm.Tests.ps1 | 5 +++++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/PSProxmoxVE.Core/Models/Vms/PveVmConfig.cs b/src/PSProxmoxVE.Core/Models/Vms/PveVmConfig.cs
index 1dd5ba3..b29fd3c 100644
--- a/src/PSProxmoxVE.Core/Models/Vms/PveVmConfig.cs
+++ b/src/PSProxmoxVE.Core/Models/Vms/PveVmConfig.cs
@@ -322,6 +322,8 @@ public class PveVmConfig
     [JsonExtensionData]
     private IDictionary<string, JToken>? ExtensionData { get; set; }
 
+    private Dictionary<string, object?>? _additionalProperties;
+
     /// <summary>
     /// Any VM config keys not surfaced as a typed property above (e.g. hostpci0,
     /// usb0, numa0, additional disk buses). Keys map to native .NET values so the
@@ -329,7 +331,10 @@ public class PveVmConfig
     /// </summary>
     [JsonIgnore]
     public Dictionary<string, object?> AdditionalProperties =>
-        ExtensionData == null
+        // Built once from the deserialized extension data (the model is effectively
+        // immutable after deserialization), avoiding a fresh allocation per access
+        // when iterating many configs in a pipeline.
+        _additionalProperties ??= ExtensionData == null
             ? new Dictionary<string, object?>()
             : ExtensionData.ToDictionary(kvp => kvp.Key, kvp => JsonHelper.ToNative(kvp.Value));
 
diff --git a/src/PSProxmoxVE/Cmdlets/Vms/NewPveVmCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Vms/NewPveVmCmdlet.cs
index 2f8f15e..af0d5b3 100644
--- a/src/PSProxmoxVE/Cmdlets/Vms/NewPveVmCmdlet.cs
+++ b/src/PSProxmoxVE/Cmdlets/Vms/NewPveVmCmdlet.cs
@@ -268,6 +268,7 @@ namespace PSProxmoxVE.Cmdlets.Vms
 
         private bool HasDiskOptions() =>
             !string.IsNullOrEmpty(DiskBus)
+            || !string.IsNullOrEmpty(ScsiHardware)
             || DiskIoThread.IsPresent
             || !string.IsNullOrEmpty(DiskAio)
             || DiskSsd.IsPresent
diff --git a/tests/PSProxmoxVE.Tests/Vms/New-PveVm.Tests.ps1 b/tests/PSProxmoxVE.Tests/Vms/New-PveVm.Tests.ps1
index f8d2cbe..af429ae 100644
--- a/tests/PSProxmoxVE.Tests/Vms/New-PveVm.Tests.ps1
+++ b/tests/PSProxmoxVE.Tests/Vms/New-PveVm.Tests.ps1
@@ -222,6 +222,11 @@ Describe 'New-PveVm' {
                 Should -Throw '*virtio-scsi-single*'
         }
 
+        It 'Should reject -DiskIoThread on scsi with a wrong -ScsiHardware (virtio-scsi-pci)' {
+            { New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '32' -DiskBus scsi -ScsiHardware 'virtio-scsi-pci' -DiskIoThread -WhatIf -ErrorAction Stop } |
+                Should -Throw '*virtio-scsi-single*'
+        }
+
         It 'Should accept -DiskIoThread on scsi with -ScsiHardware virtio-scsi-single' {
             { New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '32' -DiskBus scsi -ScsiHardware 'virtio-scsi-single' -DiskIoThread -WhatIf -ErrorAction Stop } |
                 Should -Not -Throw