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>
This commit is contained in:
Clint Branham
2026-05-22 14:18:32 -05:00
parent 63ee16a9e7
commit 80b70cdaf6
3 changed files with 12 additions and 1 deletions
@@ -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));
@@ -268,6 +268,7 @@ namespace PSProxmoxVE.Cmdlets.Vms
private bool HasDiskOptions() =>
!string.IsNullOrEmpty(DiskBus)
|| !string.IsNullOrEmpty(ScsiHardware)
|| DiskIoThread.IsPresent
|| !string.IsNullOrEmpty(DiskAio)
|| DiskSsd.IsPresent
@@ -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