Merge pull request #337 from dalgaaf/wip-da-CID-medium.v2

Fix several medium impact CID issues V2

Reviewed-by: Sage Weil <sage@inktank.com>

7 files changed, 65 insertions, 22 deletions

diff --git a/.gitignore b/.gitignore
index a28d5158e05..6dc8d49b3ae 100644
--- a/.gitignore
+++ b/.gitignore
@@ -66,3 +66,6 @@ m4/
 # where is this from?
 web/*.html
 
+# dir from coverity tools
+cov-int/
+
diff --git a/src/mds/Locker.cc b/src/mds/Locker.cc
index 57154b3d9f6..74a766256b4 100644
--- a/src/mds/Locker.cc
+++ b/src/mds/Locker.cc
@@ -615,6 +615,7 @@ void Locker::eval_gather(SimpleLock *lock, bool first, bool *pneed_issue, list<C
   bool need_issue = false;
 
   int loner_issued = 0, other_issued = 0, xlocker_issued = 0;
+  assert(!caps || in != NULL);
   if (caps && in->is_head()) {
     in->get_caps_issued(&loner_issued, &other_issued, &xlocker_issued,
 			lock->get_cap_shift(), lock->get_cap_mask());
@@ -2793,7 +2794,8 @@ bool Locker::_do_cap_update(CInode *in, Capability *cap,
       dout(10) << " i want to change file_max, but lock won't allow it (yet)" << dendl;
       if (in->filelock.is_stable()) {
 	bool need_issue = false;
-	cap->inc_suppress();
+	if (cap)
+	  cap->inc_suppress();
 	if (in->mds_caps_wanted.empty() &&
 	    (in->get_loner() >= 0 || (in->get_wanted_loner() >= 0 && in->try_set_loner()))) {
 	  if (in->filelock.get_state() != LOCK_EXCL)
@@ -2802,7 +2804,8 @@ bool Locker::_do_cap_update(CInode *in, Capability *cap,
 	  simple_lock(&in->filelock, &need_issue);
 	if (need_issue)
 	  issue_caps(in);
-	cap->dec_suppress();
+	if (cap)
+	  cap->dec_suppress();
       }
       if (!in->filelock.can_wrlock(client) &&
 	  !in->filelock.can_force_wrlock(client)) {
@@ -3463,11 +3466,13 @@ bool Locker::simple_sync(SimpleLock *lock, bool *need_issue)
       }
     }
     
-    if (lock->get_type() == CEPH_LOCK_IFILE &&
-	in->state_test(CInode::STATE_NEEDSRECOVER)) {
-      mds->mdcache->queue_file_recover(in);
-      mds->mdcache->do_file_recover();
-      gather++;
+    if (lock->get_type() == CEPH_LOCK_IFILE) {
+      assert(in);
+      if (in->state_test(CInode::STATE_NEEDSRECOVER)) {
+        mds->mdcache->queue_file_recover(in);
+        mds->mdcache->do_file_recover();
+        gather++;
+      }
     }
     
     if (!gather && lock->is_dirty()) {
@@ -3601,11 +3606,13 @@ void Locker::simple_lock(SimpleLock *lock, bool *need_issue)
     }
   }
 
-  if (lock->get_type() == CEPH_LOCK_IFILE &&
-      in->state_test(CInode::STATE_NEEDSRECOVER)) {
-    mds->mdcache->queue_file_recover(in);
-    mds->mdcache->do_file_recover();
-    gather++;
+  if (lock->get_type() == CEPH_LOCK_IFILE) {
+    assert(in);
+    if(in->state_test(CInode::STATE_NEEDSRECOVER)) {
+      mds->mdcache->queue_file_recover(in);
+      mds->mdcache->do_file_recover();
+      gather++;
+    }
   }
 
   if (lock->get_parent()->is_replicated() &&
diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc
index 0c279b66a91..8c17172e5c4 100644
--- a/src/mds/MDCache.cc
+++ b/src/mds/MDCache.cc
@@ -496,6 +496,7 @@ void MDCache::_create_system_file_finish(Mutation *mut, CDentry *dn, version_t d
   CDir *dir = 0;
   if (in->inode.is_dir()) {
     dir = in->get_dirfrag(frag_t());
+    assert(dir);
     dir->mark_dirty(1, mut->ls);
     dir->mark_new(mut->ls);
   }
@@ -2126,6 +2127,7 @@ void MDCache::predirty_journal_parents(Mutation *mut, EMetaBlob *blob,
   }
 
   // now, stick it in the blob
+  assert(parent);
   assert(parent->is_auth());
   blob->add_dir_context(parent);
   blob->add_dir(parent, true);
@@ -4011,6 +4013,7 @@ void MDCache::handle_cache_rejoin_weak(MMDSCacheRejoin *weak)
 	 p != weak->inode_scatterlocks.end();
 	 ++p) {
       CInode *in = get_inode(p->first);
+      assert(in);
       dout(10) << " including base inode (due to potential scatterlock update) " << *in << dendl;
       acked_inodes.insert(in->vino());
       ack->add_inode_base(in);
@@ -6625,9 +6628,10 @@ void MDCache::handle_cache_expire(MCacheExpire *m)
 	} else {
 	  // which dirfrag for this dentry?
 	  CDir *dir = diri->get_dirfrag(diri->pick_dirfrag(p->first.first));
+	  assert(dir); 
 	  assert(dir->is_auth());
 	  dn = dir->lookup(p->first.first, p->first.second);
-	} 
+	}
 
 	if (!dn) 
 	  dout(0) << "  missing dentry for " << p->first.first << " snap " << p->first.second << " in " << *dir << dendl;
diff --git a/src/mds/Migrator.cc b/src/mds/Migrator.cc
index 92962424e46..59d7d8ebfb8 100644
--- a/src/mds/Migrator.cc
+++ b/src/mds/Migrator.cc
@@ -414,11 +414,13 @@ void Migrator::handle_mds_failure_or_stop(int who)
 	break;
 
       case IMPORT_LOGGINGSTART:
+	assert(dir);
 	dout(10) << "import state=loggingstart : reversing import on " << *dir << dendl;
 	import_reverse(dir);
 	break;
 
       case IMPORT_ACKING:
+	assert(dir);
 	// hrm.  make this an ambiguous import, and wait for exporter recovery to disambiguate
 	dout(10) << "import state=acking : noting ambiguous import " << *dir << dendl;
 	{
@@ -429,6 +431,7 @@ void Migrator::handle_mds_failure_or_stop(int who)
 	break;
 	
       case IMPORT_ABORTING:
+	assert(dir);
 	dout(10) << "import state=aborting : ignoring repeat failure " << *dir << dendl;
 	break;
       }
@@ -2518,6 +2521,7 @@ int Migrator::decode_import_dir(bufferlist::iterator& blp,
     }
     else if (icode == 'I') {
       // inode
+      assert(le);
       decode_import_inode(dn, blp, oldauth, ls, le->get_start_off(), cap_imports, updated_scatterlocks);
     }
     
diff --git a/src/mds/Server.cc b/src/mds/Server.cc
index 98dafc3e285..abeea2c12d5 100644
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -859,6 +859,7 @@ void Server::early_reply(MDRequest *mdr, CInode *tracei, CDentry *tracedn)
  */
 void Server::reply_request(MDRequest *mdr, MClientReply *reply, CInode *tracei, CDentry *tracedn) 
 {
+  assert(mdr);
   MClientRequest *req = mdr->client_request;
   
   char buf[80];
@@ -1115,6 +1116,7 @@ void Server::handle_client_request(MClientRequest *req)
   // trim completed_request list
   if (req->get_oldest_client_tid() > 0) {
     dout(15) << " oldest_client_tid=" << req->get_oldest_client_tid() << dendl;
+    assert(session);
     session->trim_completed_requests(req->get_oldest_client_tid());
   }
 
@@ -1927,12 +1929,14 @@ void Server::apply_allocated_inos(MDRequest *mdr)
     mds->inotable->apply_alloc_id(mdr->alloc_ino);
   }
   if (mdr->prealloc_inos.size()) {
+    assert(session);
     session->pending_prealloc_inos.subtract(mdr->prealloc_inos);
     session->info.prealloc_inos.insert(mdr->prealloc_inos);
     mds->sessionmap.version++;
     mds->inotable->apply_alloc_ids(mdr->prealloc_inos);
   }
   if (mdr->used_prealloc_ino) {
+    assert(session);
     session->info.used_inos.erase(mdr->used_prealloc_ino);
     mds->sessionmap.version++;
   }
