blobfs: Make the behaviour of "delete file" as unlink.

Mark the file as deleted in the function spdk_fs_delete_file()
when the referance is not 0, and delete the file when it is closed,
make the behaviour as "unlink".

Change-Id: Ia934bb73c82c48fdbab79dbe4b56296a73abc01e
Signed-off-by: Cunyin Chang <cunyin.chang@intel.com>
Reviewed-on: https://review.gerrithub.io/374944
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Daniel Verkamp <daniel.verkamp@intel.com>

lib/blobfs/blobfs.c

@@ -80,6 +80,7 @@ struct spdk_file {
 	struct spdk_blob	*blob;
 	char			*name;
 	uint64_t		length;
+	bool                    is_deleted;
 	bool			open_for_writing;
 	uint64_t		length_flushed;
 	uint64_t		append_pos;
@@ -97,6 +98,11 @@ struct spdk_file {
 	TAILQ_ENTRY(spdk_file)	cache_tailq;
 };
 
+struct spdk_deleted_file {
+	spdk_blob_id	id;
+	TAILQ_ENTRY(spdk_deleted_file)	tailq;
+};
+
 struct spdk_filesystem {
 	struct spdk_blob_store	*bs;
 	TAILQ_HEAD(, spdk_file)	files;
@@ -136,6 +142,9 @@ struct spdk_fs_cb_args {
 	int rc;
 	bool from_request;
 	union {
+		struct {
+			TAILQ_HEAD(, spdk_deleted_file)	deleted_files;
+		} fs_load;
 		struct {
 			uint64_t	length;
 		} truncate;
@@ -484,19 +493,57 @@ file_alloc(struct spdk_filesystem *fs)
 	return file;
 }
 
+static void iter_delete_cb(void *ctx, int bserrno);
+
+static int
+_handle_deleted_files(struct spdk_fs_request *req)
+{
+	struct spdk_fs_cb_args *args = &req->args;
+	struct spdk_filesystem *fs = args->fs;
+
+	if (!TAILQ_EMPTY(&args->op.fs_load.deleted_files)) {
+		struct spdk_deleted_file *deleted_file;
+
+		deleted_file = TAILQ_FIRST(&args->op.fs_load.deleted_files);
+		TAILQ_REMOVE(&args->op.fs_load.deleted_files, deleted_file, tailq);
+		spdk_bs_md_delete_blob(fs->bs, deleted_file->id, iter_delete_cb, req);
+		free(deleted_file);
+		return 0;
+	}
+
+	return 1;
+}
+
+static void
+iter_delete_cb(void *ctx, int bserrno)
+{
+	struct spdk_fs_request *req = ctx;
+	struct spdk_fs_cb_args *args = &req->args;
+	struct spdk_filesystem *fs = args->fs;
+
+	if (_handle_deleted_files(req) == 0)
+		return;
+
+	args->fn.fs_op_with_handle(args->arg, fs, 0);
+	free_fs_request(req);
+
+}
+
 static void
 iter_cb(void *ctx, struct spdk_blob *blob, int rc)
 {
 	struct spdk_fs_request *req = ctx;
 	struct spdk_fs_cb_args *args = &req->args;
 	struct spdk_filesystem *fs = args->fs;
-	struct spdk_file *f;
 	uint64_t *length;
 	const char *name;
+	uint32_t *is_deleted;
 	size_t value_len;
 
 	if (rc == -ENOENT) {
 		/* Finished iterating */
+		if (_handle_deleted_files(req) == 0)
+			return;
 		args->fn.fs_op_with_handle(args->arg, fs, 0);
 		free_fs_request(req);
 		return;
@@ -519,21 +566,39 @@ iter_cb(void *ctx, struct spdk_blob *blob, int rc)
 		free_fs_request(req);
 		return;
 	}
+
 	assert(value_len == 8);
 
-	f = file_alloc(fs);
+	/* This file could be deleted last time without close it, then app crashed, so we delete it now */
-	if (f == NULL) {
+	rc = spdk_bs_md_get_xattr_value(blob, "is_deleted", (const void **)&is_deleted, &value_len);
-		args->fn.fs_op_with_handle(args->arg, fs, -ENOMEM);
+	if (rc < 0) {
-		free_fs_request(req);
+		struct spdk_file *f;
-		return;
-	}
 
-	f->name = strdup(name);
+		f = file_alloc(fs);
-	f->blobid = spdk_blob_get_id(blob);
+		if (f == NULL) {
-	f->length = *length;
+			args->fn.fs_op_with_handle(args->arg, fs, -ENOMEM);
-	f->length_flushed = *length;
+			free_fs_request(req);
-	f->append_pos = *length;
+			return;
-	SPDK_DEBUGLOG(SPDK_TRACE_BLOBFS, "added file %s length=%ju\n", f->name, f->length);
+		}
+
+		f->name = strdup(name);
+		f->blobid = spdk_blob_get_id(blob);
+		f->length = *length;
+		f->length_flushed = *length;
+		f->append_pos = *length;
+		SPDK_DEBUGLOG(SPDK_TRACE_BLOBFS, "added file %s length=%ju\n", f->name, f->length);
+	} else {
+		struct spdk_deleted_file *deleted_file;
+
+		deleted_file = calloc(1, sizeof(*deleted_file));
+		if (deleted_file == NULL) {
+			args->fn.fs_op_with_handle(args->arg, fs, -ENOMEM);
+			free_fs_request(req);
+			return;
+		}
+		deleted_file->id = spdk_blob_get_id(blob);
+		TAILQ_INSERT_TAIL(&args->op.fs_load.deleted_files, deleted_file, tailq);
+	}
 
 	spdk_bs_md_iter_next(fs->bs, &blob, iter_cb, req);
 }
@@ -586,7 +651,7 @@ spdk_fs_load(struct spdk_bs_dev *dev, fs_send_request_fn send_request_fn,
 	args->fn.fs_op_with_handle = cb_fn;
 	args->arg = cb_arg;
 	args->fs = fs;
-
+	TAILQ_INIT(&args->op.fs_load.deleted_files);
 	spdk_bs_load(dev, load_cb, req);
 }
 
@@ -919,6 +984,11 @@ spdk_fs_open_file_async(struct spdk_filesystem *fs, const char *name, uint32_t f
 		return;
 	}
 
+	if (f != NULL && f->is_deleted == true) {
+		cb_fn(cb_arg, NULL, -ENOENT);
+		return;
+	}
+
 	req = alloc_fs_request(fs->md_target.md_fs_channel);
 	if (req == NULL) {
 		cb_fn(cb_arg, NULL, -ENOMEM);
@@ -1160,18 +1230,24 @@ spdk_fs_delete_file_async(struct spdk_filesystem *fs, const char *name,
 		return;
 	}
 
-	if (f->ref_count > 0) {
-		/* For now, do not allow deleting files with open references. */
-		cb_fn(cb_arg, -EBUSY);
-		return;
-	}
-
 	req = alloc_fs_request(fs->md_target.md_fs_channel);
 	if (req == NULL) {
 		cb_fn(cb_arg, -ENOMEM);
 		return;
 	}
 
+	args = &req->args;
+	args->fn.file_op = cb_fn;
+	args->arg = cb_arg;
+
+	if (f->ref_count > 0) {
+		/* If the ref > 0, we mark the file as deleted and delete it when we close it. */
+		f->is_deleted = true;
+		spdk_blob_md_set_xattr(f->blob, "is_deleted", &f->is_deleted, sizeof(bool));
+		spdk_bs_md_sync_blob(f->blob, blob_delete_cb, args);
+		return;
+	}
+
 	TAILQ_REMOVE(&fs->files, f, tailq);
 
 	cache_free_buffers(f);
@@ -1182,9 +1258,6 @@ spdk_fs_delete_file_async(struct spdk_filesystem *fs, const char *name,
 	free(f->tree);
 	free(f);
 
-	args = &req->args;
-	args->fn.file_op = cb_fn;
-	args->arg = cb_arg;
 	spdk_bs_md_delete_blob(fs->bs, blobid, blob_delete_cb, req);
 }
 
@@ -2218,7 +2291,12 @@ __file_close_async_done(void *ctx, int bserrno)
 {
 	struct spdk_fs_request *req = ctx;
 	struct spdk_fs_cb_args *args = &req->args;
+	struct spdk_file *file = args->file;
 
+	if (file->is_deleted) {
+		spdk_fs_delete_file_async(file->fs, file->name, blob_delete_cb, ctx);
+		return;
+	}
 	args->fn.file_op(args->arg, bserrno);
 	free_fs_request(req);
 }

test/lib/blobfs/blobfs_async_ut/blobfs_async_ut.c

@@ -150,24 +150,14 @@ fs_open(void)
 	CU_ASSERT(iter == NULL);
 
 	g_fserrno = 0;
-	/* Delete should fail, since we have an open reference. */
+	/* Delete should successful, we will mark the file as deleted. */
 	spdk_fs_delete_file_async(fs, "file1", delete_cb, NULL);
-	CU_ASSERT(g_fserrno == -EBUSY);
+	CU_ASSERT(g_fserrno == 0);
 	CU_ASSERT(!TAILQ_EMPTY(&fs->files));
 
 	g_fserrno = 1;
 	spdk_file_close_async(g_file, fs_op_complete, NULL);
 	CU_ASSERT(g_fserrno == 0);
-	CU_ASSERT(g_file->ref_count == 0);
-
-	g_fserrno = 0;
-	spdk_file_close_async(g_file, fs_op_complete, NULL);
-	CU_ASSERT(g_fserrno == -EBADF);
-	CU_ASSERT(g_file->ref_count == 0);
-
-	g_fserrno = 1;
-	spdk_fs_delete_file_async(fs, "file1", delete_cb, NULL);
-	CU_ASSERT(g_fserrno == 0);
 	CU_ASSERT(TAILQ_EMPTY(&fs->files));
 
 	g_fserrno = 1;

test/lib/blobfs/blobfs_sync_ut/blobfs_sync_ut.c

@@ -286,6 +286,41 @@ cache_append_no_cache(void)
 	ut_send_request(_fs_unload, NULL);
 }
 
+static void
+fs_delete_file_without_close(void)
+{
+	int rc;
+	struct spdk_io_channel *channel;
+	struct spdk_file *file;
+
+	ut_send_request(_fs_init, NULL);
+	spdk_allocate_thread(_fs_send_msg, NULL, "thread0");
+	channel = spdk_fs_alloc_io_channel_sync(g_fs);
+	CU_ASSERT(channel != NULL);
+
+	rc = spdk_fs_open_file(g_fs, channel, "testfile", SPDK_BLOBFS_OPEN_CREATE, &g_file);
+	CU_ASSERT(rc == 0);
+
+	rc = spdk_fs_delete_file(g_fs, channel, "testfile");
+	CU_ASSERT(rc == 0);
+	CU_ASSERT(g_file->ref_count != 0);
+	CU_ASSERT(g_file->is_deleted == true);
+
+	rc = spdk_fs_open_file(g_fs, channel, "testfile", 0, &file);
+	CU_ASSERT(rc != 0);
+
+	spdk_file_close(g_file, channel);
+
+	rc = spdk_fs_open_file(g_fs, channel, "testfile", 0, &file);
+	CU_ASSERT(rc != 0);
+
+	spdk_fs_free_io_channel(channel);
+	spdk_free_thread();
+
+	ut_send_request(_fs_unload, NULL);
+
+}
+
 static void
 terminate_spdk_thread(void *arg)
 {
@@ -337,7 +372,8 @@ int main(int argc, char **argv)
 		CU_add_test(suite, "write", cache_write) == NULL ||
 		CU_add_test(suite, "write_null_buffer", cache_write_null_buffer) == NULL ||
 		CU_add_test(suite, "create_sync", fs_create_sync) == NULL ||
-		CU_add_test(suite, "append_no_cache", cache_append_no_cache) == NULL
+		CU_add_test(suite, "append_no_cache", cache_append_no_cache) == NULL ||
+		CU_add_test(suite, "delete_file_without_close", fs_delete_file_without_close) == NULL
 	) {
 		CU_cleanup_registry();
 		return CU_get_error();