-
Notifications
You must be signed in to change notification settings - Fork 1
errorcode: more information as part of return code #17
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
base: gmaster
Are you sure you want to change the base?
Conversation
The details of how to achieve this is described in the issue, please refer. Idea is, utilze those unused 31bits in 'op_ret' (ie, return value) field for identifying the exact error (or as close as possible). Plan is to handle fop by fop for this change. Best way to see the result is by changing the lookup() fop, as that actually is expected to return failure most times than success. use 'IS_ERROR(ret)' instead of (ret < 0) or (ret == -1) cases. Updates: #280 Change-Id: If6e22e7aa170f3800c0f29156c4bfc700f790fef Signed-off-by: Amar Tumballi <amar@kadalu.io>
amarts
left a comment
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.
Round 1 of 100 files!
| } | ||
|
|
||
| if (ret == -1) | ||
| if (ret < 0) |
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.
revisit this page, and change things to IS_ERROR()
| pida[0] = pid; | ||
| ret = prociter(find_gsyncd, pida); | ||
| if (ret == -1 || pida[1] == -1) { | ||
| if (IS_ERROR(ret) || pida[1] == -1) { |
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.
need IS_ERROR() on plda too.
geo-replication/src/procdiggy.c
Outdated
| break; | ||
|
|
||
| if (gf_string2int(de->d_name, &pid) != -1 && pid >= 0) { | ||
| if (NOT_ERROR(gf_string2int(de->d_name, &pid) != -1 && pid)) { |
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.
This is wrong!
libglusterfs/src/cluster-syncop.c
Outdated
|
|
||
| for (i = 0; i < numsubvols; i++) { | ||
| if (replies[i].valid && replies[i].op_ret >= 0) { | ||
| if (NOT_ERROR(replies[i].valid && replies[i].op_ret)) { |
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.
Fix it
| int32_t | ||
| gf_error_to_errno(int32_t error); | ||
|
|
||
| #define SET_ERROR(xlidx, xlid, reason) \ |
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.
think of using THIS.
libglusterfs/src/glusterfs/list.h
Outdated
|
|
||
| while (pos != head) { | ||
| if (compare(new, pos) >= 0) | ||
| if (NOT_ERROR(compare(new, pos))) |
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.
no need for this!
libglusterfs/src/logging.c
Outdated
| PRINT_SIZE_CHECK(ret, out, strsize); | ||
|
|
||
| for ((i = size - 3); i >= 0; i--) { | ||
| for (NOT_ERROR((i = size - 3); i); i--) { |
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.
fix this
libglusterfs/src/store.c
Outdated
| ret = gf_store_sync_direntry(tmppath); | ||
| out: | ||
| if (shandle && shandle->tmp_fd >= 0) { | ||
| if (NOT_ERROR(shandle && shandle->tmp_fd)) { |
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.
fix this
libglusterfs/src/store.c
Outdated
| } | ||
| out: | ||
| if (shandle && shandle->tmp_fd >= 0) { | ||
| if (NOT_ERROR(shandle && shandle->tmp_fd)) { |
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.
same here
| if (IS_ERROR(read_subvol)) { | ||
| ret = afr_inode_split_brain_choice_get(inode, this, &spb_choice); | ||
| if ((ret == 0) && spb_choice >= 0) | ||
| if (NOT_ERROR((ret == 0) && spb_choice)) |
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.
fix this
amarts
left a comment
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.
another 20files
| case AFR_FAV_CHILD_BY_SIZE: | ||
| fav_child = afr_sh_fav_by_size(this, replies, inode); | ||
| if (policy_str && fav_child >= 0) { | ||
| if (NOT_ERROR(policy_str && fav_child)) { |
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.
fix here and below
* for cases of ((ret) >= 0) Change-Id: I275f77d91262a856f24e389f3366180cac4a5cbb Signed-off-by: Amar Tumballi <amar@kadalu.io>
8808225 to
d3051dc
Compare
Change-Id: I34711960e57a54a726307e0ee17df5ecc9b36766 Signed-off-by: Amar Tumballi <amar@kadalu.io>
The details of how to achieve this is described in the issue, please
refer. Idea is, utilze those unused 31bits in 'op_ret' (ie, return
value) field for identifying the exact error (or as close as possible).
Plan is to handle fop by fop for this change. Best way to see the result
is by changing the lookup() fop, as that actually is expected to return
failure most times than success.
use 'IS_ERROR(ret)' instead of (ret < 0) or (ret == -1) cases.
Updates: #280
Change-Id: If6e22e7aa170f3800c0f29156c4bfc700f790fef
Signed-off-by: Amar Tumballi amar@kadalu.io