@@ -5088,8 +5092,10 @@ void Server::_unlink_local(MDRequest *mdr, CDentry *dn, CDentry *straydn)
 
   dn->push_projected_linkage();
 
-  if (in->is_dir())
+  if (in->is_dir()) {
+    assert(straydn);
     mds->mdcache->project_subtree_rename(in, dn->get_dir(), straydn->get_dir());
+  }
 
   journal_and_reply(mdr, 0, dn, le, new C_MDS_unlink_local_finish(mds, mdr, dn, straydn));
 }
@@ -7439,8 +7445,7 @@ void Server::handle_client_rmsnap(MDRequest *mdr)
     reply_request(mdr, -EINVAL);   // can't prune a parent snap, currently.
     return;
   }
-  if (diri->snaprealm &&
-      !diri->snaprealm->exists(snapname)) {
+  if (!diri->snaprealm || !diri->snaprealm->exists(snapname)) {
     reply_request(mdr, -ENOENT);
     return;
   }
diff --git a/src/test/bench/dumb_backend.cc b/src/test/bench/dumb_backend.cc
index a763c45f0c8..c36dce392da 100644
--- a/src/test/bench/dumb_backend.cc
+++ b/src/test/bench/dumb_backend.cc
@@ -22,7 +22,14 @@ void DumbBackend::_write(
     std::cout << full_path << ": errno is " << errno << std::endl;
     assert(0);
   }
-  ::lseek(fd, offset, SEEK_SET);
+
+  int r =  ::lseek(fd, offset, SEEK_SET);
+  if (r < 0) {
+    r = errno;
+    std::cout << "lseek failed, errno is: " << r << std::endl;
+    ::close(fd);
+    return;
+  }
   bl.write_fd(fd);
   on_applied->complete(0);
   if (do_fsync)
@@ -30,9 +37,12 @@ void DumbBackend::_write(
   if (do_sync_file_range)
     ::sync_file_range(fd, offset, bl.length(),
 		      SYNC_FILE_RANGE_WAIT_AFTER);
-  if (do_fadvise)
-    ::posix_fadvise(fd, offset, bl.length(),
-		    POSIX_FADV_DONTNEED);
+  if (do_fadvise) {
+    int fa_r = ::posix_fadvise(fd, offset, bl.length(), POSIX_FADV_DONTNEED);
+    if (fa_r) {
+        std::cout << "posix_fadvise failed, errno is: " << fa_r << std::endl;
+    }
+  }
   ::close(fd);
   {
     Mutex::Locker l(pending_commit_mutex);
@@ -55,6 +65,8 @@ void DumbBackend::read(
 
   int r = ::lseek(fd, offset, SEEK_SET);
   if (r < 0) {
+    r = errno;
+    std::cout << "lseek failed, errno is: " << r << std::endl;
     ::close(fd);
     return;
   }
diff --git a/src/test/bench/small_io_bench_fs.cc b/src/test/bench/small_io_bench_fs.cc
index 4a3adc9e5dd..61fbacc5570 100644
--- a/src/test/bench/small_io_bench_fs.cc
+++ b/src/test/bench/small_io_bench_fs.cc
@@ -125,8 +125,16 @@ int main(int argc, char **argv)
 
   FileStore fs(vm["filestore-path"].as<string>(),
 	       vm["journal-path"].as<string>());
-  fs.mkfs();
-  fs.mount();
+
+  if (fs.mkfs() < 0) {
+    cout << "mkfs failed" << std::endl;
+    return 1;
+  }
+
+  if (fs.mount() < 0) {
+    cout << "mount failed" << std::endl;
+    return 1;
+  }
 
   ostream *detailed_ops = 0;
   ofstream myfile;