sidecar: remove assert in sidecar node creation flow (#4044)

[babayet2]

Backport of #4044

The sidecar client initialization flow assumes that sidecar nodes are contiguous, and that the nodes' CPU range begin at CPU 0. This does not always hold; in scale tests in which we stress the IO path, most VPs need a full linux kernel, and the CPU range for the first sidecar client in nonzero.

This PR removes the assert.

[github-actions]

⚠️ Unsafe Code Detected

This PR modifies files containing unsafe Rust code. Extra scrutiny is required during review.

For more on why we check whole files, instead of just diffs, check out the Rustonomicon

[Copilot]

Pull request overview

This PR backports #4044 to relax assumptions in the sidecar client initialization so sidecar nodes can have non-zero base CPU ranges without triggering an assertion.

Changes:

• Removed the CPU base contiguity assertion during sidecar node discovery/initialization.
• Allows sidecar nodes whose base_cpu does not begin at CPU 0.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

openhcl/sidecar_client/src/lib.rs:153

• After removing the contiguity assert, expected_base is now never read. This will trigger an unused-variable warning (and may fail CI if warnings are denied). Either remove it entirely, or rename it to _expected_base if it’s intentionally kept for future use.

                }
                Err(err) => return Err(err),
            };
            if node.cpus.start > expected_base {
                tracing::info!(

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

openhcl/sidecar_client/src/lib.rs:156

• The new gap log message is misleading: this loop cannot “skip earlier node(s)” because SidecarNode::new stops iteration at the first missing /dev/mshv_vtl_sidecar{node}. Also, after removing the assert_eq!, a node with cpus.start < expected_base (overlap / out-of-order range) would be silently accepted and could cause incorrect CPU->node mapping later. Consider treating start < expected_base as an InvalidData error, and adjust the log text to describe a CPU-index gap (not skipped nodes).

            if node.cpus.start > expected_base {
                tracing::info!(
                    node = nodes.len(),
                    gap_start = expected_base,
                    gap_end = node.cpus.start,

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

openhcl/sidecar_client/src/lib.rs:161

• With the contiguous-CPU assert removed, an out-of-order/overlapping CPU layout (node.cpus.start < expected_base) would now pass silently, and the expected_base = node.cpus.end update could move backwards and hide a real layout issue. Consider explicitly warning on overlaps/out-of-order layouts and keeping expected_base monotonic. Also, the current info message says "earlier node(s) skipped" but the condition is a CPU-range gap (including before the first node), so the wording is inaccurate in the nonzero-base scenario described in the PR.

                    "sidecar node follows a gap; earlier node(s) skipped (no sidecar-started APs)"
                );
            }
            expected_base = node.cpus.end;
            nodes.push(node);

[github-actions]

2 Petri tests failed (0 unstable)