From 9d6df0b3065867d5ca1a597bedb10eab5a1c9235 Mon Sep 17 00:00:00 2001
From: "Neil.wrz" <wangrunze13@huawei.com>
Date: Mon, 20 Mar 2023 23:47:25 -0700
Subject: [PATCH 53/53] bugfix when refresh can't load or pull images

Signed-off-by: Neil.wrz <wangrunze13@huawei.com>
---
 src/daemon/modules/image/oci/oci_image.c      | 105 +++++++++++++++++-
 .../remote_layer_support/remote_support.c     |  34 +++++-
 .../remote_layer_support/remote_support.h     |   4 +-
 .../modules/image/oci/storage/storage.c       |   2 +-
 .../modules/image/oci/storage/storage.h       |   2 +
 5 files changed, 143 insertions(+), 4 deletions(-)

diff --git a/src/daemon/modules/image/oci/oci_image.c b/src/daemon/modules/image/oci/oci_image.c
index fa92a861..40e9a88f 100644
--- a/src/daemon/modules/image/oci/oci_image.c
+++ b/src/daemon/modules/image/oci/oci_image.c
@@ -44,6 +44,39 @@
 
 struct oci_image_module_data g_oci_image_module_data = { 0 };
 
+#ifdef ENABLE_REMOTE_LAYER_STORE
+// intend to make remote refresh and oci ops exlusive
+static bool g_enable_remote;
+static pthread_rwlock_t g_remote_lock = PTHREAD_RWLOCK_INITIALIZER;
+
+static inline bool oci_remote_lock(pthread_rwlock_t *remote_lock, bool writable)
+{
+    int nret = 0;
+
+    if (writable) {
+        nret = pthread_rwlock_wrlock(remote_lock);
+    } else {
+        nret = pthread_rwlock_rdlock(remote_lock);
+    }
+    if (nret != 0) {
+        ERROR("Lock memory store failed: %s", strerror(nret));
+        return false;
+    }
+
+    return true;
+}
+
+static inline void oci_remote_unlock(pthread_rwlock_t *remote_lock)
+{
+    int nret = 0;
+
+    nret = pthread_rwlock_unlock(remote_lock);
+    if (nret != 0) {
+        FATAL("Unlock memory store failed: %s", strerror(nret));
+    }
+}
+#endif
+
 static void free_oci_image_data(void)
 {
     free(g_oci_image_module_data.root_dir);
@@ -220,6 +253,7 @@ static int storage_module_init_helper(const isulad_daemon_configs *args)
 
 #ifdef ENABLE_REMOTE_LAYER_STORE
     storage_opts->enable_remote_layer = args->storage_enable_remote_layer;
+    storage_opts->remote_lock = &g_remote_lock;
 #endif
 
     if (util_dup_array_of_strings((const char **)args->storage_opts, args->storage_opts_len, &storage_opts->driver_opts,
@@ -303,6 +337,10 @@ int oci_init(const isulad_daemon_configs *args)
         goto out;
     }
 
+#ifdef ENABLE_REMOTE_LAYER_STORE
+    g_enable_remote = args->storage_enable_remote_layer;
+#endif
+
     if (storage_module_init_helper(args) != 0) {
         ret = -1;
         goto out;
@@ -321,6 +359,7 @@ void oci_exit()
 
 int oci_pull_rf(const im_pull_request *request, im_pull_response *response)
 {
+    int ret = 0;
     if (request == NULL || request->image == NULL || response == NULL) {
         ERROR("Invalid NULL param");
         return -1;
@@ -331,8 +370,24 @@ int oci_pull_rf(const im_pull_request *request, im_pull_response *response)
         isulad_try_set_error_message("Invalid image name: %s", request->image);
         return -1;
     }
+#ifdef ENABLE_REMOTE_LAYER_STORE
+    // read lock here because pull have exclusive access against remote refresh
+    // pull can work concurrently with other oci operations.
+    if (g_enable_remote && !oci_remote_lock(&g_remote_lock, false)) {
+        ERROR("Failed to lock oci remote lock when load image");
+        return -1;
+    }
+#endif
+
+    ret = oci_do_pull_image(request, response);
+
+#ifdef ENABLE_REMOTE_LAYER_STORE
+    if (g_enable_remote) {
+        oci_remote_unlock(&g_remote_lock);
+    }
+#endif
 
-    return oci_do_pull_image(request, response);
+    return ret;
 }
 
 int oci_prepare_rf(const im_prepare_request *request, char **real_rootfs)
@@ -441,6 +496,15 @@ int oci_rmi(const im_rmi_request *request)
         return -1;
     }
 
+#ifdef ENABLE_REMOTE_LAYER_STORE
+    // read lock here because load have exclusive access against remote refresh
+    // load can work concurrently with other oci operations.
+    if (g_enable_remote && !oci_remote_lock(&g_remote_lock, false)) {
+        ERROR("Failed to lock oci remote lock when load image");
+        return -1;
+    }
+#endif
+
     if (!util_valid_image_name(request->image.image)) {
         ERROR("Invalid image name: %s", request->image.image);
         isulad_try_set_error_message("Invalid image name: %s", request->image.image);
@@ -502,6 +566,11 @@ int oci_rmi(const im_rmi_request *request)
     }
 
 out:
+#ifdef ENABLE_REMOTE_LAYER_STORE
+    if (g_enable_remote) {
+        oci_remote_unlock(&g_remote_lock);
+    }
+#endif
     free(real_image_name);
     free(image_ID);
     util_free_array_by_len(image_names, image_names_len);
@@ -527,7 +596,24 @@ int oci_import(const im_import_request *request, char **id)
         goto err_out;
     }
 
+#ifdef ENABLE_REMOTE_LAYER_STORE
+    // read lock here because import have exclusive access against remote refresh
+    // import can work concurrently with other oci operations.
+    if (g_enable_remote && !oci_remote_lock(&g_remote_lock, false)) {
+        ERROR("Failed to lock oci remote lock when load image");
+        ret = -1;
+        goto err_out;
+    }
+#endif
+
     ret = oci_do_import(request->file, dest_name, id);
+
+#ifdef ENABLE_REMOTE_LAYER_STORE
+    if (g_enable_remote) {
+        oci_remote_unlock(&g_remote_lock);
+    }
+#endif
+
     if (ret != 0) {
         goto err_out;
     }
@@ -677,7 +763,24 @@ int oci_load_image(const im_load_request *request)
         goto out;
     }
 
