[Libreoffice-commits] online.git: common/FileUtil.cpp common/JailUtil.cpp common/JailUtil.hpp kit/Kit.cpp loolwsd-systemplate-setup
Ashod Nakashian (via logerrit)
logerrit at kemper.freedesktop.org
Mon Aug 17 11:52:14 UTC 2020
common/FileUtil.cpp | 48 +++++++++++---------------
common/JailUtil.cpp | 84 ++++++++++++++++++++++------------------------
common/JailUtil.hpp | 3 +
kit/Kit.cpp | 15 ++++++--
loolwsd-systemplate-setup | 11 ++++--
5 files changed, 83 insertions(+), 78 deletions(-)
New commits:
commit bc8da0cb339cc685f9a24c78e8a92aadcd690479
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
AuthorDate: Sun Aug 16 22:46:33 2020 -0400
Commit: Andras Timar <andras.timar at collabora.com>
CommitDate: Mon Aug 17 13:51:56 2020 +0200
wsd: support read-only systemplate
For various reasons, systemplate may be read-only
or under a different owner and therefore impossible
to update the dynamic files in it.
To support such a scenario, we first link the
eight dynamic files in /etc when creating systemplate.
If this fails, we copy the files.
When creating jails, we always check that all the
dynamic files are up-to-date. If they are, nothing
further is necessary and we bind-mount, if enabled
and possible.
However, if the dynamic files are not up-to-date,
we disable bind-mounting and force linking
the files in the jails. Failing that, we copy them,
which is not ideal, but allows us to ensure the
dynamic files are up-to-date as we copy them too.
Ideally, the dynamic files in question would be
hard-link (or at least soft-linked) in systemplate
at creation. From then on we would bind-mount
the jails and everything would work perfectly and
no files would need updating. This patch is fallback
for when this scheme fails, which should be exceedingly
rare anyway, but which still ensures correct operation.
Change-Id: I09c6f057c49396579aaddb1b8bf4af0930dd4247
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/100834
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
Tested-by: Jenkins
Reviewed-by: Andras Timar <andras.timar at collabora.com>
diff --git a/common/FileUtil.cpp b/common/FileUtil.cpp
index 6a61b926e..7f8e767d7 100644
--- a/common/FileUtil.cpp
+++ b/common/FileUtil.cpp
@@ -12,7 +12,9 @@
#include "FileUtil.hpp"
#include <dirent.h>
+#include <exception>
#include <ftw.h>
+#include <stdexcept>
#include <sys/time.h>
#ifdef __linux
#include <sys/vfs.h>
@@ -89,27 +91,19 @@ namespace FileUtil
bool copy(const std::string& fromPath, const std::string& toPath, bool log, bool throw_on_error)
{
int from = -1, to = -1;
- try {
+ try
+ {
from = open(fromPath.c_str(), O_RDONLY);
if (from < 0)
- {
- LOG_SYS("Failed to open src " << anonymizeUrl(fromPath));
- throw;
- }
+ throw std::runtime_error("Failed to open src " + anonymizeUrl(fromPath));
struct stat st;
if (fstat(from, &st) != 0)
- {
- LOG_SYS("Failed to fstat src " << anonymizeUrl(fromPath));
- throw;
- }
+ throw std::runtime_error("Failed to fstat src " + anonymizeUrl(fromPath));
to = open(toPath.c_str(), O_CREAT | O_TRUNC | O_WRONLY, st.st_mode);
if (to < 0)
- {
- LOG_SYS("Failed to fstat dest " << anonymizeUrl(toPath));
- throw;
- }
+ throw std::runtime_error("Failed to open dest " + anonymizeUrl(toPath));
// Logging may be redundant and/or noisy.
if (log)
@@ -120,14 +114,14 @@ namespace FileUtil
int n;
off_t bytesIn = 0;
- do {
+ do
+ {
while ((n = ::read(from, buffer, sizeof(buffer))) < 0 && errno == EINTR)
LOG_TRC("EINTR reading from " << anonymizeUrl(fromPath));
if (n < 0)
- {
- LOG_SYS("Failed to read from " << anonymizeUrl(fromPath) << " at " << bytesIn << " bytes in");
- throw;
- }
+ throw std::runtime_error("Failed to read from " + anonymizeUrl(fromPath)
+ + " at " + std::to_string(bytesIn) + " bytes in");
+
bytesIn += n;
if (n == 0) // EOF
break;
@@ -140,13 +134,14 @@ namespace FileUtil
LOG_TRC("EINTR writing to " << anonymizeUrl(toPath));
if (written < 0)
{
- LOG_SYS("Failed to write " << n << " bytes to " << anonymizeUrl(toPath) << " at " <<
- bytesIn << " bytes into " << anonymizeUrl(fromPath));
- throw;
+ throw std::runtime_error("Failed to write " + std::to_string(n)
+ + " bytes to " + anonymizeUrl(toPath) + " at "
+ + std::to_string(bytesIn) + " bytes into "
+ + anonymizeUrl(fromPath));
}
j += written;
}
- } while(true);
+ } while (true);
if (bytesIn != st.st_size)
{
LOG_WRN("Unusual: file " << anonymizeUrl(fromPath) << " changed size "
@@ -156,19 +151,18 @@ namespace FileUtil
close(to);
return true;
}
- catch (...)
+ catch (const std::exception& ex)
{
std::ostringstream oss;
- oss << "Failed to copy from " << anonymizeUrl(fromPath) << " to "
- << anonymizeUrl(toPath);
+ oss << "Error while copying from " << anonymizeUrl(fromPath) << " to "
+ << anonymizeUrl(toPath) << ": " << ex.what();
const std::string err = oss.str();
-
LOG_SYS(err);
close(from);
close(to);
unlink(toPath.c_str());
if (throw_on_error)
- throw Poco::Exception(err);
+ throw std::runtime_error(err);
}
return false;
diff --git a/common/JailUtil.cpp b/common/JailUtil.cpp
index bc0138c0f..c7ce612df 100644
--- a/common/JailUtil.cpp
+++ b/common/JailUtil.cpp
@@ -247,17 +247,38 @@ namespace SysTemplate
/// that systemplate will get re-generated after installation.
static const auto DynamicFilePaths
= { "/etc/passwd", "/etc/group", "/etc/host.conf", "/etc/hosts",
- "/etc/nsswitch.conf", "/etc/resolv.conf", "etc/timezone", "etc/localtime" };
+ "/etc/nsswitch.conf", "/etc/resolv.conf", "/etc/timezone", "/etc/localtime" };
/// Copy (false) by default for KIT_IN_PROCESS.
static bool LinkDynamicFiles = false;
+static bool updateDynamicFilesImpl(const std::string& sysTemplate);
+
void setupDynamicFiles(const std::string& sysTemplate)
{
LOG_INF("Setting up systemplate dynamic files in [" << sysTemplate << "].");
const std::string etcSysTemplatePath = Poco::Path(sysTemplate, "etc").toString();
LinkDynamicFiles = true; // Prefer linking, unless it fails.
+
+ if (!updateDynamicFilesImpl(sysTemplate))
+ {
+ // Can't copy!
+ LOG_WRN("Failed to update the dynamic files in ["
+ << sysTemplate
+ << "]. Will disable bind-mounting in this run and clone systemplate into the "
+ "jails, which is more resource intensive.");
+ unsetenv("LOOL_BIND_MOUNT"); // We can't mount from incomplete systemplate.
+ }
+
+ if (LinkDynamicFiles)
+ LOG_INF("Systemplate dynamic files in ["
+ << sysTemplate << "] are linked and will remain up-to-date.");
+}
+
+bool updateDynamicFilesImpl(const std::string& sysTemplate)
+{
+ LOG_INF("Updating systemplate dynamic files in [" << sysTemplate << "].");
for (const auto& srcFilename : DynamicFilePaths)
{
const Poco::File srcFilePath(srcFilename);
@@ -275,6 +296,7 @@ void setupDynamicFiles(const std::string& sysTemplate)
continue;
}
+ LOG_INF("File [" << dstFilename << "] needs to be updated.");
if (LinkDynamicFiles)
{
LOG_INF("Linking [" << srcFilename << "] -> [" << dstFilename << "].");
@@ -290,8 +312,12 @@ void setupDynamicFiles(const std::string& sysTemplate)
const int linkerr = errno;
// With parallel tests, another test might have linked already.
- if (Poco::File(dstFilename).exists()) // stat again.
+ FileUtil::Stat dstStat2(dstFilename);
+ if (dstStat2.isUpToDate(srcStat))
+ {
+ LOG_INF("File [" << dstFilename << "] now seems to be up-to-date.");
continue;
+ }
// Failed to link a file. Disable linking and copy instead.
LOG_WRN("Failed to link ["
@@ -301,55 +327,27 @@ void setupDynamicFiles(const std::string& sysTemplate)
}
// Linking failed, just copy.
- LOG_INF("Copying [" << srcFilename << "] -> [" << dstFilename << "].");
- if (!FileUtil::copyAtomic(srcFilename, dstFilename, true))
+ if (!LinkDynamicFiles)
{
- if (!Poco::File(dstFilename).exists()) // stat again.
- LOG_ERR("Failed to copy [" << srcFilename << "] -> [" << dstFilename
- << "], some functionality may be missing.");
+ LOG_INF("Copying [" << srcFilename << "] -> [" << dstFilename << "].");
+ if (!FileUtil::copyAtomic(srcFilename, dstFilename, true))
+ {
+ FileUtil::Stat dstStat2(dstFilename); // Stat again.
+ if (!dstStat2.isUpToDate(srcStat))
+ {
+ return false; // No point in trying the remaining files.
+ }
+ }
}
}
- if (LinkDynamicFiles)
- LOG_INF("Successfully linked the systemplate dynamic files in ["
- << sysTemplate << "] and will not need to update them again.");
+ return true;
}
-void updateDynamicFiles(const std::string& sysTemplate)
+bool updateDynamicFiles(const std::string& sysTemplate)
{
// If the files are linked, they are always up-to-date.
- if (!LinkDynamicFiles)
- {
- LOG_INF("Updating systemplate dynamic files in [" << sysTemplate << "].");
-
- const std::string etcSysTemplatePath = Poco::Path(sysTemplate, "etc").toString();
- for (const auto& srcFilename : DynamicFilePaths)
- {
- const Poco::File srcFilePath(srcFilename);
- FileUtil::Stat srcStat(srcFilename);
- if (!srcStat.exists())
- continue;
-
- const std::string dstFilename = Poco::Path(sysTemplate, srcFilename).toString();
- FileUtil::Stat dstStat(dstFilename);
-
- // Is it outdated?
- if (dstStat.isUpToDate(srcStat))
- {
- LOG_INF("File [" << dstFilename << "] is already up-to-date.");
- }
- else
- {
- LOG_INF("Copying [" << srcFilename << "] -> [" << dstFilename << "].");
- if (!FileUtil::copyAtomic(srcFilename, dstFilename, true))
- {
- if (!Poco::File(dstFilename).exists()) // stat again.
- LOG_ERR("Failed to copy [" << srcFilename << "] -> [" << dstFilename
- << "], some functionality may be missing.");
- }
- }
- }
- }
+ return LinkDynamicFiles ? true : updateDynamicFilesImpl(sysTemplate);
}
void setupLoSymlink(const std::string& sysTemplate, const std::string& loTemplate,
diff --git a/common/JailUtil.hpp b/common/JailUtil.hpp
index d783b53d3..949fdca50 100644
--- a/common/JailUtil.hpp
+++ b/common/JailUtil.hpp
@@ -61,7 +61,8 @@ void setupRandomDeviceLinks(const std::string& root);
void setupDynamicFiles(const std::string& sysTemplate);
/// Update the dynamic files within the sysTemplate before each child fork.
-void updateDynamicFiles(const std::string& sysTemplate);
+/// Returns false on failure.
+bool updateDynamicFiles(const std::string& sysTemplate);
} // namespace SysTemplate
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index 8819947fd..2a312b986 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -2135,10 +2135,10 @@ void lokit_main(
if (std::getenv("LOOL_BIND_MOUNT"))
{
const std::string destPath = jailPath.toString();
- LOG_DBG("Mounting " << sysTemplate << " -> " << destPath);
+ LOG_INF("Mounting " << sysTemplate << " -> " << destPath);
if (!JailUtil::bind(sysTemplate, destPath))
{
- LOG_INF("Failed to mount [" << sysTemplate << "] -> [" << destPath
+ LOG_WRN("Failed to mount [" << sysTemplate << "] -> [" << destPath
<< "], will link/copy contents.");
linkOrCopy(sysTemplate, destPath, LinkOrCopyType::All);
}
@@ -2147,11 +2147,11 @@ void lokit_main(
// Mount lotemplate inside it.
const std::string loDestPath = Poco::Path(jailPath, "lo").toString();
- LOG_DBG("Mounting " << loTemplate << " -> " << loDestPath);
+ LOG_INF("Mounting " << loTemplate << " -> " << loDestPath);
Poco::File(loDestPath).createDirectories();
if (!JailUtil::bind(loTemplate, loDestPath))
{
- LOG_INF("Failed to mount [" << loTemplate << "] -> [" << loDestPath
+ LOG_WRN("Failed to mount [" << loTemplate << "] -> [" << loDestPath
<< "], will link/copy contents.");
linkOrCopy(sysTemplate, loDestPath, LinkOrCopyType::LO);
}
@@ -2180,6 +2180,13 @@ void lokit_main(
linkOrCopy(loTemplate, jailLOInstallation, LinkOrCopyType::LO);
JailUtil::setupJailDevNodes(Poco::Path(jailPath, "/tmp").toString());
+
+ // Update the dynamic files inside the jail.
+ if (!JailUtil::SysTemplate::updateDynamicFiles(jailPath.toString()))
+ {
+ LOG_WRN("Failed to update the dynamic files in the jail ["
+ << jailPath.toString() << "]. Some functionality may be missing.");
+ }
}
::setenv("TMPDIR", "/tmp", 1);
diff --git a/loolwsd-systemplate-setup b/loolwsd-systemplate-setup
index 2f6e462e0..b50d10cc3 100755
--- a/loolwsd-systemplate-setup
+++ b/loolwsd-systemplate-setup
@@ -58,13 +58,18 @@ grep -v dynamic | cut -d " " -f 3 | grep -E '^(/lib|/usr)' | sort -u | sed -e 's
# This will now copy the file a symlink points to, but whatever.
cpio -p -d -L $CHROOT
-# Remove the dynamic files, which are linked by loolwsd.
-rm -f $CHROOT/etc/{hosts,nsswitch.conf,resolv.conf,passwd,group,host.conf,timezone,localtime}
-
mkdir -p $CHROOT/lo
mkdir -p $CHROOT/dev
mkdir -p $CHROOT/tmp/dev
+# Link the dynamic files, replacing any existing.
+for file in hosts nsswitch.conf resolv.conf passwd group host.conf timezone localtime
+do
+ #echo "Linking/Copying /etc/$file"
+ # Prefer hard linking, fallback to soft linking, and finally to just copying.
+ ln -f /etc/$file $CHROOT/etc/ 2> /dev/null || ln -f -s /etc/$file $CHROOT/etc/ || cp /etc/$file $CHROOT/etc/ || echo "Failed to link or copy $file"
+done
+
# /usr/share/fonts needs to be taken care of separately because the
# directory time stamps must be preserved for fontconfig to trust
# its cache.
More information about the Libreoffice-commits
mailing list