Commit 4137577a authored by Alex Elder's avatar Alex Elder Committed by Sage Weil

libceph: clean up skipped message logic

In ceph_con_in_msg_alloc() it is possible for a connection's
alloc_msg method to indicate an incoming message should be skipped.
By default, read_partial_message() initializes the skip variable
to 0 before it gets provided to ceph_con_in_msg_alloc().

The osd client, mon client, and mds client each supply an alloc_msg
method.  The mds client always assigns skip to be 0.

The other two leave the skip value of as-is, or assigns it to zero,
except:
    - if no (osd or mon) request having the given tid is found, in
      which case skip is set to 1 and NULL is returned; or
    - in the osd client, if the data of the reply message is not
      adequate to hold the message to be read, it assigns skip
      value 1 and returns NULL.
So the returned message pointer will always be NULL if skip is ever
non-zero.

Clean up the logic a bit in ceph_con_in_msg_alloc() to make this
state of affairs more obvious.  Add a comment explaining how a null
message pointer can mean either a message that should be skipped or
a problem allocating a message.

This resolves:
    http://tracker.ceph.com/issues/4324Reported-by: default avatarGreg Farnum <greg@inktank.com>
Signed-off-by: default avatarAlex Elder <elder@inktank.com>
Reviewed-by: default avatarGreg Farnum <greg@inktank.com>
parent 0fff87ec
...@@ -2819,18 +2819,21 @@ static int ceph_con_in_msg_alloc(struct ceph_connection *con, int *skip) ...@@ -2819,18 +2819,21 @@ static int ceph_con_in_msg_alloc(struct ceph_connection *con, int *skip)
ceph_msg_put(msg); ceph_msg_put(msg);
return -EAGAIN; return -EAGAIN;
} }
if (msg) {
BUG_ON(*skip);
con->in_msg = msg; con->in_msg = msg;
if (con->in_msg) {
con->in_msg->con = con->ops->get(con); con->in_msg->con = con->ops->get(con);
BUG_ON(con->in_msg->con == NULL); BUG_ON(con->in_msg->con == NULL);
} } else {
if (*skip) { /*
con->in_msg = NULL; * Null message pointer means either we should skip
* this message or we couldn't allocate memory. The
* former is not an error.
*/
if (*skip)
return 0; return 0;
} con->error_msg = "error allocating memory for incoming message";
if (!con->in_msg) {
con->error_msg =
"error allocating memory for incoming message";
return -ENOMEM; return -ENOMEM;
} }
memcpy(&con->in_msg->hdr, &con->in_hdr, sizeof(con->in_hdr)); memcpy(&con->in_msg->hdr, &con->in_hdr, sizeof(con->in_hdr));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment