[PATCH v3 3/4] target: add device vendor_id configfs attribute

[PATCH v3 3/4] target: add device vendor_id configfs attribute

[David Disseldorp]

The vendor_id attribute will allow for the modification of the T10
Vendor Identification string returned in inquiry responses. Its value
can be viewed and modified via the ConfigFS path at:
target/core/$backstore/$name/wwn/vendor_id

"LIO-ORG" remains the default value, which is set when the backstore
device is enabled.

Signed-off-by: David Disseldorp <ddiss@suse.de>
---
 drivers/target/target_core_configfs.c | 55 +++++++++++++++++++++++++++++++++++
 include/target/target_core_base.h     |  3 +-
 2 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index f6b1549f4142..64e95376d998 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -1211,6 +1211,59 @@ static struct t10_wwn *to_t10_wwn(struct config_item *item)
 }
 
 /*
+ * STANDARD and VPD page 0x80 T10 Vendor Identification
+ */
+static ssize_t target_wwn_vendor_id_show(struct config_item *item,
+		char *page)
+{
+	return sprintf(page, "T10 Vendor Identification: %."
+		       __stringify(INQUIRY_VENDOR_IDENTIFIER_LEN) "s\n",
+		       &to_t10_wwn(item)->vendor[0]);
+}
+
+static ssize_t target_wwn_vendor_id_store(struct config_item *item,
+		const char *page, size_t count)
+{
+	struct t10_wwn *t10_wwn = to_t10_wwn(item);
+	struct se_device *dev = t10_wwn->t10_dev;
+	/* +1 to ensure buf is zero terminated for stripping */
+	unsigned char buf[INQUIRY_VENDOR_IDENTIFIER_LEN + 1];
+
+	if (strlen(page) > INQUIRY_VENDOR_IDENTIFIER_LEN) {
+		pr_err("Emulated T10 Vendor Identification exceeds"
+			" INQUIRY_VENDOR_IDENTIFIER_LEN: %d\n",
+			INQUIRY_VENDOR_IDENTIFIER_LEN);
+		return -EOVERFLOW;
+	}
+	strncpy(buf, page, sizeof(buf));
+	/*
+	 * Check to see if any active $FABRIC_MOD exports exist.  If they
+	 * do exist, fail here as changing this information on the fly
+	 * (underneath the initiator side OS dependent multipath code)
+	 * could cause negative effects.
+	 */
+	if (dev->export_count) {
+		pr_err("Unable to set T10 Vendor Identification while"
+			" active %d $FABRIC_MOD exports exist\n",
+			dev->export_count);
+		return -EINVAL;
+	}
+
+	/*
+	 * Assume ASCII encoding. Strip any newline added from userspace.
+	 * The result may *not* be null terminated.
+	 */
+	strncpy(dev->t10_wwn.vendor, strstrip(buf),
+		INQUIRY_VENDOR_IDENTIFIER_LEN);
+
+	pr_debug("Target_Core_ConfigFS: Set emulated T10 Vendor Identification:"
+		 " %." __stringify(INQUIRY_VENDOR_IDENTIFIER_LEN) "s\n",
+		 dev->t10_wwn.vendor);
+
+	return count;
+}
+
+/*
  * VPD page 0x80 Unit serial
  */
 static ssize_t target_wwn_vpd_unit_serial_show(struct config_item *item,
@@ -1356,6 +1409,7 @@ DEF_DEV_WWN_ASSOC_SHOW(vpd_assoc_target_port, 0x10);
 /* VPD page 0x83 Association: SCSI Target Device */
 DEF_DEV_WWN_ASSOC_SHOW(vpd_assoc_scsi_target_device, 0x20);
 
+CONFIGFS_ATTR(target_wwn_, vendor_id);
 CONFIGFS_ATTR(target_wwn_, vpd_unit_serial);
 CONFIGFS_ATTR_RO(target_wwn_, vpd_protocol_identifier);
 CONFIGFS_ATTR_RO(target_wwn_, vpd_assoc_logical_unit);
@@ -1363,6 +1417,7 @@ CONFIGFS_ATTR_RO(target_wwn_, vpd_assoc_target_port);
 CONFIGFS_ATTR_RO(target_wwn_, vpd_assoc_scsi_target_device);
 
 static struct configfs_attribute *target_core_dev_wwn_attrs[] = {
+	&target_wwn_attr_vendor_id,
 	&target_wwn_attr_vpd_unit_serial,
 	&target_wwn_attr_vpd_protocol_identifier,
 	&target_wwn_attr_vpd_assoc_logical_unit,
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index e3bdb0550a59..45be5427326d 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -45,6 +45,7 @@
 #define INQUIRY_VPD_SERIAL_LEN			254
 /* Used by transport_get_inquiry_vpd_device_ident() */
 #define INQUIRY_VPD_DEVICE_IDENTIFIER_LEN	254
+#define INQUIRY_VENDOR_IDENTIFIER_LEN		8
 
 /* Attempts before moving from SHORT to LONG */
 #define PYX_TRANSPORT_WINDOW_CLOSED_THRESHOLD	3
@@ -314,7 +315,7 @@ struct t10_vpd {
 };
 
 struct t10_wwn {
-	char vendor[8];
+	char vendor[INQUIRY_VENDOR_IDENTIFIER_LEN];
 	char model[16];
 	char revision[4];
 	char unit_serial[INQUIRY_VPD_SERIAL_LEN];
-- 
2.13.7

Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

[Bart Van Assche]

On Mon, 2018-11-19 at 22:06 +-0100, David Disseldorp wrote:
+AD4  /+ACo
+AD4 +- +ACo STANDARD and VPD page 0x80 T10 Vendor Identification
+AD4 +- +ACo-/
+AD4 +-static ssize+AF8-t target+AF8-wwn+AF8-vendor+AF8-id+AF8-show(struct config+AF8-item +ACo-item,
+AD4 +-		char +ACo-page)
+AD4 +-+AHs
+AD4 +-	return sprintf(page, +ACI-T10 Vendor Identification: +ACU.+ACI
+AD4 +-		       +AF8AXw-stringify(INQUIRY+AF8-VENDOR+AF8-IDENTIFIER+AF8-LEN) +ACI-s+AFw-n+ACI,
+AD4 +-		       +ACY-to+AF8-t10+AF8-wwn(item)-+AD4-vendor+AFs-0+AF0)+ADs
+AD4 +-+AH0

This doesn't follow the convention used by other configfs attributes,
namely that only the value should be reported and no prefix. Please leave
out the +ACI-T10 Vendor Identification: +ACI prefix.

+AD4 +-static ssize+AF8-t target+AF8-wwn+AF8-vendor+AF8-id+AF8-store(struct config+AF8-item +ACo-item,
+AD4 +-		const char +ACo-page, size+AF8-t count)
+AD4 +-+AHs
+AD4 +-	struct t10+AF8-wwn +ACo-t10+AF8-wwn +AD0 to+AF8-t10+AF8-wwn(item)+ADs
+AD4 +-	struct se+AF8-device +ACo-dev +AD0 t10+AF8-wwn-+AD4-t10+AF8-dev+ADs
+AD4 +-	/+ACo +-1 to ensure buf is zero terminated for stripping +ACo-/
+AD4 +-	unsigned char buf+AFs-INQUIRY+AF8-VENDOR+AF8-IDENTIFIER+AF8-LEN +- 1+AF0AOw
+AD4 +-
+AD4 +-	if (strlen(page) +AD4 INQUIRY+AF8-VENDOR+AF8-IDENTIFIER+AF8-LEN) +AHs
+AD4 +-		pr+AF8-err(+ACI-Emulated T10 Vendor Identification exceeds+ACI
+AD4 +-			+ACI INQUIRY+AF8-VENDOR+AF8-IDENTIFIER+AF8-LEN: +ACU-d+AFw-n+ACI,
+AD4 +-			INQUIRY+AF8-VENDOR+AF8-IDENTIFIER+AF8-LEN)+ADs
+AD4 +-		return -EOVERFLOW+ADs
+AD4 +-	+AH0

Trailing newline(s) should be stripped before the length check is performed. I
don't think that you want to force users to use +ACI-echo -n+ACI instead of +ACI-echo+ACI when
setting this attribute.

+AD4 +-	strncpy(buf, page, sizeof(buf))+ADs

Isn't strncpy() deprecated? How about using strlcpy() instead?

+AD4 +-	/+ACo
+AD4 +-	 +ACo Check to see if any active +ACQ-FABRIC+AF8-MOD exports exist.  If they
+AD4 +-	 +ACo do exist, fail here as changing this information on the fly
+AD4 +-	 +ACo (underneath the initiator side OS dependent multipath code)
+AD4 +-	 +ACo could cause negative effects.
+AD4 +-	 +ACo-/
+AD4 +-	if (dev-+AD4-export+AF8-count) +AHs
+AD4 +-		pr+AF8-err(+ACI-Unable to set T10 Vendor Identification while+ACI
+AD4 +-			+ACI active +ACU-d +ACQ-FABRIC+AF8-MOD exports exist+AFw-n+ACI,
+AD4 +-			dev-+AD4-export+AF8-count)+ADs
+AD4 +-		return -EINVAL+ADs
+AD4 +-	+AH0

Are there any users who understand what +ACIAJA-FABRIC+AF8-MOD+ACI means? Please leave out that
string or change it into the name of the fabric driver followed by the name of the
target port associated with 'item'.

+AD4 +-
+AD4 +-	/+ACo
+AD4 +-	 +ACo Assume ASCII encoding. Strip any newline added from userspace.
+AD4 +-	 +ACo The result may +ACo-not+ACo be null terminated.
+AD4 +-	 +ACo-/
+AD4 +-	strncpy(dev-+AD4-t10+AF8-wwn.vendor, strstrip(buf),
+AD4 +-		INQUIRY+AF8-VENDOR+AF8-IDENTIFIER+AF8-LEN)+ADs

Keeping strings around that are not '+AFw-0'-terminated is a booby trap. It is very
easy for anyone who modifies or reviews code that uses such strings to overlook
that the string is not '+AFw-0'-terminated. Please increase the size of the vendor+AFsAXQ
array by one and make sure that that string is '+AFw-0'-terminated.

Thanks,

Bart.

Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

[David Disseldorp]

On Tue, 20 Nov 2018 09:24:39 -0800, Bart Van Assche wrote:

> On Mon, 2018-11-19 at 22:06 +0100, David Disseldorp wrote:
> >  /*
> > + * STANDARD and VPD page 0x80 T10 Vendor Identification
> > + */
> > +static ssize_t target_wwn_vendor_id_show(struct config_item *item,
> > +		char *page)
> > +{
> > +	return sprintf(page, "T10 Vendor Identification: %."
> > +		       __stringify(INQUIRY_VENDOR_IDENTIFIER_LEN) "s\n",
> > +		       &to_t10_wwn(item)->vendor[0]);
> > +}  
> 
> This doesn't follow the convention used by other configfs attributes,
> namely that only the value should be reported and no prefix. Please leave
> out the "T10 Vendor Identification: " prefix.

I based this on the convention used with
target_wwn_vpd_unit_serial_show(). I'm happy to drop the prefix if you
prefer.

> > +static ssize_t target_wwn_vendor_id_store(struct config_item *item,
> > +		const char *page, size_t count)
> > +{
> > +	struct t10_wwn *t10_wwn = to_t10_wwn(item);
> > +	struct se_device *dev = t10_wwn->t10_dev;
> > +	/* +1 to ensure buf is zero terminated for stripping */
> > +	unsigned char buf[INQUIRY_VENDOR_IDENTIFIER_LEN + 1];
> > +
> > +	if (strlen(page) > INQUIRY_VENDOR_IDENTIFIER_LEN) {
> > +		pr_err("Emulated T10 Vendor Identification exceeds"
> > +			" INQUIRY_VENDOR_IDENTIFIER_LEN: %d\n",
> > +			INQUIRY_VENDOR_IDENTIFIER_LEN);
> > +		return -EOVERFLOW;
> > +	}  
> 
> Trailing newline(s) should be stripped before the length check is performed. I
> don't think that you want to force users to use "echo -n" instead of "echo" when
> setting this attribute.

This is also a target_wwn_vpd_unit_serial_store() carryover, which
checks the length prior to the strip. Doing so makes buffer length
a little easier to determine.

> > +	strncpy(buf, page, sizeof(buf));  
> 
> Isn't strncpy() deprecated? How about using strlcpy() instead?

Will change to use strlcpy in the next round.

> > +	/*
> > +	 * Check to see if any active $FABRIC_MOD exports exist.  If they
> > +	 * do exist, fail here as changing this information on the fly
> > +	 * (underneath the initiator side OS dependent multipath code)
> > +	 * could cause negative effects.
> > +	 */
> > +	if (dev->export_count) {
> > +		pr_err("Unable to set T10 Vendor Identification while"
> > +			" active %d $FABRIC_MOD exports exist\n",
> > +			dev->export_count);
> > +		return -EINVAL;
> > +	}  
> 
> Are there any users who understand what "$FABRIC_MOD" means? Please leave out that
> string or change it into the name of the fabric driver followed by the name of the
> target port associated with 'item'.

Another target_wwn_vpd_unit_serial_store() carryover. Will drop the
string from the next round.

> > +
> > +	/*
> > +	 * Assume ASCII encoding. Strip any newline added from userspace.
> > +	 * The result may *not* be null terminated.
> > +	 */
> > +	strncpy(dev->t10_wwn.vendor, strstrip(buf),
> > +		INQUIRY_VENDOR_IDENTIFIER_LEN);  
> 
> Keeping strings around that are not '\0'-terminated is a booby trap. It is very
> easy for anyone who modifies or reviews code that uses such strings to overlook
> that the string is not '\0'-terminated. Please increase the size of the vendor[]
> array by one and make sure that that string is '\0'-terminated.

I tend to agree that it's dangerous, but chose to stay somewhat
consistent with the other t10_wwn strings that are treated as though
they may not be NULL terminated.

If you're in favour adding an extra terminator byte here, then I think
it'd make sense to do the same for model[], revision[] and unit_serial[]
too. Are you okay with that approach?

Cheers, David

Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

[Bart Van Assche]

On Tue, 2018-11-20 at 19:00 +-0100, David Disseldorp wrote:
+AD4 I tend to agree that it's dangerous, but chose to stay somewhat
+AD4 consistent with the other t10+AF8-wwn strings that are treated as though
+AD4 they may not be NULL terminated.
+AD4 
+AD4 If you're in favour adding an extra terminator byte here, then I think
+AD4 it'd make sense to do the same for model+AFsAXQ, revision+AFsAXQ and unit+AF8-serial+AFsAXQ
+AD4 too. Are you okay with that approach?

Sure, I would welcome that change.

Thanks,

Bart.

Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

[David Disseldorp]

On Tue, 20 Nov 2018 19:00:57 +0100, David Disseldorp wrote:

> > > +	strncpy(buf, page, sizeof(buf));    
> > 
> > Isn't strncpy() deprecated? How about using strlcpy() instead?  
> 
> Will change to use strlcpy in the next round.

Just a follow up here. I think it's safer to retain strncpy() in this
function for the purpose of zero filling. scsi_dump_inquiry() and
target_stat_lu_vend_show() walk the entire length of the t10_wwn.vendor
buffer.

Cheers, David

Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

[Bart Van Assche]

On Wed, 2018-11-28 at 16:37 +-0100, David Disseldorp wrote:
+AD4 On Tue, 20 Nov 2018 19:00:57 +-0100, David Disseldorp wrote:
+AD4 
+AD4 +AD4 +AD4 +AD4 +-	strncpy(buf, page, sizeof(buf))+ADs    
+AD4 +AD4 +AD4 
+AD4 +AD4 +AD4 Isn't strncpy() deprecated? How about using strlcpy() instead?  
+AD4 +AD4 
+AD4 +AD4 Will change to use strlcpy in the next round.
+AD4 
+AD4 Just a follow up here. I think it's safer to retain strncpy() in this
+AD4 function for the purpose of zero filling. scsi+AF8-dump+AF8-inquiry() and
+AD4 target+AF8-stat+AF8-lu+AF8-vend+AF8-show() walk the entire length of the t10+AF8-wwn.vendor
+AD4 buffer.

How about adding a comment about above struct t10+AF8-wwn that vendor+AFsAXQ, model+AFsAXQ
and revision+AFsAXQ in that structure may but do not have to be terminated with a
trailing '+AFw-0' and also that unit+AF8-serial+AFsAXQ is '+AFw-0'-terminated?

It would make me happy if the size of the character arrays in that structure
would be increased by one and if the target code would be modified such that
these strings are always '+AFw-0'-terminated.

Thanks,

Bart.

Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

[David Disseldorp]

On Wed, 28 Nov 2018 08:08:30 -0800, Bart Van Assche wrote:

> > Just a follow up here. I think it's safer to retain strncpy() in this
> > function for the purpose of zero filling. scsi_dump_inquiry() and
> > target_stat_lu_vend_show() walk the entire length of the t10_wwn.vendor
> > buffer.  
> 
> How about adding a comment about above struct t10_wwn that vendor[], model[]
> and revision[] in that structure may but do not have to be terminated with a
> trailing '\0' and also that unit_serial[] is '\0'-terminated?
> 
> It would make me happy if the size of the character arrays in that structure
> would be increased by one and if the target code would be modified such that
> these strings are always '\0'-terminated.

I'm working on the +1 length increase for those t10_wwn members ATM, but
just thought I'd mention that the zeroing of unused bytes is still
required due to the scsi_dump_inquiry() + target_stat_lu_vend_show()
behaviour.

Cheers, David

Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

[Bart Van Assche]

On Wed, 2018-11-28 at 17:28 +-0100, David Disseldorp wrote:
+AD4 On Wed, 28 Nov 2018 08:08:30 -0800, Bart Van Assche wrote:
+AD4 
+AD4 +AD4 +AD4 Just a follow up here. I think it's safer to retain strncpy() in this
+AD4 +AD4 +AD4 function for the purpose of zero filling. scsi+AF8-dump+AF8-inquiry() and
+AD4 +AD4 +AD4 target+AF8-stat+AF8-lu+AF8-vend+AF8-show() walk the entire length of the t10+AF8-wwn.vendor
+AD4 +AD4 +AD4 buffer.  
+AD4 +AD4 
+AD4 +AD4 How about adding a comment about above struct t10+AF8-wwn that vendor+AFsAXQ, model+AFsAXQ
+AD4 +AD4 and revision+AFsAXQ in that structure may but do not have to be terminated with a
+AD4 +AD4 trailing '+AFw-0' and also that unit+AF8-serial+AFsAXQ is '+AFw-0'-terminated?
+AD4 +AD4 
+AD4 +AD4 It would make me happy if the size of the character arrays in that structure
+AD4 +AD4 would be increased by one and if the target code would be modified such that
+AD4 +AD4 these strings are always '+AFw-0'-terminated.
+AD4 
+AD4 I'm working on the +-1 length increase for those t10+AF8-wwn members ATM, but
+AD4 just thought I'd mention that the zeroing of unused bytes is still
+AD4 required due to the scsi+AF8-dump+AF8-inquiry() +- target+AF8-stat+AF8-lu+AF8-vend+AF8-show()
+AD4 behaviour.

Hi David,

Maybe I'm missing something, but why is zeroing of unused bytes in these functions
necessary? Would the following be correct if all strings in struct t10+AF8-wwn would be
'+AFw-0'-terminated?

 static ssize+AF8-t target+AF8-stat+AF8-lu+AF8-vend+AF8-show(struct config+AF8-item +ACo-item, char +ACo-page)
 +AHs
 	struct se+AF8-device +ACo-dev +AD0 to+AF8-stat+AF8-lu+AF8-dev(item)+ADs
-	int i+ADs
-	char str+AFs-sizeof(dev-+AD4-t10+AF8-wwn.vendor)+-1+AF0AOw
 
-	/+ACo scsiLuVendorId +ACo-/
-	for (i +AD0 0+ADs i +ADw sizeof(dev-+AD4-t10+AF8-wwn.vendor)+ADs i+-+-)
-		str+AFs-i+AF0 +AD0 ISPRINT(dev-+AD4-t10+AF8-wwn.vendor+AFs-i+AF0) ?
-			dev-+AD4-t10+AF8-wwn.vendor+AFs-i+AF0 : ' '+ADs
-	str+AFs-i+AF0 +AD0 '+AFw-0'+ADs
-	return snprintf(page, PAGE+AF8-SIZE, +ACIAJQ-s+AFw-n+ACI, str)+ADs
+-	return snprintf(page, PAGE+AF8-SIZE, +ACIAJQ--16s+AFw-n+ACI, dev-+AD4-t10+AF8-wwn.vendor)+ADs
 +AH0
+AFs ... +AF0
 static void scsi+AF8-dump+AF8-inquiry(struct se+AF8-device +ACo-dev)
 +AHs
 	struct t10+AF8-wwn +ACo-wwn +AD0 +ACY-dev-+AD4-t10+AF8-wwn+ADs
-	char buf+AFs-17+AF0AOw
-	int i, device+AF8-type+ADs
+-	int device+AF8-type +AD0 dev-+AD4-transport-+AD4-get+AF8-device+AF8-type(dev)+ADs
+-
 	/+ACo
 	 +ACo Print Linux/SCSI style INQUIRY formatting to the kernel ring buffer
 	 +ACo-/
-	for (i +AD0 0+ADs i +ADw 8+ADs i+-+-)
-		if (wwn-+AD4-vendor+AFs-i+AF0 +AD4APQ 0x20)
-			buf+AFs-i+AF0 +AD0 wwn-+AD4-vendor+AFs-i+AF0AOw
-		else
-			buf+AFs-i+AF0 +AD0 ' '+ADs
-	buf+AFs-i+AF0 +AD0 '+AFw-0'+ADs
-	pr+AF8-debug(+ACI  Vendor: +ACU-s+AFw-n+ACI, buf)+ADs
-
-	for (i +AD0 0+ADs i +ADw 16+ADs i+-+-)
-		if (wwn-+AD4-model+AFs-i+AF0 +AD4APQ 0x20)
-			buf+AFs-i+AF0 +AD0 wwn-+AD4-model+AFs-i+AF0AOw
-		else
-			buf+AFs-i+AF0 +AD0 ' '+ADs
-	buf+AFs-i+AF0 +AD0 '+AFw-0'+ADs
-	pr+AF8-debug(+ACI  Model: +ACU-s+AFw-n+ACI, buf)+ADs
-
-	for (i +AD0 0+ADs i +ADw 4+ADs i+-+-)
-		if (wwn-+AD4-revision+AFs-i+AF0 +AD4APQ 0x20)
-			buf+AFs-i+AF0 +AD0 wwn-+AD4-revision+AFs-i+AF0AOw
-		else
-			buf+AFs-i+AF0 +AD0 ' '+ADs
-	buf+AFs-i+AF0 +AD0 '+AFw-0'+ADs
-	pr+AF8-debug(+ACI  Revision: +ACU-s+AFw-n+ACI, buf)+ADs
-
-	device+AF8-type +AD0 dev-+AD4-transport-+AD4-get+AF8-device+AF8-type(dev)+ADs
+-	pr+AF8-debug(+ACI  Vendor: +ACU--8s+AFw-n+ACI, wwn-+AD4-vendor)+ADs
+-	pr+AF8-debug(+ACI  Model: +ACU--16s+AFw-n+ACI, wwn-+AD4-model)+ADs
+-	pr+AF8-debug(+ACI  Revision: +ACU--4s+AFw-n+ACI, wwn-+AD4-revision)+ADs
 	pr+AF8-debug(+ACI  Type:   +ACU-s +ACI, scsi+AF8-device+AF8-type(device+AF8-type))+ADs
 +AH0

Thanks,

Bart.

Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

[David Disseldorp]

Hi Bart,

On Wed, 28 Nov 2018 08:36:19 -0800, Bart Van Assche wrote:

> Maybe I'm missing something, but why is zeroing of unused bytes in these functions
> necessary? Would the following be correct if all strings in struct t10_wwn would be
> '\0'-terminated?

Your patch looks good to me. Mind if I tack it on to the end of my
t10_wwn.vendor/model/revision[size+1] patchset, with your authorship?

Cheers, David

Re: [PATCH v3 3/4] target: add device vendor_id configfs attribute

[Bart Van Assche]

On Wed, 2018-11-28 at 17:44 +-0100, David Disseldorp wrote:
+AD4 Hi Bart,
+AD4 
+AD4 On Wed, 28 Nov 2018 08:36:19 -0800, Bart Van Assche wrote:
+AD4 
+AD4 +AD4 Maybe I'm missing something, but why is zeroing of unused bytes in these functions
+AD4 +AD4 necessary? Would the following be correct if all strings in struct t10+AF8-wwn would be
+AD4 +AD4 '+AFw-0'-terminated?
+AD4 
+AD4 Your patch looks good to me. Mind if I tack it on to the end of my
+AD4 t10+AF8-wwn.vendor/model/revision+AFs-size+-1+AF0 patchset, with your authorship?

Sure, that sounds fine to me.

Bart.