Repository navigation
openvmm: bound HTTP blob requests and retry transient failures - #938
Pedro Henrique Penna (ppenna) merged 2 commits into
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
The pinned asynchronous HTTP disk changes need final human review, with exact-head cross-platform VMM validation still reported as pending.
No open issues 🎉
What changed in this PR
This PR advances the OpenVMM pin to address #170’s intermittent blob-backed disk stalls through bounded HTTP requests, transient-failure retries, and request diagnostics. The reviewed documentation records the new revision and its upstream integration plan.
Changes:
- Updates the documented NVX pin to
8f1c28253, at chain position 350. - Records the three HTTP blob commits and their eligibility for independent upstream submission.
- Updates the branch diagram and records the merge of positions 346–347.
| File | Description |
|---|---|
doc/openvmm-upstream-roadmap.md |
Updates the pin, commit mapping, and upstream status for the HTTP blob fixes. |
Review effort: Balanced
Pin microsoft/openvmm#4773 at b92daa350c93914b2eb556acf47886cc313d396b, three commits on d2338ea22, the previous pin, which is the head of feature/microvm/dev. HttpBlob, which streams the VHDs of OpenVMM's VMM tests from Azure Blob Storage, sent each range request with no connect timeout, no deadline for the response or its body, no TCP keepalive, and no retry. When the network silently stopped delivering a connection's packets, a guest's disk read never completed, and the Linux/KVM VMM tests stopped, in GRUB or after boot, until petri's ten-minute watchdog failed them. NVX's microVMs do not use blob disks; the VMM tests that NVX's CI runs do. The first commit bounds every request in time: a 10 s connect timeout, a 20 s response-header timeout, a 20 s body inactivity timeout, which never cuts off a slow body that is still arriving, and a 120 s operation deadline, with TCP keepalive. A request that runs out of time fails with a TimedOut error, which the guest sees as a failed read. It also refuses a read whose last byte would lie past u64::MAX, which no guest read reaches. The second retries timeouts, lost connections, and statuses 408, 429, 500, 502, 503, and 504, for up to four attempts with a backoff from 250 ms, each retry on a new connection, and fails permanent errors at once. A transient stall now costs about 20 s instead of the test, and a persistent one fails the read after about 82 s. The third shows each request in progress, with counts of its attempts, retries, timeouts, and failures, in the blob's inspect node, so that a watchdog dump shows whether a stuck read is waiting on HTTP. Refs #170 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Record the three commits that the previous commit pins as positions 350-352 of the microVM chain, and move the NVX pin to b92daa350. Positions 350-352 bound HttpBlob's requests in time, retry the ones that fail transiently on a new connection, and show them in inspect. They change only disk_blob, which upstream main and the fork base have alike and no other commit of the chain changes, so they can go upstream on their own; say so in the table's remarks. The pin is now 195 commits past the mapped tip. Positions 348 and 349 are no longer only a proposal: microsoft/openvmm#4769 merged them, so position 349, d2338ea22, is the head of feature/microvm/dev. Say so in the scope, and name microsoft/openvmm#4773 as the proposal of positions 350-352. Refs #170 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
3c8bf55 to
5383998
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The OpenVMM pin imports substantial HTTP timeout, retry, and cancellation changes with no recorded source reviews and pending exact-head KVM/MSHV/WHP validation, requiring human review and platform results before promotion.
No open issues 🎉
Review effort: Balanced
Refs #170. Pins microsoft/openvmm#4773 at
b92daa350c93914b2eb556acf47886cc313d396b, which bounds every request of OpenVMM's HTTP blob disk in time, retries the ones that fail transiently on a new connection, and shows them in inspect, so that a network path that silently stops delivering a connection's packets no longer hangs a guest's disk read until petri's ten-minute watchdog fails the test. That hang is the root cause of #170's intermittent Linux/KVM OpenVMM VMM-test stalls. #170 stays open until the Linux/KVM lane has run clean with this pin.Based on
devatb855c4d55. microsoft/openvmm#4773 is three commits directly ond2338ea22, the head offeature/microvm/devand the revision thatdevpins, and the pin is that pull request's head commit itself, so this branch's two commits are its whole diff.Change
351d2d45fopenvmmopenvmmd2338ea22→b92daa350: the three commits of microsoft/openvmm#4773538399841docdoc/openvmm-upstream-roadmap.mddisk_blob, which upstream main and the fork base have alike, so they can go upstream on their own; moves the NVX pin; and records that microsoft/openvmm#4769 merged positions 348 and 349 intofeature/microvm/devNo other NVX change is needed: OpenVMM's control-session protocol vectors and built-in CPU profiles are unchanged at the pin.
OpenVMM changes
8c1772d83). A 10 s connect timeout, a 20 s response-header timeout, a 20 s body inactivity timeout, which never cuts off a slow body that is still arriving, and a 120 s operation deadline, with TCP keepalive after 30 s idle. A request that runs out of time fails withTimedOut, which the guest sees as a failed read rather than one that never completes.HttpBlob::new's HEAD probe is bounded the same way, and a read whose last byte would lie pastu64::MAXis refused without a request.b47786f2a). Timeouts, lost connections, and statuses 408, 429, 500, 502, 503, and 504 are retried for up to four attempts, after a backoff from 250 ms that doubles up to 4 s, each retry on a new connection; other failures are returned at once. A transient stall now costs about 20 s instead of the test, and a persistent one fails the read after about 82 s. Each retry is a rate-limited warning.b92daa350). The blob's inspect node lists each request in progress, with its range, attempt, state, and age, and counts attempts, retries, timeouts, and failures, so that the next watchdog dump shows whether a stuck read is waiting on HTTP.NVX's microVMs do not use blob disks, so NVX's runtime is unaffected. The change matters to the OpenVMM VMM tests that NVX's CI runs on KVM, MSHV, and WHP, whose UEFI guests boot from VHDs that petri streams from
hvlitetestvhds.blob.core.windows.net.Evidence
azure-kvm-2,azure-kvm-4, andazure-kvm-5) timed out seven UEFI tests with a guestREADin flight on the blob-backed LUN0:0:0; in 39490938890, two VMs stopped 4 ms apart on the same blob, and 39492370835 also loggedConnection reset by peer (os error 104)from the blob client.HttpBlobfrom a loopback server that goes silent, as a dead network path does, or resets the connection. WithHttpBlobas it was, the stall, reset, 503, and inspect tests fail, the same 13 in each of 5 runs, the stalls on the tests' 10 s limit; with the change, the suite passes repeatedly on Linux and Windows.HttpBlobas it was, one stalled range request froze the guest in GRUB's initramfs load, or after* Cleaning /tmp directory ... [ ok ], for over 10 minutes with aREADin flight, the signature of the CI dumps; with the change, the same faults were retried about 20 s later and the guest booted, resets were retried within a second and a half with no error reaching the guest, and normal reads took no longer. These runs used a revision of [DO NOT REVIEW] disk_blob: bound HTTP blob requests and retry transient failures openvmm#4773 that lacked only its range check, the classification of connect timeouts, and the cap on the first backoff, which none of these paths reach.Validation
cargo clippy --all-targets,cargo doc --no-deps, andcargo nextest run --profile agentfordisk_blob(13, 22, and 23 passed), andcargo xtask fmt --fix, all clean.351d2d45f:python scripts\nvx.py verify(consistent); theopenvmm_vectorsandopenvmm_cpu_profilescomparisons with--ignored(3 and 1 passed), against the pinned revision; and, at538399841,python scripts\check_markdown_links.py.538399841ee4bed99d6ae55bd75d793b1d5b8d07.b855c4d55after openvmm: pin amd.genoa.v2 for Genoa hosts without the TSA immunities #935 moveddev's pin tod2338ea22: [DO NOT REVIEW] disk_blob: bound HTTP blob requests and retry transient failures openvmm#4773 was rebased onto that revision without conflicts (git range-diffshows its three patches unchanged), and this branch's two commits were made again ondev. The previous head,3c8bf5544, passed the full CI matrix with the same OpenVMM patches on57d0a18b3.