From 35d199896583ae63ec6eefa4d21ba9c208d93d54 Mon Sep 17 00:00:00 2001 From: Ravi kumar Veeramally Date: Mon, 22 Jan 2024 23:09:40 +0200 Subject: [PATCH] vmm: Replace Debug with Display rendering in HTTP error message Bumping anyhow crate from 1.0.75 to 1.0.79 will cause seccomp failures through integration tests. Newly added backtrace support relies on readlink and many other syscalls. Issue noticed with test_api_http_pause_resume test, where second time of VM PAUSE or VM RESUME prints error and causes panic. Noticed that panic message in a thread which is not allowed to write output triggered the issue. So implementing Display trait for HttpError and ApiError enums to avoid adding many syscalls to seccomp filter section. Signed-off-by: Ravi kumar Veeramally (cherry picked from commit 895dc12a74c52265b763865bf9ecbbc39ad73c87) --- vmm/src/api/dbus/mod.rs | 4 ++-- vmm/src/api/http/mod.rs | 17 +++++++++++++++- vmm/src/api/mod.rs | 44 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 3 deletions(-) diff --git a/vmm/src/api/dbus/mod.rs b/vmm/src/api/dbus/mod.rs index c063e1008..826689129 100644 --- a/vmm/src/api/dbus/mod.rs +++ b/vmm/src/api/dbus/mod.rs @@ -33,8 +33,8 @@ pub struct DBusApi { api_sender: futures::lock::Mutex>, } -fn api_error(error: impl std::fmt::Debug) -> fdo::Error { - fdo::Error::Failed(format!("{error:?}")) +fn api_error(error: impl std::fmt::Debug + std::fmt::Display) -> fdo::Error { + fdo::Error::Failed(format!("{error}")) } // This method is intended to ensure that the DBusApi thread has enough time to diff --git a/vmm/src/api/http/mod.rs b/vmm/src/api/http/mod.rs index 2f4b0283c..66cf26dd1 100644 --- a/vmm/src/api/http/mod.rs +++ b/vmm/src/api/http/mod.rs @@ -7,12 +7,14 @@ use self::http_endpoint::{VmActionHandler, VmCreate, VmInfo, VmmPing, VmmShutdow use crate::api::{ApiError, ApiRequest, VmAction}; use crate::seccomp_filters::{get_seccomp_filter, Thread}; use crate::{Error as VmmError, Result}; +use core::fmt; use hypervisor::HypervisorType; use micro_http::{Body, HttpServer, MediaType, Method, Request, Response, StatusCode, Version}; use once_cell::sync::Lazy; use seccompiler::{apply_filter, SeccompAction}; use serde_json::Error as SerdeError; use std::collections::BTreeMap; +use std::fmt::Display; use std::fs::File; use std::os::unix::io::{IntoRawFd, RawFd}; use std::os::unix::net::UnixListener; @@ -44,6 +46,19 @@ pub enum HttpError { ApiError(ApiError), } +impl Display for HttpError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use self::HttpError::*; + match self { + BadRequest => write!(f, "Bad Request"), + NotFound => write!(f, "Not Found"), + InternalServerError => write!(f, "Internal Server Error"), + SerdeJsonDeserialize(serde_error) => write!(f, "{}", serde_error), + ApiError(api_error) => write!(f, "{}", api_error), + } + } +} + impl From for HttpError { fn from(e: serde_json::Error) -> Self { HttpError::SerdeJsonDeserialize(e) @@ -54,7 +69,7 @@ const HTTP_ROOT: &str = "/api/v1"; pub fn error_response(error: HttpError, status: StatusCode) -> Response { let mut response = Response::new(Version::Http11, status); - response.set_body(Body::new(format!("{error:?}"))); + response.set_body(Body::new(format!("{error}"))); response } diff --git a/vmm/src/api/mod.rs b/vmm/src/api/mod.rs index aaae8ee34..a0ad2f320 100644 --- a/vmm/src/api/mod.rs +++ b/vmm/src/api/mod.rs @@ -43,8 +43,10 @@ use crate::config::{ }; use crate::device_tree::DeviceTree; use crate::vm::{Error as VmError, VmState}; +use core::fmt; use micro_http::Body; use serde::{Deserialize, Serialize}; +use std::fmt::Display; use std::io; use std::sync::mpsc::{channel, RecvError, SendError, Sender}; use std::sync::{Arc, Mutex}; @@ -158,6 +160,48 @@ pub enum ApiError { } pub type ApiResult = std::result::Result; +impl Display for ApiError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use self::ApiError::*; + match self { + EventFdWrite(serde_error) => write!(f, "{}", serde_error), + RequestSend(send_error) => write!(f, "{}", send_error), + ResponsePayloadType => write!(f, "Wrong response payload type"), + ResponseRecv(recv_error) => write!(f, "{}", recv_error), + VmBoot(vm_error) => write!(f, "{}", vm_error), + VmCreate(vm_error) => write!(f, "{}", vm_error), + VmDelete(vm_error) => write!(f, "{}", vm_error), + VmInfo(vm_error) => write!(f, "{}", vm_error), + VmPause(vm_error) => write!(f, "{}", vm_error), + VmResume(vm_error) => write!(f, "{}", vm_error), + VmNotBooted => write!(f, "VM is not booted"), + VmNotCreated => write!(f, "VM is not created"), + VmShutdown(vm_error) => write!(f, "{}", vm_error), + VmReboot(vm_error) => write!(f, "{}", vm_error), + VmSnapshot(vm_error) => write!(f, "{}", vm_error), + VmRestore(vm_error) => write!(f, "{}", vm_error), + VmCoredump(vm_error) => write!(f, "{}", vm_error), + VmmShutdown(vm_error) => write!(f, "{}", vm_error), + VmResize(vm_error) => write!(f, "{}", vm_error), + VmResizeZone(vm_error) => write!(f, "{}", vm_error), + VmAddDevice(vm_error) => write!(f, "{}", vm_error), + VmAddUserDevice(vm_error) => write!(f, "{}", vm_error), + VmRemoveDevice(vm_error) => write!(f, "{}", vm_error), + CreateSeccompFilter(seccomp_error) => write!(f, "{}", seccomp_error), + ApplySeccompFilter(seccomp_error) => write!(f, "{}", seccomp_error), + VmAddDisk(vm_error) => write!(f, "{}", vm_error), + VmAddFs(vm_error) => write!(f, "{}", vm_error), + VmAddPmem(vm_error) => write!(f, "{}", vm_error), + VmAddNet(vm_error) => write!(f, "{}", vm_error), + VmAddVdpa(vm_error) => write!(f, "{}", vm_error), + VmAddVsock(vm_error) => write!(f, "{}", vm_error), + VmReceiveMigration(migratable_error) => write!(f, "{}", migratable_error), + VmSendMigration(migratable_error) => write!(f, "{}", migratable_error), + VmPowerButton(vm_error) => write!(f, "{}", vm_error), + } + } +} + #[derive(Clone, Deserialize, Serialize)] pub struct VmInfo { pub config: Arc>,