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 <[email protected]>

---
 drivers/block/rbd.c |    2 ++
 1 file changed, 2 insertions(+)

drivers/block/rbd.c +0 -0

index cf5d109..65e9f1f 100644
@@ -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,

Signature · 1 line

1.7.9.5

Dan Mick

2012-10-22 17:18 UTC #2 lore raw

Re: [PATCH 1/2] rbd: zero return code in rbd_dev_image_id()

I really feel like we ought to root-cause this before we patch the kernel client. Something isn't working the way we expect.

Quoted 39 lines · Alex Elder

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 <[email protected]>
---
  drivers/block/rbd.c |    2 ++
  1 file changed, 2 insertions(+)
drivers/block/rbd.c +1 -0
index cf5d109..65e9f1f 100644
@@ -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,
drivers/block/rbd.c +1 -0
@@ -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,

Dan Mick

2012-10-22 20:00 UTC #3 lore raw

Re: [PATCH 1/2] rbd: zero return code in rbd_dev_image_id()

OK, assuming the comment is changed to reflect the "we expect a data length back here" indication, Alex explained this offline and I agree.

Quoted 43 lines · Dan Mick

I really feel like we ought to root-cause this before we patch the kernel client. Something isn't working the way we expect.
Quoted 39 lines · Alex Elder
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 <[email protected]>
---
  drivers/block/rbd.c |    2 ++
  1 file changed, 2 insertions(+)
drivers/block/rbd.c +1 -0
index cf5d109..65e9f1f 100644
@@ -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,
drivers/block/rbd.c +1 -0
@@ -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,

Josh Durgin

Reviewed-by on v1 1/2

2012-10-24 16:59 UTC #4 lore raw

Re: [PATCH 1/2] rbd: zero return code in rbd_dev_image_id()

In reply to Alex Elder

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 <[email protected]>

Quoted 39 lines · Alex Elder

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 <[email protected]>
---
  drivers/block/rbd.c |    2 ++
  1 file changed, 2 insertions(+)
drivers/block/rbd.c +1 -0
index cf5d109..65e9f1f 100644
@@ -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,
drivers/block/rbd.c +1 -0
@@ -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,

Alex Elder

author

2012-10-22 16:45 UTC #5 lore raw

PATCH 2/2 rbd: fix read-only option name

In reply to Alex Elder

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 <[email protected]>

---
 drivers/block/rbd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/block/rbd.c +1 -1

index 65e9f1f..d032883 100644
@@ -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 */

Signature · 1 line

1.7.9.5

Josh Durgin

Reviewed-by on v1 2/2

2012-10-24 17:02 UTC #6 lore raw

Re: [PATCH 2/2] rbd: fix read-only option name

Reviewed-by Josh Durgin <[email protected]>

Quoted 26 lines · Alex Elder

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 <[email protected]>
---
  drivers/block/rbd.c |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/block/rbd.c +1 -1
index 65e9f1f..d032883 100644
@@ -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 */