|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 10/12] tools/libxl: add support for xenstore quota in domain_config
On Fri, Mar 20, 2026 at 3:02 PM Juergen Gross <jgross@xxxxxxxx> wrote: Add support for xenstore quota in the struct domain_config. Initially This change seems to cause `xl list` to hang on oxenstored systems. I'm not an expert in ocaml; Claude seems to think that the problem is as follows: 1. oxenstored doesn't implement XS_GET_QUOTA, and 2. when it receives an unknown message it returns an error, but 3. the error payload is length zero, and 4. oxenstored has a bug such that it will never actually send messages with a zero-length payload. Below is a more complete report from Claude; as I said, I don't know enough ocaml to evaluate it. Plan `xl list` only triggers it on staging, but apparently `xl list -x` will trigger it in Xen 4.21. Assuming the analysis is correct, it's concerning that an issue with the default xenstored instance wasn't caught earlier. -George 8<--- Subject: oxenstored hangs on any xenstore request with a zero-length payload Affects, on oxenstored systems: - `xl info -x` and `xenstore watch -d N` -- RELEASE-4.21.0, RELEASE-4.21.1 - `xl list -l`, `xl save`, `xl migrate` -- 4.22.0-rc1..rc3, unstable The C xenstored and xenstore-stubdom are unaffected. Symptom ------- On an idle dom0 with no guests, running oxenstored, `xl list -l` never returns and produces no output. The same holds for `xl save`, `xl migrate`, `xl info -x`, and `xenstore watch -d N`. Meanwhile `xl list`, `xl list -v`, `xl info -n`, `xenstore-read` and `xenstore-ls` all work normally. The client is blocked in read(fd, buf, 16) awaiting an xsd_sockmsg header that never arrives. Both socket queues are empty. oxenstored itself remains healthy and continues to serve every other connection. Root cause ---------- oxenstored's socket backend never delivers a request whose payload length is zero. This is independent of the opcode: XS_READ and XS_DIRECTORY with len == 0 hang exactly as XS_GET_QUOTA does. In tools/ocaml/libs/xb/xb.ml, Xb.input() consumes the 16-byte header and sets partial_in <- HaveHdr pkt, then returns None unconditionally: | NoHdr (i, buf) -> (* we complete the partial header *) if sz > 0 then Bytes.blit b 0 buf (Partial.header_size () - i) sz; con.partial_in <- if sz = i then HaveHdr (Partial.of_string (Bytes.to_string buf)) else NoHdr (i - sz, buf); None The packet would be produced by the *next* call to input() -- the HaveHdr branch handles Partial.to_complete = 0 without reading anything further. But has_more_input is hardcoded false for socket backends: let has_more_input con = match con.backend with | Fd _ -> false | Xenmmap backend -> backend.work_again so xenstored.ml re-enters do_input only when poll() reports the fd readable. - For len > 0, the payload is still buffered in the socket, so poll() fires again and the packet completes. This is why every other request works. - For len == 0, nothing remains to be read. poll() never fires, and a fully-received request sits parked in partial_in indefinitely. It is finally processed only when further input arrives on that connection -- including POLLHUP at client teardown -- at which point the reply is written to a dead socket. oxenstored's dispatch layer is correct: XS_GET_QUOTA with a non-empty payload returns XS_ERROR "ENOSYS" in under a millisecond. Only the framing layer is broken. Guest ring (Xenmmap) connections appear unaffected, since there has_more_input follows work_again. NOTE: this is a source-level inference; I did not exercise a ring connection with a zero-length request. Evidence -------- strace of oxenstored. \31 = 25 = XS_GET_QUOTA, \2 = XS_READ: # len=0, unknown opcode 25 -- nothing written, silence, then client EOF 14:54:01.801066 read(23, "\31\0\0\0\336\300\0\0\0\0\0\0\0\0\0\0", 16) = 16 14:54:06.806325 write(18, "[...] A81 invalid", 50) <- only at POLLHUP 14:54:06.806811 read(23, "", 16) = 0 # len=0, WELL-KNOWN opcode 2 -- identical hang 14:54:10.840293 read(23, "\2\0\0\0\336\300\0\0\0\0\0\0\0\0\0\0", 16) = 16 14:54:15.845644 read(23, "", 16) = 0 # same opcode 25, len=2 -- answered instantly 14:54:19.879492 read(23, "\31\0\0\0\336\300\0\0\0\0\0\0\2\0\0\0", 16) = 16 14:54:19.879698 read(23, "0\0", 2) = 2 14:54:19.879810 write(18, "[...] A83 invalid / error ENOSYS", ...) The poll() following the parked read requests POLLIN only -- no POLLOUT -- i.e. oxenstored has nothing queued to send. xenstored.log contains neither "process packet:" nor "got a bad client": process_packet is never entered. Decisive: send the len == 0 header, wait three seconds (nothing), then send one unrelated byte. The ENOSYS reply *for the original request id* is emitted within 1 ms. The request was never dropped -- it was parked, waiting for a poll() that had nothing left to deliver. Trigger ------- libxenstore has exactly two zero-length senders, both recent. Before them, no xenstore client had ever sent a zero-length request over the socket, so the defect lay dormant. xs_get_features_supported() XS_GET_FEATURE tools/libs/store/xs.c:1486 added by 5234b61eab in: RELEASE-4.21.0, RELEASE-4.21.1, 4.22.0-rc*, unstable reached by: xl info -x, xenstore watch -d N xs_get_quota_names() XS_GET_QUOTA tools/libs/store/xs.c:1537 added by ba90589447 in: 4.22.0-rc1..rc3, unstable reached by: xl list -l, xl save, xl migrate -- since a6f159f92c made libxl_retrieve_domain_configuration() query xenstore quota Both are the only `struct iovec iov[1]` call sites in xs.c. libxl is not at fault --------------------- tools/libs/light/libxl_xsquota.c explicitly treats quota as optional: names = xs_get_quota_names(ctx->xsh, &num); if (!names) { /* Xenstore quota support is optional! */ if (errno != ENOSYS) { ... rc = ERROR_FAIL; } else { rc = 0; } Confirmed empirically: with an LD_PRELOAD shim forcing xs_get_quota_names() to fail with ENOSYS, against the same hanging oxenstored, `xl list -l` returns rc=0 with full JSON and `xl save` writes a 2.1 GB savefile. A xenstored that lacks opcode 25 but *answers* ENOSYS works fine. The C xenstored answers ENOSYS to unknown opcodes at any length. Age --- The defect is present in oxenstored's original commit: c3afd398ba7f349fdf59d79aaed4da256176806a Keir Fraser <keir.fraser@xxxxxxxxxx>, 2010-05-06 "ocaml: Add XS bindings." `git log -S has_more_input -- tools/ocaml/libs/xb/xb.ml` returns that commit and no other; has_more_input has never been modified. The NoHdr branch is unchanged in substance since RELEASE-4.2.0, and `git diff RELEASE-4.21.0 HEAD -- tools/ocaml/libs/xb/{xb,partial,op}.ml` is empty -- the files are byte-identical. Reproducer ---------- Attached: oxs-zerolen.c (50 lines, no dependencies). gcc -o oxs-zerolen oxs-zerolen.c && sudo ./oxs-zerolen Against oxenstored: XS_READ(2) len=21 (sanity) -> reply type=2 len=8 XS_GET_QUOTA(25) len=2 (unknown op) -> reply type=16 len=7 [ENOSYS] XS_READ(2) len=0 (known op!) -> NO REPLY (5s timeout) XS_DIRECTORY(1) len=0 (known op!) -> NO REPLY (5s timeout) XS_GET_FEATURE(23) len=0 (xl info -x) -> NO REPLY (5s timeout) XS_GET_QUOTA(25) len=0 (xl list -l) -> NO REPLY (5s timeout) Against the C xenstored: all six reply in 0.000 s. Fix --- In Xb.input, return the packet as soon as a complete header with len == 0 has been read, rather than returning None and waiting for input that will never arrive: | NoHdr (i, buf) -> if sz > 0 then Bytes.blit b 0 buf (Partial.header_size () - i) sz; if sz = i then begin let partial_pkt = Partial.of_string (Bytes.to_string buf) in if Partial.to_complete partial_pkt = 0 then begin con.partial_in <- init_partial_in (); Some (Packet.of_partialpkt partial_pkt) (* was: None *) end else (con.partial_in <- HaveHdr partial_pkt; None) end else (con.partial_in <- NoHdr (i - sz, buf); None) Alternatively, fix has_more_input, which is where the invariant is actually violated -- "a complete packet is buffered" ought to imply more work is pending. This duplicates no packet construction, since the HaveHdr branch already handles to_complete = 0 with no read: let has_more_input con = match con.backend with | Fd _ -> (match con.partial_in with | HaveHdr p -> Partial.to_complete p = 0 | _ -> false) | Xenmmap backend -> backend.work_again The Fd arm being a hardcoded `false` while Xenmmap consults work_again is the asymmetry that hid this. Tested on --------- Xen 4.23-unstable, git e58a8e0889, debug=n, bare metal (Debian 13, x86_64). oxenstored built from the same tree; running binary sha256-verified against tools/ocaml/xenstored/oxenstored. Not tested: 4.21.0 / 4.22-rc binaries were not built and run. The claim that they are affected rests on (a) the xb library being byte-identical to HEAD and (b) 5234b61eab / ba90589447 being present in those tags. Both were checked with git; neither was exercised. ---oxs-zerolen.c /* * oxs-zerolen.c - oxenstored never answers a request with len == 0. * * gcc -o oxs-zerolen oxs-zerolen.c && sudo ./oxs-zerolen * * Against oxenstored: every len==0 request times out, whatever the opcode. * Against C xenstored: every case replies immediately. */ #include <stdio.h> #include <string.h> #include <stdint.h> #include <unistd.h> #include <poll.h> #include <sys/socket.h> #include <sys/un.h> struct xsd_sockmsg { uint32_t type, req_id, tx_id, len; }; /* Send one request, wait up to 5s for a reply header. */ static void probe(const char *what, uint32_t type, const char *pl, uint32_t len) { struct sockaddr_un a = { .sun_family = AF_UNIX }; struct xsd_sockmsg m = { type, 0xC0DE, 0, len }, r; struct pollfd p; int fd = socket(AF_UNIX, SOCK_STREAM, 0); strcpy(a.sun_path, "/var/run/xenstored/socket"); if (connect(fd, (struct sockaddr *)&a, sizeof(a))) { perror("connect"); return; } if (write(fd, &m, sizeof(m)) != sizeof(m)) { perror("write"); return; } if (len && write(fd, pl, len) != (ssize_t)len) { perror("write"); return; } p.fd = fd; p.events = POLLIN; if (poll(&p, 1, 5000) == 0) printf("%-42s -> NO REPLY (5s timeout)\n", what); else if (read(fd, &r, sizeof(r)) == sizeof(r)) printf("%-42s -> reply type=%u len=%u\n", what, r.type, r.len); close(fd); } int main(void) { probe("XS_READ(2) len=21 (sanity)", 2, "/local/domain/0/name", 21); probe("XS_GET_QUOTA(25) len=2 (unknown op)", 25, "0\0", 2); probe("XS_READ(2) len=0 (known op!)", 2, NULL, 0); probe("XS_DIRECTORY(1) len=0 (known op!)", 1, NULL, 0); probe("XS_GET_FEATURE(23) len=0 (xl info -x)", 23, NULL, 0); probe("XS_GET_QUOTA(25) len=0 (xl list -l)", 25, NULL, 0); return 0; }
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |