Skip to content
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

Add a try_fail_point! macro and use it in more places #4259

Merged
merged 1 commit into from
Jan 12, 2023
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
6 changes: 6 additions & 0 deletions rust/src/cxxrsutil.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,12 @@ mod err {
}
}

impl From<Box<dyn std::error::Error + Send + Sync>> for CxxError {
fn from(e: Box<dyn std::error::Error + Send + Sync>) -> Self {
Self(e.to_string())
}
}

impl From<cxx::Exception> for CxxError {
fn from(v: cxx::Exception) -> Self {
Self(format!("{:#}", v))
Expand Down
16 changes: 0 additions & 16 deletions rust/src/failpoint_bridge.rs

This file was deleted.

34 changes: 34 additions & 0 deletions rust/src/failpoints.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//! Wrappers and utilities on top of the `fail` crate.
// SPDX-License-Identifier: Apache-2.0 OR MIT

use anyhow::Result;

/// TODO: Use https://github.com/tikv/fail-rs/pull/68 once it merges
#[macro_export]
macro_rules! try_fail_point {
($name:expr) => {{
if let Some(e) = fail::eval($name, |msg| {
let msg = msg.unwrap_or_else(|| "synthetic failpoint".to_string());
anyhow::Error::msg(msg)
}) {
return Err(From::from(e));
}
}};
($name:expr, $cond:expr) => {{
if $cond {
$crate::try_fail_point!($name);
}
}};
}

/// Expose the `fail::fail_point` macro to C++.
pub fn failpoint(p: &str) -> Result<()> {
ostree_ext::glib::g_debug!("rpm-ostree", "{}", p);
fail::fail_point!(p, |r| {
Err(match r {
Some(ref msg) => anyhow::anyhow!("{}", msg),
None => anyhow::anyhow!("failpoint {}", p),
})
});
Ok(())
}
8 changes: 4 additions & 4 deletions rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ pub mod ffi {
fn generate_object_path(base: &str, next_segment: &str) -> Result<String>;
}

// failpoint_bridge.rs
// failpoints.rs
extern "Rust" {
fn failpoint(p: &str) -> Result<()>;
}
Expand Down Expand Up @@ -914,8 +914,8 @@ pub(crate) use daemon::*;
mod deployment_utils;
pub(crate) use deployment_utils::*;
mod dirdiff;
pub mod failpoint_bridge;
use failpoint_bridge::*;
pub mod failpoints;
use failpoints::*;
mod extensions;
pub(crate) use extensions::*;
#[cfg(feature = "fedora-integration")]
Expand Down Expand Up @@ -956,6 +956,6 @@ mod testutils;
pub(crate) use self::testutils::*;
mod treefile;
pub use self::treefile::*;
mod utils;
pub(crate) mod utils;
pub use self::utils::*;
mod variant_utils;
2 changes: 1 addition & 1 deletion rust/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ fn inner_main() -> Result<i32> {
}
// Initialize failpoints
let _scenario = fail::FailScenario::setup();
fail::fail_point!("main");
rpmostree_rust::try_fail_point!("main");
// Call this early on; it invokes e.g. setenv() so must be done
// before we create threads.
rpmostree_rust::ffi::early_main();
Expand Down
3 changes: 3 additions & 0 deletions rust/src/sysroot_upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ pub(crate) fn pull_container(
let cancellable = cancellable.glib_reborrow();
let imgref = &OstreeImageReference::try_from(imgref)?;

crate::try_fail_point!("sysroot-upgrade::container-pull");

let r = Handle::current().block_on(async {
crate::utils::run_with_cancellable(
async { pull_container_async(repo, imgref).await },
Expand All @@ -141,6 +143,7 @@ pub(crate) fn layer_prune(
c.set_error_if_cancelled()?;
}
tracing::debug!("pruning image layers");
crate::try_fail_point!("sysroot-upgrade::layer-prune");
let n_pruned = ostree_ext::container::store::gc_image_layers(repo)?;
systemd::journal::print(6, &format!("Pruned container image layers: {n_pruned}"));
Ok(())
Expand Down
14 changes: 13 additions & 1 deletion src/daemon/rpmostreed-transaction.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,17 @@ transaction_execute_thread (GTask *task, gpointer source_object, gpointer task_d
// Further, we join the main Tokio async runtime.
auto guard = rpmostreecxx::rpmostreed_daemon_tokio_enter (rpmostreed_daemon_get ());

if (clazz->execute != NULL)
try
{
rpmostreecxx::failpoint ("transaction::execute");
}
catch (std::exception &e)
{
success = FALSE;
glnx_throw (&local_error, "%s", e.what ());
}

if (success && clazz->execute != NULL)
{
// This try/catch shouldn't be needed; every CXX call should be wrapped with the CXX macro.
// But in case we regress on this, it's better to error out than crash.
Expand Down Expand Up @@ -605,6 +615,8 @@ transaction_initable_init (GInitable *initable, GCancellable *cancellable, GErro
return FALSE;
sd_journal_print (LOG_INFO, "Loaded sysroot");

CXX_TRY (rpmostreecxx::failpoint ("transaction::lock"), error);

if (!ostree_sysroot_try_lock (priv->sysroot, &lock_acquired, error))
return FALSE;

Expand Down
6 changes: 6 additions & 0 deletions src/libpriv/rpmostree-core.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -1416,6 +1416,7 @@ check_goal_solution (RpmOstreeContext *self, GPtrArray *removed_pkgnames,
GHashTable *replaced_pkgnames, GError **error)
{
HyGoal goal = dnf_context_get_goal (self->dnfctx);
CXX_TRY (rpmostreecxx::failpoint ("core::check-goal-solution"), error);

/* check that we're not removing anything we didn't expect */
{
Expand Down Expand Up @@ -4057,6 +4058,8 @@ rpmostree_context_assemble (RpmOstreeContext *self, GCancellable *cancellable, G
return FALSE;
int tmprootfs_dfd = self->tmprootfs_dfd; /* Alias to avoid bigger diff */

CXX_TRY (rpmostreecxx::failpoint ("core::assemble"), error);

/* In e.g. removing a package we walk librpm which doesn't have canonical
* /usr, so we need to build up a mapping.
*/
Expand Down Expand Up @@ -4503,6 +4506,8 @@ rpmostree_context_assemble (RpmOstreeContext *self, GCancellable *cancellable, G
gboolean
rpmostree_context_assemble_end (RpmOstreeContext *self, GCancellable *cancellable, GError **error)
{
CXX_TRY (rpmostreecxx::failpoint ("core::assemble-end"), error);

if (!ensure_tmprootfs_dfd (self, error))
return FALSE;
if (self->treefile_rs->get_cliwrap ())
Expand Down Expand Up @@ -4534,6 +4539,7 @@ rpmostree_context_commit (RpmOstreeContext *self, const char *parent,
g_autofree char *ret_commit_checksum = NULL;

auto task = rpmostreecxx::progress_begin_task ("Writing OSTree commit");
CXX_TRY (rpmostreecxx::failpoint ("core::commit"), error);

g_auto (RpmOstreeRepoAutoTransaction) txn = {
0,
Expand Down
2 changes: 2 additions & 0 deletions src/libpriv/rpmostree-importer.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,8 @@ rpmostree_importer_run (RpmOstreeImporter *self, char **out_csum, char **out_met
if (g_cancellable_set_error_if_cancelled (cancellable, error))
return FALSE;

CXX_TRY (rpmostreecxx::failpoint ("rpm-importer::run"), error);

g_autofree char *metadata_sha256 = NULL;
g_autofree char *csum = NULL;
if (!import_rpm_to_repo (self, &csum, &metadata_sha256, cancellable, error))
Expand Down