[bugfix] some error occured when using upper layer

Signed-off-by: zhuangbowei.zbw <[email protected]>

Author: WaberZhuang

src/image_file.cpp

@@ -481,6 +481,7 @@ int ImageFile::init_image_file() {
         goto ERROR_EXIT;
     }
 
+    // have only RO layers
     if (upper.index() == "" || upper.data() == "") {
         LOG_INFO("RW layer path not set. return RO layers.");
         m_file = lower_file;
@@ -493,29 +494,36 @@ int ImageFile::init_image_file() {
         LOG_ERROR("open upper layer failed.");
         goto ERROR_EXIT;
     }
-    // We have to maintain the lower_file because prefetcher need the readonly
-    // lower_file but not the writable m_file.
-    //
-    // Otherwise, lower_file and upper_file will lose the ownership of m_files
-    // after stack_files(...), so we could safely delete them.
+
+    // have only one RW layer
+    if (!lower_file) {
+        LOG_INFO("RO layers path not set. return RW layer.");
+        m_file = upper_file;
+        read_only = false;
+        goto SUCCESS_EXIT;
+    }
+
+    // stack_files(..., ownership=true) will destruct lower_file and upper_file
+    // immediately, but the read-only lower_file is needed by prefetcher, so we
+    // have to use stack_files(..., ownership=false) instead.
     //
-    // upper_file will be deleted immediately since it's useless.
-    // lower_file will be held by ImageFile until deconstruct.
+    // For this reason, lower_file and upper_file must be maintained until m_file
+    // is deconstructed.
     stack_ret = LSMT::stack_files(upper_file, lower_file, false, false);
     if (!stack_ret) {
         LOG_ERROR("LSMT::stack_files(`, `)", (uint64_t)upper_file, true);
         goto ERROR_EXIT;
     }
     m_file = stack_ret;
     read_only = false;
-    delete upper_file;
     m_lower_file = lower_file;
+    m_upper_file = upper_file;
 
 SUCCESS_EXIT:
     if (conf.download().enable() && !record_no_download) {
         start_bk_dl_thread();
     }
-    if (m_prefetcher != nullptr) {
+    if (m_prefetcher && lower_file) {
         m_prefetcher->replay(lower_file);
     }
     return 1;

src/image_file.h

@@ -62,9 +62,8 @@ class ImageFile : public photon::fs::ForwardFile {
             m_file->close();
             delete m_file;
         }
-        if (m_lower_file) {
-            delete m_lower_file;
-        }
+        if (m_lower_file) delete m_lower_file;
+        if (m_upper_file) delete m_upper_file;
     }
 
     int fstat(struct stat *buf) override {
@@ -121,6 +120,7 @@ class ImageFile : public photon::fs::ForwardFile {
     photon::join_handle *dl_thread_jh = nullptr;
     ImageService &image_service;
     photon::fs::IFile *m_lower_file = nullptr;
+    photon::fs::IFile *m_upper_file = nullptr;
 
     int init_image_file();
     template<typename...Ts> void set_failed(const Ts&...xs);

src/overlaybd/lsmt/file.cpp

@@ -1807,7 +1807,7 @@ IFileRW *stack_files(IFileRW *upper_layer, IFileRO *lower_layers, bool ownership
         rst = new LSMTWarpFile;
         delta++;
     }
-    idx = create_combo_index((IMemoryIndex0 *)u->m_index, l->m_index, l->m_files.size(), true);
+    idx = create_combo_index((IMemoryIndex0 *)u->m_index, l->m_index, l->m_files.size(), ownership);
     rst->m_index = idx;
     rst->m_findex = u->m_findex;
     rst->m_vsize = u->m_vsize;
@@ -1830,9 +1830,9 @@ IFileRW *stack_files(IFileRW *upper_layer, IFileRO *lower_layers, bool ownership
     }
     rst->m_uuid.push_back(u->m_uuid[0]);
     rst->m_rw_tag = rst->m_files.size() - delta;
-    u->m_index = l->m_index = nullptr;
-    l->m_file_ownership = u->m_file_ownership = false;
     if (ownership) {
+        u->m_index = l->m_index = nullptr;
+        l->m_file_ownership = u->m_file_ownership = false;
         delete u;
         delete l;
     }

src/overlaybd/tar/erofs/test/erofs_stress_base.cpp

@@ -352,7 +352,7 @@ LSMT::IFileRW *StressBase::mkfs()
 
 		LSMT::IFileRW *img_file = nullptr;
 		if (i > 0)
-			img_file = LSMT::stack_files(current_layer, lowers, false, false);
+			img_file = LSMT::stack_files(current_layer, lowers, true, false);
 		else
 			img_file = current_layer;
 
@@ -362,7 +362,6 @@ LSMT::IFileRW *StressBase::mkfs()
 			delete img_file;
 			LOG_ERROR_RETURN(-1, nullptr, "fail to extract tar");
 		}
-		delete lowers;
 		delete tar;
 		lowers = img_file;
 	}