From 59cfdaaff0d32036bb40926600713deeda0f73a7 Mon Sep 17 00:00:00 2001 From: Dennis Kempin Date: Thu, 2 Jun 2022 22:51:40 +0000 Subject: [PATCH 1/3] tools: Add tools/cl script as a helper for uploading to gerrit See USAGE for documentation. This tool will be needed to upload to the upstream repository, as we will no longer be able to use repo. Currently, for testing it still points to the chromiumos repository. BUG=b:221088786 TEST=Follow USAGE Change-Id: I4a8d88a8354942f0ddfb7f9420ef1cf7f5885867 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3687419 Reviewed-by: Daniel Verkamp Tested-by: kokoro --- tools/cl | 164 +++++++++++++++++++++++++++++++++++++++++++ tools/impl/common.py | 21 +++++- 2 files changed, 183 insertions(+), 2 deletions(-) create mode 100755 tools/cl diff --git a/tools/cl b/tools/cl new file mode 100755 index 0000000000..d06036554f --- /dev/null +++ b/tools/cl @@ -0,0 +1,164 @@ +#!/usr/bin/env python3 +# Copyright 2022 The Chromium OS Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +from impl.common import confirm, run_commands, cmd, CROSVM_ROOT +import sys + +USAGE = """\ +./tools/cl [upload|rebase|status] + +Upload changes to the upstream crosvm gerrit. + +Multiple projects have their own downstream repository of crosvm and tooling +to upload to those. + +This tool allows developers to send commits to the upstream gerrit review site +of crosvm and helps rebase changes if needed. + +You need to be on a local branch tracking a remote one. `repo start` does this +for AOSP and chromiumos, or you can do this yourself: + + $ git checkout -b mybranch --track origin/main + +Then to upload commits you have made: + + [mybranch] $ ./tools/cl upload + +If you are tracking a different branch (e.g. aosp/main or cros/chromeos), the upload may +fail if your commits do not apply cleanly. This tool can help rebase the changes, it will +create a new branch tracking origin/main and cherry-picks your commits. + + [mybranch] $ ./tools/cl rebase + [mybranch-upstream] ... resolve conflicts + [mybranch-upstream] $ git add . + [mybranch-upstream] $ git cherry-pick --continue + [mybranch-upstream] $ ./tools/cl upload + +""" + +GERRIT_URL = "https://chromium-review.googlesource.com" +CROSVM_URL = "https://chromium.googlesource.com/chromiumos/platform/crosvm" + +git = cmd("git") +curl = cmd("curl --silent --fail") +chmod = cmd("chmod") + + +def get_upstream(branch: str = ""): + try: + return git(f"rev-parse --abbrev-ref --symbolic-full-name {branch}@{{u}}").stdout() + except: + return None + + +def list_local_changes(branch: str = ""): + upstream = get_upstream(branch) + if not upstream: + return [] + for line in git(f"log --oneline --first-parent {upstream}..{branch or 'HEAD'}").lines(): + yield line.split(" ", 1) + + +def list_local_branches(): + return git("for-each-ref --format=%(refname:short) refs/heads").lines() + + +def get_active_upstream(): + upstream = get_upstream() + if not upstream: + raise Exception("You are not tracking an upstream branch.") + parts = upstream.split("/") + if len(parts) != 2: + raise Exception(f"Your upstream branch '{upstream}' is not remote.") + return (parts[0], parts[1]) + + +def prerequisites(): + print("This tool is experimental and a work in progress, please use carefully.") + print() + + if not git("remote get-url origin").success(): + print("Setting up origin") + git("remote add origin", CROSVM_URL).fg() + if git("remote get-url origin").stdout() != CROSVM_URL: + print("Your remote 'origin' does not point to the main crosvm repository.") + if confirm(f"Do you want to fix it?"): + git("remote set-url origin", CROSVM_URL).fg() + else: + sys.exit(1) + + # Install gerrit commit hook + hook_path = CROSVM_ROOT / ".git/hooks/commit-msg" + if not hook_path.exists(): + hook_path.parent.mkdir(exist_ok=True) + curl(f"{GERRIT_URL}/tools/hooks/commit-msg").write_to(hook_path) + chmod("+x", hook_path).fg() + + +def status(): + """ + Lists all branches and their local commits. + """ + for branch in list_local_branches(): + print("Branch", branch, "tracking", get_upstream(branch)) + changes = [*list_local_changes(branch)] + for sha, title in changes: + print(" ", title) + if not changes: + print(" No changes") + print() + + +def rebase(): + """ + Rebases changes from the current branch onto origin/main. + + Will create a new branch called 'current-branch'-upstream tracking origin/main. Changes from + the current branch will then be rebased into the -upstream branch. + """ + prerequisites() + branch_name = git("branch --show-current").stdout() + upstream_branch_name = branch_name + "-upstream" + + print(f"Checking out '{upstream_branch_name}'") + rev = git("rev-parse", upstream_branch_name).stdout(check=False) + if rev: + print(f"Leaving behind previous revision of {upstream_branch_name}: {rev}") + git("checkout -B", upstream_branch_name, "origin/main").fg(quiet=True) + + print(f"Cherry-picking changes from {branch_name}") + git(f"cherry-pick {branch_name}@{{u}}..{branch_name}").fg() + + +def upload(): + """ + Uploads changes to the crosvm main branch. + """ + prerequisites() + + remote, branch = get_active_upstream() + changes = [*list_local_changes()] + if not changes: + print("No changes to upload") + return + + print("Uploading to origin/main:") + for sha, title in changes: + print(" ", sha, title) + print() + + if (remote, branch) != ("origin", "main"): + print(f"WARNING! Your changes are based on {remote}/{branch}, not origin/main.") + print("If gerrit rejects your changes, try `./tools/cl rebase -h`.") + print() + if not confirm("Upload anyway?"): + return + print() + + git("push", remote, f"HEAD:refs/for/{branch}").fg() + + +if __name__ == "__main__": + run_commands(upload, rebase, status, usage=USAGE) diff --git a/tools/impl/common.py b/tools/impl/common.py index 04b37f9e8e..f1cea7e8c9 100644 --- a/tools/impl/common.py +++ b/tools/impl/common.py @@ -191,6 +191,9 @@ class Command(object): raise subprocess.CalledProcessError(result.returncode, str(self), result.stdout) return result.returncode + def success(self): + return self.fg(check=False, quiet=True) == 0 + def stdout(self, check: bool = True): """ Runs a program and returns stdout. Stderr is still directed to the user. @@ -467,7 +470,11 @@ def run_main(main_fn: Callable[..., Any]): run_commands(default_fn=main_fn) -def run_commands(*functions: Callable[..., Any], default_fn: Optional[Callable[..., Any]] = None): +def run_commands( + *functions: Callable[..., Any], + default_fn: Optional[Callable[..., Any]] = None, + usage: Optional[str] = None, +): """ Allow the user to call the provided functions with command line arguments translated to function arguments via argh: https://pythonhosted.org/argh @@ -480,7 +487,7 @@ def run_commands(*functions: Callable[..., Any], default_fn: Optional[Callable[. sys.exit(1) try: # Add global verbose arguments - parser = argparse.ArgumentParser() + parser = argparse.ArgumentParser(usage=usage) add_verbose_args(parser) # Add provided commands to parser. Do not use sub-commands if we just got one function. @@ -547,6 +554,16 @@ def find_scripts(path: Path, shebang: str): yield file +def confirm(message: str, default=False): + print(message, "[y/N]" if default == False else "[Y/n]") + response = sys.stdin.readline().strip() + if response in ("y", "Y"): + return True + if response in ("n", "N"): + return False + return default + + if __name__ == "__main__": import doctest From b803f10fa259df0396d2a75a5f897996c75526f3 Mon Sep 17 00:00:00 2001 From: Noah Gold Date: Wed, 1 Jun 2022 18:31:19 -0700 Subject: [PATCH 2/3] base: add unix no op impl of enable_high_res_timers. This is needed by broker_ipc (next crate in the chain). BUG=232318124 TEST=builds Change-Id: I7d71b9208123dd055b7150fd3ae6703cdc21bf6a Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3685809 Commit-Queue: Noah Gold Tested-by: kokoro Reviewed-by: Vikram Auradkar --- base/src/lib.rs | 1 + base/src/sys/unix/mod.rs | 1 + base/src/sys/unix/platform_timer_resolution.rs | 14 ++++++++++++++ 3 files changed, 16 insertions(+) create mode 100644 base/src/sys/unix/platform_timer_resolution.rs diff --git a/base/src/lib.rs b/base/src/lib.rs index 9389e2652b..a0d265fb34 100644 --- a/base/src/lib.rs +++ b/base/src/lib.rs @@ -126,6 +126,7 @@ pub use crate::descriptor::{ }; pub use platform::getpid; +pub use platform::platform_timer_resolution::enable_high_res_timers; pub use platform::{get_filesystem_type, open_file}; pub use platform::{number_of_logical_cores, pagesize, round_up_to_page_size}; pub use platform::{FileReadWriteAtVolatile, FileReadWriteVolatile, FileSetLen, FileSync}; diff --git a/base/src/sys/unix/mod.rs b/base/src/sys/unix/mod.rs index 395c19c0a4..634cde535e 100644 --- a/base/src/sys/unix/mod.rs +++ b/base/src/sys/unix/mod.rs @@ -30,6 +30,7 @@ mod mmap; pub mod net; mod netlink; mod notifiers; +pub mod platform_timer_resolution; mod poll; mod priority; pub mod rand; diff --git a/base/src/sys/unix/platform_timer_resolution.rs b/base/src/sys/unix/platform_timer_resolution.rs new file mode 100644 index 0000000000..e8838a7461 --- /dev/null +++ b/base/src/sys/unix/platform_timer_resolution.rs @@ -0,0 +1,14 @@ +// Copyright 2022 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +use crate::{EnabledHighResTimer, Result}; + +/// Noop struct on unix. +/// On windows, restores the platform timer resolution to its original value on Drop. +pub struct UnixSetTimerResolution {} +impl EnabledHighResTimer for UnixSetTimerResolution {} + +pub fn enable_high_res_timers() -> Result> { + Ok(Box::new(UnixSetTimerResolution {})) +} From eac74eb83684b8932653ed1dd130adee6ca8573a Mon Sep 17 00:00:00 2001 From: Noah Gold Date: Fri, 13 May 2022 15:14:15 -0700 Subject: [PATCH 3/3] broker_ipc: add crate Adds the broker_ipc crate, which is used for communication & setup tasks shared between the broker (Windows orchestrator for crosvm & vhost-user devices) and all other crosvm processes. BUG=b:232318124 TEST=builds Change-Id: I17d529305dd348a50574e6c3c082808e97ffff45 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3648474 Reviewed-by: Dennis Kempin Tested-by: kokoro Commit-Queue: Noah Gold --- Cargo.toml | 2 + broker_ipc/Cargo.toml | 12 ++++++ broker_ipc/src/generic.rs | 32 ++++++++++++++++ broker_ipc/src/lib.rs | 78 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 124 insertions(+) create mode 100644 broker_ipc/Cargo.toml create mode 100644 broker_ipc/src/generic.rs create mode 100644 broker_ipc/src/lib.rs diff --git a/Cargo.toml b/Cargo.toml index 134bc09a61..4d4aa5ef0f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,6 +47,7 @@ members = [ "argh_helpers", "base", "bit_field", + "broker_ipc", "cros_async", "crosvm-fuzz", "crosvm_control", @@ -154,6 +155,7 @@ assertions = { path = "common/assertions" } audio_streams = "*" base = "*" bit_field = { path = "bit_field" } +broker_ipc = { path = "broker_ipc" } cfg-if = "1.0.0" crosvm_plugin = { path = "crosvm_plugin", optional = true } data_model = "*" diff --git a/broker_ipc/Cargo.toml b/broker_ipc/Cargo.toml new file mode 100644 index 0000000000..37b18c4c89 --- /dev/null +++ b/broker_ipc/Cargo.toml @@ -0,0 +1,12 @@ +[package] +name = "broker_ipc" +authors = ["The Chromium OS Authors"] +version = "0.1.0" +edition = "2021" + +[dependencies] +anyhow = "1.0.32" +base = { path = "../base" } +serde = { version = "1", features = [ "derive" ] } +metrics = { path = "../metrics" } + diff --git a/broker_ipc/src/generic.rs b/broker_ipc/src/generic.rs new file mode 100644 index 0000000000..b39820169d --- /dev/null +++ b/broker_ipc/src/generic.rs @@ -0,0 +1,32 @@ +// Copyright 2022 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +//! Generic implementation of product specific functions that are called on child process +//! initialization. + +use crate::{log_file_from_path, CommonChildStartupArgs}; +use base::Tube; +use serde::{Deserialize, Serialize}; +use std::path::PathBuf; + +#[derive(Serialize, Deserialize)] +pub struct ProductAttributes {} + +impl CommonChildStartupArgs { + pub fn new(syslog_path: Option, metrics_tube: Option) -> anyhow::Result { + Ok(Self { + product_attrs: ProductAttributes {}, + metrics_tube, + syslog_file: log_file_from_path(syslog_path)?, + }) + } +} + +pub(crate) fn init_child_crash_reporting(_attrs: &ProductAttributes) { + // Do nothing. Crash reporting is implemented by a specific product. +} + +pub(crate) fn product_child_setup(_attrs: &ProductAttributes) -> anyhow::Result<()> { + Ok(()) +} diff --git a/broker_ipc/src/lib.rs b/broker_ipc/src/lib.rs new file mode 100644 index 0000000000..060c8d6a38 --- /dev/null +++ b/broker_ipc/src/lib.rs @@ -0,0 +1,78 @@ +// Copyright 2022 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +//! Contains shared code between the broker & its children, specifically any IPC messages or common +//! bootstrapping code. + +use anyhow::Context; +use base::{enable_high_res_timers, syslog, FromRawDescriptor, IntoRawDescriptor, SafeDescriptor}; +use base::{EnabledHighResTimer, Tube}; +use serde::{Deserialize, Serialize}; +use std::fs::{File, OpenOptions}; + +mod generic; +use generic as product; + +use product::{init_child_crash_reporting, product_child_setup, ProductAttributes}; + +use std::path::PathBuf; + +/// Arguments that are common to all devices & helper processes. +#[derive(Serialize, Deserialize)] +pub struct CommonChildStartupArgs { + syslog_file: Option, + metrics_tube: Option, + product_attrs: ProductAttributes, +} + +pub struct ChildLifecycleCleanup { + _timer_resolution: Box, +} + +/// Initializes crash reporting, metrics, logging, and product specific features +/// for a process. +/// +/// Returns a value that should be dropped when the process exits. +pub fn common_child_setup(args: CommonChildStartupArgs) -> anyhow::Result { + // Crash reporting should start as early as possible, in case other startup tasks fail. + init_child_crash_reporting(&args.product_attrs); + + let mut cfg = syslog::LogConfig::default(); + if let Some(log_file_descriptor) = args.syslog_file { + // Safe because we are taking ownership of a SafeDescriptor. + let log_file = + unsafe { File::from_raw_descriptor(log_file_descriptor.into_raw_descriptor()) }; + cfg.pipe = Some(Box::new(log_file)); + cfg.stderr = false; + } else { + cfg.stderr = true; + } + syslog::init_with(cfg)?; + + // Initialize anything product specific. + product_child_setup(&args.product_attrs)?; + + if let Some(metrics_tube) = args.metrics_tube { + metrics::initialize(metrics_tube); + } + + let timer_resolution = enable_high_res_timers().context("failed to enable high res timer")?; + + Ok(ChildLifecycleCleanup { + _timer_resolution: timer_resolution, + }) +} + +pub(crate) fn log_file_from_path(path: Option) -> anyhow::Result> { + Ok(match path { + Some(path) => Some(SafeDescriptor::from( + OpenOptions::new() + .append(true) + .create(true) + .open(path.as_path()) + .context(format!("failed to open log file {}", path.display()))?, + )), + None => None, + }) +}