[RFC PATCH 0/3] SCSI target logs

[RFC PATCH 0/3] SCSI target logs

[Anastasia Kovaleva]

This patch is an initial work on scsi target logging improvement, that
was discussed here:
https://lore.kernel.org/target-devel/ZF0MiCRW8HWm8YYj@yadro.com/
This is an example of how it will look.

Things worth mentioning:

1. I've decided not to implement target_lun_LEVEL() and implement
   target_cmd_LEVEL() instead because the mapped lun is stored in the
   se_cmd structure, and most of the time we already have the cmd when
   we want to log a lun.

2. Target prefix such as "core" or "iscsi" will be defined in the
   beginning of each file.

For example, you can test it with such command:

   sg_inq -f -c --page=255 /dev/sda


And the log output will look like that:

   [ 2229.586394] target core (iqn.1996-04.de.suse:01:6e84cb30fc63 -> 1/0): INQUIRY with EVPD==0 but PAGE CODE=ff


For bio errors log will look like that:

   [ 3354.495867] target iblock (iqn.1996-04.de.suse:01:6e84cb30fc63 -> 1/0): bio error: 0000000097ff0ac0, err: 10

I'll be thankful for your comments and suggestions. It would also be
great if you could tell me whether it would be better to send the entire
series for all modules at onse, or send patches for each module
individually as soon as they are ready.

Kind regards,
Anastasia Kovaleva

Anastasia Kovaleva (3):
  target: core: Initial work on improving SCSI target logs
  target: core: apply the new wrapper to spc
  target: core: apply the new wrapper to iblock

 drivers/target/target_core_iblock.c | 94 ++++++++++++++---------------
 drivers/target/target_core_spc.c    | 27 ++++-----
 include/target/target_core_base.h   | 25 ++++++++
 3 files changed, 82 insertions(+), 64 deletions(-)

-- 
2.40.1

[RFC PATCH 1/3] target: core: Initial work on improving SCSI target logs

[Anastasia Kovaleva]

Introduce the new logging wrapper.

Signed-off-by: Anastasia Kovaleva <a.kovaleva@yadro.com>
---
 include/target/target_core_base.h | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 159567359bbb..ba8c05dc3d54 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -11,6 +11,31 @@
 
 #define TARGET_CORE_VERSION		"v5.0"
 
