[PATCH 0/4] libosd: Last updates for Linux 2.6.30-rc

[PATCH 0/4] libosd: Last updates for Linux 2.6.30-rc

[Boaz Harrosh]

Hi dear James, list

I would like to push these 3 patches to support OSD2 revision 5 wire
format, for the Linux 2.6.30-rcx series. So to minimize the maintainability
headache of having two Kernel versions with two separate wire formats.
(Pretending to be OSD2)

I have finally fixed the OSC's osd-target code to support OSD2 up to
revision 5, and am able to push these changes to the Linux initiator.
(Users will need the new HEAD at osd-target git-tree:
  git://git.open-osd.org/osc-osd master)

Revision 5 is suppose to be the very last changes to OSD2 wire format,
and is in general frozen state that should only accept fixes to the wording
of the text, if any exist.

The very first patch is a BUG fix sent by Dan Carpenter found with "smatch",
followed by three patches to fix the wire format for support of revision 5.
Only the very last patch actually changes the wire format, the first two
only refactor the code in preparation of the actual change.

Here is the list of patches:

[PATCH 1/4] potential ERR_PTR dereference in osd_initiator.c

[PATCH 2/4] libosd: OSD2r05: Prepare for rev5 attribute list changes
[PATCH 3/4] libosd: OSD2r05: OSD_CRYPTO_KEYID_SIZE will grow 20 => 32 bytes
[PATCH 4/4] libosd: OSD2r05: on-the-wire changes for latest OSD2 draft.

Please see last patch for summery of what changed.

Thanks in advance
Boaz

[PATCH 1/4] libosd: potential ERR_PTR dereference in osd_initiator.c

[Boaz Harrosh]

bio_map_kern() returns an ERR_PTR() not NULL.

Found by smatch (http://repo.or.cz/w/smatch.git).  Compile tested.

regards,
dan carpenter

Signed-off-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/osd/osd_initiator.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index 2a5f077..76de889 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -612,9 +612,9 @@ static int _osd_req_list_objects(struct osd_request *or,
 
 	WARN_ON(or->in.bio);
 	bio = bio_map_kern(q, list, len, or->alloc_flags);
-	if (!bio) {
+	if (IS_ERR(bio)) {
 		OSD_ERR("!!! Failed to allocate list_objects BIO\n");
-		return -ENOMEM;
+		return PTR_ERR(bio);
 	}
 
 	bio->bi_rw &= ~(1 << BIO_RW);
-- 
1.6.2.1

Re: [PATCH 1/4] libosd: potential ERR_PTR dereference in osd_initiator.c

[Boaz Harrosh]

Sorry I forgot to put:

From: Dan Carpenter <error27@gmail.com>
On 04/19/2009 07:07 PM, Boaz Harrosh wrote:
> bio_map_kern() returns an ERR_PTR() not NULL.
> 
> Found by smatch (http://repo.or.cz/w/smatch.git).  Compile tested.
> 
> regards,
> dan carpenter
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
> ---
>  drivers/scsi/osd/osd_initiator.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
> index 2a5f077..76de889 100644
> --- a/drivers/scsi/osd/osd_initiator.c
> +++ b/drivers/scsi/osd/osd_initiator.c
> @@ -612,9 +612,9 @@ static int _osd_req_list_objects(struct osd_request *or,
>  
>  	WARN_ON(or->in.bio);
>  	bio = bio_map_kern(q, list, len, or->alloc_flags);
> -	if (!bio) {
> +	if (IS_ERR(bio)) {
>  		OSD_ERR("!!! Failed to allocate list_objects BIO\n");
> -		return -ENOMEM;
> +		return PTR_ERR(bio);
>  	}
>  
>  	bio->bi_rw &= ~(1 << BIO_RW);

[PATCH 2/4] libosd: OSD2r05: Prepare for rev5 attribute list changes

[Boaz Harrosh]

In OSD2r05 draft each attribute list element header was changed
so attribute-value would be 8 bytes aligned. In OSD2r01-r04
it was aligned on 2 bytes. (This is because in OSD2r01 the complete
element was 8 bytes padded at end but the header was not adjusted
and caused permanent miss-alignment.)

OSD1 elements are not padded and might be or might not be aligned.
OSD1 is still supported.

In this code we do all the code re-factoring to separate OSD1/OSD2
differences but do not change actual wire format. All wire format
changes will happen in one patch later, for bisect-ability.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/osd/osd_initiator.c |   86 ++++++++++++++++++++++++++++++--------
 include/scsi/osd_protocol.h      |   20 +++++++--
 2 files changed, 83 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index 76de889..e266f80 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -205,6 +205,69 @@ static unsigned _osd_req_alist_elem_size(struct osd_request *or, unsigned len)
 		osdv2_attr_list_elem_size(len);
 }
 
+static void _osd_req_alist_elem_encode(struct osd_request *or,
+	void *attr_last, const struct osd_attr *oa)
+{
+	if (osd_req_is_ver1(or)) {
+		struct osdv1_attributes_list_element *attr = attr_last;
+
+		attr->attr_page = cpu_to_be32(oa->attr_page);
+		attr->attr_id = cpu_to_be32(oa->attr_id);
+		attr->attr_bytes = cpu_to_be16(oa->len);
+		memcpy(attr->attr_val, oa->val_ptr, oa->len);
+	} else {
+		struct osdv2_attributes_list_element *attr = attr_last;
+
+		attr->attr_page = cpu_to_be32(oa->attr_page);
+		attr->attr_id = cpu_to_be32(oa->attr_id);
+		attr->attr_bytes = cpu_to_be16(oa->len);
+		memcpy(attr->attr_val, oa->val_ptr, oa->len);
+	}
+}
+
+static int _osd_req_alist_elem_decode(struct osd_request *or,
+	void *cur_p, struct osd_attr *oa, unsigned max_bytes)
+{
+	unsigned inc;
+	if (osd_req_is_ver1(or)) {
+		struct osdv1_attributes_list_element *attr = cur_p;
+
+		if (max_bytes < sizeof(*attr))
+			return -1;
+
+		oa->len = be16_to_cpu(attr->attr_bytes);
+		inc = _osd_req_alist_elem_size(or, oa->len);
+		if (inc > max_bytes)
+			return -1;
+
+		oa->attr_page = be32_to_cpu(attr->attr_page);
+		oa->attr_id = be32_to_cpu(attr->attr_id);
+
+		/* OSD1: On empty attributes we return a pointer to 2 bytes
+		 * of zeros. This keeps similar behaviour with OSD2.
+		 * (See below)
+		 */
+		oa->val_ptr = likely(oa->len) ? attr->attr_val :
+						(u8 *)&attr->attr_bytes;
+	} else {
+		struct osdv2_attributes_list_element *attr = cur_p;
+
+		if (max_bytes < sizeof(*attr))
+			return -1;
+
+		oa->len = be16_to_cpu(attr->attr_bytes);
+		inc = _osd_req_alist_elem_size(or, oa->len);
+		if (inc > max_bytes)
+			return -1;
+
+		oa->attr_page = be32_to_cpu(attr->attr_page);
+		oa->attr_id = be32_to_cpu(attr->attr_id);
+
+		oa->val_ptr = attr->attr_val;
+	}
+	return inc;
+}
+
 static unsigned _osd_req_alist_size(struct osd_request *or, void *list_head)
 {
 	return osd_req_is_ver1(or) ?
@@ -798,7 +861,6 @@ int osd_req_add_set_attr_list(struct osd_request *or,
 	attr_last = or->set_attr.buff + total_bytes;
 
 	for (; nelem; --nelem) {
-		struct osd_attributes_list_element *attr;
 		unsigned elem_size = _osd_req_alist_elem_size(or, oa->len);
 
 		total_bytes += elem_size;
@@ -811,11 +873,7 @@ int osd_req_add_set_attr_list(struct osd_request *or,
 				or->set_attr.buff + or->set_attr.total_bytes;
 		}
 
-		attr = attr_last;
-		attr->attr_page = cpu_to_be32(oa->attr_page);
-		attr->attr_id = cpu_to_be32(oa->attr_id);
-		attr->attr_bytes = cpu_to_be16(oa->len);
-		memcpy(attr->attr_val, oa->val_ptr, oa->len);
+		_osd_req_alist_elem_encode(or, attr_last, oa);
 
 		attr_last += elem_size;
 		++oa;
@@ -1070,15 +1128,10 @@ int osd_req_decode_get_attr_list(struct osd_request *or,
 	}
 
 	for (n = 0; (n < *nelem) && (cur_bytes < returned_bytes); ++n) {
-		struct osd_attributes_list_element *attr = cur_p;
-		unsigned inc;
+		int inc = _osd_req_alist_elem_decode(or, cur_p, oa,
+						 returned_bytes - cur_bytes);
 
-		oa->len = be16_to_cpu(attr->attr_bytes);
-		inc = _osd_req_alist_elem_size(or, oa->len);
-		OSD_DEBUG("oa->len=%d inc=%d cur_bytes=%d\n",
-			  oa->len, inc, cur_bytes);
-		cur_bytes += inc;
-		if (cur_bytes > returned_bytes) {
+		if (inc < 0) {
 			OSD_ERR("BAD FOOD from target. list not valid!"
 				"c=%d r=%d n=%d\n",
 				cur_bytes, returned_bytes, n);
@@ -1086,10 +1139,7 @@ int osd_req_decode_get_attr_list(struct osd_request *or,
 			break;
 		}
 
-		oa->attr_page = be32_to_cpu(attr->attr_page);
-		oa->attr_id = be32_to_cpu(attr->attr_id);
-		oa->val_ptr = attr->attr_val;
-
+		cur_bytes += inc;
 		cur_p += inc;
 		++oa;
 	}
diff --git a/include/scsi/osd_protocol.h b/include/scsi/osd_protocol.h
index cd3cbf7..fa8343c 100644
--- a/include/scsi/osd_protocol.h
+++ b/include/scsi/osd_protocol.h
@@ -301,14 +301,24 @@ struct osd_attributes_list_attrid {
 } __packed;
 
 /*
+ * NOTE: v1: is not aligned.
+ */
+struct osdv1_attributes_list_element {
+	__be32 attr_page;
+	__be32 attr_id;
+	__be16 attr_bytes; /* valid bytes at attr_val without padding */
+	u8 attr_val[0];
+} __packed;
+
+/*
  * osd2r03: 7.1.3.3 List entry format for retrieved attributes and
  *                  for setting attributes
- * NOTE: v2 is 8-bytes aligned, v1 is not aligned.
+ * NOTE: v2 is 8-bytes aligned
  */
-struct osd_attributes_list_element {
+struct osdv2_attributes_list_element {
 	__be32 attr_page;
 	__be32 attr_id;
-	__be16 attr_bytes;
+	__be16 attr_bytes; /* valid bytes at attr_val without padding */
 	u8 attr_val[0];
 } __packed;
 
@@ -324,13 +334,13 @@ enum {
 
 static inline unsigned osdv1_attr_list_elem_size(unsigned len)
 {
-	return ALIGN(len + sizeof(struct osd_attributes_list_element),
+	return ALIGN(len + sizeof(struct osdv1_attributes_list_element),
 		     OSDv1_ATTRIBUTES_ELEM_ALIGN);
 }
 
 static inline unsigned osdv2_attr_list_elem_size(unsigned len)
 {
-	return ALIGN(len + sizeof(struct osd_attributes_list_element),
+	return ALIGN(len + sizeof(struct osdv2_attributes_list_element),
 		     OSD_ATTRIBUTES_ELEM_ALIGN);
 }
 
-- 
1.6.2.1

[PATCH 3/4] libosd: OSD2r05: OSD_CRYPTO_KEYID_SIZE will grow 20 => 32 bytes

[Boaz Harrosh]

In OSD2r04 draft, cryptographic key size changed to 32 bytes from
OSD1's 20 bytes. This causes a couple of on-the-wire structures
to change, including the CDB.

In this patch the OSD1/OSD2 handling is separated out in regard
to affected structures, but on-the-wire is still the same. All
on the wire changes will be submitted in one patch for bisect-ability.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/osd/osd_initiator.c |   30 ++++++++++++++++----
 include/scsi/osd_protocol.h      |   55 ++++++++++++++++++++++++++++++-------
 2 files changed, 68 insertions(+), 17 deletions(-)

diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index e266f80..f61ab84 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -345,9 +345,9 @@ _osd_req_sec_params(struct osd_request *or)
 	struct osd_cdb *ocdb = &or->cdb;
 
 	if (osd_req_is_ver1(or))
-		return &ocdb->v1.sec_params;
+		return (struct osd_security_parameters *)&ocdb->v1.sec_params;
 	else
-		return &ocdb->v2.sec_params;
+		return (struct osd_security_parameters *)&ocdb->v2.sec_params;
 }
 
 void osd_dev_init(struct osd_dev *osdd, struct scsi_device *scsi_device)
@@ -1209,6 +1209,24 @@ static int _osd_req_finalize_attr_page(struct osd_request *or)
 	return ret;
 }
 
+static inline void osd_sec_parms_set_out_offset(bool is_v1,
+	struct osd_security_parameters *sec_parms, osd_cdb_offset offset)
+{
+	if (is_v1)
+		sec_parms->v1.data_out_integrity_check_offset = offset;
+	else
+		sec_parms->v2.data_out_integrity_check_offset = offset;
+}
+
+static inline void osd_sec_parms_set_in_offset(bool is_v1,
+	struct osd_security_parameters *sec_parms, osd_cdb_offset offset)
+{
+	if (is_v1)
+		sec_parms->v1.data_in_integrity_check_offset = offset;
+	else
+		sec_parms->v2.data_in_integrity_check_offset = offset;
+}
+
 static int _osd_req_finalize_data_integrity(struct osd_request *or,
 	bool has_in, bool has_out, const u8 *cap_key)
 {
@@ -1232,8 +1250,8 @@ static int _osd_req_finalize_data_integrity(struct osd_request *or,
 		or->out_data_integ.get_attributes_bytes = cpu_to_be64(
 			or->enc_get_attr.total_bytes);
 
-		sec_parms->data_out_integrity_check_offset =
-			osd_req_encode_offset(or, or->out.total_bytes, &pad);
+		osd_sec_parms_set_out_offset(osd_req_is_ver1(or), sec_parms,
+			osd_req_encode_offset(or, or->out.total_bytes, &pad));
 
 		ret = _req_append_segment(or, pad, &seg, or->out.last_seg,
 					  &or->out);
@@ -1253,8 +1271,8 @@ static int _osd_req_finalize_data_integrity(struct osd_request *or,
 		};
 		unsigned pad;
 
-		sec_parms->data_in_integrity_check_offset =
-			osd_req_encode_offset(or, or->in.total_bytes, &pad);
+		osd_sec_parms_set_in_offset(osd_req_is_ver1(or), sec_parms,
+			osd_req_encode_offset(or, or->in.total_bytes, &pad));
 
 		ret = _req_append_segment(or, pad, &seg, or->in.last_seg,
 					  &or->in);
diff --git a/include/scsi/osd_protocol.h b/include/scsi/osd_protocol.h
index fa8343c..bbeceeb 100644
--- a/include/scsi/osd_protocol.h
+++ b/include/scsi/osd_protocol.h
@@ -33,8 +33,10 @@ enum {
 	OSD_CAP_LEN = OSDv1_CAP_LEN,/* FIXME: Pete rev-001 sup */
 
 	OSD_SYSTEMID_LEN = 20,
-	OSD_CRYPTO_KEYID_SIZE = 20,
+	OSDv1_CRYPTO_KEYID_SIZE = 20,
 	/*FIXME: OSDv2_CRYPTO_KEYID_SIZE = 32,*/
+	OSDv2_CRYPTO_KEYID_SIZE = 20,
+	OSD_CRYPTO_KEYID_SIZE = OSDv2_CRYPTO_KEYID_SIZE,
 	OSD_CRYPTO_SEED_SIZE = 4,
 	OSD_CRYPTO_NONCE_SIZE = 12,
 	OSD_MAX_SENSE_LEN = 252, /* from SPC-3 */
@@ -204,29 +206,40 @@ struct osd_cdb_head {
 /*80*/
 
 /*160 v1*/
-/*184 v2*/
-struct osd_security_parameters {
-/*160*/u8	integrity_check_value[OSD_CRYPTO_KEYID_SIZE];
+struct osdv1_security_parameters {
+/*160*/u8	integrity_check_value[OSDv1_CRYPTO_KEYID_SIZE];
 /*180*/u8	request_nonce[OSD_CRYPTO_NONCE_SIZE];
 /*192*/osd_cdb_offset	data_in_integrity_check_offset;
 /*196*/osd_cdb_offset	data_out_integrity_check_offset;
 } __packed;
 /*200 v1*/
-/*224 v2*/
 
-/* FIXME: osdv2_security_parameters */
+/*184 v2*/
+struct osdv2_security_parameters {
+/*184*/u8	integrity_check_value[OSDv2_CRYPTO_KEYID_SIZE];
+/*216*/u8	request_nonce[OSD_CRYPTO_NONCE_SIZE];
+/*228*/osd_cdb_offset	data_in_integrity_check_offset;
+/*232*/osd_cdb_offset	data_out_integrity_check_offset;
+} __packed;
+/*236 v2*/
+
+struct osd_security_parameters {
+	union {
+		struct osdv1_security_parameters v1;
+		struct osdv2_security_parameters v2;
+	};
+};
 
 struct osdv1_cdb {
 	struct osd_cdb_head h;
 	u8 caps[OSDv1_CAP_LEN];
-	struct osd_security_parameters sec_params;
+	struct osdv1_security_parameters sec_params;
 } __packed;
 
 struct osdv2_cdb {
 	struct osd_cdb_head h;
 	u8 caps[OSD_CAP_LEN];
-	struct osd_security_parameters sec_params;
-	/* FIXME: osdv2_security_parameters */
+	struct osdv2_security_parameters sec_params;
 } __packed;
 
 struct osd_cdb {
@@ -429,15 +442,35 @@ struct osd_data_out_integrity_info {
 	__be64 data_bytes;
 	__be64 set_attributes_bytes;
 	__be64 get_attributes_bytes;
-	__be64 integrity_check_value;
+	__u8 integrity_check_value[OSD_CRYPTO_KEYID_SIZE];
 } __packed;
 
+/* Same osd_data_out_integrity_info is used for OSD2/OSD1. The only difference
+ * Is the sizeof the structure since in OSD1 the last array is smaller. Use
+ * below for version independent handling of this structure
+ */
+static inline int osd_data_out_integrity_info_sizeof(bool is_ver1)
+{
+	return sizeof(struct osd_data_out_integrity_info) -
+		(is_ver1 * (OSDv2_CRYPTO_KEYID_SIZE - OSDv1_CRYPTO_KEYID_SIZE));
+}
+
 struct osd_data_in_integrity_info {
 	__be64 data_bytes;
 	__be64 retrieved_attributes_bytes;
-	__be64 integrity_check_value;
+	__u8 integrity_check_value[OSD_CRYPTO_KEYID_SIZE];
 } __packed;
 
+/* Same osd_data_in_integrity_info is used for OSD2/OSD1. The only difference
+ * Is the sizeof the structure since in OSD1 the last array is smaller. Use
+ * below for version independent handling of this structure
+ */
+static inline int osd_data_in_integrity_info_sizeof(bool is_ver1)
+{
+	return sizeof(struct osd_data_in_integrity_info) -
+		(is_ver1 * (OSDv2_CRYPTO_KEYID_SIZE - OSDv1_CRYPTO_KEYID_SIZE));
+}
+
 struct osd_timestamp {
 	u8 time[6]; /* number of milliseconds since 1/1/1970 UT (big endian) */
 } __packed;
-- 
1.6.2.1

[PATCH 4/4] libosd: OSD2r05: on-the-wire changes for latest OSD2 revision 5.

[Boaz Harrosh]

>From this point on we support only post CDB_VER_OSD2r01 tag from
OSC's OSD2 target: [git clone git://git.open-osd.org/osc-osd/ master]
(Initiator code prior to this patch must use: "git checkout CDB_VER_OSD2r01"
 in the target tree above)

This is a summery of the wire changes:

 * OSDv2_ADDITIONAL_CDB_LENGTH == 192 => 228 (Total CDB is now 236 bytes)
 * Attributes List Element Header grew, so attribute values are 8 bytes
   aligned.
 * Cryptographic keys and signatures are 20 => 32
 * Few new definitions.

(Still missing new standard definitions attribute values, these do not change
 wire format and will be added later when needed)

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/osd/osd_initiator.c |    7 ++++++-
 include/scsi/osd_protocol.h      |   23 +++++++++++++----------
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index f61ab84..1ce6b24 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -263,7 +263,12 @@ static int _osd_req_alist_elem_decode(struct osd_request *or,
 		oa->attr_page = be32_to_cpu(attr->attr_page);
 		oa->attr_id = be32_to_cpu(attr->attr_id);
 
-		oa->val_ptr = attr->attr_val;
+		/* OSD2: For convenience, on empty attributes, we return 8 bytes
+		 * of zeros here. This keeps the same behaviour with OSD2r04,
+		 * and is nice with null terminating ASCII fields.
+		 * oa->val_ptr == NULL marks the end-of-list, or error.
+		 */
+		oa->val_ptr = likely(oa->len) ? attr->attr_val : attr->reserved;
 	}
 	return inc;
 }
diff --git a/include/scsi/osd_protocol.h b/include/scsi/osd_protocol.h
index bbeceeb..62b2ab8 100644
--- a/include/scsi/osd_protocol.h
+++ b/include/scsi/osd_protocol.h
@@ -24,18 +24,17 @@ enum {
 	OSDv1_ADDITIONAL_CDB_LENGTH = 192,
 	OSDv1_TOTAL_CDB_LEN = OSDv1_ADDITIONAL_CDB_LENGTH + 8,
 	OSDv1_CAP_LEN = 80,
+
 	/* Latest supported version */
-/* 	OSD_ADDITIONAL_CDB_LENGTH = 216,*/
+	OSDv2_ADDITIONAL_CDB_LENGTH = 228,
 	OSD_ADDITIONAL_CDB_LENGTH =
-		OSDv1_ADDITIONAL_CDB_LENGTH, /* FIXME: Pete rev-001 sup */
+		OSDv2_ADDITIONAL_CDB_LENGTH,
 	OSD_TOTAL_CDB_LEN = OSD_ADDITIONAL_CDB_LENGTH + 8,
-/* 	OSD_CAP_LEN = 104,*/
-	OSD_CAP_LEN = OSDv1_CAP_LEN,/* FIXME: Pete rev-001 sup */
+	OSD_CAP_LEN = 104,
 
 	OSD_SYSTEMID_LEN = 20,
 	OSDv1_CRYPTO_KEYID_SIZE = 20,
-	/*FIXME: OSDv2_CRYPTO_KEYID_SIZE = 32,*/
-	OSDv2_CRYPTO_KEYID_SIZE = 20,
+	OSDv2_CRYPTO_KEYID_SIZE = 32,
 	OSD_CRYPTO_KEYID_SIZE = OSDv2_CRYPTO_KEYID_SIZE,
 	OSD_CRYPTO_SEED_SIZE = 4,
 	OSD_CRYPTO_NONCE_SIZE = 12,
@@ -166,7 +165,11 @@ struct osd_cdb_head {
 			/* called allocation_length in some commands */
 /*32*/			__be64	length;
 /*40*/			__be64	start_address;
-/*48*/			__be32 list_identifier;/* Rarely used */
+			union {
+/*48*/				__be32 list_identifier;/* Rarely used */
+				/* OSD2r05 5.2.5 CDB continuation length */
+/*48*/				__be32 cdb_continuation_length;
+			};
 		} __packed v2;
 	};
 /*52*/	union { /* selected attributes mode Page/List/Single */
@@ -331,6 +334,7 @@ struct osdv1_attributes_list_element {
 struct osdv2_attributes_list_element {
 	__be32 attr_page;
 	__be32 attr_id;
+	u8 reserved[6];
 	__be16 attr_bytes; /* valid bytes at attr_val without padding */
 	u8 attr_val[0];
 } __packed;
@@ -520,7 +524,7 @@ enum osd_capability_bit_masks {
 
 	OSD_SEC_CAP_NONE1	= BIT(8),
 	OSD_SEC_CAP_NONE2	= BIT(9),
-	OSD_SEC_CAP_NONE3	= BIT(10),
+	OSD_SEC_GBL_REM 	= BIT(10), /*v2 only*/
 	OSD_SEC_CAP_QUERY	= BIT(11), /*v2 only*/
 	OSD_SEC_CAP_M_OBJECT	= BIT(12), /*v2 only*/
 	OSD_SEC_CAP_POL_SEC	= BIT(13),
@@ -595,8 +599,7 @@ struct osdv1_capability {
 
 struct osd_capability {
 	struct osd_capability_head h;
-/* 	struct osd_cap_object_descriptor od;*/
-	struct osdv1_cap_object_descriptor od; /* FIXME: Pete rev-001 sup */
+	struct osd_cap_object_descriptor od;
 } __packed;
 
 /**
-- 
1.6.2.1