>Are you hitting the non async code path? Did you hit the
>iscsid_req_by_rec call? How many targets or portals were you logging into?
No, I never hit iscsid_req_by_rec call. I was just reading the codes
of iscsiadm and thought that there is less consideration when calling
iscsid_req_by_rec() than iscsid_req_by_rec_async().
>If iscsid_req_by_rec fails, then won't we log an error here?
Sorry, my point was very unclear for you and my patch was also not good.
I basically have two points that I want to improve.
No1, checking the return value of iscisd_request() when calling
iscsid_req_by_rec().
No2, outputting login success message even when calling iscsid_req_by_rec().
In the origicanl code of login_portal() below,
if (async_req)
rc = iscsid_req_by_rec_async(MGMT_IPC_SESSION_LOGIN,
rec, &fd);
else
rc = iscsid_req_by_rec(MGMT_IPC_SESSION_LOGIN, rec);
/* we raced with another app or instance of iscsiadm */
if (rc == MGMT_IPC_ERR_EXISTS) {
if (async_req)
free(async_req);
return 0;
} else if (rc) {
iscsid_handle_error(rc);
if (async_req)
free(async_req);
return ENOTCONN;
}
the return value of iscsid_request() is checked when calling
iscsid_req_by_rec_async().
However, the same return value is not checked when calling iscsid_req_by_rec().
What you check here with rc = iscsid_req_by_rec() is not the return value of
iscsid_request(), but that of iscsid_req_wait() as you can see in the
codes below.
int iscsid_req_by_rec(iscsiadm_cmd_e cmd, node_rec_t *rec)
{
int err, fd;
err = iscsid_req_by_rec_async(cmd, rec, &fd);
if (err)
return err;
return iscsid_req_wait(cmd, fd);
}
So, I check the return value of iscsid_request() inside iscsid_req_by_rec().
(I actually did not put the codes to achive this in the second patch of util.c.
and maybe that made you confused...
I will send a new patch of util.c that includes below codes.)
The code below will achive my point No1.
int iscsid_req_by_rec(iscsiadm_cmd_e cmd, node_rec_t *rec)
{
int err, fd;
err = iscsid_req_by_rec_async(cmd, rec, &fd);
if (err == MGMT_IPC_ERR_EXISTS) {
return 0;
} else if (err) {
iscsid_handle_error(err);
return ENOTCONN;
}
I also check the return value of iscsid_req_wait() inside
iscsid_req_by_rec() and
outputting logs of login status. This will achive my point No2.
err = iscsid_req_wait(cmd, fd);
if (err) {
log_error("Could not login to [iface: %s, target: %s, "
"portal: %s,%d]: ", rec->iface.name,
rec->name, rec->conn[0].address,
rec->conn[0].port);
} else
printf("Login to [iface: %s, target: %s, portal: "
"%s,%d]: successful\n", rec->iface.name,
rec->name, rec->conn[0].address,
rec->conn[0].port);
return err;
}
Since we checked the return value of iscsid_request() inside
iscsid_req_by_rec(),
what we need to check is only the return value of iscsid_req_wait() of
iscsid_req_by_rec().
So, patched login_portal() should be something like this now.
if (async_req) {
rc = iscsid_req_by_rec_async(MGMT_IPC_SESSION_LOGIN,
rec, &fd);
if (rc == MGMT_IPC_ERR_EXISTS) {
if (async_req)
free(async_req);
return 0;
} else if (rc) {
iscsid_handle_error(rc);
if (async_req)
free(async_req);
return ENOTCONN;
}
list_add_tail(&async_req->list, list);
async_req->fd = fd;
async_req->data = rec;
return 0;
} else {
rc = iscsid_req_by_rec(MGMT_IPC_SESSION_LOGIN, rec);
if (rc) {
iscsid_handle_error(rc);
return ENOTCONN;
} else
return rc;
}
}
I know that iscsid_req_by_rec() is almost never called in the reality.
But, I just thought the codes looks little better if we take care of
iscsid_req_by_rec() more.
This patch unnecessary? Unclear? Give me your frank opinion, Mike.
Kim.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~----------~----~----~----~------~----~------~--~---
