librbd: Make metadata_set support multi kv pairs

Signed-off-by: Haomai Wang <haomaiwang@gmail.com>

diff --git a/src/cls/rbd/cls_rbd.cc b/src/cls/rbd/cls_rbd.cc
index 413b066bc22b4d64501768c162663fd567f235fb..11b1685c1df55a9cb8d321760ca4f27890481a01 100644 (file)
--- a/src/cls/rbd/cls_rbd.cc
+++ b/src/cls/rbd/cls_rbd.cc
@@ -2136,10 +2136,9 @@ static const string metadata_name_from_key(const string &key)
  */
 int metadata_list(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
 {
-  map<string, bufferlist> vals;
-  map<string, string> out_map;
+  map<string, bufferlist> raw_data, data;
   int r = cls_cxx_map_get_vals(hctx, RBD_METADATA_KEY_PREFIX, RBD_METADATA_KEY_PREFIX,
-                               RBD_MAX_METAADATA_KEYS, &vals);
+                               RBD_MAX_METAADATA_KEYS, &raw_data);
   if (r < 0) {
     CLS_ERR("failed to read the vals off of disk: %s", cpp_strerror(r).c_str());
     return r;
@@ -2147,49 +2146,39 @@ int metadata_list(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
 
   bufferlist::iterator iter;
   string v;
-  for (map<string, bufferlist>::iterator it = vals.begin();
-       it != vals.end(); ++it) {
-    iter = it->second.begin();
-    try {
-      ::decode(v, iter);
-    } catch (const buffer::error &err) {
-      CLS_ERR("failed to decode the vals off of disk");
-      return -EINVAL;
-    }
-
+  for (map<string, bufferlist>::iterator it = raw_data.begin(); it != raw_data.end(); ++it) {
     assert(it->first.find(RBD_METADATA_KEY_PREFIX, 0) == 0);
-    out_map[metadata_name_from_key(it->first)] = v;
+    data[metadata_name_from_key(it->first)].swap(it->second);
   }
 
-  ::encode(out_map, *out);
+  ::encode(data, *out);
   return 0;
 }
 
 /**
  * Input:
- * @param key
- * @param value
+ * @param data <map(key, value)>
  *
  * Output:
  * @returns 0 on success, negative error code on failure
  */
 int metadata_set(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
 {
-  string key, value;
+  map<string, bufferlist> data, raw_data;
 
   bufferlist::iterator iter = in->begin();
   try {
-    ::decode(key, iter);
-    ::decode(value, iter);
+    ::decode(data, iter);
   } catch (const buffer::error &err) {
     return -EINVAL;
   }
 
-  CLS_LOG(20, "metdata_set key=%s value=%s", key.c_str(), value.c_str());
-
-  bufferlist bl;
-  ::encode(value, bl);
-  int r = cls_cxx_map_set_val(hctx, metadata_key_for_name(key), &bl);
+  for (map<string, bufferlist>::iterator it = data.begin();
+       it != data.end(); ++it) {
+    CLS_LOG(20, "metdata_set key=%s value=%s", it->first.c_str(), it->second.c_str());
+    raw_data[metadata_key_for_name(it->first)].swap(it->second);
+  }
+  int r = cls_cxx_map_set_vals(hctx, &raw_data);
   if (r < 0) {
     CLS_ERR("error writing metadata: %d", r);
     return r;
@@ -2237,7 +2226,8 @@ int metadata_remove(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
  */
 int metadata_get(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
 {
-  string key, value;
+  string key;
+  bufferlist value;
 
   bufferlist::iterator iter = in->begin();
   try {
@@ -2248,7 +2238,7 @@ int metadata_get(cls_method_context_t hctx, bufferlist *in, bufferlist *out)
 
   CLS_LOG(20, "metdata_get key=%s", key.c_str());
 
-  int r = read_key(hctx, metadata_key_for_name(key), &value);
+  int r = cls_cxx_map_get_val(hctx, metadata_key_for_name(key), &value);
   if (r < 0) {
     CLS_ERR("error get metadata: %d", r);
     return r;

diff --git a/src/cls/rbd/cls_rbd_client.cc b/src/cls/rbd/cls_rbd_client.cc
index 77864e777bbae26a9999c252978bb5bffab0058e..9a3561ca522d5d1878f59a251e30eff7ef992731 100644 (file)
--- a/src/cls/rbd/cls_rbd_client.cc
+++ b/src/cls/rbd/cls_rbd_client.cc
@@ -750,10 +750,9 @@ namespace librbd {
     }
 
     int metadata_set(librados::IoCtx *ioctx, const std::string &oid,
-                     const std::string &key, const std::string &data)
+                     const map<string, bufferlist> &data)
     {
       bufferlist in;
-      ::encode(key, in);
       ::encode(data, in);
       bufferlist out;
       return ioctx->exec(oid, "rbd", "metadata_set", in, out);
@@ -769,7 +768,7 @@ namespace librbd {
     }
 
     int metadata_list(librados::IoCtx *ioctx, const std::string &oid,
-                     map<string, string> *pairs)
+                     map<string, bufferlist> *pairs)
     {
       assert(pairs);
       bufferlist in, out;

diff --git a/src/cls/rbd/cls_rbd_client.h b/src/cls/rbd/cls_rbd_client.h
index 174a2aa5b07c8285f8f5e531296c7e98f1b27b94..3ab7b8f70bbe5c0d740413ed06289e1843ad40d2 100644 (file)
--- a/src/cls/rbd/cls_rbd_client.h
+++ b/src/cls/rbd/cls_rbd_client.h
@@ -88,9 +88,9 @@ namespace librbd {
     int set_stripe_unit_count(librados::IoCtx *ioctx, const std::string &oid,
                              uint64_t stripe_unit, uint64_t stripe_count);
     int metadata_list(librados::IoCtx *ioctx, const std::string &oid,
-                      map<string, string> *pairs);
+                      map<string, bufferlist> *pairs);
     int metadata_set(librados::IoCtx *ioctx, const std::string &oid,
-                     const std::string &key, const std::string &data);
+                     const map<std::string, bufferlist> &data);
     int metadata_remove(librados::IoCtx *ioctx, const std::string &oid,
                         const std::string &key);
     int metadata_get(librados::IoCtx *ioctx, const std::string &oid,

diff --git a/src/include/rbd/librbd.hpp b/src/include/rbd/librbd.hpp
index 8beeb9659e300881988597f3b1584594f25aa2bc..6e5df6d182c5508be7260a7e68da21ef79d3934d 100644 (file)
--- a/src/include/rbd/librbd.hpp
+++ b/src/include/rbd/librbd.hpp
@@ -247,7 +247,7 @@ public:
   /**
    * Returns a pair of key/value for this image
    */
-  int metadata_list(std::map<std::string, std::string> *pairs);
+  int metadata_list(std::map<std::string, ceph::bufferlist> *pairs);
 
 private:
   friend class RBD;

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 337ac75a3f42c3e265e12e1c13e0d3530362d57d..e48da6fe0c2ec271d7c7abfb571c140be2de44fa 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -1077,7 +1077,7 @@ reprotect_and_return_err:
     int remove_r;
     librbd::NoOpProgressContext no_op;
     ImageCtx *c_imctx = NULL;
-    map<string, string> pairs;
+    map<string, bufferlist> pairs;
     // make sure parent snapshot exists
     ImageCtx *p_imctx = new ImageCtx(p_name, "", p_snap_name, p_ioctx, true);
     r = open_image(p_imctx);
@@ -1149,13 +1149,10 @@ reprotect_and_return_err:
       lderr(cct) << "couldn't list metadata: " << r << dendl;
       goto err_close_child;
     }
-    for (map<string, string>::iterator it = pairs.begin(); it != pairs.end(); ++it) {
-      r = cls_client::metadata_set(&c_ioctx, c_imctx->header_oid,
-                                   it->first, it->second);
-      if (r < 0) {
-        lderr(cct) << "couldn't set metadata: " << r << dendl;
-        goto err_close_child;
-      }
+    r = cls_client::metadata_set(&c_ioctx, c_imctx->header_oid, pairs);
+    if (r < 0) {
+      lderr(cct) << "couldn't set metadata: " << r << dendl;
+      goto err_close_child;
     }
 
     p_imctx->md_lock.get_write();
@@ -2277,19 +2274,17 @@ reprotect_and_return_err:
       return -EINVAL;
     }
     int r;
-    map<string, string> pairs;
+    map<string, bufferlist> pairs;
 
     r = cls_client::metadata_list(&src->md_ctx, src->header_oid, &pairs);
     if (r < 0) {
       lderr(cct) << "couldn't list metadata: " << r << dendl;
       return r;
     }
-    for (map<string, string>::iterator it = pairs.begin(); it != pairs.end(); ++it) {
-      r = cls_client::metadata_set(&dest->md_ctx, dest->header_oid, it->first, it->second);
-      if (r < 0) {
-        lderr(cct) << "couldn't set metadata: " << r << dendl;
-        return r;
-      }
+    r = cls_client::metadata_set(&dest->md_ctx, dest->header_oid, pairs);
+    if (r < 0) {
+      lderr(cct) << "couldn't set metadata: " << r << dendl;
+      return r;
     }
 
     SimpleThrottle throttle(cct->_conf->rbd_concurrent_management_ops, false);
@@ -3409,7 +3404,9 @@ reprotect_and_return_err:
       return r;
     }
 
-    return cls_client::metadata_set(&ictx->md_ctx, ictx->header_oid, key, value);
+    map<string, bufferlist> data;
+    data[key].append(value);
+    return cls_client::metadata_set(&ictx->md_ctx, ictx->header_oid, data);
   }
 
   int metadata_remove(ImageCtx *ictx, const string &key)
@@ -3425,7 +3422,7 @@ reprotect_and_return_err:
     return cls_client::metadata_remove(&ictx->md_ctx, ictx->header_oid, key);
   }
 
-  int metadata_list(ImageCtx *ictx, map<string, string> *pairs)
+  int metadata_list(ImageCtx *ictx, map<string, bufferlist> *pairs)
   {
     CephContext *cct = ictx->cct;
     ldout(cct, 20) << "metadata_list " << ictx << dendl;

diff --git a/src/librbd/internal.h b/src/librbd/internal.h
index 3cb63cf94e2d36ac0cf128e4a3f30e4653f514fc..024b91cd1221f3a6c5b57e4fc4cfb9af03344c82 100644 (file)
--- a/src/librbd/internal.h
+++ b/src/librbd/internal.h
@@ -207,7 +207,7 @@ namespace librbd {
   int flush(ImageCtx *ictx);
   int _flush(ImageCtx *ictx);
   int invalidate_cache(ImageCtx *ictx);
-  int metadata_list(ImageCtx *ictx, map<string, string> *pairs);
+  int metadata_list(ImageCtx *ictx, map<string, bufferlist> *pairs);
   int metadata_get(ImageCtx *ictx, const std::string &key, std::string *value);
   int metadata_set(ImageCtx *ictx, const std::string &key, const std::string &value);
   int metadata_remove(ImageCtx *ictx, const std::string &key);

diff --git a/src/librbd/librbd.cc b/src/librbd/librbd.cc
index 772027790f7582adcbd5e3ff29d2c8be0035b598..88734f4d629eb2271a5bd3d456406d7cb6c94f23 100644 (file)
--- a/src/librbd/librbd.cc
+++ b/src/librbd/librbd.cc
@@ -823,13 +823,13 @@ namespace librbd {
     return r;
   }
 
-  int Image::metadata_list(map<string, string> *pairs)
+  int Image::metadata_list(map<string, bufferlist> *pairs)
   {
     ImageCtx *ictx = (ImageCtx *)ctx;
     tracepoint(librbd, metadata_list_enter, ictx);
     int r = librbd::metadata_list(ictx, pairs);
     if (r >= 0) {
-      for (map<string, string>::const_iterator it = pairs->begin();
+      for (map<string, bufferlist>::iterator it = pairs->begin();
            it != pairs->end(); ++it) {
         tracepoint(librbd, metadata_list_entry, it->first.c_str(), it->second.c_str());
       }
@@ -1786,11 +1786,11 @@ extern "C" int rbd_metadata_list(rbd_image_t image,  char *key, size_t *key_len,
 {
   librbd::ImageCtx *ictx = (librbd::ImageCtx *)image;
   tracepoint(librbd, metadata_list_enter, ictx);
-  map<string, string> pairs;
+  map<string, bufferlist> pairs;
   int r = librbd::metadata_list(ictx, &pairs);
   size_t key_total_len = 0, val_total_len = 0;
   bool too_short = false;
-  for (map<string, string>::iterator it = pairs.begin();
+  for (map<string, bufferlist>::iterator it = pairs.begin();
        it != pairs.end(); ++it) {
     key_total_len += it->first.size() + 1;
     val_total_len += it->second.length() + 1;
@@ -1806,11 +1806,11 @@ extern "C" int rbd_metadata_list(rbd_image_t image,  char *key, size_t *key_len,
 
   char *key_p = key, *value_p = value;
 
-  for (map<string, string>::iterator it = pairs.begin();
+  for (map<string, bufferlist>::iterator it = pairs.begin();
        it != pairs.end(); ++it) {
     strncpy(key_p, it->first.c_str(), it->first.size());
     key_p += it->first.size() + 1;
-    strncpy(value_p, it->second.c_str(), it->second.size());
+    strncpy(value_p, it->second.c_str(), it->second.length());
     value_p += it->second.length() + 1;
     tracepoint(librbd, metadata_list_entry, it->first.c_str(), it->second.c_str());
   }

diff --git a/src/rbd.cc b/src/rbd.cc
index f5b3350183736093155cd8d59392beeb69c9424f..8335e2cbcf6155aec9049b2814dce4f533e83749 100644 (file)
--- a/src/rbd.cc
+++ b/src/rbd.cc
@@ -2104,7 +2104,7 @@ done:
 
 static int do_metadata_list(librbd::Image& image, Formatter *f)
 {
-  map<string, string> pairs;
+  map<string, bufferlist> pairs;
   int r;
   TextTable tbl;
 
@@ -2127,12 +2127,12 @@ static int do_metadata_list(librbd::Image& image, Formatter *f)
            << " metadata" << (one ? "" : "s") << " on this image.\n";
     }
 
-    for (map<string, string>::iterator it = pairs.begin();
+    for (map<string, bufferlist>::iterator it = pairs.begin();
          it != pairs.end(); ++it) {
       if (f) {
-        f->dump_string(it->first.c_str(), it->second);
+        f->dump_string(it->first.c_str(), it->second.c_str());
       } else {
-        tbl << it->first << it->second << TextTable::endrow;
+        tbl << it->first << it->second.c_str() << TextTable::endrow;
       }
     }
     if (!f)

diff --git a/src/test/cls_rbd/test_cls_rbd.cc b/src/test/cls_rbd/test_cls_rbd.cc
index 76c119b33a163fc20b9079b8c9089950d768e4fc..c97d55bed4b83fb22c6799e4df61e308df1e30f8 100644 (file)
--- a/src/test/cls_rbd/test_cls_rbd.cc
+++ b/src/test/cls_rbd/test_cls_rbd.cc
@@ -1078,19 +1078,21 @@ TEST_F(TestClsRbd, metadata)
   string oid = get_temp_image_name();
   ASSERT_EQ(0, create_image(&ioctx, oid, 0, 22, 0, oid));
 
-  map<string, string> pairs;
+  map<string, bufferlist> pairs;
   string value;
   ASSERT_EQ(0, metadata_list(&ioctx, oid, &pairs));
   ASSERT_TRUE(pairs.empty());
 
-  ASSERT_EQ(0, metadata_set(&ioctx, oid, "key1", "value1"));
-  ASSERT_EQ(0, metadata_set(&ioctx, oid, "key2", "value2"));
+  pairs["key1"].append("value1");
+  pairs["key2"].append("value2");
+  ASSERT_EQ(0, metadata_set(&ioctx, oid, pairs));
   ASSERT_EQ(0, metadata_get(&ioctx, oid, "key1", &value));
   ASSERT_EQ(0, strcmp("value1", value.c_str()));
+  pairs.clear();
   ASSERT_EQ(0, metadata_list(&ioctx, oid, &pairs));
   ASSERT_EQ(2U, pairs.size());
-  ASSERT_EQ(0, strcmp("value1", pairs["key1"].c_str()));
-  ASSERT_EQ(0, strcmp("value2", pairs["key2"].c_str()));
+  ASSERT_EQ(0, strncmp("value1", pairs["key1"].c_str(), 6));
+  ASSERT_EQ(0, strncmp("value2", pairs["key2"].c_str(), 6));
 
   pairs.clear();
   ASSERT_EQ(0, metadata_remove(&ioctx, oid, "key1"));
@@ -1098,7 +1100,7 @@ TEST_F(TestClsRbd, metadata)
   ASSERT_TRUE(metadata_get(&ioctx, oid, "key1", &value) < 0);
   ASSERT_EQ(0, metadata_list(&ioctx, oid, &pairs));
   ASSERT_EQ(1U, pairs.size());
-  ASSERT_EQ(0, strcmp("value2", pairs["key2"].c_str()));
+  ASSERT_EQ(0, strncmp("value2", pairs["key2"].c_str(), 6));
 
   ioctx.close();
 }

diff --git a/src/test/librbd/test_librbd.cc b/src/test/librbd/test_librbd.cc
index ab336c1ebec9045cd039b8631aa386f840616cb6..4d3c575d846a30a8fbcbc8474ea764c8d1fac2ec 100644 (file)
--- a/src/test/librbd/test_librbd.cc
+++ b/src/test/librbd/test_librbd.cc
@@ -2613,7 +2613,7 @@ TEST_F(TestLibRBD, Metadata)
 
   librbd::Image image1;
   ASSERT_EQ(0, rbd.open(ioctx, image1, name.c_str(), NULL));
-  map<string, string> pairs;
+  map<string, bufferlist> pairs;
   ASSERT_EQ(0, image1.metadata_list(&pairs));
   ASSERT_TRUE(pairs.empty());