From de99c964d2cb3513b3fda5e8541d80cd545d0576 Mon Sep 17 00:00:00 2001 From: Ankit Sharma Date: Thu, 9 Apr 2026 15:25:15 +0530 Subject: [PATCH] Added SetDiskSanPolicy dependency to enable writable JBOD disks on Windows --- .../WindowsDiskManagerTests.cs | 100 ++++++++++++++++++ .../VirtualClient.Core/DiskManager.cs | 3 + .../VirtualClient.Core/IDiskManager.cs | 9 ++ .../VirtualClient.Core/UnixDiskManager.cs | 9 ++ .../VirtualClient.Core/WindowsDiskManager.cs | 84 +++++++++++++++ .../SetDiskSanPolicyTests.cs | 71 +++++++++++++ .../SetDiskSanPolicy.cs | 58 ++++++++++ .../InMemoryDiskManager.cs | 9 ++ 8 files changed, 343 insertions(+) create mode 100644 src/VirtualClient/VirtualClient.Dependencies.UnitTests/SetDiskSanPolicyTests.cs create mode 100644 src/VirtualClient/VirtualClient.Dependencies/SetDiskSanPolicy.cs diff --git a/src/VirtualClient/VirtualClient.Core.UnitTests/WindowsDiskManagerTests.cs b/src/VirtualClient/VirtualClient.Core.UnitTests/WindowsDiskManagerTests.cs index 016b79eb0a..0af55ebed6 100644 --- a/src/VirtualClient/VirtualClient.Core.UnitTests/WindowsDiskManagerTests.cs +++ b/src/VirtualClient/VirtualClient.Core.UnitTests/WindowsDiskManagerTests.cs @@ -1670,6 +1670,106 @@ public async Task WindowsDiskManagerGetsTheExpectedDisks_Scenario1() actualDisks.ToList().ForEach(disk => Assert.IsTrue(disk.Volumes.Count() == 2)); } + [Test] + public async Task WindowsDiskManagerCallsTheExpectedDiskPartCommandsToSetSanPolicy() + { + this.testProcess.OnHasExited = () => true; + this.testProcess.OnStart = () => true; + + List expectedCommands = new List + { + "san", + "san policy=onlineall", + "exit" + }; + + List actualCommands = new List(); + + this.standardInput.BytesWritten += (sender, data) => + { + string input = data.ToString().Trim(); + actualCommands.Add(input); + + if (input == "san") + { + // Simulate a policy that is NOT already OnlineAll. + this.testProcess.StandardOutput.Append("SAN Policy : Offline Shared"); + } + else if (input.Contains("san policy=onlineall")) + { + this.testProcess.StandardOutput.Append("DiskPart successfully changed the SAN policy for the current operating system."); + } + else if (input == "exit") + { + // Expected + } + else + { + Assert.Fail($"Unexpected command called: {input}"); + } + }; + + await this.diskManager.SetSanPolicyAsync(CancellationToken.None).ConfigureAwait(false); + + Assert.IsNotEmpty(actualCommands); + Assert.AreEqual(expectedCommands.Count, actualCommands.Count); + CollectionAssert.AreEquivalent(expectedCommands, actualCommands); + } + + [Test] + public async Task WindowsDiskManagerSkipsSettingSanPolicyWhenItIsAlreadyOnlineAll() + { + this.testProcess.OnHasExited = () => true; + this.testProcess.OnStart = () => true; + + // Only "san" and "exit" — no "san policy=onlineall". + List expectedCommands = new List + { + "san", + "exit" + }; + + List actualCommands = new List(); + + this.standardInput.BytesWritten += (sender, data) => + { + string input = data.ToString().Trim(); + actualCommands.Add(input); + + if (input == "san") + { + // Simulate a policy that is already OnlineAll. + this.testProcess.StandardOutput.Append("SAN Policy : Online All"); + } + else if (input == "exit") + { + // Expected + } + else + { + Assert.Fail($"Unexpected command called: {input}"); + } + }; + + await this.diskManager.SetSanPolicyAsync(CancellationToken.None).ConfigureAwait(false); + + Assert.IsNotEmpty(actualCommands); + Assert.AreEqual(expectedCommands.Count, actualCommands.Count); + CollectionAssert.AreEquivalent(expectedCommands, actualCommands); + } + + [Test] + public void WindowsDiskManagerThrowsWhenSettingSanPolicyTimesOut() + { + this.testProcess.OnHasExited = () => true; + this.testProcess.OnStart = () => true; + + // Do not write any response to standard output — the WaitForResponseAsync will time out. + this.standardInput.BytesWritten += (sender, data) => { }; + + Assert.ThrowsAsync(() => this.diskManager.SetSanPolicyAsync(CancellationToken.None)); + } + private class TestWindowsDiskManager : WindowsDiskManager { public TestWindowsDiskManager(ProcessManager processManager) diff --git a/src/VirtualClient/VirtualClient.Core/DiskManager.cs b/src/VirtualClient/VirtualClient.Core/DiskManager.cs index 59f9c3f890..06cb55c54c 100644 --- a/src/VirtualClient/VirtualClient.Core/DiskManager.cs +++ b/src/VirtualClient/VirtualClient.Core/DiskManager.cs @@ -45,5 +45,8 @@ protected DiskManager(ILogger logger = null) /// public abstract Task> GetDisksAsync(CancellationToken cancellationToken); + + /// + public abstract Task SetSanPolicyAsync(CancellationToken cancellationToken); } } diff --git a/src/VirtualClient/VirtualClient.Core/IDiskManager.cs b/src/VirtualClient/VirtualClient.Core/IDiskManager.cs index 12aaa07050..ae5e360a7c 100644 --- a/src/VirtualClient/VirtualClient.Core/IDiskManager.cs +++ b/src/VirtualClient/VirtualClient.Core/IDiskManager.cs @@ -38,5 +38,14 @@ public interface IDiskManager /// /// A token that can be used to cancel the operation. Task> GetDisksAsync(CancellationToken cancellationToken); + + /// + /// Sets the SAN (Storage Area Network) policy so that newly discovered disks are brought + /// online and writable rather than left offline or read-only. On Windows this runs the + /// DiskPart command san policy=onlineall, which prevents JBOD disks from being + /// marked read-only by the OS. This operation is a no-op on Linux. + /// + /// A token that can be used to cancel the operation. + Task SetSanPolicyAsync(CancellationToken cancellationToken); } } diff --git a/src/VirtualClient/VirtualClient.Core/UnixDiskManager.cs b/src/VirtualClient/VirtualClient.Core/UnixDiskManager.cs index f6d5608efe..3359637e6d 100644 --- a/src/VirtualClient/VirtualClient.Core/UnixDiskManager.cs +++ b/src/VirtualClient/VirtualClient.Core/UnixDiskManager.cs @@ -81,6 +81,15 @@ public override Task CreateMountPointAsync(DiskVolume volume, string mountPoint, }); } + /// + /// SAN policy is a Windows-only concept. This operation is a no-op on Linux/Unix. + /// + /// A token that can be used to cancel the operation. + public override Task SetSanPolicyAsync(CancellationToken cancellationToken) + { + return Task.CompletedTask; + } + /// /// Partitions and formats the disk for file system operations. /// diff --git a/src/VirtualClient/VirtualClient.Core/WindowsDiskManager.cs b/src/VirtualClient/VirtualClient.Core/WindowsDiskManager.cs index 1e51fc7bed..491676b431 100644 --- a/src/VirtualClient/VirtualClient.Core/WindowsDiskManager.cs +++ b/src/VirtualClient/VirtualClient.Core/WindowsDiskManager.cs @@ -151,6 +151,90 @@ await process.WriteInput(command) }); } + /// + /// Sets the SAN policy to onlineall so that newly discovered JBOD disks are brought + /// online and writable rather than remaining offline or read-only. + /// + /// A token that can be used to cancel the operation. + public override Task SetSanPolicyAsync(CancellationToken cancellationToken) + { + EventContext context = EventContext.Persisted(); + + return this.Logger.LogMessageAsync($"{nameof(WindowsDiskManager)}.SetSanPolicy", context, async () => + { + string command = string.Empty; + int retries = -1; + + try + { + await this.RetryPolicy.ExecuteAsync(async () => + { + retries++; + if (!cancellationToken.IsCancellationRequested) + { + using (IProcessProxy process = this.ProcessManager.CreateProcess("DiskPart", string.Empty)) + { + try + { + process.Interactive(); + if (!process.Start()) + { + throw new ProcessException("Failed to enter DiskPart session.", ErrorReason.DiskFormatFailed); + } + + // Query the current SAN policy first. + command = "san"; + await process.WriteInput(command) + .WaitForResponseAsync(@"SAN Policy\s*:", cancellationToken, timeout: TimeSpan.FromSeconds(30)) + .ConfigureAwait(false); + + string sanOutput = process.StandardOutput.ToString(); + this.Logger.LogTraceMessage($"Current SAN policy output: {sanOutput}", context); + + // Only set the policy if it is not already OnlineAll. + // DiskPart reports the policy in the format: "SAN Policy : Online All" + if (Regex.IsMatch(sanOutput, @"SAN Policy\s*:\s*Online All", RegexOptions.IgnoreCase)) + { + this.Logger.LogTraceMessage("SAN policy is already set to OnlineAll. No change required.", context); + } + else + { + // Set SAN policy to OnlineAll so that newly discovered disks are + // brought online and writable instead of remaining offline/read-only. + command = "san policy=onlineall"; + await process.WriteInput(command) + .WaitForResponseAsync(@"DiskPart successfully changed the SAN policy for the current operating system\.", cancellationToken, timeout: TimeSpan.FromSeconds(30)) + .ConfigureAwait(false); + + this.Logger.LogTraceMessage("SAN policy set to OnlineAll.", context); + } + } + catch (TimeoutException exc) + { + throw new ProcessException( + $"Failed to set SAN policy. DiskPart command timed out (command={command}, retries={retries}). {Environment.NewLine}{process.StandardOutput}", + exc, + ErrorReason.DiskFormatFailed); + } + finally + { + process.WriteInput("exit"); + await Task.Delay(this.WaitTime).ConfigureAwait(false); + context.AddProcessDetails(process.ToProcessDetails("diskpart"), "diskpartProcess"); + } + } + } + }).ConfigureAwait(false); + } + catch (Win32Exception exc) when (exc.Message.Contains("requires elevation")) + { + throw new ProcessException( + $"Requires elevated permissions. The current operation set requires the application to be run with administrator privileges.", + ErrorReason.Unauthorized); + } + }); + } + /// /// Partitions and formats the disk for file system operations. /// diff --git a/src/VirtualClient/VirtualClient.Dependencies.UnitTests/SetDiskSanPolicyTests.cs b/src/VirtualClient/VirtualClient.Dependencies.UnitTests/SetDiskSanPolicyTests.cs new file mode 100644 index 0000000000..abc3423e30 --- /dev/null +++ b/src/VirtualClient/VirtualClient.Dependencies.UnitTests/SetDiskSanPolicyTests.cs @@ -0,0 +1,71 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace VirtualClient.Dependencies +{ + using System; + using System.Collections.Generic; + using System.Threading; + using System.Threading.Tasks; + using Moq; + using NUnit.Framework; + using VirtualClient.Contracts; + + [TestFixture] + [Category("Unit")] + public class SetDiskSanPolicyTests + { + private MockFixture mockFixture; + + [Test] + public async Task SetDiskSanPolicyCallsDiskManagerSetSanPolicyOnWindows() + { + this.mockFixture = new MockFixture(); + this.mockFixture.Setup(PlatformID.Win32NT); + + using (SetDiskSanPolicy component = new SetDiskSanPolicy(this.mockFixture.Dependencies, this.mockFixture.Parameters)) + { + await component.ExecuteAsync(CancellationToken.None); + + this.mockFixture.DiskManager.Verify( + mgr => mgr.SetSanPolicyAsync(It.IsAny()), + Times.Once); + } + } + + [Test] + public async Task SetDiskSanPolicyDoesNotCallDiskManagerSetSanPolicyOnLinux() + { + this.mockFixture = new MockFixture(); + this.mockFixture.Setup(PlatformID.Unix); + + using (SetDiskSanPolicy component = new SetDiskSanPolicy(this.mockFixture.Dependencies, this.mockFixture.Parameters)) + { + await component.ExecuteAsync(CancellationToken.None); + + this.mockFixture.DiskManager.Verify( + mgr => mgr.SetSanPolicyAsync(It.IsAny()), + Times.Never); + } + } + + [Test] + public void SetDiskSanPolicyPropagatesExceptionsThrownByDiskManagerOnWindows() + { + this.mockFixture = new MockFixture(); + this.mockFixture.Setup(PlatformID.Win32NT); + + this.mockFixture.DiskManager + .Setup(mgr => mgr.SetSanPolicyAsync(It.IsAny())) + .ThrowsAsync(new ProcessException("DiskPart SAN policy command failed.", ErrorReason.DiskFormatFailed)); + + using (SetDiskSanPolicy component = new SetDiskSanPolicy(this.mockFixture.Dependencies, this.mockFixture.Parameters)) + { + ProcessException exc = Assert.ThrowsAsync( + () => component.ExecuteAsync(CancellationToken.None)); + + Assert.AreEqual(ErrorReason.DiskFormatFailed, exc.Reason); + } + } + } +} diff --git a/src/VirtualClient/VirtualClient.Dependencies/SetDiskSanPolicy.cs b/src/VirtualClient/VirtualClient.Dependencies/SetDiskSanPolicy.cs new file mode 100644 index 0000000000..84517d1415 --- /dev/null +++ b/src/VirtualClient/VirtualClient.Dependencies/SetDiskSanPolicy.cs @@ -0,0 +1,58 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +namespace VirtualClient.Dependencies +{ + using System; + using System.Collections.Generic; + using System.Threading; + using System.Threading.Tasks; + using Microsoft.Extensions.DependencyInjection; + using VirtualClient.Common.Extensions; + using VirtualClient.Common.Telemetry; + using VirtualClient.Contracts; + + /// + /// A dependency that sets the Windows SAN (Storage Area Network) policy to OnlineAll + /// so that newly discovered JBOD disks are brought online and writable rather than being + /// left offline or marked as read-only by the operating system. + /// + /// + /// On Windows, disks discovered through SAN controllers (including JBOD configurations) + /// are sometimes left offline or marked read-only depending on the SAN policy in effect. + /// Running the DiskPart commands san followed by san policy=onlineall configures + /// Windows to automatically bring all newly discovered disks online and writable. + /// This dependency is a no-op on Linux. + /// + public class SetDiskSanPolicy : VirtualClientComponent + { + private ISystemManagement systemManagement; + + /// + /// Initializes a new instance of the class. + /// + public SetDiskSanPolicy(IServiceCollection dependencies, IDictionary parameters) + : base(dependencies, parameters) + { + this.systemManagement = this.Dependencies.GetService(); + } + + /// + /// Executes the DiskPart SAN policy change. Only runs on Windows; skipped on Linux. + /// + protected override async Task ExecuteAsync(EventContext telemetryContext, CancellationToken cancellationToken) + { + if (this.Platform == PlatformID.Win32NT) + { + await this.systemManagement.DiskManager.SetSanPolicyAsync(cancellationToken) + .ConfigureAwait(false); + } + else + { + this.Logger.LogTraceMessage( + $"{nameof(SetDiskSanPolicy)}: SAN policy is a Windows-only concept. Skipping on platform '{this.Platform}'.", + telemetryContext); + } + } + } +} diff --git a/src/VirtualClient/VirtualClient.TestFramework/InMemoryDiskManager.cs b/src/VirtualClient/VirtualClient.TestFramework/InMemoryDiskManager.cs index 7af71c8bf7..6c614cfb45 100644 --- a/src/VirtualClient/VirtualClient.TestFramework/InMemoryDiskManager.cs +++ b/src/VirtualClient/VirtualClient.TestFramework/InMemoryDiskManager.cs @@ -129,6 +129,15 @@ public Task> GetDisksAsync(CancellationToken cancellationToken return Task.FromResult((IEnumerable)this); } + /// + /// No-op in the test/in-memory disk manager. SAN policy changes are Windows-only. + /// + /// A token that can be used to cancel the operation. + public Task SetSanPolicyAsync(CancellationToken cancellationToken) + { + return Task.CompletedTask; + } + private void AddVolumeToDisk(Disk disk, FileSystemType fileSystemType) { DiskVolume newVolume = null;