On 10.09.21 10:41, Dmitry Bogdanov wrote:
Many fabric modules provide their own implementation of enable
attribute in tpg.
The change provides a way to remove code duplication in the fabric
modules and automatically add "enable" attribute if a fabric module has
an implementation of fabric_enable_tpg() ops.
Reviewed-by: Roman Bolshakov <r.bolshakov@xxxxxxxxx>
Signed-off-by: Dmitry Bogdanov <d.bogdanov@xxxxxxxxx>
---
drivers/target/target_core_configfs.c | 1 +
drivers/target/target_core_fabric_configfs.c | 78 +++++++++++++++++++-
include/target/target_core_base.h | 1 +
include/target/target_core_fabric.h | 1 +
4 files changed, 79 insertions(+), 2 deletions(-)
diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index 102ec644bc8a..3b9e50c1ccef 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -490,6 +490,7 @@ void target_unregister_template(const struct target_core_fabric_ops *fo)
* fabric driver unload of TFO->module to proceed.
*/
rcu_barrier();
+ kfree(t->tf_tpg_base_cit.ct_attrs);
kfree(t);
return;
}
diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c
index fc7edc04ee09..0b65de9f2df1 100644
--- a/drivers/target/target_core_fabric_configfs.c
+++ b/drivers/target/target_core_fabric_configfs.c
@@ -815,8 +815,76 @@ static struct configfs_item_operations target_fabric_tpg_base_item_ops = {
.release = target_fabric_tpg_release,
};
-TF_CIT_SETUP_DRV(tpg_base, &target_fabric_tpg_base_item_ops, NULL);
+static ssize_t target_fabric_tpg_base_enable_show(struct config_item *item,
+ char *page)
+{
+ return sysfs_emit(page, "%d\n", to_tpg(item)->enabled);
+}
+
+static ssize_t target_fabric_tpg_base_enable_store(struct config_item *item,
+ const char *page,
+ size_t count)
+{
+ struct se_portal_group *se_tpg = to_tpg(item);
+ int ret;
+ bool op;
+
+ ret = strtobool(page, &op);
+ if (ret)
+ return ret;
+
+ if (se_tpg->enabled == op)
+ return count;
Sorry for jumping in lately.
Just one nit:
In case someone tries to enable or disable the same tpg a second time,
with the change we always do nothing and return count (--> OK).
I just checked iscsi and qla2xxx. AFAICS iscsi before the patch rejected
the second enable or disable with -EINVAL, while qla2xxx accepts the
second disable and rejects the second enable with -EEXIST.
Of course it sounds good to unify the behavior of existing enable
attributes. OTOH: even if enabling/disabling the same tpg twice can be
seen as suspicious behavior, are we sure to not confuse existing user
space tools by changing the result?
Bodo
