Follow-up on the questions left open in this thread. Three things came out differently from what I said above, so corrections first, and my earlier headline numbers need revising upward.

My earlier ratios were too low: the rig was the bottleneck

Everything I posted before ran NVMe to NVMe, where the destination disk saturates around 250 to 300 MB/s and degrades as it fills. That capped the faster arms, arm C worst of all.

Rebuilt with RAM-backed SRs on both hosts, no storage ceiling, full four-arm matrix on one rig, 3 runs per arm, interleaved.

Per-arm spread is 0.3% to 1.7%.

The full stack is worth 7.4x, not 5.51x, and TCP_NODELAY alone is worth 4.26x, not 2.80x.

The two changes are substitutes, not complements:

• TCP_NODELAY alone: 4.26x
• pipelining alone: 7.41x
• both together: 7.39x

So:

• Pipelining alone captures the whole win.
• Adding the socket option on top of it is 0.997x, with overlapping ranges. Nothing.
• The reverse is not true: pipelining on top of NODELAY is still worth 1.73x.
• NODELAY does not substitute for pipelining. Pipelining substitutes for NODELAY, on throughput.

What the socket option still does after pipelining is remove the stall, 16.1% of samples down to 1.1%, for 1.14% more segments. On this rig that buys no throughput. See the RTT caveat before writing it off.

Correction 1: I said Chunked might regress. It does.

xe vdi-copy between two host-local SRs, 100 GiB, interleaved A/B/A/B. @TeddyAstie was right.

Both controls beat both patched runs with no overlap, so this is not drift. TCP_NODELAY costs +2.7% wall clock and +0.70% segments here, and buys nothing.

Nothing, because the stall it fixes does not occur on this path:

• Sub-MSS stalls are 0.0% in every arm, including the unpatched control.
• Chunked writes a 12 byte header then data, with no per-request reply.
• So the send queue stays backed up (median notsent about 3.3 MB, rwnd_limited 99.7%).
• Nagle never faces a small-segment decision. Packing is already 0.993x MSS before the patch.

Where the extra packets come from, and it is not mainly the headers:

• The disk is fully allocated, so expand_copy (f.ml:2745) splits at its 2 MiB cap: 51,200 chunks.
• That is only 9.8% of the 521,126 extra segments.
• The rest is TLS record boundaries. The channel is unbuffered (channels.ml:129), so each 16 KiB TLS record is its own 16,413 byte write, 11.33x MSS, leaving a sub-MSS remainder.
• With Nagle those coalesce with the next record. With NODELAY they go out uncoalesced whenever the send queue drains.
• strace confirms both shapes: 41 byte writes are the headers (12 plus 29 of AES-GCM), 16,413 byte writes are the bulk records.

Data is correct either way: the patched copy md5s identical to the source, ce647d9436b48401cd4b489c955ef0f7.

Protocol confirmed two ways, for anyone reproducing:

• An https destination not advertising transfer-encoding: nbd yields [Chunked; NoProtocol] (impl.ml:1084), and the code takes List.hd.
• At runtime the receiver runs vhd-tool serve --source-format raw --source-protocol chunked --source-fd 8.

Correction 2: my reasoning for "NODELAY is a no-op after pipelining" was wrong

I said that with 8 requests in flight there is almost always an MSS queued, so the sub-MSS condition rarely arises. Not what happens. Arm D is pipelining with the socket patch removed, and it still stalls on 16.1% of samples against 1.1% with it. Depth 8 does not keep the queue above MSS.

The conclusion survives, the reasoning does not: NODELAY after pipelining gives no throughput gain because the stall stops being the limiting factor, not because the stall goes away.

Correction 3: the export path needs nothing

• Export runs stream_raw (impl.ml:335) : export_raw_vdi.ml:56 passes "none" as the destination protocol. No headers, no replies, so no write-write-read pattern.
• The patch could not cover it anyway: --destination-fd becomes fd://N, then File_descr, then Channels.of_raw_fd (impl.ml:991), never touching the socket helper.
• It does not need to. http_svr.ml:624 already sets TCP_NODELAY on the listening socket and Linux passes it to accepted sockets. Verified on the 4.19 kernel these hosts run, with a listener without the option as a control returning 0.
• stunnel sets it independently too (xapi_stunnel_server.ml:66-68).

The cost of TCP_NODELAY, every test we ran

About 1.2% more segments, reproducible to three digits across four independent experiments. Small, and it is the real permanent cost of the option.

Where the ceiling is now

At 7.4x we are at about 525 MiB/s, and it is not the obvious things:

