From a5127d2ffd44253275d785424ab1bed31ae2f623 Mon Sep 17 00:00:00 2001 From: tlrx Date: Fri, 19 Dec 2014 10:38:27 +0100 Subject: [PATCH] Plugins: Installation failed when bin/ and plugins/ directories are on different filesystems Plugin installation failed when bin/, conf/ and plugins/ directories are on different file systems. The method File.move() can't be used to move a non-empty directory between different filesystems. I didn't find a simple way to unittest that, even with in-memory filesystems like jimfs or the Lucene test framework. Closes #8999 --- .../common/io/FileSystemUtils.java | 37 ++++++++++++++++--- .../elasticsearch/plugins/PluginManager.java | 6 ++- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/elasticsearch/common/io/FileSystemUtils.java b/src/main/java/org/elasticsearch/common/io/FileSystemUtils.java index 53d0f4b7a16..148c8ac028e 100644 --- a/src/main/java/org/elasticsearch/common/io/FileSystemUtils.java +++ b/src/main/java/org/elasticsearch/common/io/FileSystemUtils.java @@ -20,7 +20,6 @@ package org.elasticsearch.common.io; import com.google.common.collect.Iterators; -import com.google.common.collect.Sets; import org.apache.lucene.util.IOUtils; import org.elasticsearch.common.logging.ESLogger; @@ -33,7 +32,6 @@ import java.nio.charset.Charset; import java.nio.charset.CharsetDecoder; import java.nio.file.*; import java.nio.file.attribute.BasicFileAttributes; -import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; import static java.nio.file.FileVisitResult.CONTINUE; @@ -185,7 +183,7 @@ public final class FileSystemUtils { if (!Files.exists(path)) { // We just move the structure to new dir // we can't do atomic move here since src / dest might be on different mounts? - Files.move(dir, path); + move(dir, path); // We just ignore sub files from here return FileVisitResult.SKIP_SUBTREE; } @@ -224,16 +222,34 @@ public final class FileSystemUtils { * @param destination destination dir */ public static void copyDirectoryRecursively(Path source, Path destination) throws IOException { - Files.walkFileTree(source, new TreeCopier(source, destination)); + Files.walkFileTree(source, new TreeCopier(source, destination, false)); + } + + /** + * Move or rename a file to a target file. This method supports moving a file from + * different filesystems (not supported by Files.move()). + * + * @param source source file + * @param destination destination file + */ + public static void move(Path source, Path destination) throws IOException { + try { + // We can't use atomic move here since source & target can be on different filesystems. + Files.move(source, destination); + } catch (DirectoryNotEmptyException e) { + Files.walkFileTree(source, new TreeCopier(source, destination, true)); + } } static class TreeCopier extends SimpleFileVisitor { private final Path source; private final Path target; + private final boolean delete; - TreeCopier(Path source, Path target) { + TreeCopier(Path source, Path target, boolean delete) { this.source = source; this.target = target; + this.delete = delete; } @Override @@ -249,11 +265,22 @@ public final class FileSystemUtils { return CONTINUE; } + @Override + public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException { + if (delete) { + IOUtils.rm(dir); + } + return CONTINUE; + } + @Override public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { Path newFile = target.resolve(source.relativize(file)); try { Files.copy(file, newFile); + if ((delete) && (Files.exists(newFile))) { + Files.delete(file); + } } catch (IOException x) { // We ignore this } diff --git a/src/main/java/org/elasticsearch/plugins/PluginManager.java b/src/main/java/org/elasticsearch/plugins/PluginManager.java index 95bb2bd27aa..99b89235e6d 100644 --- a/src/main/java/org/elasticsearch/plugins/PluginManager.java +++ b/src/main/java/org/elasticsearch/plugins/PluginManager.java @@ -243,7 +243,11 @@ public class PluginManager { if (Files.exists(toLocation)) { IOUtils.rm(toLocation); } - Files.move(binFile, toLocation); + try { + FileSystemUtils.move(binFile, toLocation); + } catch (IOException e) { + throw new IOException("Could not move [" + binFile + "] to [" + toLocation + "]", e); + } if (Files.getFileStore(toLocation).supportsFileAttributeView(PosixFileAttributeView.class)) { final Set perms = new HashSet<>(); perms.add(PosixFilePermission.OWNER_EXECUTE);