[Stgt-devel] [PATCH] Proof of Concept -> Separate thread init/tear-down from backing store open/close.

[FUJITA Tomonori]

On Wed, 30 Jul 2008 17:44:25 +1000
Mark Harvey <markh794 at gmail.com> wrote:

> Patch also included as an attachment - just in case.
> 
> >From 715e434fff633ff8f346d181aeae3f27c9564553 Mon Sep 17 00:00:00 2001
> From: Mark Harvey <markh794 at gmail.com>
> Date: Wed, 30 Jul 2008 17:27:28 +1000
> Subject: Separate thread init/tear-down from backing store open/close.
> 
> Fix segfault when lu created without a backing store.
>  - Devices defined as 'removable' are able to be configured without a backing
>    store defined.
>  - thread init via bs_init() which is called for all logical units
>  - thread tear-down via bs_exit() which is called for all logical units.
>  - bs_open() limited to opening backing store path (called when required).
>  - bs_close() limited to closing backing store fd (called when required).
> 
> Note: bs_aio and bs_mmap compile-tested only.
> 
> Signed-off-by: Mark Harvey <markh794 at gmail.com>
> ---
>  usr/bs_aio.c  |   25 +++++++++++++++++--------
>  usr/bs_mmap.c |   22 ++++++++++++----------
>  usr/bs_rdwr.c |   25 ++++++++++++++-----------
>  usr/bs_ssc.c  |   25 +++++++++++++------------
>  usr/target.c  |   15 ++++++++++++---
>  usr/tgtd.h    |    2 ++
>  6 files changed, 70 insertions(+), 44 deletions(-)

Thanks, looks nice.

We need to check the return value of bs_init but the rest is fine by
me (I've attached a patch).

Have you tested this?

If it works fine for ssc and mmc, I'll apply the patches.

=
From: FUJITA Tomonori <fujita.tomonori at lab.ntt.co.jp>
Subject: [PATCH] check the return value of bs_init in tgt_device_create

Signed-off-by: FUJITA Tomonori <fujita.tomonori at lab.ntt.co.jp>
---
 usr/target.c |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/usr/target.c b/usr/target.c
index 62f284c..70bf72a 100644
--- a/usr/target.c
+++ b/usr/target.c
@@ -485,21 +485,24 @@ int tgt_device_create(int tid, int dev_type, uint64_t lun, char *params,
  	if (lu->dev_type_template.lu_init) {
 		ret = lu->dev_type_template.lu_init(lu);
 		if (ret)
-			goto free_lu;
+			goto fail_lu_init;
 	}
 
-	if (lu->bst->bs_init)
-		lu->bst->bs_init(lu);
+	if (lu->bst->bs_init) {
+		ret = lu->bst->bs_init(lu);
+		if (ret)
+			goto fail_bs_init;
+	}
 
 	if (backing && !path && !lu->attrs.removable) {
 		ret = TGTADM_INVALID_REQUEST;
-		goto free_lu;
+		goto fail_bs_init;
 	}
 
 	if (backing && path) {
 		ret = tgt_device_path_update(target, lu, path);
 		if (ret)
-			goto free_lu;
+			goto fail_bs_init;
 	}
 
 	if (tgt_drivers[target->lid]->lu_create)
@@ -541,7 +544,10 @@ out:
 	if (path)
 		free(path);
 	return ret;
-free_lu:
+fail_bs_init:
+	if (lu->bst->bs_exit)
+		lu->bst->bs_exit(lu);
+fail_lu_init:
 	free(lu);
 	goto out;
 }
-- 
1.5.4.2