From 580f0cd350c8740145eeee84ea93c40a722714c1 Mon Sep 17 00:00:00 2001 From: "Olof Lagerkvist (LTRData)" Date: Sun, 8 Nov 2026 16:11:46 +0100 Subject: [PATCH] iSCSI: honor unconditional sparse-stream durability flushes --- .../DiscUtils.Iscsi/DiscUtils.Iscsi.csproj | 4 + Library/DiscUtils.Iscsi/Disk.cs | 4 +- Library/DiscUtils.Iscsi/DiskStream.cs | 73 ++++--------------- Library/DiscUtils.Iscsi/README.md | 6 ++ .../Iscsi/ScsiSynchronizeCacheTest.cs | 58 +++++++++++++++ 5 files changed, 84 insertions(+), 61 deletions(-) create mode 100644 Tests/LibraryTests/Iscsi/ScsiSynchronizeCacheTest.cs diff --git a/Library/DiscUtils.Iscsi/DiscUtils.Iscsi.csproj b/Library/DiscUtils.Iscsi/DiscUtils.Iscsi.csproj index 484acbe48..b1344c2da 100644 --- a/Library/DiscUtils.Iscsi/DiscUtils.Iscsi.csproj +++ b/Library/DiscUtils.Iscsi/DiscUtils.Iscsi.csproj @@ -9,4 +9,8 @@ + + + + diff --git a/Library/DiscUtils.Iscsi/Disk.cs b/Library/DiscUtils.Iscsi/Disk.cs index 7d46b9775..45b8a7be8 100644 --- a/Library/DiscUtils.Iscsi/Disk.cs +++ b/Library/DiscUtils.Iscsi/Disk.cs @@ -180,7 +180,7 @@ protected override void Dispose(bool disposing) { try { - _stream?.Flush(); + _stream?.Flush(flushToDisk: true); } catch (Exception e) when (e is IOException or ObjectDisposedException) { @@ -189,4 +189,4 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } -} \ No newline at end of file +} diff --git a/Library/DiscUtils.Iscsi/DiskStream.cs b/Library/DiscUtils.Iscsi/DiskStream.cs index 18754421f..9a505ace4 100644 --- a/Library/DiscUtils.Iscsi/DiskStream.cs +++ b/Library/DiscUtils.Iscsi/DiskStream.cs @@ -39,12 +39,6 @@ internal class DiskStream : SparseStream private long _position; private readonly Session _session; - // Set by a write and cleared by a flush, so a flush with nothing written costs no round trip. - private bool _unflushedWrites; - - // The target rejected SYNCHRONIZE CACHE as an unknown command: it has no volatile cache to flush. - private bool _noSynchronizeCache; - public DiskStream(Session session, long lun, FileAccess access) { _session = session; @@ -78,60 +72,23 @@ public override long Position } /// - /// Makes the target put the writes it acknowledged on stable storage (SCSI SYNCHRONIZE CACHE). A target may - /// acknowledge a write while it is still in a volatile cache; until this returns, a power loss on the target can - /// lose it or keep a later write without an earlier one. Does nothing if nothing was written since the last flush. + /// Requests that the target synchronize its write cache with stable storage (SCSI SYNCHRONIZE CACHE). + /// This covers the entire LUN, including writes made through other streams or before this stream was opened. + /// A failed or unsupported synchronization is reported to the caller. /// - public override void Flush() - { - if (!_unflushedWrites) - { - return; - } - - if (!_noSynchronizeCache) - { - try - { - _session.SynchronizeCache(_lun); - } - catch (ScsiCommandException e) when (IsUnknownCommand(e)) - { - _noSynchronizeCache = true; - } - } + public override void Flush() => Flush(flushToDisk: true); - _unflushedWrites = false; - } + /// + /// Requests a target-wide cache synchronization. iSCSI has no separate buffered data in this stream to flush, + /// so both overloads preserve the established behavior and issue SYNCHRONIZE CACHE. + /// In particular, set to true is an unconditional durability barrier, + /// even if this stream has not written any data itself. + /// + public override void Flush(bool flushToDisk) => _session.SynchronizeCache(_lun); - /// + /// Asynchronously synchronizes the whole LUN, even if this stream has not written anything. public override async Task FlushAsync(CancellationToken cancellationToken) - { - if (!_unflushedWrites) - { - return; - } - - if (!_noSynchronizeCache) - { - try - { - await _session.SynchronizeCacheAsync(_lun, cancellationToken).ConfigureAwait(false); - } - catch (ScsiCommandException e) when (IsUnknownCommand(e)) - { - _noSynchronizeCache = true; - } - } - - _unflushedWrites = false; - } - - // ILLEGAL REQUEST / INVALID COMMAND OPERATION CODE: a target without a write cache need not implement the command. - private static bool IsUnknownCommand(ScsiCommandException e) - => e.TryGetSenseData(out var sense) - && sense.SenseKey == ScsiSenseKey.IllegalRequest - && sense.AdditionalSenseCode == 0x20; + => await _session.SynchronizeCacheAsync(_lun, cancellationToken).ConfigureAwait(false); public override int Read(byte[] buffer, int offset, int count) => Read(buffer.AsSpan(offset, count)); @@ -232,7 +189,6 @@ public override void Write(ReadOnlySpan buffer) ? _session.FirstBurstLength : (_session.ActiveConnection.MaxTargetReceiveDataSegmentLength ?? _session.ActiveConnection.MaxInitiatorTransmitDataSegmentLength); - _unflushedWrites = true; var numWritten = 0; while (numWritten < buffer.Length) @@ -281,7 +237,6 @@ public override async ValueTask WriteAsync(ReadOnlyMemory buffer, Cancella ? _session.FirstBurstLength : (_session.ActiveConnection.MaxTargetReceiveDataSegmentLength ?? _session.ActiveConnection.MaxInitiatorTransmitDataSegmentLength); - _unflushedWrites = true; var numWritten = 0; while (numWritten < buffer.Length) @@ -304,4 +259,4 @@ public override async ValueTask WriteAsync(ReadOnlyMemory buffer, Cancella } } } -} \ No newline at end of file +} diff --git a/Library/DiscUtils.Iscsi/README.md b/Library/DiscUtils.Iscsi/README.md index 51ec60960..95815b634 100644 --- a/Library/DiscUtils.Iscsi/README.md +++ b/Library/DiscUtils.Iscsi/README.md @@ -22,6 +22,12 @@ dotnet add package LTRData.DiscUtils.Iscsi Initiator and Session provide explicit connections and LUN access. The package also supplies an iscsi transport for generic disk opening. This is a client library, not an iSCSI server or an operating-system initiator driver. Registration is AOT-compatible; protocol-key reflection elsewhere in iSCSI is outside that guarantee. +### Write-cache synchronization + +`Disk.Content.Flush()` and `Disk.Content.Flush(true)` issue SCSI `SYNCHRONIZE CACHE (10)` for the entire LUN, with `IMMED` clear. The request is sent even when the current stream has not written anything: a stream-local dirty flag cannot establish whether the target has outstanding cached writes from an earlier session or another stream. `FlushAsync()` requests the same synchronization. + +An unsupported command (including SCSI `ILLEGAL REQUEST` / ASC `0x20`) or other synchronization failure is reported to the caller. An unsupported command does **not** establish that the target has no volatile write cache. The `Disk` disposal flush remains best-effort and suppresses errors; applications that need a durability barrier must call and check `Content.Flush(true)` explicitly before disposal. Even a successful command depends on the target and backing storage honoring SCSI cache synchronization. + ## Registration For automatic discovery, register the providers implemented by this package: diff --git a/Tests/LibraryTests/Iscsi/ScsiSynchronizeCacheTest.cs b/Tests/LibraryTests/Iscsi/ScsiSynchronizeCacheTest.cs new file mode 100644 index 000000000..240a495c5 --- /dev/null +++ b/Tests/LibraryTests/Iscsi/ScsiSynchronizeCacheTest.cs @@ -0,0 +1,58 @@ +// +// Copyright (c) 2026, DiscUtils contributors +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the "Software"), +// to deal in the Software without restriction, including without limitation +// the rights to use, copy, modify, merge, publish, distribute, sublicense, +// and/or sell copies of the Software, and to permit persons to whom the +// Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. +// + +using System; +using DiscUtils.Iscsi; + +namespace LibraryTests.Iscsi; + +public class ScsiSynchronizeCacheTest +{ + [Fact] + public void SynchronizeCache10RequestsWholeLunAndWaitsForCompletion() + { + var command = new ScsiSynchronizeCache10Command(0x123456); + var bytes = new byte[command.Size]; + for (var i = 0; i < bytes.Length; i++) + { + bytes[i] = 0xFF; + } + + command.WriteTo(bytes); + + Assert.Equal(10, command.Size); + Assert.Equal((ulong)0x123456, command.TargetLun); + Assert.Equal(TaskAttributes.Simple, command.TaskAttributes); + // LBA 0, number of blocks 0: all blocks through the end of the LUN. + // IMMED=0: do not acknowledge before synchronization finishes. + Assert.Equal(new byte[] { 0x35, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, bytes); + } + + [Fact] + public void DiskStreamOverridesTheStrongSparseFlushOverload() + { + var flush = typeof(DiskStream).GetMethod(nameof(DiskStream.Flush), new[] { typeof(bool) }); + + Assert.NotNull(flush); + Assert.Equal(typeof(DiskStream), flush.DeclaringType); + } +}