change logic wrt and ordering, dont umount, change ownership earlier, add force

Change-Id: I1f35c9597580f3392f0bb80ab0ef0659c17c8ca2

diff --git a/src/service/src/dir_backend.hpp b/src/service/src/dir_backend.hpp
index 1fa75ea51a3e268d2c7a180d2b5f76f4127f3735..d3e8c62934fa3b50bea32bf9aa5e3fd294a82b3c 100644 (file)
--- a/src/service/src/dir_backend.hpp
+++ b/src/service/src/dir_backend.hpp
@@ -12,7 +12,7 @@ struct DirBackend {
 
 struct DirBackendAdd {
         virtual fs::path AddSubsessionPrepare (const fs::path& subsession_path, int uid, int gid) const = 0;
-        virtual fs::path AddSubsessionPrepareFromTemplate (const fs::path& subsession_path, const fs::path& main_path) const = 0;
+        virtual fs::path AddSubsessionPrepareFromTemplate (const fs::path& subsession_path, const fs::path& main_path, int uid, int gid) const = 0;
        virtual void AddSubsessionFinalize (const fs::path& tmpdir_path, const fs::path& subsession_path, int uid, int gid) const = 0;
        virtual void AddSubsessionCleanupFailure (const fs::path& tmpdir_path, const fs::path& subsession_path) const = 0;
 };

diff --git a/src/service/src/dir_backend_fixed_size.cpp b/src/service/src/dir_backend_fixed_size.cpp
index a717420a11f2608cfd7c1b66918b565a2892c4b4..bb087b478bfff3f771d62bedee49bb6f23e5500c 100644 (file)
--- a/src/service/src/dir_backend_fixed_size.cpp
+++ b/src/service/src/dir_backend_fixed_size.cpp
@@ -44,17 +44,36 @@ fs::path DirBackendFixedSize::GetImagePathFromSubsessionPath(fs::path subsession
        return subsession_path;
 }
 
-fs::path DirBackendAddFixedSize::AddSubsessionPrepareFromTemplate (const fs::path& subsession_path, const fs::path& main_path) const
+fs::path DirBackendAddFixedSize::AddSubsessionPrepareFromTemplate (const fs::path& subsession_path, const fs::path& main_path, int uid, int gid) const
 {
        fs::path template_path = main_path / ".template";
 
        auto tmp_subsession_path = subsession_path;
        tmp_subsession_path.replace_filename(TMP_NEW_PREFIX + subsession_path.filename().native());
+       auto image_template = DirBackendFixedSize::GetImagePathFromSubsessionPath(template_path);
+       auto image_new_tmp  = DirBackendFixedSize::GetImagePathFromSubsessionPath(tmp_subsession_path);
+       fs::copy_file (image_template, image_new_tmp);
+       do_resize2fs (image_new_tmp, size_kB);
 
-       fs::copy_file (DirBackendFixedSize::GetImagePathFromSubsessionPath(template_path),
-                      DirBackendFixedSize::GetImagePathFromSubsessionPath(tmp_subsession_path));
+       try {
+               fs::create_directory(tmp_subsession_path);
+               OS::change_owner_and_group(tmp_subsession_path, uid, gid);
+       } catch (const std::exception& ex) {
+               fs::remove(image_new_tmp);
+               fs::remove(tmp_subsession_path);
+               throw;
+       }
 
-       do_resize2fs  (DirBackendFixedSize::GetImagePathFromSubsessionPath(tmp_subsession_path), size_kB);
+/*
+       // We do not really need to do this, frankly it will make add_user() slower and switch() later due to mount done here
+       try {
+               do_mount(image_path, tmp_subsession_path);
+       } catch (const std::exception& ex) {
+               fs::remove(tmp_subsession_path);
+               fs::remove(image_path);
+               throw;
+       }
+*/
 
        return tmp_subsession_path;
 }
@@ -73,6 +92,7 @@ fs::path DirBackendAddFixedSize::AddSubsessionPrepare (const fs::path& subsessio
 
        try {
                fs::create_directory(tmp_subsession_path);
+               OS::change_owner_and_group(tmp_subsession_path, uid, gid);
        } catch (const std::exception& ex) {
                fs::remove(image_path);
                throw;
@@ -100,11 +120,13 @@ void DirBackendAddFixedSize::AddSubsessionCleanupFailure (const fs::path& tmpdir
 
 void DirBackendAddFixedSize::AddSubsessionFinalize (const fs::path& tmpdir_path, const fs::path& subsession_path, int uid, int gid) const
 {
-       umount_and_remove(tmpdir_path);
+       // XXX keep mounts! it should be possible to reuse this dir too!
+        // umount_and_remove(tmpdir_path);
 
        const auto image_path = DirBackendFixedSize::GetImagePathFromSubsessionPath(subsession_path);
        auto temp_image_path = image_path;
        temp_image_path.replace_filename(TMP_NEW_PREFIX + image_path.filename().native());
+       LOGI("rename %s %s", temp_image_path.data(), image_path.data());
        fs::rename(temp_image_path, image_path);
 
        /* The image file (as opposed to the FS inside it) doesn't
@@ -117,13 +139,15 @@ void DirBackendAddFixedSize::AddSubsessionFinalize (const fs::path& tmpdir_path,
        /* Order matters - handle .img first and the directory last,
         * since all other logic assumes that the existence of the dir
         * signifies that a valid subsession exists. */
-       fs::create_directory(subsession_path);
+       //fs::create_directory(subsession_path);
+       fs::rename(tmpdir_path, subsession_path);
 
        /* The mountpoint's properties don't actually matter, since
         * they are completely replaced with the image filesystem's
         * root dir's, but this way they are consistent regardless of
         * whether the image is mounted or not. */
-       OS::change_owner_and_group(subsession_path, uid, gid);
+       // XXX moved to AddSubsessionPrepare*
+       //OS::change_owner_and_group(subsession_path, uid, gid);
 }
 
 void DirBackendFixedSize::RemoveSubsession (const fs::path& subsession_path) const

diff --git a/src/service/src/dir_backend_fixed_size.hpp b/src/service/src/dir_backend_fixed_size.hpp
index 149a2327cc38cb0a665243c79beaccbadf48986d..d2ab98df4de7842d4d3380d00d6f4dba1e51b9d5 100644 (file)
--- a/src/service/src/dir_backend_fixed_size.hpp
+++ b/src/service/src/dir_backend_fixed_size.hpp
@@ -17,7 +17,7 @@ struct DirBackendAddFixedSize : public DirBackendAdd {
        DirBackendAddFixedSize(uint32_t s) : size_kB(s) { }
 
        fs::path AddSubsessionPrepare (const fs::path& subsession_path, int uid, int gid) const override;
-       fs::path AddSubsessionPrepareFromTemplate (const fs::path& subsession_path, const fs::path& main_path) const override;
+       fs::path AddSubsessionPrepareFromTemplate (const fs::path& subsession_path, const fs::path& main_path, int uid, int gid) const override;
        void AddSubsessionFinalize (const fs::path& tmpdir_path, const fs::path& subsession_path, int uid, int gid) const override;
        void AddSubsessionCleanupFailure (const fs::path& tmpdir_path, const fs::path& subsession_path) const override;
 };

diff --git a/src/service/src/dir_backend_regular_dir.cpp b/src/service/src/dir_backend_regular_dir.cpp
index 4eaaec04e9be65cf6143ce741f51b17aa95559ad..4bb79552658f44ba53412be16ec1529057790e50 100644 (file)
--- a/src/service/src/dir_backend_regular_dir.cpp
+++ b/src/service/src/dir_backend_regular_dir.cpp
@@ -25,7 +25,7 @@
 
 using namespace std::string_literals;
 
-fs::path DirBackendAddRegularDir::AddSubsessionPrepareFromTemplate (const fs::path& subsession_path, const fs::path &main_path) const
+fs::path DirBackendAddRegularDir::AddSubsessionPrepareFromTemplate (const fs::path& subsession_path, const fs::path &main_path, int uid, int gid) const
 {
        throw std::runtime_error("not supported");
 }

diff --git a/src/service/src/dir_backend_regular_dir.hpp b/src/service/src/dir_backend_regular_dir.hpp
index a5b7888e10a49e9f51269b0e08a6e9d0f9663053..d749b4316e1c77bc1008e768d16a444b3de193a4 100644 (file)
--- a/src/service/src/dir_backend_regular_dir.hpp
+++ b/src/service/src/dir_backend_regular_dir.hpp
@@ -10,7 +10,7 @@ struct DirBackendRegularDir : public DirBackend {
 
 struct DirBackendAddRegularDir : public DirBackendAdd {
        fs::path AddSubsessionPrepare (const fs::path& subsession_path, int uid, int gid) const override;
-        fs::path AddSubsessionPrepareFromTemplate (const fs::path& subsession_path, const fs::path &main_path) const override;
+        fs::path AddSubsessionPrepareFromTemplate (const fs::path& subsession_path, const fs::path &main_path, int uid, int gid) const override;
        void AddSubsessionFinalize (const fs::path& tmpdir_path, const fs::path& subsession_path, int uid, int gid) const override;
        void AddSubsessionCleanupFailure (const fs::path& tmpdir_path, const fs::path& subsession_path) const override;
 };

diff --git a/src/service/src/fs_helpers.cpp b/src/service/src/fs_helpers.cpp
index bdb36c30b897500058905fa3f05ecac7bb322b52..f74290d0b2dd18a720fb871e827b35df112a10a9 100644 (file)
--- a/src/service/src/fs_helpers.cpp
+++ b/src/service/src/fs_helpers.cpp
@@ -146,17 +146,17 @@ void add_user_subsession(const int session_uid, const std::string_view subsessio
 
                const int system_share_gid = OS::get_gid_from_name(system_share_group);
                const auto tmp_subsession_path = backend.AddSubsessionPrepareFromTemplate
-                       ( subsession_path, main_path );
+                       ( subsession_path, main_path, session_uid, system_share_gid );
                backend.AddSubsessionFinalize(tmp_subsession_path, subsession_path, session_uid, system_share_gid);
 
        }
        catch (const std::exception &ex) {
                LOGE("Logic exception while copying skel from template [session_uid=%d subsession_id=%s]: %s", session_uid, subsession_id.data(), ex.what());
-               return add_user_subsession_inner(session_uid, subsession_id, backend);
+               return add_user_subsession_inner(session_uid, subsession_id, backend, false);
        }
 }
 
-void add_user_subsession_inner(const int session_uid, const std::string_view subsession_id, const DirBackendAdd &backend)
+void add_user_subsession_inner(const int session_uid, const std::string_view subsession_id, const DirBackendAdd &backend, bool force)
 {
        try {
                fs::path main_path = get_main_dir_by_user_id(session_uid);
@@ -165,9 +165,14 @@ void add_user_subsession_inner(const int session_uid, const std::string_view sub
 
                fs::path subsession_path = main_path / subsession_id;
 
-               if (fs::exists(subsession_path))
-                       throw std::system_error(EEXIST, std::generic_category(),
-                               "Subsession directory already exists");
+               if (fs::exists(subsession_path)) {
+                       if (force) {
+                               LOGI("Removing old subsession subsession_id=%s in force mode", subsession_id.data());
+                               fs::remove_all(subsession_path);
+                       } else
+                               throw std::system_error(EEXIST, std::generic_category(),
+                                       "Subsession directory already exists");
+               }
 
                const int system_share_gid = OS::get_gid_from_name(system_share_group);
 

diff --git a/src/service/src/fs_helpers.hpp b/src/service/src/fs_helpers.hpp
index c4ace408da20d74aca60d22a641436bd8394408f..c2b8b5cb1e15ec156ba23dfc6ae60d76c4a9ed3d 100644 (file)
--- a/src/service/src/fs_helpers.hpp
+++ b/src/service/src/fs_helpers.hpp
@@ -12,7 +12,7 @@ std::filesystem::path get_main_dir_by_user_id(const int uid);
 std::filesystem::path get_last_subsession_path_by_user_id(const int uid);
 bool subsession_exists(const int session_uid, const std::string_view subsession_id);
 void add_user_subsession(const int session_uid, const std::string_view subsession_id, const DirBackendAdd& backend);
-void add_user_subsession_inner(const int session_uid, const std::string_view subsession_id, const DirBackendAdd& backend);
+void add_user_subsession_inner(const int session_uid, const std::string_view subsession_id, const DirBackendAdd& backend, bool force);
 void remove_user_subsession(const int session_uid, const std::string_view subsession_id);
 bool switch_user_subsession(const int session_uid, const std::string_view prev_subsession, const std::string_view next_subsession);
 std::vector<std::string> get_user_list(const int session_uid);

diff --git a/src/service/src/main_skel.cpp b/src/service/src/main_skel.cpp
index d0f66d5fc9c062b16637361a1b02a88d0161b411..be9c50c0f47f413c3e74f868ec8fdbf0fa25b3e4 100644 (file)
--- a/src/service/src/main_skel.cpp
+++ b/src/service/src/main_skel.cpp
@@ -33,7 +33,7 @@ int regenerate_skel() {
                const int username_uid = OS::get_uid_from_name(username.string());
 
                try {
-                       add_user_subsession_inner(username_uid, ".template", DirBackendAddFixedSize {template_img_size_kb});
+                       add_user_subsession_inner(username_uid, ".template", DirBackendAddFixedSize {template_img_size_kb}, true);
                } catch (std::exception& ex) {
                        LOGW("Unable to generate skel template for username %s (uid %d): %s", username, username_uid, ex.what());
                        continue;