common/obj_bencher.cc: fix verification crashing when there's no objects

When specified pool is empty or doesn't contain valid bench objects, rados
can crash because it attempts to memcmp through NULL pointer. Added check
for valid object size (which also fixes the case when objects are there, but
they're truncated), and made object_size being size_t instead of int.
In seq_read_bench, there's a missing lock.Lock() which causes rados to trigger
assert when object reading fails. Moved lock.Lock() before completion_ret()
call.

Signed-off-by: Piotr Dałek <piotr.dalek@ts.fujitsu.com>

diff --git a/src/common/obj_bencher.cc b/src/common/obj_bencher.cc
index 3772eb865781fa97fe285266c3371fe5e3d5d452..9ecfbec4d31d42897ddda61f3bdab8379509f213 100644 (file)
--- a/src/common/obj_bencher.cc
+++ b/src/common/obj_bencher.cc
@@ -52,7 +52,7 @@ static std::string generate_object_name(int objnum, int pid = 0)
   return oss.str();
 }
 
-static void sanitize_object_contents (bench_data *data, int length) {
+static void sanitize_object_contents (bench_data *data, size_t length) {
   memset(data->object_contents, 'z', length);
 }
 
@@ -204,7 +204,7 @@ void *ObjBencher::status_printer(void *_bencher) {
 
 int ObjBencher::aio_bench(
   int operation, int secondsToRun,
-  int concurrentios, int object_size, bool cleanup, const std::string& run_name, bool no_verify) {
+  int concurrentios, size_t object_size, bool cleanup, const std::string& run_name, bool no_verify) {
 
   if (concurrentios <= 0) 
     return -EINVAL;
@@ -322,11 +322,11 @@ static T vec_stddev(vector<T>& v)
   return sqrt(stddev);
 }
 
-int ObjBencher::fetch_bench_metadata(const std::string& metadata_file, int* object_size, int* num_objects, int* prevPid) {
+int ObjBencher::fetch_bench_metadata(const std::string& metadata_file, size_t* object_size, int* num_objects, int* prevPid) {
   int r = 0;
   bufferlist object_data;
 
-  r = sync_read(metadata_file, object_data, sizeof(int)*3);
+  r = sync_read(metadata_file, object_data, sizeof(int) * 2 + sizeof(size_t));
   if (r <= 0) {
     // treat an empty file as a file that does not exist
     if (r == 0) {
@@ -666,7 +666,8 @@ int ObjBencher::seq_read_bench(int seconds_to_run, int num_objects, int concurre
   
     if (!no_verify) {
       snprintf(data.object_contents, data.object_size, "I'm the %16dth object!", current_index);
-      if (memcmp(data.object_contents, cur_contents->c_str(), data.object_size) != 0) {
+      if ( (cur_contents->length() != data.object_size) || 
+           (memcmp(data.object_contents, cur_contents->c_str(), data.object_size) != 0) ) {
         cerr << name[slot] << " is not correct!" << std::endl;
         ++errors;
       }
@@ -676,13 +677,13 @@ int ObjBencher::seq_read_bench(int seconds_to_run, int num_objects, int concurre
     index[slot] = data.started;
     lock.Unlock();
     completion_wait(slot);
+    lock.Lock();
     r = completion_ret(slot);
     if (r < 0) {
       cerr << "read got " << r << std::endl;
       lock.Unlock();
       goto ERR;
     }
-    lock.Lock();
     total_latency += data.cur_latency;
     if (data.cur_latency > data.max_latency) data.max_latency = data.cur_latency;
     if (data.cur_latency < data.min_latency) data.min_latency = data.cur_latency;
@@ -728,7 +729,8 @@ int ObjBencher::seq_read_bench(int seconds_to_run, int num_objects, int concurre
     if (!no_verify) {
       snprintf(data.object_contents, data.object_size, "I'm the %16dth object!", index[slot]);
       lock.Unlock();
-      if (memcmp(data.object_contents, contents[slot]->c_str(), data.object_size) != 0) {
+      if ((contents[slot]->length() != data.object_size) || 
+         (memcmp(data.object_contents, contents[slot]->c_str(), data.object_size) != 0)) {
         cerr << name[slot] << " is not correct!" << std::endl;
         ++errors;
       }
@@ -899,7 +901,8 @@ int ObjBencher::rand_read_bench(int seconds_to_run, int num_objects, int concurr
     
     if (!no_verify) {
       snprintf(data.object_contents, data.object_size, "I'm the %16dth object!", current_index);
-      if (memcmp(data.object_contents, cur_contents->c_str(), data.object_size) != 0) {
+      if ((cur_contents->length() != data.object_size) || 
+          (memcmp(data.object_contents, cur_contents->c_str(), data.object_size) != 0)) {
         cerr << name[slot] << " is not correct!" << std::endl;
         ++errors;
       }
@@ -950,7 +953,8 @@ int ObjBencher::rand_read_bench(int seconds_to_run, int num_objects, int concurr
     if (!no_verify) {
       snprintf(data.object_contents, data.object_size, "I'm the %16dth object!", index[slot]);
       lock.Unlock();
-      if (memcmp(data.object_contents, contents[slot]->c_str(), data.object_size) != 0) {
+      if ((contents[slot]->length() != data.object_size) || 
+          (memcmp(data.object_contents, contents[slot]->c_str(), data.object_size) != 0)) {
         cerr << name[slot] << " is not correct!" << std::endl;
         ++errors;
       }
@@ -1010,7 +1014,7 @@ int ObjBencher::rand_read_bench(int seconds_to_run, int num_objects, int concurr
 
 int ObjBencher::clean_up(const std::string& prefix, int concurrentios, const std::string& run_name) {
   int r = 0;
-  int object_size;
+  size_t object_size;
   int num_objects;
   int prevPid;
 

diff --git a/src/common/obj_bencher.h b/src/common/obj_bencher.h
index 34e22c2b599a5e19b369043132e0d60a94d81a78..1c9681583d75b1749f9974fe9a9f6c9a2267f9f1 100644 (file)
--- a/src/common/obj_bencher.h
+++ b/src/common/obj_bencher.h
@@ -36,7 +36,7 @@ struct bench_history {
 
 struct bench_data {
   bool done; //is the benchmark is done
-  int object_size; //the size of the objects
+  size_t object_size; //the size of the objects
   // same as object_size for write tests
   int in_flight; //number of reads/writes being waited on
   int started;
@@ -71,7 +71,7 @@ protected:
 
   struct bench_data data;
 
-  int fetch_bench_metadata(const std::string& metadata_file, int* object_size, int* num_objects, int* prevPid);
+  int fetch_bench_metadata(const std::string& metadata_file, size_t* object_size, int* num_objects, int* prevPid);
 
   int write_bench(int secondsToRun, int concurrentios, const string& run_name_meta);
   int seq_read_bench(int secondsToRun, int num_objects, int concurrentios, int writePid, bool no_verify=false);
@@ -107,7 +107,7 @@ public:
   virtual ~ObjBencher() {}
   int aio_bench(
     int operation, int secondsToRun,
-    int concurrentios, int op_size, bool cleanup, const std::string& run_name, bool no_verify=false);
+    int concurrentios, size_t op_size, bool cleanup, const std::string& run_name, bool no_verify=false);
   int clean_up(const std::string& prefix, int concurrentios, const std::string& run_name);
 
   void set_show_time(bool dt) {