• Not the network: 10G link, roughly 46% utilised.
• Not the sender: sparse_dd at mean 57%, peak 64% of one core.
• On the receiving host, tapdisk is roughly 72% of all busy CPU (python3 9%, xapi 6%, stunnel 6%).

That sample spans more than one leg and sums several tapdisk processes, so read it as dominance rather than a precise figure. It is where we would look next.

Our reading

Both changes are worth having, they are independent, and they are not additive, so the order is a real decision.

TCP_NODELAY

• Strongest argument is precedent, not the benchmark: vhd-tool is the only NBD client in the toolstack that leaves Nagle on.
• QEMU sets it unconditionally (nbd/client-connection.c:143) and forwards it through TLS. blktap treats failing to set it as fatal (drivers/block-nbd.c:793). xapi and stunnel both set it.
• 12 lines, 4.26x on code that ships today.
• Costs, now measured rather than assumed: about 1.2% more segments everywhere, and 2.7% wall clock on vdi-copy between two host-local SRs where it gains nothing.

Pipelining

• 7.41x on its own, and it makes the socket option irrelevant to throughput.
• Less work than it looks: the multiplexer already exists and is unused (nbd/lib/client.ml:78), so this is not the NBD redesign it first appears to be.
• The real obstacle is buffer ownership. expand_copy hands out slices of one shared 2 MiB buffer, so a naive pipeline silently corrupts data while reporting success.
• Our prototype works around it with a local buffer pool and a memcpy per block. The proper fix belongs inside expand_copy in f.ml, a shared library with other consumers.
• The prototype is not mergeable as it stands: progress counts issued rather than completed work, and a failed write leaves its siblings unawaited.

We will follow whichever route the XAPI team prefers and are happy to do the work either way. Our own order would be the socket option first, then pipelining done properly rather than our prototype. That is a sequencing preference, not a claim that the gains compose. If you would rather go straight to pipelining and skip the socket change, our data supports that: it reaches 7.41x on its own.

One caveat that cuts in favour of the socket option, and our rig cannot measure it:

• Every number here is from a 0.2 ms RTT link.
• The Nagle stall costs a delayed-ACK round trip, so its cost scales with RTT.
• 0.2 ms is therefore the regime least favourable to fixing it.
• On a higher-RTT link (cross-rack, cross-site, DR replication) the same 16.1% stall rate that costs nothing here should cost real throughput.
• So read 4.26x as a floor for TCP_NODELAY, and "pipelining makes it redundant" as a statement about short links specifically.
• We have not measured a long-RTT link and would like to hear from anyone who has.

Open questions

1. Is the 2.7% on vdi-copy acceptable, or should the option be scoped to NBD? Unconditional is simpler and matches every other component. Scoping avoids a measured regression on a path that cannot benefit.
2. Is the Chunked header worth batching regardless? It is a separate unbuffered 12 byte write before every chunk, which under TLS becomes its own 41 byte record. Writing header and payload together removes that independently of any socket option, and helps the Nagle-on case too.
3. Should expand_copy own a buffer pool? Any pipelining implementation needs it. Fixing it in f.ml fixes it for every consumer, but that is a wider blast radius than we wanted to take unilaterally.
4. Is the receiving tapdisk the next real ceiling? At 7.4x the sender and the network both still have headroom and tapdisk dominates destination CPU. We have not dug into why.
5. Does the picture change on a high-RTT or faster link? Both conclusions, that NODELAY is worth 4.26x and that pipelining makes it redundant, are properties of a short fast link that never saturated.

Rig

• Two host XCP-ng 8.3 pool, xapi 26.1.16, 10G, 0.2 ms RTT.
• Arms differ by exactly one setsockopt where relevant, same tree and toolchain.
• Every run verified by binary sha256 before it started, transfers checksum verified.
• RAM SRs are ext4 on a brd ramdisk. tmpfs does not work: no O_DIRECT, so VDIs can be written onto such an SR but never migrated back off.

I have no issue using Claude, but I would prefer your own conclusions/recommendations in the end and less text

I'm doing more tests right now, ideally with RAM drives to make sure we measure the right bottleneck

Hi!

1. It is not the XOCE to xo-server link, it is the credential xo-server uses to log into XAPI on your host.
2. It has to be reversible, not hashed, because xo-server replays it to XAPI on every connect and auto-reconnect. So yes, plaintext by default is expected. The security perimeter here is Redis itself (bound to localhost or a unix socket, never exposed) and root access on the XO VM.

That said, since XO 6.5 there is an opt-in encryption at rest. In your xo-server config:

[redis]
encryptCredentialDatabase = true

