Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Unit tests for chain_xattr.cc #40

Merged
merged 1 commit into from

2 participants

@dachary
Owner

Create a new test file covering most (> 97%) of src/os/chain_xattr.{cc,h}
lines of code. The following functions are tested:

int chain_getxattr(const char *fn, const char *name, void *val, size_t size);
int chain_fgetxattr(int fd, const char *name, void *val, size_t size);
int chain_setxattr(const char *fn, const char *name, const void *val, size_t size);
int chain_fsetxattr(int fd, const char *name, const void *val, size_t size);
int chain_listxattr(const char *fn, char *names, size_t len);
int chain_flistxattr(int fd, char *names, size_t len);
int chain_removexattr(const char *fn, const char *name);
int chain_fremovexattr(int fd, const char *name);

The function translate_raw_name substitutes @@ into @. When the trailing
character is a @, it breaks. However, such an occurence cannot be created by
chain_setxattr because it always create pairs of @. Instead of silently
breaking the loop, the function should probably return on error so that the
caller can ignore it.

The function chain_fgetxattr_len may return on error if fgetxattr
returns on error. However, it is only called after another attr function
returned success and the tests cannot create the conditions under which
it would fail.

The function chain_fsetxattr may leak attributes when used to override
an existing attribute. This leak is not tested or fixed, it is just
discussed in http://marc.info/?l=ceph-devel&m=136027076615853&w=4

If the file system in which the tests are run does not support extended
attributes, the tests are not run. The detection uses the same logic as
the one implemented in FileStore::_detect_fs

The output of the tests are silenced to reduce the output when testing
assertions ( except for the dout_emergency function which cannot be
controlled).

@dachary dachary Unit tests for chain_xattr.cc
Create a new test file covering most (> 97%) of src/os/chain_xattr.{cc,h}
lines of code. The following functions are tested:

int chain_getxattr(const char *fn, const char *name, void *val, size_t size);
int chain_fgetxattr(int fd, const char *name, void *val, size_t size);
int chain_setxattr(const char *fn, const char *name, const void *val, size_t size);
int chain_fsetxattr(int fd, const char *name, const void *val, size_t size);
int chain_listxattr(const char *fn, char *names, size_t len);
int chain_flistxattr(int fd, char *names, size_t len);
int chain_removexattr(const char *fn, const char *name);
int chain_fremovexattr(int fd, const char *name);

The function translate_raw_name substitutes @@ into @. When the trailing
character is a @, it breaks. However, such an occurence cannot be created by
chain_setxattr because it always create pairs of @. Instead of silently
breaking the loop, the function should probably return on error so that the
caller can ignore it.

The function chain_fgetxattr_len may return on error if fgetxattr
returns on error. However, it is only called after another attr function
returned success and the tests cannot create the conditions under which
it would fail.

The function chain_fsetxattr may leak attributes when used to override
an existing attribute. This leak is not tested or fixed, it is just
discussed in http://marc.info/?l=ceph-devel&m=136027076615853&w=4

If the file system in which the tests are run does not support extended
attributes, the tests are not run. The detection uses the same logic as
the one implemented in FileStore::_detect_fs

The output of the tests are silenced to reduce the output when testing
assertions ( except for the dout_emergency function which cannot be
controlled).
a62d519
@liewegas
Owner

Hi Loic-

Are there changes you want/need to make to this given the email conversation about removing chained xattrs, or is this ready as-is?

@dachary
Owner

If you are refering to the http://marc.info/?l=ceph-devel&m=136027076615853&w=4 email conversation, I already amended the tests accordingly. Is there another discussion related to obseleting chained xattrs entirely ?

@liewegas
Owner

Great, just checking. Thanks!

