Merge pull request #614 from ceph/wip-rados-model

Prevent copy_from against yourself, and fix issues with the testing framework around it

Reviewed-by: Greg Farnum <greg@inktank.com>

3 files changed, 17 insertions, 11 deletions

diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc
index a92403ae370..a48372fe561 100644
--- a/src/osd/ReplicatedPG.cc
+++ b/src/osd/ReplicatedPG.cc
@@ -3532,6 +3532,11 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
 	  hobject_t src(src_name, src_oloc.key, src_snapid,
 			raw_pg.ps(), raw_pg.pool(),
 			src_oloc.nspace);
+	  if (src == soid) {
+	    dout(20) << " copy from self is invalid" << dendl;
+	    result = -EINVAL;
+	    break;
+	  }
 	  result = start_copy(ctx, src, src_oloc, src_version, &ctx->copy_op);
 	  if (result < 0)
 	    goto fail;
diff --git a/src/test/osd/RadosModel.h b/src/test/osd/RadosModel.h
index 3a73ac33faf..ab0b13d73e4 100644
--- a/src/test/osd/RadosModel.h
+++ b/src/test/osd/RadosModel.h
@@ -792,8 +792,6 @@ public:
     context->oid_in_use.insert(oid);
     context->oid_not_in_use.erase(oid);
 
-    context->seq_num++;
-
     vector<uint64_t> snapset(context->snaps.size());
     int j = 0;
     for (map<int,uint64_t>::reverse_iterator i = context->snaps.rbegin();
@@ -804,6 +802,7 @@ public:
     interval_set<uint64_t> ranges;
     context->cont_gen.get_ranges(cont, ranges);
     std::cout << num << ":  seq_num " << context->seq_num << " ranges " << ranges << std::endl;
+    context->seq_num++;
     context->state_lock.Unlock();
 
     int r = context->io_ctx.selfmanaged_snap_set_write_ctx(context->seq, snapset);
@@ -1081,8 +1080,8 @@ public:
 	  context->errors++;
 	}
 	if (to_check != old_value.most_recent()) {
-	  cerr << num << ": Found incorrect object contents " << to_check
-	       << ", expected " << old_value.most_recent() << " oid " << oid << std::endl;
+	  cerr << num << ": oid " << oid << " found incorrect object contents " << to_check
+	       << ", expected " << old_value.most_recent() << std::endl;
 	  context->errors++;
 	}
 	if (!old_value.check(result)) {
@@ -1483,9 +1482,6 @@ public:
 	context->update_object_full(oid, src_value);
 	context->update_object_version(oid, comp->get_version64());
       }
-      context->oid_in_use.erase(oid_src);
-      context->oid_not_in_use.insert(oid_src);
-      context->kick();
     } else if (info->id == 1) {
       // racing read
       assert(comp_racing_read->is_complete());
@@ -1500,11 +1496,14 @@ public:
 	assert(!version || comp_racing_read->get_version64() == version);
 	version = comp_racing_read->get_version64();
       }
+    }
+    if (++done == 2) {
       context->oid_in_use.erase(oid);
       context->oid_not_in_use.insert(oid);
+      context->oid_in_use.erase(oid_src);
+      context->oid_not_in_use.insert(oid_src);
       context->kick();
     }
-    ++done;
   }
 
   bool finished()
diff --git a/src/test/osd/TestRados.cc b/src/test/osd/TestRados.cc
index 1b6bd073a12..be919161579 100644
--- a/src/test/osd/TestRados.cc
+++ b/src/test/osd/TestRados.cc
@@ -155,7 +155,9 @@ private:
 
     case TEST_OP_COPY_FROM:
       oid = *(rand_choose(context.oid_not_in_use));
-      oid2 = *(rand_choose(context.oid_not_in_use));
+      do {
+	oid2 = *(rand_choose(context.oid_not_in_use));
+      } while (oid == oid2);
       cout << "copy_from oid " << oid << " from oid " << oid2
 	   << " current snap is " << context.current_snap << std::endl;
       return new CopyFromOp(m_op, &context, oid, oid2, m_stats);
@@ -282,8 +284,8 @@ int main(int argc, char **argv)
     return 1;
   }
 
-  if (max_in_flight > objects) {
-    cerr << "Error: max_in_flight must be less than the number of objects"
+  if (max_in_flight * 2 > objects) {
+    cerr << "Error: max_in_flight must be <= than the number of objects / 2"
 	 << std::endl;
     return 1;
   }