fix: normalize -DiskSize and -RootFsSize before sending to PVE

Disk and rootfs size strings were interpolated directly into the disk
spec as "<storage>:<size>", so "60G" produced "local-lvm:60G". On
LVM/LVM-thin storages PVE parses the value after the colon as a volume
name unless it is a bare integer, returning "unable to parse lvm volume
name '60G'". File-backed storages mask this by accepting either form.

SizeParser.NormalizeToGibibytes() now strips G/GB/T/TB suffixes and
returns a bare GiB integer string, so the documented "32G" call shape
works on every storage type. Sub-GB units are rejected with a clear
error rather than being silently truncated.

Tracked as F086. Closes #58.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Clint Branham
2026-05-20 17:52:44 -05:00
parent f2925ba49f
commit 6181c8ce77
5 changed files with 204 additions and 6 deletions
+31
View File
@@ -2815,6 +2815,37 @@
"evidence": "Replaced JArray? Nodelist with List<Dictionary<string, object?>>? + NativeListConverter, and JObject? Totem with Dictionary<string, object?>? + NativeDictionaryConverter. Removed Newtonsoft.Json.Linq dependency.",
"verified_by": "dotnet build + dotnet test (382 passed)"
}
},
{
"id": "F086",
"title": "New-PveVm -DiskSize and New-PveContainer -RootFsSize pass unit suffix verbatim, LVM rejects 'NG'",
"category": "api_contract",
"severity": "high",
"status": "resolved",
"first_detected": "2026-05-20",
"github_issue": 58,
"files": [
"src/PSProxmoxVE/Cmdlets/Vms/NewPveVmCmdlet.cs",
"src/PSProxmoxVE/Cmdlets/Containers/NewPveContainerCmdlet.cs"
],
"description": "Disk and rootfs sizes are interpolated directly into the disk spec as '<storage>:<size>'. The parameter docstring advertises 'e.g. 32G' but on LVM/LVM-thin storages PVE parses the value after the colon as a volume name unless it is a bare integer, failing with 'unable to parse lvm volume name \"32G\"'. File-backed storages (NFS, directory) accept either form, masking the bug in mixed environments.",
"scan_history": [
{
"scan_date": "2026-05-20",
"local_id": null,
"status": "new"
},
{
"scan_date": "2026-05-20",
"local_id": null,
"status": "fixed"
}
],
"resolution": {
"scan_date": "2026-05-20",
"evidence": "Added SizeParser.NormalizeToGibibytes() which strips G/GB/T/TB suffixes (and rejects sub-GB units with a clear error). New-PveVm and New-PveContainer normalize -DiskSize and -RootFsSize through it before constructing the disk spec, so '32G' becomes '<storage>:32' on every storage type.",
"verified_by": "dotnet build + dotnet test (576 passed, 31 new SizeParserTests)"
}
}
]
}
@@ -0,0 +1,73 @@
using System;
using System.Globalization;
using System.Text.RegularExpressions;
namespace PSProxmoxVE.Core.Utilities
{
/// <summary>
/// Parses storage size strings (e.g. "32G", "1T", "60") and normalizes them
/// to a bare integer count of gibibytes for use in PVE disk specs.
/// </summary>
/// <remarks>
/// PVE accepts a size suffix on file-backed storages (NFS, directory) but parses
/// the value after the colon as a volume name on LVM-backed storages — so
/// <c>local-lvm:32G</c> fails with "unable to parse lvm volume name '32G'" while
/// <c>local-lvm:32</c> works on every storage type. Cmdlets that build disk specs
/// must normalize size inputs through this helper before joining with the storage.
/// </remarks>
public static class SizeParser
{
private static readonly Regex Pattern = new Regex(
@"^\s*(?<num>\d+)\s*(?<unit>[A-Za-z]*)\s*$",
RegexOptions.Compiled);
/// <summary>
/// Parses a size string and returns the value as a bare integer count of GiB.
/// Accepts values like "60", "60G", "60GB" (= 60), "1T", "1TB" (= 1024).
/// Sub-GB units are rejected because PVE disk allocation is GB-granular.
/// </summary>
/// <param name="value">The size string supplied by the user.</param>
/// <param name="parameterName">Parameter name used in the error message.</param>
/// <returns>The size in whole GiB as a string, suitable for direct use in disk specs.</returns>
/// <exception cref="ArgumentException">The input cannot be parsed or uses an unsupported unit.</exception>
public static string NormalizeToGibibytes(string value, string parameterName = "size")
{
if (string.IsNullOrWhiteSpace(value))
throw new ArgumentException($"{parameterName} must not be null or empty.", parameterName);
var match = Pattern.Match(value);
if (!match.Success)
throw new ArgumentException(
$"{parameterName} '{value}' is not a valid size. Expected a positive integer optionally suffixed with G, GB, T, or TB (e.g. '32G', '1T', '60').",
parameterName);
if (!long.TryParse(match.Groups["num"].Value, NumberStyles.Integer, CultureInfo.InvariantCulture, out var num) || num <= 0)
throw new ArgumentException(
$"{parameterName} '{value}' must be a positive integer.",
parameterName);
var unit = match.Groups["unit"].Value.ToUpperInvariant();
long gib;
switch (unit)
{
case "":
case "G":
case "GB":
case "GIB":
gib = num;
break;
case "T":
case "TB":
case "TIB":
gib = checked(num * 1024L);
break;
default:
throw new ArgumentException(
$"{parameterName} '{value}' uses unsupported unit '{unit}'. Use G, GB, T, or TB. Sub-GB units (M, MB, K, KB) are not supported by PVE disk allocation.",
parameterName);
}
return gib.ToString(CultureInfo.InvariantCulture);
}
}
}
@@ -6,6 +6,7 @@ using Newtonsoft.Json.Linq;
using PSProxmoxVE.Core.Client;
using PSProxmoxVE.Core.Models.Vms;
using PSProxmoxVE.Core.Services;
using PSProxmoxVE.Core.Utilities;
namespace PSProxmoxVE.Cmdlets.Containers
{
@@ -59,9 +60,13 @@ namespace PSProxmoxVE.Cmdlets.Containers
public int? Cores { get; set; }
/// <summary>
/// <para type="description">Size of the root filesystem (e.g., "8G").</para>
/// <para type="description">
/// Size of the root filesystem. Accepts a bare integer in GiB ("8") or a value
/// suffixed with G/GB/T/TB (case-insensitive); the value is normalized to a
/// bare GiB count before being sent to the API.
/// </para>
/// </summary>
[Parameter(Mandatory = false, HelpMessage = "Size of the root filesystem (e.g. 8G).")]
[Parameter(Mandatory = false, HelpMessage = "Size of the root filesystem in GiB (e.g. 8 or 8G).")]
public string? RootFsSize { get; set; }
/// <summary>
@@ -161,7 +166,10 @@ namespace PSProxmoxVE.Cmdlets.Containers
{
var rootFsValue = RootFsStorage!;
if (!string.IsNullOrEmpty(RootFsSize))
rootFsValue += $":{RootFsSize}";
{
var sizeGib = SizeParser.NormalizeToGibibytes(RootFsSize!, nameof(RootFsSize));
rootFsValue += $":{sizeGib}";
}
config["rootfs"] = rootFsValue;
}
@@ -4,6 +4,7 @@ using Newtonsoft.Json.Linq;
using PSProxmoxVE.Core.Client;
using PSProxmoxVE.Core.Models.Vms;
using PSProxmoxVE.Core.Services;
using PSProxmoxVE.Core.Utilities;
namespace PSProxmoxVE.Cmdlets.Vms
{
@@ -74,9 +75,13 @@ namespace PSProxmoxVE.Cmdlets.Vms
public string? Machine { get; set; }
/// <summary>
/// <para type="description">Size of the primary disk (e.g., "32G").</para>
/// <para type="description">
/// Size of the primary disk. Accepts a bare integer in GiB ("32") or a value
/// suffixed with G/GB/T/TB (case-insensitive); the value is normalized to a
/// bare GiB count before being sent to the API.
/// </para>
/// </summary>
[Parameter(Mandatory = false, HelpMessage = "Size of the primary disk (e.g. 32G).")]
[Parameter(Mandatory = false, HelpMessage = "Size of the primary disk in GiB (e.g. 32 or 32G).")]
public string? DiskSize { get; set; }
/// <summary>
@@ -163,7 +168,8 @@ namespace PSProxmoxVE.Cmdlets.Vms
if (!string.IsNullOrEmpty(DiskStorage) && !string.IsNullOrEmpty(DiskSize))
{
var diskValue = $"{DiskStorage}:{DiskSize}";
var sizeGib = SizeParser.NormalizeToGibibytes(DiskSize!, nameof(DiskSize));
var diskValue = $"{DiskStorage}:{sizeGib}";
if (!string.IsNullOrEmpty(DiskFormat))
diskValue += $",format={DiskFormat}";
config["virtio0"] = diskValue;
@@ -0,0 +1,80 @@
using System;
using PSProxmoxVE.Core.Utilities;
using Xunit;
namespace PSProxmoxVE.Core.Tests.Utilities
{
public class SizeParserTests
{
[Theory]
[InlineData("32", "32")]
[InlineData("32G", "32")]
[InlineData("32g", "32")]
[InlineData("32GB", "32")]
[InlineData("32gb", "32")]
[InlineData("32GiB", "32")]
[InlineData("1T", "1024")]
[InlineData("1t", "1024")]
[InlineData("1TB", "1024")]
[InlineData("1TiB", "1024")]
[InlineData("2T", "2048")]
[InlineData(" 60G ", "60")]
[InlineData("60 G", "60")]
public void NormalizeToGibibytes_AcceptedInputs_ReturnsBareGibibyteString(string input, string expected)
{
var result = SizeParser.NormalizeToGibibytes(input);
Assert.Equal(expected, result);
}
[Theory]
[InlineData("512M")]
[InlineData("512MB")]
[InlineData("1024K")]
[InlineData("1024KB")]
[InlineData("100B")]
[InlineData("1P")]
[InlineData("1PB")]
public void NormalizeToGibibytes_UnsupportedUnit_Throws(string input)
{
var ex = Assert.Throws<ArgumentException>(() => SizeParser.NormalizeToGibibytes(input));
Assert.Contains("unsupported unit", ex.Message, StringComparison.OrdinalIgnoreCase);
}
[Theory]
[InlineData("")]
[InlineData(" ")]
[InlineData(null)]
public void NormalizeToGibibytes_EmptyOrWhitespace_Throws(string? input)
{
Assert.Throws<ArgumentException>(() => SizeParser.NormalizeToGibibytes(input!));
}
[Theory]
[InlineData("abc")]
[InlineData("G")]
[InlineData("-32")]
[InlineData("32.5G")]
[InlineData("32 G B")]
public void NormalizeToGibibytes_Malformed_Throws(string input)
{
Assert.Throws<ArgumentException>(() => SizeParser.NormalizeToGibibytes(input));
}
[Theory]
[InlineData("0")]
[InlineData("0G")]
public void NormalizeToGibibytes_Zero_Throws(string input)
{
var ex = Assert.Throws<ArgumentException>(() => SizeParser.NormalizeToGibibytes(input));
Assert.Contains("positive", ex.Message, StringComparison.OrdinalIgnoreCase);
}
[Fact]
public void NormalizeToGibibytes_UsesProvidedParameterNameInError()
{
var ex = Assert.Throws<ArgumentException>(() => SizeParser.NormalizeToGibibytes("512M", "DiskSize"));
Assert.Equal("DiskSize", ex.ParamName);
Assert.Contains("DiskSize", ex.Message);
}
}
}