Add scp subcommand to ephemeral and libvirt#277
Open
cgwalters wants to merge 1 commit into
Open
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces SCP file transfer support for both ephemeral and libvirt domains via a new scp subcommand, complete with CLI options, execution logic, and integration tests. The reviewer provided constructive feedback recommending that the libvirt SCP command enforce that exactly one path uses the DOMAIN: prefix (matching the ephemeral SCP validation) and that the SCP command execution use .status() instead of .output() to allow real-time progress reporting during file transfers.
cgwalters
added a commit
to cgwalters/bcvk-fork
that referenced
this pull request
May 28, 2026
- Use domain: (lowercase) prefix consistently in doc strings, error messages, and examples instead of DOMAIN:, matching conventional CLI style. - Fix validation to reject both "neither is remote" and "both are remote" by using source_is_remote == dest_is_remote; previously only the first case was caught. - Replace .output() with .status() for the final scp invocation so stdout/stderr stream to the terminal in real time rather than being buffered and printed after the fact. - Extract the SSH readiness retry loop into a shared free function wait_for_ssh_ready() in ssh.rs, removing the near-identical duplicate in scp.rs. Assisted-by: OpenCode (Claude Sonnet 4.6)
cgwalters
force-pushed
the
libvirt-scp
branch
3 times, most recently
from
d641d35 to
396d0fa
Compare
Since we offer `ssh`, we should absolutely offer `scp`. I had an agent try to hack this by piping to ssh and try to base64 encode things. Motivated by simple use cases like "copy out a log file" or "copy in a new bootc binary" (though that one is better with a sysext flow, but that's a whole other concern). The libvirt implementation validates that exactly one of source or destination uses the `domain:` prefix, uses `.status()` so scp progress streams to the terminal in real time, and shares the SSH readiness retry loop with the `ssh` subcommand via a `wait_for_ssh_ready()` helper. Assisted-by: OpenCode (Claude Sonnet 4.6) Signed-off-by: Colin Walters <walters@verbum.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since we offer
ssh, we should absolutely offerscp. I had an agent try to hack this by piping to ssh and try to base64 encode things.Motivated by simple use cases like "copy out a log file" or "copy in a new bootc binary" (though that one is better with a sysext flow, but that's a whole other concern).
Assisted-by: pi (Sonnet 4.6 + Gemini 3.5 Flash)