@liewegas liewegas merged commit 61eb0e9 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 11, 2013
  1. @dachary

    Unit tests for chain_xattr.cc

    dachary authored
    Create a new test file covering most (> 97%) of src/os/chain_xattr.{cc,h}
    lines of code. The following functions are tested:
    
    int chain_getxattr(const char *fn, const char *name, void *val, size_t size);
    int chain_fgetxattr(int fd, const char *name, void *val, size_t size);
    int chain_setxattr(const char *fn, const char *name, const void *val, size_t size);
    int chain_fsetxattr(int fd, const char *name, const void *val, size_t size);
    int chain_listxattr(const char *fn, char *names, size_t len);
    int chain_flistxattr(int fd, char *names, size_t len);
    int chain_removexattr(const char *fn, const char *name);
    int chain_fremovexattr(int fd, const char *name);
    
    The function translate_raw_name substitutes @@ into @. When the trailing
    character is a @, it breaks. However, such an occurence cannot be created by
    chain_setxattr because it always create pairs of @. Instead of silently
    breaking the loop, the function should probably return on error so that the
    caller can ignore it.
    
    The function chain_fgetxattr_len may return on error if fgetxattr
    returns on error. However, it is only called after another attr function
    returned success and the tests cannot create the conditions under which
    it would fail.
    
    The function chain_fsetxattr may leak attributes when used to override
    an existing attribute. This leak is not tested or fixed, it is just
    discussed in http://marc.info/?l=ceph-devel&m=136027076615853&w=4
    
    If the file system in which the tests are run does not support extended
    attributes, the tests are not run. The detection uses the same logic as
    the one implemented in FileStore::_detect_fs
    
    The output of the tests are silenced to reduce the output when testing
    assertions ( except for the dout_emergency function which cannot be
    controlled).
This page is out of date. Refresh to see the latest.
Showing with 223 additions and 1 deletion.
  1. +1 −1  COPYING
  2. +6 −0 src/Makefile.am
  3. +216 −0 src/test/filestore/chain_xattr.cc
