On 03/16/2012 12:59 AM, Sage Weil wrote:
On Mon, 12 Mar 2012, Alex Elder wrote:
There are helpers defined in "include/linux/ceph/osdmap.h" for
accessing the fields of an on-disk ceph file layout structure.
Use them--consistently--throughout the code.

Also define CEPH_FILE_LAYOUT_PG_PREFERRED_NONE, to represent
symbolically the explicit "no preferred PG" value.

Make a few casts explicit too (to make it more obvious it's
occuring).  This produces some long lines, but they go away in an
upcoming patch.

Signed-off-by: Alex Elder<[email protected]>

A few comments below.  I'm about to re-post this series.

. . .

diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
index 75960b1..bfd5b93 100644
--- a/fs/ceph/xattr.c
+++ b/fs/ceph/xattr.c
@@ -115,11 +115,12 @@ static size_t ceph_vxattrcb_file_layout(struct
ceph_inode_info *ci, char *val,

        ret = snprintf(val, size,
                "chunk_bytes=%lld\nstripe_count=%lld\nobject_size=%lld\n",
-               (unsigned long long)ceph_file_layout_su(&ci->i_layout),
+               (unsigned long
long)ceph_file_layout_stripe_unit(&ci->i_layout),
                (unsigned long
long)ceph_file_layout_stripe_count(&ci->i_layout),
                (unsigned long
long)ceph_file_layout_object_size(&ci->i_layout));

-       if (ceph_file_layout_pg_preferred(&ci->i_layout)>= 0) {
+       if (ceph_file_layout_pg_preferred(&ci->i_layout) !=
+                       CEPH_FILE_LAYOUT_PG_PREFERRED_NONE) {

The function returns cpu order, #define is little endian

I will fix this--throughout.  I'm going to define the constant
in CPU order instead (and fix the one place that depended on it
being in little endian order).

                val += ret;
                size -= ret;
                ret += snprintf(val, size, "preferred_osd=%lld\n",
diff --git a/include/linux/ceph/ceph_fs.h b/include/linux/ceph/ceph_fs.h
index 2645704..cf86032 100644
--- a/include/linux/ceph/ceph_fs.h
+++ b/include/linux/ceph/ceph_fs.h

. . .

@@ -1011,23 +1011,24 @@ int ceph_calc_object_layout(struct ceph_object_layout
*ol,
        if (!pool)
                return -EIO;
        ps = ceph_str_hash(pool->v.object_hash, oid, strlen(oid));
-       if (preferred>= 0) {
+
+       if (preferred == CEPH_FILE_LAYOUT_PG_PREFERRED_NONE) {

Here, too.  I'd just stick with>= ...

No, I intentionally defined this constant to have a specific
meaning, and I don't want any sense that it is greater than
or less than something--just distinguished from all other
valid preferred PG values.


+               num = le32_to_cpu(pool->v.pg_num);
+               num_mask = pool->pg_num_mask;
+       } else {
                ps += preferred;
                num = le32_to_cpu(pool->v.lpg_num);
                num_mask = pool->lpg_num_mask;
-       } else {

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

Reply via email to