Add mdata-client: Rust rewrite of SmartOS metadata client#19
Draft
Add mdata-client: Rust rewrite of SmartOS metadata client#19
Conversation
Implements the V1/V2 metadata protocol with support for Unix domain sockets (SmartOS zones) and serial ports (KVM/HVM guests). Provides four binaries matching the original C tools: mdata-get, mdata-put, mdata-list, mdata-delete. Tested on SmartOS (base-64-lts zone) and Ubuntu 24.04 (bhyve VM). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Ran /simplify code review which identified four issues worth fixing: - Bound retry loops: execute() gives up after 3 timeout retries, V2 stale frame handling stops after 5 mismatched frames - Typed error for request ID mismatch: replace string matching on error messages with FrameError::ReqIdMismatch enum variant - Protocol encapsulation: move base64 encoding and V2 version checks into Protocol::put() and Protocol::delete() methods so binaries don't depend on protocol internals - FD leak fix: close serial fd if configure_serial_raw fails Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Split transport module into platform-specific files: - transport/unix.rs: Unix domain sockets + serial (poll-based I/O) - transport/windows.rs: COM2 serial port (Win32 API) - transport/mod.rs: shared types (TransportError, TransportConfig) Windows transport uses CreateFileW/SetCommState/SetCommTimeouts/ ReadFile/WriteFile with inline FFI definitions (no additional dependencies). The original mdata-get.exe in sdc-vmtools was a V1-only GET client from Visual Studio 2010; this provides all four commands with V2 protocol support. Verified: compiles for x86_64-pc-windows-msvc target. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Type safety:
- Add Command enum (Get/Put/Delete/Keys) replacing bare strings
- Add ProtocolVersion enum (V1/V2) replacing magic u8 comparisons
- Add #[must_use] and Debug derives on Response
Correctness:
- Replace std::mem::forget with into_raw_fd for fd ownership
- Handle EINTR in send() (was only handled in recv_line)
- Loop on EINTR in poll_readable instead of returning Ok(false)
- Replace File::open("/dev/urandom") with getrandom crate
(fixes weak request IDs on Windows where /dev/urandom doesn't exist)
Testability:
- Extract MetadataTransport trait from concrete Transport
- Make Protocol generic over MetadataTransport
- Add MockTransport and protocol-level tests (V1 get, negotiation)
Documentation:
- Document PUT's intentional double base64 encoding (wire format)
- Remove unused version() getter
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move negotiate() from free function into Protocol impl (issue 16) - Extract parse_v2_body() from inner closure in parse_v2_frame (issue 17) - Add manual Debug impl for Protocol<T> (issue 14) - Document error handling boundary in module doc (issue 15) - Document why Windows FFI is hand-rolled vs windows crate (issue 13) - Remove conditional no-op test from unix.rs (issue 18) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add tracing + tracing-subscriber for debug logging, controlled by the MDATA_DEBUG=1 env var. Normal operation produces no extra output (matching the original C tools). Debug mode logs transport detection, protocol negotiation, and timeout retries to stderr. - Add init_logging() helper in lib.rs - Convert retry eprintln to tracing::warn - Add tracing::debug for transport detection and negotiation - Call init_logging() from all 4 binaries Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace raw libc/Win32 FFI with std::fs::File and UnixStream for all read/write/open/close operations. The socket transport (zones, the common case) now uses zero unsafe. Unsafe is kept only where no safe Rust equivalent exists: - termios configuration (tcgetattr/tcsetattr) - exclusive file locking (fcntl F_SETLK) - poll-based serial timeouts (File has no set_read_timeout) - Windows serial config (GetCommState/SetCommState/SetCommTimeouts) All remaining unsafe blocks have SAFETY comments. Removed entirely: - libc::open/close/read/write (replaced by File/UnixStream) - Custom Drop impls (Rust ownership handles fd/handle cleanup) - Win32 CreateFileW/CloseHandle/ReadFile/WriteFile (replaced by File) Unsafe blocks: 29 -> 15 (socket path: 0) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Error handling (err-context-chain):
- Use .context() instead of map_err(|e| anyhow!("...: {e}")) in
decode_b64_payload and generate_request_id to preserve error chains
Correctness:
- Check return values of fcntl(F_SETFL) and tcflush, log on failure
API quality (api-common-traits):
- Add Hash to Command enum
- Add Clone, PartialEq to Response enum
- Use f.write_str() instead of write!() for string literals in
Command::Display (mem-avoid-format)
Tests:
- Add serial negotiation test (verifies \n reset sent before NEGOTIATE)
- Add PUT double-encoding roundtrip test
- Add stale V2 frame discard test
Test count: 10 -> 13
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rust implementation of the SmartOS metadata client (
mdata-get,mdata-put,mdata-list,mdata-delete), replacing the existing C implementation in triton/mdata-client.base64,crc32fast, andlibccratespoll()for I/O timeouts across all platforms (vs platform-specific event ports/epoll/kqueue in the C version)Test plan
Tested on two live instances with full CRUD operations:
SmartOS zone (
smartos-build1, base-64-lts@24.4.1 — Unix socket transport):Ubuntu 24.04 bhyve VM (
edge_foundry— serial port transport):Cross-tool interoperability (SmartOS): Native C
mdata-put→ Rustmdata-getand vice versa — both directions work correctly.🤖 Generated with Claude Code