[PATCH] target: allow setting dbroot as a module parameter

[PATCH] target: allow setting dbroot as a module parameter

[Maurizio Lombardi]

The target driver prevents the users from changing
the database root directory if a target module like ib_srpt
has already been loaded during boot.

Let the users set their preferred root directory
by passing it as a module parameter. This, combined
with the modprobe.d's "options" command,
will mitigate the issue because the parameter will be
automatically passed every time the target_core_mod module is inserted
into the kernel; whether directly (using modprobe) or
because another module being loaded depends on it.

If the directory cannot be opened, the target driver will print
an error and fall back to its default (/etc/target)

Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---
 drivers/target/target_core_configfs.c | 29 ++++++++++++++++++---------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index 274ffb6b83a1..c0fc6312aa20 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -100,7 +100,10 @@ static ssize_t target_core_item_version_show(struct config_item *item,
 CONFIGFS_ATTR_RO(target_core_item_, version);
 
 char db_root[DB_ROOT_LEN] = DB_ROOT_DEFAULT;
-static char db_root_stage[DB_ROOT_LEN];
+static char db_root_stage[DB_ROOT_LEN] = {0};
+
+module_param_string(dbroot, db_root_stage, DB_ROOT_LEN, 0);
+MODULE_PARM_DESC(dbroot, "Target database root directory (def=/etc/target)");
 
 static ssize_t target_core_item_dbroot_show(struct config_item *item,
 					    char *page)
@@ -3507,25 +3510,25 @@ void target_setup_backend_cits(struct target_backend *tb)
 	target_core_setup_dev_stat_cit(tb);
 }
 
-static void target_init_dbroot(void)
+static int target_init_dbroot(char *path)
 {
 	struct file *fp;
 
-	snprintf(db_root_stage, DB_ROOT_LEN, DB_ROOT_PREFERRED);
-	fp = filp_open(db_root_stage, O_RDONLY, 0);
+	fp = filp_open(path, O_RDONLY, 0);
 	if (IS_ERR(fp)) {
-		pr_err("db_root: cannot open: %s\n", db_root_stage);
-		return;
+		pr_err("db_root: cannot open: %s\n", path);
+		return -EINVAL;
 	}
 	if (!S_ISDIR(file_inode(fp)->i_mode)) {
 		filp_close(fp, NULL);
-		pr_err("db_root: not a valid directory: %s\n", db_root_stage);
-		return;
+		pr_err("db_root: not a valid directory: %s\n", path);
+		return -EINVAL;
 	}
 	filp_close(fp, NULL);
 
-	strncpy(db_root, db_root_stage, DB_ROOT_LEN);
+	strlcpy(db_root, path, DB_ROOT_LEN);
 	pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
+	return 0;
 }
 
 static int __init target_core_init_configfs(void)
@@ -3608,7 +3611,13 @@ static int __init target_core_init_configfs(void)
 	if (ret < 0)
 		goto out;
 
-	target_init_dbroot();
+	if (db_root_stage[0]) {
+		ret = target_init_dbroot(db_root_stage);
+		if (ret < 0)
+			target_init_dbroot(DB_ROOT_PREFERRED);
+	} else {
+		target_init_dbroot(DB_ROOT_PREFERRED);
+	}
 
 	return 0;
 
-- 
2.27.0

Re: [PATCH] target: allow setting dbroot as a module parameter

[Mike Christie]

On 9/27/21 2:38 PM, Maurizio Lombardi wrote:
> The target driver prevents the users from changing
> the database root directory if a target module like ib_srpt
> has already been loaded during boot.

Why is that not allowed if we have a fabric driver loaded?

It looks like we don't start using the db_root until we have created
a device (alua and pr metadata), so you could add a:

static bool target_device_created;

target_core_make_subdev()
{
	....

	target_device_created = true;
}


target_core_item_dbroot_store()
{
	if (target_device_created)
		return -EINVAL
....

Re: [PATCH] target: allow setting dbroot as a module parameter

[Maurizio Lombardi]

Hi Mike,

po 11. 10. 2021 v 18:50 odesílatel Mike Christie
<michael.christie@oracle.com> napsal:
>
> On 9/27/21 2:38 PM, Maurizio Lombardi wrote:
> > The target driver prevents the users from changing
> > the database root directory if a target module like ib_srpt
> > has already been loaded during boot.
>
> Why is that not allowed if we have a fabric driver loaded?
>
> It looks like we don't start using the db_root until we have created
> a device (alua and pr metadata)

Your solution would work perfectly for me but I still have a doubt:

CC: Lee Duncan

I think that changing db_root is not allowed if we have a fabric driver loaded
because the latter could have potentially used the core's dbroot
attribute to access the
directory's content.

See the description of the commit that introduced the configurable dbroot:
--------
commit a96e9783e05851d5f06da0ae7635aec55a228e3d
Author: Lee Duncan <lduncan@suse.com>
    target: make target db location configurable

    This commit adds the read-write attribute "dbroot",
    in the top-level CONFIGFS (core) target directory,
    normally /sys/kernel/config/target. This attribute
    defaults to "/var/target" but can be changed by
    writing a new pathname string to it. Changing this
    attribute is only allowed when no fabric drivers
    are loaded and the supplied value specifies an
    existing directory.

    Target modules that care about the target database
    root directory will be modified to use this
    attribute in a future commit.
--------

Maurizio