feat: disk controller/IO options on New-PveVm + surface all Get-PveVmConfig keys

New-PveVm (F089):
- Add -DiskBus (virtio/scsi/sata/ide, default virtio), -ScsiHardware (scsihw),
  -DiskIoThread, -DiskAio, -DiskSsd, -DiskDiscard, -DiskCache so a tuned disk
  (e.g. virtio-scsi-single + scsi0,iothread=1,aio=native,ssd=1,discard=on) can
  be created in one call instead of diskless + a hand-built Set-PveVmConfig string.
- Disk spec built via BuildDiskSpec; ValidateDiskOptions runs before ShouldProcess
  and rejects ssd on virtio and iothread on sata/ide or scsi-without-virtio-scsi-single
  with clear errors, instead of letting PVE fail at VM start.

Get-PveVmConfig (F090):
- PveVmConfig was a fixed allow-list, silently dropping keys like scsihw, efidisk0,
  tpmstate0, hostpci0. Add typed scsihw/efidisk0/tpmstate0 plus a [JsonExtensionData]
  catch-all exposed as AdditionalProperties (native types via JsonHelper.ToNative,
  per D013 — no JToken leakage). Makes the disk tuning above verifiable by reading
  the config back.

Closes #65.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Clint Branham
2026-05-22 14:10:14 -05:00
parent 5e5a8fcda4
commit c1714048b4
5 changed files with 358 additions and 9 deletions
+60
View File
@@ -2881,6 +2881,66 @@
"evidence": "Added PveSession.Timeout (default 100s) and a TimeSpan? override on PveHttpClient. Connect-PveServer exposes -TimeoutSeconds to set the session-default; Send-PveFile and Invoke-PveStorageDownload expose -TimeoutSeconds with a 30-minute implicit default so large uploads/downloads do not trip the 100s HttpClient default. -TimeoutSeconds 0 means Timeout.InfiniteTimeSpan. PveHttpClient.SendAsync now catches the TimeoutException-wrapped TaskCanceledException and rethrows it as PveApiException(RequestTimeout) with the resource path.",
"verified_by": "dotnet build + dotnet test (passed including new SendAsync_TimeoutFires xUnit test) + Pester (-TimeoutSeconds coverage on all three cmdlets)"
}
},
{
"id": "F089",
"title": "New-PveVm cannot set disk controller or IO options at create time",
"category": "enhancement",
"severity": "medium",
"status": "resolved",
"first_detected": "2026-05-22",
"github_issue": 65,
"files": [
"src/PSProxmoxVE/Cmdlets/Vms/NewPveVmCmdlet.cs"
],
"description": "New-PveVm only emitted a plain virtio0 from -DiskStorage/-DiskSize/-DiskFormat. There was no way to choose the controller (virtio-scsi-single) or set performance/IO options (iothread, aio, ssd, discard, cache) at create time, forcing callers to create the VM diskless and then hand-build a raw scsi0 string via Set-PveVmConfig.",
"scan_history": [
{
"scan_date": "2026-05-22",
"local_id": null,
"status": "new"
},
{
"scan_date": "2026-05-22",
"local_id": null,
"status": "fixed"
}
],
"resolution": {
"scan_date": "2026-05-22",
"evidence": "Added -DiskBus (virtio/scsi/sata/ide, default virtio), -ScsiHardware (scsihw), -DiskIoThread, -DiskAio, -DiskSsd, -DiskDiscard, -DiskCache. The disk spec is built via a BuildDiskSpec helper; ValidateDiskOptions enforces (before ShouldProcess) that ssd is not used on virtio and that iothread is only used on virtio or scsi+virtio-scsi-single, surfacing clear errors at create time instead of at VM start.",
"verified_by": "dotnet build (0 warnings) + Pester (47 New-PveVm tests incl. disk-option validation matrix)"
}
},
{
"id": "F090",
"title": "Get-PveVmConfig silently drops config keys not in the typed allow-list",
"category": "completeness",
"severity": "medium",
"status": "resolved",
"first_detected": "2026-05-22",
"github_issue": 65,
"files": [
"src/PSProxmoxVE.Core/Models/Vms/PveVmConfig.cs"
],
"description": "PveVmConfig was a fixed allow-list of [JsonProperty] fields with no catch-all, so keys like scsihw, efidisk0, tpmstate0, hostpci0, and additional disk buses were silently dropped on deserialize — making the F089 workaround unverifiable by reading the config back.",
"scan_history": [
{
"scan_date": "2026-05-22",
"local_id": null,
"status": "new"
},
{
"scan_date": "2026-05-22",
"local_id": null,
"status": "fixed"
}
],
"resolution": {
"scan_date": "2026-05-22",
"evidence": "Added typed scsihw/efidisk0/tpmstate0 properties plus a private [JsonExtensionData] landing field exposed as AdditionalProperties (Dictionary<string, object?> via JsonHelper.ToNative, so values are native .NET types per D013 — no JToken leakage). Typed keys do not duplicate into the catch-all.",
"verified_by": "dotnet build + dotnet test (586 passed, new VmModelTests for scsihw/efidisk/tpm, native-typed AdditionalProperties, and no typed-key leakage)"
}
}
]
}
@@ -1,4 +1,8 @@
using System.Collections.Generic;
using System.Linq;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using PSProxmoxVE.Core.Utilities;
namespace PSProxmoxVE.Core.Models.Vms;
@@ -52,6 +56,24 @@ public class PveVmConfig
[JsonProperty("machine")]
public string? Machine { get; set; }
/// <summary>
/// SCSI controller hardware model (e.g., "virtio-scsi-single", "lsi").
/// </summary>
[JsonProperty("scsihw")]
public string? ScsiHardware { get; set; }
/// <summary>
/// EFI vars disk spec (present on OVMF/UEFI VMs), e.g. "local-lvm:vm-100-disk-1,...".
/// </summary>
[JsonProperty("efidisk0")]
public string? EfiDisk0 { get; set; }
/// <summary>
/// TPM state disk spec (present on VMs with a virtual TPM).
/// </summary>
[JsonProperty("tpmstate0")]
public string? TpmState0 { get; set; }
// -------------------------------------------------------------------------
// Boot / Args
// -------------------------------------------------------------------------
@@ -288,6 +310,29 @@ public class PveVmConfig
[JsonProperty("searchdomain")]
public string? Searchdomain { get; set; }
// -------------------------------------------------------------------------
// Catch-all for unmapped config keys
// -------------------------------------------------------------------------
/// <summary>
/// Raw landing spot for any config key not mapped to a typed property above.
/// Populated by Newtonsoft during deserialization; exposed natively via
/// <see cref="AdditionalProperties"/>.
/// </summary>
[JsonExtensionData]
private IDictionary<string, JToken>? ExtensionData { get; set; }
/// <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
/// dictionary works naturally in PowerShell pipelines.
/// </summary>
[JsonIgnore]
public Dictionary<string, object?> AdditionalProperties =>
ExtensionData == null
? new Dictionary<string, object?>()
: ExtensionData.ToDictionary(kvp => kvp.Key, kvp => JsonHelper.ToNative(kvp.Value));
/// <inheritdoc />
public override string ToString()
{
+130 -9
View File
@@ -96,6 +96,67 @@ namespace PSProxmoxVE.Cmdlets.Vms
[Parameter(Mandatory = false, HelpMessage = "Disk format (e.g. raw, qcow2).")]
public string? DiskFormat { get; set; }
/// <summary>
/// <para type="description">
/// Bus/controller for the primary disk: "virtio" (default), "scsi", "sata", or "ide".
/// Determines the device key (virtio0, scsi0, sata0, ide0).
/// </para>
/// </summary>
[Parameter(Mandatory = false, HelpMessage = "Primary disk bus: virtio (default), scsi, sata, ide.")]
[ValidateSet("virtio", "scsi", "sata", "ide", IgnoreCase = true)]
public string? DiskBus { get; set; }
/// <summary>
/// <para type="description">
/// SCSI controller hardware model (sets the VM-level "scsihw" key), e.g.
/// "virtio-scsi-single", "virtio-scsi-pci", "lsi". Required as
/// "virtio-scsi-single" when combining -DiskBus scsi with -DiskIoThread.
/// </para>
/// </summary>
[Parameter(Mandatory = false, HelpMessage = "SCSI controller model (e.g. virtio-scsi-single).")]
public string? ScsiHardware { get; set; }
/// <summary>
/// <para type="description">
/// Enable a dedicated IO thread for the primary disk (iothread=1). Valid only for
/// the virtio bus or for the scsi bus with -ScsiHardware virtio-scsi-single.
/// </para>
/// </summary>
[Parameter(Mandatory = false, HelpMessage = "Enable a dedicated IO thread (iothread=1).")]
public SwitchParameter DiskIoThread { get; set; }
/// <summary>
/// <para type="description">Async IO mode for the primary disk: "native", "threads", or "io_uring".</para>
/// </summary>
[Parameter(Mandatory = false, HelpMessage = "Async IO mode: native, threads, io_uring.")]
[ValidateSet("native", "threads", "io_uring", IgnoreCase = true)]
public string? DiskAio { get; set; }
/// <summary>
/// <para type="description">
/// Mark the primary disk as SSD (ssd=1). Not supported on the virtio bus — use
/// -DiskBus scsi, sata, or ide.
/// </para>
/// </summary>
[Parameter(Mandatory = false, HelpMessage = "Mark the primary disk as SSD (ssd=1).")]
public SwitchParameter DiskSsd { get; set; }
/// <summary>
/// <para type="description">Enable discard/TRIM passthrough on the primary disk (discard=on).</para>
/// </summary>
[Parameter(Mandatory = false, HelpMessage = "Enable discard/TRIM passthrough (discard=on).")]
public SwitchParameter DiskDiscard { get; set; }
/// <summary>
/// <para type="description">
/// Cache mode for the primary disk: "none", "writethrough", "writeback",
/// "directsync", or "unsafe".
/// </para>
/// </summary>
[Parameter(Mandatory = false, HelpMessage = "Disk cache mode: none, writethrough, writeback, directsync, unsafe.")]
[ValidateSet("none", "writethrough", "writeback", "directsync", "unsafe", IgnoreCase = true)]
public string? DiskCache { get; set; }
/// <summary>
/// <para type="description">Network interface model (e.g., "virtio", "e1000").</para>
/// </summary>
@@ -128,13 +189,16 @@ namespace PSProxmoxVE.Cmdlets.Vms
protected override void ProcessRecord()
{
// Validate -DiskSize before ShouldProcess so typos like "512M" are rejected
// even with -WhatIf, and so the error is raised regardless of whether
// -DiskStorage is also supplied.
// Validate -DiskSize and disk-option combinations before ShouldProcess so
// typos and invalid combos are rejected even with -WhatIf, and so the error
// is raised regardless of whether -DiskStorage is also supplied.
string? diskSizeGib = null;
if (!string.IsNullOrEmpty(DiskSize))
diskSizeGib = SizeParser.NormalizeToGibibytes(DiskSize!, nameof(DiskSize));
var diskBus = string.IsNullOrEmpty(DiskBus) ? "virtio" : DiskBus!.ToLowerInvariant();
ValidateDiskOptions(diskBus);
if (!ShouldProcess($"VM on node '{Node}'", "New-PveVm"))
return;
@@ -172,14 +236,15 @@ namespace PSProxmoxVE.Cmdlets.Vms
config["machine"] = Machine!;
if (!string.IsNullOrEmpty(OsType))
config["ostype"] = OsType!;
if (!string.IsNullOrEmpty(ScsiHardware))
config["scsihw"] = ScsiHardware!;
if (!string.IsNullOrEmpty(DiskStorage) && diskSizeGib != null)
{
var diskValue = $"{DiskStorage}:{diskSizeGib}";
if (!string.IsNullOrEmpty(DiskFormat))
diskValue += $",format={DiskFormat}";
config["virtio0"] = diskValue;
}
config[$"{diskBus}0"] = BuildDiskSpec(DiskStorage!, diskSizeGib);
else if (HasDiskOptions())
WriteWarning("Disk IO options (-DiskBus/-DiskIoThread/-DiskAio/-DiskSsd/-DiskDiscard/-DiskCache) "
+ "were specified but no disk is being created (-DiskStorage and -DiskSize are both required). "
+ "The options were ignored.");
if (!string.IsNullOrEmpty(Bridge))
{
@@ -200,5 +265,61 @@ namespace PSProxmoxVE.Cmdlets.Vms
WriteObject(task);
}
private bool HasDiskOptions() =>
!string.IsNullOrEmpty(DiskBus)
|| DiskIoThread.IsPresent
|| !string.IsNullOrEmpty(DiskAio)
|| DiskSsd.IsPresent
|| DiskDiscard.IsPresent
|| !string.IsNullOrEmpty(DiskCache);
/// <summary>
/// Validates disk option combinations that PVE would otherwise reject at VM start
/// rather than at create time, surfacing a clear error up front.
/// </summary>
private void ValidateDiskOptions(string diskBus)
{
if (DiskSsd.IsPresent && diskBus == "virtio")
throw new PSArgumentException(
"-DiskSsd is not supported on the virtio bus. Use -DiskBus scsi, sata, or ide.",
nameof(DiskSsd));
if (DiskIoThread.IsPresent)
{
if (diskBus == "sata" || diskBus == "ide")
throw new PSArgumentException(
"-DiskIoThread requires -DiskBus virtio or scsi.",
nameof(DiskIoThread));
if (diskBus == "scsi"
&& !string.Equals(ScsiHardware, "virtio-scsi-single", System.StringComparison.OrdinalIgnoreCase))
throw new PSArgumentException(
"-DiskIoThread on a scsi disk requires -ScsiHardware virtio-scsi-single.",
nameof(DiskIoThread));
}
}
/// <summary>
/// Builds the disk volume spec ("&lt;storage&gt;:&lt;sizeGiB&gt;[,opt=val...]") for the
/// primary disk, appending format and any requested IO options in a stable order.
/// </summary>
private string BuildDiskSpec(string storage, string sizeGib)
{
var sb = new System.Text.StringBuilder($"{storage}:{sizeGib}");
if (!string.IsNullOrEmpty(DiskFormat))
sb.Append($",format={DiskFormat}");
if (!string.IsNullOrEmpty(DiskCache))
sb.Append($",cache={DiskCache!.ToLowerInvariant()}");
if (!string.IsNullOrEmpty(DiskAio))
sb.Append($",aio={DiskAio!.ToLowerInvariant()}");
if (DiskSsd.IsPresent)
sb.Append(",ssd=1");
if (DiskDiscard.IsPresent)
sb.Append(",discard=on");
if (DiskIoThread.IsPresent)
sb.Append(",iothread=1");
return sb.ToString();
}
}
}
@@ -173,5 +173,57 @@ namespace PSProxmoxVE.Core.Tests.Models
Assert.Equal("8.8.8.8", config.Nameserver);
Assert.Equal("example.com", config.Searchdomain);
}
[Fact]
public void PveVmConfig_Deserialize_SurfacesScsiHardware()
{
var json = TestHelper.LoadFixture("pve9_vm_config.json");
var data = JObject.Parse(json)["data"];
Assert.NotNull(data);
var config = data.ToObject<PveVmConfig>();
Assert.NotNull(config);
Assert.Equal("virtio-scsi-single", config.ScsiHardware);
}
[Fact]
public void PveVmConfig_Deserialize_SurfacesEfiDiskAndTpm()
{
var json = @"{ ""efidisk0"": ""local-lvm:vm-100-disk-1,efitype=4m,size=528K"",
""tpmstate0"": ""local-lvm:vm-100-disk-2,size=4M,version=v2.0"" }";
var config = JObject.Parse(json).ToObject<PveVmConfig>();
Assert.NotNull(config);
Assert.Contains("efitype=4m", config!.EfiDisk0);
Assert.Contains("version=v2.0", config.TpmState0);
}
[Fact]
public void PveVmConfig_UnmappedKeys_LandInAdditionalProperties_AsNativeTypes()
{
// hostpci0 and numa0 are not typed properties; they must not be dropped.
var json = @"{ ""cores"": 2,
""hostpci0"": ""0000:01:00.0,pcie=1"",
""numa0"": ""cpus=0-1,memory=2048"" }";
var config = JObject.Parse(json).ToObject<PveVmConfig>();
Assert.NotNull(config);
Assert.Equal(2, config!.Cores); // typed property still works
Assert.True(config.AdditionalProperties.ContainsKey("hostpci0"));
Assert.Equal("0000:01:00.0,pcie=1", config.AdditionalProperties["hostpci0"]);
// Value must be a native type (string), never a Newtonsoft JToken (D013).
Assert.IsType<string>(config.AdditionalProperties["hostpci0"]);
Assert.DoesNotContain("Newtonsoft", config.AdditionalProperties["hostpci0"]!.GetType().FullName);
}
[Fact]
public void PveVmConfig_TypedKeys_DoNotLeakIntoAdditionalProperties()
{
var json = TestHelper.LoadFixture("pve9_vm_config.json");
var data = JObject.Parse(json)["data"];
var config = data!.ToObject<PveVmConfig>();
Assert.NotNull(config);
// scsihw and cores are typed → they must not also appear in the catch-all.
Assert.False(config!.AdditionalProperties.ContainsKey("scsihw"));
Assert.False(config.AdditionalProperties.ContainsKey("cores"));
}
}
}
@@ -168,4 +168,75 @@ Describe 'New-PveVm' {
Should -Not -Throw
}
}
Context 'Disk controller / IO parameter metadata' {
BeforeAll { $script:Cmd = Get-Command 'New-PveVm' }
It 'Should have <_> parameter' -ForEach @(
'DiskBus', 'ScsiHardware', 'DiskIoThread', 'DiskAio', 'DiskSsd', 'DiskDiscard', 'DiskCache'
) {
$script:Cmd.Parameters.ContainsKey($_) | Should -BeTrue
}
It 'DiskBus should have a ValidateSet of virtio, scsi, sata, ide' {
$vs = $script:Cmd.Parameters['DiskBus'].Attributes |
Where-Object { $_ -is [System.Management.Automation.ValidateSetAttribute] } |
Select-Object -First 1
$vs.ValidValues | Should -Contain 'virtio'
$vs.ValidValues | Should -Contain 'scsi'
$vs.ValidValues | Should -Contain 'sata'
$vs.ValidValues | Should -Contain 'ide'
}
It 'DiskIoThread and DiskSsd should be switch parameters' {
$script:Cmd.Parameters['DiskIoThread'].ParameterType | Should -Be ([System.Management.Automation.SwitchParameter])
$script:Cmd.Parameters['DiskSsd'].ParameterType | Should -Be ([System.Management.Automation.SwitchParameter])
}
}
Context 'Disk option validation' {
# Validation runs before ShouldProcess, so -WhatIf exercises it offline.
It 'Should reject -DiskSsd on the virtio bus' {
{ New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '32' -DiskSsd -WhatIf -ErrorAction Stop } |
Should -Throw '*virtio bus*'
}
It 'Should reject -DiskSsd on the default (virtio) bus when bus omitted' {
{ New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '32' -DiskSsd -WhatIf -ErrorAction Stop } |
Should -Throw '*virtio bus*'
}
It 'Should accept -DiskSsd on the scsi bus' {
{ New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '32' -DiskBus scsi -DiskSsd -WhatIf -ErrorAction Stop } |
Should -Not -Throw
}
It 'Should reject -DiskIoThread on the sata bus' {
{ New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '32' -DiskBus sata -DiskIoThread -WhatIf -ErrorAction Stop } |
Should -Throw '*requires -DiskBus virtio or scsi*'
}
It 'Should reject -DiskIoThread on scsi without -ScsiHardware virtio-scsi-single' {
{ New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '32' -DiskBus scsi -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
}
It 'Should accept -DiskIoThread on the default virtio bus' {
{ New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '32' -DiskIoThread -WhatIf -ErrorAction Stop } |
Should -Not -Throw
}
It 'Should accept a fully tuned scsi disk spec' {
{ New-PveVm -Node 'pve-node1' -DiskStorage 'local-lvm' -DiskSize '60' -DiskBus scsi `
-ScsiHardware 'virtio-scsi-single' -DiskIoThread -DiskAio native -DiskSsd -DiskDiscard `
-DiskCache none -WhatIf -ErrorAction Stop } |
Should -Not -Throw
}
}
}