rados_sync: more fixes

* separate BackedUpObject::from_path and BackedUpObject::from_file.

* librados functions return negative values on error, but most may
return positive values on success, even things like setxattr.

* implement read_xattrs_from_rados

* change rados_sync printout a bit

* fix up test_rados_sync xattr tests

Signed-off-by: Colin McCabe <colin.mccabe@dreamhost.com>

diff --git a/src/rados_sync.cc b/src/rados_sync.cc
index 32cd9841757ddb56c00d04c89a2b911041e76976..38a9159eea7745e732ffb3a55380e1f001f34256 100644 (file)
--- a/src/rados_sync.cc
+++ b/src/rados_sync.cc
@@ -155,15 +155,20 @@ public:
   static int from_file(const char *file_name, const char *dir_name,
                            std::auto_ptr<BackedUpObject> &obj)
   {
-    int ret;
     char obj_path[strlen(dir_name) + strlen(file_name) + 2];
     snprintf(obj_path, sizeof(obj_path), "%s/%s", dir_name, file_name);
-    FILE *fp = fopen(obj_path, "r");
+    return BackedUpObject::from_path(obj_path, obj);
+  }
+
+  static int from_path(const char *path, std::auto_ptr<BackedUpObject> &obj)
+  {
+    int ret;
+    FILE *fp = fopen(path, "r");
     if (!fp) {
       ret = errno;
       if (ret != ENOENT) {
-       cerr << ERR_PREFIX << "BackedUpObject::from_file: error while trying to "
-            << "open '" << obj_path << "': " <<  cpp_strerror(ret) << std::endl;
+       cerr << ERR_PREFIX << "BackedUpObject::from_path: error while trying to "
+            << "open '" << path << "': " <<  cpp_strerror(ret) << std::endl;
       }
       return ret;
     }
@@ -174,8 +179,8 @@ public:
     if (ret) {
       ret = errno;
       fclose(fp);
-      cerr << ERR_PREFIX << "BackedUpObject::from_file: error while trying "
-          << "to stat '" << obj_path << "': " <<  cpp_strerror(ret) << std::endl;
+      cerr << ERR_PREFIX << "BackedUpObject::from_path: error while trying "
+          << "to stat '" << path << "': " <<  cpp_strerror(ret) << std::endl;
       return ret;
     }
 
@@ -185,13 +190,13 @@ public:
       fclose(fp);
       ret = errno;
       if (res == 0) {
-       cerr << ERR_PREFIX << "BackedUpObject::from_file: found empty "
-            << XATTR_FULLNAME << " attribute on '" << file_name
+       cerr << ERR_PREFIX << "BackedUpObject::from_path: found empty "
+            << XATTR_FULLNAME << " attribute on '" << path
             << "'" << std::endl;
        ret = ENOATTR;
       } else if (ret == ENOATTR) {
-       cerr << ERR_PREFIX << "BackedUpObject::from_file: there was no "
-            << XATTR_FULLNAME << " attribute found on '" << file_name
+       cerr << ERR_PREFIX << "BackedUpObject::from_path: there was no "
+            << XATTR_FULLNAME << " attribute found on '" << path
             << "'" << std::endl;
       } else {
        cerr << ERR_PREFIX << "getxattr error: " << cpp_strerror(ret) << std::endl;
@@ -215,12 +220,11 @@ public:
       fclose(fp);
       return ENOBUFS;
     }
-    ret = o->read_xattrs(fileno(fp));
+    ret = o->read_xattrs_from_file(fileno(fp));
     if (ret) {
       fclose(fp);
-      cerr << ERR_PREFIX << "BackedUpObject::from_file(file_name = '"
-          << file_name << "', dir_name = '" << dir_name << "'): "
-          << "read_xattrs returned " << ret << std::endl;
+      cerr << ERR_PREFIX << "BackedUpObject::from_path(path = '"
+          << path << "): read_xattrs_from_file returned " << ret << std::endl;
       delete o;
       return ret;
     }
@@ -239,12 +243,20 @@ public:
     if (ret == -ENOENT) {
       // don't complain here about ENOENT
       return ret;
-    } else if (ret) {
+    } else if (ret < 0) {
       cerr << ERR_PREFIX << "BackedUpObject::from_rados(rados_name_ = '"
           << rados_name_ << "'): stat failed with error " << ret << std::endl;
       return ret;
     }
     BackedUpObject *o = new BackedUpObject(rados_name_, rados_size_, rados_time_);
+    ret = o->read_xattrs_from_rados(io_ctx);
+    if (ret) {
+      cerr << ERR_PREFIX << "BackedUpObject::from_rados(rados_name_ = '"
+           << rados_name_ << "'): read_xattrs_from_rados returned "
+           << ret << std::endl;
+      delete o;
+      return ret;
+    }
     obj.reset(o);
     return 0;
   }
