[Pkg-ceph-commits] [ceph] 01/01: New "bug-9814.patch" to prevent OSD crash. Thanks, Haomai Wang.

[Dmitry Smirnov]

This is an automated email from the git hooks/post-receive script.

onlyjob pushed a commit to branch experimental
in repository ceph.

commit 7c844b1 (experimental)
Author: Dmitry Smirnov <onlyjob at member.fsf.org>
Date:   Mon Oct 20 07:34:19 2014

    New "bug-9814.patch" to prevent OSD crash. Thanks, Haomai Wang.
---
 debian/patches/bug-9814.patch | 141 ++++++++++++++++++++++++++++++++++++++++++
 debian/patches/series         |   1 +
 2 files changed, 142 insertions(+)

diff --git a/debian/patches/bug-9814.patch b/debian/patches/bug-9814.patch
new file mode 100644
index 0000000..f839118
--- /dev/null
+++ b/debian/patches/bug-9814.patch
@@ -0,0 +1,141 @@
+Last-update: 2014-10-20
+Forwarded: not-needed
+Origin: upstream, https://github.com/ceph/ceph/pull/2710
+Bug-Ceph: http://tracker.ceph.com/issues/9814
+Author: Haomai Wang <haomaiwang at gmail.com>
+Description: fixes OSD crash
+ FAILED assert(0) In function 'GenericObjectMap::Header GenericObjectMap::lookup_parent(GenericObje ctMap::Header)'
+
+ Clone will set parent header and rm_keys wll lookup_parent which will try
+ to find parent header. So it will let lookup_parent fail when "clone" and
+ "rm_keys" in one transaction. Here have to sync transaction to make
+ visiable for lookup_parent
+
+--- a/src/os/KeyValueStore.cc
++++ b/src/os/KeyValueStore.cc
+@@ -1172,8 +1172,9 @@
+   dout(10) << "_do_transaction on " << &transaction << dendl;
+ 
+   Transaction::iterator i = transaction.begin();
+   uint64_t op_num = 0;
++  bool exist_clone = false;
+ 
+   while (i.have_op()) {
+     if (handle)
+       handle->reset_tp_timeout();
+@@ -1292,8 +1293,9 @@
+       {
+         coll_t cid = i.decode_cid();
+         ghobject_t oid = i.decode_oid();
+         ghobject_t noid = i.decode_oid();
++        exist_clone = true;
+         r = _clone(cid, oid, noid, t);
+       }
+       break;
+ 
+@@ -1303,8 +1305,9 @@
+         ghobject_t oid = i.decode_oid();
+         ghobject_t noid = i.decode_oid();
+         uint64_t off = i.decode_length();
+         uint64_t len = i.decode_length();
++        exist_clone = true;
+         r = _clone_range(cid, oid, noid, off, len, off, t);
+       }
+       break;
+ 
+@@ -1315,8 +1318,9 @@
+         ghobject_t noid = i.decode_oid();
+         uint64_t srcoff = i.decode_length();
+         uint64_t len = i.decode_length();
+         uint64_t dstoff = i.decode_length();
++        exist_clone = true;
+         r = _clone_range(cid, oid, noid, srcoff, len, dstoff, t);
+       }
+       break;
+ 
+@@ -1516,12 +1520,11 @@
+ 
+       if (!ok) {
+         const char *msg = "unexpected error code";
+ 
+-        if (r == -ENOENT && (op == Transaction::OP_CLONERANGE ||
+-                            op == Transaction::OP_CLONE ||
+-                            op == Transaction::OP_CLONERANGE2))
+-          msg = "ENOENT on clone suggests osd bug";
++        if (exist_clone) {
++          dout(0) << "BUG: clone failed will lead to paritial transaction applied" << dendl;
++        }
+ 
+         if (r == -ENOSPC)
+           // For now, if we hit _any_ ENOSPC, crash, before we do any damage
+           // by partially applying transactions.
+--- a/src/os/GenericObjectMap.cc
++++ b/src/os/GenericObjectMap.cc
+@@ -672,8 +672,15 @@
+   if (new_header)
+     *old_header = source;
+   if (new_header)
+     *new_header = destination;
++
++  // Clone will set parent header and rm_keys wll lookup_parent which will try
++  // to find parent header. So it will let lookup_parent fail when "clone" and
++  // "rm_keys" in one transaction. Here have to sync transaction to make
++  // visiable for lookup_parent
++  int r = submit_transaction_sync(t);
++  assert(r == 0);
+ }
+ 
+ void GenericObjectMap::rename(const Header old_header, const coll_t &cid,
+                               const ghobject_t &target,
+--- a/src/test/objectstore/store_test.cc
++++ b/src/test/objectstore/store_test.cc
+@@ -216,8 +216,49 @@
+     ASSERT_EQ(r, 0);
+   }
+ }
+ 
++TEST_P(StoreTest, SimpleCloneTest) {
++  int r;
++  coll_t cid = coll_t("coll");
++  {
++    ObjectStore::Transaction t;
++    t.create_collection(cid);
++    cerr << "Creating collection " << cid << std::endl;
++    r = store->apply_transaction(t);
++    ASSERT_EQ(r, 0);
++  }
++  ghobject_t hoid(hobject_t(sobject_t("Object 1", CEPH_NOSNAP)));
++  bufferlist small;
++  small.append("small");
++  {
++    ObjectStore::Transaction t;
++    t.touch(cid, hoid);
++    t.setattr(cid, hoid, "attr1", small);
++    cerr << "Creating object and set attr " << hoid << std::endl;
++    r = store->apply_transaction(t);
++    ASSERT_EQ(r, 0);
++  }
++  ghobject_t hoid2(hobject_t(sobject_t("Object 2", CEPH_NOSNAP)));
++  {
++    ObjectStore::Transaction t;
++    t.clone(cid, hoid, hoid2);
++    t.rmattr(cid, hoid, "attr1");
++    cerr << "Clone object and rm attr" << std::endl;
++    r = store->apply_transaction(t);
++    ASSERT_EQ(r, 0);
++  }
++  {
++    ObjectStore::Transaction t;
++    t.remove(cid, hoid);
++    t.remove(cid, hoid2);
++    t.remove_collection(cid);
++    cerr << "Cleaning" << std::endl;
++    r = store->apply_transaction(t);
++    ASSERT_EQ(r, 0);
++  }
++}
++
+ TEST_P(StoreTest, SimpleObjectLongnameTest) {
+   int r;
+   coll_t cid = coll_t("coll");
+   {
diff --git a/debian/patches/series b/debian/patches/series
index 638e800..6098e34 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,5 +1,6 @@
 ## Backported / Upstream
 bug-9341.patch
+bug-9814.patch
 sleep-recover.patch
 
 ## Debian

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-ceph/ceph.git