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.

Sign up for GitHub

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

Jump to bottom

DAOS-14547: Source pool/cont only needs read permission #568

Merged
merged 1 commit into from
Apr 11, 2024
Merged

DAOS-14547: Source pool/cont only needs read permission #568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
122 changes: 69 additions & 53 deletions src/common/mfu_daos.c
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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