| Message ID | 20210927193814.79111-1-mlombard@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | target: allow setting dbroot as a module parameter | expand |
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 ....
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
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;
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(-)