[PATCH] rbd: don't treat CEPH_OSD_OP_DELETE as extent op

2014-11-24 Thread Ilya Dryomov
CEPH_OSD_OP_DELETE is not an extent op, stop treating it as such.  This
sneaked in with discard patches - it's one of the three osd ops (the
other two are CEPH_OSD_OP_TRUNCATE and CEPH_OSD_OP_ZERO) that discard
is implemented with.

Signed-off-by: Ilya Dryomov idryo...@redhat.com
---
 drivers/block/rbd.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 27b71a0b72d0..1df0802bf6cb 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -2370,8 +2370,12 @@ static void rbd_img_obj_request_fill(struct 
rbd_obj_request *obj_request,
opcode = CEPH_OSD_OP_READ;
}
 
-   osd_req_op_extent_init(osd_request, num_ops, opcode, offset, length,
-   0, 0);
+   if (opcode == CEPH_OSD_OP_DELETE)
+   osd_req_op_init(osd_request, num_ops, opcode);
+   else
+   osd_req_op_extent_init(osd_request, num_ops, opcode,
+  offset, length, 0, 0);
+
if (obj_request-type == OBJ_REQUEST_BIO)
osd_req_op_extent_osd_data_bio(osd_request, num_ops,
obj_request-bio_list, length);
-- 
2.1.1

--
To unsubscribe from this list: send the line unsubscribe ceph-devel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] rbd: don't treat CEPH_OSD_OP_DELETE as extent op

2014-11-24 Thread Alex Elder
On 11/24/2014 03:59 AM, Ilya Dryomov wrote:
 CEPH_OSD_OP_DELETE is not an extent op, stop treating it as such.  This
 sneaked in with discard patches - it's one of the three osd ops (the
 other two are CEPH_OSD_OP_TRUNCATE and CEPH_OSD_OP_ZERO) that discard
 is implemented with.
 
 Signed-off-by: Ilya Dryomov idryo...@redhat.com

Is the CEPH_OSD_OP_DELETE used in ceph_zero_partial_object()
an extent op?  If it is not, you should get rid of the BUG_ON()
in osd_req_op_extent_init() that allows CEPH_OSD_OP_DELETE.

And if that's the case it looks like that function or
ceph_osdc_new_request() handle CEPH_OSD_OP_DELETE
properly--so it's not treated as an extent op.

And:  osd_req_encode_op() encodes a CEPH_OSD_OP_DELETE
as an extent op as well.

If it *can* be an extent op (but just not as used by RBD)
then it warrants a comment here that explains why it is
not being initialized as an extent op.

-Alex

 ---
  drivers/block/rbd.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
 index 27b71a0b72d0..1df0802bf6cb 100644
 --- a/drivers/block/rbd.c
 +++ b/drivers/block/rbd.c
 @@ -2370,8 +2370,12 @@ static void rbd_img_obj_request_fill(struct 
 rbd_obj_request *obj_request,
   opcode = CEPH_OSD_OP_READ;
   }
  
 - osd_req_op_extent_init(osd_request, num_ops, opcode, offset, length,
 - 0, 0);
 + if (opcode == CEPH_OSD_OP_DELETE)
 + osd_req_op_init(osd_request, num_ops, opcode);
 + else
 + osd_req_op_extent_init(osd_request, num_ops, opcode,
 +offset, length, 0, 0);
 +
   if (obj_request-type == OBJ_REQUEST_BIO)
   osd_req_op_extent_osd_data_bio(osd_request, num_ops,
   obj_request-bio_list, length);
 

--
To unsubscribe from this list: send the line unsubscribe ceph-devel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] rbd: don't treat CEPH_OSD_OP_DELETE as extent op

2014-11-24 Thread Ilya Dryomov
On Mon, Nov 24, 2014 at 3:23 PM, Alex Elder el...@ieee.org wrote:
 On 11/24/2014 03:59 AM, Ilya Dryomov wrote:
 CEPH_OSD_OP_DELETE is not an extent op, stop treating it as such.  This
 sneaked in with discard patches - it's one of the three osd ops (the
 other two are CEPH_OSD_OP_TRUNCATE and CEPH_OSD_OP_ZERO) that discard
 is implemented with.

 Signed-off-by: Ilya Dryomov idryo...@redhat.com

 Is the CEPH_OSD_OP_DELETE used in ceph_zero_partial_object()
 an extent op?  If it is not, you should get rid of the BUG_ON()
 in osd_req_op_extent_init() that allows CEPH_OSD_OP_DELETE.

 And if that's the case it looks like that function or
 ceph_osdc_new_request() handle CEPH_OSD_OP_DELETE
 properly--so it's not treated as an extent op.

 And:  osd_req_encode_op() encodes a CEPH_OSD_OP_DELETE
 as an extent op as well.

 If it *can* be an extent op (but just not as used by RBD)
 then it warrants a comment here that explains why it is
 not being initialized as an extent op.

Hi Alex,

Clearly I didn't provide enough background.

OSDs don't look at extent part of the op union when processing
CEPH_OSD_OP_DELETE, so it's not an extent op.

Zheng added support for CEPH_OSD_OP_CREATE and in the same commit
changed osd_req_op_extent_init(), ceph_osdc_new_request() and
osd_req_encode_op() to not allow/encode CEPH_OSD_OP_DELETE, see [1].
This patch was rebased into testing before [1] in order to not break
git bisect as Zheng didn't care of rbd, which only recently started
issuing CEPH_OSD_OP_DELETE for whole-object discards.

[1] 
https://github.com/ceph/ceph-client/commit/6d23aa137d1861fc48f86ba6532458fcebcdd038

Thanks,

Ilya
--
To unsubscribe from this list: send the line unsubscribe ceph-devel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html