| Message ID | 508577F5.3020102@inktank.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
I really feel like we ought to root-cause this before we patch the kernel client. Something isn't working the way we expect. On 10/22/2012 09:44 AM, Alex Elder wrote: > There is a call in rbd_dev_image_id() to rbd_req_sync_exec() > to get the image id for an image. Despite the "get_id" class > method only returning 0 on success, I am getting back a positive > value (I think the number of bytes returned with the call). > > That may or may not be how rbd_req_sync_exec() is supposed to > behave, but zeroing the return value if successful makes it moot > and makes this path through the code work as desired. > > Do the same in rbd_dev_v2_object_prefix(). > > Signed-off-by: Alex Elder <elder@inktank.com> > --- > drivers/block/rbd.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c > index cf5d109..65e9f1f 100644 > --- a/drivers/block/rbd.c > +++ b/drivers/block/rbd.c > @@ -2169,6 +2169,7 @@ static int rbd_dev_v2_object_prefix(struct > rbd_device *rbd_dev) > dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret); > if (ret < 0) > goto out; > + ret = 0; /* rbd_req_sync_exec() can return positive */ > > p = reply_buf; > rbd_dev->header.object_prefix = ceph_extract_encoded_string(&p, > @@ -2862,6 +2863,7 @@ static int rbd_dev_image_id(struct rbd_device > *rbd_dev) > dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret); > if (ret < 0) > goto out; > + ret = 0; /* rbd_req_sync_exec() can return positive */ > > p = response; > rbd_dev->image_id = ceph_extract_encoded_string(&p, > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
OK, assuming the comment is changed to reflect the "we expect a data length back here" indication, Alex explained this offline and I agree. On 10/22/2012 10:18 AM, Dan Mick wrote: > I really feel like we ought to root-cause this before we patch the > kernel client. Something isn't working the way we expect. > > On 10/22/2012 09:44 AM, Alex Elder wrote: >> There is a call in rbd_dev_image_id() to rbd_req_sync_exec() >> to get the image id for an image. Despite the "get_id" class >> method only returning 0 on success, I am getting back a positive >> value (I think the number of bytes returned with the call). >> >> That may or may not be how rbd_req_sync_exec() is supposed to >> behave, but zeroing the return value if successful makes it moot >> and makes this path through the code work as desired. >> >> Do the same in rbd_dev_v2_object_prefix(). >> >> Signed-off-by: Alex Elder <elder@inktank.com> >> --- >> drivers/block/rbd.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c >> index cf5d109..65e9f1f 100644 >> --- a/drivers/block/rbd.c >> +++ b/drivers/block/rbd.c >> @@ -2169,6 +2169,7 @@ static int rbd_dev_v2_object_prefix(struct >> rbd_device *rbd_dev) >> dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret); >> if (ret < 0) >> goto out; >> + ret = 0; /* rbd_req_sync_exec() can return positive */ >> >> p = reply_buf; >> rbd_dev->header.object_prefix = ceph_extract_encoded_string(&p, >> @@ -2862,6 +2863,7 @@ static int rbd_dev_image_id(struct rbd_device >> *rbd_dev) >> dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret); >> if (ret < 0) >> goto out; >> + ret = 0; /* rbd_req_sync_exec() can return positive */ >> >> p = response; >> rbd_dev->image_id = ceph_extract_encoded_string(&p, >> -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Now that we tracked down the cause of the positive return value to the client side the commit message could be updated. Reviewed-by: Josh Durgin <josh.durgin@inktank.com> On 10/22/2012 09:44 AM, Alex Elder wrote: > There is a call in rbd_dev_image_id() to rbd_req_sync_exec() > to get the image id for an image. Despite the "get_id" class > method only returning 0 on success, I am getting back a positive > value (I think the number of bytes returned with the call). > > That may or may not be how rbd_req_sync_exec() is supposed to > behave, but zeroing the return value if successful makes it moot > and makes this path through the code work as desired. > > Do the same in rbd_dev_v2_object_prefix(). > > Signed-off-by: Alex Elder <elder@inktank.com> > --- > drivers/block/rbd.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c > index cf5d109..65e9f1f 100644 > --- a/drivers/block/rbd.c > +++ b/drivers/block/rbd.c > @@ -2169,6 +2169,7 @@ static int rbd_dev_v2_object_prefix(struct > rbd_device *rbd_dev) > dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret); > if (ret < 0) > goto out; > + ret = 0; /* rbd_req_sync_exec() can return positive */ > > p = reply_buf; > rbd_dev->header.object_prefix = ceph_extract_encoded_string(&p, > @@ -2862,6 +2863,7 @@ static int rbd_dev_image_id(struct rbd_device > *rbd_dev) > dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret); > if (ret < 0) > goto out; > + ret = 0; /* rbd_req_sync_exec() can return positive */ > > p = response; > rbd_dev->image_id = ceph_extract_encoded_string(&p, > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index cf5d109..65e9f1f 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -2169,6 +2169,7 @@ static int rbd_dev_v2_object_prefix(struct rbd_device *rbd_dev) dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret); if (ret < 0) goto out; + ret = 0; /* rbd_req_sync_exec() can return positive */ p = reply_buf;
There is a call in rbd_dev_image_id() to rbd_req_sync_exec() to get the image id for an image. Despite the "get_id" class method only returning 0 on success, I am getting back a positive value (I think the number of bytes returned with the call). That may or may not be how rbd_req_sync_exec() is supposed to behave, but zeroing the return value if successful makes it moot and makes this path through the code work as desired. Do the same in rbd_dev_v2_object_prefix(). Signed-off-by: Alex Elder <elder@inktank.com> --- drivers/block/rbd.c | 2 ++ 1 file changed, 2 insertions(+) rbd_dev->header.object_prefix = ceph_extract_encoded_string(&p, @@ -2862,6 +2863,7 @@ static int rbd_dev_image_id(struct rbd_device *rbd_dev) dout("%s: rbd_req_sync_exec returned %d\n", __func__, ret); if (ret < 0) goto out; + ret = 0; /* rbd_req_sync_exec() can return positive */ p = response; rbd_dev->image_id = ceph_extract_encoded_string(&p,