View
2  COPYING
@@ -98,6 +98,6 @@ License:
-Files: test/common/Throttle.cc
+Files: src/test/common/Throttle.cc src/test/filestore/chain_xattr.cc
Copyright: Copyright (C) 2013 Cloudwatt <libre.licensing@cloudwatt.com>
License: LGPL2 or later
View
6 src/Makefile.am
@@ -767,6 +767,12 @@ unittest_escape_LDADD = ${UNITTEST_LDADD} $(LIBGLOBAL_LDA)
unittest_escape_CXXFLAGS = ${AM_CXXFLAGS} ${UNITTEST_CXXFLAGS}
check_PROGRAMS += unittest_escape
+unittest_chain_xattr_SOURCES = test/filestore/chain_xattr.cc
+unittest_chain_xattr_LDFLAGS = ${AM_LDFLAGS}
+unittest_chain_xattr_LDADD = ${UNITTEST_STATIC_LDADD} $(LIBOS_LDA) $(LIBGLOBAL_LDA)
+unittest_chain_xattr_CXXFLAGS = ${AM_CXXFLAGS} ${UNITTEST_CXXFLAGS} $(LEVELDB_INCLUDE) ${CRYPTO_CXXFLAGS}
+check_PROGRAMS += unittest_chain_xattr
+
unittest_strtol_SOURCES = test/strtol.cc
unittest_strtol_LDFLAGS = $(PTHREAD_CFLAGS) ${AM_LDFLAGS}
unittest_strtol_LDADD = ${UNITTEST_LDADD} $(LIBGLOBAL_LDA)
View
216 src/test/filestore/chain_xattr.cc
@@ -0,0 +1,216 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+/*
+ * Ceph - scalable distributed file system
+ *
+ * Copyright (C) 2013 Cloudwatt <libre.licensing@cloudwatt.com>
+ *
+ * Author: Loic Dachary <loic@dachary.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Library Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Library Public License for more details.
+ *
+ */
+
+#include <stdio.h>
+#include <signal.h>
+#include "os/chain_xattr.h"
+#include "include/Context.h"
+#include "common/errno.h"
+#include "common/ceph_argparse.h"
+#include "global/global_init.h"
+#include <gtest/gtest.h>
+
+#define LARGE_BLOCK_LEN CHAIN_XATTR_MAX_BLOCK_LEN + 1024
+
+TEST(chain_xattr, get_and_set) {
+ const char* file = "testfile";
+ ::unlink(file);
+ int fd = ::open(file, O_CREAT|O_WRONLY|O_TRUNC, 0700);
+ const string user("user.");
+
+ {
+ const string name = user + string(CHAIN_XATTR_MAX_NAME_LEN - user.size(), '@');
+ const string x(LARGE_BLOCK_LEN, 'X');
+
+ {
+ char y[LARGE_BLOCK_LEN];
+ ASSERT_EQ(LARGE_BLOCK_LEN, chain_setxattr(file, name.c_str(), x.c_str(), LARGE_BLOCK_LEN));
+ ASSERT_EQ(LARGE_BLOCK_LEN, chain_getxattr(file, name.c_str(), 0, 0));
+ ASSERT_EQ(LARGE_BLOCK_LEN, chain_getxattr(file, name.c_str(), y, LARGE_BLOCK_LEN));
+ ASSERT_EQ(0, chain_removexattr(file, name.c_str()));
+ ASSERT_EQ(0, memcmp(x.c_str(), y, LARGE_BLOCK_LEN));
+ }
+
+ {
+ char y[LARGE_BLOCK_LEN];
+ ASSERT_EQ(LARGE_BLOCK_LEN, chain_fsetxattr(fd, name.c_str(), x.c_str(), LARGE_BLOCK_LEN));
+ ASSERT_EQ(LARGE_BLOCK_LEN, chain_fgetxattr(fd, name.c_str(), 0, 0));
+ ASSERT_EQ(LARGE_BLOCK_LEN, chain_fgetxattr(fd, name.c_str(), y, LARGE_BLOCK_LEN));
+ ASSERT_EQ(0, chain_fremovexattr(fd, name.c_str()));
+ ASSERT_EQ(0, memcmp(x.c_str(), y, LARGE_BLOCK_LEN));
+ }
+ }
+
+ //
+ // when chain_setxattr is used to store value that is
+ // CHAIN_XATTR_MAX_BLOCK_LEN * 2 + 10 bytes long it
+ //
+ // add user.foo => CHAIN_XATTR_MAX_BLOCK_LEN bytes
+ // add user.foo@1 => CHAIN_XATTR_MAX_BLOCK_LEN bytes
+ // add user.foo@2 => 10 bytes
+ //
+ // then ( no chain_removexattr in between ) when it is used to
+ // override with a value that is exactly CHAIN_XATTR_MAX_BLOCK_LEN
+ // bytes long it will
+ //
+ // replace user.foo => CHAIN_XATTR_MAX_BLOCK_LEN bytes
+ // remove user.foo@1 => CHAIN_XATTR_MAX_BLOCK_LEN bytes
+ // leak user.foo@2 => 10 bytes
+ //
+ // see http://marc.info/?l=ceph-devel&m=136027076615853&w=4 for the
+ // discussion
+ //
+ {
+ const string name = user + string(CHAIN_XATTR_MAX_NAME_LEN - user.size(), '@');
+ const string x(LARGE_BLOCK_LEN, 'X');
+
+ {
+ char y[CHAIN_XATTR_MAX_NAME_LEN];
+ ASSERT_EQ(LARGE_BLOCK_LEN, chain_setxattr(file, name.c_str(), x.c_str(), LARGE_BLOCK_LEN));
+ ASSERT_EQ(CHAIN_XATTR_MAX_BLOCK_LEN, chain_setxattr(file, name.c_str(), x.c_str(), CHAIN_XATTR_MAX_BLOCK_LEN));
+ ASSERT_EQ(CHAIN_XATTR_MAX_BLOCK_LEN, chain_getxattr(file, name.c_str(), 0, 0));
+ ASSERT_EQ(CHAIN_XATTR_MAX_BLOCK_LEN, chain_getxattr(file, name.c_str(), y, CHAIN_XATTR_MAX_BLOCK_LEN));
+ ASSERT_EQ(0, chain_removexattr(file, name.c_str()));
+ ASSERT_EQ(0, memcmp(x.c_str(), y, CHAIN_XATTR_MAX_BLOCK_LEN));
+ }
+
+ {
+ char y[CHAIN_XATTR_MAX_BLOCK_LEN];
+ ASSERT_EQ(LARGE_BLOCK_LEN, chain_fsetxattr(fd, name.c_str(), x.c_str(), LARGE_BLOCK_LEN));
+ ASSERT_EQ(CHAIN_XATTR_MAX_BLOCK_LEN, chain_fsetxattr(fd, name.c_str(), x.c_str(), CHAIN_XATTR_MAX_BLOCK_LEN));
+ ASSERT_EQ(CHAIN_XATTR_MAX_BLOCK_LEN, chain_fgetxattr(fd, name.c_str(), 0, 0));
+ ASSERT_EQ(CHAIN_XATTR_MAX_BLOCK_LEN, chain_fgetxattr(fd, name.c_str(), y, CHAIN_XATTR_MAX_BLOCK_LEN));
+ ASSERT_EQ(0, chain_fremovexattr(fd, name.c_str()));
+ ASSERT_EQ(0, memcmp(x.c_str(), y, CHAIN_XATTR_MAX_BLOCK_LEN));
+ }
+ }
+
+ {
+ int x = 0;
+ ASSERT_EQ(-ENOENT, chain_setxattr("UNLIKELY_TO_EXIST", "NAME", &x, sizeof(x)));
+ ASSERT_EQ(-ENOENT, chain_getxattr("UNLIKELY_TO_EXIST", "NAME", 0, 0));
+ ASSERT_EQ(-ENOENT, chain_getxattr("UNLIKELY_TO_EXIST", "NAME", &x, sizeof(x)));
+ ASSERT_EQ(-ENOENT, chain_removexattr("UNLIKELY_TO_EXIST", "NAME"));
+ int unlikely_to_be_a_valid_fd = 400;
+ ASSERT_EQ(-EBADF, chain_fsetxattr(unlikely_to_be_a_valid_fd, "NAME", &x, sizeof(x)));
+ ASSERT_EQ(-EBADF, chain_fgetxattr(unlikely_to_be_a_valid_fd, "NAME", 0, 0));
+ ASSERT_EQ(-EBADF, chain_fgetxattr(unlikely_to_be_a_valid_fd, "NAME", &x, sizeof(x)));
+ ASSERT_EQ(-EBADF, chain_fremovexattr(unlikely_to_be_a_valid_fd, "NAME"));
+ }
+
+ {
+ int x;
+ const string name = user + string(CHAIN_XATTR_MAX_NAME_LEN * 2, '@');
+ ASSERT_THROW(chain_setxattr(file, name.c_str(), &x, sizeof(x)), FailedAssertion);
+ ASSERT_THROW(chain_fsetxattr(fd, name.c_str(), &x, sizeof(x)), FailedAssertion);
+ }
+
+ {
+ const string name = user + string(CHAIN_XATTR_MAX_NAME_LEN - user.size(), '@');
+ const string x(LARGE_BLOCK_LEN, 'X');
+ {
+ char y[LARGE_BLOCK_LEN];
+ ASSERT_EQ(LARGE_BLOCK_LEN, chain_setxattr(file, name.c_str(), x.c_str(), LARGE_BLOCK_LEN));
+ ASSERT_EQ(-ERANGE, chain_getxattr(file, name.c_str(), y, LARGE_BLOCK_LEN - 1));
+ ASSERT_EQ(-ERANGE, chain_getxattr(file, name.c_str(), y, CHAIN_XATTR_MAX_BLOCK_LEN));
+ ASSERT_EQ(0, chain_removexattr(file, name.c_str()));
+ }
+
+ {
+ char y[LARGE_BLOCK_LEN];
+ ASSERT_EQ(LARGE_BLOCK_LEN, chain_fsetxattr(fd, name.c_str(), x.c_str(), LARGE_BLOCK_LEN));
+ ASSERT_EQ(-ERANGE, chain_fgetxattr(fd, name.c_str(), y, LARGE_BLOCK_LEN - 1));
+ ASSERT_EQ(-ERANGE, chain_fgetxattr(fd, name.c_str(), y, CHAIN_XATTR_MAX_BLOCK_LEN));
+ ASSERT_EQ(0, chain_fremovexattr(fd, name.c_str()));
+ }
+ }
+
+ ::close(fd);
+ ::unlink(file);
+}
+
+TEST(chain_xattr, listxattr) {
+ const char* file = "testfile";
+ ::unlink(file);
+ int fd = ::open(file, O_CREAT|O_WRONLY|O_TRUNC, 0700);
+ const string user("user.");
+ const string name1 = user + string(CHAIN_XATTR_MAX_NAME_LEN - user.size(), '1');
+ const string name2 = user + string(CHAIN_XATTR_MAX_NAME_LEN - user.size(), '@');
+ const string x(LARGE_BLOCK_LEN, 'X');
+ const int y = 1234;
+
+ ASSERT_EQ(LARGE_BLOCK_LEN, chain_setxattr(file, name1.c_str(), x.c_str(), LARGE_BLOCK_LEN));
+ ASSERT_EQ((int)sizeof(y), chain_setxattr(file, name2.c_str(), &y, sizeof(y)));
+
+ int buffer_size = name1.size() + sizeof('\0') + name2.size() + sizeof('\0');
+ char* expected = (char*)malloc(buffer_size);
+ ::strcpy(expected, name1.c_str());
+ ::strcpy(expected + name1.size() + 1, name2.c_str());
+ char* actual = (char*)calloc(1, buffer_size);
+ ASSERT_LT(buffer_size, chain_listxattr(file, NULL, 0)); // size evaluation is conservative
+ chain_listxattr(file, actual, buffer_size);
+ ::memset(actual, '\0', buffer_size);
+ chain_flistxattr(fd, actual, buffer_size);
+ ASSERT_EQ(0, ::memcmp(expected, actual, buffer_size));
+
+ int unlikely_to_be_a_valid_fd = 400;
+ ASSERT_GT(0, chain_listxattr("UNLIKELY_TO_EXIST", actual, 0));
+ ASSERT_GT(0, chain_listxattr("UNLIKELY_TO_EXIST", actual, buffer_size));
+ ASSERT_GT(0, chain_flistxattr(unlikely_to_be_a_valid_fd, actual, 0));
+ ASSERT_GT(0, chain_flistxattr(unlikely_to_be_a_valid_fd, actual, buffer_size));
+ ASSERT_EQ(-ERANGE, chain_listxattr(file, actual, 1));
+ ASSERT_EQ(-ERANGE, chain_flistxattr(fd, actual, 1));
+
+ ASSERT_EQ(0, chain_removexattr(file, name1.c_str()));
+ ASSERT_EQ(0, chain_removexattr(file, name2.c_str()));
+}
+
+int main(int argc, char **argv) {
+ vector<const char*> args;
+ argv_to_vec(argc, (const char **)argv, args);
+
+ global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT, CODE_ENVIRONMENT_UTILITY, 0);
+ common_init_finish(g_ceph_context);
+ g_ceph_context->_conf->set_val("log_file", "LOGFILE");
+ g_ceph_context->_conf->set_val("err_to_stderr", "false");
+ g_ceph_context->_conf->set_val("log_to_stderr", "false");
+ g_ceph_context->_conf->apply_changes(NULL);
+
+ const char* file = "testfile";
+ int x = 1234;
+ int y = 0;
+ int tmpfd = ::open(file, O_CREAT|O_WRONLY|O_TRUNC, 0700);
+ int ret = ::ceph_os_fsetxattr(tmpfd, "user.test", &x, sizeof(x));
+ if (ret >= 0)
+ ret = ::ceph_os_fgetxattr(tmpfd, "user.test", &y, sizeof(y));
+ ::close(tmpfd);
+ ::unlink(file);
+ if ((ret < 0) || (x != y)) {
+ cerr << "SKIP all tests because extended attributes don't appear to work in the file system in which the tests are run: " << cpp_strerror(ret) << std::endl;
+ } else {
+ ::testing::InitGoogleTest(&argc, argv);
+ return RUN_ALL_TESTS();
+ }
+}
+
+// Local Variables:
+// compile-command: "cd ../.. ; make unittest_chain_xattr ; valgrind --tool=memcheck ./unittest_chain_xattr # --gtest_filter=chain_xattr.get_and_set"
+// End:
Something went wrong with that request. Please try again.