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<elder@xxxxxxxxxxxxx>
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
