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

daos: copy user attrs and cont props on create #478

Merged
merged 1 commit into from
May 3, 2021

Conversation

daltonbohning
Copy link
Collaborator

When creating a new DAOS container, copy the user attributes
and container properties from the src container.

Signed-off-by: Dalton Bohning daltonx.bohning@intel.com

@daltonbohning daltonbohning added the daos DAOS-related issues label Apr 5, 2021
@daltonbohning daltonbohning requested a review from dsikich April 5, 2021 20:18
@daltonbohning daltonbohning self-assigned this Apr 5, 2021
props->dpp_entries[12].dpe_type = DAOS_PROP_CO_OWNER_GROUP;
props->dpp_entries[13].dpe_type = DAOS_PROP_CO_DEDUP;
props->dpp_entries[14].dpe_type = DAOS_PROP_CO_DEDUP_THRESHOLD;
props->dpp_entries[15].dpe_type = DAOS_PROP_CO_ALLOCED_OID;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daltonbohning this property should not be set for POSIX containers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dsikich Thanks! Specifically, it should not be set when we use DFS. I think we should still set it for POSIX -> POSIX with the Object API, just not DFS. It seemed the simplest way was to just pass a get_oid flag

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dsikich Thanks! Specifically, it should not be set when we use DFS. I think we should still set it for POSIX -> POSIX with the Object API, just not DFS. It seemed the simplest way was to just pass a get_oid flag

@daltonbohning yeah, I agree 👍

When creating a new DAOS container, copy the user attributes
and container properties from the src container.

Signed-off-by: Dalton Bohning <daltonx.bohning@intel.com>
Copy link

@dsikich dsikich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daltonbohning looks good. There will eventually be a DFS version of serialization/deserialization, so I can just update the flag when the container is serialized for that case whenever it gets added.

@dsikich
Copy link

dsikich commented Apr 28, 2021

@adammoody can you take a look at this one? I am working on an option/feature that builds on this PR.

Comment on lines +768 to +773
rc = daos_cont_list_attr(coh, name_buf, &total_size, NULL);
if (rc != 0) {
MFU_LOG(MFU_LOG_ERR, "failed to list user attributes "DF_RC, DP_RC(rc));
rc = 1;
goto out;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious about the daos_cost_list_attr interface. Is total_size used as an input parameter here to specify the buffer size?

If so, I'm guessing this function might return an error if the attribute list size has increased, since the first time this was called.

And I suspect it's valid for the attribute list size to be less than what it was?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adammoody I believe it is valid for it to be less than it was, but I will note that we should return an error if the attribute size has increased since the first time it was called.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adammoody @dsikich Yes, it is possible for the actual size to change between the first call to daos_cont_list_attr (which just gets the size) and the second call (which gets the names).
https://github.com/daos-stack/daos/blob/011b78cb28aaf162af635f37d1fcabc62076ef0b/src/include/daos_cont.h#L382-L399
I think this is how it works, based on other DAOS paradigms:

  • If size is less than the size passed in
    • The output size set accordingly, since it's passed by reference, and the buffer contains all the data
  • If size is more than the size passed in
    • The output size is set accordingly, and the buffer is truncated

The "foolproof" way to handle this would be to have a retry loop, and realloc and fetch every time the size is more than the size of the buffer.

@adammoody
Copy link
Member

Thanks @daltonbohning and @dsikich . This looks good to me.

@dsikich dsikich merged commit bdf06d4 into hpc:master May 3, 2021
@daltonbohning
Copy link
Collaborator Author

Thanks @daltonbohning and @dsikich . This looks good to me.

Thanks @adammoody !

@daltonbohning daltonbohning deleted the dbohninx-cont-meta branch May 3, 2021 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
daos DAOS-related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants