Skip to content

api: Added GET method for machine-config #242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
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
27 changes: 16 additions & 11 deletions api_server/src/http_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,17 +198,22 @@ fn parse_request<'a>(
.map_err(|s| Error::Generic(StatusCode::BadRequest, s))?),
_ => Err(Error::InvalidPathMethod(path, method)),
},
"machine-config" => match v[1..].len() {
0 if is_get => Ok(ParsedRequest::Dummy),

0 if is_put => Ok(
serde_json::from_slice::<request::MachineConfigurationBody>(body)
.map_err(Error::SerdeJson)?
.into_parsed_request()
.map_err(|s| Error::Generic(StatusCode::BadRequest, s))?,
),
_ => Err(Error::InvalidPathMethod(path, method)),
},
"machine-config" => {
match v[1..].len() {
0 if is_get => Ok(serde_json::from_slice::<request::MachineConfigurationBody>(
body,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is there a body on the GET request?

Copy link
Member Author

Choose a reason for hiding this comment

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

I deleted it, check the new PR: #248

).map_err(Error::SerdeJson)?
.into_parsed_request(Method::Get)
.map_err(|s| Error::Generic(StatusCode::BadRequest, s))?),

0 if is_put => Ok(serde_json::from_slice::<request::MachineConfigurationBody>(
body,
).map_err(Error::SerdeJson)?
.into_parsed_request(Method::Put)
.map_err(|s| Error::Generic(StatusCode::BadRequest, s))?),
_ => Err(Error::InvalidPathMethod(path, method)),
}
}
"network-interfaces" => match v[1..].len() {
0 if is_get => Ok(ParsedRequest::Dummy),

Expand Down
2 changes: 1 addition & 1 deletion api_server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ extern crate fc_util;
extern crate net_util;
extern crate sys_util;

mod http_service;
pub mod http_service;
pub mod request;

use std::cell::RefCell;
Expand Down
19 changes: 13 additions & 6 deletions api_server/src/request/sync/machine_configuration.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::result;

use futures::sync::oneshot;
use hyper::{Response, StatusCode};
use hyper::{Method, Response, StatusCode};

use http_service::{empty_response, json_fault_message, json_response};
use request::{ParsedRequest, SyncRequest};
Expand Down Expand Up @@ -56,11 +56,18 @@ impl GenerateResponse for PutMachineConfigurationOutcome {
}

impl MachineConfigurationBody {
pub fn into_parsed_request(self) -> result::Result<ParsedRequest, String> {
pub fn into_parsed_request(self, method: Method) -> result::Result<ParsedRequest, String> {
let (sender, receiver) = oneshot::channel();
Ok(ParsedRequest::Sync(
SyncRequest::PutMachineConfiguration(self, sender),
receiver,
))
match method {
Method::Get => Ok(ParsedRequest::Sync(
SyncRequest::GetMachineConfiguration(sender),
receiver,
)),
Method::Put => Ok(ParsedRequest::Sync(
SyncRequest::PutMachineConfiguration(self, sender),
receiver,
)),
_ => Ok(ParsedRequest::Dummy),
}
}
}
1 change: 1 addition & 0 deletions api_server/src/request/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ pub enum DeviceState {
// This enum contains messages for the VMM which represent sync requests. They each contain various
// bits of information (ids, paths, etc.), together with an OutcomeSender, which is always present.
pub enum SyncRequest {
GetMachineConfiguration(SyncOutcomeSender),
PutBootSource(BootSourceBody, SyncOutcomeSender),
PutDrive(DriveDescription, SyncOutcomeSender),
PutMachineConfiguration(MachineConfigurationBody, SyncOutcomeSender),
Expand Down
1 change: 1 addition & 0 deletions vmm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ version = "0.1.0"
authors = ["Amazon firecracker team <[email protected]>"]

[dependencies]
hyper = "=0.11.16"
libc = ">=0.2.39"
epoll = "=2.1.0"
scopeguard = "=0.3.3"
Expand Down
23 changes: 21 additions & 2 deletions vmm/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
extern crate epoll;
extern crate hyper;
extern crate libc;

extern crate api_server;
Expand Down Expand Up @@ -27,6 +28,7 @@ use std::sync::{Arc, Barrier, RwLock};
use std::thread;

use api_server::ApiRequest;
use api_server::http_service::json_response;
use api_server::request::async::{AsyncOutcome, AsyncRequest};
use api_server::request::instance_info::{InstanceInfo, InstanceState};
use api_server::request::sync::{DriveError, Error as SyncError, GenerateResponse,
Expand Down Expand Up @@ -283,8 +285,7 @@ pub struct KernelConfig {
pub cmdline_addr: GuestAddress,
}

// This structure should replace MachineCfg; For now it is safer to duplicate the work as the
// net support is not fuully integrated.
#[derive(Clone)]
pub struct VirtualMachineConfig {
vcpu_count: u8,
mem_size_mib: usize,
Expand All @@ -299,6 +300,18 @@ impl Default for VirtualMachineConfig {
}
}

impl GenerateResponse for VirtualMachineConfig {
Copy link
Contributor

Choose a reason for hiding this comment

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

I vote for having this block of code in "api_server/" rather than "vmm/". It's common to implement a trait for an object in a different place/crate than the object definition.
I believe vmm is already linked in api_server.
This also helps us avoid hyper dependencies in vmm while also keeping HTTP/API-related code contained to api_server crate.

Copy link
Member Author

Choose a reason for hiding this comment

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

I created a shared crate so this code is not in the VMM any more: #248

fn generate_response(&self) -> hyper::Response {
json_response(
hyper::StatusCode::Ok,
format!(
"{{ \"vcpu_count\": {:?}, \"mem_size_mib\": {:?} }}",
self.vcpu_count, self.mem_size_mib
),
)
}
}

pub struct Vmm {
_kvm_fd: Kvm,

Expand Down Expand Up @@ -911,6 +924,12 @@ impl Vmm {
}
ApiRequest::Sync(req) => {
match req {
SyncRequest::GetMachineConfiguration(sender) => {
sender
.send(Box::new(self.vm_config.clone()))
.map_err(|_| ())
.expect("one-shot channel closed");
}
SyncRequest::PutDrive(drive_description, sender) => {
match self.put_block_device(BlockDeviceConfig::from(drive_description)) {
Ok(_) =>
Expand Down