On Tue, Oct 9, 2018 at 4:10 PM Ilya Dryomov <idryomov@xxxxxxxxx> wrote:
>
> On Tue, Oct 9, 2018 at 12:47 PM Luis Henriques <lhenriques@xxxxxxxx> wrote:
> >
> > Add support for performing remote object copies using the 'copy-from'
> > operation.
> >
> > Signed-off-by: Luis Henriques <lhenriques@xxxxxxxx>
> > ---
> > include/linux/ceph/osd_client.h | 17 ++++++++
> > include/linux/ceph/rados.h | 19 +++++++++
> > net/ceph/osd_client.c | 72 +++++++++++++++++++++++++++++++++
> > 3 files changed, 108 insertions(+)
>
> Change the title to "libceph: ...".
>
> >
> > diff --git a/include/linux/ceph/osd_client.h b/include/linux/ceph/osd_client.h
> > index 02096da01845..f25b4aaaab09 100644
> > --- a/include/linux/ceph/osd_client.h
> > +++ b/include/linux/ceph/osd_client.h
> > @@ -136,6 +136,13 @@ struct ceph_osd_req_op {
> > u64 expected_object_size;
> > u64 expected_write_size;
> > } alloc_hint;
> > + struct {
> > + u64 snapid;
> > + u64 src_version;
> > + u8 flags;
> > + u32 src_fadvise_flags;
> > + struct ceph_osd_data osd_data;
> > + } copy_from;
> > };
> > };
> >
> > @@ -511,6 +518,16 @@ extern int ceph_osdc_writepages(struct ceph_osd_client *osdc,
> > struct timespec64 *mtime,
> > struct page **pages, int nr_pages);
> >
> > +extern int ceph_osdc_copy_from(struct ceph_osd_client *osdc,
> > + u64 src_snapid, u64 src_version,
> > + struct ceph_object_id *src_oid,
> > + struct ceph_object_locator *src_oloc,
> > + u32 src_fadvise_flags,
> > + u64 dst_snapid,
> > + struct ceph_object_id *dst_oid,
> > + struct ceph_object_locator *dst_oloc,
> > + u8 dst_fadvise_flags);
>
> Drop extern here.
>
> > +
> > /* watch/notify */
> > struct ceph_osd_linger_request *
> > ceph_osdc_watch(struct ceph_osd_client *osdc,
> > diff --git a/include/linux/ceph/rados.h b/include/linux/ceph/rados.h
> > index f1988387c5ad..d47540986eff 100644
> > --- a/include/linux/ceph/rados.h
> > +++ b/include/linux/ceph/rados.h
> > @@ -410,6 +410,14 @@ enum {
> > enum {
> > CEPH_OSD_OP_FLAG_EXCL = 1, /* EXCL object create */
> > CEPH_OSD_OP_FLAG_FAILOK = 2, /* continue despite failure */
> > + CEPH_OSD_OP_FLAG_FADVISE_RANDOM = 0x4, /* the op is random */
> > + CEPH_OSD_OP_FLAG_FADVISE_SEQUENTIAL = 0x8, /* the op is sequential */
> > + CEPH_OSD_OP_FLAG_FADVISE_WILLNEED = 0x10,/* data will be accessed in
> > + the near future */
> > + CEPH_OSD_OP_FLAG_FADVISE_DONTNEED = 0x20,/* data will not be accessed
> > + in the near future */
> > + CEPH_OSD_OP_FLAG_FADVISE_NOCACHE = 0x40,/* data will be accessed only
> > + once by this client */
> > };
> >
> > #define EOLDSNAPC ERESTART /* ORDERSNAP flag set; writer has old snapc*/
> > @@ -497,6 +505,17 @@ struct ceph_osd_op {
> > __le64 expected_object_size;
> > __le64 expected_write_size;
> > } __attribute__ ((packed)) alloc_hint;
> > + struct {
> > + __le64 snapid;
> > + __le64 src_version;
> > + __u8 flags;
>
> Add /* CEPH_OSD_COPY_FROM_FLAG_* */ and pull in that enum.
>
> > + /*
> > + * __le32 flags: CEPH_OSD_OP_FLAG_FADVISE_: mean the
> > + * fadvise flags for dest object src_fadvise_flags mean
> > + * the fadvise flags for src object
> > + */
> > + __le32 src_fadvise_flags;
>
> This comment is super confusing. It makes it look like a commented out
> field, but it actually refers to op.flags. This seems to have confused
> you into thinking that dst_fadvise_flags go into the above __u8 flags.
>
> Change it to
>
> /*
> * CEPH_OSD_OP_FLAG_FADVISE_*
> *
> * These are fadvise flags for src object, fadvise flags for dest
> * object are in ceph_osd_op::flags.
> */
>
> I'll send a PR to get this clarified on the userspace side as well.
https://github.com/ceph/ceph/pull/24497
Thanks,
Ilya
