Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ mac_address = "1.1.5"
sysinfo = "0.31.4"
log = "0.4.0"
env_logger = "0.11.8"
regex = "1.11.1"
5 changes: 2 additions & 3 deletions docs/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@ Set the log level for libkrun. Supported values: 0=off, 1=error, 2=warn, 3=info

- `--restful-uri`

The URI (address) of the RESTful service. If not specified, defaults to `tcp://localhost:8081`. `tcp` is the only
valid scheme.

The URI (address) of the RESTful service. If not specified, defaults to `tcp://localhost:8081`. Valid schemes are
`tcp`, `none`, or `unix`. A scheme of `none` disables the RESTful service.
### Virtual Machine Resources

- `--cpus`
Expand Down
10 changes: 6 additions & 4 deletions src/cmdline.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: Apache-2.0

use crate::{status::RestfulUriAddr, virtio::VirtioDeviceConfig};
use crate::{status::RestfulUri, virtio::VirtioDeviceConfig};

use std::{collections::HashMap, path::PathBuf, str::FromStr};

Expand Down Expand Up @@ -29,7 +29,7 @@ pub struct Args {

/// URI of the status/shutdown listener.
#[arg(long = "restful-uri")]
pub restful_uri: Option<RestfulUriAddr>,
pub restful_uri: Option<RestfulUri>,

/// GUI option for compatibility with vfkit (ignored).
#[arg(long, default_value_t = false)]
Expand Down Expand Up @@ -514,8 +514,10 @@ mod tests {

let restful_uri = args.restful_uri.expect("restful-uri argument not found");

assert_eq!(restful_uri.ip_addr, Ipv4Addr::new(127, 0, 0, 1));
assert_eq!(restful_uri.port, 49573);
assert_eq!(
restful_uri,
RestfulUri::Tcp(Ipv4Addr::new(127, 0, 0, 1), 49573)
);

assert_eq!(args.gui, true);
assert_eq!(args.krun_log_level, 5);
Expand Down
6 changes: 4 additions & 2 deletions src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use super::*;

use crate::{
status::{get_shutdown_eventfd, status_listener},
status::{get_shutdown_eventfd, status_listener, RestfulUri},
virtio::KrunContextSet,
};

Expand Down Expand Up @@ -125,7 +125,9 @@ impl KrunContext {
let shutdown_eventfd = unsafe { get_shutdown_eventfd(self.id) };
let uri = self.args.restful_uri.clone();

thread::spawn(move || status_listener(shutdown_eventfd, uri).unwrap());
if uri != Some(RestfulUri::None) {
thread::spawn(move || status_listener(shutdown_eventfd, uri).unwrap());
Comment thread
jakecorrenti marked this conversation as resolved.
}

// Run the workload.
if unsafe { krun_start_enter(self.id) } < 0 {
Expand Down
259 changes: 203 additions & 56 deletions src/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@

use std::{
fs::File,
io::{Read, Write},
io::{ErrorKind, Read, Write},
net::{Ipv4Addr, TcpListener},
os::fd::{FromRawFd, RawFd},
os::{
fd::{FromRawFd, RawFd},
unix::net::UnixListener,
},
str::FromStr,
};

use anyhow::{anyhow, Context};
use clap::Parser;

#[link(name = "krun-efi")]
extern "C" {
Expand All @@ -22,51 +24,84 @@ const HTTP_RUNNING: &str =
const HTTP_STOPPING: &str =
"HTTP/1.1 200 OK\r\nContent-type: application/json\r\n\r\n{\"state\": \"VirtualMachineStateStopping\"}\0";

/// Socket address in which the restful URI socket should listen on. Identical to Rust's
/// SocketAddrV4, but requires a modified FromStr implementation due to how the address is
/// presented on the command line.
#[derive(Clone, Debug, Parser)]
pub struct RestfulUriAddr {
pub ip_addr: Ipv4Addr,
pub port: u16,
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
pub enum UriScheme {
#[default]
Tcp,
Unix,
None,
}

impl FromStr for RestfulUriAddr {
impl FromStr for UriScheme {
type Err = anyhow::Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let mut string = String::from(s);

if let Some(removed) = string.strip_prefix("tcp://") {
string = String::from(removed);
match s {
"tcp" => Ok(Self::Tcp),
"unix" => Ok(Self::Unix),
"none" => Ok(Self::None),
_ => Err(anyhow!("invalid scheme")),
}
Comment thread
jakecorrenti marked this conversation as resolved.
}
}

let mut parts: Vec<String> = string.split(':').map(|s| s.to_string()).collect();
if parts.len() != 2 {
return Err(anyhow!("restful URI formatted incorrectly"));
}
/// Socket address in which the restful URI socket should listen on. Identical to Rust's
/// SocketAddrV4, but requires a modified FromStr implementation due to how the address is
/// presented on the command line.
#[derive(Clone, Debug, PartialEq)]
pub enum RestfulUri {
Tcp(Ipv4Addr, u16),
Unix(String),
None,
}

// Ipv4Address's FromStr does not understand that the "localhost" IP address translates to
// 127.0.0.1, this must be manually translated.
if &parts[0][..] == "localhost" {
parts[0] = String::from("127.0.0.1");
impl FromStr for RestfulUri {
type Err = anyhow::Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let expression = regex::Regex::new(r"^(?P<scheme>none|tcp|unix)://(?P<value>.*)").unwrap();
let Some(cap) = expression.captures(s) else {
return Err(anyhow!("invalid scheme input"));
};
let scheme = &cap["scheme"];
let value = &cap["value"];
match UriScheme::from_str(scheme)? {
UriScheme::Tcp => {
let (ip_addr, port) = parse_tcp_input(value)?;
Ok(Self::Tcp(ip_addr, port))
}
Comment thread
jakecorrenti marked this conversation as resolved.
UriScheme::Unix => {
if value.is_empty() {
return Err(anyhow!("empty unix socket path"));
}
Comment thread
jakecorrenti marked this conversation as resolved.
Ok(Self::Unix(value.to_string()))
}
UriScheme::None => Ok(Self::None),
Comment thread
jakecorrenti marked this conversation as resolved.
}
}
}

let ip_addr = Ipv4Addr::from_str(&parts[0])
.context("restful URI IP address formatted incorrectly")?;
let port =
u16::from_str(&parts[1]).context("restful URI port number formatted incorrectly")?;
fn parse_tcp_input(input: &str) -> Result<(Ipv4Addr, u16), anyhow::Error> {
let mut parts: Vec<String> = input.split(':').map(|s| s.to_string()).collect();
if parts.len() != 2 {
return Err(anyhow!("restful URI formatted incorrectly"));
}

Ok(Self { ip_addr, port })
// Ipv4Address's FromStr does not understand that the "localhost" IP address translates to
// 127.0.0.1, this must be manually translated.
if &parts[0][..] == "localhost" {
parts[0] = String::from("127.0.0.1");
}

let ip_addr =
Ipv4Addr::from_str(&parts[0]).context("restful URI IP address formatted incorrectly")?;
let port = u16::from_str(&parts[1]).context("restful URI port number formatted incorrectly")?;
Ok((ip_addr, port))
}

impl Default for RestfulUriAddr {
impl Default for RestfulUri {
fn default() -> Self {
Self {
ip_addr: Ipv4Addr::new(127, 0, 0, 1),
port: 8081,
}
Self::Tcp(Ipv4Addr::new(127, 0, 0, 1), 8081)
}
}

Expand All @@ -79,42 +114,154 @@ pub unsafe fn get_shutdown_eventfd(ctx_id: u32) -> i32 {
fd
}

fn handle_incoming_stream<T: Read + Write>(stream: &mut T, shutdown_fd: &mut File) {
let mut buf = [0u8; 4096];
match stream.read(&mut buf) {
Ok(_sz) => {
let request = String::from_utf8_lossy(&buf);
if request.contains("POST") {
// Send a VirtualMachineStateStopping message to the client.
if let Err(e) = stream.write_all(HTTP_STOPPING.as_bytes()) {
println!("Error writing POST response: {e}");
}

// Shut down the VM.
if let Err(e) = shutdown_fd.write_all(&1u64.to_le_bytes()) {
println!("Error writing to shutdown fd: {e}");
}
} else if let Err(e) = stream.write_all(HTTP_RUNNING.as_bytes()) {
println!("Error writing GET response: {e}");
}
}
Err(e) => println!("Error reading stream: {}", e),
}
}

/// Listen for status and shutdown requests from the client. Shut down the krun VM when prompted.
pub fn status_listener(
shutdown_eventfd: RawFd,
addr: Option<RestfulUriAddr>,
addr: Option<RestfulUri>,
) -> Result<(), anyhow::Error> {
// VM is shut down by writing to the shutdown event file.
let mut shutdown = unsafe { File::from_raw_fd(shutdown_eventfd) };

let addr = addr.unwrap_or_default();

let listener = TcpListener::bind((addr.ip_addr, addr.port)).unwrap();

for stream in listener.incoming() {
let mut buf = [0u8; 4096];
let mut stream = stream.unwrap();

match stream.read(&mut buf) {
Ok(_sz) => {
let request = String::from_utf8_lossy(&buf);
if request.contains("POST") {
// Send a VirtualMachineStateStopping message to the client.
if let Err(e) = stream.write_all(HTTP_STOPPING.as_bytes()) {
println!("Error writting POST response: {e}");
}

// Shut down the VM.
if let Err(e) = shutdown.write_all(&1u64.to_le_bytes()) {
println!("Error writting to shutdown fd: {e}");
}
} else if let Err(e) = stream.write_all(HTTP_RUNNING.as_bytes()) {
println!("Error writting GET response: {e}");
match addr {
RestfulUri::Tcp(addr, port) => {
let listener = TcpListener::bind((addr, port))
.map_err(|e| anyhow!("Unable to bind to TCP listener: {}", e))?;

for stream in listener.incoming() {
handle_incoming_stream(&mut stream.unwrap(), &mut shutdown)
}
}
RestfulUri::Unix(path) => {
Comment thread
jakecorrenti marked this conversation as resolved.
if let Err(e) = std::fs::remove_file(&path) {
if e.kind() != ErrorKind::NotFound {
return Err(anyhow!("failed to remove socket with error {e}"));
}
}
Err(e) => println!("Error reading stream: {}", e),
let listener = UnixListener::bind(path)
.map_err(|e| anyhow!("Unable to bind to unix socket: {}", e))?;

for stream in listener.incoming() {
handle_incoming_stream(&mut stream.unwrap(), &mut shutdown)
}
}
RestfulUri::None => unreachable!(),
}

Ok(())
}

#[allow(unused_imports)]
mod tests {
use super::*;

#[test]
fn parse_valid_unix_scheme() {
assert_eq!(
RestfulUri::Unix("/tmp/path".to_string()),
RestfulUri::from_str("unix:///tmp/path").unwrap()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this errors, the test panics and we don't get any info on the failure.

I tested this change, simulating a bug in the code:

diff --git a/src/status.rs b/src/status.rs
index c0e889e..b8520b0 100644
--- a/src/status.rs
+++ b/src/status.rs
@@ -179,7 +179,7 @@ mod tests {
         // unix valid
         assert_eq!(
             RestfulUri::Unix("/tmp/path".to_string()),
-            RestfulUri::from_str("unix:///tmp/path").unwrap()
+            RestfulUri::from_str("uni:///tmp/path").unwrap()
         );
 
         // unix missing path
% cargo test
...
test status::tests::uri_input_parsing ... FAILED

failures:

---- status::tests::uri_input_parsing stdout ----
thread 'status::tests::uri_input_parsing' panicked at src/status.rs:182:53:
called `Result::unwrap()` on an `Err` value: invalid scheme input
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    status::tests::uri_input_parsing

test result: FAILED. 9 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

error: test failed, to rerun pass `--bin krunkit`

The test cannot assume that the code does not return an error, it must validate this.

@jakecorrenti jakecorrenti Jun 10, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I fully understand. Are you suggesting not using .unwrap in the test when trying to assert a successful parse?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if we fail to parse we want to report an error. If we parsed and got the wrong results we want to report the wrong result. If in both case we stop running the current test this will not make much difference.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thinks is fine now since we have single assert per test, and the assert reports the failure with the error message or the mismatched value.

);
}

#[test]
fn parse_unix_scheme_missing_path() {
assert_eq!(
anyhow!("empty unix socket path").to_string(),
RestfulUri::from_str("unix://").err().unwrap().to_string()
);
}

#[test]
fn parse_unix_scheme_missing_slashes() {
assert_eq!(
anyhow!("invalid scheme input").to_string(),
RestfulUri::from_str("unix:").err().unwrap().to_string()
);
}

#[test]
fn parse_unix_scheme_misspelling() {
assert_eq!(
anyhow!("invalid scheme input").to_string(),
RestfulUri::from_str("uni://path")
.err()
.unwrap()
.to_string()
);
}

#[test]
fn parse_valid_tcp_scheme() {
assert_eq!(
RestfulUri::Tcp(Ipv4Addr::new(127, 0, 0, 1), 8080),
RestfulUri::from_str("tcp://localhost:8080").unwrap(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same - error will break the test instead of failing a single test.

);
}

#[test]
fn parse_tcp_scheme_missing_port() {
assert_eq!(
anyhow!("restful URI formatted incorrectly").to_string(),
RestfulUri::from_str("tcp://localhost")
.err()
.unwrap()
.to_string()
);
}

#[test]
fn parse_tcp_scheme_with_unix_path() {
assert_eq!(
anyhow!("restful URI formatted incorrectly").to_string(),
RestfulUri::from_str("tcp:///tmp/path")
.err()
.unwrap()
.to_string(),
);
}

#[test]
fn parse_valid_none_scheme() {
assert_eq!(RestfulUri::None, RestfulUri::from_str("none://").unwrap());
}

#[test]
fn parse_none_scheme_missing_postfix() {
assert_eq!(
anyhow!("invalid scheme input").to_string(),
RestfulUri::from_str("none").err().unwrap().to_string(),
);
}

#[test]
fn parse_random_string_scheme() {
assert_eq!(
anyhow!("invalid scheme input").to_string(),
RestfulUri::from_str("foobar").err().unwrap().to_string(),
);
}
}
Loading