+#ifdef ENABLE_REMOTE_LAYER_STORE
+    // read lock here because load have exclusive access against remote refresh
+    // load can work concurrently with other oci operations.
+    if (g_enable_remote && !oci_remote_lock(&g_remote_lock, false)) {
+        ERROR("Failed to lock oci remote lock when load image");
+        ret = -1;
+        goto out;
+    }
+#endif
+
     ret = oci_do_load(request);
+
+#ifdef ENABLE_REMOTE_LAYER_STORE
+    if (g_enable_remote) {
+        oci_remote_unlock(&g_remote_lock);
+    }
+#endif
+
     if (ret != 0) {
         ERROR("Failed to load image");
         goto out;
diff --git a/src/daemon/modules/image/oci/storage/remote_layer_support/remote_support.c b/src/daemon/modules/image/oci/storage/remote_layer_support/remote_support.c
index 3c7d0f54..7d457755 100644
--- a/src/daemon/modules/image/oci/storage/remote_layer_support/remote_support.c
+++ b/src/daemon/modules/image/oci/storage/remote_layer_support/remote_support.c
@@ -24,10 +24,38 @@ struct supporters {
     struct remote_image_data *image_data;
     struct remote_layer_data *layer_data;
     struct remote_overlay_data *overlay_data;
+    pthread_rwlock_t *remote_lock;
 };
 
 static struct supporters supporters;
 
+static inline bool remote_refresh_lock(pthread_rwlock_t *remote_lock, bool writable)
+{
+    int nret = 0;
+
+    if (writable) {
+        nret = pthread_rwlock_wrlock(remote_lock);
+    } else {
+        nret = pthread_rwlock_rdlock(remote_lock);
+    }
+    if (nret != 0) {
+        ERROR("Lock memory store failed: %s", strerror(nret));
+        return false;
+    }
+
+    return true;
+}
+
+static inline void remote_refresh_unlock(pthread_rwlock_t *remote_lock)
+{
+    int nret = 0;
+
+    nret = pthread_rwlock_unlock(remote_lock);
+    if (nret != 0) {
+        FATAL("Unlock memory store failed: %s", strerror(nret));
+    }
+}
+
 static void *remote_refresh_ro_symbol_link(void *arg)
 {
     struct supporters *refresh_supporters = (struct supporters *)arg;
@@ -37,16 +65,18 @@ static void *remote_refresh_ro_symbol_link(void *arg)
         util_usleep_nointerupt(5 * 1000 * 1000);
         DEBUG("remote refresh start\n");
 
+        remote_refresh_lock(supporters.remote_lock, true);
         remote_overlay_refresh(refresh_supporters->overlay_data);
         remote_layer_refresh(refresh_supporters->layer_data);
         remote_image_refresh(refresh_supporters->image_data);
+        remote_refresh_unlock(supporters.remote_lock);
 
         DEBUG("remote refresh end\n");
     }
     return NULL;
 }
 