@@ -315,7 +327,7 @@ public:
 
   /* Convert the xattrs on this BackedUpObject to a kind of JSON-like string.
    * This is only used for debugging.
-   * Note that we're assuming we can just treat the xattr data as a 
+   * Note that we're assuming we can just treat the xattr data as a
    * null-terminated string, which isn't true. Again, this is just for debugging,
    * so it doesn't matter.
    */
@@ -326,7 +338,10 @@ public:
     for (std::map < std::string, Xattr* >::const_iterator x = xattrs.begin();
           x != xattrs.end(); ++x)
     {
-      oss << prefix << "{" << x->first << "," << x->second->data << "}";
+      char buf[x->second->len + 1];
+      memcpy(buf, x->second->data, x->second->len);
+      buf[x->second->len] = '\0';
+      oss << prefix << "{" << x->first << ":" << buf << "}";
       prefix = ", ";
     }
     return oss.str();
@@ -515,19 +530,20 @@ private:
   {
   }
 
-  int read_xattrs(int fd)
+  int read_xattrs_from_file(int fd)
   {
     ssize_t blen = flistxattr(fd, NULL, 0);
     if (blen > 0x1000000) {
-      cerr << ERR_PREFIX << "BackedUpObject::read_xattrs: unwilling to allocate a "
-          << "buffer of size " << blen << " on the stack for flistxattr." << std::endl;
+      cerr << ERR_PREFIX << "BackedUpObject::read_xattrs_from_file: unwilling "
+          << "to allocate a buffer of size " << blen << " on the stack for "
+          << "flistxattr." << std::endl;
       return ENOBUFS;
     }
     char buf[blen + 1];
     memset(buf, 0, sizeof(buf));
     ssize_t blen2 = flistxattr(fd, buf, blen);
     if (blen != blen2) {
-      cerr << ERR_PREFIX << "BackedUpObject::read_xattrs: xattrs changed while "
+      cerr << ERR_PREFIX << "BackedUpObject::read_xattrs_from_file: xattrs changed while "
           << "we were trying to "
           << "list them? First length was " << blen << ", but now it's " << blen2
           << std::endl;
@@ -540,29 +556,32 @@ private:
        ssize_t attr_len = fgetxattr(fd, b, NULL, 0);
        if (attr_len < 0) {
          int err = errno;
-         cerr << ERR_PREFIX << "BackedUpObject::read_xattrs: fgetxattr(rados_name = '"
-              << rados_name << "', xattr_name='" << b << "') failed: "
-              << cpp_strerror(err) << std::endl;
+         cerr << ERR_PREFIX << "BackedUpObject::read_xattrs_from_file: "
+              << "fgetxattr(rados_name = '" << rados_name << "', xattr_name='"
+              << b << "') failed: " << cpp_strerror(err) << std::endl;
          return EDOM;
        }
        char *attr = (char*)malloc(attr_len);
        if (!attr) {
-         cerr << ERR_PREFIX << "BackedUpObject::read_xattrs: malloc(" << attr_len
-              << ") failed for xattr_name='" << b << "'" << std::endl;
+         cerr << ERR_PREFIX << "BackedUpObject::read_xattrs_from_file: "
+              << "malloc(" << attr_len << ") failed for xattr_name='"
+              << b << "'" << std::endl;
          return ENOBUFS;
        }
        ssize_t attr_len2 = fgetxattr(fd, b, attr, attr_len);
        if (attr_len2 < 0) {
          int err = errno;
-         cerr << ERR_PREFIX << "BackedUpObject::read_xattrs: fgetxattr(rados_name = '"
-              << rados_name << "', xattr_name='" << b << "') failed: "
+         cerr << ERR_PREFIX << "BackedUpObject::read_xattrs_from_file: "
+              << "fgetxattr(rados_name = '" << rados_name << "', "
+              << "xattr_name='" << b << "') failed: "
               << cpp_strerror(err) << std::endl;
          free(attr);
          return EDOM;
        }
        if (attr_len2 != attr_len) {
-         cerr << ERR_PREFIX << "BackedUpObject::read_xattrs: xattr changed whil we were "
-              << "trying to get it? fgetxattr(rados_name = '"<< rados_name
+         cerr << ERR_PREFIX << "BackedUpObject::read_xattrs_from_file: xattr "
+              << "changed while we were trying to get it? "
+              << "fgetxattr(rados_name = '"<< rados_name
               << "', xattr_name='" << b << "') returned a different length "
               << "than when we first called it! old_len = " << attr_len
               << "new_len = " << attr_len2 << std::endl;
@@ -576,6 +595,34 @@ private:
     return 0;
   }
 
+  int read_xattrs_from_rados(IoCtx &io_ctx)
+  {
+    map<std::string, bufferlist> attrset;
+    int ret = io_ctx.getxattrs(rados_name, attrset);
+    if (ret) {
+      cerr << ERR_PREFIX << "BackedUpObject::read_xattrs_from_rados: "
+          << "getxattrs failed with error code " << ret << std::endl;
+      return ret;
+    }
+    for (map<std::string, bufferlist>::iterator i = attrset.begin();
+        i != attrset.end(); )
+    {
+      bufferlist& bl(i->second);
+      char *data = (char*)malloc(bl.length());
+      if (!data)
+       return ENOBUFS;
+      memcpy(data, bl.c_str(), bl.length());
+      Xattr *xattr = new Xattr(data, bl.length());
+      if (!xattr) {
+       free(data);
+       return ENOBUFS;
+      }
+      xattrs[i->first] = xattr;
+      attrset.erase(i++);
+    }
+    return 0;
+  }
+
   // don't allow copying
   BackedUpObject &operator=(const BackedUpObject &rhs);
   BackedUpObject(const BackedUpObject &rhs);
@@ -610,13 +657,13 @@ static int do_export(IoCtx& io_ctx, const char *dir_name)
       return ret;
     }
     std::string obj_path(sobj->get_fs_path(dir_name));
-    ret = BackedUpObject::from_file(obj_path.c_str(), dir_name, dobj);
+    ret = BackedUpObject::from_path(obj_path.c_str(), dobj);
     if (ret == ENOENT) {
       sobj->get_xattrs(only_in_a);
       flags |= CHANGED_CONTENTS;
     }
     else if (ret) {
-      cerr << ERR_PREFIX << "BackedUpObject::from_file returned "
+      cerr << ERR_PREFIX << "BackedUpObject::from_path returned "
           << ret << std::endl;
       return ret;
     }
@@ -646,7 +693,7 @@ static int do_export(IoCtx& io_ctx, const char *dir_name)
       ret = setxattr(obj_path.c_str(), x->c_str(), xattr->data, xattr->len, 0);
       if (ret) {
        ret = errno;
-       cerr << ERR_PREFIX << "sexattr error: " << cpp_strerror(ret) << std::endl;
+       cerr << ERR_PREFIX << "setxattr error: " << cpp_strerror(ret) << std::endl;
        return ret;
       }
     }
@@ -661,7 +708,7 @@ static int do_export(IoCtx& io_ctx, const char *dir_name)
       }
     }
     if (flags & CHANGED_CONTENTS) {
-      cout << "[uploaded]     " << rados_name << std::endl;
+      cout << "[exported]     " << rados_name << std::endl;
     }
     else if (flags & CHANGED_XATTRS) {
       cout << "[xattr]        " << rados_name << std::endl;
@@ -746,9 +793,10 @@ static int do_import(IoCtx& io_ctx, const char *dir_name)
       bufferlist bl;
       bl.append(xattr->data, xattr->len);
       ret = io_ctx.setxattr(rados_name, x->c_str(), bl);
-      if (ret) {
+      if (ret < 0) {
        ret = errno;
-       cerr << ERR_PREFIX << "io_ctx.sexattr error1: " << cpp_strerror(ret)
+       cerr << ERR_PREFIX << "io_ctx.setxattr(rados_name='" << rados_name
+            << "', xattr_name='" << x->c_str() << "'): " << cpp_strerror(ret)
             << std::endl;
        return ret;
       }
@@ -764,16 +812,16 @@ static int do_import(IoCtx& io_ctx, const char *dir_name)
       bufferlist bl;
       bl.append(xattr->data, xattr->len);
       ret = io_ctx.rmxattr(rados_name, x->c_str());
-      if (ret) {
+      if (ret < 0) {
        cerr << ERR_PREFIX << "io_ctx.rmxattr error2: " << cpp_strerror(ret)
             << std::endl;
        return ret;
       }
       ret = io_ctx.setxattr(rados_name, x->c_str(), bl);
-      if (ret) {
+      if (ret < 0) {
        ret = errno;
-       cerr << ERR_PREFIX << "io_ctx.sexattr error2: " << cpp_strerror(ret)
-            << std::endl;
+       cerr << ERR_PREFIX << "io_ctx.setxattr(rados_name='" << rados_name
+            << "', xattr='" << x->c_str() << "'): " << cpp_strerror(ret) << std::endl;
        return ret;
       }
     }
@@ -781,14 +829,14 @@ static int do_import(IoCtx& io_ctx, const char *dir_name)
         x != only_in_b.end(); ++x) {
       flags |= CHANGED_XATTRS;
       ret = io_ctx.rmxattr(rados_name, x->c_str());
-      if (ret) {
+      if (ret < 0) {
        ret = errno;
        cerr << ERR_PREFIX << "rmxattr error3: " << cpp_strerror(ret) << std::endl;
        return ret;
       }
     }
     if (flags & CHANGED_CONTENTS) {
-      cout << "[downloaded]   " << rados_name << std::endl;
+      cout << "[imported]     " << rados_name << std::endl;
     }
     else if (flags & CHANGED_XATTRS) {
       cout << "[xattr]        " << rados_name << std::endl;
@@ -879,7 +927,7 @@ int main(int argc, const char **argv)
       exit(ENOENT);
     }
   }
-  if (ret) {
+  if (ret < 0) {
     cerr << argv[0] << ": error opening pool " << pool_name << ": "
         << cpp_strerror(ret) << std::endl;
     exit(ret);

diff --git a/src/test/test_rados_sync.sh b/src/test/test_rados_sync.sh
index 3622ddbdff79a90d94a91f531b329ad717c8df97..df2f35362d018728aee6f29b50372337c1254baf 100755 (executable)
--- a/src/test/test_rados_sync.sh
+++ b/src/test/test_rados_sync.sh
@@ -68,11 +68,11 @@ mkdir "$TDIR/dirb"
 mkdir "$TDIR/dirc"
 touch "$TDIR/dirc/000029c4_foo"
 attr -q -s rados_full_name -V "foo" "$TDIR/dirc/000029c4_foo"
-attr -q -s "user.rados.toothbrush" -V "toothbrush" "$TDIR/dirc/000029c4_foo"
-attr -q -s "user.rados.toothpaste" -V "crest" "$TDIR/dirc/000029c4_foo"
-attr -q -s "user.rados.floss" -V "myfloss" "$TDIR/dirc/000029c4_foo"
+attr -q -s "rados.toothbrush" -V "toothbrush" "$TDIR/dirc/000029c4_foo"
+attr -q -s "rados.toothpaste" -V "crest" "$TDIR/dirc/000029c4_foo"
+attr -q -s "rados.floss" -V "myfloss" "$TDIR/dirc/000029c4_foo"
 touch "$TDIR/dirc/00003036_foo2"
-attr -q -s "user.rados.toothbrush" -V "green" "$TDIR/dirc/00003036_foo2"
+attr -q -s "rados.toothbrush" -V "green" "$TDIR/dirc/00003036_foo2"
 attr -q -s rados_full_name -V "foo2" "$TDIR/dirc/00003036_foo2"
 
 # make sure that --create works
@@ -100,8 +100,8 @@ run_expect_succ "$RADOS_SYNC" import "$TDIR/dirc" "$POOL"
 run_expect_succ "$RADOS_SYNC" -C export "$POOL" "$TDIR/dirc_copy"
 
 # check to make sure extended attributes were preserved
-PRE_EXPORT=`attr -qg user.rados.toothbrush "$TDIR/dirc/000029c4_foo"`
-POST_EXPORT=`attr -qg user.rados.toothbrush "$TDIR/dirc_copy/000029c4_foo"`
+PRE_EXPORT=`attr -qg rados.toothbrush "$TDIR/dirc/000029c4_foo"`
+POST_EXPORT=`attr -qg rados.toothbrush "$TDIR/dirc_copy/000029c4_foo"`
 if [ "$PRE_EXPORT" != "$POST_EXPORT" ]; then
     die "xattr not preserved across import/export! \
 \$PRE_EXPORT = $PRE_EXPORT, \$POST_EXPORT = $POST_EXPORT"