>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 open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~----------~----~----~----~------~----~------~--~---