Re: [PATCH v5 1/7] target: core: add common tpg/enable attribute

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux SCSI]     [Kernel Newbies]     [Linux SCSI Target Infrastructure]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Device Mapper]

  Powered by Linux