From 03d84a07d1d8e739909f4ae89e7c7d8ef5871587 Mon Sep 17 00:00:00 2001 From: Ka-Hing Cheung Date: Mon, 12 Jan 2015 14:46:24 -0800 Subject: [PATCH] fix rename before close nautilus does this when you drag and drop to overwrite a file: 1) create .goutputstream-XXXXXX to write to 2) fsync the fd for .goutputstream-XXXXXX 3) rename .goutputstream-XXXXXX to target file 4) close the fd for .goutputstream-XXXXXX previously, doing this on s3fs would result in an empty target file because after the rename, s3fs would not flush the content of .goutputstream-XXXXXX to target file. this change moves the FdEntity from the old path to the new path whenever rename happens. On flush s3fs would now flush the correct content to the rename target. --- src/fdcache.cpp | 13 ++ src/fdcache.h | 2 + src/s3fs.cpp | 3 + test/Makefile.am | 6 + test/integration-test-common.sh | 2 +- test/integration-test-main.sh | 292 ++++++++++++++++++++++++++++++++ test/rename_before_close.c | 67 ++++++++ test/small-integration-test.sh | 270 +---------------------------- 8 files changed, 386 insertions(+), 269 deletions(-) create mode 100755 test/integration-test-main.sh create mode 100644 test/rename_before_close.c diff --git a/src/fdcache.cpp b/src/fdcache.cpp index 7215bcd..c0903da 100644 --- a/src/fdcache.cpp +++ b/src/fdcache.cpp @@ -1177,6 +1177,19 @@ FdEntity* FdManager::Open(const char* path, off_t size, time_t time, bool force_ return ent; } +void FdManager::Rename(const std::string &from, const std::string &to) +{ + fdent_map_t::iterator iter = fent.find(from); + if(fent.end() != iter){ + // found + FPRNINFO("[from=%s][to=%s]", from.c_str(), to.c_str()); + FdEntity* ent = (*iter).second; + fent.erase(iter); + ent->SetPath(to); + fent[to] = ent; + } +} + bool FdManager::Close(FdEntity* ent) { FPRNINFO("[ent->file=%s][ent->fd=%d]", ent ? ent->GetPath() : "", ent ? ent->GetFd() : -1); diff --git a/src/fdcache.h b/src/fdcache.h index 3d7aac2..03efdfc 100644 --- a/src/fdcache.h +++ b/src/fdcache.h @@ -119,6 +119,7 @@ class FdEntity bool IsOpen(void) const { return (-1 != fd); } int Open(off_t size = -1, time_t time = -1); const char* GetPath(void) const { return path.c_str(); } + void SetPath(const std::string &newpath) { path = newpath; } int GetFd(void) const { return fd; } int SetMtime(time_t time); bool GetSize(off_t& size); @@ -169,6 +170,7 @@ class FdManager FdEntity* GetFdEntity(const char* path); FdEntity* Open(const char* path, off_t size = -1, time_t time = -1, bool force_tmpfile = false, bool is_create = true); FdEntity* ExistOpen(const char* path) { return Open(path, -1, -1, false, false); } + void Rename(const std::string &from, const std::string &to); bool Close(FdEntity* ent); }; diff --git a/src/s3fs.cpp b/src/s3fs.cpp index 352e6d4..8efc035 100644 --- a/src/s3fs.cpp +++ b/src/s3fs.cpp @@ -1077,6 +1077,9 @@ static int rename_object(const char* from, const char* to) if(0 != (result = put_headers(to, meta, true))){ return result; } + + FdManager::get()->Rename(from, to); + result = s3fs_unlink(from); StatCache::getStatCacheData()->DelStat(to); diff --git a/test/Makefile.am b/test/Makefile.am index e841fd6..61e7460 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -27,3 +27,9 @@ EXTRA_DIST = \ sample_delcache.sh \ sample_ahbe.conf +testdir = test + +test_PROGRAMS=rename_before_close + +rename_before_close_SOURCES = rename_before_close.c + diff --git a/test/integration-test-common.sh b/test/integration-test-common.sh index cf2d0d1..96a09a9 100644 --- a/test/integration-test-common.sh +++ b/test/integration-test-common.sh @@ -5,7 +5,7 @@ S3FS=../src/s3fs S3FS_CREDENTIALS_FILE=$(eval echo ~${SUDO_USER}/.passwd-s3fs) TEST_BUCKET_1=${SUDO_USER}-s3fs-integration-test -TEST_BUCKET_MOUNT_POINT_1=/mnt/${TEST_BUCKET_1} +TEST_BUCKET_MOUNT_POINT_1=${TEST_BUCKET_1} if [ ! -f "$S3FS_CREDENTIALS_FILE" ] then diff --git a/test/integration-test-main.sh b/test/integration-test-main.sh new file mode 100755 index 0000000..e07d4f6 --- /dev/null +++ b/test/integration-test-main.sh @@ -0,0 +1,292 @@ +#!/bin/bash -e +COMMON=integration-test-common.sh +source $COMMON + +# Configuration +TEST_TEXT="HELLO WORLD" +TEST_TEXT_FILE=test-s3fs.txt +TEST_DIR=testdir +ALT_TEST_TEXT_FILE=test-s3fs-ALT.txt +TEST_TEXT_FILE_LENGTH=15 + +CUR_DIR=`pwd` +TEST_BUCKET_MOUNT_POINT_1=$1 +cd $TEST_BUCKET_MOUNT_POINT_1 + +if [ -e $TEST_TEXT_FILE ] +then + rm -f $TEST_TEXT_FILE +fi + +# Write a small test file +for x in `seq 1 $TEST_TEXT_FILE_LENGTH` +do + echo "echo ${TEST_TEXT} to ${TEST_TEXT_FILE}" + echo $TEST_TEXT >> $TEST_TEXT_FILE +done + +# Verify contents of file +echo "Verifying length of test file" +FILE_LENGTH=`wc -l $TEST_TEXT_FILE | awk '{print $1}'` +if [ "$FILE_LENGTH" -ne "$TEST_TEXT_FILE_LENGTH" ] +then + echo "error: expected $TEST_TEXT_FILE_LENGTH , got $FILE_LENGTH" + exit 1 +fi + +# Delete the test file +rm $TEST_TEXT_FILE +if [ -e $TEST_TEXT_FILE ] +then + echo "Could not delete file, it still exists" + exit 1 +fi + +########################################################## +# Rename test (individual file) +########################################################## +echo "Testing mv file function ..." + +# if the rename file exists, delete it +if [ -e $ALT_TEST_TEXT_FILE ] +then + rm $ALT_TEST_TEXT_FILE +fi + +if [ -e $ALT_TEST_TEXT_FILE ] +then + echo "Could not delete file ${ALT_TEST_TEXT_FILE}, it still exists" + exit 1 +fi + +# create the test file again +echo $TEST_TEXT > $TEST_TEXT_FILE +if [ ! -e $TEST_TEXT_FILE ] +then + echo "Could not create file ${TEST_TEXT_FILE}, it does not exist" + exit 1 +fi + +#rename the test file +mv $TEST_TEXT_FILE $ALT_TEST_TEXT_FILE +if [ ! -e $ALT_TEST_TEXT_FILE ] +then + echo "Could not move file" + exit 1 +fi + +# Check the contents of the alt file +ALT_TEXT_LENGTH=`echo $TEST_TEXT | wc -c | awk '{print $1}'` +ALT_FILE_LENGTH=`wc -c $ALT_TEST_TEXT_FILE | awk '{print $1}'` +if [ "$ALT_FILE_LENGTH" -ne "$ALT_TEXT_LENGTH" ] +then + echo "moved file length is not as expected expected: $ALT_TEXT_LENGTH got: $ALT_FILE_LENGTH" + exit 1 +fi + +# clean up +rm $ALT_TEST_TEXT_FILE + +if [ -e $ALT_TEST_TEXT_FILE ] +then + echo "Could not cleanup file ${ALT_TEST_TEXT_FILE}, it still exists" + exit 1 +fi + +########################################################## +# Rename test (individual directory) +########################################################## +echo "Testing mv directory function ..." +if [ -e $TEST_DIR ]; then + echo "Unexpected, this file/directory exists: ${TEST_DIR}" + exit 1 +fi + +mkdir ${TEST_DIR} + +if [ ! -d ${TEST_DIR} ]; then + echo "Directory ${TEST_DIR} was not created" + exit 1 +fi + +mv ${TEST_DIR} ${TEST_DIR}_rename + +if [ ! -d "${TEST_DIR}_rename" ]; then + echo "Directory ${TEST_DIR} was not renamed" + exit 1 +fi + +rmdir ${TEST_DIR}_rename +if [ -e "${TEST_DIR}_rename" ]; then + echo "Could not remove the test directory, it still exists: ${TEST_DIR}_rename" + exit 1 +fi + +################################################################### +# test redirects > and >> +################################################################### +echo "Testing redirects ..." + +echo ABCDEF > $TEST_TEXT_FILE +if [ ! -e $TEST_TEXT_FILE ] +then + echo "Could not create file ${TEST_TEXT_FILE}, it does not exist" + exit 1 +fi + +CONTENT=`cat $TEST_TEXT_FILE` + +if [ ${CONTENT} != "ABCDEF" ]; then + echo "CONTENT read is unexpected, got ${CONTENT}, expected ABCDEF" + exit 1 +fi + +echo XYZ > $TEST_TEXT_FILE + +CONTENT=`cat $TEST_TEXT_FILE` + +if [ ${CONTENT} != "XYZ" ]; then + echo "CONTENT read is unexpected, got ${CONTENT}, expected XYZ" + exit 1 +fi + +echo 123456 >> $TEST_TEXT_FILE + +LINE1=`sed -n '1,1p' $TEST_TEXT_FILE` +LINE2=`sed -n '2,2p' $TEST_TEXT_FILE` + +if [ ${LINE1} != "XYZ" ]; then + echo "LINE1 was not as expected, got ${LINE1}, expected XYZ" + exit 1 +fi + +if [ ${LINE2} != "123456" ]; then + echo "LINE2 was not as expected, got ${LINE2}, expected 123456" + exit 1 +fi + + +# clean up +rm $TEST_TEXT_FILE + +if [ -e $TEST_TEXT_FILE ] +then + echo "Could not cleanup file ${TEST_TEXT_FILE}, it still exists" + exit 1 +fi + +##################################################################### +# Simple directory test mkdir/rmdir +##################################################################### +echo "Testing creation/removal of a directory" + +if [ -e $TEST_DIR ]; then + echo "Unexpected, this file/directory exists: ${TEST_DIR}" + exit 1 +fi + +mkdir ${TEST_DIR} + +if [ ! -d ${TEST_DIR} ]; then + echo "Directory ${TEST_DIR} was not created" + exit 1 +fi + +rmdir ${TEST_DIR} +if [ -e $TEST_DIR ]; then + echo "Could not remove the test directory, it still exists: ${TEST_DIR}" + exit 1 +fi + +########################################################## +# File permissions test (individual file) +########################################################## +echo "Testing chmod file function ..." + +# create the test file again +echo $TEST_TEXT > $TEST_TEXT_FILE +if [ ! -e $TEST_TEXT_FILE ] +then + echo "Could not create file ${TEST_TEXT_FILE}" + exit 1 +fi + +ORIGINAL_PERMISSIONS=$(stat --format=%a $TEST_TEXT_FILE) + +chmod 777 $TEST_TEXT_FILE; + +# if they're the same, we have a problem. +if [ $(stat --format=%a $TEST_TEXT_FILE) == $ORIGINAL_PERMISSIONS ] +then + echo "Could not modify $TEST_TEXT_FILE permissions" + exit 1 +fi + +# clean up +rm $TEST_TEXT_FILE + +if [ -e $TEST_TEXT_FILE ] +then + echo "Could not cleanup file ${TEST_TEXT_FILE}" + exit 1 +fi + +########################################################## +# File permissions test (individual file) +########################################################## +echo "Testing chown file function ..." + +# create the test file again +echo $TEST_TEXT > $TEST_TEXT_FILE +if [ ! -e $TEST_TEXT_FILE ] +then + echo "Could not create file ${TEST_TEXT_FILE}" + exit 1 +fi + +ORIGINAL_PERMISSIONS=$(stat --format=%u:%g $TEST_TEXT_FILE) + +chown 1000:1000 $TEST_TEXT_FILE; + +# if they're the same, we have a problem. +if [ $(stat --format=%a $TEST_TEXT_FILE) == $ORIGINAL_PERMISSIONS ] +then + echo "Could not modify $TEST_TEXT_FILE ownership" + exit 1 +fi + +# clean up +rm -f $TEST_TEXT_FILE + +if [ -e $TEST_TEXT_FILE ] +then + echo "Could not cleanup file ${TEST_TEXT_FILE}" + exit 1 +fi + +########################################################## +# Testing rename before close +########################################################## +echo "Testing rename before close ..." +$CUR_DIR/rename_before_close $TEST_TEXT_FILE +if [ $? != 0 ]; then + echo "rename before close failed" + exit 1 +fi + +# clean up +rm -f $TEST_TEXT_FILE + +if [ -e $TEST_TEXT_FILE ] +then + echo "Could not cleanup file ${TEST_TEXT_FILE}" + exit 1 +fi + +##################################################################### +# Tests are finished +##################################################################### + +# Unmount the bucket +cd $CUR_DIR +echo "All tests complete." diff --git a/test/rename_before_close.c b/test/rename_before_close.c new file mode 100644 index 0000000..1048aa0 --- /dev/null +++ b/test/rename_before_close.c @@ -0,0 +1,67 @@ +#include +#include +#include +#include +#include +#include +#include + +static const char FILE_CONTENT[] = "XXXX"; + +static char * +filename_to_mkstemp_template(const char *file) +{ + size_t len = strlen(file); + static const char suffix[] = ".XXXXXX"; + size_t new_len = len + sizeof(suffix); + char *ret_str = calloc(1, new_len); + int ret = snprintf(ret_str, new_len, "%s%s", file, suffix); + assert(ret == new_len - 1); + assert(ret_str[new_len] == '\0'); + return ret_str; +} + +static off_t +get_file_size(const char *file) +{ + struct stat ss; + int ret = lstat(file, &ss); + assert(ret == 0); + return ss.st_size; +} + +static void +test_rename_before_close(const char *file) +{ + char *template = filename_to_mkstemp_template(file); + int fd = mkstemp(template); + assert(fd >= 0); + + int ret = write(fd, FILE_CONTENT, sizeof(FILE_CONTENT)); + assert(ret == sizeof(FILE_CONTENT)); + + ret = fsync(fd); + assert(ret == 0); + + assert(get_file_size(template) == sizeof(FILE_CONTENT)); + + ret = rename(template, file); + assert(ret == 0); + + ret = close(fd); + assert(ret == 0); + + assert(get_file_size(file) == sizeof(FILE_CONTENT)); +} + +int +main(int argc, char *argv[]) +{ + if (argc < 2) { + printf("Usage: %s ", argv[0]); + return 1; + } + + test_rename_before_close(argv[1]); + return 0; +} diff --git a/test/small-integration-test.sh b/test/small-integration-test.sh index 548f5c1..ef1da1e 100755 --- a/test/small-integration-test.sh +++ b/test/small-integration-test.sh @@ -1,17 +1,8 @@ #!/bin/bash -e -COMMON=integration-test-common.sh -source $COMMON # Require root REQUIRE_ROOT=require-root.sh -source $REQUIRE_ROOT - -# Configuration -TEST_TEXT="HELLO WORLD" -TEST_TEXT_FILE=test-s3fs.txt -TEST_DIR=testdir -ALT_TEST_TEXT_FILE=test-s3fs-ALT.txt -TEST_TEXT_FILE_LENGTH=15 +#source $REQUIRE_ROOT # Mount the bucket if [ ! -d $TEST_BUCKET_MOUNT_POINT_1 ] @@ -19,266 +10,9 @@ then mkdir -p $TEST_BUCKET_MOUNT_POINT_1 fi $S3FS $TEST_BUCKET_1 $TEST_BUCKET_MOUNT_POINT_1 -o passwd_file=$S3FS_CREDENTIALS_FILE -CUR_DIR=`pwd` -cd $TEST_BUCKET_MOUNT_POINT_1 -if [ -e $TEST_TEXT_FILE ] -then - rm -f $TEST_TEXT_FILE -fi +./integration-test-main.sh $TEST_BUCKET_MOUNT_POINT_1 -# Write a small test file -for x in `seq 1 $TEST_TEXT_FILE_LENGTH` -do - echo "echo ${TEST_TEXT} to ${TEST_TEXT_FILE}" - echo $TEST_TEXT >> $TEST_TEXT_FILE -done - -# Verify contents of file -echo "Verifying length of test file" -FILE_LENGTH=`wc -l $TEST_TEXT_FILE | awk '{print $1}'` -if [ "$FILE_LENGTH" -ne "$TEST_TEXT_FILE_LENGTH" ] -then - echo "error: expected $TEST_TEXT_FILE_LENGTH , got $FILE_LENGTH" - exit 1 -fi - -# Delete the test file -rm $TEST_TEXT_FILE -if [ -e $TEST_TEXT_FILE ] -then - echo "Could not delete file, it still exists" - exit 1 -fi - -########################################################## -# Rename test (individual file) -########################################################## -echo "Testing mv file function ..." - -# if the rename file exists, delete it -if [ -e $ALT_TEST_TEXT_FILE ] -then - rm $ALT_TEST_TEXT_FILE -fi - -if [ -e $ALT_TEST_TEXT_FILE ] -then - echo "Could not delete file ${ALT_TEST_TEXT_FILE}, it still exists" - exit 1 -fi - -# create the test file again -echo $TEST_TEXT > $TEST_TEXT_FILE -if [ ! -e $TEST_TEXT_FILE ] -then - echo "Could not create file ${TEST_TEXT_FILE}, it does not exist" - exit 1 -fi - -#rename the test file -mv $TEST_TEXT_FILE $ALT_TEST_TEXT_FILE -if [ ! -e $ALT_TEST_TEXT_FILE ] -then - echo "Could not move file" - exit 1 -fi - -# Check the contents of the alt file -ALT_TEXT_LENGTH=`echo $TEST_TEXT | wc -c | awk '{print $1}'` -ALT_FILE_LENGTH=`wc -c $ALT_TEST_TEXT_FILE | awk '{print $1}'` -if [ "$ALT_FILE_LENGTH" -ne "$ALT_TEXT_LENGTH" ] -then - echo "moved file length is not as expected expected: $ALT_TEXT_LENGTH got: $ALT_FILE_LENGTH" - exit 1 -fi - -# clean up -rm $ALT_TEST_TEXT_FILE - -if [ -e $ALT_TEST_TEXT_FILE ] -then - echo "Could not cleanup file ${ALT_TEST_TEXT_FILE}, it still exists" - exit 1 -fi - -########################################################## -# Rename test (individual directory) -########################################################## -echo "Testing mv directory function ..." -if [ -e $TEST_DIR ]; then - echo "Unexpected, this file/directory exists: ${TEST_DIR}" - exit 1 -fi - -mkdir ${TEST_DIR} - -if [ ! -d ${TEST_DIR} ]; then - echo "Directory ${TEST_DIR} was not created" - exit 1 -fi - -mv ${TEST_DIR} ${TEST_DIR}_rename - -if [ ! -d "${TEST_DIR}_rename" ]; then - echo "Directory ${TEST_DIR} was not renamed" - exit 1 -fi - -rmdir ${TEST_DIR}_rename -if [ -e "${TEST_DIR}_rename" ]; then - echo "Could not remove the test directory, it still exists: ${TEST_DIR}_rename" - exit 1 -fi - -################################################################### -# test redirects > and >> -################################################################### -echo "Testing redirects ..." - -echo ABCDEF > $TEST_TEXT_FILE -if [ ! -e $TEST_TEXT_FILE ] -then - echo "Could not create file ${TEST_TEXT_FILE}, it does not exist" - exit 1 -fi - -CONTENT=`cat $TEST_TEXT_FILE` - -if [ ${CONTENT} != "ABCDEF" ]; then - echo "CONTENT read is unexpected, got ${CONTENT}, expected ABCDEF" - exit 1 -fi - -echo XYZ > $TEST_TEXT_FILE - -CONTENT=`cat $TEST_TEXT_FILE` - -if [ ${CONTENT} != "XYZ" ]; then - echo "CONTENT read is unexpected, got ${CONTENT}, expected XYZ" - exit 1 -fi - -echo 123456 >> $TEST_TEXT_FILE - -LINE1=`sed -n '1,1p' $TEST_TEXT_FILE` -LINE2=`sed -n '2,2p' $TEST_TEXT_FILE` - -if [ ${LINE1} != "XYZ" ]; then - echo "LINE1 was not as expected, got ${LINE1}, expected XYZ" - exit 1 -fi - -if [ ${LINE2} != "123456" ]; then - echo "LINE2 was not as expected, got ${LINE2}, expected 123456" - exit 1 -fi - - -# clean up -rm $TEST_TEXT_FILE - -if [ -e $TEST_TEXT_FILE ] -then - echo "Could not cleanup file ${TEST_TEXT_FILE}, it still exists" - exit 1 -fi - -##################################################################### -# Simple directory test mkdir/rmdir -##################################################################### -echo "Testing creation/removal of a directory" - -if [ -e $TEST_DIR ]; then - echo "Unexpected, this file/directory exists: ${TEST_DIR}" - exit 1 -fi - -mkdir ${TEST_DIR} - -if [ ! -d ${TEST_DIR} ]; then - echo "Directory ${TEST_DIR} was not created" - exit 1 -fi - -rmdir ${TEST_DIR} -if [ -e $TEST_DIR ]; then - echo "Could not remove the test directory, it still exists: ${TEST_DIR}" - exit 1 -fi - -########################################################## -# File permissions test (individual file) -########################################################## -echo "Testing chmod file function ..." - -# create the test file again -echo $TEST_TEXT > $TEST_TEXT_FILE -if [ ! -e $TEST_TEXT_FILE ] -then - echo "Could not create file ${TEST_TEXT_FILE}" - exit 1 -fi - -ORIGINAL_PERMISSIONS=$(stat --format=%a $TEST_TEXT_FILE) - -chmod 777 $TEST_TEXT_FILE; - -# if they're the same, we have a problem. -if [ $(stat --format=%a $TEST_TEXT_FILE) == $ORIGINAL_PERMISSIONS ] -then - echo "Could not modify $TEST_TEXT_FILE permissions" - exit 1 -fi - -# clean up -rm $TEST_TEXT_FILE - -if [ -e $TEST_TEXT_FILE ] -then - echo "Could not cleanup file ${TEST_TEXT_FILE}" - exit 1 -fi - -########################################################## -# File permissions test (individual file) -########################################################## -echo "Testing chown file function ..." - -# create the test file again -echo $TEST_TEXT > $TEST_TEXT_FILE -if [ ! -e $TEST_TEXT_FILE ] -then - echo "Could not create file ${TEST_TEXT_FILE}" - exit 1 -fi - -ORIGINAL_PERMISSIONS=$(stat --format=%u:%g $TEST_TEXT_FILE) - -chown 1000:1000 $TEST_TEXT_FILE; - -# if they're the same, we have a problem. -if [ $(stat --format=%a $TEST_TEXT_FILE) == $ORIGINAL_PERMISSIONS ] -then - echo "Could not modify $TEST_TEXT_FILE ownership" - exit 1 -fi - -# clean up -rm $TEST_TEXT_FILE - -if [ -e $TEST_TEXT_FILE ] -then - echo "Could not cleanup file ${TEST_TEXT_FILE}" - exit 1 -fi - -##################################################################### -# Tests are finished -##################################################################### - -# Unmount the bucket -cd $CUR_DIR umount $TEST_BUCKET_MOUNT_POINT_1 echo "All tests complete."