On Fri, Feb 16, 2018 at 5:30 PM, Maged Mokhtar <mmokhtar@xxxxxxxxxxx> wrote: > On 2018-02-13 11:58, Ilya Dryomov wrote: > >> Hello, >> >> I've pushed wip-fancy-striping with my striping v2 patches. I didn't >> want to patch bomb the list and opened a dummy PR for feedback: >> >> https://github.com/ceph/ceph-client/pull/19 >> >> As this is a full striping v2 implementation with adjacent extent >> merging and no limitations on parent layouts, the patch set is very >> invasive -- it's basically a rewrite of the entire I/O path. Any >> review comments and testing cycles would be greatly appreciated! >> >> Thanks, >> >> Ilya > > > Hi Ilya, > > I looked at the patches, it is not a small change but is very nicely done. i > was surprised since even though you added complex feature support, the new > code is less (in rbd.c) and is clearer. The different fill from functions > and different osd request creation functions is a very clean design. > > If i may say any minor/cosmetic comments: > > Out of all the old code i was happy to see removed, the only thing i would > "miss" is the obj_requests list in the rbd_img_request struct. It is not > needed now, but i tend to think it is logical to have a place to find all > osd requests for a particular top block layer request. it maybe just a warm > a and fuzzy feeling to have this control, but it could be useful for > debugging or if in the future we think of implementing a task abort > operation and need to find the osd inflight ops in a quick way. It's still there -- rbd_img_request::object_extents. It holds object extents, which are embedded in object requests. As before, each object request holds an OSD request. > > Another cosmetic thing, i tend to think the new object <-> file extent > conversion be placed in a separate new file rather than osdmap.c ceph_calc_file_object_mapping() is in osdmap.c, so I just piled on. You are right though, it has nothing to do with osdmaps. > > I would not be able to do full test any time soon, but i do intend to do > some manual testing of basic functionality next week. i have in mind writing > data pattern with each value repeating the size of stripe unit and doing a > mix of read/write interchangeably between rbd and librbd, i would also test > copyup functionality in clones. I will let you know if i do find any issues. Thanks! Ilya -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
