Skip to content

Conversation

@amarts
Copy link
Owner

@amarts amarts commented May 5, 2020

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

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>
Copy link
Owner Author

@amarts amarts left a 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)
Copy link
Owner Author

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) {
Copy link
Owner Author

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.

break;

if (gf_string2int(de->d_name, &pid) != -1 && pid >= 0) {
if (NOT_ERROR(gf_string2int(de->d_name, &pid) != -1 && pid)) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is wrong!


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)) {
Copy link
Owner Author

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) \
Copy link
Owner Author

Choose a reason for hiding this comment

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

think of using THIS.


while (pos != head) {
if (compare(new, pos) >= 0)
if (NOT_ERROR(compare(new, pos)))
Copy link
Owner Author

Choose a reason for hiding this comment

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

no need for this!

PRINT_SIZE_CHECK(ret, out, strsize);

for ((i = size - 3); i >= 0; i--) {
for (NOT_ERROR((i = size - 3); i); i--) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

fix this

ret = gf_store_sync_direntry(tmppath);
out:
if (shandle && shandle->tmp_fd >= 0) {
if (NOT_ERROR(shandle && shandle->tmp_fd)) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

fix this

}
out:
if (shandle && shandle->tmp_fd >= 0) {
if (NOT_ERROR(shandle && shandle->tmp_fd)) {
Copy link
Owner Author

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))
Copy link
Owner Author

Choose a reason for hiding this comment

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

fix this

Copy link
Owner Author

@amarts amarts left a 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)) {
Copy link
Owner Author

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>
@amarts amarts force-pushed the errcode branch 2 times, most recently from 8808225 to d3051dc Compare May 6, 2020 06:07
Change-Id: I34711960e57a54a726307e0ee17df5ecc9b36766
Signed-off-by: Amar Tumballi <amar@kadalu.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants