Skip to content

Enable DiskSpdExecutor to run read-only diskspd workloads on raw physical disks#683

Open
ankitsharma-99 wants to merge 8 commits intomicrosoft:mainfrom
ankitsharma-99:users/ankitshar/diskspdOnBareDisk
Open

Enable DiskSpdExecutor to run read-only diskspd workloads on raw physical disks#683
ankitsharma-99 wants to merge 8 commits intomicrosoft:mainfrom
ankitsharma-99:users/ankitshar/diskspdOnBareDisk

Conversation

@ankitsharma-99
Copy link
Copy Markdown
Contributor

@ankitsharma-99 ankitsharma-99 commented Apr 10, 2026

Adds bare-disk (raw physical disk) I/O benchmarking support to DiskSpdExecutor, along with a new execution profile
PERF-IO-DISKSPD-RAWDISK targeting Windows systems with unformatted or raw data disks.

@ankitsharma-99 ankitsharma-99 marked this pull request as ready for review April 14, 2026 09:07
@ankitsharma-99 ankitsharma-99 changed the title Add raw/bare disk I/O support to DiskSpdExecutor (PERF-IO-DISKSPD-RAWDISK profile) Enable DiskSpdExecutor to run read-only diskspd workloads on raw physical disks Apr 14, 2026
@@ -0,0 +1,42 @@
{
"Description": "DiskSpd I/O Stress Performance Workload (Raw Disk / Bare Metal)",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The term "RAWDISK" is a weird name and can be confusing in some ways. What makes something "RAW"? Recommend renaming the profile to:

PERF-IO-DISKSPD-PHYSICAL-DISK.json

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I have updated the profile name.

/// When enabled the <see cref="DiskFill"/> step is skipped and no test file path is appended to
/// the DiskSpd command line — the device path is passed instead.
/// </summary>
public bool RawDiskTarget
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not clear to me why we cannot use the DiskFilter here for these scenarios and avoid changes to the DiskSpdExecutor as well as the creation of a new profile.

This seems like we should really be making additions to the "DiskFilter" capability vs. an implementation directly in a single workload such as DiskSpd. We already capture things like the device path in the disks info on Windows systems. It would seem like we can use "DiskFilter=DevicePath:.\.\PHYSICALDISK0" or something along those lines to get the desired outcome.

Use the following query to see the disk info we parse from Windows disks:
Traces
| where Timestamp >= ago(3d) and ProfileName == "PERF-IO-DISKSPD.json"
| where Message == "WindowsDiskManager.GetDisksStop"
| extend Disks = todynamic(CustomDimensions.disks)
| project Disks
| take 10

e.g.
"Disks": [
{
"properties": {
"type": "SAS",
"model": "Microsoft Virtual Disk",
"path": "0",
"status": "Online",
"index": 0,
"disk ID": "{AFD4524A-3E00-49BD-9DC8-2D2E7151D9DA}",
"target": "0",
"lun ID": "0",
"location Path": "ACPI(SB)#ACPI(VMOD)#ACPI(VMBS)#VMBUS({ba6163d9-04a1-4d29-b605-72e2ffb1dc7f}#{f8b3781a-1e82-4818-a1c3-63d806ec15bb})#SAS(P00T00L00)",
"current Read-only State": "No",
"read-only": "No",
"boot Disk": "Yes",
"pagefile Disk": "Yes",
"hibernation File Disk": "No",
"crashdump Disk": "Yes",
"clustered Disk": "No"
},
"index": 0,
"devicePath": "\\.\PHYSICALDISK0",
"volumes": [
{
"properties": {
"type": "e3c9e316-0b5c-4db8-817d-f92df00215ae",
"partitionIndex": "1",
"hidden": "Yes",
"required": "No",
"attrib": "0XC000000000000000",
"offset in Bytes": "1048576"
},
"devicePath": "",
"accessPaths": []
},
{
"properties": {
"type": "Partition",
"status": "Healthy",
"index": "2",
"partitionIndex": "2",
"hidden": "Yes",
"required": "No",
"attrib": "0000000000000000",
"offset in Bytes": "17825792",
"ltr": null,
"label": "Recovery",
"fs": "NTFS",
"size": "450 MB",
"info": "Hidden"
},
"index": 2,
"devicePath": "",
"accessPaths": []
},
{
"properties": {
"type": "Partition",
"status": "Healthy",
"index": "3",
"partitionIndex": "3",
"hidden": "Yes",
"required": "No",
"attrib": "0000000000000000",
"offset in Bytes": "489684992",
"ltr": null,
"label": null,
"fs": "FAT32",
"size": "99 MB",
"info": "System"
},
"index": 3,
"devicePath": "",
"accessPaths": []
},
{
"properties": {
"type": "Partition",
"status": "Healthy",
"index": "1",
"partitionIndex": "4",
"hidden": "No",
"required": "No",
"attrib": "0000000000000000",
"offset in Bytes": "593494016",
"ltr": "C",
"label": "Windows",
"fs": "NTFS",
"size": "126 GB",
"info": "Boot"
},
"index": 1,
"devicePath": "C:\",
"accessPaths": [
"C:\"
]
}
]
},
{
"properties": {
"type": "SAS",
"model": "Microsoft Virtual Disk",
"path": "0",
"status": "Online",
"index": 1,
"disk ID": "DD6925A4",
"target": "0",
"lun ID": "0",
"location Path": "ACPI(SB)#ACPI(VMOD)#ACPI(VMBS)#VMBUS({ba6163d9-04a1-4d29-b605-72e2ffb1dc7f}#{f8b3781b-1e82-4818-a1c3-63d806ec15bb})#SAS(P00T00L00)",
"current Read-only State": "No",
"read-only": "No",
"boot Disk": "No",
"pagefile Disk": "No",
"hibernation File Disk": "No",
"crashdump Disk": "No",
"clustered Disk": "No"
},
"index": 1,
"devicePath": "\\.\PHYSICALDISK1",
"volumes": [
{
"properties": {
"type": "Partition",
"status": "Healthy",
"index": "4",
"partitionIndex": "1",
"hidden": "No",
"offset in Bytes": "1048576",
"ltr": "E",
"label": null,
"fs": "NTFS",
"size": "2047 GB",
"info": null,
"active": "No"
},
"index": 4,
"devicePath": "E:\",
"accessPaths": [
"E:\"
]
}
]
}
]

Copy link
Copy Markdown
Contributor Author

@ankitsharma-99 ankitsharma-99 Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review. I spent some time exploring whether we could address this purely via DiskFilter (including DevicePath) and validated behavior directly on the storage headnode with the JBOD attached. A few observations that led me to the current proposal:

Executor changes are needed regardless of how disks are selected
DiskSpd uses different syntax for filesystem targets vs. physical disks (E:\file.dat vs #N). Today the execution path always calls GetPreferredAccessPath(), which throws for disks with no volumes (NumberOfPartitions = 0). This happens before DiskSpd starts, independent of DiskFilter (even with DevicePath), so we need changes in DiskSpdExecutor in either approach. Specifically, even with DiskFilter=DevicePath:..., there's no existing mechanism in the executor to know it should use #N (physical disk) syntax instead of a filesystem path - that's what RawDiskTarget provides.

Some valid JBOD disks are filtered out before DiskFilter applies
We currently drop IsReadOnly = True disks as a pre-filter. Many JBOD disks show up this way but are valid raw I/O targets. DiskFilter can't override this today; addressing it would require an explicit include/read-only capability.

GetDisksAsync() doesn't scale well for large JBODs
It relies on sequential DiskPart calls with a fixed delay per disk (~300ms × 2 per disk × ~180 disks ≈ 108 seconds of fixed latency before any I/O starts). Using Get-PhysicalDisk issues a single query and scales much better for this scenario.

Generalizing DiskFilter impacts shared infrastructure
Supporting things like MediaType requires changes to GetDisksAsync(), which has multiple callers (FormatDisks, MountDisks, etc.). That feels like a broader change best handled in a dedicated PR.

Separately, a new profile is still required. PERF-IO-DISKSPD.json is filesystem-oriented, and parameters like DiskFillSize/FileSize don't apply to physical disk access.

My proposal: merge this PR as a targeted change limited to DiskSpdExecutor to unblock on the immediate JBOD physical‑disk scenario, and track DiskFilter + GetDisksAsync() extensions as follow-up work so we can evolve that capability safely.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All very valid and fair points. I agree with the assessment while wanting to find a balanced solution. So as to maintain some amount of cohesion with "today's needs" vs. "framework consistency", I would like to propose a compromise that allows you to do what you are trying to do while also setting a pattern we can genericize in the future (when we get to it) so that the disk filter mechanics can be made to support this type of targeting as well. We may need the same support on Linux with FIO for example with some programs.

Compromise Proposal:

  1. Still use the "DiskFilter" parameter to support targeting specific disks (by index). Don't introduce new parameters to DiskSpd (e.g. RawDiskTarget and RawDiskIndexRange).
  2. Do not implement the support in the disk filter extensions. Just implement the support directly in the DiskSpdExecutor (as you are doing now).

e.g.
DiskFilter="DiskIndex:0"
DiskFilter="DiskIndex:0,1,2,3,4,5"
DiskFilter="DiskIndex:6-180"

The term "DiskIndex" is not currently known to the underlying disk filter code paths. It is thus safe to "stage it" ahead of time so that we can fully/properly integrate it in the future.

I would recommend adding the following method to the "DiskFilters" static class and then using this in the DiskSpdExecutor.

static class DiskFilters
{
public static bool TryGetDiskIndexes(string diskFilter, out IEnumerable indexes);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This in turn removes the need for the custom profile PERF-IO-DISKSPD-PHYSICAL-DISK.json. You can now just use the out-of-box PERF-IO-DISKSPD.json profile or place a variation of it in the internal/ADO repo.

e.g.
VirtualClient.exe --profile=PERF-IO-DISKSPD.json --parameters="DiskFilter=DiskIndex:6-108" --scenarios=XYZ

Copy link
Copy Markdown
Contributor Author

@ankitsharma-99 ankitsharma-99 Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the thoughtful compromise proposal. I agree with the direction, and I’ve implemented it exactly as suggested.

What was done:

  1. Added DiskFilters.TryGetDiskIndexes(string diskFilter, out IEnumerable indexes) to the DiskFilters static class, supporting:
    DiskIndex:0
    DiskIndex:0,1,2
    DiskIndex:6-180
    The method returns false for any filter not prefixed with DiskIndex:, keeping it invisible to existing DiskFilter code paths.

  2. Added support for DiskIndex:hdd as a sentinel value. In this case, TryGetDiskIndexes returns true with a null index set, signaling DiskSpdExecutor to auto‑discover JBOD HDDs via
    Get-PhysicalDisk | Where MediaType -eq 'HDD'.
    This covers the primary use case where physical disk count/indexes aren’t known ahead of time.

  3. Removed RawDiskTarget and RawDiskIndexRange entirely. DiskSpdExecutor now branches internally on TryGetDiskIndexes.

This effectively stages the DiskIndex: pattern for future generalization into the core disk filter framework (e.g., FIO on Linux), without committing to a full implementation today.

On retaining PERF-IO-DISKSPD-PHYSICAL-DISK.json:
I’d still recommend keeping this dedicated profile. The existing PERF-IO-DISKSPD.json profile includes a DiskFill action and a FormatDisks dependency, both of which are inappropriate (and potentially destructive) for physical/raw disk targets.
The dedicated profile:

Omits formatting and fill actions entirely
Includes only safe/read workloads
Sets DiskFilter=DiskIndex:hdd by default for automatic JBOD discovery

Users can still override this with an explicit range (e.g. --parameters="DiskFilter=DiskIndex:6-180") when the disk topology is known. This provides a safe, purpose‑built entry point for raw disk testing without requiring callers to carefully override multiple parameters from the base profile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants