-
Notifications
You must be signed in to change notification settings - Fork 301
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-15388 tools: daos fs copy: error when src is dst #13962
Conversation
Ticket title is 'daos fs copy should detect copying a directory into itself' |
ad6925d
to
847fd97
Compare
Test stage Functional Hardware Medium completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-13962/2/display/redirect |
Test stage Functional Hardware Medium Verbs Provider completed with status FAILURE. https://build.hpdd.intel.com/job/daos-stack/job/daos/job/PR-13962/2/display/redirect |
847fd97
to
1daaa13
Compare
Test stage Functional Hardware Medium Verbs Provider completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-13962/5/execution/node/836/log |
1daaa13
to
9f9d514
Compare
Test stage Functional on EL 8.8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-13962/6/testReport/ |
e0ba885
to
2d0870f
Compare
Test stage Functional Hardware Medium completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-13962/9/testReport/ |
Test stage Functional Hardware Medium Verbs Provider completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-13962/9/execution/node/1540/log |
Features: daos_fs_copy Check if the source and destination are the same. Check if the source is a subset of the destination. Required-githooks: true Signed-off-by: Dalton Bohning <dalton.bohning@intel.com>
2d0870f
to
d4cc14f
Compare
Test stage Functional Hardware Medium Verbs Provider completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-13962/10/execution/node/1435/log |
src/utils/daos_hdlr.c
Outdated
} else { | ||
char *tmp_src = NULL; | ||
|
||
D_ASPRINTF(tmp_src, "%s/", src_path); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should check tmp_src != NULL, but do you really need an extra allocation and a tmp string here?
why not just compare:
(strncmp(src_path, dst_path, src_path_len) == 0 && dst_path[src_path_len] == "/") ?
im not sure though if this is correct though, because consider
what if src == dst == /d1/d2
I see that the same as
src == /d1/d2 and dst == /d1/d2/
but the result is different in this case, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively you could stat the src and dest and if the st_dev
and st_ino
fields are the same then they two are the same. This has the advantage that it'll work if you access the same tree via different paths, full vs relative or in the presence of symlinks etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just compare:
(strncmp(src_path, dst_path, src_path_len) == 0 && dst_path[src_path_len] == "/") ?
You're right. I tested locally and that works the same (with minor tweaks).
Per my other comment to @wiliamhuang: #13962 (comment)
At this point, dst_path
is either /
, has a /
at the same location as src_path
, or src_path
is not a subpath of dst_path
.
For my previous implementation and your suggestion, all four of these error as expected:
daos fs copy --src src --dst src
daos fs copy --src src --dst src/
daos fs copy --src src/ --dst src/
daos fs copy --src src/ --dst src
I pushed your suggestion and added more comments here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively you could stat the src and dest and if the st_dev and st_ino fields are the same
That would be more complete, but I think we'd have to do some recursive parse + stat to detect subpaths. E.g.
src_path = /some/deep/dir
dst_path = /some/deep/dir/that/keeps/going
We'd have to stat like this to figure it out (unless I'm missing something)
src - /some/deep/dir
dst - /some/deep/dir/that/keeps/going
dst - /some/deep/dir/that/keeps
dst - /some/deep/dir/that
dst - /some/deep/dir
Features: daos_fs_copy Required-githooks: true Signed-off-by: Dalton Bohning <dalton.bohning@intel.com>
Features: daos_fs_copy Required-githooks: true Signed-off-by: Dalton Bohning <dalton.bohning@intel.com>
Features: daos_fs_copy Required-githooks: true Signed-off-by: Dalton Bohning <dalton.bohning@intel.com>
Test stage Functional Hardware Medium completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-13962/14/execution/node/616/log |
Features: daos_fs_copy
Check if the source and destination are the same.
Check if the source is a subset of the destination.
Required-githooks: true
Before requesting gatekeeper:
Features:
(orTest-tag*
) commit pragma was used or there is a reason documented that there are no appropriate tags for this PR.Gatekeeper: