* [PATCH 0/2] rbd: two bugs @ 2012-10-22 16:43 Alex Elder 2012-10-22 16:44 ` [PATCH 1/2] rbd: zero return code in rbd_dev_image_id() Alex Elder 2012-10-22 16:44 ` [PATCH 2/2] rbd: fix read-only option name Alex Elder 0 siblings, 2 replies; 7+ messages in thread From: Alex Elder @ 2012-10-22 16:43 UTC (permalink / raw) To: ceph-devel This series fixes two bugs, one that affects rbd format 2 images only, and one that fixes an inadvertent change in an earlier commit. -Alex [PATCH 1/2] rbd: zero return code in rbd_dev_image_id() [PATCH 2/2] rbd: fix read-only option name ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] rbd: zero return code in rbd_dev_image_id() 2012-10-22 16:43 [PATCH 0/2] rbd: two bugs Alex Elder @ 2012-10-22 16:44 ` Alex Elder 2012-10-22 17:18 ` Dan Mick 2012-10-24 16:59 ` Josh Durgin 2012-10-22 16:44 ` [PATCH 2/2] rbd: fix read-only option name Alex Elder 1 sibling, 2 replies; 7+ messages in thread From: Alex Elder @ 2012-10-22 16:44 UTC (permalink / raw) To: ceph-devel 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, -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] rbd: zero return code in rbd_dev_image_id() 2012-10-22 16:44 ` [PATCH 1/2] rbd: zero return code in rbd_dev_image_id() Alex Elder @ 2012-10-22 17:18 ` Dan Mick 2012-10-22 20:00 ` Dan Mick 2012-10-24 16:59 ` Josh Durgin 1 sibling, 1 reply; 7+ messages in thread From: Dan Mick @ 2012-10-22 17:18 UTC (permalink / raw) To: Alex Elder; +Cc: ceph-devel 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, > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] rbd: zero return code in rbd_dev_image_id() 2012-10-22 17:18 ` Dan Mick @ 2012-10-22 20:00 ` Dan Mick 0 siblings, 0 replies; 7+ messages in thread From: Dan Mick @ 2012-10-22 20:00 UTC (permalink / raw) To: Alex Elder; +Cc: ceph-devel 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, >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] rbd: zero return code in rbd_dev_image_id() 2012-10-22 16:44 ` [PATCH 1/2] rbd: zero return code in rbd_dev_image_id() Alex Elder 2012-10-22 17:18 ` Dan Mick @ 2012-10-24 16:59 ` Josh Durgin 1 sibling, 0 replies; 7+ messages in thread From: Josh Durgin @ 2012-10-24 16:59 UTC (permalink / raw) To: Alex Elder; +Cc: ceph-devel 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, > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] rbd: fix read-only option name 2012-10-22 16:43 [PATCH 0/2] rbd: two bugs Alex Elder 2012-10-22 16:44 ` [PATCH 1/2] rbd: zero return code in rbd_dev_image_id() Alex Elder @ 2012-10-22 16:44 ` Alex Elder 2012-10-24 17:02 ` Josh Durgin 1 sibling, 1 reply; 7+ messages in thread From: Alex Elder @ 2012-10-22 16:44 UTC (permalink / raw) To: ceph-devel The name of the "read-only" mapping option was inadvertently changed in this commit: f84344f3 rbd: separate mapping info in rbd_dev Revert that hunk to return it to what it should be. Signed-off-by: Alex Elder <elder@inktank.com> --- drivers/block/rbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 65e9f1f..d032883 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -397,7 +397,7 @@ enum { static match_table_t rbd_opts_tokens = { /* int args above */ /* string args above */ - {Opt_read_only, "mapping.read_only"}, + {Opt_read_only, "read_only"}, {Opt_read_only, "ro"}, /* Alternate spelling */ {Opt_read_write, "read_write"}, {Opt_read_write, "rw"}, /* Alternate spelling */ -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] rbd: fix read-only option name 2012-10-22 16:44 ` [PATCH 2/2] rbd: fix read-only option name Alex Elder @ 2012-10-24 17:02 ` Josh Durgin 0 siblings, 0 replies; 7+ messages in thread From: Josh Durgin @ 2012-10-24 17:02 UTC (permalink / raw) To: Alex Elder; +Cc: ceph-devel Reviewed-by: Josh Durgin <josh.durgin@inktank.com> On 10/22/2012 09:44 AM, Alex Elder wrote: > The name of the "read-only" mapping option was inadvertently changed > in this commit: > > f84344f3 rbd: separate mapping info in rbd_dev > > Revert that hunk to return it to what it should be. > > Signed-off-by: Alex Elder <elder@inktank.com> > --- > drivers/block/rbd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c > index 65e9f1f..d032883 100644 > --- a/drivers/block/rbd.c > +++ b/drivers/block/rbd.c > @@ -397,7 +397,7 @@ enum { > static match_table_t rbd_opts_tokens = { > /* int args above */ > /* string args above */ > - {Opt_read_only, "mapping.read_only"}, > + {Opt_read_only, "read_only"}, > {Opt_read_only, "ro"}, /* Alternate spelling */ > {Opt_read_write, "read_write"}, > {Opt_read_write, "rw"}, /* Alternate spelling */ > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-10-24 17:02 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-10-22 16:43 [PATCH 0/2] rbd: two bugs Alex Elder 2012-10-22 16:44 ` [PATCH 1/2] rbd: zero return code in rbd_dev_image_id() Alex Elder 2012-10-22 17:18 ` Dan Mick 2012-10-22 20:00 ` Dan Mick 2012-10-24 16:59 ` Josh Durgin 2012-10-22 16:44 ` [PATCH 2/2] rbd: fix read-only option name Alex Elder 2012-10-24 17:02 ` Josh Durgin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.