On Thu, Aug 16, 2018 at 3:08 PM Alex Elder <elder@xxxxxxxx> wrote:
>
> On 08/16/2018 12:59 AM, Dongsheng Yang wrote:
> > This is a cls client module for journaler.
> >
> > Signed-off-by: Dongsheng Yang <dongsheng.yang@xxxxxxxxxxxx>
>
> Trivial comments. Sorry, I can't offer a "proper" review...
>
> -Alex
>
> > ---
> > include/linux/ceph/cls_journaler_client.h | 87 ++++++
> > net/ceph/cls_journaler_client.c | 501 ++++++++++++++++++++++++++++++
> > 2 files changed, 588 insertions(+)
> > create mode 100644 include/linux/ceph/cls_journaler_client.h
> > create mode 100644 net/ceph/cls_journaler_client.c
> >
> > diff --git a/include/linux/ceph/cls_journaler_client.h b/include/linux/ceph/cls_journaler_client.h
> > new file mode 100644
> > index 0000000..cc9be96
> > --- /dev/null
> > +++ b/include/linux/ceph/cls_journaler_client.h
> > @@ -0,0 +1,87 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _LINUX_CEPH_CLS_JOURNAL_CLIENT_H
> > +#define _LINUX_CEPH_CLS_JOURNAL_CLIENT_H
> > +
> > +#include <linux/ceph/osd_client.h>
> > +
> > +struct ceph_journaler;
> > +struct ceph_journaler_client;
> > +
> > +struct ceph_journaler_object_pos {
> > + struct list_head node;
> > + u64 object_num;
> > + u64 tag_tid;
> > + u64 entry_tid;
> > +};
> > +
> > +struct ceph_journaler_client {
> > + struct list_head node;
> > + size_t id_len;
> > + char *id;
> > + size_t data_len;
> > + char *data;
> > + struct list_head object_positions;
> > +};
> > +
> > +struct ceph_journaler_tag {
> > + uint64_t tid;
> > + uint64_t tag_class;
> > + size_t data_len;
> > + char *data;
> > +};
> > +
> > +int ceph_cls_journaler_get_immutable_metas(struct ceph_osd_client *osdc,
> > + struct ceph_object_id *oid,
> > + struct ceph_object_locator *oloc,
> > + uint8_t *order,
> > + uint8_t *splay_width,
> > + int64_t *pool_id);
> > +
> > +int ceph_cls_journaler_get_mutable_metas(struct ceph_osd_client *osdc,
> > + struct ceph_object_id *oid,
> > + struct ceph_object_locator *oloc,
> > + uint64_t *minimum_set, uint64_t *active_set);
> > +
> > +int ceph_cls_journaler_client_list(struct ceph_osd_client *osdc,
> > + struct ceph_object_id *oid,
> > + struct ceph_object_locator *oloc,
> > + struct ceph_journaler_client **clients,
> > + uint32_t *client_num);
> > +
> > +int ceph_cls_journaler_get_next_tag_tid(struct ceph_osd_client *osdc,
> > + struct ceph_object_id *oid,
> > + struct ceph_object_locator *oloc,
> > + uint64_t *tag_tid);
> > +
> > +int ceph_cls_journaler_get_tag(struct ceph_osd_client *osdc,
> > + struct ceph_object_id *oid,
> > + struct ceph_object_locator *oloc,
> > + uint64_t tag_tid, struct ceph_journaler_tag *tag);
> > +
> > +int ceph_cls_journaler_tag_create(struct ceph_osd_client *osdc,
> > + struct ceph_object_id *oid,
> > + struct ceph_object_locator *oloc,
> > + uint64_t tag_tid, uint64_t tag_class,
> > + void *buf, uint32_t buf_len);
> > +
> > +int ceph_cls_journaler_client_committed(struct ceph_osd_client *osdc,
> > + struct ceph_object_id *oid,
> > + struct ceph_object_locator *oloc,
> > + struct ceph_journaler_client *client,
> > + struct list_head *object_positions);
> > +
> > +int ceph_cls_journaler_set_active_set(struct ceph_osd_client *osdc,
> > + struct ceph_object_id *oid,
> > + struct ceph_object_locator *oloc,
> > + uint64_t active_set);
> > +
> > +int ceph_cls_journaler_set_minimum_set(struct ceph_osd_client *osdc,
> > + struct ceph_object_id *oid,
> > + struct ceph_object_locator *oloc,
> > + uint64_t minimum_set);
> > +
> > +int ceph_cls_journaler_guard_append(struct ceph_osd_client *osdc,
> > + struct ceph_object_id *oid,
> > + struct ceph_object_locator *oloc,
> > + uint64_t soft_limit);
> > +#endif
> > diff --git a/net/ceph/cls_journaler_client.c b/net/ceph/cls_journaler_client.c
> > new file mode 100644
> > index 0000000..971fc5d
> > --- /dev/null
> > +++ b/net/ceph/cls_journaler_client.c
> > @@ -0,0 +1,501 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <linux/ceph/ceph_debug.h>
> > +
> > +#include <linux/types.h>
> > +#include <linux/slab.h>
> > +
> > +#include <linux/ceph/cls_journaler_client.h>
> > +#include <linux/ceph/decode.h>
> > +#include <linux/ceph/journaler.h>
> > +
> > +//TODO get all metas in one single request
>
> You should get rid of "TODO" comments; if it's a to-do item, describe the
> work to be done elsewhere (like a tracker entry).
This is a matter of taste, but I disagree, especially for RFC patches.
These comments are valuable because they are localized to a block of
code and make it easy to see what is intended to be revisited without
referencing the cover letter or, worse, some tracker.
Thanks,
Ilya
