From 4830aac41ce569e3aba5cc8e818edd0698bd83ca Mon Sep 17 00:00:00 2001 From: doccaz Date: Sat, 19 Sep 2026 10:39:32 -0300 Subject: [PATCH 1/4] Add VixDiskLib_QueryAllocatedBlocks; investigate NFC_DELTA_DISK VixDiskLib_QueryAllocatedBlocks (AIO type 13) returns which blocks within a disk are allocated (non-sparse), useful for skipping empty regions when reading a snapshot delta file. Reverse-engineered via an SSL/write-hook capture extended to a ctypes call to VixDiskLib_QueryAllocatedBlocks after Open, hitting and resolving two non-obvious wire-format bugs along the way: - Field-order swap: a first capture used startSector=0, which made two request fields (a reserved field and start_offset_bytes) both read as zero -- indistinguishable. Implementing from that single capture put start_offset_bytes at the wrong byte offset (8 instead of 24) and silently returned wrong (all-empty) results for every non-zero start. A second capture against a known-allocated region with a non-zero startSector broke the tie. - Bitmap padding: the reply's allocation bitmap is padded to a 4-byte boundary, not the raw ceil(chunk_count/8) an earlier draft assumed -- invisible for a chunk_count that's already a multiple of 4, but it under-read and desynced the connection for smaller chunk counts. Also investigated the "NFC_DELTA_DISK" backlog item (reading directly from a snapshot delta chain), expecting a distinct wire message. `strings` on libvixDiskLib.so found it's actually an alternate OPEN_FILE file-type value used by an internal VDDK client-side optimization for very sparse VMFS redo logs -- not a correctness requirement. Verified end-to-end that reading, writing, and query_allocated_blocks against an actual post-snapshot delta file all already work with the existing NFC_DISK-only implementation. Found and documented a real gotcha along the way: querying allocated blocks on the same still-open handle a write just went through can see stale (pre-write) data -- reproduced identically on native VDDK (two-process capture, since loading native VDDK in the same process as pyVmomi segfaults on this host's OpenSSL), so this is real server/VMFS behavior, not a client bug. Validated against a live standalone ESXi 8.0.3 host: full-disk query, a known-allocated sub-range, an aligned empty range, and a real snapshot delta file all match native VDDK's own output exactly. Adds unit tests for the bitmap decode/merge logic and input validation (no lab needed) plus integration tests for the live scenarios above. Full protocol details and both bugs are in docs/nfc_read.md; the NFC_DELTA_DISK investigation is in docs/reverse_engineering_procedure.md. --- README.md | 13 ++- docs/nfc_open.md | 8 +- docs/nfc_read.md | 75 ++++++++++++++- docs/reverse_engineering_procedure.md | 66 ++++++++++++- docs/ssl_hook.md | 12 +++ openvixdisklib/nfc_open.py | 115 +++++++++++++++++++++++ openvixdisklib/openvixdisklib.py | 21 +++++ tests/integration/test_openvixdisklib.py | 106 +++++++++++++++++++++ tests/unit/test_nfc_open.py | 66 +++++++++++++ 9 files changed, 472 insertions(+), 10 deletions(-) create mode 100644 tests/unit/test_nfc_open.py diff --git a/README.md b/README.md index 254794b..4d699e4 100644 --- a/README.md +++ b/README.md @@ -24,10 +24,17 @@ Implemented against vCenter 8 / ESXi 8. Default transport is `nbdssl` - `VixDiskLib_Open` (datastore path, read-only or read-write) - `VixDiskLib_Read` (optional ``skip_decompression`` packs FastLZ extras) - `VixDiskLib_Write` +- `VixDiskLib_QueryAllocatedBlocks` (allocated-block bitmap; see + `docs/nfc_read.md`) -Not implemented: compression open flags other than FastLZ, CBT / -allocated-block queries, disk geometry (`DDB_GET`), encrypted disks, -and direct ESXi `ha-nfc` without vCenter `vpxa-nfc`. +Reading/writing a snapshot delta file directly (and running +`query_allocated_blocks` against it) already works — `NFC_DELTA_DISK` +turned out to be an optional VMFS-only VDDK client optimization, not a +correctness requirement (see `docs/reverse_engineering_procedure.md`). + +Not implemented: compression open flags other than FastLZ, CBT, +disk geometry (`DDB_GET`), encrypted disks, and direct ESXi `ha-nfc` +without vCenter `vpxa-nfc`. Requires Python 3.10 or later. diff --git a/docs/nfc_open.md b/docs/nfc_open.md index bf61550..0d5e319 100644 --- a/docs/nfc_open.md +++ b/docs/nfc_open.md @@ -247,9 +247,15 @@ I/O: `docs/nfc_read.md`, `docs/nfc_write.md`, and ## What is still VDDK-only - `DDB_GET` / geometry / zlib and skipz compression / encryption keys -- `NFC_DELTA_DISK`, change-block tracking +- change-block tracking - Host-switch (`NFC_AIO_SWITCH_HOST_*`) - Direct ESXi `ha-nfc` without vCenter `vpxa-nfc` +Reading/writing a snapshot delta file directly, and running +`query_allocated_blocks` against it, both already work with the +existing implementation — `NFC_DELTA_DISK` turned out to be an +optional VMFS-only VDDK client optimization, not a correctness +requirement; see `docs/reverse_engineering_procedure.md`. + Reads after open are in `docs/nfc_read.md`. Writes are in `docs/nfc_write.md`. diff --git a/docs/nfc_read.md b/docs/nfc_read.md index ee67652..6f16e2b 100644 --- a/docs/nfc_read.md +++ b/docs/nfc_read.md @@ -189,9 +189,82 @@ uncompressed request (offsets in this read, not on disk) buf when skip_decompression=True: extras packed densely from offset 0 ``` +## `VixDiskLib_QueryAllocatedBlocks` (AIO type 13) + +Reverse-engineered by extending the SSL/write-hook capture (Step 13/14 +technique, `docs/reverse_engineering_procedure.md`) to a ctypes call to +`VixDiskLib_QueryAllocatedBlocks` after `Open`, first with +`startSector=0` then — after the first capture's field guesses turned +out wrong — again with a non-zero `startSector` against a known +already-allocated region, to disambiguate fields that are 0 in the +degenerate zero-start case. + +No SOAP or authd traffic; it is one more AIO message type in the +already-open NFC/AIO session (like `DDB_GET`). + +Request (48 bytes):: + + uint64 handle (from OPEN_FILE) + uint64 reserved (0) + uint64 chunk_size_bytes (chunk_size_sectors * sector_size) + uint64 start_offset_bytes (start_sector * sector_size) + uint64 chunk_count (num_sectors // chunk_size_sectors) + uint64 reserved (0) + +**Field-order pitfall:** `start_offset_bytes` is at byte offset 24, not +8 — offset 8 is a reserved/always-zero field. A capture with +`startSector=0` can't tell these two apart (both read 0); only a +capture with a non-zero start distinguishes them. A first +implementation attempt put `start_offset_bytes` at offset 8 and got +`chunk_count`-many all-zero bits back for every non-zero-start query, +even for byte ranges known (from a zero-start, full-range query) to be +allocated — the server was silently ignoring the offset the client +thought it was requesting and returning an artifact of a different +misread field. + +Reply: a 48-byte body (offset 32 echoes `chunk_count`) followed by a +bitmap extra, one bit per chunk (LSB-first, `1` = chunk has allocated +data), **padded up to a 4-byte boundary** — `ceil(chunk_count / 8)` +alone is correct only when that value is already a multiple of 4 +(true for the `chunk_count=16384` case tested first, which is why the +padding bug wasn't caught immediately; a `chunk_count=16` query +exposed it, since `ceil(16/8)=2` bytes under-reads the real 4-byte +reply and desyncs the connection — the *next* AIO reply's header then +reads as garbage). + +Both `start_sector` and `num_sectors` must be exact multiples of +`chunk_size_sectors`; the server returns an `NFC_AIO_MSG_ERROR` (type +1) reply otherwise (hit by accident during validation with a +non-aligned `start_sector`). + +`openvixdisklib.nfc_open.NfcDisk.query_allocated_blocks` implements +this and run-length-merges contiguous set bits into +`AllocatedBlock(offset, length)` tuples (sectors, matching VDDK's +`VixDiskLibBlock`), exposed as +`VixDiskLibHandle.query_allocated_blocks`. Validated against the live +ESXi lab: a full-disk query, a query of a known-allocated sub-range, +and an aligned empty range all match native VDDK's own +`VixDiskLib_QueryAllocatedBlocks` output on the same disk. + +### Gotcha: query on the same still-open write handle can see stale data + +Writing a sector and then immediately calling +`query_allocated_blocks` **on that same open handle, without closing +it first**, can report the just-written region as *not* allocated — +the allocation metadata this call reads apparently isn't guaranteed +current until the write handle is closed. Closing after the write and +reopening (or querying from a separate handle opened after the write +completed) reports it correctly. Confirmed on both native VDDK and +this implementation — same-session-no-close showed the write as +unallocated on both, a fresh handle after close showed it correctly +on both — so this is a real server/VMFS behavior, not a bug in either +client. Real backup tools reading allocation before a read pass +naturally do this anyway (open read-only after the writer's handle +already closed), so it's unlikely to bite in practice, but do not +call `query_allocated_blocks` right after a write on the same handle +and expect it to reflect that write. ## What is still VDDK-only - zlib and skipz NBD compression flags - `VixDiskLib_ReadAsync` (same IO messages, different client threading) -- `VixDiskLib_QueryAllocatedBlocks` / allocation bitmaps - `VixDiskLib_GetInfo` capacity (not required to read a known range) diff --git a/docs/reverse_engineering_procedure.md b/docs/reverse_engineering_procedure.md index b988f5c..890da95 100644 --- a/docs/reverse_engineering_procedure.md +++ b/docs/reverse_engineering_procedure.md @@ -8,8 +8,9 @@ This file is the **sequence of steps**, including dead ends, so later NFC work can follow the same loop instead of rediscovering it. Scope so far: `VixDiskLib_ConnectEx` + `VixDiskLib_Open` + -`VixDiskLib_Read` + `VixDiskLib_Write` against lab vCenter 8.0.1 / -ESXi 8, transports `nbd` and `nbdssl`. Validation method: +`VixDiskLib_Read` + `VixDiskLib_Write` + `VixDiskLib_QueryAllocatedBlocks` +against lab vCenter 8.0.1 / ESXi 8, transports `nbd` and `nbdssl`. +Validation method: `tests/integration/` (the session-scoped `lab` fixture creates a temporary empty VM with a 10 GiB disk and destroys it when the pytest session ends). @@ -381,8 +382,63 @@ not an OPEN_FILE bit. Capture VDDK with that flag (NBD + the port-902 Replay: pip `pyfastlz` via `openvixdisklib/fastlz.py` (NFC extra is raw FastLZ, without the wrapper's 4-byte length prefix) plus `NfcDisk` compression on each IO. Proof: -`tests/integration/test_nfc_read_write.py` (`fastlz`) and -`tests/perf/test_compare.py`. +## Step 15 — `VixDiskLib_QueryAllocatedBlocks` (AIO type 13) + +Same SSL/write-hook technique, extended to `VixDiskLib_QueryAllocatedBlocks` +after `Open`. A first capture with `startSector=0` produced a request +where two 0-valued 8-byte fields were ambiguous — either could plausibly +be `start_offset_bytes`. Implementing from that guess alone put +`start_offset_bytes` at the wrong offset (8 instead of 24) and passed +every zero-start test while silently returning wrong (all-empty) +results for any non-zero start. A second capture with a **non-zero** +`startSector` against a region already known (from the first capture) +to be allocated broke the tie and found the real field order. + +A second, independent bug (bitmap reply length) was caught the same +way: `ceil(chunk_count/8)` matched the observed reply length for +`chunk_count=16384` (already a multiple of 4) but under-read and +desynced the connection for `chunk_count=16` — the reply pads the +bitmap to a 4-byte boundary. Diagnosed by testing candidate `extra_recv` +byte counts against whether the *next* AIO round-trip (a normal +`CLOSE_FILE`) completed cleanly, rather than guessing from a single +capture. + +Full protocol detail, the exact request/reply layout, and both bugs: +`docs/nfc_read.md`. Implemented as +`openvixdisklib.nfc_open.NfcDisk.query_allocated_blocks` +(`AllocatedBlock` dataclass) and +`VixDiskLibHandle.query_allocated_blocks`. Validated against the live +ESXi lab: full-disk query, a known-allocated sub-range, and an aligned +empty range all match native VDDK's own output on the same disk. + + +## Note — `NFC_DELTA_DISK` needed no protocol work either + +Investigated the backlog item "`NFC_DELTA_DISK` (reading directly +from a snapshot chain)" expecting a distinct wire message or OPEN_FILE +variant, similar to the CBT/`QueryAllocatedBlocks` split earlier. +`strings` on `libvixDiskLib.so` found `NFC_DELTA_DISK` is a **file-type +value** (like `NFC_DISK`), used by an internal VDDK client-side +heuristic ("`"%s" would probably benefit from bitmap copying, so +overriding file type to NFC_DELTA_DISK`") — a VMFS-only optimization +for very sparse redo logs, skipped entirely on NFS per an adjacent +string, and never observed to trigger in this lab's captures (no such +log line, `strings`-confirmed heuristic notwithstanding). + +Verified end-to-end that reading, writing, and `query_allocated_blocks` +against an actual post-snapshot delta file all already work correctly +with the existing NFC_DISK-only implementation — no code change +needed. The one real finding from this investigation was a gotcha, not +a gap: an initial test that wrote a sector and immediately queried +allocated blocks *on the same still-open write handle* reported the +write as unallocated; closing the handle first (or opening a separate +one) reported it correctly. Reproduced identically against **native +VDDK** on the same delta file (two-process capture, since loading +native VDDK in the same process as pyVmomi segfaults on this host's +OpenSSL — see `docs/ssl_hook.md`'s Limits section), so this is real +server/VMFS behavior, not specific to either client. Documented as a +`query_allocated_blocks` caveat in `docs/nfc_read.md`. + ## What to write down @@ -405,7 +461,7 @@ OpenVixDiskLib. Not yet reversed, same loop as above: - `DDB_GET` / disk geometry, zlib/skipz compression, encrypted disks -- `NFC_DELTA_DISK`, CBT / `QueryAllocatedBlocks` +- CBT - `VixDiskLib_GetInfo` capacity - Host-switch AIO messages - Direct ESXi `ha-nfc` without vCenter `vpxa-nfc` diff --git a/docs/ssl_hook.md b/docs/ssl_hook.md index cca3a43..6d52ceb 100644 --- a/docs/ssl_hook.md +++ b/docs/ssl_hook.md @@ -143,3 +143,15 @@ skipped. that is done offline on the hex log. - It must not ship in OpenVixDiskLib. Keep it out of the library path used by `openvixdisklib/nfc_auth.py`. +- `ctypes.CDLL` on `libvixDiskLib.so` **in the same process as + pyVmomi** can segfault, at least on this lab's Python/glibc build: + VDDK's bundled OpenSSL and the system OpenSSL pyVmomi already loaded + (for its own HTTPS) collide. Symptom: `Segmentation fault (core + dumped)`, no Python traceback. Split into two separate processes + instead — one doing pyVmomi/setup work, one doing only + `ctypes.CDLL`/native VDDK calls, handing data between them via a + file (see `docs/reverse_engineering_procedure.md`'s note on + `NFC_DELTA_DISK` for an example). This is the same underlying + conflict as `tests/integration/test_vddk.py` / + `test_crosscheck.py` needing `tox -e integration`'s isolated + subprocess env rather than running inside the main pytest process. diff --git a/openvixdisklib/nfc_open.py b/openvixdisklib/nfc_open.py index 1b25bd5..bb1bbdf 100644 --- a/openvixdisklib/nfc_open.py +++ b/openvixdisklib/nfc_open.py @@ -64,8 +64,14 @@ NFC_AIO_MSG_IO = 7 NFC_AIO_MSG_SET_SOCK_OPTS = 9 NFC_AIO_MSG_DDB_GET = 11 +NFC_AIO_MSG_QUERY_ALLOCATED_BLOCKS = 13 NFC_AIO_MSG_SET_RES_POOL = 22 +# Chunk size used in this project's capture/validation of +# query_allocated_blocks (128 sectors = 64 KiB); not a documented VDDK +# default, just a convenient granularity that worked in this lab. +NFC_QUERY_ALLOCATED_BLOCKS_CHUNK_SECTORS = 128 + # Open-file body: file type NFC_DISK. 0x1e is what VDDK sends for # VIXDISKLIB_FLAG_OPEN_READ_ONLY; writable opens clear bit 0x04 (0x1a). NFC_DISK = 2 @@ -82,6 +88,14 @@ NFC_COMPRESSION_FASTLZ = 2 +@dataclass(frozen=True, slots=True) +class AllocatedBlock: + """One allocated run, matching VDDK's ``VixDiskLibBlock`` (sectors).""" + + offset: int + length: int + + @dataclass(frozen=True, slots=True) class ReadFragment: """One NFC AIO extra in a packed skip-decompression ``buf``. @@ -516,6 +530,78 @@ def write(self, start_sector: int, num_sectors: int, data: bytes) -> None: f"expected type={NFC_AIO_MSG_IO} opId={op_id}" ) + def query_allocated_blocks( + self, + start_sector: int, + num_sectors: int, + chunk_size_sectors: int = NFC_QUERY_ALLOCATED_BLOCKS_CHUNK_SECTORS, + ) -> tuple[AllocatedBlock, ...]: + """Return allocated (non-sparse) runs. Matches ``VixDiskLib_QueryAllocatedBlocks``. + + Captured from VDDK: a 48-byte request:: + + uint64 handle + uint64 reserved (0) + uint64 chunk_size_bytes (chunk_size_sectors * sector_size) + uint64 start_offset_bytes (start_sector * sector_size) + uint64 chunk_count (num_sectors // chunk_size_sectors) + uint64 reserved (0) + + (``start_offset_bytes`` and the first ``reserved`` field are + easy to swap — both are 0 in a ``start_sector=0`` capture, + which is what an earlier draft of this method got wrong; a + second capture with a non-zero ``start_sector`` was needed to + tell them apart.) + + The reply echoes a 48-byte body whose offset 32 carries the + same chunk count back, followed by a bitmap extra — one bit + per chunk, LSB-first, set when that chunk contains allocated + data, padded up to a **4-byte boundary** (``ceil(chunk_count / 8)`` + alone under-reads and desyncs the connection whenever that + raw byte count isn't already a multiple of 4). + This mirrors VDDK's own client-side behavior of run-length + merging contiguous set bits into ``VixDiskLibBlock`` entries + (offset/length here are in **sectors**, matching the public + VDDK struct, unlike the bytes used on the wire). See + ``docs/nfc_read.md``. + + Args: + start_sector: Sector offset from the start of the disk; + must be a multiple of ``chunk_size_sectors`` (the + server returns an AIO error otherwise). + num_sectors: Number of sectors to query; must be a multiple + of ``chunk_size_sectors``. + chunk_size_sectors: Minimum run granularity, in sectors. + """ + if num_sectors % chunk_size_sectors != 0: + raise ValueError("num_sectors must be a multiple of chunk_size_sectors") + if start_sector % chunk_size_sectors != 0: + raise ValueError("start_sector must be a multiple of chunk_size_sectors") + chunk_count = num_sectors // chunk_size_sectors + bitmap_bytes = -(-((chunk_count + 7) // 8) // 4) * 4 + request = struct.pack( + " None: """Close the VMDK, the AIO session, and the classic NFC session.""" if self._closed: @@ -591,6 +677,35 @@ def _aio_prepare(disk: NfcDisk) -> None: disk._aio_roundtrip(NFC_AIO_MSG_SET_RES_POOL, struct.pack(" tuple[AllocatedBlock, ...]: + """Run-length-merge a QueryAllocatedBlocks bitmap into ``AllocatedBlock``s. + + ``bitmap`` is one bit per chunk, LSB-first (bit 0 of byte 0 is + chunk 0), possibly longer than strictly needed for padding; only + the first ``chunk_count`` bits are read. + """ + blocks = [] + run_start = None + for chunk_idx in range(chunk_count): + allocated = (bitmap[chunk_idx // 8] >> (chunk_idx % 8)) & 1 + if allocated and run_start is None: + run_start = chunk_idx + elif not allocated and run_start is not None: + blocks.append((run_start, chunk_idx - run_start)) + run_start = None + if run_start is not None: + blocks.append((run_start, chunk_count - run_start)) + return tuple( + AllocatedBlock( + offset=start_sector + run_chunk * chunk_size_sectors, + length=run_len * chunk_size_sectors, + ) + for run_chunk, run_len in blocks + ) + + def _parse_open_reply(body: bytes) -> tuple[int, int]: if len(body) < 40: raise NfcProtocolError(f"OPEN_FILE reply too short: {len(body)}") diff --git a/openvixdisklib/openvixdisklib.py b/openvixdisklib/openvixdisklib.py index cafb7f9..262c34b 100644 --- a/openvixdisklib/openvixdisklib.py +++ b/openvixdisklib/openvixdisklib.py @@ -28,6 +28,7 @@ ReadResult = nfc_open.ReadResult ReadFragment = nfc_open.ReadFragment +AllocatedBlock = nfc_open.AllocatedBlock LOG = logging.getLogger(__name__) @@ -326,6 +327,26 @@ def open( finally: self.close(handle) + def query_allocated_blocks( + self, + disk_handle: _DiskHandle, + start_sector: int, + num_sectors: int, + chunk_size_sectors: int = nfc_open.NFC_QUERY_ALLOCATED_BLOCKS_CHUNK_SECTORS, + ) -> tuple[nfc_open.AllocatedBlock, ...]: + """Return allocated runs. Matches ``VixDiskLib_QueryAllocatedBlocks``. + + Args: + disk_handle: Handle from ``open()``. + start_sector: Sector offset from the start of the disk. + num_sectors: Number of sectors to query; must be a multiple + of ``chunk_size_sectors``. + chunk_size_sectors: Minimum run granularity, in sectors. + """ + return disk_handle.disk.query_allocated_blocks( + start_sector, num_sectors, chunk_size_sectors + ) + def read( self, disk_handle: _DiskHandle, diff --git a/tests/integration/test_openvixdisklib.py b/tests/integration/test_openvixdisklib.py index 6c853d6..c853832 100644 --- a/tests/integration/test_openvixdisklib.py +++ b/tests/integration/test_openvixdisklib.py @@ -94,6 +94,44 @@ def test_write_and_read_sector_zero_and_one_gib( handle.read(disk, start, 1, read_buf) assert read_buf.raw[:SECTOR_SIZE] == expected + def test_query_allocated_blocks(self, lab: LabEnv) -> None: + """A written sector's chunk shows up as an allocated run. + + Write and query use separate handles (write closed first): see + docs/nfc_read.md's gotcha -- querying on the same still-open + handle a write just went through can see stale (pre-write) + data. + """ + handle = vixdisklib.VixDiskLibHandle(vixdisklib_compatibility_version="8.0") + chunk_size_sectors = 128 + # Chunk-aligned offset away from what other tests in this shared + # session-scoped VM write to (sector 0, SECTOR_AT_1GB, ...). + write_sector = 300 * chunk_size_sectors + write_buf = vixdisklib.get_buffer(SECTOR_SIZE) + write_buf[:SECTOR_SIZE] = pattern_bytes(SECTOR_SIZE, b"OVDL-QAB") + connect_kwargs = lab.vixdisklib_connect_kwargs( + {"allow_untrusted": lab.allow_untrusted} + ) + with ( + handle.connect(**connect_kwargs) as conn, + handle.open(conn, lab.disk_path, flags=0) as disk, + ): + handle.write(disk, write_sector, 1, write_buf) + + with ( + handle.connect(**connect_kwargs) as conn, + handle.open(conn, lab.disk_path, flags=0) as disk, + ): + blocks = handle.query_allocated_blocks( + disk, + start_sector=(write_sector // chunk_size_sectors) * chunk_size_sectors, + num_sectors=chunk_size_sectors, + chunk_size_sectors=chunk_size_sectors, + ) + assert any( + b.offset <= write_sector < b.offset + b.length for b in blocks + ), f"written sector {write_sector} not covered by {blocks}" + def test_read_only_open_snapshot_parent(self, lab: LabEnv) -> None: """Read-only Open uses NfcGetVmFiles, including a snapshot parent path. @@ -162,6 +200,74 @@ def read_sector(path: str) -> bytes: finally: Disconnect(si) + def test_write_and_query_allocated_blocks_on_delta_disk(self, lab: LabEnv) -> None: + """Read/write/query all work directly on a post-snapshot delta file. + + ``NFC_DELTA_DISK`` (an alternate OPEN_FILE file-type value, + per ``strings`` on ``libvixDiskLib.so``) turned out to be an + optional VMFS-only VDDK client optimization, not needed for + correctness — this exercises the plain ``NFC_DISK`` path + against an actual delta file. See + ``docs/reverse_engineering_procedure.md``. + + Also covers the gotcha documented in ``docs/nfc_read.md``: + querying allocated blocks on the *same still-open* handle a + write just went through can see stale (pre-write) data: the + write below is done in its own ``with`` block, closed, before + the separate query. + """ + handle = vixdisklib.VixDiskLibHandle(vixdisklib_compatibility_version="8.0") + chunk_size_sectors = 128 + write_sector = 400 * chunk_size_sectors + expected = pattern_bytes(SECTOR_SIZE, b"OVDL-DELTA") + write_buf = vixdisklib.get_buffer(SECTOR_SIZE) + write_buf[:SECTOR_SIZE] = expected + connect_kwargs = lab.vixdisklib_connect_kwargs( + {"allow_untrusted": lab.allow_untrusted} + ) + read_buf = vixdisklib.get_buffer(SECTOR_SIZE) + + si = _connect_vim( + lab.host, lab.username, lab.password, lab.port, lab.thumbprint, lab.allow_untrusted + ) + try: + vm = vim.VirtualMachine(lab.vm_moref, si._stub) + _wait_for_task(vm.CreateSnapshot_Task("ovdl-delta", "", False, False)) + delta_path = _virtual_disk_backing(vm).fileName + assert delta_path != lab.disk_path + + with ( + handle.connect(**connect_kwargs) as conn, + handle.open(conn, delta_path, flags=0) as disk, + ): + handle.write(disk, write_sector, 1, write_buf) + + # Fresh handle after close, per the gotcha above. + with ( + handle.connect(**connect_kwargs) as conn, + handle.open(conn, delta_path, flags=0) as disk, + ): + read_buf[:SECTOR_SIZE] = b"\xa5" * SECTOR_SIZE + handle.read(disk, write_sector, 1, read_buf) + assert read_buf.raw[:SECTOR_SIZE] == expected + + blocks = handle.query_allocated_blocks( + disk, + start_sector=(write_sector // chunk_size_sectors) * chunk_size_sectors, + num_sectors=chunk_size_sectors, + chunk_size_sectors=chunk_size_sectors, + ) + assert any( + b.offset <= write_sector < b.offset + b.length for b in blocks + ), f"written sector {write_sector} on delta file not covered by {blocks}" + finally: + try: + vm = vim.VirtualMachine(lab.vm_moref, si._stub) + if vm.snapshot is not None: + _wait_for_task(vm.RemoveAllSnapshots_Task()) + finally: + Disconnect(si) + @pytest.mark.parametrize( "aio_buffer_size, n_sectors, n_fragments", [ diff --git a/tests/unit/test_nfc_open.py b/tests/unit/test_nfc_open.py new file mode 100644 index 0000000..78f450c --- /dev/null +++ b/tests/unit/test_nfc_open.py @@ -0,0 +1,66 @@ +# Copyright 2026 Cloudbase Solutions Srl +# All Rights Reserved. + +"""Unit tests for the OPEN_FILE reply parsing in ``nfc_open``.""" + +import struct + +import pytest + +from openvixdisklib import nfc_open + + +class TestDecodeAllocatedBitmap: + def test_merges_contiguous_runs(self) -> None: + """Contiguous set bits become one run; gaps split into separate ones.""" + # chunks: 1,1,1,1,0,0,0,0,1,1 (10 chunks -> 2 bytes, LSB-first) + bitmap = bytes([0b00001111, 0b00000011]) + blocks = nfc_open._decode_allocated_bitmap( + bitmap, chunk_count=10, start_sector=1000, chunk_size_sectors=128 + ) + assert blocks == ( + nfc_open.AllocatedBlock(offset=1000, length=4 * 128), + nfc_open.AllocatedBlock(offset=1000 + 8 * 128, length=2 * 128), + ) + + def test_all_zero_bitmap_returns_no_blocks(self) -> None: + """A bitmap with no set bits produces an empty result.""" + blocks = nfc_open._decode_allocated_bitmap( + bytes(4), chunk_count=16, start_sector=0, chunk_size_sectors=128 + ) + assert blocks == () + + def test_run_extending_to_the_end_is_closed(self) -> None: + """A run of set bits reaching the last chunk is still reported.""" + # chunks: 0,1,1,1 (4 chunks, 1 byte; only lower nibble meaningful) + bitmap = bytes([0b00001110]) + blocks = nfc_open._decode_allocated_bitmap( + bitmap, chunk_count=4, start_sector=0, chunk_size_sectors=1 + ) + assert blocks == (nfc_open.AllocatedBlock(offset=1, length=3),) + + def test_ignores_bits_beyond_chunk_count(self) -> None: + """Padding bits past chunk_count (from 4-byte reply alignment) are unused.""" + # 2 real chunks (both set) + 2 padding bytes with garbage bits set. + bitmap = bytes([0b00000011, 0xFF, 0xFF, 0xFF]) + blocks = nfc_open._decode_allocated_bitmap( + bitmap, chunk_count=2, start_sector=0, chunk_size_sectors=128 + ) + assert blocks == (nfc_open.AllocatedBlock(offset=0, length=256),) + + + + +class TestQueryAllocatedBlocksValidation: + def _disk(self) -> nfc_open.NfcDisk: + return nfc_open.NfcDisk(sock=None, path="[ds] a.vmdk", handle=1, sector_size=512) + + def test_num_sectors_not_a_multiple_raises(self) -> None: + with pytest.raises(ValueError, match="num_sectors must be a multiple"): + self._disk().query_allocated_blocks(0, 100, chunk_size_sectors=128) + + def test_start_sector_not_a_multiple_raises(self) -> None: + with pytest.raises(ValueError, match="start_sector must be a multiple"): + self._disk().query_allocated_blocks(100, 128, chunk_size_sectors=128) + + From 43d8fe062fd202d148f2e63873b9e90f24e8801a Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Wed, 23 Sep 2026 12:16:15 +0000 Subject: [PATCH 2/4] Fix linter errors --- openvixdisklib/nfc_open.py | 4 +++- tests/integration/test_openvixdisklib.py | 15 ++++++++++++--- tests/unit/test_nfc_open.py | 10 +++------- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/openvixdisklib/nfc_open.py b/openvixdisklib/nfc_open.py index bb1bbdf..db59bc9 100644 --- a/openvixdisklib/nfc_open.py +++ b/openvixdisklib/nfc_open.py @@ -536,7 +536,9 @@ def query_allocated_blocks( num_sectors: int, chunk_size_sectors: int = NFC_QUERY_ALLOCATED_BLOCKS_CHUNK_SECTORS, ) -> tuple[AllocatedBlock, ...]: - """Return allocated (non-sparse) runs. Matches ``VixDiskLib_QueryAllocatedBlocks``. + """Return allocated (non-sparse) runs. + + Matches ``VixDiskLib_QueryAllocatedBlocks``. Captured from VDDK: a 48-byte request:: diff --git a/tests/integration/test_openvixdisklib.py b/tests/integration/test_openvixdisklib.py index c853832..57dd32d 100644 --- a/tests/integration/test_openvixdisklib.py +++ b/tests/integration/test_openvixdisklib.py @@ -228,7 +228,12 @@ def test_write_and_query_allocated_blocks_on_delta_disk(self, lab: LabEnv) -> No read_buf = vixdisklib.get_buffer(SECTOR_SIZE) si = _connect_vim( - lab.host, lab.username, lab.password, lab.port, lab.thumbprint, lab.allow_untrusted + lab.host, + lab.username, + lab.password, + lab.port, + lab.thumbprint, + lab.allow_untrusted, ) try: vm = vim.VirtualMachine(lab.vm_moref, si._stub) @@ -253,13 +258,17 @@ def test_write_and_query_allocated_blocks_on_delta_disk(self, lab: LabEnv) -> No blocks = handle.query_allocated_blocks( disk, - start_sector=(write_sector // chunk_size_sectors) * chunk_size_sectors, + start_sector=(write_sector // chunk_size_sectors) + * chunk_size_sectors, num_sectors=chunk_size_sectors, chunk_size_sectors=chunk_size_sectors, ) assert any( b.offset <= write_sector < b.offset + b.length for b in blocks - ), f"written sector {write_sector} on delta file not covered by {blocks}" + ), ( + f"written sector {write_sector} " + f"on delta file not covered by {blocks}" + ) finally: try: vm = vim.VirtualMachine(lab.vm_moref, si._stub) diff --git a/tests/unit/test_nfc_open.py b/tests/unit/test_nfc_open.py index 78f450c..e76e041 100644 --- a/tests/unit/test_nfc_open.py +++ b/tests/unit/test_nfc_open.py @@ -3,8 +3,6 @@ """Unit tests for the OPEN_FILE reply parsing in ``nfc_open``.""" -import struct - import pytest from openvixdisklib import nfc_open @@ -49,11 +47,11 @@ def test_ignores_bits_beyond_chunk_count(self) -> None: assert blocks == (nfc_open.AllocatedBlock(offset=0, length=256),) - - class TestQueryAllocatedBlocksValidation: def _disk(self) -> nfc_open.NfcDisk: - return nfc_open.NfcDisk(sock=None, path="[ds] a.vmdk", handle=1, sector_size=512) + return nfc_open.NfcDisk( + sock=None, path="[ds] a.vmdk", handle=1, sector_size=512 + ) def test_num_sectors_not_a_multiple_raises(self) -> None: with pytest.raises(ValueError, match="num_sectors must be a multiple"): @@ -62,5 +60,3 @@ def test_num_sectors_not_a_multiple_raises(self) -> None: def test_start_sector_not_a_multiple_raises(self) -> None: with pytest.raises(ValueError, match="start_sector must be a multiple"): self._disk().query_allocated_blocks(100, 128, chunk_size_sectors=128) - - From 494ba06a3673bdcc6525ce5e31816389d37a74dd Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Wed, 23 Sep 2026 12:26:52 +0000 Subject: [PATCH 3/4] Add an inline comment describing allocation bitmap parsing For better readability, we'll add an inline comment that describes how allocation bitmaps are parsed. --- openvixdisklib/nfc_open.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/openvixdisklib/nfc_open.py b/openvixdisklib/nfc_open.py index db59bc9..e5a5e94 100644 --- a/openvixdisklib/nfc_open.py +++ b/openvixdisklib/nfc_open.py @@ -691,6 +691,9 @@ def _decode_allocated_bitmap( blocks = [] run_start = None for chunk_idx in range(chunk_count): + # One bit per chunk, LSB-first: byte = chunk_idx // 8, bit = + # chunk_idx % 8. Shift that bit to position 0 and keep it with & 1 + # (1 = allocated, 0 = hole). Chunk 10 is bit 2 of bitmap[1]. allocated = (bitmap[chunk_idx // 8] >> (chunk_idx % 8)) & 1 if allocated and run_start is None: run_start = chunk_idx From 5ce54b78f96f33697c3c1bffe8368db6411e9e38 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Wed, 23 Sep 2026 12:33:57 +0000 Subject: [PATCH 4/4] Fix mypy failure We'll use the same typing.Protocol approach as https://github.com/cloudbase/OpenVixDiskLib/pull/4, minimizing merge conflicts. --- openvixdisklib/nfc_open.py | 22 ++++++++++++++++++---- tests/unit/test_nfc_open.py | 23 ++++++++++++++++++++++- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/openvixdisklib/nfc_open.py b/openvixdisklib/nfc_open.py index e5a5e94..bf0c20b 100644 --- a/openvixdisklib/nfc_open.py +++ b/openvixdisklib/nfc_open.py @@ -26,6 +26,7 @@ import ssl import struct from dataclasses import dataclass +from typing import Protocol from openvixdisklib import fastlz from openvixdisklib.nfc_auth import NfcAuthSession, _ssl_client_context @@ -184,18 +185,31 @@ def wrap_nfcssl_socket(ssock: ssl.SSLSocket, server_hostname: str) -> ssl.SSLSoc raise +class NfcTransport(Protocol): + """Byte pipe used after the NFC handshake (TCP, TLS, or a test fake).""" + + def sendall(self, data: bytes) -> None: + """Send ``data`` in full.""" + + def recv_into(self, buffer: memoryview, nbytes: int = 0, flags: int = 0) -> int: + """Read into ``buffer`` and return the number of bytes stored.""" + + def close(self) -> None: + """Close the underlying connection.""" + + def _enable_tcp_nodelay(sock: socket.socket) -> None: """Disable Nagle so a small AIO header is not held back from its extra.""" sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1) -def _recvn(sock: socket.socket, size: int) -> bytes: +def _recvn(sock: NfcTransport, size: int) -> bytes: buf = bytearray(size) _recvn_into(sock, memoryview(buf)) return bytes(buf) -def _recvn_into(sock: socket.socket, buf: memoryview) -> None: +def _recvn_into(sock: NfcTransport, buf: memoryview) -> None: """Read exactly ``len(buf)`` bytes into ``buf``.""" view = buf.cast("B") if buf.format != "B" else buf filled = 0 @@ -234,7 +248,7 @@ def _aio_extra_len(ctype: int, body: bytes, chunk_len: int) -> int: raise NfcProtocolError(f"unsupported NFC IO compression type {ctype}") -def _send_nfc_msg(sock: socket.socket, msg_type: int, body: bytes = b"") -> None: +def _send_nfc_msg(sock: NfcTransport, msg_type: int, body: bytes = b"") -> None: if len(body) > NFC_MSG_SIZE - 4: raise ValueError("NFC classic message body too large") frame = struct.pack(" None: + self._replies = replies + self.sent: list[bytes] = [] + + def sendall(self, data: bytes) -> None: + self.sent.append(bytes(data)) + + def recv_into(self, buffer: memoryview, nbytes: int = 0, flags: int = 0) -> int: + del nbytes, flags + n = min(len(buffer), len(self._replies)) + buffer[:n] = self._replies[:n] + self._replies = self._replies[n:] + return n + + def close(self) -> None: + pass + + class TestDecodeAllocatedBitmap: def test_merges_contiguous_runs(self) -> None: """Contiguous set bits become one run; gaps split into separate ones.""" @@ -50,7 +71,7 @@ def test_ignores_bits_beyond_chunk_count(self) -> None: class TestQueryAllocatedBlocksValidation: def _disk(self) -> nfc_open.NfcDisk: return nfc_open.NfcDisk( - sock=None, path="[ds] a.vmdk", handle=1, sector_size=512 + sock=_FakeSocket(), path="[ds] a.vmdk", handle=1, sector_size=512 ) def test_num_sectors_not_a_multiple_raises(self) -> None: