Skip to content

Commit

Permalink
Changes to DBus and engine APIs for idempotence
Browse files Browse the repository at this point in the history 
Make internal changes to the engine to better support
idempotent operations using the DBus API so that the return
value of DBus methods provides information on what was changed
and omits resources that were not changed.

Co-Authored-By: mulhern <[email protected]>
  • Loading branch information
@jbaublitz @mulkieran
jbaublitz and mulkieran committed Oct 9, 2019
1 parent 9c3f61e commit ed57a38
Show file tree
Hide file tree
Showing 26 changed files with 785 additions and 394 deletions.
85 changes: 55 additions & 30 deletions src/dbus_api/api.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use crate::{
engine_to_dbus_err_tuple, get_next_arg, msg_code_ok, msg_string_ok, tuple_to_option,
},
},
engine::{Engine, Pool, PoolUuid},
engine::{CreateAction, DeleteAction, Engine, Pool, PoolUuid},
stratis::VERSION,
};

Expand All @@ -47,28 +47,30 @@ fn create_pool(m: &MethodInfo<MTFn<TData>, TData>) -> MethodResult {

let return_message = message.method_return();

let default_return: (dbus::Path, Vec<dbus::Path>) = (dbus::Path::default(), Vec::new());
let default_return: (bool, (dbus::Path<'static>, Vec<dbus::Path<'static>>)) =
(false, (dbus::Path::default(), Vec::new()));

let msg = match result {
Ok(pool_uuid) => {
let (_, pool) = get_mut_pool!(engine; pool_uuid; default_return; return_message);

let pool_object_path: dbus::Path =
create_dbus_pool(dbus_context, object_path.clone(), pool_uuid, pool);

let bd_object_paths = pool
.blockdevs_mut()
.into_iter()
.map(|(uuid, bd)| {
create_dbus_blockdev(dbus_context, pool_object_path.clone(), uuid, bd)
})
.collect::<Vec<_>>();

return_message.append3(
(pool_object_path, bd_object_paths),
msg_code_ok(),
msg_string_ok(),
)
Ok(pool_uuid_action) => {
let results = match pool_uuid_action {
CreateAction::Created(uuid) => {
let (_, pool) = get_mut_pool!(engine; uuid; default_return; return_message);

let pool_object_path: dbus::Path =
create_dbus_pool(dbus_context, object_path.clone(), uuid, pool);

let bd_paths = pool
.blockdevs_mut()
.into_iter()
.map(|(uuid, bd)| {
create_dbus_blockdev(dbus_context, pool_object_path.clone(), uuid, bd)
})
.collect::<Vec<_>>();
(true, (pool_object_path, bd_paths))
}
CreateAction::Identity => default_return,
};
return_message.append3(results, msg_code_ok(), msg_string_ok())
}
Err(x) => {
let (rc, rs) = engine_to_dbus_err_tuple(&x);
Expand All @@ -82,15 +84,20 @@ fn destroy_pool(m: &MethodInfo<MTFn<TData>, TData>) -> MethodResult {
let message: &Message = m.msg;
let mut iter = message.iter_init();

let object_path: dbus::Path<'static> = get_next_arg(&mut iter, 0)?;
let pool_path: dbus::Path<'static> = get_next_arg(&mut iter, 0)?;

let dbus_context = m.tree.get_data();

let default_return = false;
let default_return = (false, uuid_to_string!(PoolUuid::nil()));
let return_message = message.method_return();

let pool_uuid = match m.tree.get(&object_path) {
Some(pool_path) => get_data!(pool_path; default_return; return_message).uuid,
let pool_uuid = match m
.tree
.get(&pool_path)
.and_then(|op| op.get_data().as_ref())
.map(|d| d.uuid)
{
Some(uuid) => uuid,
None => {
return Ok(vec![return_message.append3(
default_return,
Expand All @@ -101,12 +108,19 @@ fn destroy_pool(m: &MethodInfo<MTFn<TData>, TData>) -> MethodResult {
};

let msg = match dbus_context.engine.borrow_mut().destroy_pool(pool_uuid) {
Ok(action) => {
Ok(DeleteAction::Deleted(uuid)) => {
dbus_context
.actions
.borrow_mut()
.push_remove(&object_path, m.tree);
return_message.append3(action, msg_code_ok(), msg_string_ok())
.push_remove(&pool_path, m.tree);
return_message.append3(
(true, uuid_to_string!(uuid)),
msg_code_ok(),
msg_string_ok(),
)
}
Ok(DeleteAction::Identity) => {
return_message.append3(default_return, msg_code_ok(), msg_string_ok())
}
Err(err) => {
let (rc, rs) = engine_to_dbus_err_tuple(&err);
Expand Down Expand Up @@ -155,14 +169,25 @@ fn get_base_tree<'a>(dbus_context: DbusContext) -> (Tree<MTFn<TData>, TData>, db
.in_arg(("name", "s"))
.in_arg(("redundancy", "(bq)"))
.in_arg(("devices", "as"))
.out_arg(("result", "(oao)"))
// In order from left to right:
// b: true if a pool was created and object paths were returned
// o: Object path for Pool
// a(o): Array of object paths for block devices
//
// Rust representation: (bool, (dbus::Path, Vec<dbus::Path>))
.out_arg(("result", "(b(oao))"))
.out_arg(("return_code", "q"))
.out_arg(("return_string", "s"));

let destroy_pool_method = f
.method("DestroyPool", (), destroy_pool)
.in_arg(("pool", "o"))
.out_arg(("action", "b"))
// In order from left to right:
// b: true if a valid UUID is returned - otherwise no action was performed
// s: String representation of pool UUID that was destroyed
//
// Rust representation: (bool, String)
.out_arg(("result", "(bs)"))
.out_arg(("return_code", "q"))
.out_arg(("return_string", "s"));

Expand Down
27 changes: 23 additions & 4 deletions src/dbus_api/blockdev.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::{
msg_code_ok, msg_string_ok,
},
},
engine::{BlockDev, BlockDevTier, MaybeDbusPath},
engine::{BlockDev, BlockDevTier, DevUuid, MaybeDbusPath, RenameAction},
};

pub fn create_dbus_blockdev<'a>(
Expand All @@ -36,7 +36,11 @@ pub fn create_dbus_blockdev<'a>(
let set_userid_method = f
.method("SetUserInfo", (), set_user_info)
.in_arg(("id", "s"))
.out_arg(("changed", "b"))
// b: false if no change to the user info
// s: UUID of the changed device
//
// Rust representation: (bool, String)
.out_arg(("changed", "(bs)"))
.out_arg(("return_code", "q"))
.out_arg(("return_string", "s"));

Expand Down Expand Up @@ -131,7 +135,7 @@ fn set_user_info(m: &MethodInfo<MTFn<TData>, TData>) -> MethodResult {
let dbus_context = m.tree.get_data();
let object_path = m.path.get_name();
let return_message = message.method_return();
let default_return = false;
let default_return = (false, uuid_to_string!(DevUuid::nil()));

let blockdev_path = m
.tree
Expand All @@ -148,7 +152,22 @@ fn set_user_info(m: &MethodInfo<MTFn<TData>, TData>) -> MethodResult {
let result = pool.set_blockdev_user_info(&pool_name, blockdev_data.uuid, new_id);

let msg = match result {
Ok(id_changed) => return_message.append3(id_changed, msg_code_ok(), msg_string_ok()),
Ok(RenameAction::NoSource) => {
let error_message = format!(
"pool doesn't know about block device {}",
blockdev_data.uuid
);
let (rc, rs) = (DbusErrorEnum::INTERNAL_ERROR as u16, error_message);
return_message.append3(default_return, rc, rs)
}
Ok(RenameAction::Renamed(uuid)) => return_message.append3(
(true, uuid_to_string!(uuid)),
msg_code_ok(),
msg_string_ok(),
),
Ok(RenameAction::Identity) => {
return_message.append3(default_return, msg_code_ok(), msg_string_ok())
}
Err(err) => {
let (rc, rs) = engine_to_dbus_err_tuple(&err);
return_message.append3(default_return, rc, rs)
Expand Down
22 changes: 15 additions & 7 deletions src/dbus_api/filesystem.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ use dbus::{
Message,
};

use uuid::Uuid;

use crate::{
dbus_api::{
consts,
Expand All @@ -23,21 +21,27 @@ use crate::{
msg_code_ok, msg_string_ok,
},
},
engine::{filesystem_mount_path, Filesystem, MaybeDbusPath, Name, RenameAction},
engine::{
filesystem_mount_path, Filesystem, FilesystemUuid, MaybeDbusPath, Name, RenameAction,
},
};

pub fn create_dbus_filesystem<'a>(
dbus_context: &DbusContext,
parent: dbus::Path<'static>,
uuid: Uuid,
uuid: FilesystemUuid,
filesystem: &mut dyn Filesystem,
) -> dbus::Path<'a> {
let f = Factory::new_fn();

let rename_method = f
.method("SetName", (), rename_filesystem)
.in_arg(("name", "s"))
.out_arg(("action", "b"))
// b: true if UUID of changed resource has been returned
// s: UUID of changed resource
//
// Rust representation: (bool, String)
.out_arg(("result", "(bs)"))
.out_arg(("return_code", "q"))
.out_arg(("return_string", "s"));

Expand Down Expand Up @@ -108,7 +112,7 @@ fn rename_filesystem(m: &MethodInfo<MTFn<TData>, TData>) -> MethodResult {
let dbus_context = m.tree.get_data();
let object_path = m.path.get_name();
let return_message = message.method_return();
let default_return = false;
let default_return = (false, uuid_to_string!(FilesystemUuid::nil()));

let filesystem_path = m
.tree
Expand All @@ -134,7 +138,11 @@ fn rename_filesystem(m: &MethodInfo<MTFn<TData>, TData>) -> MethodResult {
Ok(RenameAction::Identity) => {
return_message.append3(default_return, msg_code_ok(), msg_string_ok())
}
Ok(RenameAction::Renamed) => return_message.append3(true, msg_code_ok(), msg_string_ok()),
Ok(RenameAction::Renamed(uuid)) => return_message.append3(
(true, uuid_to_string!(uuid)),
msg_code_ok(),
msg_string_ok(),
),
Err(err) => {
let (rc, rs) = engine_to_dbus_err_tuple(&err);
return_message.append3(default_return, rc, rs)
Expand Down
Loading

0 comments on commit ed57a38

Please sign in to comment.