Skip to content

Commit

Permalink
DAOS-14547: Source pool/cont only needs read permission (#568)
Browse files Browse the repository at this point in the history 
Open the source pool and container with RO, and the destination with RW.

Signed-off-by: Dalton Bohning <[email protected]>
  • Loading branch information
@daltonbohning
daltonbohning authored Apr 11, 2024
1 parent 3e970d8 commit 0a4c530
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 58 deletions.
122 changes: 69 additions & 53 deletions src/common/mfu_daos.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,9 @@ int daos_parse_path(
} else {
strncpy(path, dattr.da_rel_path, path_len);
}
} else if (strncmp(path, "daos:", 5) == 0) {
/* Actual error, since we expect a daos path */
rc = -1;
} else {
/* If basename does not exist yet then duns_resolve_path will fail even
* if dirname is a UNS path */
Expand Down Expand Up @@ -354,9 +357,6 @@ int daos_parse_path(
snprintf(*pool_str, DAOS_PROP_LABEL_MAX_LEN + 1, "%s", dattr.da_pool);
snprintf(*cont_str, DAOS_PROP_LABEL_MAX_LEN + 1, "%s", dattr.da_cont);
strncpy(path, dattr.da_rel_path, path_len);
} else if (strncmp(path, "daos:", 5) == 0) {
/* Actual error, since we expect a daos path */
rc = -1;
} else {
/* We didn't parse a daos path,
* but we weren't necessarily looking for one */
Expand Down Expand Up @@ -450,7 +450,7 @@ static int daos_set_paths(
return rc;
}

static int daos_get_cont_type(
static int mfu_daos_get_cont_type(
daos_handle_t coh,
enum daos_cont_props* type)
{
Expand Down Expand Up @@ -516,7 +516,7 @@ static int daos_check_cont_status(daos_handle_t coh, bool *status_healthy)
* Try to set the file type based on the container type,
* using api as a guide.
*/
static int daos_set_api_cont_type(
static int mfu_daos_set_api_cont_type(
mfu_file_t* mfu_file,
enum daos_cont_props cont_type,
daos_api_t api)
Expand Down Expand Up @@ -1013,7 +1013,7 @@ void daos_bcast_handle(
}

/* connect to DAOS pool, and then open container */
int daos_connect(
int mfu_daos_connect(
int rank,
daos_args_t* da,
char (*pool)[],
Expand All @@ -1022,7 +1022,9 @@ int daos_connect(
daos_handle_t* coh,
bool force_serialize,
bool connect_pool,
unsigned int pool_connect_flags,
bool create_cont,
unsigned int cont_open_flags,
bool require_new_cont,
bool preserve,
mfu_file_t* mfu_src_file,
Expand Down Expand Up @@ -1059,12 +1061,12 @@ int daos_connect(
}
#endif
/* Connect to DAOS pool */
if (connect_pool) {
if (connect_pool && !daos_handle_is_valid(*poh)) {
daos_pool_info_t pool_info = {0};
#if DAOS_API_VERSION_MAJOR < 1
rc = daos_pool_connect(*pool, NULL, NULL, DAOS_PC_RW, poh, &pool_info, NULL);
rc = daos_pool_connect(*pool, NULL, NULL, pool_connect_flags, poh, &pool_info, NULL);
#else
rc = daos_pool_connect(*pool, NULL, DAOS_PC_RW, poh, &pool_info, NULL);
rc = daos_pool_connect(*pool, NULL, pool_connect_flags, poh, &pool_info, NULL);
#endif
if (rc != 0) {
MFU_LOG(MFU_LOG_ERR, "Failed to connect to pool: "DF_RC, DP_RC(rc));
Expand All @@ -1082,9 +1084,9 @@ int daos_connect(
* not generated */
char cont_str[130];
uuid_unparse(da->dst_cont_uuid, cont_str);
rc = daos_cont_open(*poh, cont_str, DAOS_COO_RW, coh, &co_info, NULL);
rc = daos_cont_open(*poh, cont_str, cont_open_flags, coh, &co_info, NULL);
} else {
rc = daos_cont_open(*poh, *cont, DAOS_COO_RW, coh, &co_info, NULL);
rc = daos_cont_open(*poh, *cont, cont_open_flags, coh, &co_info, NULL);
}
if (rc != 0) {
if (rc != -DER_NONEXIST || !create_cont) {
Expand Down Expand Up @@ -1146,7 +1148,7 @@ int daos_connect(
/* if nothing is passed in for destination a uuid is always
* generated unless user passed one in, destination container
* labels are not generated */
rc = dfs_cont_create(*poh, da->dst_cont_uuid, &attr, NULL, NULL);
rc = dfs_cont_create(*poh, &da->dst_cont_uuid, &attr, NULL, NULL);
}
if (rc != 0) {
MFU_LOG(MFU_LOG_ERR, "Failed to create container: (%d %s)", rc, strerror(rc));
Expand All @@ -1156,7 +1158,7 @@ int daos_connect(
if (dst_cont_passed && !is_uuid) {
rc = daos_cont_create_with_label(*poh, *cont, props, &da->dst_cont_uuid, NULL);
} else {
rc = daos_cont_create(*poh, da->dst_cont_uuid, props, NULL);
rc = daos_cont_create(*poh, &da->dst_cont_uuid, props, NULL);
}
if (rc != 0) {
MFU_LOG(MFU_LOG_ERR, "Failed to create container: "DF_RC, DP_RC(rc));
Expand All @@ -1174,11 +1176,11 @@ int daos_connect(

/* try to open it again */
if (dst_cont_passed && !is_uuid) {
rc = daos_cont_open(*poh, *cont, DAOS_COO_RW, coh, &co_info, NULL);
rc = daos_cont_open(*poh, *cont, cont_open_flags, coh, &co_info, NULL);
} else {
char cont_str[130];
uuid_unparse(da->dst_cont_uuid, cont_str);
rc = daos_cont_open(*poh, cont_str, DAOS_COO_RW, coh, &co_info, NULL);
rc = daos_cont_open(*poh, cont_str, cont_open_flags, coh, &co_info, NULL);
}
if (rc != 0) {
MFU_LOG(MFU_LOG_ERR, "Failed to open container: "DF_RC, DP_RC(rc));
Expand Down Expand Up @@ -1554,8 +1556,7 @@ int daos_setup(
}

/* Make sure there weren't any errors before continuing.
* Since daos_connect has a collective broadcast.
* we have to make sure same_pool below is valid. */
* Since mfu_daos_connect has a collective broadcast. */
if (daos_any_error(rank, local_daos_error, flag_daos_args)) {
tmp_rc = daos_fini();
return 1;
Expand All @@ -1569,10 +1570,16 @@ int daos_setup(
bool have_src_cont = strlen(da->src_cont) > 0 ? true : false;
bool have_dst_pool = strlen(da->dst_pool) > 0 ? true : false;

/* Check if containers are in the same pool */
bool same_pool = (strcmp(da->src_pool, da->dst_pool) == 0);
/* Need W on destination pools for creating a container */
unsigned int src_pool_connect_flags = DAOS_PC_RO;
unsigned int dst_pool_connect_flags = DAOS_PC_RW;

/* For POSIX containers, the source only needs read, but the destination needs read and write.
* For DAOS (object-level), both containers need read and write.
* Open the source with RO first, then elevate to RW if needed below. */
unsigned int src_cont_open_flags = DAOS_COO_RO;
unsigned int dst_cont_open_flags = DAOS_COO_RW;

bool connect_pool = true;
bool create_cont = false;
bool require_new_cont = false;
bool preserve = false;
Expand All @@ -1587,28 +1594,52 @@ int daos_setup(
local_daos_error = true;
goto out;
}
tmp_rc = daos_connect(rank, da, &da->src_pool, &da->src_cont,
&da->src_poh, &da->src_coh, false,
connect_pool, create_cont, require_new_cont,
preserve, mfu_src_file, dst_cont_passed);
tmp_rc = mfu_daos_connect(rank, da, &da->src_pool, &da->src_cont,
&da->src_poh, &da->src_coh, false,
true, src_pool_connect_flags, create_cont, src_cont_open_flags, require_new_cont,
preserve, mfu_src_file, dst_cont_passed);
if (tmp_rc != 0) {
/* tmp_rc from daos_connect is collective */
/* tmp_rc from mfu_daos_connect is collective */
local_daos_error = true;
goto out;
}
/* Get the container type */
tmp_rc = daos_get_cont_type(da->src_coh, &da->src_cont_type);
tmp_rc = mfu_daos_get_cont_type(da->src_coh, &da->src_cont_type);
if (tmp_rc != 0) {
/* ideally, this should be the same for each process */
local_daos_error = true;
goto out;
}
/* Set the src api based on the container type */
tmp_rc = daos_set_api_cont_type(mfu_src_file, da->src_cont_type, da->api);
tmp_rc = mfu_daos_set_api_cont_type(mfu_src_file, da->src_cont_type, da->api);
if (tmp_rc != 0) {
local_daos_error = true;
goto out;
}
/* If using the DAOS API, we need to elevate permissions to RW for creating a snapshot */
if (mfu_src_file->type == DAOS) {
MPI_Barrier(MPI_COMM_WORLD);

tmp_rc = daos_cont_close(da->src_coh, NULL);
if (tmp_rc != 0) {
local_daos_error = true;
goto out;
}
da->src_coh = DAOS_HDL_INVAL;

/* Re-open with RW */
src_cont_open_flags = DAOS_COO_RW;
tmp_rc = mfu_daos_connect(rank, da, &da->src_pool, &da->src_cont,
&da->src_poh, &da->src_coh, false,
true, src_pool_connect_flags, create_cont, src_cont_open_flags, require_new_cont,
preserve, mfu_src_file, dst_cont_passed);
if (tmp_rc != 0) {
/* tmp_rc from mfu_daos_connect is collective */
local_daos_error = true;
goto out;
}
}

/* Sanity check before we create a new container */
if (da->src_cont_type != DAOS_PROP_CO_LAYOUT_POSIX) {
if (strcmp(da->src_path, "/") != 0) {
Expand All @@ -1622,7 +1653,7 @@ int daos_setup(
}

/* If we're using the DAOS API, the destination container cannot
* exist already, unless overriden by allow_exist_dst_cont. */
* exist already, unless overridden by allow_exist_dst_cont. */
if (mfu_src_file->type != POSIX && mfu_src_file->type != DFS && !da->allow_exist_dst_cont) {
require_new_cont = true;
}
Expand All @@ -1646,11 +1677,11 @@ int daos_setup(
create_cont = true;
/* do check that src is POSIX and since dst has a pool,
* then the dst *should* always be DFS, but this is not set
* on the destintaion until after daos_connect is called, and
* on the destintaion until after mfu_daos_connect is called, and
* we should read the properties *before* the container is
* created. We do not have the destination container type
* here yet which is why extra check is used then passed
* to daos_connect */
* to mfu_daos_connect */
if (da->daos_preserve) {
if (mfu_src_file->type == POSIX) {
preserve = true;
Expand All @@ -1662,34 +1693,25 @@ int daos_setup(
goto out;
}
}
if (same_pool) {
connect_pool = false;
tmp_rc = daos_connect(rank, da, &da->dst_pool, &da->dst_cont,
&da->src_poh, &da->dst_coh, false,
connect_pool, create_cont, require_new_cont,
preserve, mfu_src_file, dst_cont_passed);
} else {
connect_pool = true;
tmp_rc = daos_connect(rank, da, &da->dst_pool, &da->dst_cont,
tmp_rc = mfu_daos_connect(rank, da, &da->dst_pool, &da->dst_cont,
&da->dst_poh, &da->dst_coh, false,
connect_pool, create_cont, require_new_cont,
preserve, mfu_src_file, dst_cont_passed);
}
true, dst_pool_connect_flags, create_cont, dst_cont_open_flags,
require_new_cont, preserve, mfu_src_file, dst_cont_passed);
if (tmp_rc != 0) {
/* tmp_rc from daos_connect is collective */
/* tmp_rc from mfu_daos_connect is collective */
local_daos_error = true;
goto out;
}
/* Get the container type */
tmp_rc = daos_get_cont_type(da->dst_coh, &da->dst_cont_type);
tmp_rc = mfu_daos_get_cont_type(da->dst_coh, &da->dst_cont_type);
if (tmp_rc != 0) {
/* ideally, this should be the same for each process */
local_daos_error = true;
goto out;
}
if (have_dst) {
/* Set the dst api based on the container type */
tmp_rc = daos_set_api_cont_type(mfu_dst_file, da->dst_cont_type, da->api);
tmp_rc = mfu_daos_set_api_cont_type(mfu_dst_file, da->dst_cont_type, da->api);
if (tmp_rc != 0) {
local_daos_error = true;
goto out;
Expand All @@ -1716,11 +1738,7 @@ int daos_setup(
if (have_dst) {
/* Mount destination DFS container */
if (mfu_dst_file->type == DFS) {
if (same_pool) {
tmp_rc = mfu_dfs_mount(mfu_dst_file, &da->src_poh, &da->dst_coh);
} else {
tmp_rc = mfu_dfs_mount(mfu_dst_file, &da->dst_poh, &da->dst_coh);
}
tmp_rc = mfu_dfs_mount(mfu_dst_file, &da->dst_poh, &da->dst_coh);
if (tmp_rc != 0) {
local_daos_error = true;
goto out;
Expand Down Expand Up @@ -1767,8 +1785,6 @@ int daos_cleanup(
int rank;
MPI_Comm_rank(MPI_COMM_WORLD, &rank);

bool same_pool = (strcmp(da->src_pool, da->dst_pool) == 0);

/* Destroy source snapshot */
if (rank == 0 && da->src_epc != 0) {
tmp_rc = mfu_daos_destroy_snap(da->src_coh, da->src_epc);
Expand Down Expand Up @@ -1837,7 +1853,7 @@ int daos_cleanup(
}

/* Close destination pool */
if (daos_handle_is_valid(da->dst_poh) && !same_pool) {
if (daos_handle_is_valid(da->dst_poh)) {
tmp_rc = daos_pool_disconnect(da->dst_poh, NULL);
if (tmp_rc != 0) {
MFU_LOG(MFU_LOG_ERR, "Failed to disconnect from destination pool "DF_RC, DP_RC(tmp_rc));
Expand Down
4 changes: 3 additions & 1 deletion src/common/mfu_daos.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ int daos_parse_path(

/* connect to DAOS pool,
* and then open container */
int daos_connect(
int mfu_daos_connect(
int rank,
daos_args_t* da,
char (*pool)[],
Expand All @@ -310,7 +310,9 @@ int daos_connect(
daos_handle_t* coh,
bool force_serialize,
bool connect_pool,
unsigned int pool_connect_flags,
bool create_cont,
unsigned int cont_open_flags,
bool require_new_cont,
bool preserve,
mfu_file_t* mfu_src_file,
Expand Down
8 changes: 4 additions & 4 deletions src/daos-serialize/daos-serialize.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,10 @@ int main(int argc, char** argv)
rc = 1;
}

tmp_rc = daos_connect(rank, daos_args, &daos_args->src_pool,
&daos_args->src_cont, &daos_args->src_poh,
&daos_args->src_coh, force_serialize, true,
false, false, false, NULL, true);
tmp_rc = mfu_daos_connect(rank, daos_args, &daos_args->src_pool,
&daos_args->src_cont, &daos_args->src_poh,
&daos_args->src_coh, force_serialize, true, DAOS_PC_RW,
false, DAOS_COO_RW, false, false, NULL, true);
if (tmp_rc != 0) {
daos_fini();
mfu_finalize();
Expand Down

0 comments on commit 0a4c530

Please sign in to comment.