-int remote_start_refresh_thread(void)
+int remote_start_refresh_thread(pthread_rwlock_t *remote_lock)
 {
     int res = 0;
     pthread_t a_thread;
@@ -67,6 +97,8 @@ int remote_start_refresh_thread(void)
         goto free_out;
     }
 
+    supporters.remote_lock = remote_lock;
+
     res = pthread_create(&a_thread, NULL, remote_refresh_ro_symbol_link, (void *)&supporters);
     if (res != 0) {
         CRIT("Thread creation failed");
diff --git a/src/daemon/modules/image/oci/storage/remote_layer_support/remote_support.h b/src/daemon/modules/image/oci/storage/remote_layer_support/remote_support.h
index 892a9155..30e3ebb0 100644
--- a/src/daemon/modules/image/oci/storage/remote_layer_support/remote_support.h
+++ b/src/daemon/modules/image/oci/storage/remote_layer_support/remote_support.h
@@ -16,6 +16,8 @@
 #ifndef DAEMON_MODULES_IMAGE_OCI_STORAGE_LAYER_STORE_REMOTE_LAYER_SUPPORT_REMOTE_SUPPORT_H
 #define DAEMON_MODULES_IMAGE_OCI_STORAGE_LAYER_STORE_REMOTE_LAYER_SUPPORT_REMOTE_SUPPORT_H
 
+#include <pthread.h>
+
 #include "linked_list.h"
 #include "map.h"
 #include "ro_symlink_maintain.h"
@@ -64,7 +66,7 @@ void remote_overlay_refresh(struct remote_overlay_data *data);
 bool remote_overlay_layer_valid(const char *layer_id);
 
 // start refresh remote
-int remote_start_refresh_thread(void);
+int remote_start_refresh_thread(pthread_rwlock_t *remote_lock);
 
 // extra map utils
 char **remote_deleted_layers(const map_t *old, const map_t *new_l);
diff --git a/src/daemon/modules/image/oci/storage/storage.c b/src/daemon/modules/image/oci/storage/storage.c
index f9830ac3..836ccf4d 100644
--- a/src/daemon/modules/image/oci/storage/storage.c
+++ b/src/daemon/modules/image/oci/storage/storage.c
@@ -1874,7 +1874,7 @@ int storage_module_init(struct storage_module_init_options *opts)
     }
 
 #ifdef ENABLE_REMOTE_LAYER_STORE
-    if (opts->enable_remote_layer && remote_start_refresh_thread() != 0) {
+    if (opts->enable_remote_layer && remote_start_refresh_thread(opts->remote_lock) != 0) {
         ERROR("Failed to start remote refresh thread");
     }
 #endif
diff --git a/src/daemon/modules/image/oci/storage/storage.h b/src/daemon/modules/image/oci/storage/storage.h
index 7404ee54..df9fd761 100644
--- a/src/daemon/modules/image/oci/storage/storage.h
+++ b/src/daemon/modules/image/oci/storage/storage.h
@@ -18,6 +18,7 @@
 #include <stdint.h>
 #include <stdbool.h>
 #include <stddef.h>
+#include <pthread.h>
 #include <isula_libutils/imagetool_image.h>
 #include <isula_libutils/json_common.h>
 
@@ -72,6 +73,7 @@ struct storage_module_init_options {
     bool integration_check;
 #ifdef ENABLE_REMOTE_LAYER_STORE
     bool enable_remote_layer;
+    pthread_rwlock_t *remote_lock;
 #endif
 };
 
-- 
2.25.1