On the next start, every record is encrypted with AES-256-GCM (values get an enc: prefix) and index keys become HMAC-SHA256 blind indexes, so hostnames and emails are not readable either. The key is split in two halves, one in XenStore (vm-data/xo-encryption-key) and one in /var/lib/xo-server/data/xo-encryption-key.

Caveats for a source install:

• XO must run as a VM on XCP-ng/XenServer with xen-tools, and xo-server needs access to the xenstored socket (root, typically).
• Config export then requires a passphrase.
• If you lose one key half while encryption is on, do not restart: XO would regenerate both halves and the existing records would become undecryptable.

Full doc: https://docs.xen-orchestra.com/credential-encryption

About the PoolAlreadyConnected part, that is a separate issue. It means two server entries resolve to the same pool, typically a slave host registered next to the master, or the pool re-added after a master change. Removing the entry with DEL xo:server:<id> is not the right fix: it drops the hash but leaves the id in xo:server_ids and in the host index. Remove the server from the UI instead. If the error persists, it is the other entry pointing to that same pool that needs to go.

@TeddyAstie you were right, and it's the bigger win. I prototyped the pipelining you described and measured it. Same rig, same 107 GiB disk, same direction as the earlier runs.

Pipelining is worth a further 1.97x on top of the Nagle fix. Your diagnosis was correct: the per-request reply gating, not TCP, is the dominant limit.

Verified byte for byte, source and destination md5 of the 107 GiB disk both ce647d9436b48401cd4b489c955ef0f7. That mattered more than the stopwatch here, for reasons below.

It's cheaper than you thought: the multiplexer already exists

No NBD redesign is needed. nbd/lib/client.ml:78 is already module Rpc = Mux.Make (NbdRpc), and that multiplexer:

• assigns every request a unique handle (get_handle)
• registers a waiter in id_to_wakeup keyed by that handle
• serialises only the send under outgoing_mutex, then returns a promise
• runs a background dispatcher thread that reads replies and wakes the matching waiter

So concurrent Client.write calls already interleave correctly. The whole request/reply machinery is there and unused. The serialisation is one fold_left in stream_nbd that awaits each write before pulling the next element.

The rest of the chain was already fine too:

Depth 8 matches tapdisk's pool. The fio sweep says deeper is not better.

The actual trap: buffer ownership

This is the part worth writing down, because it is invisible from the protocol level and it bites silently.

Vhd_format.F.expand_copy allocates one 2 MiB buffer and hands out slices of it:

