From 337cfc0a7ba7f84fae24278aa5dbad22701728e3 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Wed, 19 Nov 2008 21:50:39 -0800 Subject: [PATCH] os: clean up ObjectStore::Transaction interface Also, fix attrset * thing.. that doesn't look safe! --- src/ebofs/Ebofs.cc | 147 +++++++++----------------- src/os/FileStore.cc | 245 +++++++++++-------------------------------- src/os/ObjectStore.h | 39 ++++--- 3 files changed, 130 insertions(+), 301 deletions(-) diff --git a/src/ebofs/Ebofs.cc b/src/ebofs/Ebofs.cc index 3eff62ad26eb1..a63b13193d9dc 100644 --- a/src/ebofs/Ebofs.cc +++ b/src/ebofs/Ebofs.cc @@ -2485,10 +2485,8 @@ unsigned Ebofs::_apply_transaction(Transaction& t) case Transaction::OP_TOUCH: { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); + coll_t cid = t.get_cid(); + pobject_t oid = t.get_oid(); if (_touch(oid) < 0) { dout(7) << "apply_transaction fail on _touch" << dendl; r &= bit; @@ -2498,15 +2496,11 @@ unsigned Ebofs::_apply_transaction(Transaction& t) case Transaction::OP_WRITE: { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - __u64 offset, len; - t.get_length(offset); - t.get_length(len); - bufferlist bl; - t.get_bl(bl); + coll_t cid = t.get_cid(); + pobject_t oid = t.get_oid(); + __u64 offset = t.get_length(); + __u64 len = t.get_length(); + bufferlist& bl = t.get_bl(); if (_write(oid, offset, len, bl) < 0) { dout(7) << "apply_transaction fail on _write" << dendl; r &= bit; @@ -2516,13 +2510,10 @@ unsigned Ebofs::_apply_transaction(Transaction& t) case Transaction::OP_ZERO: { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - __u64 offset, len; - t.get_length(offset); - t.get_length(len); + coll_t cid = t.get_cid(); + pobject_t oid = t.get_oid(); + __u64 offset = t.get_length(); + __u64 len = t.get_length(); if (_zero(oid, offset, len) < 0) { dout(7) << "apply_transaction fail on _zero" << dendl; r &= bit; @@ -2532,26 +2523,20 @@ unsigned Ebofs::_apply_transaction(Transaction& t) case Transaction::OP_TRIMCACHE: { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - __u64 offset, len; - t.get_length(offset); - t.get_length(len); + coll_t cid = t.get_cid(); + pobject_t oid = t.get_oid(); + __u64 offset = t.get_length(); + __u64 len = t.get_length(); _trim_from_cache(oid, offset, len); } break; case Transaction::OP_TRUNCATE: { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - __u64 len; - t.get_length(len); - if (_truncate(oid, len) < 0) { + coll_t cid = t.get_cid(); + pobject_t oid = t.get_oid(); + __u64 offset = t.get_length(); + if (_truncate(oid, offset) < 0) { dout(7) << "apply_transaction fail on _truncate" << dendl; r &= bit; } @@ -2560,10 +2545,8 @@ unsigned Ebofs::_apply_transaction(Transaction& t) case Transaction::OP_REMOVE: { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); + coll_t cid = t.get_cid(); + pobject_t oid = t.get_oid(); if (_remove(oid) < 0) { dout(7) << "apply_transaction fail on _remove" << dendl; r &= bit; @@ -2573,14 +2556,10 @@ unsigned Ebofs::_apply_transaction(Transaction& t) case Transaction::OP_SETATTR: { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - const char *attrname; - t.get_attrname(attrname); - bufferlist bl; - t.get_bl(bl); + coll_t cid = t.get_cid(); + pobject_t oid = t.get_oid(); + const char *attrname = t.get_attrname(); + bufferlist& bl = t.get_bl(); if (_setattr(oid, attrname, bl.c_str(), bl.length()) < 0) { dout(7) << "apply_transaction fail on _setattr" << dendl; r &= bit; @@ -2590,13 +2569,10 @@ unsigned Ebofs::_apply_transaction(Transaction& t) case Transaction::OP_SETATTRS: { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - map *pattrset; - t.get_pattrset(pattrset); - if (_setattrs(oid, *pattrset) < 0) { + coll_t cid = t.get_cid(); + pobject_t oid = t.get_oid(); + map& attrset = t.get_attrset(); + if (_setattrs(oid, attrset) < 0) { dout(7) << "apply_transaction fail on _setattrs" << dendl; r &= bit; } @@ -2605,12 +2581,9 @@ unsigned Ebofs::_apply_transaction(Transaction& t) case Transaction::OP_RMATTR: { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - const char *attrname; - t.get_attrname(attrname); + coll_t cid = t.get_cid(); + pobject_t oid = t.get_oid(); + const char *attrname = t.get_attrname(); if (_rmattr(oid, attrname) < 0) { dout(7) << "apply_transaction fail on _rmattr" << dendl; r &= bit; @@ -2620,12 +2593,9 @@ unsigned Ebofs::_apply_transaction(Transaction& t) case Transaction::OP_CLONE: { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - pobject_t noid; - t.get_oid(noid); + coll_t cid = t.get_cid(); + pobject_t oid = t.get_oid(); + pobject_t noid = t.get_oid(); if (_clone(oid, noid) < 0) { dout(7) << "apply_transaction fail on _clone" << dendl; r &= bit; @@ -2635,15 +2605,11 @@ unsigned Ebofs::_apply_transaction(Transaction& t) case Transaction::OP_CLONERANGE: { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - pobject_t noid; - t.get_oid(noid); - __u64 off, len; - t.get_length(off); - t.get_length(len); + coll_t cid = t.get_cid(); + pobject_t oid = t.get_oid(); + pobject_t noid = t.get_oid(); + __u64 off = t.get_length(); + __u64 len = t.get_length(); if (_clone_range(oid, noid, off, len) < 0) { dout(7) << "apply_transaction fail on _clone_range" << dendl; r &= bit; @@ -2653,8 +2619,7 @@ unsigned Ebofs::_apply_transaction(Transaction& t) case Transaction::OP_MKCOLL: { - coll_t cid; - t.get_cid(cid); + coll_t cid = t.get_cid(); if (_create_collection(cid) < 0) { dout(7) << "apply_transaction fail on _create_collection" << dendl; r &= bit; @@ -2664,8 +2629,7 @@ unsigned Ebofs::_apply_transaction(Transaction& t) case Transaction::OP_RMCOLL: { - coll_t cid; - t.get_cid(cid); + coll_t cid = t.get_cid(); if (_destroy_collection(cid) < 0) { dout(7) << "apply_transaction fail on _destroy_collection" << dendl; r &= bit; @@ -2675,10 +2639,8 @@ unsigned Ebofs::_apply_transaction(Transaction& t) case Transaction::OP_COLL_ADD: { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); + coll_t cid = t.get_cid(); + pobject_t oid = t.get_oid(); if (_collection_add(cid, oid) < 0) { //dout(7) << "apply_transaction fail on _collection_add" << dendl; //r &= bit; @@ -2688,10 +2650,8 @@ unsigned Ebofs::_apply_transaction(Transaction& t) case Transaction::OP_COLL_REMOVE: { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); + coll_t cid = t.get_cid(); + pobject_t oid = t.get_oid(); if (_collection_remove(cid, oid) < 0) { dout(7) << "apply_transaction fail on _collection_remove" << dendl; r &= bit; @@ -2701,12 +2661,9 @@ unsigned Ebofs::_apply_transaction(Transaction& t) case Transaction::OP_COLL_SETATTR: { - coll_t cid; - t.get_cid(cid); - const char *attrname; - t.get_attrname(attrname); - bufferlist bl; - t.get_bl(bl); + coll_t cid = t.get_cid(); + const char *attrname = t.get_attrname(); + bufferlist& bl = t.get_bl(); if (_collection_setattr(cid, attrname, bl.c_str(), bl.length()) < 0) { //if (_collection_setattr(cid, attrname, attrval.first, attrval.second) < 0) { dout(7) << "apply_transaction fail on _collection_setattr" << dendl; @@ -2717,10 +2674,8 @@ unsigned Ebofs::_apply_transaction(Transaction& t) case Transaction::OP_COLL_RMATTR: { - coll_t cid; - t.get_cid(cid); - const char *attrname; - t.get_attrname(attrname); + coll_t cid = t.get_cid(); + const char *attrname = t.get_attrname(); if (_collection_rmattr(cid, attrname) < 0) { dout(7) << "apply_transaction fail on _collection_rmattr" << dendl; r &= bit; diff --git a/src/os/FileStore.cc b/src/os/FileStore.cc index be370cb525d32..81308f780772b 100644 --- a/src/os/FileStore.cc +++ b/src/os/FileStore.cc @@ -595,211 +595,88 @@ unsigned FileStore::_apply_transaction(Transaction& t) int op = t.get_op(); switch (op) { case Transaction::OP_TOUCH: - { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - _touch(cid, oid); - } + _touch(t.get_cid(), t.get_oid()); break; case Transaction::OP_WRITE: - { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - __u64 offset, len; - t.get_length(offset); - t.get_length(len); - bufferlist bl; - t.get_bl(bl); - _write(cid, oid, offset, len, bl); - } + _write(t.get_cid(), t.get_oid(), t.get_length(), t.get_length(), t.get_bl()); break; case Transaction::OP_ZERO: - { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - __u64 offset, len; - t.get_length(offset); - t.get_length(len); - _zero(cid, oid, offset, len); - } + _zero(t.get_cid(), t.get_oid(), t.get_length(), t.get_length()); break; case Transaction::OP_TRIMCACHE: + trim_from_cache(t.get_cid(), t.get_oid(), t.get_length(), t.get_length()); + break; + + case Transaction::OP_TRUNCATE: + _truncate(t.get_cid(), t.get_oid(), t.get_len()); + break; + + case Transaction::OP_REMOVE: + _remove(t.get_cid(), t.get_oid()); + break; + + case Transaction::OP_SETATTR: { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - __u64 offset, len; - t.get_length(offset); - t.get_length(len); - trim_from_cache(cid, oid, offset, len); - } - break; - - case Transaction::OP_TRUNCATE: - { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - __u64 len; - t.get_length(len); - _truncate(cid, oid, len); - } - break; - - case Transaction::OP_REMOVE: - { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - _remove(cid, oid); - } - break; - - case Transaction::OP_SETATTR: - { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - const char *attrname; - t.get_attrname(attrname); - bufferlist bl; - t.get_bl(bl); - _setattr(cid, oid, attrname, bl.c_str(), bl.length()); - } - break; - case Transaction::OP_SETATTRS: - { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - map *pattrset; - t.get_pattrset(pattrset); - _setattrs(cid, oid, *pattrset); - } - break; - - case Transaction::OP_RMATTR: - { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - const char *attrname; - t.get_attrname(attrname); - _rmattr(cid, oid, attrname); - } - break; - - case Transaction::OP_CLONE: - { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - pobject_t noid; - t.get_oid(noid); - _clone(cid, oid, noid); - } - break; + bufferlist& bl = t.get_bl(); + _setattr(t.get_cid(), t.get_oid(), t.get_attrname(), bl.c_str(), bl.length()); + } + break; + + case Transaction::OP_SETATTRS: + _setattrs(t.get_cid(), t.get_oid(), t.get_attrset()); + break; - case Transaction::OP_CLONERANGE: - { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - pobject_t noid; - t.get_oid(noid); - __u64 off, len; - t.get_length(off); - t.get_length(len); - _clone_range(cid, oid, noid, off, len); - } - break; + case Transaction::OP_RMATTR: + _rmattr(t.get_cid(), t.get_oid(), t.get_attrname()); + break; + + case Transaction::OP_CLONE: + _clone(t.get_cid(), t.get_oid(), t.get_oid()); + break; - case Transaction::OP_MKCOLL: - { - coll_t cid; - t.get_cid(cid); - _create_collection(cid); - } - break; + case Transaction::OP_CLONERANGE: + _clone_range(t.get_cid(), t.get_oid(), t.get_oid(), t.get_length(), t.get_length()); + break; - case Transaction::OP_RMCOLL: - { - coll_t cid; - t.get_cid(cid); - _destroy_collection(cid); - } - break; + case Transaction::OP_MKCOLL: + _create_collection(t.get_cid()); + break; - case Transaction::OP_COLL_ADD: - { - coll_t cid, ocid; - t.get_cid(cid); - t.get_cid(ocid); - pobject_t oid; - t.get_oid(oid); - _collection_add(cid, ocid, oid); - } - break; + case Transaction::OP_RMCOLL: + _destroy_collection(t.get_cid()); + break; - case Transaction::OP_COLL_REMOVE: - { - coll_t cid; - t.get_cid(cid); - pobject_t oid; - t.get_oid(oid); - _collection_remove(cid, oid); - } - break; + case Transaction::OP_COLL_ADD: + _collection_add(t.get_cid(), t.get_cid(), t.get_oid()); + break; - case Transaction::OP_COLL_SETATTR: - { - coll_t cid; - t.get_cid(cid); - const char *attrname; - t.get_attrname(attrname); - bufferlist bl; - t.get_bl(bl); - _collection_setattr(cid, attrname, bl.c_str(), bl.length()); - } - break; + case Transaction::OP_COLL_REMOVE: + _collection_remove(t.get_cid(), t.get_oid()); + break; - case Transaction::OP_COLL_RMATTR: - { - coll_t cid; - t.get_cid(cid); - const char *attrname; - t.get_attrname(attrname); - _collection_rmattr(cid, attrname); - } - break; + case Transaction::OP_COLL_SETATTR: + { + bufferlist& bl = t.get_bl(); + _collection_setattr(t.get_cid(), t.get_attrname(), bl.c_str(), bl.length()); + } + break; + case Transaction::OP_COLL_RMATTR: + _collection_rmattr(t.get_cid(), t.get_attrname()); + break; - default: - cerr << "bad op " << op << std::endl; - assert(0); - } + default: + cerr << "bad op " << op << std::endl; + assert(0); } - _transaction_finish(id); - - return 0; // FIXME count errors } + _transaction_finish(id); + + return 0; // FIXME count errors +} /*********************************************/ diff --git a/src/os/ObjectStore.h b/src/os/ObjectStore.h index f266923779823..14711e4ef7dea 100644 --- a/src/os/ObjectStore.h +++ b/src/os/ObjectStore.h @@ -111,8 +111,7 @@ public: // but, decode to a full value, and create pointers to that. vector attrnames; vector attrnames2; - vector *> attrsets; - vector > attrsets2; + vector > attrsets; unsigned opp, blp, oidp, cidp, lengthp, attrnamep, attrsetp; @@ -136,23 +135,23 @@ public: int get_op() { return ops[opp++]; } - void get_bl(bufferlist& bl) { - bl = bls[blp++]; + bufferlist &get_bl() { + return bls[blp++]; } - void get_oid(pobject_t& oid) { - oid = oids[oidp++]; + pobject_t get_oid() { + return oids[oidp++]; } - void get_cid(coll_t& cid) { - cid = cids[cidp++]; + coll_t get_cid() { + return cids[cidp++]; } - void get_length(__u64& len) { - len = lengths[lengthp++]; + __u64 get_length() { + return lengths[lengthp++]; } - void get_attrname(const char * &p) { - p = attrnames[attrnamep++]; + const char *get_attrname() { + return attrnames[attrnamep++]; } - void get_pattrset(map* &ps) { - ps = attrsets[attrsetp++]; + map& get_attrset() { + return attrsets[attrsetp++]; } void touch(coll_t cid, pobject_t oid) { @@ -231,11 +230,13 @@ public: blen++; } void setattrs(coll_t cid, pobject_t oid, map& attrset) { + map empty; int op = OP_SETATTRS; ops.push_back(op); cids.push_back(cid); oids.push_back(oid); - attrsets.push_back(&attrset); + attrsets.push_back(empty); + attrsets.back().swap(attrset); len++; blen += 5 + attrset.size(); // HACK allowance for removing old attrs } @@ -331,7 +332,7 @@ public: int op = OP_COLL_SETATTRS; ops.push_back(op); cids.push_back(cid); - attrsets.push_back(&aset); + attrsets.push_back(aset); len++; blen += 5 + aset.size(); } @@ -372,11 +373,7 @@ public: p != attrnames2.end(); ++p) attrnames.push_back((*p).c_str()); - ::decode(attrsets2, bl); - for (vector >::iterator p = attrsets2.begin(); - p != attrsets2.end(); - ++p) - attrsets.push_back(&(*p)); + ::decode(attrsets, bl); } }; -- 2.39.5