From f91a9f673a664d95399dc9cd9cb00e58f44ac609 Mon Sep 17 00:00:00 2001 From: Dennis Kempin Date: Tue, 11 Oct 2022 21:58:08 +0000 Subject: [PATCH] Remove plugin-render-server feature This feature was used for experiments and is no longer needed. BUG=b:253086623 TEST=presubmit Change-Id: Id8ece0e1e8588a5716794961c359482bc60dc729 Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3946029 Reviewed-by: Daniel Verkamp Commit-Queue: Dennis Kempin --- Cargo.toml | 3 -- crosvm_plugin/crosvm.h | 5 ---- crosvm_plugin/src/lib.rs | 13 -------- src/crosvm/plugin/mod.rs | 48 +----------------------------- src/crosvm/plugin/process.rs | 57 +++++++++++------------------------- src/crosvm/sys.rs | 4 --- tools/health-check | 1 - 7 files changed, 18 insertions(+), 113 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 198b9e203c..7f99a706f9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -246,9 +246,6 @@ libvda = ["devices/libvda"] ## This feature is used to integrate Parallels with crosvm and is not functional upstream. plugin = ["protos/plugin", "crosvm_plugin", "kvm", "kvm_sys", "protobuf"] -## Defaults to use virgl_renderer render server when running in plugin mode. -plugin-render-server = [] - ## Enables battery reporting via the ChromeOS powerd. Requires access to the ## org.chromium.PowerManager dbus service. power-monitor-powerd = ["arch/power-monitor-powerd"] diff --git a/crosvm_plugin/crosvm.h b/crosvm_plugin/crosvm.h index 3382e5a607..ca51283f1c 100644 --- a/crosvm_plugin/crosvm.h +++ b/crosvm_plugin/crosvm.h @@ -174,11 +174,6 @@ static_assert(sizeof(struct crosvm_net_config) == 20, "extra padding in struct crosvm_net_config"); #endif -/* - * Gets fd for the gpu server. - */ -int crosvm_get_render_server_fd(void); - /* * Gets the network configuration. */ diff --git a/crosvm_plugin/src/lib.rs b/crosvm_plugin/src/lib.rs index eddf665289..7e31a47273 100644 --- a/crosvm_plugin/src/lib.rs +++ b/crosvm_plugin/src/lib.rs @@ -1388,19 +1388,6 @@ fn to_crosvm_rc(r: result::Result) -> c_int { } } -#[no_mangle] -pub unsafe extern "C" fn crosvm_get_render_server_fd() -> c_int { - let fd = match env::var(CROSVM_GPU_SERVER_FD_ENV) { - Ok(v) => v, - _ => return -EINVAL, - }; - - match fd.parse() { - Ok(v) if v >= 0 => v, - _ => -EINVAL, - } -} - #[no_mangle] pub unsafe extern "C" fn crosvm_connect(out: *mut *mut crosvm) -> c_int { let _u = record(Stat::Connect); diff --git a/src/crosvm/plugin/mod.rs b/src/crosvm/plugin/mod.rs index 7cf7b1b191..2db4f435ba 100644 --- a/src/crosvm/plugin/mod.rs +++ b/src/crosvm/plugin/mod.rs @@ -37,7 +37,6 @@ use base::register_rt_signal_handler; use base::validate_raw_descriptor; use base::warn; use base::AsRawDescriptor; -use base::Descriptor; use base::Error as SysError; use base::Event; use base::EventToken; @@ -97,8 +96,6 @@ use crate::Config; const MAX_DATAGRAM_SIZE: usize = 4096; const MAX_VCPU_DATAGRAM_SIZE: usize = 0x40000; -#[cfg(feature = "virgl_renderer_next")] -const CROSVM_GPU_SERVER_FD_ENV: &str = "CROSVM_GPU_SERVER_FD"; /// An error that occurs when communicating with the plugin process. #[sorted] @@ -566,42 +563,6 @@ pub fn run_config(cfg: Config) -> Result<()> { add_fd_flags(stderr_rd.as_raw_descriptor(), O_NONBLOCK) .context("error marking stderr nonblocking")?; - #[allow(unused_mut)] - let mut env_fds: Vec<(String, Descriptor)> = Vec::default(); - - #[cfg(all(feature = "gpu", feature = "virgl_renderer_next"))] - // Hold on to the render server jail so it keeps running until we exit run_config() - let (_render_server_jail, _render_server_fd) = { - let _default_render_server_params = crate::crosvm::sys::GpuRenderServerParameters { - path: std::path::PathBuf::from("/usr/libexec/virgl_render_server"), - cache_path: None, - cache_size: None, - }; - - let gpu_render_server_parameters = - if let Some(parameters) = &cfg.gpu_render_server_parameters { - Some(parameters) - } else if cfg!(feature = "plugin-render-server") { - Some(&_default_render_server_params) - } else { - None - }; - - if let Some(parameters) = gpu_render_server_parameters { - let (jail, fd) = crate::crosvm::sys::start_gpu_render_server(&cfg, parameters)?; - env_fds.push(( - CROSVM_GPU_SERVER_FD_ENV.to_string(), - Descriptor(fd.as_raw_descriptor()), - )); - ( - Some(crate::crosvm::sys::jail_helpers::ScopedMinijail(jail)), - Some(fd), - ) - } else { - (None, None) - } - }; - let jail = if let Some(jail_config) = &cfg.jail_config { // An empty directory for jailed plugin pivot root. let root_path = match &cfg.plugin_root { @@ -718,14 +679,7 @@ pub fn run_config(cfg: Config) -> Result<()> { .context("failed to create kvm irqchip")?; vm.create_pit().context("failed to create kvm PIT")?; - let mut plugin = Process::new( - vcpu_count, - plugin_path, - &plugin_args, - jail, - stderr_wr, - env_fds, - )?; + let mut plugin = Process::new(vcpu_count, plugin_path, &plugin_args, jail, stderr_wr)?; // Now that the jail for the plugin has been created and we had a chance to adjust gids there, // we can drop all our capabilities in case we had any. drop_capabilities().context("failed to drop process capabilities")?; diff --git a/src/crosvm/plugin/process.rs b/src/crosvm/plugin/process.rs index db29d26796..f9b320bbf5 100644 --- a/src/crosvm/plugin/process.rs +++ b/src/crosvm/plugin/process.rs @@ -176,15 +176,12 @@ impl Process { /// * `args`: arguments to plugin executable /// * `jail`: jail to launch plugin in. If None plugin will just be spawned as a child /// * `stderr`: File to redirect stderr of plugin process to - /// * `env_fds`: collection of (Name, FD) where FD will be inherited by spawned process - /// and added to child's environment as a variable Name pub fn new( cpu_count: u32, cmd: &Path, args: &[&str], jail: Option, stderr: File, - env_fds: Vec<(String, Descriptor)>, ) -> Result { let (request_socket, child_socket) = new_seqpacket_pair().context("error creating main request socket")?; @@ -203,49 +200,29 @@ impl Process { CROSVM_SOCKET_ENV, child_socket.as_raw_descriptor().to_string(), ); - env_fds - .iter() - .for_each(|(k, fd)| set_var(k, fd.as_raw_descriptor().to_string())); jail.run_remap( cmd, - &env_fds - .into_iter() - .map(|(_, fd)| (fd.as_raw_descriptor(), fd.as_raw_descriptor())) - .chain( - [ - (stderr.as_raw_descriptor(), STDERR_FILENO), - ( - child_socket.as_raw_descriptor(), - child_socket.as_raw_descriptor(), - ), - ] - .into_iter(), - ) - .collect::>(), + &[ + (stderr.as_raw_descriptor(), STDERR_FILENO), + ( + child_socket.as_raw_descriptor(), + child_socket.as_raw_descriptor(), + ), + ], args, ) .context("failed to run plugin jail")? } - None => { - for (_, fd) in env_fds.iter() { - base::clear_descriptor_cloexec(fd)?; - } - Command::new(cmd) - .args(args) - .envs( - env_fds - .into_iter() - .map(|(k, fd)| (k, { fd.as_raw_descriptor().to_string() })), - ) - .env( - CROSVM_SOCKET_ENV, - child_socket.as_raw_descriptor().to_string(), - ) - .stderr(stderr) - .spawn() - .context("failed to spawn plugin")? - .id() as pid_t - } + None => Command::new(cmd) + .args(args) + .env( + "CROSVM_SOCKET", + child_socket.as_raw_descriptor().to_string(), + ) + .stderr(stderr) + .spawn() + .context("failed to spawn plugin")? + .id() as pid_t, }; Ok(Process { diff --git a/src/crosvm/sys.rs b/src/crosvm/sys.rs index 51e48c6a7c..5a4064032d 100644 --- a/src/crosvm/sys.rs +++ b/src/crosvm/sys.rs @@ -14,10 +14,6 @@ cfg_if::cfg_if! { #[cfg(feature = "gpu")] pub(crate) use unix::gpu::GpuRenderServerParameters; - #[cfg(all(feature = "virgl_renderer_next", feature = "plugin"))] - pub(crate) use unix::gpu::start_gpu_render_server; - #[cfg(all(feature = "virgl_renderer_next", feature = "plugin"))] - pub(crate) use unix::jail_helpers; } else if #[cfg(windows)] { use windows as platform; } else { diff --git a/tools/health-check b/tools/health-check index 7f133fe2f0..f520c8d11d 100755 --- a/tools/health-check +++ b/tools/health-check @@ -114,7 +114,6 @@ KNOWN_DISABLED_FEATURES = [ "direct", "gfxstream", "libvda", - "plugin-render-server", "vaapi", "video-encoder", "whpx",