+/*
+ * Unified target core logs
+ */
+#define target_sess_log(lvl, sess, fmt, ...) pr_##lvl("target " TARGET_PREFIX " (%s -> %d): " fmt, \
+		(sess)->se_node_acl->initiatorname, (sess)->se_tpg->tpg_rtpi, ##__VA_ARGS__)
+#define target_cmd_log(lvl, cmd, fmt, ...) pr_##lvl("target " TARGET_PREFIX " (%s -> %d/%lld): " fmt, \
+		(cmd)->se_sess->se_node_acl->initiatorname, (cmd)->se_lun->lun_tpg->tpg_rtpi, \
+		(cmd)->orig_fe_lun, ##__VA_ARGS__)
+#define target_log(lvl, fmt, ...) pr_##lvl("target " TARGET_PREFIX ": " fmt, ##__VA_ARGS__)
+
+#define target_sess_debug(sess, fmt, ...) target_sess_log(debug, sess, fmt, ##__VA_ARGS__)
+#define target_sess_info(sess, fmt, ...)  target_sess_log(info, sess, fmt, ##__VA_ARGS__)
+#define target_sess_warn(sess, fmt, ...)  target_sess_log(warn, sess, fmt, ##__VA_ARGS__)
+#define target_sess_err(sess, fmt, ...)   target_sess_log(err, sess, fmt, ##__VA_ARGS__)
+
+#define target_cmd_debug(cmd, fmt, ...) target_cmd_log(debug, cmd, fmt, ##__VA_ARGS__)
+#define target_cmd_info(cmd, fmt, ...)  target_cmd_log(info, cmd, fmt, ##__VA_ARGS__)
+#define target_cmd_warn(cmd, fmt, ...)  target_cmd_log(warn, cmd, fmt, ##__VA_ARGS__)
+#define target_cmd_err(cmd, fmt, ...)   target_cmd_log(err, cmd, fmt, ##__VA_ARGS__)
+
+#define target_debug(fmt, ...) target_log(debug, fmt, ##__VA_ARGS__)
+#define target_info(fmt, ...)  target_log(info, fmt, ##__VA_ARGS__)
+#define target_warn(fmt, ...)  target_log(warn, fmt, ##__VA_ARGS__)
+#define target_err(fmt, ...)   target_log(err, fmt, ##__VA_ARGS__)
+
 /*
  * Maximum size of a CDB that can be stored in se_cmd without allocating
  * memory dynamically for the CDB.
-- 
2.40.1

[RFC PATCH 2/3] target: core: apply the new wrapper to spc

[Anastasia Kovaleva]

Signed-off-by: Anastasia Kovaleva <a.kovaleva@yadro.com>
---
 drivers/target/target_core_spc.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index 50290abc07bc..8defcf11cde3 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -25,6 +25,8 @@
 #include "target_core_ua.h"
 #include "target_core_xcopy.h"
 
+#define TARGET_PREFIX "core"
+
 static void spc_fill_alua_data(struct se_lun *lun, unsigned char *buf)
 {
 	struct t10_alua_tg_pt_gp *tg_pt_gp;
@@ -742,7 +744,7 @@ spc_emulate_inquiry(struct se_cmd *cmd)
 
 	buf = kzalloc(SE_INQUIRY_BUF, GFP_KERNEL);
 	if (!buf) {
-		pr_err("Unable to allocate response buffer for INQUIRY\n");
+		target_cmd_err(cmd, "Unable to allocate response buffer for INQUIRY\n");
 		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 	}
 
@@ -750,8 +752,7 @@ spc_emulate_inquiry(struct se_cmd *cmd)
 
 	if (!(cdb[1] & 0x1)) {
 		if (cdb[2]) {
-			pr_err("INQUIRY with EVPD==0 but PAGE CODE=%02x\n",
-			       cdb[2]);
+			target_cmd_err(cmd, "INQUIRY with EVPD==0 but PAGE CODE=%02x\n", cdb[2]);
 			ret = TCM_INVALID_CDB_FIELD;
 			goto out;
 		}
@@ -770,7 +771,7 @@ spc_emulate_inquiry(struct se_cmd *cmd)
 		}
 	}
 
-	pr_debug("Unknown VPD Code: 0x%02x\n", cdb[2]);
+	target_cmd_debug(cmd, "Unknown VPD Code: 0x%02x\n", cdb[2]);
 	ret = TCM_INVALID_CDB_FIELD;
 
 out:
@@ -1085,7 +1086,7 @@ static sense_reason_t spc_emulate_modesense(struct se_cmd *cmd)
 
 	if (page == 0x3f) {
 		if (subpage != 0x00 && subpage != 0xff) {
-			pr_warn("MODE_SENSE: Invalid subpage code: 0x%02x\n", subpage);
+			target_cmd_warn(cmd, "MODE_SENSE: Invalid subpage code: 0x%02x\n", subpage);
 			return TCM_INVALID_CDB_FIELD;
 		}
 
@@ -1119,8 +1120,8 @@ static sense_reason_t spc_emulate_modesense(struct se_cmd *cmd)
 	 *  - obsolete page 03h "format parameters" (checked by Solaris)
 	 */
 	if (page != 0x03)
-		pr_err("MODE SENSE: unimplemented page/subpage: 0x%02x/0x%02x\n",
-		       page, subpage);
+		target_cmd_err(cmd, "MODE SENSE: unimplemented page/subpage: 0x%02x/0x%02x\n",
+				 page, subpage);
 
 	return TCM_UNKNOWN_MODE_PAGE;
 
@@ -1212,8 +1213,7 @@ static sense_reason_t spc_emulate_request_sense(struct se_cmd *cmd)
 	memset(buf, 0, SE_SENSE_BUF);
 
 	if (cdb[1] & 0x01) {
-		pr_err("REQUEST_SENSE description emulation not"
-			" supported\n");
+		target_cmd_err(cmd, "REQUEST_SENSE description emulation not supported\n");
 		return TCM_INVALID_CDB_FIELD;
 	}
 
