From: Ulrich Weigand Date: Mon, 2 Sep 2019 19:26:53 +0000 (+0200) Subject: transaction: Fix incorrect use of __le16/32/64 X-Git-Tag: v14.2.5~173^2~5 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=21bd5f0b51163cf40c7e9840faefdf0b4193f8b3;p=ceph.git transaction: Fix incorrect use of __le16/32/64 Use ceph_le16/32/64 instead of __le16/32/64 (which are no-op outside of kernel code). Note that this updates only those uses of __le16/32/64 which are part of data structures that are serialized to disk/network (i.e. Transaction::Op and Transaction::TransactionData). Also note that code in this file performs combined operations on little-endian values (in particular ++, +=, and |=) which are not supported on the ceph_le16/32/64 class, and are therefore replaced by more primitive operations. Fixes (partially): https://tracker.ceph.com/issues/41605 Signed-off-by: Ulrich Weigand (cherry picked from commit 7d3cd10edfbdf9de6344430b0e336c351a903bfc) Changes from cherry-picked commit: - Duplicated changes into src/crimson/os/Transaction.h and src/os/ObjectStore.h (are merged into src/os/Transaction.h in mainline). Signed-off-by: Ulrich Weigand --- diff --git a/src/crimson/os/Transaction.h b/src/crimson/os/Transaction.h index a2f58b19d644..c6f660fe150b 100644 --- a/src/crimson/os/Transaction.h +++ b/src/crimson/os/Transaction.h @@ -149,42 +149,42 @@ public: }; struct Op { - __le32 op; - __le32 cid; - __le32 oid; - __le64 off; - __le64 len; - __le32 dest_cid; - __le32 dest_oid; //OP_CLONE, OP_CLONERANGE - __le64 dest_off; //OP_CLONERANGE + ceph_le32 op; + ceph_le32 cid; + ceph_le32 oid; + ceph_le64 off; + ceph_le64 len; + ceph_le32 dest_cid; + ceph_le32 dest_oid; //OP_CLONE, OP_CLONERANGE + ceph_le64 dest_off; //OP_CLONERANGE union { struct { - __le32 hint_type; //OP_COLL_HINT + ceph_le32 hint_type; //OP_COLL_HINT }; struct { - __le32 alloc_hint_flags; //OP_SETALLOCHINT + ceph_le32 alloc_hint_flags; //OP_SETALLOCHINT }; }; - __le64 expected_object_size; //OP_SETALLOCHINT - __le64 expected_write_size; //OP_SETALLOCHINT - __le32 split_bits; //OP_SPLIT_COLLECTION2,OP_COLL_SET_BITS, + ceph_le64 expected_object_size; //OP_SETALLOCHINT + ceph_le64 expected_write_size; //OP_SETALLOCHINT + ceph_le32 split_bits; //OP_SPLIT_COLLECTION2,OP_COLL_SET_BITS, //OP_MKCOLL - __le32 split_rem; //OP_SPLIT_COLLECTION2 + ceph_le32 split_rem; //OP_SPLIT_COLLECTION2 } __attribute__ ((packed)) ; struct TransactionData { - __le64 ops; - __le32 largest_data_len; - __le32 largest_data_off; - __le32 largest_data_off_in_data_bl; - __le32 fadvise_flags; + ceph_le64 ops; + ceph_le32 largest_data_len; + ceph_le32 largest_data_off; + ceph_le32 largest_data_off_in_data_bl; + ceph_le32 fadvise_flags; TransactionData() noexcept : - ops(0), - largest_data_len(0), - largest_data_off(0), - largest_data_off_in_data_bl(0), - fadvise_flags(0) { } + ops(init_le64(0)), + largest_data_len(init_le32(0)), + largest_data_off(init_le32(0)), + largest_data_off_in_data_bl(init_le32(0)), + fadvise_flags(init_le32(0)) { } // override default move operations to reset default values TransactionData(TransactionData&& other) noexcept : @@ -498,13 +498,13 @@ public: /// operations are removed from the parameter Transaction void append(Transaction& other) { - data.ops += other.data.ops; + data.ops = data.ops + other.data.ops; if (other.data.largest_data_len > data.largest_data_len) { data.largest_data_len = other.data.largest_data_len; data.largest_data_off = other.data.largest_data_off; data.largest_data_off_in_data_bl = data_bl.length() + other.data.largest_data_off_in_data_bl; } - data.fadvise_flags |= other.data.fadvise_flags; + data.fadvise_flags = data.fadvise_flags | other.data.fadvise_flags; on_applied.splice(on_applied.end(), other.on_applied); on_commit.splice(on_commit.end(), other.on_commit); on_applied_sync.splice(on_applied_sync.end(), other.on_applied_sync); @@ -777,7 +777,7 @@ public: void nop() { Op* _op = _get_next_op(); _op->op = OP_NOP; - data.ops++; + data.ops = data.ops + 1; } /** * touch @@ -790,7 +790,7 @@ public: _op->op = OP_TOUCH; _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); - data.ops++; + data.ops = data.ops + 1; } /** * Write data to an offset within an object. If the object is too @@ -824,7 +824,7 @@ public: // we are about to data.largest_data_off_in_data_bl = orig_len + sizeof(__u32); } - data.ops++; + data.ops = data.ops + 1; } /** * zero out the indicated byte range within an object. Some @@ -843,7 +843,7 @@ public: _op->oid = _get_object_id(oid); _op->off = off; _op->len = len; - data.ops++; + data.ops = data.ops + 1; } /// Discard all data in the object beyond the specified size. void truncate(const coll_t& cid, const ghobject_t& oid, uint64_t off) { @@ -852,7 +852,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); _op->off = off; - data.ops++; + data.ops = data.ops + 1; } /// Remove an object. All four parts of the object are removed. void remove(const coll_t& cid, const ghobject_t& oid) { @@ -860,7 +860,7 @@ public: _op->op = OP_REMOVE; _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); - data.ops++; + data.ops = data.ops + 1; } /// Set an xattr of an object void setattr(const coll_t& cid, const ghobject_t& oid, @@ -879,7 +879,7 @@ public: _op->oid = _get_object_id(oid); encode(s, data_bl); encode(val, data_bl); - data.ops++; + data.ops = data.ops + 1; } /// Set multiple xattrs of an object void setattrs(const coll_t& cid, const ghobject_t& oid, @@ -890,7 +890,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); encode(attrset, data_bl); - data.ops++; + data.ops = data.ops + 1; } /// Set multiple xattrs of an object void setattrs(const coll_t& cid, const ghobject_t& oid, @@ -901,7 +901,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); encode(attrset, data_bl); - data.ops++; + data.ops = data.ops + 1; } /// remove an xattr from an object void rmattr(const coll_t& cid, const ghobject_t& oid, const char *name) { @@ -916,7 +916,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); encode(s, data_bl); - data.ops++; + data.ops = data.ops + 1; } /// remove all xattrs from an object void rmattrs(const coll_t& cid, const ghobject_t& oid) { @@ -924,7 +924,7 @@ public: _op->op = OP_RMATTRS; _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); - data.ops++; + data.ops = data.ops + 1; } /** * Clone an object into another object. @@ -944,7 +944,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); _op->dest_oid = _get_object_id(noid); - data.ops++; + data.ops = data.ops + 1; } /** * Clone a byte range from one object to another. @@ -969,7 +969,7 @@ public: _op->off = srcoff; _op->len = srclen; _op->dest_off = dstoff; - data.ops++; + data.ops = data.ops + 1; } /// Create the collection @@ -978,7 +978,7 @@ public: _op->op = OP_MKCOLL; _op->cid = _get_coll_id(cid); _op->split_bits = bits; - data.ops++; + data.ops = data.ops + 1; } /** @@ -996,7 +996,7 @@ public: _op->cid = _get_coll_id(cid); _op->hint_type = type; encode(hint, data_bl); - data.ops++; + data.ops = data.ops + 1; } /// remove the collection, the collection must be empty @@ -1004,7 +1004,7 @@ public: Op* _op = _get_next_op(); _op->op = OP_RMCOLL; _op->cid = _get_coll_id(cid); - data.ops++; + data.ops = data.ops + 1; } void collection_move(const coll_t& cid, const coll_t &oldcid, const ghobject_t& oid) @@ -1016,13 +1016,13 @@ public: _op->cid = _get_coll_id(oldcid); _op->oid = _get_object_id(oid); _op->dest_cid = _get_coll_id(cid); - data.ops++; + data.ops = data.ops + 1; _op = _get_next_op(); _op->op = OP_COLL_REMOVE; _op->cid = _get_coll_id(oldcid); _op->oid = _get_object_id(oid); - data.ops++; + data.ops = data.ops + 1; } void collection_move_rename(const coll_t& oldcid, const ghobject_t& oldoid, const coll_t &cid, const ghobject_t& oid) { @@ -1032,7 +1032,7 @@ public: _op->oid = _get_object_id(oldoid); _op->dest_cid = _get_coll_id(cid); _op->dest_oid = _get_object_id(oid); - data.ops++; + data.ops = data.ops + 1; } void try_rename(const coll_t &cid, const ghobject_t& oldoid, const ghobject_t& oid) { @@ -1041,7 +1041,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oldoid); _op->dest_oid = _get_object_id(oid); - data.ops++; + data.ops = data.ops + 1; } /// Remove omap from oid @@ -1053,7 +1053,7 @@ public: _op->op = OP_OMAP_CLEAR; _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); - data.ops++; + data.ops = data.ops + 1; } /// Set keys on oid omap. Replaces duplicate keys. void omap_setkeys( @@ -1067,7 +1067,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); encode(attrset, data_bl); - data.ops++; + data.ops = data.ops + 1; } /// Set keys on an oid omap (bufferlist variant). @@ -1081,7 +1081,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); data_bl.append(attrset_bl); - data.ops++; + data.ops = data.ops + 1; } /// Remove keys from oid omap @@ -1096,7 +1096,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); encode(keys, data_bl); - data.ops++; + data.ops = data.ops + 1; } /// Remove keys from oid omap @@ -1110,7 +1110,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); data_bl.append(keys_bl); - data.ops++; + data.ops = data.ops + 1; } /// Remove key range from oid omap @@ -1127,7 +1127,7 @@ public: _op->oid = _get_object_id(oid); encode(first, data_bl); encode(last, data_bl); - data.ops++; + data.ops = data.ops + 1; } /// Set omap header @@ -1142,7 +1142,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); encode(bl, data_bl); - data.ops++; + data.ops = data.ops + 1; } /// Split collection based on given prefixes, objects matching the specified @@ -1158,7 +1158,7 @@ public: _op->dest_cid = _get_coll_id(destination); _op->split_bits = bits; _op->split_rem = rem; - data.ops++; + data.ops = data.ops + 1; } /// Merge collection into another. @@ -1171,7 +1171,7 @@ public: _op->cid = _get_coll_id(cid); _op->dest_cid = _get_coll_id(destination); _op->split_bits = bits; - data.ops++; + data.ops = data.ops + 1; } void collection_set_bits( @@ -1181,7 +1181,7 @@ public: _op->op = OP_COLL_SET_BITS; _op->cid = _get_coll_id(cid); _op->split_bits = bits; - data.ops++; + data.ops = data.ops + 1; } /// Set allocation hint for an object @@ -1200,7 +1200,7 @@ public: _op->expected_object_size = expected_object_size; _op->expected_write_size = expected_write_size; _op->alloc_hint_flags = flags; - data.ops++; + data.ops = data.ops + 1; } void encode(bufferlist& bl) const { diff --git a/src/os/ObjectStore.h b/src/os/ObjectStore.h index 6096b494bd6f..3f6f68fdefc2 100644 --- a/src/os/ObjectStore.h +++ b/src/os/ObjectStore.h @@ -359,42 +359,42 @@ public: }; struct Op { - __le32 op; - __le32 cid; - __le32 oid; - __le64 off; - __le64 len; - __le32 dest_cid; - __le32 dest_oid; //OP_CLONE, OP_CLONERANGE - __le64 dest_off; //OP_CLONERANGE + ceph_le32 op; + ceph_le32 cid; + ceph_le32 oid; + ceph_le64 off; + ceph_le64 len; + ceph_le32 dest_cid; + ceph_le32 dest_oid; //OP_CLONE, OP_CLONERANGE + ceph_le64 dest_off; //OP_CLONERANGE union { struct { - __le32 hint_type; //OP_COLL_HINT + ceph_le32 hint_type; //OP_COLL_HINT }; struct { - __le32 alloc_hint_flags; //OP_SETALLOCHINT + ceph_le32 alloc_hint_flags; //OP_SETALLOCHINT }; }; - __le64 expected_object_size; //OP_SETALLOCHINT - __le64 expected_write_size; //OP_SETALLOCHINT - __le32 split_bits; //OP_SPLIT_COLLECTION2,OP_COLL_SET_BITS, + ceph_le64 expected_object_size; //OP_SETALLOCHINT + ceph_le64 expected_write_size; //OP_SETALLOCHINT + ceph_le32 split_bits; //OP_SPLIT_COLLECTION2,OP_COLL_SET_BITS, //OP_MKCOLL - __le32 split_rem; //OP_SPLIT_COLLECTION2 + ceph_le32 split_rem; //OP_SPLIT_COLLECTION2 } __attribute__ ((packed)) ; struct TransactionData { - __le64 ops; - __le32 largest_data_len; - __le32 largest_data_off; - __le32 largest_data_off_in_data_bl; - __le32 fadvise_flags; + ceph_le64 ops; + ceph_le32 largest_data_len; + ceph_le32 largest_data_off; + ceph_le32 largest_data_off_in_data_bl; + ceph_le32 fadvise_flags; TransactionData() noexcept : - ops(0), - largest_data_len(0), - largest_data_off(0), - largest_data_off_in_data_bl(0), - fadvise_flags(0) { } + ops(init_le64(0)), + largest_data_len(init_le32(0)), + largest_data_off(init_le32(0)), + largest_data_off_in_data_bl(init_le32(0)), + fadvise_flags(init_le32(0)) { } // override default move operations to reset default values TransactionData(TransactionData&& other) noexcept : @@ -709,13 +709,13 @@ public: /// Append the operations of the parameter to this Transaction. Those operations are removed from the parameter Transaction void append(Transaction& other) { - data.ops += other.data.ops; + data.ops = data.ops + other.data.ops; if (other.data.largest_data_len > data.largest_data_len) { data.largest_data_len = other.data.largest_data_len; data.largest_data_off = other.data.largest_data_off; data.largest_data_off_in_data_bl = data_bl.length() + other.data.largest_data_off_in_data_bl; } - data.fadvise_flags |= other.data.fadvise_flags; + data.fadvise_flags = data.fadvise_flags | other.data.fadvise_flags; on_applied.splice(on_applied.end(), other.on_applied); on_commit.splice(on_commit.end(), other.on_commit); on_applied_sync.splice(on_applied_sync.end(), other.on_applied_sync); @@ -990,7 +990,7 @@ public: void nop() { Op* _op = _get_next_op(); _op->op = OP_NOP; - data.ops++; + data.ops = data.ops + 1; } /** * touch @@ -1003,7 +1003,7 @@ public: _op->op = OP_TOUCH; _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); - data.ops++; + data.ops = data.ops + 1; } /** * Write data to an offset within an object. If the object is too @@ -1036,7 +1036,7 @@ public: data.largest_data_off = off; data.largest_data_off_in_data_bl = orig_len + sizeof(__u32); // we are about to } - data.ops++; + data.ops = data.ops + 1; } /** * zero out the indicated byte range within an object. Some @@ -1055,7 +1055,7 @@ public: _op->oid = _get_object_id(oid); _op->off = off; _op->len = len; - data.ops++; + data.ops = data.ops + 1; } /// Discard all data in the object beyond the specified size. void truncate(const coll_t& cid, const ghobject_t& oid, uint64_t off) { @@ -1064,7 +1064,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); _op->off = off; - data.ops++; + data.ops = data.ops + 1; } /// Remove an object. All four parts of the object are removed. void remove(const coll_t& cid, const ghobject_t& oid) { @@ -1072,7 +1072,7 @@ public: _op->op = OP_REMOVE; _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); - data.ops++; + data.ops = data.ops + 1; } /// Set an xattr of an object void setattr(const coll_t& cid, const ghobject_t& oid, const char* name, bufferlist& val) { @@ -1088,7 +1088,7 @@ public: _op->oid = _get_object_id(oid); encode(s, data_bl); encode(val, data_bl); - data.ops++; + data.ops = data.ops + 1; } /// Set multiple xattrs of an object void setattrs(const coll_t& cid, const ghobject_t& oid, const map& attrset) { @@ -1098,7 +1098,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); encode(attrset, data_bl); - data.ops++; + data.ops = data.ops + 1; } /// Set multiple xattrs of an object void setattrs(const coll_t& cid, const ghobject_t& oid, const map& attrset) { @@ -1108,7 +1108,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); encode(attrset, data_bl); - data.ops++; + data.ops = data.ops + 1; } /// remove an xattr from an object void rmattr(const coll_t& cid, const ghobject_t& oid, const char *name) { @@ -1123,7 +1123,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); encode(s, data_bl); - data.ops++; + data.ops = data.ops + 1; } /// remove all xattrs from an object void rmattrs(const coll_t& cid, const ghobject_t& oid) { @@ -1131,7 +1131,7 @@ public: _op->op = OP_RMATTRS; _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); - data.ops++; + data.ops = data.ops + 1; } /** * Clone an object into another object. @@ -1151,7 +1151,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); _op->dest_oid = _get_object_id(noid); - data.ops++; + data.ops = data.ops + 1; } /** * Clone a byte range from one object to another. @@ -1176,7 +1176,7 @@ public: _op->off = srcoff; _op->len = srclen; _op->dest_off = dstoff; - data.ops++; + data.ops = data.ops + 1; } /// Create the collection @@ -1185,7 +1185,7 @@ public: _op->op = OP_MKCOLL; _op->cid = _get_coll_id(cid); _op->split_bits = bits; - data.ops++; + data.ops = data.ops + 1; } /** @@ -1203,7 +1203,7 @@ public: _op->cid = _get_coll_id(cid); _op->hint_type = type; encode(hint, data_bl); - data.ops++; + data.ops = data.ops + 1; } /// remove the collection, the collection must be empty @@ -1211,7 +1211,7 @@ public: Op* _op = _get_next_op(); _op->op = OP_RMCOLL; _op->cid = _get_coll_id(cid); - data.ops++; + data.ops = data.ops + 1; } void collection_move(const coll_t& cid, const coll_t &oldcid, const ghobject_t& oid) __attribute__ ((deprecated)) { @@ -1222,13 +1222,13 @@ public: _op->cid = _get_coll_id(oldcid); _op->oid = _get_object_id(oid); _op->dest_cid = _get_coll_id(cid); - data.ops++; + data.ops = data.ops + 1; _op = _get_next_op(); _op->op = OP_COLL_REMOVE; _op->cid = _get_coll_id(oldcid); _op->oid = _get_object_id(oid); - data.ops++; + data.ops = data.ops + 1; } void collection_move_rename(const coll_t& oldcid, const ghobject_t& oldoid, const coll_t &cid, const ghobject_t& oid) { @@ -1238,7 +1238,7 @@ public: _op->oid = _get_object_id(oldoid); _op->dest_cid = _get_coll_id(cid); _op->dest_oid = _get_object_id(oid); - data.ops++; + data.ops = data.ops + 1; } void try_rename(const coll_t &cid, const ghobject_t& oldoid, const ghobject_t& oid) { @@ -1247,7 +1247,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oldoid); _op->dest_oid = _get_object_id(oid); - data.ops++; + data.ops = data.ops + 1; } /// Remove omap from oid @@ -1259,7 +1259,7 @@ public: _op->op = OP_OMAP_CLEAR; _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); - data.ops++; + data.ops = data.ops + 1; } /// Set keys on oid omap. Replaces duplicate keys. void omap_setkeys( @@ -1273,7 +1273,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); encode(attrset, data_bl); - data.ops++; + data.ops = data.ops + 1; } /// Set keys on an oid omap (bufferlist variant). @@ -1287,7 +1287,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); data_bl.append(attrset_bl); - data.ops++; + data.ops = data.ops + 1; } /// Remove keys from oid omap @@ -1302,7 +1302,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); encode(keys, data_bl); - data.ops++; + data.ops = data.ops + 1; } /// Remove keys from oid omap @@ -1316,7 +1316,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); data_bl.append(keys_bl); - data.ops++; + data.ops = data.ops + 1; } /// Remove key range from oid omap @@ -1333,7 +1333,7 @@ public: _op->oid = _get_object_id(oid); encode(first, data_bl); encode(last, data_bl); - data.ops++; + data.ops = data.ops + 1; } /// Set omap header @@ -1348,7 +1348,7 @@ public: _op->cid = _get_coll_id(cid); _op->oid = _get_object_id(oid); encode(bl, data_bl); - data.ops++; + data.ops = data.ops + 1; } /// Split collection based on given prefixes, objects matching the specified bits/rem are @@ -1364,7 +1364,7 @@ public: _op->dest_cid = _get_coll_id(destination); _op->split_bits = bits; _op->split_rem = rem; - data.ops++; + data.ops = data.ops + 1; } /// Merge collection into another. @@ -1377,7 +1377,7 @@ public: _op->cid = _get_coll_id(cid); _op->dest_cid = _get_coll_id(destination); _op->split_bits = bits; - data.ops++; + data.ops = data.ops + 1; } void collection_set_bits( @@ -1387,7 +1387,7 @@ public: _op->op = OP_COLL_SET_BITS; _op->cid = _get_coll_id(cid); _op->split_bits = bits; - data.ops++; + data.ops = data.ops + 1; } /// Set allocation hint for an object @@ -1406,7 +1406,7 @@ public: _op->expected_object_size = expected_object_size; _op->expected_write_size = expected_write_size; _op->alloc_hint_flags = flags; - data.ops++; + data.ops = data.ops + 1; } void encode(bufferlist& bl) const {