let buffer = Memory.alloc twomib_bytes in
...
let data = Cstruct.sub buffer 0 (this * 512) in
really_read h (sector_start ** 512L) data >>= fun () ->
return (Cons (`Sectors data, next))

It refills that same buffer on every step. The current sequential code is safe only as a side effect of awaiting each write before pulling the next element.

Pipeline it naively and you get: launch write N, pull element N+1, really_read overwrites the buffer, write N puts block N+1's bytes at block N's offset. The migration completes, reports success, and the disk is corrupt. Nothing in the stack flags it.

So any implementation of this needs buffer ownership solved alongside the concurrency. My prototype takes the cheap local route: a pool of depth buffers in stream_nbd, one memcpy per 2 MiB block, buffer returned only once its write completes. The proper fix is a buffer pool inside expand_copy itself, but f.ml is a shared library with other consumers, so that is a wider change than I wanted for a measurement.

Prototype patch

Against xapi-project/xen-api, ocaml/vhd-tool/src/impl.ml, on top of the TCP_NODELAY patch from the previous post.

This is a measurement prototype, not mergeable as-is. Known gaps:

• progress reporting counts issued rather than completed work
• a failed write leaves its siblings unawaited rather than cancelled
• the per-block memcpy is a workaround for the shared buffer, not the right fix

--- a/ocaml/vhd-tool/src/impl.ml
+++ b/ocaml/vhd-tool/src/impl.ml
@@ stream_nbd
   (if not prezeroed then expand_empty s else return s) >>= fun s ->
   expand_copy s >>= fun s ->
+  (* Pipelined writes. The NBD client already multiplexes: every request gets a
+     unique handle and a background dispatcher matches replies back to waiters,
+     so several writes may be outstanding at once. Issuing them one at a time
+     makes every request pay a full round trip.
+
+     Depth 8 matches NBD_SERVER_NUM_REQS in tapdisk's NBD server. Deeper just
+     queues.
+
+     Buffer ownership matters here. [expand_copy] hands out slices of a single
+     shared 2MiB buffer that it refills on every step, so an in-flight write
+     cannot keep pointing at it: pulling the next element would overwrite the
+     bytes before they reach the wire. Each outstanding write therefore gets a
+     private buffer from a pool sized to the pipeline depth, returned only once
+     the write has completed. *)
+  let depth = 8 in
+  let twomib = 2 * 1024 * 1024 in
+  let free = ref (List.init depth (fun _ -> IO.alloc twomib)) in
+  let inflight = ref [] in
+  let reap () =
+    match !inflight with
+    | [] ->
+        return ()
+    | l ->
+        Lwt.nchoose_split (List.map fst l) >>= fun (_, pending) ->
+        let still, done_ =
+          List.partition (fun (t, _) -> List.memq t pending) l
+        in
+        inflight := still ;
+        free := List.map snd done_ @ !free ;
+        return ()
+  in
+  let rec drain () =
+    if !inflight = [] then return () else reap () >>= fun () -> drain ()
+  in
   fold_left
     (fun (sector, work_done) x ->
       ( match x with
-        | `Sectors data -> (
-            Client.write server (Int64.mul sector 512L) [data] >>= function
-            | Ok () ->
-                return Int64.(of_int (Cstruct.length data))
-            | Error _e ->
-                fail (Failure "Got error from NBD library")
-          )
+        | `Sectors data ->
+            (* Block only when the pipeline is full. *)
+            (if !free = [] then reap () else return ()) >>= fun () ->
+            let buf = List.hd !free in
+            free := List.tl !free ;
+            let len = Cstruct.length data in
+            let mine = Cstruct.sub buf 0 len in
+            Cstruct.blit data 0 mine 0 len ;
+            let t =
+              Client.write server (Int64.mul sector 512L) [mine] >>= function
+              | Ok () ->
+                  return ()
+              | Error _e ->
+                  fail (Failure "Got error from NBD library")
+            in
+            inflight := (t, buf) :: !inflight ;
+            return Int64.(of_int len)
         | `Empty _n ->
             (* must be prezeroed *)
             assert prezeroed ;
             return 0L
@@
     (0L, 0L) s.elements
   >>= fun _ ->
+  (* Every write must land before the stream is declared complete. *)
+  drain () >>= fun () ->
   p total_work ;
 
   return (Some total_work)

Caveats on the numbers

The destination SSD degrades partway through a large transfer (DRAM-less Lexar NM790, host with 3.9 GB RAM), which is why arm C starts at ~432 MiB/s and settles around 275. Arm C runs closest to that ceiling so it feels it most. On better destination storage the gap should widen, not narrow.

Also worth saying: after pipelining, TCP_NODELAY matters much less, since with 8 requests in flight there is almost always at least an MSS queued. It is still correct to set it, and it is what every other NBD client in the stack does, but the 2.8x from the previous post should be read as "what you get today with a 12 line change", not as something that stacks cleanly onto the 5.5x.

Also adding @Team-XAPI-Network

Here is the exact patch used for the benchmarks above, so the XAPI team can pick it up directly.

Target: xapi-project/xen-api, file ocaml/vhd-tool/src/impl.ml

Note the standalone xapi-project/vhd-tool repo is not the right target. It has been dead since 2021-05-21 (vendored into xen-api on 2021-09-20) and has diverged: socket sits at line 778 there versus 795 in what actually ships.

The patch

diff --git a/ocaml/vhd-tool/src/impl.ml b/ocaml/vhd-tool/src/impl.ml
--- a/ocaml/vhd-tool/src/impl.ml
+++ b/ocaml/vhd-tool/src/impl.ml
@@ -800,7 +800,20 @@ let socket sockaddr =
     | Lwt_unix.ADDR_UNIX _ ->
         Unix.PF_UNIX
   in
-  Lwt_unix.socket family Unix.SOCK_STREAM 0
+  let sock = Lwt_unix.socket family Unix.SOCK_STREAM 0 in
+  (* Disable Nagle's algorithm on TCP sockets. The stream protocols used here
+     (NBD and Chunked) write a small header and its payload with separate
+     write(2) calls, and NBD then waits for a per-request reply. Combined with
+     the peer's delayed ACKs this is the classic write-write-read stall: the
+     header sits in the send queue for up to 40ms waiting for an ACK that the
+     peer is itself delaying. Not applicable to Unix domain sockets. *)
+  ( match sockaddr with
+  | Lwt_unix.ADDR_INET _ ->
+      Lwt_unix.setsockopt sock Unix.TCP_NODELAY true
+  | Lwt_unix.ADDR_UNIX _ ->
+      ()
+  ) ;
+  sock
 
 let split ~limit ~sep str =
   Xapi_stdext_std.Xstringext.String.split ~limit sep str

Why this spot

socket is the only Lwt_unix.socket call site in the whole of vhd-tool, and both Lwt_unix.connect call sites use it:

• impl.ml:1030, the tcp: endpoint
• impl.ml:1057, the http/https destination that sparse_dd uses for storage motion

So one change covers all of vhd-tool's outbound TCP.

Two things a reviewer will probably ask

Does it survive TLS? Yes. The option is set on the fd before Channels.of_ssl_fd wraps it, so https:// destinations are covered. QEMU does the same thing deliberately in io/channel-tls.c, forwarding set_delay down to the underlying socket.

Why the ADDR_UNIX guard? unix: is a real endpoint scheme (impl.ml:764), and setsockopt(TCP_NODELAY) on an AF_UNIX socket fails with EOPNOTSUPP. Verified on the built binary: the AF_UNIX path issues no setsockopt at all and produces no EOPNOTSUPP.

The same helper is also used for the listening socket in serve (impl.ml:1301). That is harmless, since Linux inherits TCP_NODELAY onto accepted sockets, so it is a small bonus rather than a bug.

Build and verification

Built against XCP-ng 8.3 from xcp-ng-rpms/xapi branch 8.3 (1502e68), source xen-api-26.1.16.tar.gz, in ghcr.io/xcp-ng/xcp-ng-build-env:8.3. Applies cleanly on top of the 20 existing XCP-ng patches, including the qcow2 hybridqcow one, which touches a different region of the same file.

vhd-tool ships as a subpackage of the xapi SRPM. It has no package dependencies beyond shared libraries, and xapi-core's Requires: vhd-tool is unversioned, so it can be installed standalone for testing:

rpm -Uvh vhd-tool-<version>.x86_64.rpm     # add --oldpackage to downgrade
yum downgrade vhd-tool                     # to roll back

No daemon restart needed, since sparse_dd is forked per migration.

Confirmed on the resulting binary:

socket(AF_INET, SOCK_STREAM, IPPROTO_IP) = 6
setsockopt(6, SOL_TCP, TCP_NODELAY, [1], 4) = 0
connect(6, {AF_INET, ...})

The control build (same tree, patch removed) shows zero TCP_NODELAY calls, which is what makes the A/B in the previous post attributable to this change alone.

I have deliberately left the DCO Signed-off-by line off so whoever opens the PR can add their own. Original report and diagnosis credit goes to @tosh.

@TeddyAstie You're right on the important part, and I went and measured the rest.

First, the boring argument: TCP_NODELAY is already the default everywhere else in this stack.

vhd-tool is the only NBD client in the toolstack that leaves Nagle on. If it were a bad default for bulk NBD, QEMU wouldn't do it unconditionally. So this patch is less "new tuning" and more "stop being the exception".

On "bulk transfers benefit from Nagle's coalescing": correct, and it costs 1.16%.

Two-host 8.3 pool, 10G, live SXM, two RPMs from the same tree differing only by that one setsockopt (control build, so the A/B isn't a build-environment artifact).

Nagle really does pack 99.6% of MSS. But NODELAY still packs 98.4%, because vhd-tool's payload writes are 2 MiB and were never sub-MSS in the first place. The only thing being coalesced is the 28-byte NBD header, and it's paid for with a delayed-ACK round trip. No small-packet flood.

What Nagle is actually holding (107 GiB migration, ss -tinp every 200 ms):

62% of the run sitting on a 541-byte header with 4 segments in flight, on a 0.2 ms RTT link. The 0.6% left in the patched arm have 285 segments in flight, so that's a full pipe, not a stall.

Result: transfer phase 2016.9s to 679.9s (~3x), end-to-end 2080.8s to 743.8s. Same stall signature on iSCSI and on local NVMe, so it's the protocol pattern rather than the storage.

On the redesign: I agree, and I don't think it's either/or.

The reply-gating is the real ceiling. Even patched we only reach ~210 MiB/s on a 10G link, because it's still one request at a time. Pipelining is the bigger win and it's the correct fix.

But once writes are pipelined there's almost always at least an MSS queued, so the sub-MSS condition rarely arises and TCP_NODELAY becomes close to a no-op. It isn't something anyone would have to unwind afterwards. 12 lines now, redesign later, no conflict.

Where you might still be right: the regression case for NODELAY is many small writes with no application-level batching. vhd-tool's NBD path is header + 2 MiB so there's nothing to merge, but the Chunked path (12-byte header, no per-request reply) is a different shape, and that's what vdi-copy negotiates between two host-local SRs. I haven't tested that one. If anyone expects a regression, that's where I'd look.

Worth mentioning @Team-Storage

Nice! Have you tested the snapshot, revert and so on?