@@ -2113,7 +2113,6 @@ static sense_reason_t
 spc_rsoc_get_descr(struct se_cmd *cmd, struct target_opcode_descriptor **opcode)
 {
 	struct target_opcode_descriptor *descr;
-	struct se_session *sess = cmd->se_sess;
 	unsigned char *cdb = cmd->t_task_cdb;
 	u8 opts = cdb[2] & 0x3;
 	u8 requested_opcode;
@@ -2125,11 +2124,9 @@ spc_rsoc_get_descr(struct se_cmd *cmd, struct target_opcode_descriptor **opcode)
 	*opcode = NULL;
 
 	if (opts > 3) {
-		pr_debug("TARGET_CORE[%s]: Invalid REPORT SUPPORTED OPERATION CODES"
-			" with unsupported REPORTING OPTIONS %#x for 0x%08llx from %s\n",
-			cmd->se_tfo->fabric_name, opts,
-			cmd->se_lun->unpacked_lun,
-			sess->se_node_acl->initiatorname);
+		target_cmd_debug(cmd,
+				 "Invalid REPORT SUPPORTED OPERATION CODES with unsupported REPORTING OPTIONS %#x for 0x%08llx\n",
+				 opts, cmd->se_lun->unpacked_lun);
 		return TCM_INVALID_CDB_FIELD;
 	}
 
-- 
2.40.1

[RFC PATCH 3/3] target: core: apply the new wrapper to iblock

[Anastasia Kovaleva]

Signed-off-by: Anastasia Kovaleva <a.kovaleva@yadro.com>
---
 drivers/target/target_core_iblock.c | 94 ++++++++++++++---------------
 1 file changed, 45 insertions(+), 49 deletions(-)

diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c
index e6029ea87e2f..e8153d99a83b 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -37,6 +37,8 @@
 #define IBLOCK_MAX_BIO_PER_TASK	 32	/* max # of bios to submit at a time */
 #define IBLOCK_BIO_POOL_SIZE	128
 
+#define TARGET_PREFIX "iblock"
+
 static inline struct iblock_dev *IBLOCK_DEV(struct se_device *dev)
 {
 	return container_of(dev, struct iblock_dev, dev);
@@ -45,9 +47,8 @@ static inline struct iblock_dev *IBLOCK_DEV(struct se_device *dev)
 
 static int iblock_attach_hba(struct se_hba *hba, u32 host_id)
 {
-	pr_debug("CORE_HBA[%d] - TCM iBlock HBA Driver %s on"
-		" Generic Target Core Stack %s\n", hba->hba_id,
-		IBLOCK_VERSION, TARGET_CORE_VERSION);
+	target_debug("CORE_HBA[%d] - TCM iblock HBA Driver %s on Generic Target Core Stack %s\n",
+		     hba->hba_id, IBLOCK_VERSION, TARGET_CORE_VERSION);
 	return 0;
 }
 
@@ -61,7 +62,7 @@ static struct se_device *iblock_alloc_device(struct se_hba *hba, const char *nam
 
 	ib_dev = kzalloc(sizeof(struct iblock_dev), GFP_KERNEL);
 	if (!ib_dev) {
-		pr_err("Unable to allocate struct iblock_dev\n");
+		target_err("Unable to allocate struct iblock_dev\n");
 		return NULL;
 	}
 
@@ -70,7 +71,7 @@ static struct se_device *iblock_alloc_device(struct se_hba *hba, const char *nam
 	if (!ib_dev->ibd_plug)
 		goto free_dev;
 
-	pr_debug( "IBLOCK: Allocated ib_dev for %s\n", name);
+	target_debug("Allocated ib_dev for %s\n", name);
 
 	return &ib_dev->dev;
 
@@ -98,17 +99,17 @@ static int iblock_configure_device(struct se_device *dev)
 	int ret;
 
 	if (!(ib_dev->ibd_flags & IBDF_HAS_UDEV_PATH)) {
-		pr_err("Missing udev_path= parameters for IBLOCK\n");
+		target_err("Missing udev_path= parameters\n");
 		return -EINVAL;
 	}
 
 	ret = bioset_init(&ib_dev->ibd_bio_set, IBLOCK_BIO_POOL_SIZE, 0, BIOSET_NEED_BVECS);
 	if (ret) {
-		pr_err("IBLOCK: Unable to create bioset\n");
+		target_err("Unable to create bioset\n");
 		goto out;
 	}
 
-	pr_debug( "IBLOCK: Claiming struct block_device: %s\n",
+	target_debug("Claiming struct block_device: %s\n",
 			ib_dev->ibd_udev_path);
 
 	mode = FMODE_READ|FMODE_EXCL;
@@ -151,8 +152,8 @@ static int iblock_configure_device(struct se_device *dev)
 
 		if (!strcmp(bi->profile->name, "T10-DIF-TYPE3-IP") ||
 		    !strcmp(bi->profile->name, "T10-DIF-TYPE1-IP")) {
-			pr_err("IBLOCK export of blk_integrity: %s not"
-			       " supported\n", bi->profile->name);
+			target_err("Export of blk_integrity: %s not supported\n",
+				   bi->profile->name);
 			ret = -ENOSYS;
 			goto out_blkdev_put;
 		}
@@ -165,12 +166,12 @@ static int iblock_configure_device(struct se_device *dev)
 
 		if (dev->dev_attrib.pi_prot_type) {
 			if (bioset_integrity_create(bs, IBLOCK_BIO_POOL_SIZE) < 0) {
-				pr_err("Unable to allocate bioset for PI\n");
+				target_err("Unable to allocate bioset for PI\n");
 				ret = -ENOMEM;
 				goto out_blkdev_put;
 			}
-			pr_debug("IBLOCK setup BIP bs->bio_integrity_pool: %p\n",
-				 &bs->bio_integrity_pool);
+			target_debug("IBLOCK setup BIP bs->bio_integrity_pool: %p\n",
+				     &bs->bio_integrity_pool);
 		}
 		dev->dev_attrib.hw_pi_prot_type = dev->dev_attrib.pi_prot_type;
 	}
@@ -339,7 +340,7 @@ static void iblock_bio_done(struct bio *bio)
 	blk_status_t blk_status = bio->bi_status;
 
 	if (bio->bi_status) {
-		pr_err("bio error: %p,  err: %d\n", bio, bio->bi_status);
+		target_cmd_err(cmd, "bio error: %p,  err: %d\n", bio, bio->bi_status);
 		/*
 		 * Bump the ib_bio_err_cnt and release bio.
 		 */
@@ -365,7 +366,7 @@ static struct bio *iblock_get_bio(struct se_cmd *cmd, sector_t lba, u32 sg_num,
 	bio = bio_alloc_bioset(ib_dev->ibd_bd, bio_max_segs(sg_num), opf,
 			       GFP_NOIO, &ib_dev->ibd_bio_set);
 	if (!bio) {
-		pr_err("Unable to allocate memory for bio\n");
+		target_cmd_err(cmd, "Unable to allocate memory for bio\n");
 		return NULL;
 	}
 
@@ -395,7 +396,7 @@ static void iblock_end_io_flush(struct bio *bio)
 	struct se_cmd *cmd = bio->bi_private;
 
 	if (bio->bi_status)
-		pr_err("IBLOCK: cache flush failed: %d\n", bio->bi_status);
+		target_cmd_err(cmd, "Cache flush failed: %d\n", bio->bi_status);
 
 	if (cmd) {
 		if (bio->bi_status)
@@ -446,7 +447,7 @@ iblock_execute_unmap(struct se_cmd *cmd, sector_t lba, sector_t nolb)
 				   target_to_linux_sector(dev,  nolb),
 				   GFP_KERNEL);
 	if (ret < 0) {
-		pr_err("blkdev_issue_discard() failed: %d\n", ret);
+		target_cmd_err(cmd, "blkdev_issue_discard() failed: %d\n", ret);
 		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 	}
 
@@ -500,8 +501,7 @@ iblock_execute_write_same(struct se_cmd *cmd)
 					sbc_get_write_same_sectors(cmd));
 
 	if (cmd->prot_op) {
-		pr_err("WRITE_SAME: Protection information with IBLOCK"
-		       " backends not supported\n");
+		target_cmd_err(cmd, "WRITE_SAME: Protection information with IBLOCK backends not supported\n");
 		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 	}
 
@@ -512,9 +512,8 @@ iblock_execute_write_same(struct se_cmd *cmd)
 
 	if (cmd->t_data_nents > 1 ||
 	    sg->length != cmd->se_dev->dev_attrib.block_size) {
-		pr_err("WRITE_SAME: Illegal SGL t_data_nents: %u length: %u"
-			" block_size: %u\n", cmd->t_data_nents, sg->length,
-			cmd->se_dev->dev_attrib.block_size);
+		target_cmd_err(cmd, "WRITE_SAME: Illegal SGL t_data_nents: %u length: %u block_size: %u\n",
+			       cmd->t_data_nents, sg->length, cmd->se_dev->dev_attrib.block_size);
 		return TCM_INVALID_CDB_FIELD;
 	}
 
@@ -600,8 +599,7 @@ static ssize_t iblock_set_configfs_dev_params(struct se_device *dev,
 		switch (token) {
 		case Opt_udev_path:
 			if (ib_dev->ibd_bd) {
-				pr_err("Unable to set udev_path= while"
-					" ib_dev->ibd_bd exists\n");
+				target_err("Unable to set udev_path= while ib_dev->ibd_bd exists\n");
 				ret = -EEXIST;
 				goto out;
 			}
@@ -610,8 +608,7 @@ static ssize_t iblock_set_configfs_dev_params(struct se_device *dev,
 				ret = -EINVAL;
 				break;
 			}
-			pr_debug("IBLOCK: Referencing UDEV path: %s\n",
-					ib_dev->ibd_udev_path);
+			target_debug("Referencing UDEV path: %s\n", ib_dev->ibd_udev_path);
 			ib_dev->ibd_flags |= IBDF_HAS_UDEV_PATH;
 			break;
 		case Opt_readonly:
@@ -623,12 +620,11 @@ static ssize_t iblock_set_configfs_dev_params(struct se_device *dev,
 			ret = kstrtoul(arg_p, 0, &tmp_readonly);
 			kfree(arg_p);
 			if (ret < 0) {
-				pr_err("kstrtoul() failed for"
-						" readonly=\n");
+				target_err("kstrtoul() failed for readonly=\n");
 				goto out;
 			}
 			ib_dev->ibd_readonly = tmp_readonly;
-			pr_debug("IBLOCK: readonly: %d\n", ib_dev->ibd_readonly);
+			target_debug("readonly: %d\n", ib_dev->ibd_readonly);
 			break;
 		case Opt_force:
 			break;
@@ -680,13 +676,13 @@ iblock_alloc_bip(struct se_cmd *cmd, struct bio *bio,
 
 	bi = bdev_get_integrity(ib_dev->ibd_bd);
 	if (!bi) {
-		pr_err("Unable to locate bio_integrity\n");
+		target_cmd_err(cmd, "Unable to locate bio_integrity\n");
 		return -ENODEV;
 	}
 
 	bip = bio_integrity_alloc(bio, GFP_NOIO, bio_max_segs(cmd->t_prot_nents));
 	if (IS_ERR(bip)) {
-		pr_err("Unable to allocate bio_integrity_payload\n");
+		target_cmd_err(cmd, "Unable to allocate bio_integrity_payload\n");
 		return PTR_ERR(bip);
 	}
 
@@ -695,8 +691,8 @@ iblock_alloc_bip(struct se_cmd *cmd, struct bio *bio,
 	bip_set_seed(bip, bio->bi_iter.bi_sector >>
 				  (bi->interval_exp - SECTOR_SHIFT));
 
-	pr_debug("IBLOCK BIP Size: %u Sector: %llu\n", bip->bip_iter.bi_size,
-		 (unsigned long long)bip->bip_iter.bi_sector);
+	target_cmd_debug(cmd, "BIP Size: %u Sector: %llu\n", bip->bip_iter.bi_size,
+			 (unsigned long long)bip->bip_iter.bi_sector);
 
 	resid = bip->bip_iter.bi_size;
 	while (resid > 0 && sg_miter_next(miter)) {
@@ -705,13 +701,13 @@ iblock_alloc_bip(struct se_cmd *cmd, struct bio *bio,
 		rc = bio_integrity_add_page(bio, miter->page, len,
 					    offset_in_page(miter->addr));
 		if (rc != len) {
-			pr_err("bio_integrity_add_page() failed; %d\n", rc);
+			target_cmd_err(cmd, "bio_integrity_add_page() failed; %d\n", rc);
 			sg_miter_stop(miter);
 			return -ENOMEM;
 		}
 
-		pr_debug("Added bio integrity page: %p length: %zu offset: %lu\n",
-			  miter->page, len, offset_in_page(miter->addr));
+		target_cmd_debug(cmd, "Added bio integrity page: %p length: %zu offset: %lu\n",
+				 miter->page, len, offset_in_page(miter->addr));
 
 		resid -= len;
 		if (len < miter->length)
@@ -845,7 +841,7 @@ static sense_reason_t iblock_execute_pr_out(struct se_cmd *cmd, u8 sa, u64 key,
 	int ret;
 
 	if (!ops) {
-		pr_err("Block device does not support pr_ops but iblock device has been configured for PR passthrough.\n");
+		target_cmd_err(cmd, "Block device does not support pr_ops but iblock device has been configured for PR passthrough.\n");
 		return TCM_UNSUPPORTED_SCSI_OPCODE;
 	}
 
@@ -853,20 +849,20 @@ static sense_reason_t iblock_execute_pr_out(struct se_cmd *cmd, u8 sa, u64 key,
 	case PRO_REGISTER:
 	case PRO_REGISTER_AND_IGNORE_EXISTING_KEY:
 		if (!ops->pr_register) {
-			pr_err("block device does not support pr_register.\n");
+			target_cmd_err(cmd, "block device does not support pr_register.\n");
 			return TCM_UNSUPPORTED_SCSI_OPCODE;
 		}
 
 		/* The block layer pr ops always enables aptpl */
 		if (!aptpl)
-			pr_info("APTPL not set by initiator, but will be used.\n");
+			target_cmd_info(cmd, "APTPL not set by initiator, but will be used.\n");
 
 		ret = ops->pr_register(bdev, key, sa_key,
 				sa == PRO_REGISTER ? 0 : PR_FL_IGNORE_KEY);
 		break;
 	case PRO_RESERVE:
 		if (!ops->pr_reserve) {
-			pr_err("block_device does not support pr_reserve.\n");
+			target_cmd_err(cmd, "block_device does not support pr_reserve.\n");
 			return TCM_UNSUPPORTED_SCSI_OPCODE;
 		}
 
@@ -874,7 +870,7 @@ static sense_reason_t iblock_execute_pr_out(struct se_cmd *cmd, u8 sa, u64 key,
 		break;
 	case PRO_CLEAR:
 		if (!ops->pr_clear) {
-			pr_err("block_device does not support pr_clear.\n");
+			target_cmd_err(cmd, "block_device does not support pr_clear.\n");
 			return TCM_UNSUPPORTED_SCSI_OPCODE;
 		}
 
@@ -883,7 +879,7 @@ static sense_reason_t iblock_execute_pr_out(struct se_cmd *cmd, u8 sa, u64 key,
 	case PRO_PREEMPT:
 	case PRO_PREEMPT_AND_ABORT:
 		if (!ops->pr_clear) {
-			pr_err("block_device does not support pr_preempt.\n");
+			target_cmd_err(cmd, "block_device does not support pr_preempt.\n");
 			return TCM_UNSUPPORTED_SCSI_OPCODE;
 		}
 
@@ -893,14 +889,14 @@ static sense_reason_t iblock_execute_pr_out(struct se_cmd *cmd, u8 sa, u64 key,
 		break;
 	case PRO_RELEASE:
 		if (!ops->pr_clear) {
-			pr_err("block_device does not support pr_pclear.\n");
+			target_cmd_err(cmd, "block_device does not support pr_pclear.\n");
 			return TCM_UNSUPPORTED_SCSI_OPCODE;
 		}
 
 		ret = ops->pr_release(bdev, key, scsi_pr_type_to_block(type));
 		break;
 	default:
-		pr_err("Unknown PERSISTENT_RESERVE_OUT SA: 0x%02x\n", sa);
+		target_cmd_err(cmd, "Unknown PERSISTENT_RESERVE_OUT SA: 0x%02x\n", sa);
 		return TCM_UNSUPPORTED_SCSI_OPCODE;
 	}
 
@@ -966,12 +962,12 @@ static sense_reason_t iblock_pr_read_keys(struct se_cmd *cmd,
 	sense_reason_t ret;
 
 	if (!ops) {
-		pr_err("Block device does not support pr_ops but iblock device has been configured for PR passthrough.\n");
+		target_cmd_err(cmd, "Block device does not support pr_ops but iblock device has been configured for PR passthrough.\n");
 		return TCM_UNSUPPORTED_SCSI_OPCODE;
 	}
 
 	if (!ops->pr_read_keys) {
-		pr_err("Block device does not support read_keys.\n");
+		target_cmd_err(cmd, "Block device does not support read_keys.\n");
 		return TCM_UNSUPPORTED_SCSI_OPCODE;
 	}
 
@@ -1033,12 +1029,12 @@ static sense_reason_t iblock_pr_read_reservation(struct se_cmd *cmd,
 	struct pr_held_reservation rsv = { };
 
 	if (!ops) {
-		pr_err("Block device does not support pr_ops but iblock device has been configured for PR passthrough.\n");
+		target_cmd_err(cmd, "Block device does not support pr_ops but iblock device has been configured for PR passthrough.\n");
 		return TCM_UNSUPPORTED_SCSI_OPCODE;
 	}
 
 	if (!ops->pr_read_reservation) {
-		pr_err("Block device does not support read_keys.\n");
+		target_cmd_err(cmd, "Block device does not support read_keys.\n");
 		return TCM_UNSUPPORTED_SCSI_OPCODE;
 	}
 
@@ -1080,7 +1076,7 @@ static sense_reason_t iblock_execute_pr_in(struct se_cmd *cmd, u8 sa,
 		ret = iblock_pr_read_reservation(cmd, param_data);
 		break;
 	default:
-		pr_err("Unknown PERSISTENT_RESERVE_IN SA: 0x%02x\n", sa);
+		target_cmd_err(cmd, "Unknown PERSISTENT_RESERVE_IN SA: 0x%02x\n", sa);
 		return TCM_UNSUPPORTED_SCSI_OPCODE;
 	}
 
-- 
2.40.1

Re: [RFC PATCH 2/3] target: core: apply the new wrapper to spc

[Bart Van Assche]

On 7/26/23 04:55, Anastasia Kovaleva wrote:
> +#define TARGET_PREFIX "core"

I'm not sure this is a good choice for a logging prefix since this name 
does not make it clear that log lines come from the SCSI target core. 
How about "scsi_tgt" as prefix? "stgt" is probably not a good choice 
since this is the former name of a user-space SCSI target project 
(https://github.com/fujita/tgt).

Bart.

Re: [DMARC Error]Re: [RFC PATCH 2/3] target: core: apply the new wrapper to spc

[Konstantin Shelekhin]

On Wed Jul 26, 2023 at 6:15 PM MSK, Bart Van Assche wrote:
> «Внимание! Данное письмо от внешнего адресата!»
>
> On 7/26/23 04:55, Anastasia Kovaleva wrote:
> > +#define TARGET_PREFIX "core"
>
> I'm not sure this is a good choice for a logging prefix since this name
> does not make it clear that log lines come from the SCSI target core.
> How about "scsi_tgt" as prefix? "stgt" is probably not a good choice
> since this is the former name of a user-space SCSI target project
> (https://github.com/fujita/tgt).
>
> Bart.

I think scsit is a good choice, like nvmet we already have.