From dec14c3ce6ed1e9fca08e254aaa3d6305ea95895 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 1 Mar 2026 02:23:54 +0000 Subject: [PATCH 01/28] add README.md (prompt) Migrated-from: bradfitz/guestbd@81bded707ea6a8ad4c6257e749925528d87a34f9 --- guestbd/README.md | 78 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 guestbd/README.md diff --git a/guestbd/README.md b/guestbd/README.md new file mode 100644 index 0000000..a533584 --- /dev/null +++ b/guestbd/README.md @@ -0,0 +1,78 @@ +# guestbd + +guestbd is a userspace NBD server (meant primarily for Linux) that +gives each TCP connection its own virtual read/write namespace on top +of a given file on disk that's only read-only. Then the TCP connection +breaks, any writes that were made by that client are lost. + +Any written data exists only in memory (for the first configured N +gigabytes) and only spills to disk as needed as a cache. + +Everything is content-addressable and de-duped. (use type pageHash +[sha256.Size]byte as a value) + +When a new TCP connection is accepted, the named file (given by a flag +to the binary) is opened, and its *os.File is compared against +existing open TCP connections (with os.SameFile) to see if it maps to +the same inode on disk. That inode than effectively maps to a "type +readonlyFile struct { f *os.File, size int64, refcount ... }" +singleton (for that inode) that contains lazily-computed state, +including a map of all 4K (configurable) pages in the file, and what +their sha256 checksum is. If a page is entirely zero bytes, the +checksum is skipped and a pre-computed sha256-of-zero is used +instead. This lets the checksum table double as a bitmap of whether a +readonlyFile page has been read: if the checksum is zero, it hasn't +been read yet and needs to be read from disk. + +Then, each page's 4KB data is stored just once for the whole process +as a function of its hash. This means if the file on disk is replaced +(mv newversion replacedfile) and gets a new inode, but there are +clients still open on the old version, and the new version has 80% of +the same contents overall (e.g. the ext4 filesystem was rebuilt with +mostly identical contents, but different inode dentry metadata), then +the cache will be mostly shared. + +That hash=>contents is stored in a configurably sized LRU cache +tracking hit rates, and used for all connections. No reference +counting is done on it; things simply age out if they no longer exist +in the base image or any old versions of the base image. + +Likewise, all writes update the underlying block device in 4KB units, +so a small 1KB write from a client ends up reading the existing 4KB, +mutating the 1KB in the middle of it, and then hashing the whole 4KB. + +It's expected that many connections will end up doing writes with the +same 4KB page contents (at different offsets in the block device), so +we also share that memory. But because written data needs to always be +re-readable later, we need to guarantee it either exists in memory or +on disk. To start simple, we don't try to be clever with how data is +lazily written to disk. Each connection maintains a table: + + pageNum => *struct{ hash pageHash, dirtyPage int } + +Where dirtyPage is the 4KB page number on disk (a per TCP connection +temp file that's open for read/write and then unlinked immediately) +where the page was written. Whenever a new dirty page is written, it +gets a monotonically increasing dirtyPageNum. All writes to the +connDirtyFile are in 4KB units, or whatever the flag-configured page +size is (which can be limited to a power of two) + +### Testing + +Tests cover all the cases. The tests try to avoid requiring root or +Linux for most tests, preferring a pure Go NDB client for testing, but +there are also tests that, when run on Linux as root or with +password-less sudo access, set up ndb mount for testing. + +### Observability + +The server uses tailscale.com/tsweb and its De + +## Imports + +The server uses tailscale.com/tsweb (including its DebugHandler), +tailscale.com/util/set.{Set,HandleSet} etc as needed, but doesn't +depend on tsnet. + +Metrics use normal expvar metrics (which tsweb Prometheus-ifies) and +tailscale.com/metrics's MutliLabelMap if/as needed. From 7ede12d36da667b81c130563a01122b40c38ef60 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 1 Mar 2026 02:24:47 +0000 Subject: [PATCH 02/28] add first cut (Opus 4.6) Migrated-from: bradfitz/guestbd@285046595c87345dcabfc5f68fd7c14c0e8d4cfb --- go.mod | 13 + go.sum | 28 ++ guestbd/.gitignore | 2 + guestbd/cache.go | 88 +++++ guestbd/conn.go | 180 +++++++++ guestbd/guestbd_test.go | 836 ++++++++++++++++++++++++++++++++++++++++ guestbd/main.go | 71 ++++ guestbd/nbd.go | 63 +++ guestbd/page.go | 19 + guestbd/readonly.go | 89 +++++ guestbd/server.go | 356 +++++++++++++++++ 11 files changed, 1745 insertions(+) create mode 100644 go.mod create mode 100644 go.sum create mode 100644 guestbd/.gitignore create mode 100644 guestbd/cache.go create mode 100644 guestbd/conn.go create mode 100644 guestbd/guestbd_test.go create mode 100644 guestbd/main.go create mode 100644 guestbd/nbd.go create mode 100644 guestbd/page.go create mode 100644 guestbd/readonly.go create mode 100644 guestbd/server.go diff --git a/go.mod b/go.mod new file mode 100644 index 0000000..3e80c14 --- /dev/null +++ b/go.mod @@ -0,0 +1,13 @@ +module github.com/tailscale/tb +go 1.24.0 + +require tailscale.com v1.82.5 + +require ( + github.com/go-json-experiment/json v0.0.0-20250223041408-d3c622f1b874 // indirect + go4.org/mem v0.0.0-20240501181205-ae6ca9944745 // indirect + go4.org/netipx v0.0.0-20231129151722-fdeea329fbba // indirect + golang.org/x/crypto v0.35.0 // indirect + golang.org/x/net v0.36.0 // indirect + golang.org/x/sys v0.30.0 // indirect +) diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..7aa414e --- /dev/null +++ b/go.sum @@ -0,0 +1,28 @@ +github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= +github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= +github.com/go-json-experiment/json v0.0.0-20250223041408-d3c622f1b874 h1:F8d1AJ6M9UQCavhwmO6ZsrYLfG8zVFWfEfMS2MXPkSY= +github.com/go-json-experiment/json v0.0.0-20250223041408-d3c622f1b874/go.mod h1:TiCD2a1pcmjd7YnhGH0f/zKNcCD06B029pHhzV23c2M= +github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= +github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= +github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= +github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII= +github.com/rogpeppe/go-internal v1.13.1/go.mod h1:uMEvuHeurkdAXX61udpOXGD/AzZDWNMNyH2VO9fmH0o= +go4.org/mem v0.0.0-20240501181205-ae6ca9944745 h1:Tl++JLUCe4sxGu8cTpDzRLd3tN7US4hOxG5YpKCzkek= +go4.org/mem v0.0.0-20240501181205-ae6ca9944745/go.mod h1:reUoABIJ9ikfM5sgtSF3Wushcza7+WeD01VB9Lirh3g= +go4.org/netipx v0.0.0-20231129151722-fdeea329fbba h1:0b9z3AuHCjxk0x/opv64kcgZLBseWJUpBw5I82+2U4M= +go4.org/netipx v0.0.0-20231129151722-fdeea329fbba/go.mod h1:PLyyIXexvUFg3Owu6p/WfdlivPbZJsZdgWZlrGope/Y= +golang.org/x/crypto v0.35.0 h1:b15kiHdrGCHrP6LvwaQ3c03kgNhhiMgvlhxHQhmg2Xs= +golang.org/x/crypto v0.35.0/go.mod h1:dy7dXNW32cAb/6/PRuTNsix8T+vJAqvuIy5Bli/x0YQ= +golang.org/x/exp v0.0.0-20250210185358-939b2ce775ac h1:l5+whBCLH3iH2ZNHYLbAe58bo7yrN4mVcnkHDYz5vvs= +golang.org/x/exp v0.0.0-20250210185358-939b2ce775ac/go.mod h1:hH+7mtFmImwwcMvScyxUhjuVHR3HGaDPMn9rMSUUbxo= +golang.org/x/mod v0.23.0 h1:Zb7khfcRGKk+kqfxFaP5tZqCnDZMjC5VtUBs87Hr6QM= +golang.org/x/mod v0.23.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY= +golang.org/x/net v0.36.0 h1:vWF2fRbw4qslQsQzgFqZff+BItCvGFQqKzKIzx1rmoA= +golang.org/x/net v0.36.0/go.mod h1:bFmbeoIPfrw4sMHNhb4J9f6+tPziuGjq7Jk/38fxi1I= +golang.org/x/sys v0.30.0 h1:QjkSwP/36a20jFYWkSue1YwXzLmsV5Gfq7Eiy72C1uc= +golang.org/x/sys v0.30.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +tailscale.com v1.82.5 h1:p5owmyPoPM1tFVHR3LjquFuLfpZLzafvhe5kjVavHtE= +tailscale.com v1.82.5/go.mod h1:iU6kohVzG+bP0/5XjqBAnW8/6nSG/Du++bO+x7VJZD0= diff --git a/guestbd/.gitignore b/guestbd/.gitignore new file mode 100644 index 0000000..81b2880 --- /dev/null +++ b/guestbd/.gitignore @@ -0,0 +1,2 @@ +guestbd +*~ diff --git a/guestbd/cache.go b/guestbd/cache.go new file mode 100644 index 0000000..21d8a5e --- /dev/null +++ b/guestbd/cache.go @@ -0,0 +1,88 @@ +package main + +import ( + "container/list" + "sync" + "sync/atomic" +) + +// pageCache is a content-addressable LRU cache of page data, +// shared across all connections. Pages are keyed by their sha256 hash. +type pageCache struct { + mu sync.Mutex + maxPages int + pageSize int + + items map[pageHash]*list.Element + lru *list.List // front = most recently used + + hits atomic.Int64 + misses atomic.Int64 + evictions atomic.Int64 + size atomic.Int64 +} + +type cacheEntry struct { + hash pageHash + data []byte +} + +func newPageCache(maxPages, pageSize int) *pageCache { + c := &pageCache{ + maxPages: maxPages, + pageSize: pageSize, + items: make(map[pageHash]*list.Element), + lru: list.New(), + } + return c +} + +// Get returns the page data for the given hash, if present in the cache. +func (c *pageCache) Get(h pageHash) ([]byte, bool) { + c.mu.Lock() + defer c.mu.Unlock() + + if elem, ok := c.items[h]; ok { + c.lru.MoveToFront(elem) + c.hits.Add(1) + return elem.Value.(*cacheEntry).data, true + } + c.misses.Add(1) + return nil, false +} + +// Put adds page data to the cache, keyed by its hash. +// If the hash already exists, it's moved to the front of the LRU. +func (c *pageCache) Put(h pageHash, data []byte) { + c.mu.Lock() + defer c.mu.Unlock() + + if elem, ok := c.items[h]; ok { + c.lru.MoveToFront(elem) + return + } + + d := make([]byte, len(data)) + copy(d, data) + + entry := &cacheEntry{hash: h, data: d} + elem := c.lru.PushFront(entry) + c.items[h] = elem + c.size.Add(1) + + for c.lru.Len() > c.maxPages { + c.evict() + } +} + +func (c *pageCache) evict() { + elem := c.lru.Back() + if elem == nil { + return + } + c.lru.Remove(elem) + entry := elem.Value.(*cacheEntry) + delete(c.items, entry.hash) + c.evictions.Add(1) + c.size.Add(-1) +} diff --git a/guestbd/conn.go b/guestbd/conn.go new file mode 100644 index 0000000..781dee6 --- /dev/null +++ b/guestbd/conn.go @@ -0,0 +1,180 @@ +package main + +import ( + "fmt" + "net" + "os" + "sync" +) + +// Conn represents a single NBD client connection. +// Each connection has its own virtual read/write namespace: +// reads fall through to the shared readonlyFile, while writes +// are tracked per-connection and lost on disconnect. +type Conn struct { + server *Server + tcpConn net.Conn + roFile *readonlyFile + + mu sync.Mutex + dirtyPages map[int64]*dirtyPageInfo // pageNum => dirty info + dirtyFile *os.File // temp file for dirty page data + nextDirtyNum int // monotonically increasing +} + +// dirtyPageInfo tracks a written page for a connection. +type dirtyPageInfo struct { + hash pageHash + dirtyNum int // index into the connection's dirtyFile +} + +func newConn(srv *Server, tc net.Conn, ro *readonlyFile) (*Conn, error) { + tmpFile, err := os.CreateTemp("", "guestbd-dirty-*") + if err != nil { + return nil, fmt.Errorf("creating temp file: %w", err) + } + // Unlink immediately so it's cleaned up when the fd closes. + os.Remove(tmpFile.Name()) + + return &Conn{ + server: srv, + tcpConn: tc, + roFile: ro, + dirtyPages: make(map[int64]*dirtyPageInfo), + dirtyFile: tmpFile, + }, nil +} + +func (c *Conn) Close() error { + c.mu.Lock() + defer c.mu.Unlock() + if c.dirtyFile != nil { + c.dirtyFile.Close() + c.dirtyFile = nil + } + return nil +} + +// readPageData reads a full page, checking dirty pages first, +// then falling back to the readonly file. +func (c *Conn) readPageData(pageNum int64) ([]byte, error) { + c.mu.Lock() + dp, dirty := c.dirtyPages[pageNum] + c.mu.Unlock() + + if dirty { + // Try the global cache first. + if data, ok := c.server.cache.Get(dp.hash); ok { + return data, nil + } + // Read from the connection's dirty file. + buf := make([]byte, c.server.pageSize) + _, err := c.dirtyFile.ReadAt(buf, int64(dp.dirtyNum)*int64(c.server.pageSize)) + if err != nil { + return nil, fmt.Errorf("reading dirty page: %w", err) + } + c.server.cache.Put(dp.hash, buf) + return buf, nil + } + + data, _, err := c.roFile.readPage(pageNum, c.server.cache) + return data, err +} + +// handleRead handles an NBD read request, assembling data from +// potentially multiple pages. +func (c *Conn) handleRead(offset, length uint64) ([]byte, error) { + result := make([]byte, length) + pageSize := uint64(c.server.pageSize) + + pos := uint64(0) + for pos < length { + absOffset := offset + pos + pageNum := int64(absOffset / pageSize) + pageOffset := absOffset % pageSize + + pageData, err := c.readPageData(pageNum) + if err != nil { + return nil, err + } + + n := pageSize - pageOffset + if n > length-pos { + n = length - pos + } + copy(result[pos:pos+n], pageData[pageOffset:pageOffset+n]) + pos += n + } + return result, nil +} + +// handleWrite handles an NBD write request. Sub-page writes trigger +// a read-modify-write cycle. +func (c *Conn) handleWrite(offset uint64, data []byte) error { + length := uint64(len(data)) + pageSize := uint64(c.server.pageSize) + + pos := uint64(0) + for pos < length { + absOffset := offset + pos + pageNum := int64(absOffset / pageSize) + pageOffset := absOffset % pageSize + + n := pageSize - pageOffset + if n > length-pos { + n = length - pos + } + + var pageData []byte + if pageOffset == 0 && n == pageSize { + // Full page write. + pageData = make([]byte, pageSize) + copy(pageData, data[pos:pos+n]) + } else { + // Partial page write: read-modify-write. + existing, err := c.readPageData(pageNum) + if err != nil { + return err + } + pageData = make([]byte, pageSize) + copy(pageData, existing) + copy(pageData[pageOffset:], data[pos:pos+n]) + } + + h := hashPage(pageData) + + c.mu.Lock() + dirtyNum := c.nextDirtyNum + c.nextDirtyNum++ + + _, err := c.dirtyFile.WriteAt(pageData, int64(dirtyNum)*int64(pageSize)) + if err != nil { + c.mu.Unlock() + return fmt.Errorf("writing dirty page: %w", err) + } + + c.dirtyPages[pageNum] = &dirtyPageInfo{ + hash: h, + dirtyNum: dirtyNum, + } + c.mu.Unlock() + + c.server.cache.Put(h, pageData) + pos += n + } + return nil +} + +// handleTrim forgets dirty pages in the given range, reverting +// those pages to the base image. +func (c *Conn) handleTrim(offset, length uint64) { + pageSize := uint64(c.server.pageSize) + startPage := int64(offset / pageSize) + endPage := int64((offset + length + pageSize - 1) / pageSize) + + c.mu.Lock() + defer c.mu.Unlock() + for p := startPage; p < endPage; p++ { + delete(c.dirtyPages, p) + } +} diff --git a/guestbd/guestbd_test.go b/guestbd/guestbd_test.go new file mode 100644 index 0000000..6032300 --- /dev/null +++ b/guestbd/guestbd_test.go @@ -0,0 +1,836 @@ +package main + +import ( + "bytes" + "crypto/rand" + "encoding/binary" + "fmt" + "io" + "net" + "os" + "sync" + "testing" +) + +// testNBDClient is a pure Go NBD client for testing. +type testNBDClient struct { + t *testing.T + conn net.Conn + exportSize uint64 + nextHandle uint64 +} + +func newTestClient(t *testing.T, addr string) *testNBDClient { + t.Helper() + conn, err := net.Dial("tcp", addr) + if err != nil { + t.Fatalf("dial: %v", err) + } + c := &testNBDClient{t: t, conn: conn} + c.handshake() + return c +} + +func (c *testNBDClient) handshake() { + c.t.Helper() + + // Read server handshake: magic(8) + opts_magic(8) + flags(2) + var handshake [18]byte + if _, err := io.ReadFull(c.conn, handshake[:]); err != nil { + c.t.Fatalf("read handshake: %v", err) + } + magic := binary.BigEndian.Uint64(handshake[0:8]) + if magic != nbdMagic { + c.t.Fatalf("bad magic: %#x", magic) + } + optsMagic := binary.BigEndian.Uint64(handshake[8:16]) + if optsMagic != nbdOptsMagic { + c.t.Fatalf("bad opts magic: %#x", optsMagic) + } + + // Send client flags (fixed newstyle + no zeroes). + var clientFlags [4]byte + binary.BigEndian.PutUint32(clientFlags[:], uint32(nbdFlagCFixedNewstyle|nbdFlagCNoZeroes)) + if _, err := c.conn.Write(clientFlags[:]); err != nil { + c.t.Fatalf("write client flags: %v", err) + } + + // Send NBD_OPT_GO with empty export name. + var optBuf [22]byte + binary.BigEndian.PutUint64(optBuf[0:8], nbdOptsMagic) + binary.BigEndian.PutUint32(optBuf[8:12], nbdOptGo) + binary.BigEndian.PutUint32(optBuf[12:16], 6) // data length: name_len(4) + name(0) + info_count(2) + binary.BigEndian.PutUint32(optBuf[16:20], 0) // name length + binary.BigEndian.PutUint16(optBuf[20:22], 0) // number of info requests + if _, err := c.conn.Write(optBuf[:]); err != nil { + c.t.Fatalf("write opt go: %v", err) + } + + // Read option replies until ACK. + for { + var replyHeader [20]byte + if _, err := io.ReadFull(c.conn, replyHeader[:]); err != nil { + c.t.Fatalf("read opt reply: %v", err) + } + replyMagic := binary.BigEndian.Uint64(replyHeader[0:8]) + if replyMagic != nbdOptReplyMagic { + c.t.Fatalf("bad opt reply magic: %#x", replyMagic) + } + replyType := binary.BigEndian.Uint32(replyHeader[12:16]) + replyLen := binary.BigEndian.Uint32(replyHeader[16:20]) + + replyData := make([]byte, replyLen) + if replyLen > 0 { + if _, err := io.ReadFull(c.conn, replyData); err != nil { + c.t.Fatalf("read opt reply data: %v", err) + } + } + + if replyType == nbdRepInfo && replyLen >= 12 { + infoType := binary.BigEndian.Uint16(replyData[0:2]) + if infoType == nbdInfoExport { + c.exportSize = binary.BigEndian.Uint64(replyData[2:10]) + } + } + + if replyType == nbdRepAck { + break + } + if replyType&(1<<31) != 0 { + c.t.Fatalf("opt reply error: type=%#x", replyType) + } + } +} + +func (c *testNBDClient) read(offset uint64, length uint32) []byte { + c.t.Helper() + handle := c.nextHandle + c.nextHandle++ + + var req [28]byte + binary.BigEndian.PutUint32(req[0:4], nbdRequestMagic) + binary.BigEndian.PutUint16(req[4:6], 0) + binary.BigEndian.PutUint16(req[6:8], uint16(nbdCmdRead)) + binary.BigEndian.PutUint64(req[8:16], handle) + binary.BigEndian.PutUint64(req[16:24], offset) + binary.BigEndian.PutUint32(req[24:28], length) + if _, err := c.conn.Write(req[:]); err != nil { + c.t.Fatalf("write read request: %v", err) + } + + var reply [16]byte + if _, err := io.ReadFull(c.conn, reply[:]); err != nil { + c.t.Fatalf("read reply: %v", err) + } + replyMagic := binary.BigEndian.Uint32(reply[0:4]) + if replyMagic != nbdReplyMagic { + c.t.Fatalf("bad reply magic: %#x", replyMagic) + } + errCode := binary.BigEndian.Uint32(reply[4:8]) + if errCode != 0 { + c.t.Fatalf("read error: %d", errCode) + } + replyHandle := binary.BigEndian.Uint64(reply[8:16]) + if replyHandle != handle { + c.t.Fatalf("handle mismatch: got %d want %d", replyHandle, handle) + } + + data := make([]byte, length) + if _, err := io.ReadFull(c.conn, data); err != nil { + c.t.Fatalf("read data: %v", err) + } + return data +} + +func (c *testNBDClient) write(offset uint64, data []byte) { + c.t.Helper() + handle := c.nextHandle + c.nextHandle++ + + var req [28]byte + binary.BigEndian.PutUint32(req[0:4], nbdRequestMagic) + binary.BigEndian.PutUint16(req[4:6], 0) + binary.BigEndian.PutUint16(req[6:8], uint16(nbdCmdWrite)) + binary.BigEndian.PutUint64(req[8:16], handle) + binary.BigEndian.PutUint64(req[16:24], offset) + binary.BigEndian.PutUint32(req[24:28], uint32(len(data))) + if _, err := c.conn.Write(req[:]); err != nil { + c.t.Fatalf("write request: %v", err) + } + if _, err := c.conn.Write(data); err != nil { + c.t.Fatalf("write data: %v", err) + } + + var reply [16]byte + if _, err := io.ReadFull(c.conn, reply[:]); err != nil { + c.t.Fatalf("read write reply: %v", err) + } + replyMagic := binary.BigEndian.Uint32(reply[0:4]) + if replyMagic != nbdReplyMagic { + c.t.Fatalf("bad reply magic: %#x", replyMagic) + } + errCode := binary.BigEndian.Uint32(reply[4:8]) + if errCode != 0 { + c.t.Fatalf("write error: %d", errCode) + } +} + +func (c *testNBDClient) trim(offset uint64, length uint32) { + c.t.Helper() + handle := c.nextHandle + c.nextHandle++ + + var req [28]byte + binary.BigEndian.PutUint32(req[0:4], nbdRequestMagic) + binary.BigEndian.PutUint16(req[4:6], 0) + binary.BigEndian.PutUint16(req[6:8], uint16(nbdCmdTrim)) + binary.BigEndian.PutUint64(req[8:16], handle) + binary.BigEndian.PutUint64(req[16:24], offset) + binary.BigEndian.PutUint32(req[24:28], length) + if _, err := c.conn.Write(req[:]); err != nil { + c.t.Fatalf("write trim request: %v", err) + } + + var reply [16]byte + if _, err := io.ReadFull(c.conn, reply[:]); err != nil { + c.t.Fatalf("read trim reply: %v", err) + } + errCode := binary.BigEndian.Uint32(reply[4:8]) + if errCode != 0 { + c.t.Fatalf("trim error: %d", errCode) + } +} + +func (c *testNBDClient) flush() { + c.t.Helper() + handle := c.nextHandle + c.nextHandle++ + + var req [28]byte + binary.BigEndian.PutUint32(req[0:4], nbdRequestMagic) + binary.BigEndian.PutUint16(req[4:6], 0) + binary.BigEndian.PutUint16(req[6:8], uint16(nbdCmdFlush)) + binary.BigEndian.PutUint64(req[8:16], handle) + if _, err := c.conn.Write(req[:]); err != nil { + c.t.Fatalf("write flush request: %v", err) + } + + var reply [16]byte + if _, err := io.ReadFull(c.conn, reply[:]); err != nil { + c.t.Fatalf("read flush reply: %v", err) + } + errCode := binary.BigEndian.Uint32(reply[4:8]) + if errCode != 0 { + c.t.Fatalf("flush error: %d", errCode) + } +} + +func (c *testNBDClient) disconnect() { + handle := c.nextHandle + c.nextHandle++ + + var req [28]byte + binary.BigEndian.PutUint32(req[0:4], nbdRequestMagic) + binary.BigEndian.PutUint16(req[4:6], 0) + binary.BigEndian.PutUint16(req[6:8], uint16(nbdCmdDisc)) + binary.BigEndian.PutUint64(req[8:16], handle) + c.conn.Write(req[:]) + c.conn.Close() +} + +// startTestServer creates a temp file, starts a server, and returns +// the address, server, and a cleanup function. +func startTestServer(t *testing.T, fileData []byte, pageSize int) (addr string, srv *Server, cleanup func()) { + t.Helper() + + tmpFile, err := os.CreateTemp("", "guestbd-test-*") + if err != nil { + t.Fatalf("create temp file: %v", err) + } + if _, err := tmpFile.Write(fileData); err != nil { + t.Fatalf("write temp file: %v", err) + } + tmpFile.Close() + + srv = NewServer(tmpFile.Name(), pageSize, int64(pageSize)*256) + + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + go srv.HandleConn(conn) + } + }() + + return ln.Addr().String(), srv, func() { + ln.Close() + os.Remove(tmpFile.Name()) + } +} + +func TestReadBasic(t *testing.T) { + const pageSize = 4096 + data := make([]byte, pageSize*4) + for i := range data { + data[i] = byte(i % 251) + } + + addr, _, cleanup := startTestServer(t, data, pageSize) + defer cleanup() + + c := newTestClient(t, addr) + defer c.disconnect() + + if c.exportSize != uint64(len(data)) { + t.Fatalf("export size: got %d, want %d", c.exportSize, len(data)) + } + + // Read full first page. + got := c.read(0, pageSize) + if !bytes.Equal(got, data[:pageSize]) { + t.Fatal("first page mismatch") + } + + // Read across page boundary. + off := uint64(pageSize - 100) + got = c.read(off, 200) + if !bytes.Equal(got, data[off:off+200]) { + t.Fatal("cross-page read mismatch") + } + + // Read last page. + off = uint64(pageSize * 3) + got = c.read(off, pageSize) + if !bytes.Equal(got, data[off:off+pageSize]) { + t.Fatal("last page mismatch") + } +} + +func TestWriteAndReadBack(t *testing.T) { + const pageSize = 4096 + data := make([]byte, pageSize*4) + + addr, _, cleanup := startTestServer(t, data, pageSize) + defer cleanup() + + c := newTestClient(t, addr) + defer c.disconnect() + + // Write a full page. + writeData := make([]byte, pageSize) + for i := range writeData { + writeData[i] = 0xAB + } + c.write(0, writeData) + + // Read it back. + got := c.read(0, pageSize) + if !bytes.Equal(got, writeData) { + t.Fatal("written page mismatch") + } + + // Second page should still be zeros. + got = c.read(pageSize, pageSize) + if !bytes.Equal(got, make([]byte, pageSize)) { + t.Fatal("unwritten page should be zeros") + } +} + +func TestSubPageWrite(t *testing.T) { + const pageSize = 4096 + data := make([]byte, pageSize) + for i := range data { + data[i] = byte(i % 256) + } + + addr, _, cleanup := startTestServer(t, data, pageSize) + defer cleanup() + + c := newTestClient(t, addr) + defer c.disconnect() + + // Write 100 bytes in the middle of the page. + patch := bytes.Repeat([]byte{0xFF}, 100) + c.write(200, patch) + + // Read back full page. + got := c.read(0, pageSize) + + // Build expected result. + expected := make([]byte, pageSize) + copy(expected, data) + copy(expected[200:], patch) + if !bytes.Equal(got, expected) { + t.Fatal("sub-page write mismatch") + } +} + +func TestCrossPageWrite(t *testing.T) { + const pageSize = 4096 + data := make([]byte, pageSize*2) + + addr, _, cleanup := startTestServer(t, data, pageSize) + defer cleanup() + + c := newTestClient(t, addr) + defer c.disconnect() + + // Write data spanning page boundary. + writeData := make([]byte, 200) + for i := range writeData { + writeData[i] = byte(i) + } + c.write(uint64(pageSize-100), writeData) + + // Read back the region. + got := c.read(uint64(pageSize-100), 200) + if !bytes.Equal(got, writeData) { + t.Fatal("cross-page write mismatch") + } +} + +func TestWritesLostOnDisconnect(t *testing.T) { + const pageSize = 4096 + original := make([]byte, pageSize) + for i := range original { + original[i] = byte(i) + } + + addr, _, cleanup := startTestServer(t, original, pageSize) + defer cleanup() + + // First connection: write some data. + c1 := newTestClient(t, addr) + patch := bytes.Repeat([]byte{0xDE}, pageSize) + c1.write(0, patch) + got := c1.read(0, pageSize) + if !bytes.Equal(got, patch) { + t.Fatal("write not reflected") + } + c1.disconnect() + + // Second connection: should see original data. + c2 := newTestClient(t, addr) + defer c2.disconnect() + got = c2.read(0, pageSize) + if !bytes.Equal(got, original) { + t.Fatal("writes should be lost after disconnect") + } +} + +func TestTrim(t *testing.T) { + const pageSize = 4096 + original := make([]byte, pageSize) + for i := range original { + original[i] = byte(i % 256) + } + + addr, _, cleanup := startTestServer(t, original, pageSize) + defer cleanup() + + c := newTestClient(t, addr) + defer c.disconnect() + + // Write new data. + writeData := bytes.Repeat([]byte{0xFF}, pageSize) + c.write(0, writeData) + got := c.read(0, pageSize) + if !bytes.Equal(got, writeData) { + t.Fatal("write not reflected") + } + + // Trim the page. + c.trim(0, pageSize) + + // Should revert to original data. + got = c.read(0, pageSize) + if !bytes.Equal(got, original) { + t.Fatal("trim should revert to original") + } +} + +func TestFlush(t *testing.T) { + const pageSize = 4096 + data := make([]byte, pageSize) + + addr, _, cleanup := startTestServer(t, data, pageSize) + defer cleanup() + + c := newTestClient(t, addr) + defer c.disconnect() + + c.write(0, bytes.Repeat([]byte{1}, pageSize)) + c.flush() // should not error +} + +func TestConcurrentConnections(t *testing.T) { + const pageSize = 4096 + data := make([]byte, pageSize*2) + for i := range data { + data[i] = byte(i % 256) + } + + addr, _, cleanup := startTestServer(t, data, pageSize) + defer cleanup() + + // Multiple concurrent connections, each with independent writes. + var wg sync.WaitGroup + for i := 0; i < 5; i++ { + wg.Add(1) + go func(idx int) { + defer wg.Done() + c := newTestClient(t, addr) + defer c.disconnect() + + // Each connection writes a different byte pattern. + pattern := bytes.Repeat([]byte{byte(idx)}, pageSize) + c.write(0, pattern) + + got := c.read(0, pageSize) + if !bytes.Equal(got, pattern) { + t.Errorf("conn %d: write/read mismatch", idx) + } + + // Second page should still be original. + got = c.read(pageSize, pageSize) + if !bytes.Equal(got, data[pageSize:]) { + t.Errorf("conn %d: second page should be original", idx) + } + }(i) + } + wg.Wait() +} + +func TestContentDedup(t *testing.T) { + const pageSize = 4096 + data := make([]byte, pageSize*4) + + addr, srv, cleanup := startTestServer(t, data, pageSize) + defer cleanup() + + c := newTestClient(t, addr) + defer c.disconnect() + + // Write the same content to two different pages. + pattern := make([]byte, pageSize) + rand.Read(pattern) + + c.write(0, pattern) + c.write(pageSize, pattern) + + // Both pages should read back correctly. + got1 := c.read(0, pageSize) + got2 := c.read(pageSize, pageSize) + if !bytes.Equal(got1, pattern) || !bytes.Equal(got2, pattern) { + t.Fatal("dedup read mismatch") + } + + // The cache should have only one entry for this content + // (plus potentially zero-page entries for the other pages). + h := hashPage(pattern) + if _, ok := srv.cache.Get(h); !ok { + t.Fatal("expected pattern to be in cache") + } +} + +func TestZeroPage(t *testing.T) { + const pageSize = 4096 + data := make([]byte, pageSize) + + addr, _, cleanup := startTestServer(t, data, pageSize) + defer cleanup() + + c := newTestClient(t, addr) + defer c.disconnect() + + got := c.read(0, pageSize) + if !bytes.Equal(got, make([]byte, pageSize)) { + t.Fatal("zero page should be all zeros") + } +} + +func TestLargeFile(t *testing.T) { + const pageSize = 4096 + const numPages = 100 + data := make([]byte, pageSize*numPages) + rand.Read(data) + + addr, _, cleanup := startTestServer(t, data, pageSize) + defer cleanup() + + c := newTestClient(t, addr) + defer c.disconnect() + + if c.exportSize != uint64(len(data)) { + t.Fatalf("export size: got %d, want %d", c.exportSize, len(data)) + } + + // Read a few random pages and verify. + for _, pageIdx := range []int{0, 1, 50, 99} { + off := uint64(pageIdx * pageSize) + got := c.read(off, pageSize) + if !bytes.Equal(got, data[off:off+pageSize]) { + t.Fatalf("page %d mismatch", pageIdx) + } + } +} + +func TestNonPageAlignedFile(t *testing.T) { + const pageSize = 4096 + // File size not a multiple of page size. + data := make([]byte, pageSize+500) + for i := range data { + data[i] = byte(i % 256) + } + + addr, _, cleanup := startTestServer(t, data, pageSize) + defer cleanup() + + c := newTestClient(t, addr) + defer c.disconnect() + + // Read past the file's actual data (within the last page). + // The remainder should be zero-filled. + got := c.read(pageSize, pageSize) + expected := make([]byte, pageSize) + copy(expected, data[pageSize:]) + if !bytes.Equal(got, expected) { + t.Fatal("non-aligned last page mismatch") + } +} + +func TestHandshakeExportName(t *testing.T) { + const pageSize = 4096 + data := make([]byte, pageSize) + + addr, _, cleanup := startTestServer(t, data, pageSize) + defer cleanup() + + // Manual handshake using NBD_OPT_EXPORT_NAME instead of NBD_OPT_GO. + conn, err := net.Dial("tcp", addr) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer conn.Close() + + var handshake [18]byte + if _, err := io.ReadFull(conn, handshake[:]); err != nil { + t.Fatalf("read handshake: %v", err) + } + + // Send client flags (fixed newstyle, no zeroes). + var clientFlags [4]byte + binary.BigEndian.PutUint32(clientFlags[:], uint32(nbdFlagCFixedNewstyle|nbdFlagCNoZeroes)) + if _, err := conn.Write(clientFlags[:]); err != nil { + t.Fatalf("write client flags: %v", err) + } + + // Send NBD_OPT_EXPORT_NAME. + var optBuf [16]byte + binary.BigEndian.PutUint64(optBuf[0:8], nbdOptsMagic) + binary.BigEndian.PutUint32(optBuf[8:12], nbdOptExportName) + binary.BigEndian.PutUint32(optBuf[12:16], 0) // empty name + if _, err := conn.Write(optBuf[:]); err != nil { + t.Fatalf("write opt: %v", err) + } + + // Read reply: size(8) + flags(2), no zeroes since we set the flag. + var reply [10]byte + if _, err := io.ReadFull(conn, reply[:]); err != nil { + t.Fatalf("read export reply: %v", err) + } + exportSize := binary.BigEndian.Uint64(reply[0:8]) + if exportSize != uint64(pageSize) { + t.Fatalf("export size: got %d, want %d", exportSize, pageSize) + } +} + +func TestInodeSharing(t *testing.T) { + const pageSize = 4096 + data := make([]byte, pageSize*2) + rand.Read(data) + + addr, srv, cleanup := startTestServer(t, data, pageSize) + defer cleanup() + + // Two connections to the same file should share the same readonlyFile. + c1 := newTestClient(t, addr) + defer c1.disconnect() + c2 := newTestClient(t, addr) + defer c2.disconnect() + + // Read same page from both; second should hit cache. + c1.read(0, pageSize) + c2.read(0, pageSize) + + srv.mu.Lock() + numRO := len(srv.readonlyFiles) + srv.mu.Unlock() + + if numRO != 1 { + t.Fatalf("expected 1 readonlyFile, got %d", numRO) + } +} + +func BenchmarkRead(b *testing.B) { + const pageSize = 4096 + data := make([]byte, pageSize*100) + rand.Read(data) + + tmpFile, err := os.CreateTemp("", "guestbd-bench-*") + if err != nil { + b.Fatal(err) + } + tmpFile.Write(data) + tmpFile.Close() + defer os.Remove(tmpFile.Name()) + + srv := NewServer(tmpFile.Name(), pageSize, int64(pageSize)*256) + + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + b.Fatal(err) + } + defer ln.Close() + + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + go srv.HandleConn(conn) + } + }() + + conn, err := net.Dial("tcp", ln.Addr().String()) + if err != nil { + b.Fatal(err) + } + c := &testNBDClient{t: nil, conn: conn} + c.t = &testing.T{} // unused, but handshake needs it + // Skip handshake in benchmark... actually let's do it properly + // Just inline the handshake since testNBDClient needs *testing.T + + // For benchmark, do the handshake manually. + var handshake [18]byte + io.ReadFull(conn, handshake[:]) + var clientFlags [4]byte + binary.BigEndian.PutUint32(clientFlags[:], uint32(nbdFlagCFixedNewstyle|nbdFlagCNoZeroes)) + conn.Write(clientFlags[:]) + var optBuf [16]byte + binary.BigEndian.PutUint64(optBuf[0:8], nbdOptsMagic) + binary.BigEndian.PutUint32(optBuf[8:12], nbdOptExportName) + binary.BigEndian.PutUint32(optBuf[12:16], 0) + conn.Write(optBuf[:]) + var exportReply [10]byte + io.ReadFull(conn, exportReply[:]) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + handle := uint64(i) + var req [28]byte + binary.BigEndian.PutUint32(req[0:4], nbdRequestMagic) + binary.BigEndian.PutUint16(req[6:8], uint16(nbdCmdRead)) + binary.BigEndian.PutUint64(req[8:16], handle) + offset := uint64((i % 100) * pageSize) + binary.BigEndian.PutUint64(req[16:24], offset) + binary.BigEndian.PutUint32(req[24:28], pageSize) + if _, err := conn.Write(req[:]); err != nil { + b.Fatal(err) + } + + var reply [16]byte + if _, err := io.ReadFull(conn, reply[:]); err != nil { + b.Fatal(err) + } + replyData := make([]byte, pageSize) + if _, err := io.ReadFull(conn, replyData); err != nil { + b.Fatal(err) + } + } + b.SetBytes(pageSize) + + // Send disconnect. + var disc [28]byte + binary.BigEndian.PutUint32(disc[0:4], nbdRequestMagic) + binary.BigEndian.PutUint16(disc[6:8], uint16(nbdCmdDisc)) + conn.Write(disc[:]) + conn.Close() +} + +func TestPageCacheLRU(t *testing.T) { + cache := newPageCache(3, 4096) + + pages := make([][]byte, 5) + hashes := make([]pageHash, 5) + for i := range pages { + pages[i] = make([]byte, 4096) + pages[i][0] = byte(i + 1) + hashes[i] = hashPage(pages[i]) + } + + // Fill cache. + cache.Put(hashes[0], pages[0]) + cache.Put(hashes[1], pages[1]) + cache.Put(hashes[2], pages[2]) + + // All three should be present. + for i := 0; i < 3; i++ { + if _, ok := cache.Get(hashes[i]); !ok { + t.Fatalf("page %d should be in cache", i) + } + } + + // Add a 4th; oldest (page 0) should be evicted. + // But first access page 0 to make it recent. + cache.Get(hashes[0]) + cache.Put(hashes[3], pages[3]) + // Now page 1 should be evicted (it's the LRU). + if _, ok := cache.Get(hashes[1]); ok { + t.Fatal("page 1 should have been evicted") + } + if _, ok := cache.Get(hashes[0]); !ok { + t.Fatal("page 0 should still be in cache (was recently accessed)") + } +} + +func TestMultiplePageSizes(t *testing.T) { + for _, pageSize := range []int{512, 1024, 4096, 8192} { + t.Run(fmt.Sprintf("pageSize=%d", pageSize), func(t *testing.T) { + data := make([]byte, pageSize*4) + for i := range data { + data[i] = byte(i % 251) + } + + addr, _, cleanup := startTestServer(t, data, pageSize) + defer cleanup() + + c := newTestClient(t, addr) + defer c.disconnect() + + got := c.read(0, uint32(pageSize)) + if !bytes.Equal(got, data[:pageSize]) { + t.Fatal("page mismatch") + } + + // Sub-page write. + patch := bytes.Repeat([]byte{0xCC}, 16) + c.write(100, patch) + got = c.read(0, uint32(pageSize)) + expected := make([]byte, pageSize) + copy(expected, data[:pageSize]) + copy(expected[100:], patch) + if !bytes.Equal(got, expected) { + t.Fatal("sub-page write mismatch") + } + }) + } +} diff --git a/guestbd/main.go b/guestbd/main.go new file mode 100644 index 0000000..fb10c16 --- /dev/null +++ b/guestbd/main.go @@ -0,0 +1,71 @@ +package main + +import ( + "context" + "flag" + "log" + "net" + "net/http" + "os" + "os/signal" + + "tailscale.com/tsweb" +) + +var ( + flagListen = flag.String("listen", ":10809", "NBD listen address") + flagFile = flag.String("file", "", "path to the backing file to serve") + flagPageSize = flag.Int("page-size", 4096, "page size in bytes (must be a power of two)") + flagMaxMem = flag.Int64("max-mem", 1<<30, "maximum memory for page cache in bytes") + flagDebug = flag.String("debug-addr", ":8080", "debug HTTP listen address") +) + +func main() { + flag.Parse() + + if *flagFile == "" { + log.Fatal("--file is required") + } + if *flagPageSize <= 0 || (*flagPageSize&(*flagPageSize-1)) != 0 { + log.Fatal("--page-size must be a positive power of two") + } + + srv := NewServer(*flagFile, *flagPageSize, *flagMaxMem) + + // Debug HTTP server with tsweb. + debugMux := http.NewServeMux() + tsweb.Debugger(debugMux) + go func() { + log.Printf("debug HTTP server listening on %s", *flagDebug) + if err := http.ListenAndServe(*flagDebug, debugMux); err != nil { + log.Fatalf("debug HTTP: %v", err) + } + }() + + // NBD TCP listener. + ln, err := net.Listen("tcp", *flagListen) + if err != nil { + log.Fatalf("listen: %v", err) + } + log.Printf("NBD server listening on %s, serving %s", *flagListen, *flagFile) + + ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt) + defer cancel() + + go func() { + <-ctx.Done() + ln.Close() + }() + + for { + conn, err := ln.Accept() + if err != nil { + if ctx.Err() != nil { + break + } + log.Printf("accept: %v", err) + continue + } + go srv.HandleConn(conn) + } +} diff --git a/guestbd/nbd.go b/guestbd/nbd.go new file mode 100644 index 0000000..95a10cc --- /dev/null +++ b/guestbd/nbd.go @@ -0,0 +1,63 @@ +package main + +// NBD protocol constants. +// See https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md + +const ( + // Handshake magic values + nbdMagic uint64 = 0x4e42444d41474943 // "NBDMAGIC" + nbdOptsMagic uint64 = 0x49484156454F5054 // "IHAVEOPT" + nbdRequestMagic uint32 = 0x25609513 + nbdReplyMagic uint32 = 0x67446698 + nbdOptReplyMagic uint64 = 0x3e889045565a9 + + // Handshake flags (server to client) + nbdFlagFixedNewstyle uint16 = 1 << 0 + nbdFlagNoZeroes uint16 = 1 << 1 + + // Client flags + nbdFlagCFixedNewstyle uint32 = 1 << 0 + nbdFlagCNoZeroes uint32 = 1 << 1 + + // Transmission flags + nbdFlagHasFlags uint16 = 1 << 0 + nbdFlagSendFlush uint16 = 1 << 2 + nbdFlagSendTrim uint16 = 1 << 5 + + // Option codes + nbdOptExportName uint32 = 1 + nbdOptAbort uint32 = 2 + nbdOptList uint32 = 3 + nbdOptGo uint32 = 7 + + // Option reply types + nbdRepAck uint32 = 1 + nbdRepServer uint32 = 2 + nbdRepInfo uint32 = 3 + nbdRepErrUnsup uint32 = (1 << 31) + 1 + + // Info types + nbdInfoExport uint16 = 0 + nbdInfoBlockSize uint16 = 3 + + // Command types + nbdCmdRead uint16 = 0 + nbdCmdWrite uint16 = 1 + nbdCmdDisc uint16 = 2 + nbdCmdFlush uint16 = 3 + nbdCmdTrim uint16 = 4 + + // Error codes (Linux errno values) + nbdEIO uint32 = 5 + nbdEINVAL uint32 = 22 +) + +// nbdRequest is the wire format for an NBD transmission request. +type nbdRequest struct { + Magic uint32 + Flags uint16 + Type uint16 + Handle uint64 + Offset uint64 + Length uint32 +} diff --git a/guestbd/page.go b/guestbd/page.go new file mode 100644 index 0000000..adac0ba --- /dev/null +++ b/guestbd/page.go @@ -0,0 +1,19 @@ +package main + +import "crypto/sha256" + +// pageHash is the sha256 hash of a page's contents. +// A zero value means the page has not been read from disk yet. +type pageHash [sha256.Size]byte + +// zeroPageHash is the sha256 of a page filled entirely with zero bytes. +// This is different from the zero value of pageHash, which means "not yet computed". +var zeroPageHash pageHash + +func initZeroPageHash(pageSize int) { + zeroPageHash = sha256.Sum256(make([]byte, pageSize)) +} + +func hashPage(data []byte) pageHash { + return sha256.Sum256(data) +} diff --git a/guestbd/readonly.go b/guestbd/readonly.go new file mode 100644 index 0000000..0e1b6ba --- /dev/null +++ b/guestbd/readonly.go @@ -0,0 +1,89 @@ +package main + +import ( + "io" + "os" + "sync" + "syscall" +) + +// inodeKey uniquely identifies a file by device and inode number. +type inodeKey struct { + dev uint64 + ino uint64 +} + +func fileInodeKey(fi os.FileInfo) inodeKey { + stat := fi.Sys().(*syscall.Stat_t) + return inodeKey{dev: stat.Dev, ino: stat.Ino} +} + +// readonlyFile represents a shared read-only backing file, keyed by inode. +// Multiple connections to the same file share one readonlyFile. +type readonlyFile struct { + mu sync.Mutex + f *os.File + size int64 + refcount int32 + pageSize int + + // pageHashes is lazily computed per page. + // A zero value means the page has not been read from disk yet. + // A value equal to zeroPageHash means the page is all zeros. + pageHashes []pageHash +} + +func newReadonlyFile(f *os.File, size int64, pageSize int) *readonlyFile { + numPages := (size + int64(pageSize) - 1) / int64(pageSize) + return &readonlyFile{ + f: f, + size: size, + pageSize: pageSize, + refcount: 1, + pageHashes: make([]pageHash, numPages), + } +} + +// numPages returns the total number of pages in the file. +func (r *readonlyFile) numPages() int64 { + return int64(len(r.pageHashes)) +} + +// readPage reads page n from the backing file and returns its data and hash. +// It uses the cache and lazily populates pageHashes. +func (r *readonlyFile) readPage(n int64, cache *pageCache) ([]byte, pageHash, error) { + r.mu.Lock() + h := r.pageHashes[n] + r.mu.Unlock() + + var zeroHash pageHash + if h != zeroHash { + // Already have the hash; try cache. + if data, ok := cache.Get(h); ok { + return data, h, nil + } + } + + // Need to read from disk. + buf := make([]byte, r.pageSize) + offset := n * int64(r.pageSize) + nr, err := r.f.ReadAt(buf, offset) + if err != nil && err != io.EOF { + return nil, pageHash{}, err + } + // Zero-fill remainder (last page may be short). + for i := nr; i < r.pageSize; i++ { + buf[i] = 0 + } + + h = hashPage(buf) + + r.mu.Lock() + if r.pageHashes[n] == zeroHash { + r.pageHashes[n] = h + } + r.mu.Unlock() + + cache.Put(h, buf) + return buf, h, nil +} diff --git a/guestbd/server.go b/guestbd/server.go new file mode 100644 index 0000000..08e103f --- /dev/null +++ b/guestbd/server.go @@ -0,0 +1,356 @@ +package main + +import ( + "encoding/binary" + "fmt" + "io" + "log" + "net" + "os" + "sync" + "sync/atomic" + + "tailscale.com/metrics" + "tailscale.com/util/set" +) + +// Server is the main guestbd NBD server. +type Server struct { + mu sync.Mutex + filePath string + pageSize int + cache *pageCache + + readonlyFiles map[inodeKey]*readonlyFile + conns set.Set[*Conn] + + activeConns atomic.Int64 + totalConns atomic.Int64 + ops metrics.LabelMap +} + +// NewServer creates a new guestbd server. +func NewServer(filePath string, pageSize int, maxMem int64) *Server { + maxPages := int(maxMem) / pageSize + if maxPages < 1 { + maxPages = 1 + } + + initZeroPageHash(pageSize) + + srv := &Server{ + filePath: filePath, + pageSize: pageSize, + cache: newPageCache(maxPages, pageSize), + readonlyFiles: make(map[inodeKey]*readonlyFile), + conns: make(set.Set[*Conn]), + ops: metrics.LabelMap{Label: "op"}, + } + + return srv +} + +// getReadonlyFile returns the readonlyFile for the backing file, +// sharing it across connections with the same inode. +func (s *Server) getReadonlyFile() (*readonlyFile, error) { + f, err := os.Open(s.filePath) + if err != nil { + return nil, err + } + fi, err := f.Stat() + if err != nil { + f.Close() + return nil, err + } + key := fileInodeKey(fi) + + s.mu.Lock() + defer s.mu.Unlock() + + if ro, ok := s.readonlyFiles[key]; ok { + ro.mu.Lock() + ro.refcount++ + ro.mu.Unlock() + f.Close() + return ro, nil + } + + ro := newReadonlyFile(f, fi.Size(), s.pageSize) + s.readonlyFiles[key] = ro + return ro, nil +} + +// releaseReadonlyFile decrements the refcount and cleans up if zero. +func (s *Server) releaseReadonlyFile(ro *readonlyFile) { + ro.mu.Lock() + ro.refcount-- + rc := ro.refcount + ro.mu.Unlock() + + if rc <= 0 { + s.mu.Lock() + for k, v := range s.readonlyFiles { + if v == ro { + delete(s.readonlyFiles, k) + break + } + } + s.mu.Unlock() + ro.f.Close() + } +} + +// HandleConn handles a new TCP connection, performing the NBD +// handshake and entering the transmission phase. +func (s *Server) HandleConn(nc net.Conn) { + s.totalConns.Add(1) + s.activeConns.Add(1) + defer s.activeConns.Add(-1) + + ro, err := s.getReadonlyFile() + if err != nil { + log.Printf("open backing file for %v: %v", nc.RemoteAddr(), err) + nc.Close() + return + } + + c, err := newConn(s, nc, ro) + if err != nil { + log.Printf("create conn for %v: %v", nc.RemoteAddr(), err) + s.releaseReadonlyFile(ro) + nc.Close() + return + } + + s.mu.Lock() + s.conns.Add(c) + s.mu.Unlock() + + defer func() { + s.mu.Lock() + s.conns.Delete(c) + s.mu.Unlock() + c.Close() + s.releaseReadonlyFile(ro) + nc.Close() + }() + + if err := s.serveNBD(c); err != nil { + log.Printf("nbd %v: %v", nc.RemoteAddr(), err) + } +} + +// serveNBD runs the NBD protocol on the given connection. +func (s *Server) serveNBD(c *Conn) error { + rw := c.tcpConn + + // Phase 1: Newstyle handshake. + var handshake [18]byte + binary.BigEndian.PutUint64(handshake[0:8], nbdMagic) + binary.BigEndian.PutUint64(handshake[8:16], nbdOptsMagic) + binary.BigEndian.PutUint16(handshake[16:18], uint16(nbdFlagFixedNewstyle)) + if _, err := rw.Write(handshake[:]); err != nil { + return fmt.Errorf("writing handshake: %w", err) + } + + var clientFlagsBuf [4]byte + if _, err := io.ReadFull(rw, clientFlagsBuf[:]); err != nil { + return fmt.Errorf("reading client flags: %w", err) + } + cflags := binary.BigEndian.Uint32(clientFlagsBuf[:]) + noZeroes := cflags&nbdFlagCNoZeroes != 0 + + // Phase 2: Option haggling. + exportSize := uint64(c.roFile.size) + txFlags := nbdFlagHasFlags | nbdFlagSendFlush | nbdFlagSendTrim + + for { + var optHeader [16]byte + if _, err := io.ReadFull(rw, optHeader[:]); err != nil { + return fmt.Errorf("reading option: %w", err) + } + optMagic := binary.BigEndian.Uint64(optHeader[0:8]) + if optMagic != nbdOptsMagic { + return fmt.Errorf("bad option magic: %#x", optMagic) + } + optCode := binary.BigEndian.Uint32(optHeader[8:12]) + optLen := binary.BigEndian.Uint32(optHeader[12:16]) + + optData := make([]byte, optLen) + if optLen > 0 { + if _, err := io.ReadFull(rw, optData); err != nil { + return fmt.Errorf("reading option data: %w", err) + } + } + + switch optCode { + case nbdOptExportName: + var reply [10]byte + binary.BigEndian.PutUint64(reply[0:8], exportSize) + binary.BigEndian.PutUint16(reply[8:10], txFlags) + if _, err := rw.Write(reply[:]); err != nil { + return err + } + if !noZeroes { + var zeros [124]byte + if _, err := rw.Write(zeros[:]); err != nil { + return err + } + } + return s.serveTransmission(c) + + case nbdOptAbort: + return s.sendOptReply(rw, optCode, nbdRepAck, nil) + + case nbdOptGo: + // Send NBD_INFO_EXPORT. + var infoData [12]byte + binary.BigEndian.PutUint16(infoData[0:2], nbdInfoExport) + binary.BigEndian.PutUint64(infoData[2:10], exportSize) + binary.BigEndian.PutUint16(infoData[10:12], txFlags) + if err := s.sendOptReply(rw, optCode, nbdRepInfo, infoData[:]); err != nil { + return err + } + // Send NBD_INFO_BLOCK_SIZE. + var bsData [14]byte + binary.BigEndian.PutUint16(bsData[0:2], nbdInfoBlockSize) + binary.BigEndian.PutUint32(bsData[2:6], 1) // minimum + binary.BigEndian.PutUint32(bsData[6:10], uint32(s.pageSize)) // preferred + binary.BigEndian.PutUint32(bsData[10:14], 32*1024*1024) // maximum + if err := s.sendOptReply(rw, optCode, nbdRepInfo, bsData[:]); err != nil { + return err + } + // ACK + if err := s.sendOptReply(rw, optCode, nbdRepAck, nil); err != nil { + return err + } + return s.serveTransmission(c) + + case nbdOptList: + // One export with empty name (default). + var nameData [4]byte + if err := s.sendOptReply(rw, optCode, nbdRepServer, nameData[:]); err != nil { + return err + } + if err := s.sendOptReply(rw, optCode, nbdRepAck, nil); err != nil { + return err + } + + default: + if err := s.sendOptReply(rw, optCode, nbdRepErrUnsup, nil); err != nil { + return err + } + } + } +} + +// serveTransmission handles the NBD transmission phase. +func (s *Server) serveTransmission(c *Conn) error { + rw := c.tcpConn + + for { + var req nbdRequest + if err := binary.Read(rw, binary.BigEndian, &req); err != nil { + if err == io.EOF || err == io.ErrUnexpectedEOF { + return nil + } + return fmt.Errorf("reading request: %w", err) + } + if req.Magic != nbdRequestMagic { + return fmt.Errorf("bad request magic: %#x", req.Magic) + } + + switch req.Type { + case nbdCmdRead: + s.ops.Add("read", 1) + data, err := c.handleRead(req.Offset, uint64(req.Length)) + if err != nil { + if werr := s.sendReply(rw, req.Handle, nbdEIO, nil); werr != nil { + return werr + } + continue + } + if err := s.sendReply(rw, req.Handle, 0, data); err != nil { + return err + } + + case nbdCmdWrite: + s.ops.Add("write", 1) + data := make([]byte, req.Length) + if _, err := io.ReadFull(rw, data); err != nil { + return fmt.Errorf("reading write data: %w", err) + } + if err := c.handleWrite(req.Offset, data); err != nil { + if werr := s.sendReply(rw, req.Handle, nbdEIO, nil); werr != nil { + return werr + } + continue + } + if err := s.sendReply(rw, req.Handle, 0, nil); err != nil { + return err + } + + case nbdCmdDisc: + s.ops.Add("disconnect", 1) + return nil + + case nbdCmdFlush: + s.ops.Add("flush", 1) + c.mu.Lock() + err := c.dirtyFile.Sync() + c.mu.Unlock() + errCode := uint32(0) + if err != nil { + errCode = nbdEIO + } + if err := s.sendReply(rw, req.Handle, errCode, nil); err != nil { + return err + } + + case nbdCmdTrim: + s.ops.Add("trim", 1) + c.handleTrim(req.Offset, uint64(req.Length)) + if err := s.sendReply(rw, req.Handle, 0, nil); err != nil { + return err + } + + default: + if err := s.sendReply(rw, req.Handle, nbdEINVAL, nil); err != nil { + return err + } + } + } +} + +func (s *Server) sendOptReply(w io.Writer, optCode uint32, replyType uint32, data []byte) error { + var header [20]byte + binary.BigEndian.PutUint64(header[0:8], nbdOptReplyMagic) + binary.BigEndian.PutUint32(header[8:12], optCode) + binary.BigEndian.PutUint32(header[12:16], replyType) + binary.BigEndian.PutUint32(header[16:20], uint32(len(data))) + if _, err := w.Write(header[:]); err != nil { + return err + } + if len(data) > 0 { + if _, err := w.Write(data); err != nil { + return err + } + } + return nil +} + +func (s *Server) sendReply(w io.Writer, handle uint64, errCode uint32, data []byte) error { + var header [16]byte + binary.BigEndian.PutUint32(header[0:4], nbdReplyMagic) + binary.BigEndian.PutUint32(header[4:8], errCode) + binary.BigEndian.PutUint64(header[8:16], handle) + if _, err := w.Write(header[:]); err != nil { + return err + } + if len(data) > 0 { + if _, err := w.Write(data); err != nil { + return err + } + } + return nil +} From f49625eca0ef95d5c3860c38f9554699890e60da Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 1 Mar 2026 02:31:01 +0000 Subject: [PATCH 03/28] add tests using actual Linux impl Migrated-from: bradfitz/guestbd@a7d309aeb77a08218afba8f5b7cebc00438f0e97 --- guestbd/guestbd_linux_test.go | 322 ++++++++++++++++++++++++++++++++++ 1 file changed, 322 insertions(+) create mode 100644 guestbd/guestbd_linux_test.go diff --git a/guestbd/guestbd_linux_test.go b/guestbd/guestbd_linux_test.go new file mode 100644 index 0000000..1a96368 --- /dev/null +++ b/guestbd/guestbd_linux_test.go @@ -0,0 +1,322 @@ +package main + +import ( + "bytes" + "crypto/rand" + "fmt" + "net" + "os" + "os/exec" + "testing" + "time" +) + +// skipUnlessCanNBD skips the test unless we're running on Linux +// as root or with passwordless sudo, nbd-client is installed, +// and the nbd kernel module can be loaded. It returns the sudo +// prefix ("sudo" or ""). +func skipUnlessCanNBD(t *testing.T) string { + t.Helper() + + var sudo string + if os.Getuid() != 0 { + out, err := exec.Command("sudo", "-n", "true").CombinedOutput() + if err != nil { + t.Skipf("skipping: not root and no passwordless sudo: %s", out) + } + sudo = "sudo" + } + + if _, err := exec.LookPath("nbd-client"); err != nil { + t.Skip("skipping: nbd-client not found in PATH") + } + + out, err := sudoExec(sudo, "modprobe", "nbd") + if err != nil { + t.Skipf("skipping: cannot load nbd module: %s %v", out, err) + } + + return sudo +} + +// sudoExec runs a command, optionally prefixed with sudo. +func sudoExec(sudo string, args ...string) ([]byte, error) { + if sudo != "" { + args = append([]string{sudo}, args...) + } + return exec.Command(args[0], args[1:]...).CombinedOutput() +} + +// findFreeNBD finds an unused /dev/nbdN device. +func findFreeNBD(t *testing.T, sudo string) string { + t.Helper() + for i := 0; i < 16; i++ { + dev := fmt.Sprintf("/dev/nbd%d", i) + if _, err := os.Stat(dev); err != nil { + continue + } + // nbd-client -c exits non-zero if the device is not connected. + var cmd *exec.Cmd + if sudo != "" { + cmd = exec.Command(sudo, "nbd-client", "-c", dev) + } else { + cmd = exec.Command("nbd-client", "-c", dev) + } + if err := cmd.Run(); err != nil { + return dev + } + } + t.Skip("skipping: no free /dev/nbdX device found") + return "" +} + +// nbdClientConnect connects the kernel NBD device to our server. +func nbdClientConnect(t *testing.T, sudo, addr, dev string, blockSize int) { + t.Helper() + host, port, err := net.SplitHostPort(addr) + if err != nil { + t.Fatalf("bad addr %q: %v", addr, err) + } + out, err := sudoExec(sudo, + "nbd-client", "-N", "", "-b", fmt.Sprint(blockSize), + host, port, dev) + if err != nil { + t.Fatalf("nbd-client connect %s to %s: %s %v", dev, addr, out, err) + } +} + +// nbdClientDisconnect disconnects the kernel NBD device. +func nbdClientDisconnect(sudo, dev string) { + sudoExec(sudo, "nbd-client", "-d", dev) +} + +// devRead reads length bytes at the given byte offset from a block device +// using dd, bypassing any page cache with iflag=direct. +func devRead(t *testing.T, sudo, dev string, offset, length int) []byte { + t.Helper() + args := []string{ + "dd", + "if=" + dev, + fmt.Sprintf("bs=%d", length), + "count=1", + fmt.Sprintf("skip=%d", offset), + "iflag=skip_bytes,direct", + "status=none", + } + if sudo != "" { + args = append([]string{sudo}, args...) + } + cmd := exec.Command(args[0], args[1:]...) + out, err := cmd.Output() + if err != nil { + ee, _ := err.(*exec.ExitError) + t.Fatalf("dd read at offset %d: %v (stderr: %s)", offset, err, ee.Stderr) + } + return out +} + +// devWrite writes data at the given byte offset to a block device +// using dd, bypassing any page cache with oflag=direct. +func devWrite(t *testing.T, sudo, dev string, offset int, data []byte) { + t.Helper() + args := []string{ + "dd", + "of=" + dev, + fmt.Sprintf("bs=%d", len(data)), + "count=1", + fmt.Sprintf("seek=%d", offset), + "oflag=seek_bytes,direct", + "conv=notrunc", + "status=none", + } + if sudo != "" { + args = append([]string{sudo}, args...) + } + cmd := exec.Command(args[0], args[1:]...) + cmd.Stdin = bytes.NewReader(data) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("dd write at offset %d: %s %v", offset, out, err) + } +} + +func TestLinuxNBDRead(t *testing.T) { + sudo := skipUnlessCanNBD(t) + dev := findFreeNBD(t, sudo) + + const pageSize = 4096 + data := make([]byte, pageSize*10) + rand.Read(data) + + addr, _, cleanup := startTestServer(t, data, pageSize) + defer cleanup() + + nbdClientConnect(t, sudo, addr, dev, pageSize) + t.Cleanup(func() { nbdClientDisconnect(sudo, dev) }) + time.Sleep(200 * time.Millisecond) + + // Read first page. + got := devRead(t, sudo, dev, 0, pageSize) + if !bytes.Equal(got, data[:pageSize]) { + t.Fatalf("first page mismatch: got %x... want %x...", got[:16], data[:16]) + } + + // Read a page in the middle. + off := 5 * pageSize + got = devRead(t, sudo, dev, off, pageSize) + if !bytes.Equal(got, data[off:off+pageSize]) { + t.Fatalf("middle page mismatch at offset %d", off) + } + + // Read last page. + off = 9 * pageSize + got = devRead(t, sudo, dev, off, pageSize) + if !bytes.Equal(got, data[off:off+pageSize]) { + t.Fatalf("last page mismatch at offset %d", off) + } +} + +func TestLinuxNBDWriteAndReadBack(t *testing.T) { + sudo := skipUnlessCanNBD(t) + dev := findFreeNBD(t, sudo) + + const pageSize = 4096 + original := make([]byte, pageSize*4) + rand.Read(original) + + addr, _, cleanup := startTestServer(t, original, pageSize) + defer cleanup() + + nbdClientConnect(t, sudo, addr, dev, pageSize) + t.Cleanup(func() { nbdClientDisconnect(sudo, dev) }) + time.Sleep(200 * time.Millisecond) + + // Write a full page of 0xBE. + writeData := bytes.Repeat([]byte{0xBE}, pageSize) + devWrite(t, sudo, dev, 0, writeData) + + // Read it back. + got := devRead(t, sudo, dev, 0, pageSize) + if !bytes.Equal(got, writeData) { + t.Fatalf("written page read-back mismatch: got %x... want %x...", got[:16], writeData[:16]) + } + + // Verify an unmodified page is still the original data. + got = devRead(t, sudo, dev, pageSize, pageSize) + if !bytes.Equal(got, original[pageSize:2*pageSize]) { + t.Fatal("unmodified page should match original") + } + + // Write a second distinct pattern to page 2. + writeData2 := bytes.Repeat([]byte{0xEF}, pageSize) + devWrite(t, sudo, dev, 2*pageSize, writeData2) + + got = devRead(t, sudo, dev, 2*pageSize, pageSize) + if !bytes.Equal(got, writeData2) { + t.Fatal("second write read-back mismatch") + } +} + +func TestLinuxNBDWritesLostOnReconnect(t *testing.T) { + sudo := skipUnlessCanNBD(t) + dev := findFreeNBD(t, sudo) + + const pageSize = 4096 + original := make([]byte, pageSize*4) + rand.Read(original) + + addr, _, cleanup := startTestServer(t, original, pageSize) + defer cleanup() + + // First connection: write data via the kernel. + nbdClientConnect(t, sudo, addr, dev, pageSize) + time.Sleep(200 * time.Millisecond) + + writeData := bytes.Repeat([]byte{0xAA}, pageSize) + devWrite(t, sudo, dev, 0, writeData) + + // Confirm the write is visible. + got := devRead(t, sudo, dev, 0, pageSize) + if !bytes.Equal(got, writeData) { + t.Fatalf("write not visible: got %x... want %x...", got[:16], writeData[:16]) + } + + // Disconnect. + nbdClientDisconnect(sudo, dev) + time.Sleep(200 * time.Millisecond) + + // Reconnect: the server's per-connection writes should be gone. + nbdClientConnect(t, sudo, addr, dev, pageSize) + t.Cleanup(func() { nbdClientDisconnect(sudo, dev) }) + time.Sleep(200 * time.Millisecond) + + got = devRead(t, sudo, dev, 0, pageSize) + if !bytes.Equal(got, original[:pageSize]) { + t.Fatalf("writes should be lost after reconnect: got %x... want %x...", + got[:16], original[:16]) + } +} + +func TestLinuxNBDMultipleConnections(t *testing.T) { + sudo := skipUnlessCanNBD(t) + + // Find two free devices. + dev1 := findFreeNBD(t, sudo) + dev2 := "" + for i := 0; i < 16; i++ { + d := fmt.Sprintf("/dev/nbd%d", i) + if d == dev1 { + continue + } + if _, err := os.Stat(d); err != nil { + continue + } + var cmd *exec.Cmd + if sudo != "" { + cmd = exec.Command(sudo, "nbd-client", "-c", d) + } else { + cmd = exec.Command("nbd-client", "-c", d) + } + if err := cmd.Run(); err != nil { + dev2 = d + break + } + } + if dev2 == "" { + t.Skip("skipping: need two free /dev/nbdX devices") + } + + const pageSize = 4096 + original := make([]byte, pageSize*4) + rand.Read(original) + + addr, _, cleanup := startTestServer(t, original, pageSize) + defer cleanup() + + nbdClientConnect(t, sudo, addr, dev1, pageSize) + t.Cleanup(func() { nbdClientDisconnect(sudo, dev1) }) + nbdClientConnect(t, sudo, addr, dev2, pageSize) + t.Cleanup(func() { nbdClientDisconnect(sudo, dev2) }) + time.Sleep(200 * time.Millisecond) + + // Both should see the same original data. + got1 := devRead(t, sudo, dev1, 0, pageSize) + got2 := devRead(t, sudo, dev2, 0, pageSize) + if !bytes.Equal(got1, original[:pageSize]) || !bytes.Equal(got2, original[:pageSize]) { + t.Fatal("both connections should see original data") + } + + // Write on dev1 should not be visible on dev2. + writeData := bytes.Repeat([]byte{0xCC}, pageSize) + devWrite(t, sudo, dev1, 0, writeData) + + got1 = devRead(t, sudo, dev1, 0, pageSize) + if !bytes.Equal(got1, writeData) { + t.Fatal("dev1 should see its own write") + } + + got2 = devRead(t, sudo, dev2, 0, pageSize) + if !bytes.Equal(got2, original[:pageSize]) { + t.Fatalf("dev2 should still see original: got %x... want %x...", + got2[:16], original[:16]) + } +} From 77f2bf65a75bb484c023e1866c7d3f6b22db3e19 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 1 Mar 2026 18:29:53 +0000 Subject: [PATCH 04/28] add more metrics Signed-off-by: Brad Fitzpatrick Migrated-from: bradfitz/guestbd@8aa0ef0009ab7e23bd56c454b57b2b7296886e4f --- guestbd/cache.go | 32 +++++++++++------- guestbd/conn.go | 23 +++++++++++-- guestbd/guestbd_test.go | 51 ++++++++++++++++++++++++++++ guestbd/main.go | 1 + guestbd/readonly.go | 40 ++++++++++++++-------- guestbd/server.go | 74 ++++++++++++++++++++++++++++++++++++----- 6 files changed, 185 insertions(+), 36 deletions(-) diff --git a/guestbd/cache.go b/guestbd/cache.go index 21d8a5e..0f28955 100644 --- a/guestbd/cache.go +++ b/guestbd/cache.go @@ -2,8 +2,10 @@ package main import ( "container/list" + "expvar" "sync" - "sync/atomic" + + "tailscale.com/metrics" ) // pageCache is a content-addressable LRU cache of page data, @@ -16,10 +18,14 @@ type pageCache struct { items map[pageHash]*list.Element lru *list.List // front = most recently used - hits atomic.Int64 - misses atomic.Int64 - evictions atomic.Int64 - size atomic.Int64 + // counter_guestbd_cache{path="hits|misses|evictions"} + path metrics.LabelMap + + // gauge_guestbd_cache_entries + entries expvar.Int + + // gauge_guestbd_cache_bytes + bytes expvar.Int } type cacheEntry struct { @@ -28,13 +34,13 @@ type cacheEntry struct { } func newPageCache(maxPages, pageSize int) *pageCache { - c := &pageCache{ + return &pageCache{ maxPages: maxPages, pageSize: pageSize, items: make(map[pageHash]*list.Element), lru: list.New(), + path: metrics.LabelMap{Label: "path"}, } - return c } // Get returns the page data for the given hash, if present in the cache. @@ -44,10 +50,10 @@ func (c *pageCache) Get(h pageHash) ([]byte, bool) { if elem, ok := c.items[h]; ok { c.lru.MoveToFront(elem) - c.hits.Add(1) + c.path.Add("hits", 1) return elem.Value.(*cacheEntry).data, true } - c.misses.Add(1) + c.path.Add("misses", 1) return nil, false } @@ -68,7 +74,8 @@ func (c *pageCache) Put(h pageHash, data []byte) { entry := &cacheEntry{hash: h, data: d} elem := c.lru.PushFront(entry) c.items[h] = elem - c.size.Add(1) + c.entries.Add(1) + c.bytes.Add(int64(c.pageSize)) for c.lru.Len() > c.maxPages { c.evict() @@ -83,6 +90,7 @@ func (c *pageCache) evict() { c.lru.Remove(elem) entry := elem.Value.(*cacheEntry) delete(c.items, entry.hash) - c.evictions.Add(1) - c.size.Add(-1) + c.path.Add("evictions", 1) + c.entries.Add(-1) + c.bytes.Add(-int64(c.pageSize)) } diff --git a/guestbd/conn.go b/guestbd/conn.go index 781dee6..df8f078 100644 --- a/guestbd/conn.go +++ b/guestbd/conn.go @@ -62,7 +62,10 @@ func (c *Conn) readPageData(pageNum int64) ([]byte, error) { dp, dirty := c.dirtyPages[pageNum] c.mu.Unlock() + c.server.readPages.Add(1) + if dirty { + c.server.readPath.Add("from_write", 1) // Try the global cache first. if data, ok := c.server.cache.Get(dp.hash); ok { return data, nil @@ -77,13 +80,26 @@ func (c *Conn) readPageData(pageNum int64) ([]byte, error) { return buf, nil } - data, _, err := c.roFile.readPage(pageNum, c.server.cache) - return data, err + data, _, result, err := c.roFile.readPage(pageNum, c.server.cache) + if err != nil { + return nil, err + } + switch result { + case readFromCache: + c.server.readPath.Add("base_mem", 1) + case readFromDiskCold: + c.server.readPath.Add("base_disk_cold", 1) + case readFromDiskMiss: + c.server.readPath.Add("base_disk_miss", 1) + } + return data, nil } // handleRead handles an NBD read request, assembling data from // potentially multiple pages. func (c *Conn) handleRead(offset, length uint64) ([]byte, error) { + c.server.readBytes.Add(int64(length)) + result := make([]byte, length) pageSize := uint64(c.server.pageSize) @@ -114,6 +130,8 @@ func (c *Conn) handleWrite(offset uint64, data []byte) error { length := uint64(len(data)) pageSize := uint64(c.server.pageSize) + c.server.writeBytes.Add(int64(length)) + pos := uint64(0) for pos < length { absOffset := offset + pos @@ -160,6 +178,7 @@ func (c *Conn) handleWrite(offset uint64, data []byte) error { c.mu.Unlock() c.server.cache.Put(h, pageData) + c.server.writePages.Add(1) pos += n } return nil diff --git a/guestbd/guestbd_test.go b/guestbd/guestbd_test.go index 6032300..0e3b032 100644 --- a/guestbd/guestbd_test.go +++ b/guestbd/guestbd_test.go @@ -679,6 +679,57 @@ func TestInodeSharing(t *testing.T) { } } +func TestReconnectHitsCache(t *testing.T) { + const pageSize = 4096 + const numPages = 20 + data := make([]byte, pageSize*numPages) + rand.Read(data) + + addr, srv, cleanup := startTestServer(t, data, pageSize) + defer cleanup() + + // First connection: read all pages, populating the cache. + c1 := newTestClient(t, addr) + for i := 0; i < numPages; i++ { + c1.read(uint64(i*pageSize), pageSize) + } + c1.disconnect() + + // Snapshot read path counters. + diskColdBefore := srv.readPath.Get("base_disk_cold").Value() + diskMissBefore := srv.readPath.Get("base_disk_miss").Value() + baseMemBefore := srv.readPath.Get("base_mem").Value() + + // Second connection: same inode, all pages should come from cache. + c2 := newTestClient(t, addr) + defer c2.disconnect() + for i := 0; i < numPages; i++ { + got := c2.read(uint64(i*pageSize), pageSize) + want := data[i*pageSize : (i+1)*pageSize] + if !bytes.Equal(got, want) { + t.Fatalf("page %d mismatch on reconnect", i) + } + } + + diskColdAfter := srv.readPath.Get("base_disk_cold").Value() + diskMissAfter := srv.readPath.Get("base_disk_miss").Value() + baseMemAfter := srv.readPath.Get("base_mem").Value() + + newCold := diskColdAfter - diskColdBefore + newMiss := diskMissAfter - diskMissBefore + newMem := baseMemAfter - baseMemBefore + + if newCold != 0 { + t.Errorf("expected 0 base_disk_cold reads after reconnect, got %d", newCold) + } + if newMiss != 0 { + t.Errorf("expected 0 base_disk_miss reads after reconnect, got %d", newMiss) + } + if newMem != int64(numPages) { + t.Errorf("expected %d base_mem reads after reconnect, got %d", numPages, newMem) + } +} + func BenchmarkRead(b *testing.B) { const pageSize = 4096 data := make([]byte, pageSize*100) diff --git a/guestbd/main.go b/guestbd/main.go index fb10c16..9c304ea 100644 --- a/guestbd/main.go +++ b/guestbd/main.go @@ -31,6 +31,7 @@ func main() { } srv := NewServer(*flagFile, *flagPageSize, *flagMaxMem) + srv.initExpvar() // Debug HTTP server with tsweb. debugMux := http.NewServeMux() diff --git a/guestbd/readonly.go b/guestbd/readonly.go index 0e1b6ba..dd2dd0e 100644 --- a/guestbd/readonly.go +++ b/guestbd/readonly.go @@ -44,32 +44,43 @@ func newReadonlyFile(f *os.File, size int64, pageSize int) *readonlyFile { } } -// numPages returns the total number of pages in the file. -func (r *readonlyFile) numPages() int64 { - return int64(len(r.pageHashes)) +// fileInfo returns the FileInfo for the underlying file. +func (r *readonlyFile) fileInfo() os.FileInfo { + fi, _ := r.f.Stat() + return fi } -// readPage reads page n from the backing file and returns its data and hash. -// It uses the cache and lazily populates pageHashes. -func (r *readonlyFile) readPage(n int64, cache *pageCache) ([]byte, pageHash, error) { +// readResult describes where a page read was served from. +type readResult int + +const ( + readFromCache readResult = iota // hash known, data in LRU cache + readFromDiskCold // hash unknown (first read of this page), read from disk + readFromDiskMiss // hash known but evicted from LRU cache, re-read from disk +) + +// readPage reads page n from the backing file and returns its data, hash, +// and how the read was served (cache hit, cold disk read, or cache miss disk read). +func (r *readonlyFile) readPage(n int64, cache *pageCache) (data []byte, hash pageHash, result readResult, err error) { r.mu.Lock() h := r.pageHashes[n] r.mu.Unlock() var zeroHash pageHash - if h != zeroHash { + hashKnown := h != zeroHash + if hashKnown { // Already have the hash; try cache. - if data, ok := cache.Get(h); ok { - return data, h, nil + if d, ok := cache.Get(h); ok { + return d, h, readFromCache, nil } } // Need to read from disk. buf := make([]byte, r.pageSize) offset := n * int64(r.pageSize) - nr, err := r.f.ReadAt(buf, offset) - if err != nil && err != io.EOF { - return nil, pageHash{}, err + nr, readErr := r.f.ReadAt(buf, offset) + if readErr != nil && readErr != io.EOF { + return nil, pageHash{}, 0, readErr } // Zero-fill remainder (last page may be short). for i := nr; i < r.pageSize; i++ { @@ -85,5 +96,8 @@ func (r *readonlyFile) readPage(n int64, cache *pageCache) ([]byte, pageHash, er r.mu.Unlock() cache.Put(h, buf) - return buf, h, nil + if hashKnown { + return buf, h, readFromDiskMiss, nil + } + return buf, h, readFromDiskCold, nil } diff --git a/guestbd/server.go b/guestbd/server.go index 08e103f..edc107b 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -2,13 +2,13 @@ package main import ( "encoding/binary" + "expvar" "fmt" "io" "log" "net" "os" "sync" - "sync/atomic" "tailscale.com/metrics" "tailscale.com/util/set" @@ -24,9 +24,16 @@ type Server struct { readonlyFiles map[inodeKey]*readonlyFile conns set.Set[*Conn] - activeConns atomic.Int64 - totalConns atomic.Int64 - ops metrics.LabelMap + totalConns expvar.Int // counter_guestbd_total_conns + activeConns expvar.Int // gauge_guestbd_active_conns + ops metrics.LabelMap // counter_guestbd_nbd_ops{type=...} + readPath metrics.LabelMap // counter_guestbd_read_path{type=...} + readBytes expvar.Int // counter_guestbd_read_bytes + readPages expvar.Int // counter_guestbd_read_pages + writeBytes expvar.Int // counter_guestbd_write_bytes + writePages expvar.Int // counter_guestbd_write_pages + baseImagesActive expvar.Int // gauge_guestbd_base_images_active + baseImagesCached expvar.Int // gauge_guestbd_base_images_cached } // NewServer creates a new guestbd server. @@ -43,15 +50,42 @@ func NewServer(filePath string, pageSize int, maxMem int64) *Server { pageSize: pageSize, cache: newPageCache(maxPages, pageSize), readonlyFiles: make(map[inodeKey]*readonlyFile), - conns: make(set.Set[*Conn]), - ops: metrics.LabelMap{Label: "op"}, + conns: make(set.Set[*Conn]), + ops: metrics.LabelMap{Label: "type"}, + readPath: metrics.LabelMap{Label: "type"}, } return srv } +// initExpvar publishes the server's metrics to expvar. +// Called once from main; not called in tests to avoid duplicate registration panics. +func (s *Server) initExpvar() { + expvar.Publish("counter_guestbd_total_conns", &s.totalConns) + expvar.Publish("gauge_guestbd_active_conns", &s.activeConns) + expvar.Publish("counter_guestbd_nbd_ops", &s.ops) + expvar.Publish("counter_guestbd_read_path", &s.readPath) + expvar.Publish("counter_guestbd_read_bytes", &s.readBytes) + expvar.Publish("counter_guestbd_read_pages", &s.readPages) + expvar.Publish("counter_guestbd_write_bytes", &s.writeBytes) + expvar.Publish("counter_guestbd_write_pages", &s.writePages) + expvar.Publish("counter_guestbd_cache", &s.cache.path) + expvar.Publish("gauge_guestbd_cache_entries", &s.cache.entries) + expvar.Publish("gauge_guestbd_cache_bytes", &s.cache.bytes) + + expvar.Publish("gauge_guestbd_base_images_active", &s.baseImagesActive) + expvar.Publish("gauge_guestbd_base_images_cached", &s.baseImagesCached) + + ps := new(expvar.Int) + ps.Set(int64(s.pageSize)) + expvar.Publish("gauge_guestbd_page_size", ps) +} + // getReadonlyFile returns the readonlyFile for the backing file, -// sharing it across connections with the same inode. +// sharing it across connections with the same inode. The readonlyFile +// and its pageHashes table persist even when refcount drops to zero, +// so that subsequent connections to the same inode benefit from the +// already-computed hash table (which is the index into the LRU cache). func (s *Server) getReadonlyFile() (*readonlyFile, error) { f, err := os.Open(s.filePath) if err != nil { @@ -69,25 +103,46 @@ func (s *Server) getReadonlyFile() (*readonlyFile, error) { if ro, ok := s.readonlyFiles[key]; ok { ro.mu.Lock() + wasIdle := ro.refcount == 0 ro.refcount++ ro.mu.Unlock() + if wasIdle { + s.baseImagesActive.Add(1) + } f.Close() return ro, nil } ro := newReadonlyFile(f, fi.Size(), s.pageSize) s.readonlyFiles[key] = ro + s.baseImagesActive.Add(1) // new entry starts with refcount 1 + s.baseImagesCached.Add(1) return ro, nil } -// releaseReadonlyFile decrements the refcount and cleans up if zero. +// releaseReadonlyFile decrements the refcount. If the refcount reaches +// zero, it checks whether the file on disk still points to the same +// inode. If the file is gone or has a different inode, the readonlyFile +// is removed and its fd closed — no future client can use it. Otherwise +// it stays in the map so the pageHashes index remains available for the +// next connection. func (s *Server) releaseReadonlyFile(ro *readonlyFile) { ro.mu.Lock() ro.refcount-- rc := ro.refcount ro.mu.Unlock() - if rc <= 0 { + if rc > 0 { + return + } + + s.baseImagesActive.Add(-1) + + // Check whether the file on disk still matches this inode. + fi, err := os.Stat(s.filePath) + if err != nil || fileInodeKey(fi) != fileInodeKey(ro.fileInfo()) { + // File is gone or replaced; no future client will ever + // open this inode. Clean it up. s.mu.Lock() for k, v := range s.readonlyFiles { if v == ro { @@ -97,6 +152,7 @@ func (s *Server) releaseReadonlyFile(ro *readonlyFile) { } s.mu.Unlock() ro.f.Close() + s.baseImagesCached.Add(-1) } } From 7f4f483265df7be59897238bc5e489077f5d148e Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 1 Mar 2026 18:35:51 +0000 Subject: [PATCH 05/28] move zeroPageHash to server, connect readonlyFile to Server Signed-off-by: Brad Fitzpatrick Migrated-from: bradfitz/guestbd@dd673e16a9f0bccda97560c6ebd6015610cd348b --- guestbd/conn.go | 2 +- guestbd/page.go | 8 -------- guestbd/readonly.go | 20 ++++++++++++-------- guestbd/server.go | 14 +++++++------- 4 files changed, 20 insertions(+), 24 deletions(-) diff --git a/guestbd/conn.go b/guestbd/conn.go index df8f078..25b4d63 100644 --- a/guestbd/conn.go +++ b/guestbd/conn.go @@ -80,7 +80,7 @@ func (c *Conn) readPageData(pageNum int64) ([]byte, error) { return buf, nil } - data, _, result, err := c.roFile.readPage(pageNum, c.server.cache) + data, _, result, err := c.roFile.readPage(pageNum) if err != nil { return nil, err } diff --git a/guestbd/page.go b/guestbd/page.go index adac0ba..5a8da4c 100644 --- a/guestbd/page.go +++ b/guestbd/page.go @@ -6,14 +6,6 @@ import "crypto/sha256" // A zero value means the page has not been read from disk yet. type pageHash [sha256.Size]byte -// zeroPageHash is the sha256 of a page filled entirely with zero bytes. -// This is different from the zero value of pageHash, which means "not yet computed". -var zeroPageHash pageHash - -func initZeroPageHash(pageSize int) { - zeroPageHash = sha256.Sum256(make([]byte, pageSize)) -} - func hashPage(data []byte) pageHash { return sha256.Sum256(data) } diff --git a/guestbd/readonly.go b/guestbd/readonly.go index dd2dd0e..20bbfe9 100644 --- a/guestbd/readonly.go +++ b/guestbd/readonly.go @@ -21,24 +21,25 @@ func fileInodeKey(fi os.FileInfo) inodeKey { // readonlyFile represents a shared read-only backing file, keyed by inode. // Multiple connections to the same file share one readonlyFile. type readonlyFile struct { + srv *Server mu sync.Mutex f *os.File size int64 refcount int32 - pageSize int // pageHashes is lazily computed per page. // A zero value means the page has not been read from disk yet. - // A value equal to zeroPageHash means the page is all zeros. + // A value equal to Server.zeroPageHash means the page is all zeros. pageHashes []pageHash } -func newReadonlyFile(f *os.File, size int64, pageSize int) *readonlyFile { +func newReadonlyFile(srv *Server, f *os.File, size int64) *readonlyFile { + pageSize := srv.pageSize numPages := (size + int64(pageSize) - 1) / int64(pageSize) return &readonlyFile{ + srv: srv, f: f, size: size, - pageSize: pageSize, refcount: 1, pageHashes: make([]pageHash, numPages), } @@ -61,11 +62,14 @@ const ( // readPage reads page n from the backing file and returns its data, hash, // and how the read was served (cache hit, cold disk read, or cache miss disk read). -func (r *readonlyFile) readPage(n int64, cache *pageCache) (data []byte, hash pageHash, result readResult, err error) { +func (r *readonlyFile) readPage(n int64) (data []byte, hash pageHash, result readResult, err error) { r.mu.Lock() h := r.pageHashes[n] r.mu.Unlock() + cache := r.srv.cache + pageSize := r.srv.pageSize + var zeroHash pageHash hashKnown := h != zeroHash if hashKnown { @@ -76,14 +80,14 @@ func (r *readonlyFile) readPage(n int64, cache *pageCache) (data []byte, hash pa } // Need to read from disk. - buf := make([]byte, r.pageSize) - offset := n * int64(r.pageSize) + buf := make([]byte, pageSize) + offset := n * int64(pageSize) nr, readErr := r.f.ReadAt(buf, offset) if readErr != nil && readErr != io.EOF { return nil, pageHash{}, 0, readErr } // Zero-fill remainder (last page may be short). - for i := nr; i < r.pageSize; i++ { + for i := nr; i < pageSize; i++ { buf[i] = 0 } diff --git a/guestbd/server.go b/guestbd/server.go index edc107b..f25644f 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -16,10 +16,11 @@ import ( // Server is the main guestbd NBD server. type Server struct { - mu sync.Mutex - filePath string - pageSize int - cache *pageCache + mu sync.Mutex + filePath string + pageSize int + zeroPageHash pageHash // sha256 of a page filled entirely with zero bytes + cache *pageCache readonlyFiles map[inodeKey]*readonlyFile conns set.Set[*Conn] @@ -43,11 +44,10 @@ func NewServer(filePath string, pageSize int, maxMem int64) *Server { maxPages = 1 } - initZeroPageHash(pageSize) - srv := &Server{ filePath: filePath, pageSize: pageSize, + zeroPageHash: hashPage(make([]byte, pageSize)), cache: newPageCache(maxPages, pageSize), readonlyFiles: make(map[inodeKey]*readonlyFile), conns: make(set.Set[*Conn]), @@ -113,7 +113,7 @@ func (s *Server) getReadonlyFile() (*readonlyFile, error) { return ro, nil } - ro := newReadonlyFile(f, fi.Size(), s.pageSize) + ro := newReadonlyFile(s, f, fi.Size()) s.readonlyFiles[key] = ro s.baseImagesActive.Add(1) // new entry starts with refcount 1 s.baseImagesCached.Add(1) From ad409fca6a345044a5a5e1548244ec8ab5d947cc Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 1 Mar 2026 18:42:10 +0000 Subject: [PATCH 06/28] add read/write size histogram metrics Signed-off-by: Brad Fitzpatrick Migrated-from: bradfitz/guestbd@2317712546d9ba233461f011dd2913424979bc18 --- guestbd/server.go | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/guestbd/server.go b/guestbd/server.go index f25644f..70601bc 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -31,10 +31,12 @@ type Server struct { readPath metrics.LabelMap // counter_guestbd_read_path{type=...} readBytes expvar.Int // counter_guestbd_read_bytes readPages expvar.Int // counter_guestbd_read_pages - writeBytes expvar.Int // counter_guestbd_write_bytes - writePages expvar.Int // counter_guestbd_write_pages - baseImagesActive expvar.Int // gauge_guestbd_base_images_active - baseImagesCached expvar.Int // gauge_guestbd_base_images_cached + writeBytes expvar.Int // counter_guestbd_write_bytes + writePages expvar.Int // counter_guestbd_write_pages + readSizeHist *metrics.Histogram // histogram_guestbd_read_size_bytes + writeSizeHist *metrics.Histogram // histogram_guestbd_write_size_bytes + baseImagesActive expvar.Int // gauge_guestbd_base_images_active + baseImagesCached expvar.Int // gauge_guestbd_base_images_cached } // NewServer creates a new guestbd server. @@ -44,15 +46,23 @@ func NewServer(filePath string, pageSize int, maxMem int64) *Server { maxPages = 1 } + // Powers of two from 1 to 4MB. + var sizeBuckets []float64 + for v := float64(1); v <= 4*1024*1024; v *= 2 { + sizeBuckets = append(sizeBuckets, v) + } + srv := &Server{ filePath: filePath, pageSize: pageSize, zeroPageHash: hashPage(make([]byte, pageSize)), cache: newPageCache(maxPages, pageSize), readonlyFiles: make(map[inodeKey]*readonlyFile), - conns: make(set.Set[*Conn]), - ops: metrics.LabelMap{Label: "type"}, - readPath: metrics.LabelMap{Label: "type"}, + conns: make(set.Set[*Conn]), + ops: metrics.LabelMap{Label: "type"}, + readPath: metrics.LabelMap{Label: "type"}, + readSizeHist: metrics.NewHistogram(sizeBuckets), + writeSizeHist: metrics.NewHistogram(sizeBuckets), } return srv @@ -69,6 +79,8 @@ func (s *Server) initExpvar() { expvar.Publish("counter_guestbd_read_pages", &s.readPages) expvar.Publish("counter_guestbd_write_bytes", &s.writeBytes) expvar.Publish("counter_guestbd_write_pages", &s.writePages) + expvar.Publish("histogram_guestbd_read_size_bytes", s.readSizeHist) + expvar.Publish("histogram_guestbd_write_size_bytes", s.writeSizeHist) expvar.Publish("counter_guestbd_cache", &s.cache.path) expvar.Publish("gauge_guestbd_cache_entries", &s.cache.entries) expvar.Publish("gauge_guestbd_cache_bytes", &s.cache.bytes) @@ -319,6 +331,7 @@ func (s *Server) serveTransmission(c *Conn) error { switch req.Type { case nbdCmdRead: s.ops.Add("read", 1) + s.readSizeHist.Observe(float64(req.Length)) data, err := c.handleRead(req.Offset, uint64(req.Length)) if err != nil { if werr := s.sendReply(rw, req.Handle, nbdEIO, nil); werr != nil { @@ -332,6 +345,7 @@ func (s *Server) serveTransmission(c *Conn) error { case nbdCmdWrite: s.ops.Add("write", 1) + s.writeSizeHist.Observe(float64(req.Length)) data := make([]byte, req.Length) if _, err := io.ReadFull(rw, data); err != nil { return fmt.Errorf("reading write data: %w", err) From 87caedc01e5ee47bde4018931306e6b1bf3fe596 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 1 Mar 2026 18:47:07 +0000 Subject: [PATCH 07/28] add metric details to README Signed-off-by: Brad Fitzpatrick Migrated-from: bradfitz/guestbd@f6661c93861b85579e0695e2c7b893e71cc5fa20 --- guestbd/README.md | 48 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/guestbd/README.md b/guestbd/README.md index a533584..d291238 100644 --- a/guestbd/README.md +++ b/guestbd/README.md @@ -66,13 +66,53 @@ password-less sudo access, set up ndb mount for testing. ### Observability -The server uses tailscale.com/tsweb and its De +The server uses tailscale.com/tsweb (including its DebugHandler) to +expose pprof, expvar, and a Prometheus-compatible `/debug/varz` endpoint +on the `--debug-addr` (default `:8080`). + +Metrics use normal expvar metrics (which tsweb Prometheus-ifies) and +tailscale.com/metrics's LabelMap and Histogram types. + +#### Counters + +| Metric | Description | +|--------|-------------| +| `guestbd_total_conns` | Total TCP connections accepted | +| `guestbd_nbd_ops{type=read\|write\|disconnect\|flush\|trim}` | NBD operations by type | +| `guestbd_read_bytes` | Total bytes read by clients | +| `guestbd_read_pages` | Total page reads (dirty + base layer) | +| `guestbd_read_path{type=base_mem\|base_disk_cold\|base_disk_miss\|from_write}` | Read source breakdown | +| `guestbd_write_bytes` | Total bytes written by clients | +| `guestbd_write_pages` | Total dirty pages written | +| `guestbd_cache{path=hits\|misses\|evictions}` | Page cache operations | + +#### Gauges + +| Metric | Description | +|--------|-------------| +| `guestbd_active_conns` | Currently connected clients | +| `guestbd_cache_entries` | Pages in the LRU cache | +| `guestbd_cache_bytes` | Bytes used by the LRU cache | +| `guestbd_base_images_active` | readonlyFile entries with active connections | +| `guestbd_base_images_cached` | readonlyFile entries in memory (including idle) | +| `guestbd_page_size` | Configured page size | + +#### Histograms + +| Metric | Description | +|--------|-------------| +| `guestbd_read_size_bytes` | Distribution of NBD read request sizes | +| `guestbd_write_size_bytes` | Distribution of NBD write request sizes | + +The `read_path` metric is particularly useful for diagnosing cache effectiveness: + +- **`base_mem`** — page hash was known and data was in the LRU cache (ideal) +- **`base_disk_cold`** — page hash was unknown (first read ever, or readonlyFile was recreated due to inode change); had to read from disk +- **`base_disk_miss`** — page hash was known but data was evicted from the LRU cache; had to re-read from disk (indicates cache is too small) +- **`from_write`** — read of a page that was previously written on this connection ## Imports The server uses tailscale.com/tsweb (including its DebugHandler), tailscale.com/util/set.{Set,HandleSet} etc as needed, but doesn't depend on tsnet. - -Metrics use normal expvar metrics (which tsweb Prometheus-ifies) and -tailscale.com/metrics's MutliLabelMap if/as needed. From 2583ca6816524d1ac72c55be230b063aef7d8ccb Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 1 Mar 2026 19:05:17 +0000 Subject: [PATCH 08/28] use bufio.Writer, neuter sync Signed-off-by: Brad Fitzpatrick Migrated-from: bradfitz/guestbd@35365149040a126e3618067f4ca99acf5c4f2023 --- guestbd/readonly.go | 2 +- guestbd/server.go | 102 +++++++++++++++++++++++++++++--------------- 2 files changed, 69 insertions(+), 35 deletions(-) diff --git a/guestbd/readonly.go b/guestbd/readonly.go index 20bbfe9..99980ac 100644 --- a/guestbd/readonly.go +++ b/guestbd/readonly.go @@ -15,7 +15,7 @@ type inodeKey struct { func fileInodeKey(fi os.FileInfo) inodeKey { stat := fi.Sys().(*syscall.Stat_t) - return inodeKey{dev: stat.Dev, ino: stat.Ino} + return inodeKey{dev: uint64(stat.Dev), ino: uint64(stat.Ino)} } // readonlyFile represents a shared read-only backing file, keyed by inode. diff --git a/guestbd/server.go b/guestbd/server.go index 70601bc..a93ea89 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -1,6 +1,7 @@ package main import ( + "bufio" "encoding/binary" "expvar" "fmt" @@ -210,19 +211,23 @@ func (s *Server) HandleConn(nc net.Conn) { // serveNBD runs the NBD protocol on the given connection. func (s *Server) serveNBD(c *Conn) error { - rw := c.tcpConn + r := c.tcpConn + bw := bufio.NewWriterSize(c.tcpConn, 1<<20) // 1MB // Phase 1: Newstyle handshake. var handshake [18]byte binary.BigEndian.PutUint64(handshake[0:8], nbdMagic) binary.BigEndian.PutUint64(handshake[8:16], nbdOptsMagic) binary.BigEndian.PutUint16(handshake[16:18], uint16(nbdFlagFixedNewstyle)) - if _, err := rw.Write(handshake[:]); err != nil { + if _, err := bw.Write(handshake[:]); err != nil { return fmt.Errorf("writing handshake: %w", err) } + if err := bw.Flush(); err != nil { + return fmt.Errorf("flushing handshake: %w", err) + } var clientFlagsBuf [4]byte - if _, err := io.ReadFull(rw, clientFlagsBuf[:]); err != nil { + if _, err := io.ReadFull(r, clientFlagsBuf[:]); err != nil { return fmt.Errorf("reading client flags: %w", err) } cflags := binary.BigEndian.Uint32(clientFlagsBuf[:]) @@ -234,7 +239,7 @@ func (s *Server) serveNBD(c *Conn) error { for { var optHeader [16]byte - if _, err := io.ReadFull(rw, optHeader[:]); err != nil { + if _, err := io.ReadFull(r, optHeader[:]); err != nil { return fmt.Errorf("reading option: %w", err) } optMagic := binary.BigEndian.Uint64(optHeader[0:8]) @@ -246,7 +251,7 @@ func (s *Server) serveNBD(c *Conn) error { optData := make([]byte, optLen) if optLen > 0 { - if _, err := io.ReadFull(rw, optData); err != nil { + if _, err := io.ReadFull(r, optData); err != nil { return fmt.Errorf("reading option data: %w", err) } } @@ -256,19 +261,25 @@ func (s *Server) serveNBD(c *Conn) error { var reply [10]byte binary.BigEndian.PutUint64(reply[0:8], exportSize) binary.BigEndian.PutUint16(reply[8:10], txFlags) - if _, err := rw.Write(reply[:]); err != nil { + if _, err := bw.Write(reply[:]); err != nil { return err } if !noZeroes { var zeros [124]byte - if _, err := rw.Write(zeros[:]); err != nil { + if _, err := bw.Write(zeros[:]); err != nil { return err } } - return s.serveTransmission(c) + if err := bw.Flush(); err != nil { + return err + } + return s.serveTransmission(c, r, bw) case nbdOptAbort: - return s.sendOptReply(rw, optCode, nbdRepAck, nil) + if err := s.sendOptReply(bw, optCode, nbdRepAck, nil); err != nil { + return err + } + return bw.Flush() case nbdOptGo: // Send NBD_INFO_EXPORT. @@ -276,7 +287,7 @@ func (s *Server) serveNBD(c *Conn) error { binary.BigEndian.PutUint16(infoData[0:2], nbdInfoExport) binary.BigEndian.PutUint64(infoData[2:10], exportSize) binary.BigEndian.PutUint16(infoData[10:12], txFlags) - if err := s.sendOptReply(rw, optCode, nbdRepInfo, infoData[:]); err != nil { + if err := s.sendOptReply(bw, optCode, nbdRepInfo, infoData[:]); err != nil { return err } // Send NBD_INFO_BLOCK_SIZE. @@ -285,27 +296,36 @@ func (s *Server) serveNBD(c *Conn) error { binary.BigEndian.PutUint32(bsData[2:6], 1) // minimum binary.BigEndian.PutUint32(bsData[6:10], uint32(s.pageSize)) // preferred binary.BigEndian.PutUint32(bsData[10:14], 32*1024*1024) // maximum - if err := s.sendOptReply(rw, optCode, nbdRepInfo, bsData[:]); err != nil { + if err := s.sendOptReply(bw, optCode, nbdRepInfo, bsData[:]); err != nil { return err } // ACK - if err := s.sendOptReply(rw, optCode, nbdRepAck, nil); err != nil { + if err := s.sendOptReply(bw, optCode, nbdRepAck, nil); err != nil { + return err + } + if err := bw.Flush(); err != nil { return err } - return s.serveTransmission(c) + return s.serveTransmission(c, r, bw) case nbdOptList: // One export with empty name (default). var nameData [4]byte - if err := s.sendOptReply(rw, optCode, nbdRepServer, nameData[:]); err != nil { + if err := s.sendOptReply(bw, optCode, nbdRepServer, nameData[:]); err != nil { return err } - if err := s.sendOptReply(rw, optCode, nbdRepAck, nil); err != nil { + if err := s.sendOptReply(bw, optCode, nbdRepAck, nil); err != nil { + return err + } + if err := bw.Flush(); err != nil { return err } default: - if err := s.sendOptReply(rw, optCode, nbdRepErrUnsup, nil); err != nil { + if err := s.sendOptReply(bw, optCode, nbdRepErrUnsup, nil); err != nil { + return err + } + if err := bw.Flush(); err != nil { return err } } @@ -313,12 +333,10 @@ func (s *Server) serveNBD(c *Conn) error { } // serveTransmission handles the NBD transmission phase. -func (s *Server) serveTransmission(c *Conn) error { - rw := c.tcpConn - +func (s *Server) serveTransmission(c *Conn, r io.Reader, bw *bufio.Writer) error { for { var req nbdRequest - if err := binary.Read(rw, binary.BigEndian, &req); err != nil { + if err := binary.Read(r, binary.BigEndian, &req); err != nil { if err == io.EOF || err == io.ErrUnexpectedEOF { return nil } @@ -334,12 +352,18 @@ func (s *Server) serveTransmission(c *Conn) error { s.readSizeHist.Observe(float64(req.Length)) data, err := c.handleRead(req.Offset, uint64(req.Length)) if err != nil { - if werr := s.sendReply(rw, req.Handle, nbdEIO, nil); werr != nil { + if werr := s.sendReply(bw, req.Handle, nbdEIO, nil); werr != nil { + return werr + } + if werr := bw.Flush(); werr != nil { return werr } continue } - if err := s.sendReply(rw, req.Handle, 0, data); err != nil { + if err := s.sendReply(bw, req.Handle, 0, data); err != nil { + return err + } + if err := bw.Flush(); err != nil { return err } @@ -347,16 +371,22 @@ func (s *Server) serveTransmission(c *Conn) error { s.ops.Add("write", 1) s.writeSizeHist.Observe(float64(req.Length)) data := make([]byte, req.Length) - if _, err := io.ReadFull(rw, data); err != nil { + if _, err := io.ReadFull(r, data); err != nil { return fmt.Errorf("reading write data: %w", err) } if err := c.handleWrite(req.Offset, data); err != nil { - if werr := s.sendReply(rw, req.Handle, nbdEIO, nil); werr != nil { + if werr := s.sendReply(bw, req.Handle, nbdEIO, nil); werr != nil { + return werr + } + if werr := bw.Flush(); werr != nil { return werr } continue } - if err := s.sendReply(rw, req.Handle, 0, nil); err != nil { + if err := s.sendReply(bw, req.Handle, 0, nil); err != nil { + return err + } + if err := bw.Flush(); err != nil { return err } @@ -366,26 +396,30 @@ func (s *Server) serveTransmission(c *Conn) error { case nbdCmdFlush: s.ops.Add("flush", 1) - c.mu.Lock() - err := c.dirtyFile.Sync() - c.mu.Unlock() - errCode := uint32(0) - if err != nil { - errCode = nbdEIO + // No-op: dirty data is ephemeral and lost on disconnect, + // so there's no point in syncing to disk for guest VM performance. + if err := s.sendReply(bw, req.Handle, 0, nil); err != nil { + return err } - if err := s.sendReply(rw, req.Handle, errCode, nil); err != nil { + if err := bw.Flush(); err != nil { return err } case nbdCmdTrim: s.ops.Add("trim", 1) c.handleTrim(req.Offset, uint64(req.Length)) - if err := s.sendReply(rw, req.Handle, 0, nil); err != nil { + if err := s.sendReply(bw, req.Handle, 0, nil); err != nil { + return err + } + if err := bw.Flush(); err != nil { return err } default: - if err := s.sendReply(rw, req.Handle, nbdEINVAL, nil); err != nil { + if err := s.sendReply(bw, req.Handle, nbdEINVAL, nil); err != nil { + return err + } + if err := bw.Flush(); err != nil { return err } } From 2f33eae5d869296b7bf47023a832e3f92152b710 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 1 Mar 2026 19:24:46 +0000 Subject: [PATCH 09/28] add 32MB limit check Signed-off-by: Brad Fitzpatrick Migrated-from: bradfitz/guestbd@09cb293c103f8ab98ca4e0a116b049708c8ea029 --- guestbd/nbd.go | 3 +++ guestbd/server.go | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/guestbd/nbd.go b/guestbd/nbd.go index 95a10cc..889f172 100644 --- a/guestbd/nbd.go +++ b/guestbd/nbd.go @@ -47,6 +47,9 @@ const ( nbdCmdFlush uint16 = 3 nbdCmdTrim uint16 = 4 + // Maximum payload size per the NBD spec. + nbdMaxPayload uint32 = 32 * 1024 * 1024 // 32 MB + // Error codes (Linux errno values) nbdEIO uint32 = 5 nbdEINVAL uint32 = 22 diff --git a/guestbd/server.go b/guestbd/server.go index a93ea89..d787034 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -346,6 +346,10 @@ func (s *Server) serveTransmission(c *Conn, r io.Reader, bw *bufio.Writer) error return fmt.Errorf("bad request magic: %#x", req.Magic) } + if (req.Type == nbdCmdRead || req.Type == nbdCmdWrite) && req.Length > nbdMaxPayload { + return fmt.Errorf("request length %d exceeds NBD max payload %d", req.Length, nbdMaxPayload) + } + switch req.Type { case nbdCmdRead: s.ops.Add("read", 1) From 179e7aef0792780729c4c9f356fd9a38e591fc03 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 1 Mar 2026 19:39:08 +0000 Subject: [PATCH 10/28] reduce allocs, add another metric Signed-off-by: Brad Fitzpatrick Migrated-from: bradfitz/guestbd@4168a80333282af4fa395b8d01f52c6e5e37d4f7 --- guestbd/README.md | 1 + guestbd/conn.go | 12 ++++++------ guestbd/server.go | 31 +++++++++++++++++++++++++------ 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/guestbd/README.md b/guestbd/README.md index d291238..ea9b70e 100644 --- a/guestbd/README.md +++ b/guestbd/README.md @@ -96,6 +96,7 @@ tailscale.com/metrics's LabelMap and Histogram types. | `guestbd_base_images_active` | readonlyFile entries with active connections | | `guestbd_base_images_cached` | readonlyFile entries in memory (including idle) | | `guestbd_page_size` | Configured page size | +| `guestbd_max_dirty_bytes` | Dirty page bytes of the connection with the most dirty pages | #### Histograms diff --git a/guestbd/conn.go b/guestbd/conn.go index 25b4d63..d649350 100644 --- a/guestbd/conn.go +++ b/guestbd/conn.go @@ -96,11 +96,11 @@ func (c *Conn) readPageData(pageNum int64) ([]byte, error) { } // handleRead handles an NBD read request, assembling data from -// potentially multiple pages. -func (c *Conn) handleRead(offset, length uint64) ([]byte, error) { +// potentially multiple pages into dst. +func (c *Conn) handleRead(dst []byte, offset uint64) error { + length := uint64(len(dst)) c.server.readBytes.Add(int64(length)) - result := make([]byte, length) pageSize := uint64(c.server.pageSize) pos := uint64(0) @@ -111,17 +111,17 @@ func (c *Conn) handleRead(offset, length uint64) ([]byte, error) { pageData, err := c.readPageData(pageNum) if err != nil { - return nil, err + return err } n := pageSize - pageOffset if n > length-pos { n = length - pos } - copy(result[pos:pos+n], pageData[pageOffset:pageOffset+n]) + copy(dst[pos:pos+n], pageData[pageOffset:pageOffset+n]) pos += n } - return result, nil + return nil } // handleWrite handles an NBD write request. Sub-page writes trigger diff --git a/guestbd/server.go b/guestbd/server.go index d787034..0d07e2a 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -9,6 +9,7 @@ import ( "log" "net" "os" + "slices" "sync" "tailscale.com/metrics" @@ -92,6 +93,22 @@ func (s *Server) initExpvar() { ps := new(expvar.Int) ps.Set(int64(s.pageSize)) expvar.Publish("gauge_guestbd_page_size", ps) + + expvar.Publish("gauge_guestbd_max_dirty_bytes", expvar.Func(func() any { + s.mu.Lock() + defer s.mu.Unlock() + + var max int64 + for c := range s.conns { + c.mu.Lock() + n := int64(len(c.dirtyPages)) * int64(s.pageSize) + c.mu.Unlock() + if n > max { + max = n + } + } + return max + })) } // getReadonlyFile returns the readonlyFile for the backing file, @@ -334,6 +351,8 @@ func (s *Server) serveNBD(c *Conn) error { // serveTransmission handles the NBD transmission phase. func (s *Server) serveTransmission(c *Conn, r io.Reader, bw *bufio.Writer) error { + var buf []byte + for { var req nbdRequest if err := binary.Read(r, binary.BigEndian, &req); err != nil { @@ -354,8 +373,8 @@ func (s *Server) serveTransmission(c *Conn, r io.Reader, bw *bufio.Writer) error case nbdCmdRead: s.ops.Add("read", 1) s.readSizeHist.Observe(float64(req.Length)) - data, err := c.handleRead(req.Offset, uint64(req.Length)) - if err != nil { + buf = slices.Grow(buf[:0], int(req.Length))[:req.Length] + if err := c.handleRead(buf, req.Offset); err != nil { if werr := s.sendReply(bw, req.Handle, nbdEIO, nil); werr != nil { return werr } @@ -364,7 +383,7 @@ func (s *Server) serveTransmission(c *Conn, r io.Reader, bw *bufio.Writer) error } continue } - if err := s.sendReply(bw, req.Handle, 0, data); err != nil { + if err := s.sendReply(bw, req.Handle, 0, buf); err != nil { return err } if err := bw.Flush(); err != nil { @@ -374,11 +393,11 @@ func (s *Server) serveTransmission(c *Conn, r io.Reader, bw *bufio.Writer) error case nbdCmdWrite: s.ops.Add("write", 1) s.writeSizeHist.Observe(float64(req.Length)) - data := make([]byte, req.Length) - if _, err := io.ReadFull(r, data); err != nil { + buf = slices.Grow(buf[:0], int(req.Length))[:req.Length] + if _, err := io.ReadFull(r, buf); err != nil { return fmt.Errorf("reading write data: %w", err) } - if err := c.handleWrite(req.Offset, data); err != nil { + if err := c.handleWrite(req.Offset, buf); err != nil { if werr := s.sendReply(bw, req.Handle, nbdEIO, nil); werr != nil { return werr } From 4244b40b8a7248873688c12a7d82c14bce8d8c92 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 1 Mar 2026 11:44:49 -0800 Subject: [PATCH 11/28] tweak, reflow README Signed-off-by: Brad Fitzpatrick Migrated-from: bradfitz/guestbd@abc699b2a6b44985d78cefa5a7ada15b1b9e5ee8 --- guestbd/README.md | 111 +++++++++++++++++++--------------------------- 1 file changed, 45 insertions(+), 66 deletions(-) diff --git a/guestbd/README.md b/guestbd/README.md index ea9b70e..67360d8 100644 --- a/guestbd/README.md +++ b/guestbd/README.md @@ -1,68 +1,53 @@ # guestbd -guestbd is a userspace NBD server (meant primarily for Linux) that -gives each TCP connection its own virtual read/write namespace on top -of a given file on disk that's only read-only. Then the TCP connection -breaks, any writes that were made by that client are lost. - -Any written data exists only in memory (for the first configured N -gigabytes) and only spills to disk as needed as a cache. - -Everything is content-addressable and de-duped. (use type pageHash -[sha256.Size]byte as a value) - -When a new TCP connection is accepted, the named file (given by a flag -to the binary) is opened, and its *os.File is compared against -existing open TCP connections (with os.SameFile) to see if it maps to -the same inode on disk. That inode than effectively maps to a "type -readonlyFile struct { f *os.File, size int64, refcount ... }" -singleton (for that inode) that contains lazily-computed state, -including a map of all 4K (configurable) pages in the file, and what -their sha256 checksum is. If a page is entirely zero bytes, the -checksum is skipped and a pre-computed sha256-of-zero is used -instead. This lets the checksum table double as a bitmap of whether a -readonlyFile page has been read: if the checksum is zero, it hasn't -been read yet and needs to be read from disk. - -Then, each page's 4KB data is stored just once for the whole process -as a function of its hash. This means if the file on disk is replaced -(mv newversion replacedfile) and gets a new inode, but there are -clients still open on the old version, and the new version has 80% of -the same contents overall (e.g. the ext4 filesystem was rebuilt with -mostly identical contents, but different inode dentry metadata), then -the cache will be mostly shared. - -That hash=>contents is stored in a configurably sized LRU cache -tracking hit rates, and used for all connections. No reference -counting is done on it; things simply age out if they no longer exist -in the base image or any old versions of the base image. - -Likewise, all writes update the underlying block device in 4KB units, -so a small 1KB write from a client ends up reading the existing 4KB, -mutating the 1KB in the middle of it, and then hashing the whole 4KB. - -It's expected that many connections will end up doing writes with the -same 4KB page contents (at different offsets in the block device), so -we also share that memory. But because written data needs to always be -re-readable later, we need to guarantee it either exists in memory or -on disk. To start simple, we don't try to be clever with how data is -lazily written to disk. Each connection maintains a table: +guestbd is a userspace NBD server that gives each TCP connection its own virtual +read/write namespace on top of a given file on disk that's only read-only. Then +the TCP connection breaks, any writes that were made by that client are lost. + +This is meant for the being the root block device for short-lived ephemeral CI +workload VMs, where we prefer speed over any sort of durability. + +Any written data exists only in memory (for the first configured N gigabytes) +and only spills to disk as needed as a cache. + +Everything is content-addressable and de-duped. + +When a new TCP connection is accepted, the named file (given by a flag to the +binary) is opened, and its *os.File is compared against existing open TCP +connections to see if it maps to the same inode on disk. That means all +connections to same base file image share the same cache. + +Then, each page's 4KB data is stored just once for the whole process as a +function of its hash. This means if the file on disk is replaced and gets a new +inode, but there are clients still open on the old version, and the new version +has 80% of the same contents overall (e.g. the ext4 filesystem was rebuilt with +mostly identical contents, but different inode dentry metadata), then the cache +will be mostly shared. + +That hash=>contents is stored in a configurably sized LRU cache tracking hit +rates, and used for all connections. No reference counting is done on it; things +simply age out if they no longer exist in the base image or any old versions of +the base image. + +Likewise, all writes update the underlying block device in 4KB units, so a small +1KB write from a client ends up reading the existing 4KB, mutating the 1KB in +the middle of it, and then hashing the whole 4KB. + +It's expected that many connections will end up doing writes with the same 4KB +page contents (at different offsets in the block device), so we also share that +memory. But because written data needs to always be re-readable later, we need +to guarantee it either exists in memory or on disk. To start simple, we don't +try to be clever with how data is lazily written to disk. Each connection +maintains a table: pageNum => *struct{ hash pageHash, dirtyPage int } -Where dirtyPage is the 4KB page number on disk (a per TCP connection -temp file that's open for read/write and then unlinked immediately) -where the page was written. Whenever a new dirty page is written, it -gets a monotonically increasing dirtyPageNum. All writes to the -connDirtyFile are in 4KB units, or whatever the flag-configured page -size is (which can be limited to a power of two) - -### Testing - -Tests cover all the cases. The tests try to avoid requiring root or -Linux for most tests, preferring a pure Go NDB client for testing, but -there are also tests that, when run on Linux as root or with -password-less sudo access, set up ndb mount for testing. +Where dirtyPage is the 4KB page number on disk (a per TCP connection temp file +that's open for read/write and then unlinked immediately) where the page was +written. Whenever a new dirty page is written, it gets a monotonically +increasing dirtyPageNum. All writes to the connDirtyFile are in 4KB units, or +whatever the flag-configured page size is (which can be limited to a power of +two) ### Observability @@ -111,9 +96,3 @@ The `read_path` metric is particularly useful for diagnosing cache effectiveness - **`base_disk_cold`** — page hash was unknown (first read ever, or readonlyFile was recreated due to inode change); had to read from disk - **`base_disk_miss`** — page hash was known but data was evicted from the LRU cache; had to re-read from disk (indicates cache is too small) - **`from_write`** — read of a page that was previously written on this connection - -## Imports - -The server uses tailscale.com/tsweb (including its DebugHandler), -tailscale.com/util/set.{Set,HandleSet} etc as needed, but doesn't -depend on tsnet. From 98457317be343f3cb833acf5d2222ed7a5e7ac32 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 1 Mar 2026 19:48:14 +0000 Subject: [PATCH 12/28] gofmt Signed-off-by: Brad Fitzpatrick Migrated-from: bradfitz/guestbd@c61c2a6725606fc780bd721589e74013253b0bfa --- guestbd/server.go | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/guestbd/server.go b/guestbd/server.go index 0d07e2a..6024ba7 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -18,27 +18,27 @@ import ( // Server is the main guestbd NBD server. type Server struct { - mu sync.Mutex - filePath string - pageSize int - zeroPageHash pageHash // sha256 of a page filled entirely with zero bytes - cache *pageCache + mu sync.Mutex + filePath string + pageSize int + zeroPageHash pageHash // sha256 of a page filled entirely with zero bytes + cache *pageCache readonlyFiles map[inodeKey]*readonlyFile conns set.Set[*Conn] - totalConns expvar.Int // counter_guestbd_total_conns - activeConns expvar.Int // gauge_guestbd_active_conns - ops metrics.LabelMap // counter_guestbd_nbd_ops{type=...} - readPath metrics.LabelMap // counter_guestbd_read_path{type=...} - readBytes expvar.Int // counter_guestbd_read_bytes - readPages expvar.Int // counter_guestbd_read_pages - writeBytes expvar.Int // counter_guestbd_write_bytes - writePages expvar.Int // counter_guestbd_write_pages - readSizeHist *metrics.Histogram // histogram_guestbd_read_size_bytes - writeSizeHist *metrics.Histogram // histogram_guestbd_write_size_bytes - baseImagesActive expvar.Int // gauge_guestbd_base_images_active - baseImagesCached expvar.Int // gauge_guestbd_base_images_cached + totalConns expvar.Int // counter_guestbd_total_conns + activeConns expvar.Int // gauge_guestbd_active_conns + ops metrics.LabelMap // counter_guestbd_nbd_ops{type=...} + readPath metrics.LabelMap // counter_guestbd_read_path{type=...} + readBytes expvar.Int // counter_guestbd_read_bytes + readPages expvar.Int // counter_guestbd_read_pages + writeBytes expvar.Int // counter_guestbd_write_bytes + writePages expvar.Int // counter_guestbd_write_pages + readSizeHist *metrics.Histogram // histogram_guestbd_read_size_bytes + writeSizeHist *metrics.Histogram // histogram_guestbd_write_size_bytes + baseImagesActive expvar.Int // gauge_guestbd_base_images_active + baseImagesCached expvar.Int // gauge_guestbd_base_images_cached } // NewServer creates a new guestbd server. @@ -145,7 +145,7 @@ func (s *Server) getReadonlyFile() (*readonlyFile, error) { ro := newReadonlyFile(s, f, fi.Size()) s.readonlyFiles[key] = ro - s.baseImagesActive.Add(1) // new entry starts with refcount 1 + s.baseImagesActive.Add(1) // new entry starts with refcount 1 s.baseImagesCached.Add(1) return ro, nil } From 001434956da51cc2b3227eb37384f10b7f4f4622 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 1 Mar 2026 19:53:32 +0000 Subject: [PATCH 13/28] tweak some style things Migrated-from: bradfitz/guestbd@5ee07441106b917c81ce60392a37dcf888a21ccb --- guestbd/cache.go | 26 ++++++++++++++------------ guestbd/page.go | 11 ----------- 2 files changed, 14 insertions(+), 23 deletions(-) delete mode 100644 guestbd/page.go diff --git a/guestbd/cache.go b/guestbd/cache.go index 0f28955..2903ccd 100644 --- a/guestbd/cache.go +++ b/guestbd/cache.go @@ -1,13 +1,23 @@ package main import ( + "bytes" "container/list" + "crypto/sha256" "expvar" "sync" "tailscale.com/metrics" ) +// pageHash is the sha256 hash of a page's contents. +// A zero value means the page has not been read from disk yet. +type pageHash [sha256.Size]byte + +func hashPage(data []byte) pageHash { + return sha256.Sum256(data) +} + // pageCache is a content-addressable LRU cache of page data, // shared across all connections. Pages are keyed by their sha256 hash. type pageCache struct { @@ -18,14 +28,9 @@ type pageCache struct { items map[pageHash]*list.Element lru *list.List // front = most recently used - // counter_guestbd_cache{path="hits|misses|evictions"} - path metrics.LabelMap - - // gauge_guestbd_cache_entries - entries expvar.Int - - // gauge_guestbd_cache_bytes - bytes expvar.Int + path metrics.LabelMap // counter_guestbd_cache{path="hits|misses|evictions"} + entries expvar.Int // gauge_guestbd_cache_entries + bytes expvar.Int // gauge_guestbd_cache_bytes } type cacheEntry struct { @@ -68,10 +73,7 @@ func (c *pageCache) Put(h pageHash, data []byte) { return } - d := make([]byte, len(data)) - copy(d, data) - - entry := &cacheEntry{hash: h, data: d} + entry := &cacheEntry{hash: h, data: bytes.Clone(data)} elem := c.lru.PushFront(entry) c.items[h] = elem c.entries.Add(1) diff --git a/guestbd/page.go b/guestbd/page.go deleted file mode 100644 index 5a8da4c..0000000 --- a/guestbd/page.go +++ /dev/null @@ -1,11 +0,0 @@ -package main - -import "crypto/sha256" - -// pageHash is the sha256 hash of a page's contents. -// A zero value means the page has not been read from disk yet. -type pageHash [sha256.Size]byte - -func hashPage(data []byte) pageHash { - return sha256.Sum256(data) -} From baa0aa83ddfeadcee4677b31d0ff94d99234ba72 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 4 Mar 2026 23:59:48 +0000 Subject: [PATCH 14/28] add qcow2 support Signed-off-by: Brad Fitzpatrick Migrated-from: bradfitz/guestbd@fa8555b9e0cf42935c8dac6a26ecd93639538d21 --- go.mod | 8 +++- go.sum | 4 ++ guestbd/guestbd_test.go | 87 ++++++++++++++++++++++++++++++++++++++++- guestbd/readonly.go | 10 +++-- guestbd/server.go | 18 ++++++++- 5 files changed, 118 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index 3e80c14..d39c9a1 100644 --- a/go.mod +++ b/go.mod @@ -1,10 +1,14 @@ module github.com/tailscale/tb -go 1.24.0 +go 1.25.7 -require tailscale.com v1.82.5 +require ( + github.com/bradfitz/qcow2 v0.0.0-20260303185237-93afc730382b + tailscale.com v1.82.5 +) require ( github.com/go-json-experiment/json v0.0.0-20250223041408-d3c622f1b874 // indirect + github.com/klauspost/compress v1.18.4 // indirect go4.org/mem v0.0.0-20240501181205-ae6ca9944745 // indirect go4.org/netipx v0.0.0-20231129151722-fdeea329fbba // indirect golang.org/x/crypto v0.35.0 // indirect diff --git a/go.sum b/go.sum index 7aa414e..160d223 100644 --- a/go.sum +++ b/go.sum @@ -1,9 +1,13 @@ +github.com/bradfitz/qcow2 v0.0.0-20260303185237-93afc730382b h1:D6BX3KA9oZ7WGn9D7fKsDNyQjrHb1kRsQTsyc7nv35s= +github.com/bradfitz/qcow2 v0.0.0-20260303185237-93afc730382b/go.mod h1:829+KZfDIY07C9uUedqANoUnClohrBIXLTQdhr3gC9w= github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/go-json-experiment/json v0.0.0-20250223041408-d3c622f1b874 h1:F8d1AJ6M9UQCavhwmO6ZsrYLfG8zVFWfEfMS2MXPkSY= github.com/go-json-experiment/json v0.0.0-20250223041408-d3c622f1b874/go.mod h1:TiCD2a1pcmjd7YnhGH0f/zKNcCD06B029pHhzV23c2M= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/klauspost/compress v1.18.4 h1:RPhnKRAQ4Fh8zU2FY/6ZFDwTVTxgJ/EMydqSTzE9a2c= +github.com/klauspost/compress v1.18.4/go.mod h1:R0h/fSBs8DE4ENlcrlib3PsXS61voFxhIs2DeRhCvJ4= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= diff --git a/guestbd/guestbd_test.go b/guestbd/guestbd_test.go index 0e3b032..a4fab2c 100644 --- a/guestbd/guestbd_test.go +++ b/guestbd/guestbd_test.go @@ -8,6 +8,8 @@ import ( "io" "net" "os" + "os/exec" + "path/filepath" "sync" "testing" ) @@ -252,7 +254,13 @@ func startTestServer(t *testing.T, fileData []byte, pageSize int) (addr string, } tmpFile.Close() - srv = NewServer(tmpFile.Name(), pageSize, int64(pageSize)*256) + return startTestServerFile(t, tmpFile.Name(), pageSize) +} + +func startTestServerFile(t *testing.T, filePath string, pageSize int) (addr string, srv *Server, cleanup func()) { + t.Helper() + + srv = NewServer(filePath, pageSize, int64(pageSize)*256) ln, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { @@ -271,7 +279,7 @@ func startTestServer(t *testing.T, fileData []byte, pageSize int) (addr string, return ln.Addr().String(), srv, func() { ln.Close() - os.Remove(tmpFile.Name()) + os.Remove(filePath) } } @@ -885,3 +893,78 @@ func TestMultiplePageSizes(t *testing.T) { }) } } + +func TestQcow2BaseImage(t *testing.T) { + qemuImg, err := exec.LookPath("qemu-img") + if err != nil { + t.Skip("qemu-img not found") + } + + const pageSize = 4096 + const diskSize = pageSize * 4 // 16KB virtual disk + + // Create a raw image with known data. + dir := t.TempDir() + rawPath := filepath.Join(dir, "test.raw") + data := make([]byte, diskSize) + for i := range data { + data[i] = byte(i % 251) + } + if err := os.WriteFile(rawPath, data, 0644); err != nil { + t.Fatal(err) + } + + // Convert to qcow2. + qcow2Path := filepath.Join(dir, "test.qcow2") + out, err := exec.Command(qemuImg, "convert", "-f", "raw", "-O", "qcow2", rawPath, qcow2Path).CombinedOutput() + if err != nil { + t.Fatalf("qemu-img convert: %v\n%s", err, out) + } + + addr, _, cleanup := startTestServerFile(t, qcow2Path, pageSize) + defer cleanup() + + c := newTestClient(t, addr) + defer c.disconnect() + + if c.exportSize != uint64(diskSize) { + t.Fatalf("export size: got %d, want %d", c.exportSize, diskSize) + } + + // Read full first page. + got := c.read(0, pageSize) + if !bytes.Equal(got, data[:pageSize]) { + t.Fatal("first page mismatch") + } + + // Read across page boundary. + off := uint64(pageSize - 100) + got = c.read(off, 200) + if !bytes.Equal(got, data[off:off+200]) { + t.Fatal("cross-page read mismatch") + } + + // Read last page. + off = uint64(pageSize * 3) + got = c.read(off, pageSize) + if !bytes.Equal(got, data[off:off+pageSize]) { + t.Fatal("last page mismatch") + } + + // Write and read back (ephemeral write on top of qcow2 base). + writeData := make([]byte, pageSize) + for i := range writeData { + writeData[i] = 0xAB + } + c.write(0, writeData) + got = c.read(0, pageSize) + if !bytes.Equal(got, writeData) { + t.Fatal("write on qcow2 mismatch") + } + + // Second page should still be original. + got = c.read(pageSize, pageSize) + if !bytes.Equal(got, data[pageSize:2*pageSize]) { + t.Fatal("unwritten page should still be original qcow2 data") + } +} diff --git a/guestbd/readonly.go b/guestbd/readonly.go index 99980ac..68d9515 100644 --- a/guestbd/readonly.go +++ b/guestbd/readonly.go @@ -23,8 +23,9 @@ func fileInodeKey(fi os.FileInfo) inodeKey { type readonlyFile struct { srv *Server mu sync.Mutex - f *os.File - size int64 + f *os.File // underlying file (for Stat, Close) + reader io.ReaderAt // used for page reads (may be *os.File or *qcow2.Image) + size int64 // virtual size (file size for raw, virtual disk size for qcow2) refcount int32 // pageHashes is lazily computed per page. @@ -33,12 +34,13 @@ type readonlyFile struct { pageHashes []pageHash } -func newReadonlyFile(srv *Server, f *os.File, size int64) *readonlyFile { +func newReadonlyFile(srv *Server, f *os.File, size int64, reader io.ReaderAt) *readonlyFile { pageSize := srv.pageSize numPages := (size + int64(pageSize) - 1) / int64(pageSize) return &readonlyFile{ srv: srv, f: f, + reader: reader, size: size, refcount: 1, pageHashes: make([]pageHash, numPages), @@ -82,7 +84,7 @@ func (r *readonlyFile) readPage(n int64) (data []byte, hash pageHash, result rea // Need to read from disk. buf := make([]byte, pageSize) offset := n * int64(pageSize) - nr, readErr := r.f.ReadAt(buf, offset) + nr, readErr := r.reader.ReadAt(buf, offset) if readErr != nil && readErr != io.EOF { return nil, pageHash{}, 0, readErr } diff --git a/guestbd/server.go b/guestbd/server.go index 6024ba7..216e07a 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -10,8 +10,10 @@ import ( "net" "os" "slices" + "strings" "sync" + "github.com/bradfitz/qcow2" "tailscale.com/metrics" "tailscale.com/util/set" ) @@ -143,7 +145,21 @@ func (s *Server) getReadonlyFile() (*readonlyFile, error) { return ro, nil } - ro := newReadonlyFile(s, f, fi.Size()) + var reader io.ReaderAt + size := fi.Size() + if strings.HasSuffix(s.filePath, ".qcow2") { + img, err := qcow2.Open(f) + if err != nil { + f.Close() + return nil, fmt.Errorf("opening qcow2 image: %w", err) + } + reader = img + size = img.Size() + } else { + reader = f + } + + ro := newReadonlyFile(s, f, size, reader) s.readonlyFiles[key] = ro s.baseImagesActive.Add(1) // new entry starts with refcount 1 s.baseImagesCached.Add(1) From e8c544b1fd36c021267b38a319238573bc2d4855 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 5 Mar 2026 00:03:36 +0000 Subject: [PATCH 15/28] document qcow2 support Migrated-from: bradfitz/guestbd@287490d91b4b91e2b127506114824fe7ddd0f8ab --- guestbd/README.md | 5 +++++ guestbd/main.go | 2 +- guestbd/readonly.go | 6 +++--- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/guestbd/README.md b/guestbd/README.md index 67360d8..4cdba56 100644 --- a/guestbd/README.md +++ b/guestbd/README.md @@ -4,6 +4,11 @@ guestbd is a userspace NBD server that gives each TCP connection its own virtual read/write namespace on top of a given file on disk that's only read-only. Then the TCP connection breaks, any writes that were made by that client are lost. +Both raw disk images and qcow2 images are supported. Files ending in `.qcow2` +are automatically opened as qcow2 (with support for deflate and zstd +compression). qcow2 support is provided by +[github.com/bradfitz/qcow2](https://pkg.go.dev/github.com/bradfitz/qcow2). + This is meant for the being the root block device for short-lived ephemeral CI workload VMs, where we prefer speed over any sort of durability. diff --git a/guestbd/main.go b/guestbd/main.go index 9c304ea..0fb5acc 100644 --- a/guestbd/main.go +++ b/guestbd/main.go @@ -14,7 +14,7 @@ import ( var ( flagListen = flag.String("listen", ":10809", "NBD listen address") - flagFile = flag.String("file", "", "path to the backing file to serve") + flagFile = flag.String("file", "", "path to the backing file to serve; files are treated as raw files, unless filename ends in .qcow2") flagPageSize = flag.Int("page-size", 4096, "page size in bytes (must be a power of two)") flagMaxMem = flag.Int64("max-mem", 1<<30, "maximum memory for page cache in bytes") flagDebug = flag.String("debug-addr", ":8080", "debug HTTP listen address") diff --git a/guestbd/readonly.go b/guestbd/readonly.go index 68d9515..621d622 100644 --- a/guestbd/readonly.go +++ b/guestbd/readonly.go @@ -23,9 +23,9 @@ func fileInodeKey(fi os.FileInfo) inodeKey { type readonlyFile struct { srv *Server mu sync.Mutex - f *os.File // underlying file (for Stat, Close) - reader io.ReaderAt // used for page reads (may be *os.File or *qcow2.Image) - size int64 // virtual size (file size for raw, virtual disk size for qcow2) + f *os.File // underlying file (for Stat, Close) + reader io.ReaderAt // used for page reads (may be *os.File or *qcow2.Image) + size int64 // virtual size (file size for raw, virtual disk size for qcow2) refcount int32 // pageHashes is lazily computed per page. From 641b3fde1bc75ef9bb9233498dafb58285abdeae Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 6 Mar 2026 05:14:20 +0000 Subject: [PATCH 16/28] split into package and ./cmd/guestbd binary Signed-off-by: Brad Fitzpatrick Migrated-from: bradfitz/guestbd@46f3a0b5c69e9a5dc0406fbd020242ca5668bf14 --- .../main.go => cmd/guestbd/guestbd-main.go | 21 +++++------ guestbd/.gitignore | 2 +- guestbd/cache.go | 8 ++++- guestbd/conn.go | 6 +++- guestbd/doc.go | 14 ++++++++ guestbd/guestbd_linux_test.go | 2 +- guestbd/guestbd_test.go | 2 +- guestbd/nbd.go | 2 +- guestbd/readonly.go | 6 +++- guestbd/server.go | 36 +++++++++++++++---- 10 files changed, 74 insertions(+), 25 deletions(-) rename guestbd/main.go => cmd/guestbd/guestbd-main.go (75%) create mode 100644 guestbd/doc.go diff --git a/guestbd/main.go b/cmd/guestbd/guestbd-main.go similarity index 75% rename from guestbd/main.go rename to cmd/guestbd/guestbd-main.go index 0fb5acc..6c9a0b9 100644 --- a/guestbd/main.go +++ b/cmd/guestbd/guestbd-main.go @@ -1,3 +1,7 @@ +// Command guestbd runs a guestbd NBD server. It listens for NBD client +// connections over TCP and serves a backing file (raw or qcow2) with +// per-connection ephemeral writes. It also runs a debug HTTP server +// exposing expvar metrics and pprof endpoints. package main import ( @@ -9,6 +13,7 @@ import ( "os" "os/signal" + "github.com/tailscale/tb/guestbd" "tailscale.com/tsweb" ) @@ -30,8 +35,8 @@ func main() { log.Fatal("--page-size must be a positive power of two") } - srv := NewServer(*flagFile, *flagPageSize, *flagMaxMem) - srv.initExpvar() + srv := guestbd.NewServer(*flagFile, *flagPageSize, *flagMaxMem) + srv.InitExpvar() // Debug HTTP server with tsweb. debugMux := http.NewServeMux() @@ -58,15 +63,7 @@ func main() { ln.Close() }() - for { - conn, err := ln.Accept() - if err != nil { - if ctx.Err() != nil { - break - } - log.Printf("accept: %v", err) - continue - } - go srv.HandleConn(conn) + if err := srv.Serve(ln); err != nil && ctx.Err() == nil { + log.Fatalf("serve: %v", err) } } diff --git a/guestbd/.gitignore b/guestbd/.gitignore index 81b2880..9301f2f 100644 --- a/guestbd/.gitignore +++ b/guestbd/.gitignore @@ -1,2 +1,2 @@ -guestbd +./guestbd *~ diff --git a/guestbd/cache.go b/guestbd/cache.go index 2903ccd..856865f 100644 --- a/guestbd/cache.go +++ b/guestbd/cache.go @@ -1,4 +1,4 @@ -package main +package guestbd import ( "bytes" @@ -14,6 +14,7 @@ import ( // A zero value means the page has not been read from disk yet. type pageHash [sha256.Size]byte +// hashPage returns the SHA-256 hash of data. func hashPage(data []byte) pageHash { return sha256.Sum256(data) } @@ -33,11 +34,15 @@ type pageCache struct { bytes expvar.Int // gauge_guestbd_cache_bytes } +// cacheEntry is a single element in the LRU list, pairing a page hash with +// its data. type cacheEntry struct { hash pageHash data []byte } +// newPageCache returns a new page cache that holds at most maxPages pages of +// the given pageSize. func newPageCache(maxPages, pageSize int) *pageCache { return &pageCache{ maxPages: maxPages, @@ -84,6 +89,7 @@ func (c *pageCache) Put(h pageHash, data []byte) { } } +// evict removes the least recently used entry from the cache. func (c *pageCache) evict() { elem := c.lru.Back() if elem == nil { diff --git a/guestbd/conn.go b/guestbd/conn.go index d649350..91933d7 100644 --- a/guestbd/conn.go +++ b/guestbd/conn.go @@ -1,4 +1,4 @@ -package main +package guestbd import ( "fmt" @@ -28,6 +28,8 @@ type dirtyPageInfo struct { dirtyNum int // index into the connection's dirtyFile } +// newConn creates a new Conn for the given TCP connection and read-only +// backing file. It creates an unlinked temporary file for dirty page storage. func newConn(srv *Server, tc net.Conn, ro *readonlyFile) (*Conn, error) { tmpFile, err := os.CreateTemp("", "guestbd-dirty-*") if err != nil { @@ -45,6 +47,8 @@ func newConn(srv *Server, tc net.Conn, ro *readonlyFile) (*Conn, error) { }, nil } +// Close releases the connection's dirty page storage. It does not close the +// underlying TCP connection. func (c *Conn) Close() error { c.mu.Lock() defer c.mu.Unlock() diff --git a/guestbd/doc.go b/guestbd/doc.go new file mode 100644 index 0000000..1128c95 --- /dev/null +++ b/guestbd/doc.go @@ -0,0 +1,14 @@ +// Package guestbd implements a userspace NBD (Network Block Device) server +// designed for ephemeral CI workload VMs. +// +// Each TCP connection gets its own read/write namespace layered on top of a +// shared read-only base image. Writes are per-connection and ephemeral: they +// are lost when the connection disconnects. The base image may be a raw disk +// file or a qcow2 image (auto-detected by file extension). +// +// Pages are content-addressed by their SHA-256 hash and de-duplicated in a +// global LRU cache shared across all connections. Multiple connections to the +// same backing file (same inode) share the same page hash table, so +// reconnecting clients benefit from previously computed hashes without +// re-reading from disk. +package guestbd diff --git a/guestbd/guestbd_linux_test.go b/guestbd/guestbd_linux_test.go index 1a96368..33457d6 100644 --- a/guestbd/guestbd_linux_test.go +++ b/guestbd/guestbd_linux_test.go @@ -1,4 +1,4 @@ -package main +package guestbd import ( "bytes" diff --git a/guestbd/guestbd_test.go b/guestbd/guestbd_test.go index a4fab2c..0b56302 100644 --- a/guestbd/guestbd_test.go +++ b/guestbd/guestbd_test.go @@ -1,4 +1,4 @@ -package main +package guestbd import ( "bytes" diff --git a/guestbd/nbd.go b/guestbd/nbd.go index 889f172..cc1732d 100644 --- a/guestbd/nbd.go +++ b/guestbd/nbd.go @@ -1,4 +1,4 @@ -package main +package guestbd // NBD protocol constants. // See https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md diff --git a/guestbd/readonly.go b/guestbd/readonly.go index 621d622..665c412 100644 --- a/guestbd/readonly.go +++ b/guestbd/readonly.go @@ -1,4 +1,4 @@ -package main +package guestbd import ( "io" @@ -13,6 +13,8 @@ type inodeKey struct { ino uint64 } +// fileInodeKey returns the inodeKey for the given FileInfo by extracting the +// device and inode numbers from its underlying syscall.Stat_t. func fileInodeKey(fi os.FileInfo) inodeKey { stat := fi.Sys().(*syscall.Stat_t) return inodeKey{dev: uint64(stat.Dev), ino: uint64(stat.Ino)} @@ -34,6 +36,8 @@ type readonlyFile struct { pageHashes []pageHash } +// newReadonlyFile creates a new readonlyFile with a pre-allocated pageHashes +// table sized for the given virtual disk size. The refcount starts at 1. func newReadonlyFile(srv *Server, f *os.File, size int64, reader io.ReaderAt) *readonlyFile { pageSize := srv.pageSize numPages := (size + int64(pageSize) - 1) / int64(pageSize) diff --git a/guestbd/server.go b/guestbd/server.go index 216e07a..d77eed2 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -1,4 +1,4 @@ -package main +package guestbd import ( "bufio" @@ -18,7 +18,9 @@ import ( "tailscale.com/util/set" ) -// Server is the main guestbd NBD server. +// Server is an NBD server that serves a single backing file to multiple +// concurrent clients. Each client connection gets an independent copy-on-write +// layer; writes are ephemeral and discarded on disconnect. type Server struct { mu sync.Mutex filePath string @@ -43,7 +45,11 @@ type Server struct { baseImagesCached expvar.Int // gauge_guestbd_base_images_cached } -// NewServer creates a new guestbd server. +// NewServer creates a new Server that serves the file at filePath. +// Files ending in ".qcow2" are opened as qcow2 images; all others are +// treated as raw disk images. +// The pageSize must be a positive power of two (commonly 4096). +// The maxMem parameter controls the maximum memory used by the shared page cache. func NewServer(filePath string, pageSize int, maxMem int64) *Server { maxPages := int(maxMem) / pageSize if maxPages < 1 { @@ -72,9 +78,10 @@ func NewServer(filePath string, pageSize int, maxMem int64) *Server { return srv } -// initExpvar publishes the server's metrics to expvar. -// Called once from main; not called in tests to avoid duplicate registration panics. -func (s *Server) initExpvar() { +// InitExpvar publishes the server's metrics to expvar. It should be called +// once before serving. It is not called in tests to avoid duplicate +// registration panics. +func (s *Server) InitExpvar() { expvar.Publish("counter_guestbd_total_conns", &s.totalConns) expvar.Publish("gauge_guestbd_active_conns", &s.activeConns) expvar.Publish("counter_guestbd_nbd_ops", &s.ops) @@ -202,6 +209,19 @@ func (s *Server) releaseReadonlyFile(ro *readonlyFile) { } } +// Serve accepts incoming connections on the listener ln, handling +// each one on a new goroutine. Serve blocks until the listener +// returns an error. The caller is responsible for closing ln. +func (s *Server) Serve(ln net.Listener) error { + for { + conn, err := ln.Accept() + if err != nil { + return err + } + go s.HandleConn(conn) + } +} + // HandleConn handles a new TCP connection, performing the NBD // handshake and entering the transmission phase. func (s *Server) HandleConn(nc net.Conn) { @@ -465,6 +485,8 @@ func (s *Server) serveTransmission(c *Conn, r io.Reader, bw *bufio.Writer) error } } +// sendOptReply writes an NBD option reply with the given option code, reply +// type, and optional payload data. func (s *Server) sendOptReply(w io.Writer, optCode uint32, replyType uint32, data []byte) error { var header [20]byte binary.BigEndian.PutUint64(header[0:8], nbdOptReplyMagic) @@ -482,6 +504,8 @@ func (s *Server) sendOptReply(w io.Writer, optCode uint32, replyType uint32, dat return nil } +// sendReply writes an NBD transmission-phase reply with the given handle, +// error code, and optional payload data. func (s *Server) sendReply(w io.Writer, handle uint64, errCode uint32, data []byte) error { var header [16]byte binary.BigEndian.PutUint32(header[0:4], nbdReplyMagic) From 3381e3759772e21eedd7904bff48e420652eaf19 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sat, 7 Mar 2026 19:42:27 +0000 Subject: [PATCH 17/28] rename Conn to Snapshot, decouple from TCP, add variadic ServerOptions Rename Conn to Snapshot to reflect that a writable layer can outlive individual TCP connections. Remove the net.Conn field from Snapshot so snapshots are independent of any particular connection. The NBD protocol code now receives the snapshot and TCP connection separately. Introduce a ServerOption variadic pattern for NewServer, moving pageSize (default 4096), maxMem (default 1GB), and the new shared-snapshot policy out of positional parameters. Add WithPageSize, WithMaxMem, and WithSharedSnapshot options. Snapshot now implements io.ReaderAt and io.WriterAt, exporting the read/write methods that were previously unexported handleRead and handleWrite. The NBD transmission code uses these directly. The cmd/guestbd binary gains a --shared-snapshot flag that causes all connections to share a single writable snapshot, allowing reconnecting clients to see previous writes. Migrated-from: bradfitz/guestbd@b551e3e0d910bae00289df9c5e8871b02c911447 --- cmd/guestbd/guestbd-main.go | 30 +++-- guestbd/doc.go | 14 ++- guestbd/guestbd_test.go | 4 +- guestbd/server.go | 194 +++++++++++++++++++++++-------- guestbd/{conn.go => snapshot.go} | 93 ++++++++------- 5 files changed, 230 insertions(+), 105 deletions(-) rename guestbd/{conn.go => snapshot.go} (62%) diff --git a/cmd/guestbd/guestbd-main.go b/cmd/guestbd/guestbd-main.go index 6c9a0b9..eaf30d3 100644 --- a/cmd/guestbd/guestbd-main.go +++ b/cmd/guestbd/guestbd-main.go @@ -1,7 +1,9 @@ // Command guestbd runs a guestbd NBD server. It listens for NBD client -// connections over TCP and serves a backing file (raw or qcow2) with -// per-connection ephemeral writes. It also runs a debug HTTP server -// exposing expvar metrics and pprof endpoints. +// connections over TCP and serves a backing file (raw or qcow2). By default, +// each connection gets its own ephemeral writable snapshot that is discarded +// on disconnect. With --shared-snapshot, all connections share a single +// writable snapshot so that reconnections see previous writes. +// It also runs a debug HTTP server exposing expvar metrics and pprof endpoints. package main import ( @@ -18,11 +20,12 @@ import ( ) var ( - flagListen = flag.String("listen", ":10809", "NBD listen address") - flagFile = flag.String("file", "", "path to the backing file to serve; files are treated as raw files, unless filename ends in .qcow2") - flagPageSize = flag.Int("page-size", 4096, "page size in bytes (must be a power of two)") - flagMaxMem = flag.Int64("max-mem", 1<<30, "maximum memory for page cache in bytes") - flagDebug = flag.String("debug-addr", ":8080", "debug HTTP listen address") + flagListen = flag.String("listen", ":10809", "NBD listen address") + flagFile = flag.String("file", "", "path to the backing file to serve; files are treated as raw files, unless filename ends in .qcow2") + flagPageSize = flag.Int("page-size", 4096, "page size in bytes (must be a power of two)") + flagMaxMem = flag.Int64("max-mem", 1<<30, "maximum memory for page cache in bytes") + flagDebug = flag.String("debug-addr", ":8080", "debug HTTP listen address") + flagSharedSnapshot = flag.Bool("shared-snapshot", false, "use a single shared writable snapshot for all connections instead of one per connection") ) func main() { @@ -35,7 +38,16 @@ func main() { log.Fatal("--page-size must be a positive power of two") } - srv := guestbd.NewServer(*flagFile, *flagPageSize, *flagMaxMem) + opts := []guestbd.ServerOption{ + guestbd.WithPageSize(*flagPageSize), + guestbd.WithMaxMem(*flagMaxMem), + } + if *flagSharedSnapshot { + opts = append(opts, guestbd.WithSharedSnapshot()) + } + + srv := guestbd.NewServer(*flagFile, opts...) + defer srv.Close() srv.InitExpvar() // Debug HTTP server with tsweb. diff --git a/guestbd/doc.go b/guestbd/doc.go index 1128c95..f9ad0fc 100644 --- a/guestbd/doc.go +++ b/guestbd/doc.go @@ -1,14 +1,16 @@ // Package guestbd implements a userspace NBD (Network Block Device) server // designed for ephemeral CI workload VMs. // -// Each TCP connection gets its own read/write namespace layered on top of a -// shared read-only base image. Writes are per-connection and ephemeral: they -// are lost when the connection disconnects. The base image may be a raw disk -// file or a qcow2 image (auto-detected by file extension). +// Each Snapshot provides a read/write namespace layered on top of a shared +// read-only base image. By default each TCP connection gets its own Snapshot +// whose writes are discarded on disconnect, but a Snapshot can also be shared +// across multiple connections so that reconnecting clients see previous writes. +// The base image may be a raw disk file or a qcow2 image (auto-detected by +// file extension). // // Pages are content-addressed by their SHA-256 hash and de-duplicated in a -// global LRU cache shared across all connections. Multiple connections to the -// same backing file (same inode) share the same page hash table, so +// global LRU cache shared across all snapshots. Multiple snapshots backed by +// the same file (same inode) share the same page hash table, so // reconnecting clients benefit from previously computed hashes without // re-reading from disk. package guestbd diff --git a/guestbd/guestbd_test.go b/guestbd/guestbd_test.go index 0b56302..0d01845 100644 --- a/guestbd/guestbd_test.go +++ b/guestbd/guestbd_test.go @@ -260,7 +260,7 @@ func startTestServer(t *testing.T, fileData []byte, pageSize int) (addr string, func startTestServerFile(t *testing.T, filePath string, pageSize int) (addr string, srv *Server, cleanup func()) { t.Helper() - srv = NewServer(filePath, pageSize, int64(pageSize)*256) + srv = NewServer(filePath, WithPageSize(pageSize), WithMaxMem(int64(pageSize)*256)) ln, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { @@ -751,7 +751,7 @@ func BenchmarkRead(b *testing.B) { tmpFile.Close() defer os.Remove(tmpFile.Name()) - srv := NewServer(tmpFile.Name(), pageSize, int64(pageSize)*256) + srv := NewServer(tmpFile.Name(), WithPageSize(pageSize), WithMaxMem(int64(pageSize)*256)) ln, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { diff --git a/guestbd/server.go b/guestbd/server.go index d77eed2..2fab723 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -18,9 +18,39 @@ import ( "tailscale.com/util/set" ) +// ServerOption configures optional Server parameters. +type ServerOption func(*serverConfig) + +type serverConfig struct { + pageSize int + maxMem int64 + sharedSnapshot bool +} + +// WithPageSize sets the page size in bytes. It must be a positive power of two. +// The default is 4096. +func WithPageSize(n int) ServerOption { + return func(c *serverConfig) { c.pageSize = n } +} + +// WithMaxMem sets the maximum memory used by the shared page cache. +// The default is 1GB. +func WithMaxMem(n int64) ServerOption { + return func(c *serverConfig) { c.maxMem = n } +} + +// WithSharedSnapshot makes all connections share a single writable Snapshot +// instead of each getting its own. This allows reconnecting clients to see +// writes from previous connections. +func WithSharedSnapshot() ServerOption { + return func(c *serverConfig) { c.sharedSnapshot = true } +} + // Server is an NBD server that serves a single backing file to multiple -// concurrent clients. Each client connection gets an independent copy-on-write -// layer; writes are ephemeral and discarded on disconnect. +// concurrent clients. Each client connection may get an independent +// copy-on-write Snapshot, or multiple connections may share a single Snapshot +// to allow reconnection to a previous writable state. The snapshot policy is +// configured via WithSharedSnapshot at construction time. type Server struct { mu sync.Mutex filePath string @@ -28,8 +58,11 @@ type Server struct { zeroPageHash pageHash // sha256 of a page filled entirely with zero bytes cache *pageCache + useSharedSnapshot bool // set by WithSharedSnapshot + sharedSnap *Snapshot // lazily created when useSharedSnapshot is true + readonlyFiles map[inodeKey]*readonlyFile - conns set.Set[*Conn] + snapshots set.Set[*Snapshot] totalConns expvar.Int // counter_guestbd_total_conns activeConns expvar.Int // gauge_guestbd_active_conns @@ -48,10 +81,17 @@ type Server struct { // NewServer creates a new Server that serves the file at filePath. // Files ending in ".qcow2" are opened as qcow2 images; all others are // treated as raw disk images. -// The pageSize must be a positive power of two (commonly 4096). -// The maxMem parameter controls the maximum memory used by the shared page cache. -func NewServer(filePath string, pageSize int, maxMem int64) *Server { - maxPages := int(maxMem) / pageSize +func NewServer(filePath string, opts ...ServerOption) *Server { + cfg := serverConfig{ + pageSize: 4096, + maxMem: 1 << 30, // 1GB default + } + for _, o := range opts { + o(&cfg) + } + + pageSize := cfg.pageSize + maxPages := int(cfg.maxMem) / pageSize if maxPages < 1 { maxPages = 1 } @@ -63,16 +103,17 @@ func NewServer(filePath string, pageSize int, maxMem int64) *Server { } srv := &Server{ - filePath: filePath, - pageSize: pageSize, - zeroPageHash: hashPage(make([]byte, pageSize)), - cache: newPageCache(maxPages, pageSize), - readonlyFiles: make(map[inodeKey]*readonlyFile), - conns: make(set.Set[*Conn]), - ops: metrics.LabelMap{Label: "type"}, - readPath: metrics.LabelMap{Label: "type"}, - readSizeHist: metrics.NewHistogram(sizeBuckets), - writeSizeHist: metrics.NewHistogram(sizeBuckets), + filePath: filePath, + pageSize: pageSize, + useSharedSnapshot: cfg.sharedSnapshot, + zeroPageHash: hashPage(make([]byte, pageSize)), + cache: newPageCache(maxPages, pageSize), + readonlyFiles: make(map[inodeKey]*readonlyFile), + snapshots: make(set.Set[*Snapshot]), + ops: metrics.LabelMap{Label: "type"}, + readPath: metrics.LabelMap{Label: "type"}, + readSizeHist: metrics.NewHistogram(sizeBuckets), + writeSizeHist: metrics.NewHistogram(sizeBuckets), } return srv @@ -108,7 +149,7 @@ func (s *Server) InitExpvar() { defer s.mu.Unlock() var max int64 - for c := range s.conns { + for c := range s.snapshots { c.mu.Lock() n := int64(len(c.dirtyPages)) * int64(s.pageSize) c.mu.Unlock() @@ -209,6 +250,74 @@ func (s *Server) releaseReadonlyFile(ro *readonlyFile) { } } +// NewSnapshot creates a new writable Snapshot backed by the server's base +// image. The caller is responsible for calling Close when the snapshot is no +// longer needed. +func (s *Server) NewSnapshot() (*Snapshot, error) { + ro, err := s.getReadonlyFile() + if err != nil { + return nil, fmt.Errorf("opening base image: %w", err) + } + snap, err := newSnapshot(s, ro) + if err != nil { + s.releaseReadonlyFile(ro) + return nil, err + } + s.mu.Lock() + s.snapshots.Add(snap) + s.mu.Unlock() + return snap, nil +} + +// Close cleans up server resources, including any shared Snapshot created +// via WithSharedSnapshot. +func (s *Server) Close() error { + s.mu.Lock() + snap := s.sharedSnap + s.sharedSnap = nil + s.mu.Unlock() + if snap != nil { + snap.Close() + } + return nil +} + +// getOrCreateSnapshot returns a snapshot for a new connection. If the server +// is configured with WithSharedSnapshot, it lazily creates a single shared +// snapshot and returns it with owned=false. Otherwise it creates a fresh +// per-connection snapshot with owned=true. The caller must close the snapshot +// when owned is true. +func (s *Server) getOrCreateSnapshot() (snap *Snapshot, owned bool, err error) { + if !s.useSharedSnapshot { + snap, err = s.NewSnapshot() + return snap, true, err + } + + s.mu.Lock() + snap = s.sharedSnap + s.mu.Unlock() + if snap != nil { + return snap, false, nil + } + + // Lazy creation; must not hold s.mu across NewSnapshot. + snap, err = s.NewSnapshot() + if err != nil { + return nil, false, err + } + + s.mu.Lock() + if s.sharedSnap != nil { + // Another goroutine created it first. + s.mu.Unlock() + snap.Close() + return s.sharedSnap, false, nil + } + s.sharedSnap = snap + s.mu.Unlock() + return snap, false, nil +} + // Serve accepts incoming connections on the listener ln, handling // each one on a new goroutine. Serve blocks until the listener // returns an error. The caller is responsible for closing ln. @@ -223,49 +332,36 @@ func (s *Server) Serve(ln net.Listener) error { } // HandleConn handles a new TCP connection, performing the NBD -// handshake and entering the transmission phase. +// handshake and entering the transmission phase. The snapshot policy +// (per-connection or shared) is determined by the server's configuration. func (s *Server) HandleConn(nc net.Conn) { s.totalConns.Add(1) s.activeConns.Add(1) defer s.activeConns.Add(-1) - ro, err := s.getReadonlyFile() + snap, owned, err := s.getOrCreateSnapshot() if err != nil { - log.Printf("open backing file for %v: %v", nc.RemoteAddr(), err) - nc.Close() - return - } - - c, err := newConn(s, nc, ro) - if err != nil { - log.Printf("create conn for %v: %v", nc.RemoteAddr(), err) - s.releaseReadonlyFile(ro) + log.Printf("create snapshot for %v: %v", nc.RemoteAddr(), err) nc.Close() return } - s.mu.Lock() - s.conns.Add(c) - s.mu.Unlock() - defer func() { - s.mu.Lock() - s.conns.Delete(c) - s.mu.Unlock() - c.Close() - s.releaseReadonlyFile(ro) + if owned { + snap.Close() + } nc.Close() }() - if err := s.serveNBD(c); err != nil { + if err := s.serveNBD(snap, nc); err != nil { log.Printf("nbd %v: %v", nc.RemoteAddr(), err) } } // serveNBD runs the NBD protocol on the given connection. -func (s *Server) serveNBD(c *Conn) error { - r := c.tcpConn - bw := bufio.NewWriterSize(c.tcpConn, 1<<20) // 1MB +func (s *Server) serveNBD(snap *Snapshot, nc net.Conn) error { + r := nc + bw := bufio.NewWriterSize(nc, 1<<20) // 1MB // Phase 1: Newstyle handshake. var handshake [18]byte @@ -287,7 +383,7 @@ func (s *Server) serveNBD(c *Conn) error { noZeroes := cflags&nbdFlagCNoZeroes != 0 // Phase 2: Option haggling. - exportSize := uint64(c.roFile.size) + exportSize := uint64(snap.roFile.size) txFlags := nbdFlagHasFlags | nbdFlagSendFlush | nbdFlagSendTrim for { @@ -326,7 +422,7 @@ func (s *Server) serveNBD(c *Conn) error { if err := bw.Flush(); err != nil { return err } - return s.serveTransmission(c, r, bw) + return s.serveTransmission(snap, r, bw) case nbdOptAbort: if err := s.sendOptReply(bw, optCode, nbdRepAck, nil); err != nil { @@ -359,7 +455,7 @@ func (s *Server) serveNBD(c *Conn) error { if err := bw.Flush(); err != nil { return err } - return s.serveTransmission(c, r, bw) + return s.serveTransmission(snap, r, bw) case nbdOptList: // One export with empty name (default). @@ -386,7 +482,7 @@ func (s *Server) serveNBD(c *Conn) error { } // serveTransmission handles the NBD transmission phase. -func (s *Server) serveTransmission(c *Conn, r io.Reader, bw *bufio.Writer) error { +func (s *Server) serveTransmission(snap *Snapshot, r io.Reader, bw *bufio.Writer) error { var buf []byte for { @@ -410,7 +506,7 @@ func (s *Server) serveTransmission(c *Conn, r io.Reader, bw *bufio.Writer) error s.ops.Add("read", 1) s.readSizeHist.Observe(float64(req.Length)) buf = slices.Grow(buf[:0], int(req.Length))[:req.Length] - if err := c.handleRead(buf, req.Offset); err != nil { + if _, err := snap.ReadAt(buf, int64(req.Offset)); err != nil { if werr := s.sendReply(bw, req.Handle, nbdEIO, nil); werr != nil { return werr } @@ -433,7 +529,7 @@ func (s *Server) serveTransmission(c *Conn, r io.Reader, bw *bufio.Writer) error if _, err := io.ReadFull(r, buf); err != nil { return fmt.Errorf("reading write data: %w", err) } - if err := c.handleWrite(req.Offset, buf); err != nil { + if _, err := snap.WriteAt(buf, int64(req.Offset)); err != nil { if werr := s.sendReply(bw, req.Handle, nbdEIO, nil); werr != nil { return werr } @@ -466,7 +562,7 @@ func (s *Server) serveTransmission(c *Conn, r io.Reader, bw *bufio.Writer) error case nbdCmdTrim: s.ops.Add("trim", 1) - c.handleTrim(req.Offset, uint64(req.Length)) + snap.handleTrim(req.Offset, uint64(req.Length)) if err := s.sendReply(bw, req.Handle, 0, nil); err != nil { return err } diff --git a/guestbd/conn.go b/guestbd/snapshot.go similarity index 62% rename from guestbd/conn.go rename to guestbd/snapshot.go index 91933d7..0dbafa0 100644 --- a/guestbd/conn.go +++ b/guestbd/snapshot.go @@ -2,19 +2,26 @@ package guestbd import ( "fmt" - "net" + "io" "os" "sync" ) -// Conn represents a single NBD client connection. -// Each connection has its own virtual read/write namespace: -// reads fall through to the shared readonlyFile, while writes -// are tracked per-connection and lost on disconnect. -type Conn struct { - server *Server - tcpConn net.Conn - roFile *readonlyFile +// Compile-time interface checks. +var ( + _ io.ReaderAt = (*Snapshot)(nil) + _ io.WriterAt = (*Snapshot)(nil) +) + +// Snapshot is a writable layer on top of a read-only base image. +// Reads fall through to the shared base image, while writes are tracked +// in the snapshot. A Snapshot can outlive individual TCP connections, +// allowing reconnections to reattach to the same writable state. +// +// Snapshot implements io.ReaderAt and io.WriterAt. +type Snapshot struct { + server *Server + roFile *readonlyFile mu sync.Mutex dirtyPages map[int64]*dirtyPageInfo // pageNum => dirty info @@ -22,15 +29,15 @@ type Conn struct { nextDirtyNum int // monotonically increasing } -// dirtyPageInfo tracks a written page for a connection. +// dirtyPageInfo tracks a written page in a snapshot. type dirtyPageInfo struct { hash pageHash - dirtyNum int // index into the connection's dirtyFile + dirtyNum int // index into the snapshot's dirtyFile } -// newConn creates a new Conn for the given TCP connection and read-only -// backing file. It creates an unlinked temporary file for dirty page storage. -func newConn(srv *Server, tc net.Conn, ro *readonlyFile) (*Conn, error) { +// newSnapshot creates a new Snapshot for the given read-only backing file. +// It creates an unlinked temporary file for dirty page storage. +func newSnapshot(srv *Server, ro *readonlyFile) (*Snapshot, error) { tmpFile, err := os.CreateTemp("", "guestbd-dirty-*") if err != nil { return nil, fmt.Errorf("creating temp file: %w", err) @@ -38,30 +45,35 @@ func newConn(srv *Server, tc net.Conn, ro *readonlyFile) (*Conn, error) { // Unlink immediately so it's cleaned up when the fd closes. os.Remove(tmpFile.Name()) - return &Conn{ + return &Snapshot{ server: srv, - tcpConn: tc, roFile: ro, dirtyPages: make(map[int64]*dirtyPageInfo), dirtyFile: tmpFile, }, nil } -// Close releases the connection's dirty page storage. It does not close the -// underlying TCP connection. -func (c *Conn) Close() error { +// Close releases the snapshot's dirty page storage and unregisters it from +// the server. It does not close any TCP connection using the snapshot. +func (c *Snapshot) Close() error { c.mu.Lock() - defer c.mu.Unlock() if c.dirtyFile != nil { c.dirtyFile.Close() c.dirtyFile = nil } + c.mu.Unlock() + + c.server.mu.Lock() + c.server.snapshots.Delete(c) + c.server.mu.Unlock() + + c.server.releaseReadonlyFile(c.roFile) return nil } // readPageData reads a full page, checking dirty pages first, // then falling back to the readonly file. -func (c *Conn) readPageData(pageNum int64) ([]byte, error) { +func (c *Snapshot) readPageData(pageNum int64) ([]byte, error) { c.mu.Lock() dp, dirty := c.dirtyPages[pageNum] c.mu.Unlock() @@ -74,7 +86,7 @@ func (c *Conn) readPageData(pageNum int64) ([]byte, error) { if data, ok := c.server.cache.Get(dp.hash); ok { return data, nil } - // Read from the connection's dirty file. + // Read from the snapshot's dirty file. buf := make([]byte, c.server.pageSize) _, err := c.dirtyFile.ReadAt(buf, int64(dp.dirtyNum)*int64(c.server.pageSize)) if err != nil { @@ -99,12 +111,13 @@ func (c *Conn) readPageData(pageNum int64) ([]byte, error) { return data, nil } -// handleRead handles an NBD read request, assembling data from -// potentially multiple pages into dst. -func (c *Conn) handleRead(dst []byte, offset uint64) error { - length := uint64(len(dst)) +// ReadAt reads len(p) bytes from the snapshot starting at byte offset off. +// It implements io.ReaderAt. +func (c *Snapshot) ReadAt(p []byte, off int64) (int, error) { + length := uint64(len(p)) c.server.readBytes.Add(int64(length)) + offset := uint64(off) pageSize := uint64(c.server.pageSize) pos := uint64(0) @@ -115,27 +128,29 @@ func (c *Conn) handleRead(dst []byte, offset uint64) error { pageData, err := c.readPageData(pageNum) if err != nil { - return err + return int(pos), err } n := pageSize - pageOffset if n > length-pos { n = length - pos } - copy(dst[pos:pos+n], pageData[pageOffset:pageOffset+n]) + copy(p[pos:pos+n], pageData[pageOffset:pageOffset+n]) pos += n } - return nil + return int(pos), nil } -// handleWrite handles an NBD write request. Sub-page writes trigger -// a read-modify-write cycle. -func (c *Conn) handleWrite(offset uint64, data []byte) error { - length := uint64(len(data)) +// WriteAt writes len(p) bytes to the snapshot starting at byte offset off. +// Sub-page writes trigger a read-modify-write cycle. +// It implements io.WriterAt. +func (c *Snapshot) WriteAt(p []byte, off int64) (int, error) { + length := uint64(len(p)) pageSize := uint64(c.server.pageSize) c.server.writeBytes.Add(int64(length)) + offset := uint64(off) pos := uint64(0) for pos < length { absOffset := offset + pos @@ -151,16 +166,16 @@ func (c *Conn) handleWrite(offset uint64, data []byte) error { if pageOffset == 0 && n == pageSize { // Full page write. pageData = make([]byte, pageSize) - copy(pageData, data[pos:pos+n]) + copy(pageData, p[pos:pos+n]) } else { // Partial page write: read-modify-write. existing, err := c.readPageData(pageNum) if err != nil { - return err + return int(pos), err } pageData = make([]byte, pageSize) copy(pageData, existing) - copy(pageData[pageOffset:], data[pos:pos+n]) + copy(pageData[pageOffset:], p[pos:pos+n]) } h := hashPage(pageData) @@ -172,7 +187,7 @@ func (c *Conn) handleWrite(offset uint64, data []byte) error { _, err := c.dirtyFile.WriteAt(pageData, int64(dirtyNum)*int64(pageSize)) if err != nil { c.mu.Unlock() - return fmt.Errorf("writing dirty page: %w", err) + return int(pos), fmt.Errorf("writing dirty page: %w", err) } c.dirtyPages[pageNum] = &dirtyPageInfo{ @@ -185,12 +200,12 @@ func (c *Conn) handleWrite(offset uint64, data []byte) error { c.server.writePages.Add(1) pos += n } - return nil + return int(pos), nil } // handleTrim forgets dirty pages in the given range, reverting // those pages to the base image. -func (c *Conn) handleTrim(offset, length uint64) { +func (c *Snapshot) handleTrim(offset, length uint64) { pageSize := uint64(c.server.pageSize) startPage := int64(offset / pageSize) endPage := int64((offset + length + pageSize - 1) / pageSize) From f1edfb2ed8a783083ce3f1da6a82fd2eed65d620 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 10 Mar 2026 14:04:15 +0000 Subject: [PATCH 18/28] add BaseImage interface with identity-keyed caching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the file-path-based server constructor with a BaseImageSource function that returns a BaseImage interface. BaseImage includes io.ReaderAt, io.Closer, Size, and BaseImageKey. The key enables equivalence-keyed caching: when a connection closes and a new one opens the same image (same dev/ino), the idle baseImageState is reused, preserving the page hash table and avoiding cold disk reads. - Add BaseImage interface (ReaderAt + Closer + Size + BaseImageKey) - Add BaseImageSource, FileSource, NewBaseImage constructors - Add WithMaxIdleBase(n) option (default 1) - Replace single roFile with roFiles map + roFileNoKey for nil-key sources - Add LRU eviction of idle keyed entries via evictIdleLocked - Rename readonlyFile → baseImageState with documented locking rules Migrated-from: bradfitz/guestbd@6ac39611b6cbd5a76028a77b4d19510710f4e6ae --- cmd/guestbd/guestbd-main.go | 2 +- guestbd/doc.go | 14 +- guestbd/guestbd_test.go | 208 +++++++++++++++++++++++-- guestbd/readonly.go | 72 ++++----- guestbd/server.go | 294 +++++++++++++++++++++++++++--------- guestbd/snapshot.go | 6 +- 6 files changed, 462 insertions(+), 134 deletions(-) diff --git a/cmd/guestbd/guestbd-main.go b/cmd/guestbd/guestbd-main.go index eaf30d3..92de675 100644 --- a/cmd/guestbd/guestbd-main.go +++ b/cmd/guestbd/guestbd-main.go @@ -46,7 +46,7 @@ func main() { opts = append(opts, guestbd.WithSharedSnapshot()) } - srv := guestbd.NewServer(*flagFile, opts...) + srv := guestbd.NewServer(guestbd.FileSource(*flagFile), opts...) defer srv.Close() srv.InitExpvar() diff --git a/guestbd/doc.go b/guestbd/doc.go index f9ad0fc..c7c974c 100644 --- a/guestbd/doc.go +++ b/guestbd/doc.go @@ -5,12 +5,14 @@ // read-only base image. By default each TCP connection gets its own Snapshot // whose writes are discarded on disconnect, but a Snapshot can also be shared // across multiple connections so that reconnecting clients see previous writes. -// The base image may be a raw disk file or a qcow2 image (auto-detected by -// file extension). +// +// The base image is provided as a [BaseImageSource] — a function returning a +// [BaseImage] — so callers can serve images from files, object stores, +// or memory. [BaseImage.BaseImageKey] enables equivalence-keyed caching of +// idle baseImageStates so that reconnecting clients reuse the page hash table. +// The [FileSource] helper provides the common file-based workflow, with +// automatic qcow2 detection by file extension. // // Pages are content-addressed by their SHA-256 hash and de-duplicated in a -// global LRU cache shared across all snapshots. Multiple snapshots backed by -// the same file (same inode) share the same page hash table, so -// reconnecting clients benefit from previously computed hashes without -// re-reading from disk. +// global LRU cache shared across all snapshots. package guestbd diff --git a/guestbd/guestbd_test.go b/guestbd/guestbd_test.go index 0d01845..4ba1898 100644 --- a/guestbd/guestbd_test.go +++ b/guestbd/guestbd_test.go @@ -260,7 +260,7 @@ func startTestServer(t *testing.T, fileData []byte, pageSize int) (addr string, func startTestServerFile(t *testing.T, filePath string, pageSize int) (addr string, srv *Server, cleanup func()) { t.Helper() - srv = NewServer(filePath, WithPageSize(pageSize), WithMaxMem(int64(pageSize)*256)) + srv = NewServer(FileSource(filePath), WithPageSize(pageSize), WithMaxMem(int64(pageSize)*256)) ln, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { @@ -668,7 +668,7 @@ func TestInodeSharing(t *testing.T) { addr, srv, cleanup := startTestServer(t, data, pageSize) defer cleanup() - // Two connections to the same file should share the same readonlyFile. + // Two connections to the same file should share the same baseImageState. c1 := newTestClient(t, addr) defer c1.disconnect() c2 := newTestClient(t, addr) @@ -679,11 +679,11 @@ func TestInodeSharing(t *testing.T) { c2.read(0, pageSize) srv.mu.Lock() - numRO := len(srv.readonlyFiles) + hasRO := len(srv.roFiles) > 0 srv.mu.Unlock() - if numRO != 1 { - t.Fatalf("expected 1 readonlyFile, got %d", numRO) + if !hasRO { + t.Fatal("expected roFiles to be non-empty") } } @@ -696,19 +696,21 @@ func TestReconnectHitsCache(t *testing.T) { addr, srv, cleanup := startTestServer(t, data, pageSize) defer cleanup() - // First connection: read all pages, populating the cache. + // First connection: read all pages, populating the hash table and cache. c1 := newTestClient(t, addr) for i := 0; i < numPages; i++ { c1.read(uint64(i*pageSize), pageSize) } c1.disconnect() - // Snapshot read path counters. + // Snapshot read path counters after first connection. diskColdBefore := srv.readPath.Get("base_disk_cold").Value() diskMissBefore := srv.readPath.Get("base_disk_miss").Value() baseMemBefore := srv.readPath.Get("base_mem").Value() - // Second connection: same inode, all pages should come from cache. + // Second connection after full disconnect: FileSource provides identity + // so the idle baseImageState is matched and reused, preserving the page + // hash table. All reads should come from cache (0 cold reads). c2 := newTestClient(t, addr) defer c2.disconnect() for i := 0; i < numPages; i++ { @@ -728,13 +730,195 @@ func TestReconnectHitsCache(t *testing.T) { newMem := baseMemAfter - baseMemBefore if newCold != 0 { - t.Errorf("expected 0 base_disk_cold reads after reconnect, got %d", newCold) + t.Errorf("expected 0 base_disk_cold reads, got %d", newCold) } if newMiss != 0 { - t.Errorf("expected 0 base_disk_miss reads after reconnect, got %d", newMiss) + t.Errorf("expected 0 base_disk_miss reads, got %d", newMiss) } if newMem != int64(numPages) { - t.Errorf("expected %d base_mem reads after reconnect, got %d", numPages, newMem) + t.Errorf("expected %d base_mem reads, got %d", numPages, newMem) + } +} + +func TestConcurrentSharesHashTable(t *testing.T) { + const pageSize = 4096 + const numPages = 20 + data := make([]byte, pageSize*numPages) + rand.Read(data) + + addr, srv, cleanup := startTestServer(t, data, pageSize) + defer cleanup() + + // First connection: read all pages, populating the hash table. + c1 := newTestClient(t, addr) + for i := 0; i < numPages; i++ { + c1.read(uint64(i*pageSize), pageSize) + } + + // Snapshot read path counters while c1 is still connected. + diskColdBefore := srv.readPath.Get("base_disk_cold").Value() + diskMissBefore := srv.readPath.Get("base_disk_miss").Value() + baseMemBefore := srv.readPath.Get("base_mem").Value() + + // Second connection while c1 is alive: shares the baseImageState + // and its page hash table, so all reads come from cache. + c2 := newTestClient(t, addr) + for i := 0; i < numPages; i++ { + got := c2.read(uint64(i*pageSize), pageSize) + want := data[i*pageSize : (i+1)*pageSize] + if !bytes.Equal(got, want) { + t.Fatalf("page %d mismatch", i) + } + } + c2.disconnect() + c1.disconnect() + + diskColdAfter := srv.readPath.Get("base_disk_cold").Value() + diskMissAfter := srv.readPath.Get("base_disk_miss").Value() + baseMemAfter := srv.readPath.Get("base_mem").Value() + + newCold := diskColdAfter - diskColdBefore + newMiss := diskMissAfter - diskMissBefore + newMem := baseMemAfter - baseMemBefore + + if newCold != 0 { + t.Errorf("expected 0 base_disk_cold reads, got %d", newCold) + } + if newMiss != 0 { + t.Errorf("expected 0 base_disk_miss reads, got %d", newMiss) + } + if newMem != int64(numPages) { + t.Errorf("expected %d base_mem reads, got %d", numPages, newMem) + } +} + +// noKeyBaseImage wraps a BaseImage and returns nil from BaseImageKey, +// disabling equivalence-keyed caching. +type noKeyBaseImage struct { + BaseImage +} + +func (n *noKeyBaseImage) BaseImageKey() any { return nil } + +func TestReconnectNoIdentity(t *testing.T) { + const pageSize = 4096 + const numPages = 20 + data := make([]byte, pageSize*numPages) + rand.Read(data) + + tmpFile, err := os.CreateTemp("", "guestbd-test-*") + if err != nil { + t.Fatalf("create temp file: %v", err) + } + if _, err := tmpFile.Write(data); err != nil { + t.Fatalf("write temp file: %v", err) + } + tmpFile.Close() + defer os.Remove(tmpFile.Name()) + + // Use a BaseImageSource that wraps FileSource's output with nil key. + inner := FileSource(tmpFile.Name()) + noKeySource := func() (BaseImage, error) { + base, err := inner() + if err != nil { + return nil, err + } + return &noKeyBaseImage{base}, nil + } + + srv := NewServer(noKeySource, WithPageSize(pageSize), WithMaxMem(int64(pageSize)*256)) + + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + go srv.HandleConn(conn) + } + }() + defer ln.Close() + addr := ln.Addr().String() + + // First connection: read all pages. + c1 := newTestClient(t, addr) + for i := 0; i < numPages; i++ { + c1.read(uint64(i*pageSize), pageSize) + } + c1.disconnect() + + // Snapshot counters. + diskColdBefore := srv.readPath.Get("base_disk_cold").Value() + + // Second connection: nil key means the old idle roFile is replaced, + // so all reads should be cold. + c2 := newTestClient(t, addr) + defer c2.disconnect() + for i := 0; i < numPages; i++ { + got := c2.read(uint64(i*pageSize), pageSize) + want := data[i*pageSize : (i+1)*pageSize] + if !bytes.Equal(got, want) { + t.Fatalf("page %d mismatch on reconnect", i) + } + } + + diskColdAfter := srv.readPath.Get("base_disk_cold").Value() + newCold := diskColdAfter - diskColdBefore + if newCold != int64(numPages) { + t.Errorf("expected %d base_disk_cold reads (no identity), got %d", numPages, newCold) + } +} + +func TestBaseImageReplaced(t *testing.T) { + const pageSize = 4096 + const numPages = 4 + data1 := make([]byte, pageSize*numPages) + rand.Read(data1) + + dir := t.TempDir() + filePath := filepath.Join(dir, "disk.raw") + if err := os.WriteFile(filePath, data1, 0644); err != nil { + t.Fatal(err) + } + + addr, _, cleanup := startTestServerFile(t, filePath, pageSize) + defer cleanup() + + // First connection: read all pages. + c1 := newTestClient(t, addr) + for i := 0; i < numPages; i++ { + got := c1.read(uint64(i*pageSize), pageSize) + if !bytes.Equal(got, data1[i*pageSize:(i+1)*pageSize]) { + t.Fatalf("page %d mismatch (first conn)", i) + } + } + c1.disconnect() + + // Replace the file: write new content to a temp file and rename + // (new inode → different identity key). + data2 := make([]byte, pageSize*numPages) + rand.Read(data2) + tmpPath := filepath.Join(dir, "disk.raw.new") + if err := os.WriteFile(tmpPath, data2, 0644); err != nil { + t.Fatal(err) + } + if err := os.Rename(tmpPath, filePath); err != nil { + t.Fatal(err) + } + + // Second connection: should see the new data. + c2 := newTestClient(t, addr) + defer c2.disconnect() + for i := 0; i < numPages; i++ { + got := c2.read(uint64(i*pageSize), pageSize) + if !bytes.Equal(got, data2[i*pageSize:(i+1)*pageSize]) { + t.Fatalf("page %d mismatch after file replacement: got %x... want %x...", + i, got[:8], data2[i*pageSize:i*pageSize+8]) + } } } @@ -751,7 +935,7 @@ func BenchmarkRead(b *testing.B) { tmpFile.Close() defer os.Remove(tmpFile.Name()) - srv := NewServer(tmpFile.Name(), WithPageSize(pageSize), WithMaxMem(int64(pageSize)*256)) + srv := NewServer(FileSource(tmpFile.Name()), WithPageSize(pageSize), WithMaxMem(int64(pageSize)*256)) ln, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { diff --git a/guestbd/readonly.go b/guestbd/readonly.go index 665c412..1447050 100644 --- a/guestbd/readonly.go +++ b/guestbd/readonly.go @@ -2,61 +2,47 @@ package guestbd import ( "io" - "os" "sync" - "syscall" ) -// inodeKey uniquely identifies a file by device and inode number. -type inodeKey struct { - dev uint64 - ino uint64 -} - -// fileInodeKey returns the inodeKey for the given FileInfo by extracting the -// device and inode numbers from its underlying syscall.Stat_t. -func fileInodeKey(fi os.FileInfo) inodeKey { - stat := fi.Sys().(*syscall.Stat_t) - return inodeKey{dev: uint64(stat.Dev), ino: uint64(stat.Ino)} -} +// baseImageState represents a shared read-only backing image. +// Multiple connections share one baseImageState. +// +// Locking: fields are protected by either mu or Server.mu as noted. +// base and size are immutable after construction and need no lock. +// When both locks are needed, Server.mu must be acquired first; +// mu is never held when acquiring Server.mu. +type baseImageState struct { + srv *Server // immutable + base BaseImage // immutable; used for page reads + size int64 // immutable; cached from base.Size() + identityKey any // immutable; non-nil if base returned a key; used as roFiles map key + + idleSince int64 // protected by Server.mu; monotonic seq for LRU eviction; 0 while active -// readonlyFile represents a shared read-only backing file, keyed by inode. -// Multiple connections to the same file share one readonlyFile. -type readonlyFile struct { - srv *Server mu sync.Mutex - f *os.File // underlying file (for Stat, Close) - reader io.ReaderAt // used for page reads (may be *os.File or *qcow2.Image) - size int64 // virtual size (file size for raw, virtual disk size for qcow2) - refcount int32 - + refcount int32 // protected by mu // pageHashes is lazily computed per page. // A zero value means the page has not been read from disk yet. // A value equal to Server.zeroPageHash means the page is all zeros. - pageHashes []pageHash + pageHashes []pageHash // protected by mu } -// newReadonlyFile creates a new readonlyFile with a pre-allocated pageHashes +// newBaseImageState creates a new baseImageState with a pre-allocated pageHashes // table sized for the given virtual disk size. The refcount starts at 1. -func newReadonlyFile(srv *Server, f *os.File, size int64, reader io.ReaderAt) *readonlyFile { - pageSize := srv.pageSize - numPages := (size + int64(pageSize) - 1) / int64(pageSize) - return &readonlyFile{ - srv: srv, - f: f, - reader: reader, - size: size, - refcount: 1, - pageHashes: make([]pageHash, numPages), +func newBaseImageState(srv *Server, base BaseImage, key any) *baseImageState { + size := base.Size() + numPages := (size + int64(srv.pageSize) - 1) / int64(srv.pageSize) + return &baseImageState{ + srv: srv, + base: base, + size: size, + identityKey: key, + refcount: 1, + pageHashes: make([]pageHash, numPages), } } -// fileInfo returns the FileInfo for the underlying file. -func (r *readonlyFile) fileInfo() os.FileInfo { - fi, _ := r.f.Stat() - return fi -} - // readResult describes where a page read was served from. type readResult int @@ -68,7 +54,7 @@ const ( // readPage reads page n from the backing file and returns its data, hash, // and how the read was served (cache hit, cold disk read, or cache miss disk read). -func (r *readonlyFile) readPage(n int64) (data []byte, hash pageHash, result readResult, err error) { +func (r *baseImageState) readPage(n int64) (data []byte, hash pageHash, result readResult, err error) { r.mu.Lock() h := r.pageHashes[n] r.mu.Unlock() @@ -88,7 +74,7 @@ func (r *readonlyFile) readPage(n int64) (data []byte, hash pageHash, result rea // Need to read from disk. buf := make([]byte, pageSize) offset := n * int64(pageSize) - nr, readErr := r.reader.ReadAt(buf, offset) + nr, readErr := r.base.ReadAt(buf, offset) if readErr != nil && readErr != io.EOF { return nil, pageHash{}, 0, readErr } diff --git a/guestbd/server.go b/guestbd/server.go index 2fab723..69a05b0 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -12,12 +12,105 @@ import ( "slices" "strings" "sync" + "syscall" "github.com/bradfitz/qcow2" "tailscale.com/metrics" "tailscale.com/util/set" ) +// BaseImage is a read-only random-access image with a known size. +// The server calls Close when the image is no longer needed. +// Implementations that return a non-nil key from BaseImageKey enable +// equivalence-keyed caching: idle baseImageStates whose key matches a +// newly opened image are reused, preserving the page hash table. +type BaseImage interface { + io.ReaderAt + io.Closer + Size() int64 + BaseImageKey() any // nil = no coalescing; non-nil must be comparable +} + +// BaseImageSource is a function that opens and returns the base image. +// It is called lazily when the first snapshot is created. +type BaseImageSource func() (BaseImage, error) + +// NewBaseImage returns a BaseImage wrapping an io.ReaderAt with a fixed size. +// The returned BaseImage has no identity key (BaseImageKey returns nil), +// so idle baseImageStates using it will not be reused across connections. +func NewBaseImage(r io.ReaderAt, size int64) BaseImage { + return &plainBaseImage{r: r, size: size} +} + +type plainBaseImage struct { + r io.ReaderAt + size int64 +} + +func (p *plainBaseImage) ReadAt(b []byte, off int64) (int, error) { return p.r.ReadAt(b, off) } +func (p *plainBaseImage) Size() int64 { return p.size } +func (p *plainBaseImage) Close() error { return nil } +func (p *plainBaseImage) BaseImageKey() any { return nil } + +// fileIdentityKey uniquely identifies an on-disk file by device and inode. +type fileIdentityKey struct { + dev uint64 + ino uint64 +} + +// fileSizeReaderAt wraps an *os.File as a BaseImage. +type fileSizeReaderAt struct { + f *os.File + size int64 + key fileIdentityKey +} + +func (r *fileSizeReaderAt) ReadAt(p []byte, off int64) (int, error) { return r.f.ReadAt(p, off) } +func (r *fileSizeReaderAt) Size() int64 { return r.size } +func (r *fileSizeReaderAt) Close() error { return r.f.Close() } +func (r *fileSizeReaderAt) BaseImageKey() any { return r.key } + +// qcow2SizeReaderAt wraps a *qcow2.Image as a BaseImage, holding the +// underlying *os.File so it can be closed. +type qcow2SizeReaderAt struct { + img *qcow2.Image + f *os.File + key fileIdentityKey +} + +func (r *qcow2SizeReaderAt) ReadAt(p []byte, off int64) (int, error) { return r.img.ReadAt(p, off) } +func (r *qcow2SizeReaderAt) Size() int64 { return r.img.Size() } +func (r *qcow2SizeReaderAt) Close() error { return r.f.Close() } +func (r *qcow2SizeReaderAt) BaseImageKey() any { return r.key } + +// FileSource returns a BaseImageSource that opens the file at path. +// Files ending in ".qcow2" are opened as qcow2 images; all others are +// treated as raw disk images. +func FileSource(path string) BaseImageSource { + return func() (BaseImage, error) { + f, err := os.Open(path) + if err != nil { + return nil, err + } + fi, err := f.Stat() + if err != nil { + f.Close() + return nil, err + } + st := fi.Sys().(*syscall.Stat_t) + key := fileIdentityKey{dev: st.Dev, ino: st.Ino} + if strings.HasSuffix(path, ".qcow2") { + img, err := qcow2.Open(f) + if err != nil { + f.Close() + return nil, fmt.Errorf("opening qcow2 image: %w", err) + } + return &qcow2SizeReaderAt{img, f, key}, nil + } + return &fileSizeReaderAt{f, fi.Size(), key}, nil + } +} + // ServerOption configures optional Server parameters. type ServerOption func(*serverConfig) @@ -25,6 +118,7 @@ type serverConfig struct { pageSize int maxMem int64 sharedSnapshot bool + maxIdleBase int } // WithPageSize sets the page size in bytes. It must be a positive power of two. @@ -46,23 +140,35 @@ func WithSharedSnapshot() ServerOption { return func(c *serverConfig) { c.sharedSnapshot = true } } -// Server is an NBD server that serves a single backing file to multiple +// WithMaxIdleBase sets the maximum number of idle base images to keep cached +// for equivalence-keyed reuse. When a baseImageState becomes idle (refcount 0) +// and its BaseImageKey is non-nil, it is kept in a cache of up to n entries. +// The default is 1. +func WithMaxIdleBase(n int) ServerOption { + return func(c *serverConfig) { c.maxIdleBase = n } +} + +// Server is an NBD server that serves a single backing image to multiple // concurrent clients. Each client connection may get an independent // copy-on-write Snapshot, or multiple connections may share a single Snapshot // to allow reconnection to a previous writable state. The snapshot policy is // configured via WithSharedSnapshot at construction time. type Server struct { - mu sync.Mutex - filePath string - pageSize int + mu sync.Mutex + getBase BaseImageSource + pageSize int + zeroPageHash pageHash // sha256 of a page filled entirely with zero bytes cache *pageCache useSharedSnapshot bool // set by WithSharedSnapshot sharedSnap *Snapshot // lazily created when useSharedSnapshot is true - readonlyFiles map[inodeKey]*readonlyFile - snapshots set.Set[*Snapshot] + roFiles map[any]*baseImageState // identity-keyed entries (active + idle) + roFileNoKey *baseImageState // single entry for nil-key sources + maxIdleBase int // max idle entries in roFiles + nextIdleSeq int64 // monotonic counter for idle LRU ordering + snapshots set.Set[*Snapshot] totalConns expvar.Int // counter_guestbd_total_conns activeConns expvar.Int // gauge_guestbd_active_conns @@ -78,10 +184,8 @@ type Server struct { baseImagesCached expvar.Int // gauge_guestbd_base_images_cached } -// NewServer creates a new Server that serves the file at filePath. -// Files ending in ".qcow2" are opened as qcow2 images; all others are -// treated as raw disk images. -func NewServer(filePath string, opts ...ServerOption) *Server { +// NewServer creates a new Server that serves the base image returned by getBase. +func NewServer(getBase BaseImageSource, opts ...ServerOption) *Server { cfg := serverConfig{ pageSize: 4096, maxMem: 1 << 30, // 1GB default @@ -102,13 +206,19 @@ func NewServer(filePath string, opts ...ServerOption) *Server { sizeBuckets = append(sizeBuckets, v) } + maxIdleBase := cfg.maxIdleBase + if maxIdleBase == 0 { + maxIdleBase = 1 + } + srv := &Server{ - filePath: filePath, + getBase: getBase, pageSize: pageSize, useSharedSnapshot: cfg.sharedSnapshot, zeroPageHash: hashPage(make([]byte, pageSize)), cache: newPageCache(maxPages, pageSize), - readonlyFiles: make(map[inodeKey]*readonlyFile), + roFiles: make(map[any]*baseImageState), + maxIdleBase: maxIdleBase, snapshots: make(set.Set[*Snapshot]), ops: metrics.LabelMap{Label: "type"}, readPath: metrics.LabelMap{Label: "type"}, @@ -161,66 +271,77 @@ func (s *Server) InitExpvar() { })) } -// getReadonlyFile returns the readonlyFile for the backing file, -// sharing it across connections with the same inode. The readonlyFile -// and its pageHashes table persist even when refcount drops to zero, -// so that subsequent connections to the same inode benefit from the -// already-computed hash table (which is the index into the LRU cache). -func (s *Server) getReadonlyFile() (*readonlyFile, error) { - f, err := os.Open(s.filePath) +// getBaseImageState returns a baseImageState for the backing image. +// getBase is called on every invocation. If there is already an active or +// idle baseImageState with a matching BaseImageKey, the newly opened base is +// closed and the existing baseImageState is reused (preserving its page hash +// table). Sources returning nil keys share while active but are replaced +// when idle. +func (s *Server) getBaseImageState() (*baseImageState, error) { + base, err := s.getBase() if err != nil { return nil, err } - fi, err := f.Stat() - if err != nil { - f.Close() - return nil, err - } - key := fileInodeKey(fi) + + key := base.BaseImageKey() s.mu.Lock() defer s.mu.Unlock() - if ro, ok := s.readonlyFiles[key]; ok { - ro.mu.Lock() - wasIdle := ro.refcount == 0 - ro.refcount++ - ro.mu.Unlock() - if wasIdle { - s.baseImagesActive.Add(1) + if key != nil { + if ro, ok := s.roFiles[key]; ok { + // Found a matching entry (active or idle). Close the new base + // and bump refcount. + base.Close() + ro.mu.Lock() + wasIdle := ro.refcount == 0 + ro.refcount++ + ro.mu.Unlock() + if wasIdle { + s.baseImagesActive.Add(1) + ro.idleSince = 0 + } + return ro, nil } - f.Close() + // Not found — create new and insert. + ro := newBaseImageState(s, base, key) + s.roFiles[key] = ro + s.baseImagesActive.Add(1) + s.baseImagesCached.Add(1) + s.evictIdleLocked() return ro, nil } - var reader io.ReaderAt - size := fi.Size() - if strings.HasSuffix(s.filePath, ".qcow2") { - img, err := qcow2.Open(f) - if err != nil { - f.Close() - return nil, fmt.Errorf("opening qcow2 image: %w", err) + // key == nil + if ro := s.roFileNoKey; ro != nil { + ro.mu.Lock() + rc := ro.refcount + ro.mu.Unlock() + if rc > 0 { + // Active — share. + base.Close() + ro.mu.Lock() + ro.refcount++ + ro.mu.Unlock() + return ro, nil } - reader = img - size = img.Size() - } else { - reader = f + // Idle — close old base and replace. + ro.base.Close() + s.baseImagesCached.Add(-1) } - ro := newReadonlyFile(s, f, size, reader) - s.readonlyFiles[key] = ro - s.baseImagesActive.Add(1) // new entry starts with refcount 1 + ro := newBaseImageState(s, base, nil) + s.roFileNoKey = ro + s.baseImagesActive.Add(1) s.baseImagesCached.Add(1) return ro, nil } -// releaseReadonlyFile decrements the refcount. If the refcount reaches -// zero, it checks whether the file on disk still points to the same -// inode. If the file is gone or has a different inode, the readonlyFile -// is removed and its fd closed — no future client can use it. Otherwise -// it stays in the map so the pageHashes index remains available for the -// next connection. -func (s *Server) releaseReadonlyFile(ro *readonlyFile) { +// releaseBaseImageState decrements the refcount. When it reaches zero the +// baseImagesActive metric is updated. For keyed entries, the baseImageState +// stays in the roFiles map as an idle entry (subject to LRU eviction). +// For nil-key entries, it stays in roFileNoKey and is replaced next call. +func (s *Server) releaseBaseImageState(ro *baseImageState) { ro.mu.Lock() ro.refcount-- rc := ro.refcount @@ -232,20 +353,45 @@ func (s *Server) releaseReadonlyFile(ro *readonlyFile) { s.baseImagesActive.Add(-1) - // Check whether the file on disk still matches this inode. - fi, err := os.Stat(s.filePath) - if err != nil || fileInodeKey(fi) != fileInodeKey(ro.fileInfo()) { - // File is gone or replaced; no future client will ever - // open this inode. Clean it up. - s.mu.Lock() - for k, v := range s.readonlyFiles { - if v == ro { - delete(s.readonlyFiles, k) - break + s.mu.Lock() + defer s.mu.Unlock() + + if ro.identityKey != nil { + s.nextIdleSeq++ + ro.idleSince = s.nextIdleSeq + s.evictIdleLocked() + } +} + +// evictIdleLocked removes the oldest idle entries from s.roFiles until +// the number of idle entries is at most s.maxIdleBase. +func (s *Server) evictIdleLocked() { + for { + var ( + idleCount int + oldestKey any + oldestSeq int64 + oldestEntry *baseImageState + ) + for k, ro := range s.roFiles { + ro.mu.Lock() + rc := ro.refcount + ro.mu.Unlock() + if rc == 0 { + idleCount++ + if oldestEntry == nil || ro.idleSince < oldestSeq { + oldestKey = k + oldestSeq = ro.idleSince + oldestEntry = ro + } } } - s.mu.Unlock() - ro.f.Close() + if idleCount <= s.maxIdleBase { + return + } + // Evict oldest idle. + oldestEntry.base.Close() + delete(s.roFiles, oldestKey) s.baseImagesCached.Add(-1) } } @@ -254,13 +400,13 @@ func (s *Server) releaseReadonlyFile(ro *readonlyFile) { // image. The caller is responsible for calling Close when the snapshot is no // longer needed. func (s *Server) NewSnapshot() (*Snapshot, error) { - ro, err := s.getReadonlyFile() + ro, err := s.getBaseImageState() if err != nil { return nil, fmt.Errorf("opening base image: %w", err) } snap, err := newSnapshot(s, ro) if err != nil { - s.releaseReadonlyFile(ro) + s.releaseBaseImageState(ro) return nil, err } s.mu.Lock() @@ -270,15 +416,25 @@ func (s *Server) NewSnapshot() (*Snapshot, error) { } // Close cleans up server resources, including any shared Snapshot created -// via WithSharedSnapshot. +// via WithSharedSnapshot and all cached baseImageStates. func (s *Server) Close() error { s.mu.Lock() snap := s.sharedSnap s.sharedSnap = nil + roFiles := s.roFiles + s.roFiles = nil + roNoKey := s.roFileNoKey + s.roFileNoKey = nil s.mu.Unlock() if snap != nil { snap.Close() } + for _, ro := range roFiles { + ro.base.Close() + } + if roNoKey != nil { + roNoKey.base.Close() + } return nil } diff --git a/guestbd/snapshot.go b/guestbd/snapshot.go index 0dbafa0..0e3e837 100644 --- a/guestbd/snapshot.go +++ b/guestbd/snapshot.go @@ -21,7 +21,7 @@ var ( // Snapshot implements io.ReaderAt and io.WriterAt. type Snapshot struct { server *Server - roFile *readonlyFile + roFile *baseImageState mu sync.Mutex dirtyPages map[int64]*dirtyPageInfo // pageNum => dirty info @@ -37,7 +37,7 @@ type dirtyPageInfo struct { // newSnapshot creates a new Snapshot for the given read-only backing file. // It creates an unlinked temporary file for dirty page storage. -func newSnapshot(srv *Server, ro *readonlyFile) (*Snapshot, error) { +func newSnapshot(srv *Server, ro *baseImageState) (*Snapshot, error) { tmpFile, err := os.CreateTemp("", "guestbd-dirty-*") if err != nil { return nil, fmt.Errorf("creating temp file: %w", err) @@ -67,7 +67,7 @@ func (c *Snapshot) Close() error { c.server.snapshots.Delete(c) c.server.mu.Unlock() - c.server.releaseReadonlyFile(c.roFile) + c.server.releaseBaseImageState(c.roFile) return nil } From 98eb1c3ff1a2640bce16701c30ebef20cd8e81ba Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 10 Mar 2026 14:07:31 +0000 Subject: [PATCH 19/28] make page hashes be a map, not a slice otherwise an 80 GB base image of mostly zeros takes 640 MB Signed-off-by: Brad Fitzpatrick Migrated-from: bradfitz/guestbd@428f94cfa1bbe23c64bc5545d902937ab2d1c456 --- guestbd/readonly.go | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/guestbd/readonly.go b/guestbd/readonly.go index 1447050..c31938a 100644 --- a/guestbd/readonly.go +++ b/guestbd/readonly.go @@ -23,23 +23,21 @@ type baseImageState struct { mu sync.Mutex refcount int32 // protected by mu // pageHashes is lazily computed per page. - // A zero value means the page has not been read from disk yet. + // Absent from the map means the page has not been read from disk yet. // A value equal to Server.zeroPageHash means the page is all zeros. - pageHashes []pageHash // protected by mu + pageHashes map[int64]pageHash // protected by mu } -// newBaseImageState creates a new baseImageState with a pre-allocated pageHashes -// table sized for the given virtual disk size. The refcount starts at 1. +// newBaseImageState creates a new baseImageState for the given base image. +// The refcount starts at 1. func newBaseImageState(srv *Server, base BaseImage, key any) *baseImageState { - size := base.Size() - numPages := (size + int64(srv.pageSize) - 1) / int64(srv.pageSize) return &baseImageState{ srv: srv, base: base, - size: size, + size: base.Size(), identityKey: key, refcount: 1, - pageHashes: make([]pageHash, numPages), + pageHashes: make(map[int64]pageHash), } } @@ -56,14 +54,11 @@ const ( // and how the read was served (cache hit, cold disk read, or cache miss disk read). func (r *baseImageState) readPage(n int64) (data []byte, hash pageHash, result readResult, err error) { r.mu.Lock() - h := r.pageHashes[n] + h, hashKnown := r.pageHashes[n] r.mu.Unlock() cache := r.srv.cache pageSize := r.srv.pageSize - - var zeroHash pageHash - hashKnown := h != zeroHash if hashKnown { // Already have the hash; try cache. if d, ok := cache.Get(h); ok { @@ -86,7 +81,7 @@ func (r *baseImageState) readPage(n int64) (data []byte, hash pageHash, result r h = hashPage(buf) r.mu.Lock() - if r.pageHashes[n] == zeroHash { + if _, ok := r.pageHashes[n]; !ok { r.pageHashes[n] = h } r.mu.Unlock() From e678d2fd65914f4ca49d3cd5904df4d0e549a96c Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 10 Mar 2026 14:41:36 +0000 Subject: [PATCH 20/28] add no cache mode, clean up var names and struct fields Signed-off-by: Brad Fitzpatrick Migrated-from: bradfitz/guestbd@c9b37e400bd2c6c339add22b7c19f9c7bee4962f --- guestbd/guestbd_test.go | 83 +++++++++++++++++++++++++++++++++++++++++ guestbd/readonly.go | 47 ++++++++++++++++------- guestbd/server.go | 56 +++++++++++++++------------ guestbd/snapshot.go | 23 +++++++++--- 4 files changed, 167 insertions(+), 42 deletions(-) diff --git a/guestbd/guestbd_test.go b/guestbd/guestbd_test.go index 4ba1898..6a48fa0 100644 --- a/guestbd/guestbd_test.go +++ b/guestbd/guestbd_test.go @@ -1045,6 +1045,89 @@ func TestPageCacheLRU(t *testing.T) { } } +func TestNoCacheMode(t *testing.T) { + const pageSize = 4096 + data := make([]byte, pageSize*4) + for i := range data { + data[i] = byte(i % 251) + } + + tmpFile, err := os.CreateTemp("", "guestbd-test-*") + if err != nil { + t.Fatal(err) + } + if _, err := tmpFile.Write(data); err != nil { + t.Fatal(err) + } + tmpFile.Close() + defer os.Remove(tmpFile.Name()) + + srv := NewServer(FileSource(tmpFile.Name()), WithPageSize(pageSize), WithMaxMem(0)) + if srv.cache != nil { + t.Fatal("expected nil cache with WithMaxMem(0)") + } + + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + go srv.HandleConn(conn) + } + }() + addr := ln.Addr().String() + + c := newTestClient(t, addr) + defer c.disconnect() + + // Read base image data. + got := c.read(0, pageSize) + if !bytes.Equal(got, data[:pageSize]) { + t.Fatal("first page mismatch") + } + + // Full page write and read back. + writeData := make([]byte, pageSize) + for i := range writeData { + writeData[i] = 0xAB + } + c.write(0, writeData) + got = c.read(0, pageSize) + if !bytes.Equal(got, writeData) { + t.Fatal("written page mismatch") + } + + // Sub-page write (read-modify-write without cache). + patch := bytes.Repeat([]byte{0xFF}, 100) + c.write(pageSize+200, patch) + got = c.read(pageSize, pageSize) + expected := make([]byte, pageSize) + copy(expected, data[pageSize:2*pageSize]) + copy(expected[200:], patch) + if !bytes.Equal(got, expected) { + t.Fatal("sub-page write mismatch") + } + + // Trim reverts to base image. + c.trim(0, pageSize) + got = c.read(0, pageSize) + if !bytes.Equal(got, data[:pageSize]) { + t.Fatal("trim should revert to original") + } + + // Unwritten page still reads from base. + got = c.read(uint64(pageSize*3), pageSize) + if !bytes.Equal(got, data[pageSize*3:]) { + t.Fatal("unwritten page mismatch") + } +} + func TestMultiplePageSizes(t *testing.T) { for _, pageSize := range []int{512, 1024, 4096, 8192} { t.Run(fmt.Sprintf("pageSize=%d", pageSize), func(t *testing.T) { diff --git a/guestbd/readonly.go b/guestbd/readonly.go index c31938a..dc8f913 100644 --- a/guestbd/readonly.go +++ b/guestbd/readonly.go @@ -31,14 +31,17 @@ type baseImageState struct { // newBaseImageState creates a new baseImageState for the given base image. // The refcount starts at 1. func newBaseImageState(srv *Server, base BaseImage, key any) *baseImageState { - return &baseImageState{ + bs := &baseImageState{ srv: srv, base: base, size: base.Size(), identityKey: key, refcount: 1, - pageHashes: make(map[int64]pageHash), } + if srv.cache != nil { + bs.pageHashes = make(map[int64]pageHash) + } + return bs } // readResult describes where a page read was served from. @@ -52,13 +55,31 @@ const ( // readPage reads page n from the backing file and returns its data, hash, // and how the read was served (cache hit, cold disk read, or cache miss disk read). -func (r *baseImageState) readPage(n int64) (data []byte, hash pageHash, result readResult, err error) { - r.mu.Lock() - h, hashKnown := r.pageHashes[n] - r.mu.Unlock() +// +// When the server has no page cache (WithMaxMem(0)), readPage skips hashing +// and cache operations and reads directly from the base image. +func (bs *baseImageState) readPage(n int64) (data []byte, hash pageHash, result readResult, err error) { + cache := bs.srv.cache + pageSize := bs.srv.pageSize + + if cache == nil { + // No caching; read straight from the base image. + buf := make([]byte, pageSize) + offset := n * int64(pageSize) + nr, readErr := bs.base.ReadAt(buf, offset) + if readErr != nil && readErr != io.EOF { + return nil, pageHash{}, 0, readErr + } + for i := nr; i < pageSize; i++ { + buf[i] = 0 + } + return buf, pageHash{}, readFromDiskCold, nil + } + + bs.mu.Lock() + h, hashKnown := bs.pageHashes[n] + bs.mu.Unlock() - cache := r.srv.cache - pageSize := r.srv.pageSize if hashKnown { // Already have the hash; try cache. if d, ok := cache.Get(h); ok { @@ -69,7 +90,7 @@ func (r *baseImageState) readPage(n int64) (data []byte, hash pageHash, result r // Need to read from disk. buf := make([]byte, pageSize) offset := n * int64(pageSize) - nr, readErr := r.base.ReadAt(buf, offset) + nr, readErr := bs.base.ReadAt(buf, offset) if readErr != nil && readErr != io.EOF { return nil, pageHash{}, 0, readErr } @@ -80,11 +101,11 @@ func (r *baseImageState) readPage(n int64) (data []byte, hash pageHash, result r h = hashPage(buf) - r.mu.Lock() - if _, ok := r.pageHashes[n]; !ok { - r.pageHashes[n] = h + bs.mu.Lock() + if _, ok := bs.pageHashes[n]; !ok { + bs.pageHashes[n] = h } - r.mu.Unlock() + bs.mu.Unlock() cache.Put(h, buf) if hashKnown { diff --git a/guestbd/server.go b/guestbd/server.go index 69a05b0..391c84a 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -128,7 +128,8 @@ func WithPageSize(n int) ServerOption { } // WithMaxMem sets the maximum memory used by the shared page cache. -// The default is 1GB. +// The default is 1GB. A value of 0 disables page hashing and caching +// entirely; reads of non-dirty pages go directly to the base image. func WithMaxMem(n int64) ServerOption { return func(c *serverConfig) { c.maxMem = n } } @@ -154,22 +155,22 @@ func WithMaxIdleBase(n int) ServerOption { // to allow reconnection to a previous writable state. The snapshot policy is // configured via WithSharedSnapshot at construction time. type Server struct { - mu sync.Mutex - getBase BaseImageSource - pageSize int - - zeroPageHash pageHash // sha256 of a page filled entirely with zero bytes - cache *pageCache - - useSharedSnapshot bool // set by WithSharedSnapshot - sharedSnap *Snapshot // lazily created when useSharedSnapshot is true - - roFiles map[any]*baseImageState // identity-keyed entries (active + idle) - roFileNoKey *baseImageState // single entry for nil-key sources - maxIdleBase int // max idle entries in roFiles - nextIdleSeq int64 // monotonic counter for idle LRU ordering + // Immutable after construction: + getBase BaseImageSource // immutable + pageSize int // immutable + zeroPageHash pageHash // immutable; sha256 of an all-zero page + cache *pageCache // immutable; nil when WithMaxMem(0) disables caching + useSharedSnapshot bool // immutable; set by WithSharedSnapshot + maxIdleBase int // immutable; max idle entries in roFiles + + mu sync.Mutex + sharedSnap *Snapshot // lazily created when useSharedSnapshot is true + roFiles map[any]*baseImageState // identity-keyed entries (active + idle) + roFileNoKey *baseImageState // single entry for nil-key sources + nextIdleSeq int64 // monotonic counter for idle LRU ordering snapshots set.Set[*Snapshot] + // Metrics (atomic, no mutex needed): totalConns expvar.Int // counter_guestbd_total_conns activeConns expvar.Int // gauge_guestbd_active_conns ops metrics.LabelMap // counter_guestbd_nbd_ops{type=...} @@ -195,9 +196,14 @@ func NewServer(getBase BaseImageSource, opts ...ServerOption) *Server { } pageSize := cfg.pageSize - maxPages := int(cfg.maxMem) / pageSize - if maxPages < 1 { - maxPages = 1 + + var cache *pageCache + if cfg.maxMem > 0 { + maxPages := int(cfg.maxMem) / pageSize + if maxPages < 1 { + maxPages = 1 + } + cache = newPageCache(maxPages, pageSize) } // Powers of two from 1 to 4MB. @@ -215,8 +221,7 @@ func NewServer(getBase BaseImageSource, opts ...ServerOption) *Server { getBase: getBase, pageSize: pageSize, useSharedSnapshot: cfg.sharedSnapshot, - zeroPageHash: hashPage(make([]byte, pageSize)), - cache: newPageCache(maxPages, pageSize), + cache: cache, roFiles: make(map[any]*baseImageState), maxIdleBase: maxIdleBase, snapshots: make(set.Set[*Snapshot]), @@ -225,6 +230,9 @@ func NewServer(getBase BaseImageSource, opts ...ServerOption) *Server { readSizeHist: metrics.NewHistogram(sizeBuckets), writeSizeHist: metrics.NewHistogram(sizeBuckets), } + if cache != nil { + srv.zeroPageHash = hashPage(make([]byte, pageSize)) + } return srv } @@ -243,9 +251,11 @@ func (s *Server) InitExpvar() { expvar.Publish("counter_guestbd_write_pages", &s.writePages) expvar.Publish("histogram_guestbd_read_size_bytes", s.readSizeHist) expvar.Publish("histogram_guestbd_write_size_bytes", s.writeSizeHist) - expvar.Publish("counter_guestbd_cache", &s.cache.path) - expvar.Publish("gauge_guestbd_cache_entries", &s.cache.entries) - expvar.Publish("gauge_guestbd_cache_bytes", &s.cache.bytes) + if s.cache != nil { + expvar.Publish("counter_guestbd_cache", &s.cache.path) + expvar.Publish("gauge_guestbd_cache_entries", &s.cache.entries) + expvar.Publish("gauge_guestbd_cache_bytes", &s.cache.bytes) + } expvar.Publish("gauge_guestbd_base_images_active", &s.baseImagesActive) expvar.Publish("gauge_guestbd_base_images_cached", &s.baseImagesCached) diff --git a/guestbd/snapshot.go b/guestbd/snapshot.go index 0e3e837..6e6063a 100644 --- a/guestbd/snapshot.go +++ b/guestbd/snapshot.go @@ -82,9 +82,12 @@ func (c *Snapshot) readPageData(pageNum int64) ([]byte, error) { if dirty { c.server.readPath.Add("from_write", 1) - // Try the global cache first. - if data, ok := c.server.cache.Get(dp.hash); ok { - return data, nil + cache := c.server.cache + if cache != nil { + // Try the global cache first. + if data, ok := cache.Get(dp.hash); ok { + return data, nil + } } // Read from the snapshot's dirty file. buf := make([]byte, c.server.pageSize) @@ -92,7 +95,9 @@ func (c *Snapshot) readPageData(pageNum int64) ([]byte, error) { if err != nil { return nil, fmt.Errorf("reading dirty page: %w", err) } - c.server.cache.Put(dp.hash, buf) + if cache != nil { + cache.Put(dp.hash, buf) + } return buf, nil } @@ -178,7 +183,11 @@ func (c *Snapshot) WriteAt(p []byte, off int64) (int, error) { copy(pageData[pageOffset:], p[pos:pos+n]) } - h := hashPage(pageData) + cache := c.server.cache + var h pageHash + if cache != nil { + h = hashPage(pageData) + } c.mu.Lock() dirtyNum := c.nextDirtyNum @@ -196,7 +205,9 @@ func (c *Snapshot) WriteAt(p []byte, off int64) (int, error) { } c.mu.Unlock() - c.server.cache.Put(h, pageData) + if cache != nil { + cache.Put(h, pageData) + } c.server.writePages.Add(1) pos += n } From aab0c94734438dcd1671490b8415d261fd3cb700 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 10 Mar 2026 15:17:42 +0000 Subject: [PATCH 21/28] reduce allocs, fix trim on non-page boundaries Signed-off-by: Brad Fitzpatrick Migrated-from: bradfitz/guestbd@39556242f4d8bf6696730afd06d7dd132d6b097c --- guestbd/cache.go | 1 + guestbd/guestbd_test.go | 148 ++++++++++++++++++++++++++++++++++++++++ guestbd/readonly.go | 29 ++++---- guestbd/server.go | 12 +++- guestbd/snapshot.go | 114 ++++++++++++++++++++----------- 5 files changed, 251 insertions(+), 53 deletions(-) diff --git a/guestbd/cache.go b/guestbd/cache.go index 856865f..7ec1c25 100644 --- a/guestbd/cache.go +++ b/guestbd/cache.go @@ -68,6 +68,7 @@ func (c *pageCache) Get(h pageHash) ([]byte, bool) { } // Put adds page data to the cache, keyed by its hash. +// The data slice is cloned; the caller retains ownership of the original. // If the hash already exists, it's moved to the front of the LRU. func (c *pageCache) Put(h pageHash, data []byte) { c.mu.Lock() diff --git a/guestbd/guestbd_test.go b/guestbd/guestbd_test.go index 6a48fa0..501d74b 100644 --- a/guestbd/guestbd_test.go +++ b/guestbd/guestbd_test.go @@ -321,6 +321,56 @@ func TestReadBasic(t *testing.T) { } } +func TestReadAtAlignment(t *testing.T) { + const pageSize = 4096 + data := make([]byte, pageSize*6) + for i := range data { + data[i] = byte(i % 251) + } + + addr, _, cleanup := startTestServer(t, data, pageSize) + defer cleanup() + + c := newTestClient(t, addr) + defer c.disconnect() + + tests := []struct { + name string + off uint64 + length uint32 + }{ + // Within a single page, not at offset 0. + {"mid-page-short-read", 100, 50}, + // Page-aligned start, sub-page length. + {"aligned-start-sub-page", 0, 100}, + // Starts mid-page, ends at exact page boundary. + {"mid-to-page-boundary", 100, pageSize - 100}, + // Starts at page boundary, ends mid-next-page. + {"page-boundary-into-next", pageSize, pageSize + 200}, + // Unaligned start and end spanning 3 pages: + // partial first + full middle + partial last. + {"unaligned-multi-page", 100, pageSize*2 + 200}, + // Multiple full aligned pages. + {"multi-full-pages", 0, pageSize * 3}, + // Single byte at page boundary. + {"one-byte-at-boundary", pageSize, 1}, + // Single byte just before page boundary. + {"one-byte-before-boundary", pageSize - 1, 1}, + // Single byte just after page boundary. + {"one-byte-after-boundary", pageSize + 1, 1}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := c.read(tt.off, tt.length) + want := data[tt.off : tt.off+uint64(tt.length)] + if !bytes.Equal(got, want) { + t.Fatalf("off=%d len=%d: got %x... want %x...", + tt.off, tt.length, got[:min(len(got), 16)], want[:min(len(want), 16)]) + } + }) + } +} + func TestWriteAndReadBack(t *testing.T) { const pageSize = 4096 data := make([]byte, pageSize*4) @@ -464,6 +514,104 @@ func TestTrim(t *testing.T) { } } +func TestTrimAlignment(t *testing.T) { + const pageSize = 4096 + // Non-zero original so we can distinguish "reverted to base" from "zeroed". + original := make([]byte, pageSize*4) + for i := range original { + original[i] = byte(i % 251) + } + + t.Run("mid-page", func(t *testing.T) { + // Trim within a single dirty page: trimmed bytes become zero, + // rest of page stays dirty. + addr, _, cleanup := startTestServer(t, original, pageSize) + defer cleanup() + c := newTestClient(t, addr) + defer c.disconnect() + + c.write(0, bytes.Repeat([]byte{0xFF}, pageSize)) + c.trim(100, 200) // zero bytes 100..299 + + got := c.read(0, pageSize) + expected := bytes.Repeat([]byte{0xFF}, pageSize) + for i := 100; i < 300; i++ { + expected[i] = 0 + } + if !bytes.Equal(got, expected) { + t.Fatal("mid-page trim mismatch") + } + }) + + t.Run("cross-boundary-partial", func(t *testing.T) { + // Trim crossing page boundary, not fully covering either page. + // Both partial pages get zero-writes. + addr, _, cleanup := startTestServer(t, original, pageSize) + defer cleanup() + c := newTestClient(t, addr) + defer c.disconnect() + + c.write(0, bytes.Repeat([]byte{0xFF}, pageSize*2)) + c.trim(uint64(pageSize-100), 200) // zero last 100 bytes of page 0, first 100 of page 1 + + got := c.read(0, pageSize*2) + expected := bytes.Repeat([]byte{0xFF}, pageSize*2) + for i := pageSize - 100; i < pageSize+100; i++ { + expected[i] = 0 + } + if !bytes.Equal(got, expected) { + t.Fatal("cross-boundary trim mismatch") + } + }) + + t.Run("partial-full-partial", func(t *testing.T) { + // Trim partially covers first and last page, fully covers middle. + // Partial pages get zero-writes; full middle page reverts to original. + addr, _, cleanup := startTestServer(t, original, pageSize) + defer cleanup() + c := newTestClient(t, addr) + defer c.disconnect() + + c.write(0, bytes.Repeat([]byte{0xFF}, pageSize*3)) + c.trim(uint64(pageSize-100), pageSize+200) // partial page 0, full page 1, partial page 2 + + got := c.read(0, pageSize*3) + expected := bytes.Repeat([]byte{0xFF}, pageSize*3) + // Partial end of page 0: zeroed. + for i := pageSize - 100; i < pageSize; i++ { + expected[i] = 0 + } + // Full page 1: reverts to original. + copy(expected[pageSize:pageSize*2], original[pageSize:pageSize*2]) + // Partial start of page 2: zeroed. + for i := pageSize * 2; i < pageSize*2+100; i++ { + expected[i] = 0 + } + if !bytes.Equal(got, expected) { + t.Fatal("partial-full-partial trim mismatch") + } + }) + + t.Run("exact-full-page", func(t *testing.T) { + // Trim exactly covers one page: reverts to base image. + addr, _, cleanup := startTestServer(t, original, pageSize) + defer cleanup() + c := newTestClient(t, addr) + defer c.disconnect() + + c.write(0, bytes.Repeat([]byte{0xFF}, pageSize*3)) + c.trim(uint64(pageSize), uint32(pageSize)) + + got := c.read(0, pageSize*3) + expected := bytes.Repeat([]byte{0xFF}, pageSize*3) + // Page 1 reverts to original. + copy(expected[pageSize:pageSize*2], original[pageSize:pageSize*2]) + if !bytes.Equal(got, expected) { + t.Fatal("exact-full-page trim mismatch") + } + }) +} + func TestFlush(t *testing.T) { const pageSize = 4096 data := make([]byte, pageSize) diff --git a/guestbd/readonly.go b/guestbd/readonly.go index dc8f913..41c3974 100644 --- a/guestbd/readonly.go +++ b/guestbd/readonly.go @@ -53,27 +53,28 @@ const ( readFromDiskMiss // hash known but evicted from LRU cache, re-read from disk ) -// readPage reads page n from the backing file and returns its data, hash, +// readPage reads page n from the backing file into buf and returns its hash // and how the read was served (cache hit, cold disk read, or cache miss disk read). +// buf must be at least pageSize bytes long or readPage panics. // // When the server has no page cache (WithMaxMem(0)), readPage skips hashing // and cache operations and reads directly from the base image. -func (bs *baseImageState) readPage(n int64) (data []byte, hash pageHash, result readResult, err error) { +func (bs *baseImageState) readPage(buf []byte, n int64) (hash pageHash, result readResult, err error) { cache := bs.srv.cache pageSize := bs.srv.pageSize + _ = buf[pageSize-1] // bounds check hint; panics if too small if cache == nil { // No caching; read straight from the base image. - buf := make([]byte, pageSize) offset := n * int64(pageSize) - nr, readErr := bs.base.ReadAt(buf, offset) + nr, readErr := bs.base.ReadAt(buf[:pageSize], offset) if readErr != nil && readErr != io.EOF { - return nil, pageHash{}, 0, readErr + return pageHash{}, 0, readErr } for i := nr; i < pageSize; i++ { buf[i] = 0 } - return buf, pageHash{}, readFromDiskCold, nil + return pageHash{}, readFromDiskCold, nil } bs.mu.Lock() @@ -83,23 +84,23 @@ func (bs *baseImageState) readPage(n int64) (data []byte, hash pageHash, result if hashKnown { // Already have the hash; try cache. if d, ok := cache.Get(h); ok { - return d, h, readFromCache, nil + copy(buf, d) + return h, readFromCache, nil } } // Need to read from disk. - buf := make([]byte, pageSize) offset := n * int64(pageSize) - nr, readErr := bs.base.ReadAt(buf, offset) + nr, readErr := bs.base.ReadAt(buf[:pageSize], offset) if readErr != nil && readErr != io.EOF { - return nil, pageHash{}, 0, readErr + return pageHash{}, 0, readErr } // Zero-fill remainder (last page may be short). for i := nr; i < pageSize; i++ { buf[i] = 0 } - h = hashPage(buf) + h = hashPage(buf[:pageSize]) bs.mu.Lock() if _, ok := bs.pageHashes[n]; !ok { @@ -107,9 +108,9 @@ func (bs *baseImageState) readPage(n int64) (data []byte, hash pageHash, result } bs.mu.Unlock() - cache.Put(h, buf) + cache.Put(h, buf[:pageSize]) if hashKnown { - return buf, h, readFromDiskMiss, nil + return h, readFromDiskMiss, nil } - return buf, h, readFromDiskCold, nil + return h, readFromDiskCold, nil } diff --git a/guestbd/server.go b/guestbd/server.go index 391c84a..d8f80bf 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -162,6 +162,7 @@ type Server struct { cache *pageCache // immutable; nil when WithMaxMem(0) disables caching useSharedSnapshot bool // immutable; set by WithSharedSnapshot maxIdleBase int // immutable; max idle entries in roFiles + pageBufPool sync.Pool // immutable; pool of *[]byte, each pageSize long mu sync.Mutex sharedSnap *Snapshot // lazily created when useSharedSnapshot is true @@ -222,6 +223,7 @@ func NewServer(getBase BaseImageSource, opts ...ServerOption) *Server { pageSize: pageSize, useSharedSnapshot: cfg.sharedSnapshot, cache: cache, + pageBufPool: sync.Pool{New: func() any { b := make([]byte, pageSize); return &b }}, roFiles: make(map[any]*baseImageState), maxIdleBase: maxIdleBase, snapshots: make(set.Set[*Snapshot]), @@ -728,7 +730,15 @@ func (s *Server) serveTransmission(snap *Snapshot, r io.Reader, bw *bufio.Writer case nbdCmdTrim: s.ops.Add("trim", 1) - snap.handleTrim(req.Offset, uint64(req.Length)) + if err := snap.handleTrim(req.Offset, uint64(req.Length)); err != nil { + if werr := s.sendReply(bw, req.Handle, nbdEIO, nil); werr != nil { + return werr + } + if werr := bw.Flush(); werr != nil { + return werr + } + continue + } if err := s.sendReply(bw, req.Handle, 0, nil); err != nil { return err } diff --git a/guestbd/snapshot.go b/guestbd/snapshot.go index 6e6063a..706f4e5 100644 --- a/guestbd/snapshot.go +++ b/guestbd/snapshot.go @@ -24,9 +24,9 @@ type Snapshot struct { roFile *baseImageState mu sync.Mutex - dirtyPages map[int64]*dirtyPageInfo // pageNum => dirty info - dirtyFile *os.File // temp file for dirty page data - nextDirtyNum int // monotonically increasing + dirtyPages map[int64]dirtyPageInfo // pageNum => dirty info + dirtyFile *os.File // temp file for dirty page data + nextDirtyNum int // monotonically increasing } // dirtyPageInfo tracks a written page in a snapshot. @@ -48,7 +48,7 @@ func newSnapshot(srv *Server, ro *baseImageState) (*Snapshot, error) { return &Snapshot{ server: srv, roFile: ro, - dirtyPages: make(map[int64]*dirtyPageInfo), + dirtyPages: make(map[int64]dirtyPageInfo), dirtyFile: tmpFile, }, nil } @@ -71,9 +71,13 @@ func (c *Snapshot) Close() error { return nil } -// readPageData reads a full page, checking dirty pages first, +// readPageData reads a full page into buf, checking dirty pages first, // then falling back to the readonly file. -func (c *Snapshot) readPageData(pageNum int64) ([]byte, error) { +// buf must be at least pageSize bytes long or readPageData panics. +func (c *Snapshot) readPageData(buf []byte, pageNum int64) error { + pageSize := c.server.pageSize + _ = buf[pageSize-1] // bounds check hint; panics if too small + c.mu.Lock() dp, dirty := c.dirtyPages[pageNum] c.mu.Unlock() @@ -86,24 +90,24 @@ func (c *Snapshot) readPageData(pageNum int64) ([]byte, error) { if cache != nil { // Try the global cache first. if data, ok := cache.Get(dp.hash); ok { - return data, nil + copy(buf, data) + return nil } } // Read from the snapshot's dirty file. - buf := make([]byte, c.server.pageSize) - _, err := c.dirtyFile.ReadAt(buf, int64(dp.dirtyNum)*int64(c.server.pageSize)) + _, err := c.dirtyFile.ReadAt(buf[:pageSize], int64(dp.dirtyNum)*int64(pageSize)) if err != nil { - return nil, fmt.Errorf("reading dirty page: %w", err) + return fmt.Errorf("reading dirty page: %w", err) } if cache != nil { - cache.Put(dp.hash, buf) + cache.Put(dp.hash, buf[:pageSize]) } - return buf, nil + return nil } - data, _, result, err := c.roFile.readPage(pageNum) + _, result, err := c.roFile.readPage(buf, pageNum) if err != nil { - return nil, err + return err } switch result { case readFromCache: @@ -113,7 +117,7 @@ func (c *Snapshot) readPageData(pageNum int64) ([]byte, error) { case readFromDiskMiss: c.server.readPath.Add("base_disk_miss", 1) } - return data, nil + return nil } // ReadAt reads len(p) bytes from the snapshot starting at byte offset off. @@ -122,25 +126,33 @@ func (c *Snapshot) ReadAt(p []byte, off int64) (int, error) { length := uint64(len(p)) c.server.readBytes.Add(int64(length)) - offset := uint64(off) pageSize := uint64(c.server.pageSize) pos := uint64(0) for pos < length { - absOffset := offset + pos + absOffset := uint64(off) + pos pageNum := int64(absOffset / pageSize) pageOffset := absOffset % pageSize - pageData, err := c.readPageData(pageNum) - if err != nil { - return int(pos), err - } + // Read up to the end of the page or the end of the requested length, + // whichever is smaller. Usually this will be pageSize, except for the + // first and last pages if the ReadAt call isn't page-aligned. + n := min(pageSize-pageOffset, length-pos) - n := pageSize - pageOffset - if n > length-pos { - n = length - pos + if pageOffset == 0 && n == pageSize { + // Full page; read directly into the caller's buffer. + if err := c.readPageData(p[pos:pos+pageSize], pageNum); err != nil { + return int(pos), err + } + } else { + // Partial page; need a scratch buffer from the pool. + bufp := c.server.pageBufPool.Get().(*[]byte) + if err := c.readPageData(*bufp, pageNum); err != nil { + return int(pos), err + } + copy(p[pos:pos+n], (*bufp)[pageOffset:pageOffset+n]) + c.server.pageBufPool.Put(bufp) } - copy(p[pos:pos+n], pageData[pageOffset:pageOffset+n]) pos += n } return int(pos), nil @@ -167,19 +179,15 @@ func (c *Snapshot) WriteAt(p []byte, off int64) (int, error) { n = length - pos } - var pageData []byte + pageData := make([]byte, pageSize) if pageOffset == 0 && n == pageSize { // Full page write. - pageData = make([]byte, pageSize) copy(pageData, p[pos:pos+n]) } else { // Partial page write: read-modify-write. - existing, err := c.readPageData(pageNum) - if err != nil { + if err := c.readPageData(pageData, pageNum); err != nil { return int(pos), err } - pageData = make([]byte, pageSize) - copy(pageData, existing) copy(pageData[pageOffset:], p[pos:pos+n]) } @@ -199,7 +207,7 @@ func (c *Snapshot) WriteAt(p []byte, off int64) (int, error) { return int(pos), fmt.Errorf("writing dirty page: %w", err) } - c.dirtyPages[pageNum] = &dirtyPageInfo{ + c.dirtyPages[pageNum] = dirtyPageInfo{ hash: h, dirtyNum: dirtyNum, } @@ -214,16 +222,46 @@ func (c *Snapshot) WriteAt(p []byte, off int64) (int, error) { return int(pos), nil } -// handleTrim forgets dirty pages in the given range, reverting -// those pages to the base image. -func (c *Snapshot) handleTrim(offset, length uint64) { +// handleTrim zeros the given byte range and reverts any fully-covered +// dirty pages to the base image. Partial pages at the start and end of +// the range are zero-filled via WriteAt. +func (c *Snapshot) handleTrim(offset, length uint64) error { + end := offset + length pageSize := uint64(c.server.pageSize) - startPage := int64(offset / pageSize) - endPage := int64((offset + length + pageSize - 1) / pageSize) + // Round up to the first fully-covered page. + firstFullPage := int64((offset + pageSize - 1) / pageSize) + // Round down to the last fully-covered page (exclusive). + lastFullPageExcl := int64(end / pageSize) + + // Zero partial start page. + if startRem := offset % pageSize; startRem != 0 { + n := min(pageSize-startRem, length) + bufp := c.server.pageBufPool.Get().(*[]byte) + clear((*bufp)[:n]) + _, err := c.WriteAt((*bufp)[:n], int64(offset)) + c.server.pageBufPool.Put(bufp) + if err != nil { + return err + } + } + + // Zero partial end page, if on a different page than the start. + if endRem := end % pageSize; endRem != 0 && int64(end/pageSize) >= firstFullPage { + bufp := c.server.pageBufPool.Get().(*[]byte) + clear((*bufp)[:endRem]) + _, err := c.WriteAt((*bufp)[:endRem], int64(lastFullPageExcl)*int64(pageSize)) + c.server.pageBufPool.Put(bufp) + if err != nil { + return err + } + } + + // Delete fully-covered pages, reverting them to the base image. c.mu.Lock() defer c.mu.Unlock() - for p := startPage; p < endPage; p++ { + for p := firstFullPage; p < lastFullPageExcl; p++ { delete(c.dirtyPages, p) } + return nil } From 9939aac915274667e37eb8c1a4f70c1974b16dd0 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 12 Jun 2026 14:33:19 +0000 Subject: [PATCH 22/28] add latency histograms Migrated-from: bradfitz/guestbd@95d871e19a3ae6d0970022b81c181f8089dbd0ba --- guestbd/README.md | 2 ++ guestbd/guestbd_test.go | 25 +++++++++++++++++++++++++ guestbd/server.go | 29 +++++++++++++++++++++++++++-- 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/guestbd/README.md b/guestbd/README.md index 4cdba56..06d8d04 100644 --- a/guestbd/README.md +++ b/guestbd/README.md @@ -94,6 +94,8 @@ tailscale.com/metrics's LabelMap and Histogram types. |--------|-------------| | `guestbd_read_size_bytes` | Distribution of NBD read request sizes | | `guestbd_write_size_bytes` | Distribution of NBD write request sizes | +| `guestbd_read_latency_seconds` | Distribution of NBD read latencies (snapshot ReadAt only, excludes wire I/O) | +| `guestbd_write_latency_seconds` | Distribution of NBD write latencies (snapshot WriteAt only, excludes wire I/O) | The `read_path` metric is particularly useful for diagnosing cache effectiveness: diff --git a/guestbd/guestbd_test.go b/guestbd/guestbd_test.go index 501d74b..4b6a61f 100644 --- a/guestbd/guestbd_test.go +++ b/guestbd/guestbd_test.go @@ -6,14 +6,39 @@ import ( "encoding/binary" "fmt" "io" + "math" "net" "os" "os/exec" "path/filepath" "sync" "testing" + "time" ) +func TestLatencyBuckets(t *testing.T) { + got := latencyBuckets() + want := []float64{ + 10e-6, 20e-6, 40e-6, 80e-6, + 160e-6, 320e-6, 640e-6, 1280e-6, + 2560e-6, 5120e-6, 10240e-6, 20480e-6, + 40960e-6, 81920e-6, 163840e-6, 327680e-6, + 655360e-6, 1310720e-6, 2621440e-6, 5242880e-6, + 10485760e-6, + } + if len(got) != len(want) { + t.Fatalf("got %d buckets, want %d: %v", len(got), len(want), got) + } + for i, g := range got { + if math.Abs(g-want[i])/want[i] > 1e-9 { + t.Errorf("bucket %d: got %g, want %g", i, g, want[i]) + } + } + t.Logf("%d buckets, first=%v, last=%v", + len(got), time.Duration(got[0]*float64(time.Second)), + time.Duration(got[len(got)-1]*float64(time.Second))) +} + // testNBDClient is a pure Go NBD client for testing. type testNBDClient struct { t *testing.T diff --git a/guestbd/server.go b/guestbd/server.go index d8f80bf..d43d16b 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -13,6 +13,7 @@ import ( "strings" "sync" "syscall" + "time" "github.com/bradfitz/qcow2" "tailscale.com/metrics" @@ -182,10 +183,24 @@ type Server struct { writePages expvar.Int // counter_guestbd_write_pages readSizeHist *metrics.Histogram // histogram_guestbd_read_size_bytes writeSizeHist *metrics.Histogram // histogram_guestbd_write_size_bytes + readLatencyHist *metrics.Histogram // histogram_guestbd_read_latency_seconds + writeLatencyHist *metrics.Histogram // histogram_guestbd_write_latency_seconds baseImagesActive expvar.Int // gauge_guestbd_base_images_active baseImagesCached expvar.Int // gauge_guestbd_base_images_cached } +// latencyBuckets returns the histogram bucket boundaries (in seconds) +// for NBD read/write latency. The buckets are powers of two from 10µs +// through ~10s, covering cache-hit reads (microseconds), cold disk +// reads (milliseconds), and pathological slow ops. +func latencyBuckets() []float64 { + var b []float64 + for v := 10e-6; v < 16; v *= 2 { + b = append(b, v) + } + return b +} + // NewServer creates a new Server that serves the base image returned by getBase. func NewServer(getBase BaseImageSource, opts ...ServerOption) *Server { cfg := serverConfig{ @@ -231,6 +246,8 @@ func NewServer(getBase BaseImageSource, opts ...ServerOption) *Server { readPath: metrics.LabelMap{Label: "type"}, readSizeHist: metrics.NewHistogram(sizeBuckets), writeSizeHist: metrics.NewHistogram(sizeBuckets), + readLatencyHist: metrics.NewHistogram(latencyBuckets()), + writeLatencyHist: metrics.NewHistogram(latencyBuckets()), } if cache != nil { srv.zeroPageHash = hashPage(make([]byte, pageSize)) @@ -253,6 +270,8 @@ func (s *Server) InitExpvar() { expvar.Publish("counter_guestbd_write_pages", &s.writePages) expvar.Publish("histogram_guestbd_read_size_bytes", s.readSizeHist) expvar.Publish("histogram_guestbd_write_size_bytes", s.writeSizeHist) + expvar.Publish("histogram_guestbd_read_latency_seconds", s.readLatencyHist) + expvar.Publish("histogram_guestbd_write_latency_seconds", s.writeLatencyHist) if s.cache != nil { expvar.Publish("counter_guestbd_cache", &s.cache.path) expvar.Publish("gauge_guestbd_cache_entries", &s.cache.entries) @@ -674,7 +693,10 @@ func (s *Server) serveTransmission(snap *Snapshot, r io.Reader, bw *bufio.Writer s.ops.Add("read", 1) s.readSizeHist.Observe(float64(req.Length)) buf = slices.Grow(buf[:0], int(req.Length))[:req.Length] - if _, err := snap.ReadAt(buf, int64(req.Offset)); err != nil { + t0 := time.Now() + _, err := snap.ReadAt(buf, int64(req.Offset)) + s.readLatencyHist.Observe(time.Since(t0).Seconds()) + if err != nil { if werr := s.sendReply(bw, req.Handle, nbdEIO, nil); werr != nil { return werr } @@ -697,7 +719,10 @@ func (s *Server) serveTransmission(snap *Snapshot, r io.Reader, bw *bufio.Writer if _, err := io.ReadFull(r, buf); err != nil { return fmt.Errorf("reading write data: %w", err) } - if _, err := snap.WriteAt(buf, int64(req.Offset)); err != nil { + t0 := time.Now() + _, err := snap.WriteAt(buf, int64(req.Offset)) + s.writeLatencyHist.Observe(time.Since(t0).Seconds()) + if err != nil { if werr := s.sendReply(bw, req.Handle, nbdEIO, nil); werr != nil { return werr } From 8c39b0a07e1d3a5902e3fe504caa482c9ed2cb7e Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 29 Sep 2026 23:06:50 +0000 Subject: [PATCH 23/28] fix build on darwin syscall.Stat_t.Dev is an int32 on darwin (and uint64 on linux), so FileSource didn't compile there. Migrated-from: bradfitz/guestbd@f645ef53b28e1de27b52842bae2ec7be1e03e137 --- guestbd/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guestbd/server.go b/guestbd/server.go index d43d16b..661291e 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -99,7 +99,7 @@ func FileSource(path string) BaseImageSource { return nil, err } st := fi.Sys().(*syscall.Stat_t) - key := fileIdentityKey{dev: st.Dev, ino: st.Ino} + key := fileIdentityKey{dev: uint64(st.Dev), ino: st.Ino} // Dev is int32 on darwin if strings.HasSuffix(path, ".qcow2") { img, err := qcow2.Open(f) if err != nil { From c608c1eda70aeb04ce67e412883305545dae7da1 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 29 Sep 2026 23:12:45 +0000 Subject: [PATCH 24/28] support NBD_OPT_INFO Apple's Virtualization.framework NBD client (VZNetworkBlockDeviceStorageDeviceAttachment) sends NBD_OPT_INFO, asking for the block size, before NBD_OPT_GO, and gives up if the server replies NBD_REP_ERR_UNSUP. The framework then reports only "The storage device attachment is invalid" when the VM starts. Reply to NBD_OPT_INFO as for NBD_OPT_GO but stay in option haggling, as the spec says. Also gofmt. Migrated-from: bradfitz/guestbd@2c3bd8aa51ff6d6a80f3d77ad98a0e67af363294 --- guestbd/guestbd_test.go | 75 +++++++++++++++++++++++++++++++++++------ guestbd/nbd.go | 1 + guestbd/server.go | 18 +++++++--- 3 files changed, 78 insertions(+), 16 deletions(-) diff --git a/guestbd/guestbd_test.go b/guestbd/guestbd_test.go index 4b6a61f..c072324 100644 --- a/guestbd/guestbd_test.go +++ b/guestbd/guestbd_test.go @@ -11,6 +11,7 @@ import ( "os" "os/exec" "path/filepath" + "slices" "sync" "testing" "time" @@ -82,15 +83,25 @@ func (c *testNBDClient) handshake() { c.t.Fatalf("write client flags: %v", err) } - // Send NBD_OPT_GO with empty export name. - var optBuf [22]byte + c.infoOpt(nbdOptGo) +} + +// infoOpt sends NBD_OPT_GO or NBD_OPT_INFO with an empty export name and +// the given info requests, and reads replies until the ACK. It returns the +// info types the server sent. +func (c *testNBDClient) infoOpt(optCode uint32, infoReqs ...uint16) (infoTypes []uint16) { + c.t.Helper() + optBuf := make([]byte, 22+2*len(infoReqs)) binary.BigEndian.PutUint64(optBuf[0:8], nbdOptsMagic) - binary.BigEndian.PutUint32(optBuf[8:12], nbdOptGo) - binary.BigEndian.PutUint32(optBuf[12:16], 6) // data length: name_len(4) + name(0) + info_count(2) - binary.BigEndian.PutUint32(optBuf[16:20], 0) // name length - binary.BigEndian.PutUint16(optBuf[20:22], 0) // number of info requests - if _, err := c.conn.Write(optBuf[:]); err != nil { - c.t.Fatalf("write opt go: %v", err) + binary.BigEndian.PutUint32(optBuf[8:12], optCode) + binary.BigEndian.PutUint32(optBuf[12:16], uint32(6+2*len(infoReqs))) // name_len(4) + name(0) + info_count(2) + requests + binary.BigEndian.PutUint32(optBuf[16:20], 0) // name length + binary.BigEndian.PutUint16(optBuf[20:22], uint16(len(infoReqs))) // number of info requests + for i, r := range infoReqs { + binary.BigEndian.PutUint16(optBuf[22+2*i:], r) + } + if _, err := c.conn.Write(optBuf); err != nil { + c.t.Fatalf("write opt %d: %v", optCode, err) } // Read option replies until ACK. @@ -113,15 +124,16 @@ func (c *testNBDClient) handshake() { } } - if replyType == nbdRepInfo && replyLen >= 12 { + if replyType == nbdRepInfo && replyLen >= 2 { infoType := binary.BigEndian.Uint16(replyData[0:2]) - if infoType == nbdInfoExport { + infoTypes = append(infoTypes, infoType) + if infoType == nbdInfoExport && replyLen >= 12 { c.exportSize = binary.BigEndian.Uint64(replyData[2:10]) } } if replyType == nbdRepAck { - break + return infoTypes } if replyType&(1<<31) != 0 { c.t.Fatalf("opt reply error: type=%#x", replyType) @@ -129,6 +141,47 @@ func (c *testNBDClient) handshake() { } } +// TestOptInfoThenGo tests the option sequence Apple's Virtualization.framework +// NBD client uses: NBD_OPT_INFO asking for the block size, then NBD_OPT_GO. +func TestOptInfoThenGo(t *testing.T) { + const pageSize = 4096 + data := make([]byte, pageSize*2) + for i := range data { + data[i] = byte(i % 251) + } + addr, _, cleanup := startTestServer(t, data, pageSize) + defer cleanup() + + conn, err := net.Dial("tcp", addr) + if err != nil { + t.Fatal(err) + } + c := &testNBDClient{t: t, conn: conn} + defer c.disconnect() + var greeting [18]byte + if _, err := io.ReadFull(conn, greeting[:]); err != nil { + t.Fatal(err) + } + // Fixed newstyle only, as Apple's client sends. + var clientFlags [4]byte + binary.BigEndian.PutUint32(clientFlags[:], nbdFlagCFixedNewstyle) + if _, err := conn.Write(clientFlags[:]); err != nil { + t.Fatal(err) + } + + infos := c.infoOpt(nbdOptInfo, nbdInfoBlockSize) + if !slices.Contains(infos, nbdInfoExport) || !slices.Contains(infos, nbdInfoBlockSize) { + t.Fatalf("NBD_OPT_INFO replies = %v; want export and block size info", infos) + } + if c.exportSize != uint64(len(data)) { + t.Fatalf("export size = %d, want %d", c.exportSize, len(data)) + } + c.infoOpt(nbdOptGo, nbdInfoBlockSize) + if got := c.read(pageSize, 100); !bytes.Equal(got, data[pageSize:pageSize+100]) { + t.Fatal("read after NBD_OPT_INFO + NBD_OPT_GO mismatch") + } +} + func (c *testNBDClient) read(offset uint64, length uint32) []byte { c.t.Helper() handle := c.nextHandle diff --git a/guestbd/nbd.go b/guestbd/nbd.go index cc1732d..bc7b487 100644 --- a/guestbd/nbd.go +++ b/guestbd/nbd.go @@ -28,6 +28,7 @@ const ( nbdOptExportName uint32 = 1 nbdOptAbort uint32 = 2 nbdOptList uint32 = 3 + nbdOptInfo uint32 = 6 nbdOptGo uint32 = 7 // Option reply types diff --git a/guestbd/server.go b/guestbd/server.go index 661291e..fe602b4 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -166,10 +166,10 @@ type Server struct { pageBufPool sync.Pool // immutable; pool of *[]byte, each pageSize long mu sync.Mutex - sharedSnap *Snapshot // lazily created when useSharedSnapshot is true - roFiles map[any]*baseImageState // identity-keyed entries (active + idle) - roFileNoKey *baseImageState // single entry for nil-key sources - nextIdleSeq int64 // monotonic counter for idle LRU ordering + sharedSnap *Snapshot // lazily created when useSharedSnapshot is true + roFiles map[any]*baseImageState // identity-keyed entries (active + idle) + roFileNoKey *baseImageState // single entry for nil-key sources + nextIdleSeq int64 // monotonic counter for idle LRU ordering snapshots set.Set[*Snapshot] // Metrics (atomic, no mutex needed): @@ -617,7 +617,12 @@ func (s *Server) serveNBD(snap *Snapshot, nc net.Conn) error { } return bw.Flush() - case nbdOptGo: + case nbdOptInfo, nbdOptGo: + // NBD_OPT_INFO gets the same replies as NBD_OPT_GO but stays in + // option haggling. Apple's Virtualization.framework client + // sends NBD_OPT_INFO before NBD_OPT_GO and gives up if it is + // unsupported. + // // Send NBD_INFO_EXPORT. var infoData [12]byte binary.BigEndian.PutUint16(infoData[0:2], nbdInfoExport) @@ -642,6 +647,9 @@ func (s *Server) serveNBD(snap *Snapshot, nc net.Conn) error { if err := bw.Flush(); err != nil { return err } + if optCode == nbdOptInfo { + continue + } return s.serveTransmission(snap, r, bw) case nbdOptList: From 0bfecccb2d0ef3f500d2c28b27f93762b11d317e Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 29 Sep 2026 23:26:20 +0000 Subject: [PATCH 25/28] add Server.SharedSnapshot and Snapshot.WriteDirtyTo So a caller can persist a shared snapshot's writes onto a copy of the base image: boot a VM over NBD, let it write, then keep the result. mantsana uses this to build warm macOS VM snapshots, whose saved machine state only restores with the same kind of disk attachment (NBD) it was saved with. Migrated-from: bradfitz/guestbd@c2d85ddcfd9fd2866979945823de38feb84f768d --- guestbd/persist_test.go | 77 +++++++++++++++++++++++++++++++++++++++++ guestbd/server.go | 9 +++++ guestbd/snapshot.go | 37 ++++++++++++++++++++ 3 files changed, 123 insertions(+) create mode 100644 guestbd/persist_test.go diff --git a/guestbd/persist_test.go b/guestbd/persist_test.go new file mode 100644 index 0000000..9747769 --- /dev/null +++ b/guestbd/persist_test.go @@ -0,0 +1,77 @@ +package guestbd + +import ( + "bytes" + "net" + "os" + "path/filepath" + "testing" +) + +// TestWriteDirtyTo checks that writing a shared snapshot's dirty pages onto +// a copy of the base image reproduces what clients see. +func TestWriteDirtyTo(t *testing.T) { + const pageSize = 4096 + base := make([]byte, 3*pageSize+100) // not a page multiple + for i := range base { + base[i] = byte(i % 251) + } + dir := t.TempDir() + basePath := filepath.Join(dir, "base.img") + if err := os.WriteFile(basePath, base, 0o644); err != nil { + t.Fatal(err) + } + srv := NewServer(FileSource(basePath), WithPageSize(pageSize), WithSharedSnapshot(), WithMaxMem(0)) + defer srv.Close() + if srv.SharedSnapshot() != nil { + t.Fatal("SharedSnapshot non-nil before any connection") + } + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + go srv.Serve(ln) + + c := newTestClient(t, ln.Addr().String()) + c.write(10, bytes.Repeat([]byte{0xaa}, 20)) // partial first page + c.write(pageSize-5, bytes.Repeat([]byte{0xbb}, 10)) // straddles pages 0 and 1 + c.write(3*pageSize+50, bytes.Repeat([]byte{0xcc}, 50)) // partial last page, to EOF + c.write(2*pageSize, bytes.Repeat([]byte{0xdd}, pageSize)) // whole page 2... + c.trim(2*pageSize, pageSize) // ...then trimmed back to base + want := c.read(0, uint32(len(base))) + c.disconnect() + + snap := srv.SharedSnapshot() + if snap == nil { + t.Fatal("SharedSnapshot nil after a connection") + } + outPath := filepath.Join(dir, "persisted.img") + if err := os.WriteFile(outPath, base, 0o644); err != nil { + t.Fatal(err) + } + f, err := os.OpenFile(outPath, os.O_WRONLY, 0) + if err != nil { + t.Fatal(err) + } + pages, err := snap.WriteDirtyTo(f) + if err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) + } + if pages != 3 { + t.Errorf("wrote %d pages, want 3 (pages 0, 1 and 3; page 2 was trimmed)", pages) + } + got, err := os.ReadFile(outPath) + if err != nil { + t.Fatal(err) + } + if len(got) != len(base) { + t.Fatalf("persisted image is %d bytes, want %d", len(got), len(base)) + } + if !bytes.Equal(got, want) { + t.Fatal("persisted image differs from what the client read") + } +} diff --git a/guestbd/server.go b/guestbd/server.go index fe602b4..73c37e6 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -505,6 +505,15 @@ func (s *Server) getOrCreateSnapshot() (snap *Snapshot, owned bool, err error) { return snap, false, nil } +// SharedSnapshot returns the Snapshot shared by all connections of a server +// created with WithSharedSnapshot, or nil if no client has connected yet +// (or the server doesn't share one). +func (s *Server) SharedSnapshot() *Snapshot { + s.mu.Lock() + defer s.mu.Unlock() + return s.sharedSnap +} + // Serve accepts incoming connections on the listener ln, handling // each one on a new goroutine. Serve blocks until the listener // returns an error. The caller is responsible for closing ln. diff --git a/guestbd/snapshot.go b/guestbd/snapshot.go index 706f4e5..c26633b 100644 --- a/guestbd/snapshot.go +++ b/guestbd/snapshot.go @@ -3,7 +3,9 @@ package guestbd import ( "fmt" "io" + "maps" "os" + "slices" "sync" ) @@ -222,6 +224,41 @@ func (c *Snapshot) WriteAt(p []byte, off int64) (int, error) { return int(pos), nil } +// WriteDirtyTo writes every page written to the snapshot, at its offset, +// to w, and returns the number of pages written. Writing them to a copy of +// the base image makes that copy equal to the snapshot's contents, which +// persists a snapshot (for example, a VM image warmed up by booting it over +// NBD). The final page is truncated to the image size. +// +// The caller should ensure nothing writes to the snapshot meanwhile; +// concurrent writes may or may not be included. +func (c *Snapshot) WriteDirtyTo(w io.WriterAt) (pages int, err error) { + c.mu.Lock() + pageNums := slices.Sorted(maps.Keys(c.dirtyPages)) + c.mu.Unlock() + + pageSize := int64(c.server.pageSize) + size := c.roFile.size + bufp := c.server.pageBufPool.Get().(*[]byte) + defer c.server.pageBufPool.Put(bufp) + buf := *bufp + for _, p := range pageNums { + off := p * pageSize + if off >= size { + continue + } + if err := c.readPageData(buf, p); err != nil { + return pages, err + } + n := min(pageSize, size-off) + if _, err := w.WriteAt(buf[:n], off); err != nil { + return pages, err + } + pages++ + } + return pages, nil +} + // handleTrim zeros the given byte range and reverts any fully-covered // dirty pages to the base image. Partial pages at the start and end of // the range are zero-filled via WriteAt. From f0599d7d93329f19ae5b1582b9b398323a3be509 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 1 Oct 2026 15:20:37 +0000 Subject: [PATCH 26/28] guestbd: fix build on non-Unix platforms FileSource keyed base images by the device and inode from syscall.Stat_t, which doesn't exist on Windows, so guestbd and cmd/guestbd failed to build there and broke tb's Windows CI job. Move that into a fileIdentity helper for Unix, and elsewhere return a nil key, which BaseImage already defines as no identity: such base images are simply never coalesced. Updates #cleanup Signed-off-by: Brad Fitzpatrick Change-Id: I617be9c30995dd6a1c9e54bce2bc8108c6bec80a --- guestbd/fileid_other.go | 11 +++++++++++ guestbd/fileid_unix.go | 15 +++++++++++++++ guestbd/server.go | 9 ++++----- 3 files changed, 30 insertions(+), 5 deletions(-) create mode 100644 guestbd/fileid_other.go create mode 100644 guestbd/fileid_unix.go diff --git a/guestbd/fileid_other.go b/guestbd/fileid_other.go new file mode 100644 index 0000000..b01eae4 --- /dev/null +++ b/guestbd/fileid_other.go @@ -0,0 +1,11 @@ +//go:build !unix + +package guestbd + +import "os" + +// fileIdentity returns nil, as files have no device and inode identity +// here. Base images opened from files are then never coalesced. +func fileIdentity(fi os.FileInfo) any { + return nil +} diff --git a/guestbd/fileid_unix.go b/guestbd/fileid_unix.go new file mode 100644 index 0000000..bc11220 --- /dev/null +++ b/guestbd/fileid_unix.go @@ -0,0 +1,15 @@ +//go:build unix + +package guestbd + +import ( + "os" + "syscall" +) + +// fileIdentity returns a BaseImageKey identifying the file described by +// fi by its device and inode. +func fileIdentity(fi os.FileInfo) any { + st := fi.Sys().(*syscall.Stat_t) + return fileIdentityKey{dev: uint64(st.Dev), ino: st.Ino} // Dev is int32 on darwin +} diff --git a/guestbd/server.go b/guestbd/server.go index 73c37e6..41f9814 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -12,7 +12,6 @@ import ( "slices" "strings" "sync" - "syscall" "time" "github.com/bradfitz/qcow2" @@ -54,6 +53,7 @@ func (p *plainBaseImage) Close() error { return nil } func (p *plainBaseImage) BaseImageKey() any { return nil } // fileIdentityKey uniquely identifies an on-disk file by device and inode. +// See fileIdentity. type fileIdentityKey struct { dev uint64 ino uint64 @@ -63,7 +63,7 @@ type fileIdentityKey struct { type fileSizeReaderAt struct { f *os.File size int64 - key fileIdentityKey + key any // from fileIdentity } func (r *fileSizeReaderAt) ReadAt(p []byte, off int64) (int, error) { return r.f.ReadAt(p, off) } @@ -76,7 +76,7 @@ func (r *fileSizeReaderAt) BaseImageKey() any { return r.k type qcow2SizeReaderAt struct { img *qcow2.Image f *os.File - key fileIdentityKey + key any // from fileIdentity } func (r *qcow2SizeReaderAt) ReadAt(p []byte, off int64) (int, error) { return r.img.ReadAt(p, off) } @@ -98,8 +98,7 @@ func FileSource(path string) BaseImageSource { f.Close() return nil, err } - st := fi.Sys().(*syscall.Stat_t) - key := fileIdentityKey{dev: uint64(st.Dev), ino: st.Ino} // Dev is int32 on darwin + key := fileIdentity(fi) if strings.HasSuffix(path, ".qcow2") { img, err := qcow2.Open(f) if err != nil { From 0ab4e438bec51de0b43e946fa21f0993130d7f1c Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 1 Oct 2026 15:33:16 +0000 Subject: [PATCH 27/28] guestbd: fix race in TestReconnectNoIdentity The test disconnects its first client and immediately connects a second, expecting the second to get a fresh base image because sources with nil keys are replaced once idle. But the server releases the first connection's base image asynchronously, after it reads the disconnect. If the second client connected first, it shared the still-active base image, warm page hashes and all, and saw no cold reads. With GOMAXPROCS=1 that happened every time, and it was flaky on macOS CI. Wait for the server to have no active base images before reconnecting. Updates #cleanup Signed-off-by: Brad Fitzpatrick Change-Id: I70389c7db675f2b05bacb01d05bf7f795660251c --- guestbd/guestbd_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/guestbd/guestbd_test.go b/guestbd/guestbd_test.go index c072324..ce1b582 100644 --- a/guestbd/guestbd_test.go +++ b/guestbd/guestbd_test.go @@ -1026,6 +1026,19 @@ type noKeyBaseImage struct { func (n *noKeyBaseImage) BaseImageKey() any { return nil } +// waitBaseImagesIdle waits for srv to have no active base images, as +// happens once it has finished handling all disconnected clients. +func waitBaseImagesIdle(t *testing.T, srv *Server) { + t.Helper() + deadline := time.Now().Add(10 * time.Second) + for srv.baseImagesActive.Value() != 0 { + if time.Now().After(deadline) { + t.Fatalf("timeout waiting for base images to go idle; %d active", srv.baseImagesActive.Value()) + } + time.Sleep(time.Millisecond) + } +} + func TestReconnectNoIdentity(t *testing.T) { const pageSize = 4096 const numPages = 20 @@ -1077,6 +1090,10 @@ func TestReconnectNoIdentity(t *testing.T) { } c1.disconnect() + // Wait for the server to release the first connection's base image. + // Until then it is still active and would be shared, not replaced. + waitBaseImagesIdle(t, srv) + // Snapshot counters. diskColdBefore := srv.readPath.Get("base_disk_cold").Value() From d666f92605bcfc8d8aff97eb4acaf341d93d63c4 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 1 Oct 2026 15:34:19 +0000 Subject: [PATCH 28/28] guestbd: identify base image files on Windows The previous commit made guestbd build on Windows by giving files there no identity key, but that disabled base image coalescing, which TestInodeSharing and TestReconnectHitsCache test, so they failed on Windows CI. Key files on Windows by volume serial number and file index, its equivalent of a device and inode, read from the open handle. Platforms other than Unix and Windows still get no key. Skip TestBaseImageReplaced on Windows: the server keeps the idle base image open for reuse, and Windows can't rename over an open file. Verified by running the Windows test binary under Wine, which reproduced all three CI failures before this change. Updates #cleanup Signed-off-by: Brad Fitzpatrick Change-Id: I45c01fceaa4cc933f0e1c3d64514d5756c631217 --- guestbd/fileid_other.go | 4 ++-- guestbd/fileid_unix.go | 6 +++--- guestbd/fileid_windows.go | 20 ++++++++++++++++++++ guestbd/guestbd_test.go | 6 ++++++ guestbd/server.go | 2 +- 5 files changed, 32 insertions(+), 6 deletions(-) create mode 100644 guestbd/fileid_windows.go diff --git a/guestbd/fileid_other.go b/guestbd/fileid_other.go index b01eae4..e640b45 100644 --- a/guestbd/fileid_other.go +++ b/guestbd/fileid_other.go @@ -1,4 +1,4 @@ -//go:build !unix +//go:build !unix && !windows package guestbd @@ -6,6 +6,6 @@ import "os" // fileIdentity returns nil, as files have no device and inode identity // here. Base images opened from files are then never coalesced. -func fileIdentity(fi os.FileInfo) any { +func fileIdentity(f *os.File, fi os.FileInfo) any { return nil } diff --git a/guestbd/fileid_unix.go b/guestbd/fileid_unix.go index bc11220..4f4d62d 100644 --- a/guestbd/fileid_unix.go +++ b/guestbd/fileid_unix.go @@ -7,9 +7,9 @@ import ( "syscall" ) -// fileIdentity returns a BaseImageKey identifying the file described by -// fi by its device and inode. -func fileIdentity(fi os.FileInfo) any { +// fileIdentity returns a BaseImageKey identifying the open file f, +// described by fi, by its device and inode. +func fileIdentity(f *os.File, fi os.FileInfo) any { st := fi.Sys().(*syscall.Stat_t) return fileIdentityKey{dev: uint64(st.Dev), ino: st.Ino} // Dev is int32 on darwin } diff --git a/guestbd/fileid_windows.go b/guestbd/fileid_windows.go new file mode 100644 index 0000000..95b886d --- /dev/null +++ b/guestbd/fileid_windows.go @@ -0,0 +1,20 @@ +package guestbd + +import ( + "os" + "syscall" +) + +// fileIdentity returns a BaseImageKey identifying the open file f by its +// volume serial number and file index, Windows' equivalent of a device +// and inode. It returns nil, disabling coalescing, if they can't be read. +func fileIdentity(f *os.File, fi os.FileInfo) any { + var d syscall.ByHandleFileInformation + if err := syscall.GetFileInformationByHandle(syscall.Handle(f.Fd()), &d); err != nil { + return nil + } + return fileIdentityKey{ + dev: uint64(d.VolumeSerialNumber), + ino: uint64(d.FileIndexHigh)<<32 | uint64(d.FileIndexLow), + } +} diff --git a/guestbd/guestbd_test.go b/guestbd/guestbd_test.go index ce1b582..b773183 100644 --- a/guestbd/guestbd_test.go +++ b/guestbd/guestbd_test.go @@ -11,6 +11,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "slices" "sync" "testing" @@ -1117,6 +1118,11 @@ func TestReconnectNoIdentity(t *testing.T) { } func TestBaseImageReplaced(t *testing.T) { + if runtime.GOOS == "windows" { + // The server keeps the idle base image open for reuse, and + // Windows can't rename over a file that's open. + t.Skip("can't replace an open file by rename on Windows") + } const pageSize = 4096 const numPages = 4 data1 := make([]byte, pageSize*numPages) diff --git a/guestbd/server.go b/guestbd/server.go index 41f9814..928dcb4 100644 --- a/guestbd/server.go +++ b/guestbd/server.go @@ -98,7 +98,7 @@ func FileSource(path string) BaseImageSource { f.Close() return nil, err } - key := fileIdentity(fi) + key := fileIdentity(f, fi) if strings.HasSuffix(path, ".qcow2") { img, err := qcow2.Open(f) if err != nil {