[PATCH] Library: Fix incorrect Volume.vpd83 definition.

[Gris Ge]

* The Volume.vpd83 property is designed to match Linux SCSI ID.
In practice, it should match "ID_WWN" value of udev information.
It could be check via command:

udevadm info --query=all /dev/sda

* In udev code, the "ID_WWN" is VPD 0x83 NAA (identifier type 3) information.

* In SPC-3 and later version, the VPD 0x83 NAA is allowed in these formats:

Type | Type Name | Size | Code Set
-----+--------------------------+-------+--------------
2h | IEEE Extended | 08h | 1h(binary)
3h | Local Assigned | 08h | 1h(binary)
5h | IEEE Registered | 08h | 1h(binary)
6h | IEEE Registered Extended | 10h | 1h(binary)

Note: The '3h(local assigned)' is defined in SPC-4.
In spc4r37.pdf, page 757, section 7.8.6.6 NAA designator format.

* It's clear our VPD 0x83 string verification method is incorrect.
This patch is to enforce above limitation:

1. String start with 6 should with string size 32.
2. String start with 2,3,5 should with string size 16.
3. String not start with 2,3,5,6 is illegal.

Signed-off-by: Gris Ge <***@redhat.com>
---
c_binding/lsm_mgmt.cpp | 19 ++++++++++++-------
python_binding/lsm/_data.py | 6 +++---
2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/c_binding/lsm_mgmt.cpp b/c_binding/lsm_mgmt.cpp
index cbe57fe..6e6d8a6 100644
--- a/c_binding/lsm_mgmt.cpp
+++ b/c_binding/lsm_mgmt.cpp
@@ -118,15 +118,20 @@ int lsm_volume_vpd83_verify( const char *vpd83 )
int rc = LSM_ERR_INVALID_ARGUMENT;
int i;

- if( vpd83 && strlen(vpd83) == 32 ) {
- for(i = 0; i < 32; ++i) {
- char v = vpd83[i];
- // 0-9 || a-f is OK
- if( !((v >= 48 && v <= 57) || (v >= 97 && v <= 102)) ) {
- return rc;
+ if( vpd83 ){
+ if( (strlen(vpd83) == 32 && vpd83[0] == '6' ) ||
+ (strlen(vpd83) == 16 && vpd83[0] == '2' ) ||
+ (strlen(vpd83) == 16 && vpd83[0] == '3' ) ||
+ (strlen(vpd83) == 16 && vpd83[0] == '5' ) {
+ for(i = 0; i < strlen(vpd83); ++i) {
+ char v = vpd83[i];
+ // 0-9 || a-f is OK
+ if( !((v >= 48 && v <= 57) || (v >= 97 && v <= 102)) ) {
+ return rc;
+ }
}
+ rc = LSM_ERR_OK;
}
- rc = LSM_ERR_OK;
}
return rc;
}
diff --git a/python_binding/lsm/_data.py b/python_binding/lsm/_data.py
index 0e16311..7b470ff 100644
--- a/python_binding/lsm/_data.py
+++ b/python_binding/lsm/_data.py
@@ -232,7 +232,7 @@ def __str__(self):

# Lets do this once outside of the class to minimize the number of
# times it needs to be compiled.
-_vol_regex_vpd83 = re.compile('^[0-9a-f]{32}$')
+_vol_regex_vpd83 = re.compile('(?:^6[0-9a-f]{31})|(?:^[2356][0-9a-f]{15})$')


@default_property('id', doc="Unique identifier")
@@ -314,8 +314,8 @@ def __init__(self, _id, _name, _vpd83, _block_size, _num_of_blocks,
self._name = _name # Human recognisable name
if _vpd83 and not Volume.vpd83_verify(_vpd83):
raise LsmError(ErrorNumber.INVALID_ARGUMENT,
- "Incorrect format of VPD 0x83 string: '%s', "
- "expecting 32 lower case hex characters" %
+ "Incorrect format of VPD 0x83 NAA(3) string: '%s', "
+ "expecting 32 or 16 lower case hex characters" %
_vpd83)
self._vpd83 = _vpd83 # SCSI page 83 unique ID
self._block_size = _block_size # Block size

[Gris Ge]

* The Volume.vpd83 property is designed to match Linux SCSI ID.
In practice, it should match "ID_WWN" value of udev information.
It could be check via command:

udevadm info --query=all /dev/sda

* In udev code, the "ID_WWN" is VPD 0x83 NAA (identifier type 3) information.

* In SPC-3 and later version, the VPD 0x83 NAA is allowed in these formats:

Type | Type Name | Size | Code Set
-----+--------------------------+-------+--------------
2h | IEEE Extended | 08h | 1h(binary)
3h | Local Assigned | 08h | 1h(binary)
5h | IEEE Registered | 08h | 1h(binary)
6h | IEEE Registered Extended | 10h | 1h(binary)

Note: The '3h(local assigned)' is defined in SPC-4.
In spc4r37.pdf, page 757, section 7.8.6.6 NAA designator format.

* It's clear our VPD 0x83 string verification method is incorrect.
This patch is to enforce above limitation:

1. String start with 6 should with string size 32.
2. String start with 2,3,5 should with string size 16.
3. String not start with 2,3,5,6 is illegal.

Changes in V2:
* Fix the incorrect python regex:

(?:^6[0-9a-f]{31})|(?:^[2356][0-9a-f]{15})
^
|--- So remove 6.

Signed-off-by: Gris Ge <***@redhat.com>
---
c_binding/lsm_mgmt.cpp | 19 ++++++++++++-------
python_binding/lsm/_data.py | 6 +++---
2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/c_binding/lsm_mgmt.cpp b/c_binding/lsm_mgmt.cpp
index cbe57fe..6e6d8a6 100644
--- a/c_binding/lsm_mgmt.cpp
+++ b/c_binding/lsm_mgmt.cpp
@@ -118,15 +118,20 @@ int lsm_volume_vpd83_verify( const char *vpd83 )
int rc = LSM_ERR_INVALID_ARGUMENT;
int i;

- if( vpd83 && strlen(vpd83) == 32 ) {
- for(i = 0; i < 32; ++i) {
- char v = vpd83[i];
- // 0-9 || a-f is OK
- if( !((v >= 48 && v <= 57) || (v >= 97 && v <= 102)) ) {
- return rc;
+ if( vpd83 ){
+ if( (strlen(vpd83) == 32 && vpd83[0] == '6' ) ||
+ (strlen(vpd83) == 16 && vpd83[0] == '2' ) ||
+ (strlen(vpd83) == 16 && vpd83[0] == '3' ) ||
+ (strlen(vpd83) == 16 && vpd83[0] == '5' ) {
+ for(i = 0; i < strlen(vpd83); ++i) {
+ char v = vpd83[i];
+ // 0-9 || a-f is OK
+ if( !((v >= 48 && v <= 57) || (v >= 97 && v <= 102)) ) {
+ return rc;
+ }
}
+ rc = LSM_ERR_OK;
}
- rc = LSM_ERR_OK;
}
return rc;
}
diff --git a/python_binding/lsm/_data.py b/python_binding/lsm/_data.py
index 0e16311..ce133a2 100644
--- a/python_binding/lsm/_data.py
+++ b/python_binding/lsm/_data.py
@@ -232,7 +232,7 @@ def __str__(self):

# Lets do this once outside of the class to minimize the number of
# times it needs to be compiled.
-_vol_regex_vpd83 = re.compile('^[0-9a-f]{32}$')
+_vol_regex_vpd83 = re.compile('(?:^6[0-9a-f]{31})|(?:^[235][0-9a-f]{15})$')


@default_property('id', doc="Unique identifier")
@@ -314,8 +314,8 @@ def __init__(self, _id, _name, _vpd83, _block_size, _num_of_blocks,
self._name = _name # Human recognisable name
if _vpd83 and not Volume.vpd83_verify(_vpd83):
raise LsmError(ErrorNumber.INVALID_ARGUMENT,
- "Incorrect format of VPD 0x83 string: '%s', "
- "expecting 32 lower case hex characters" %
+ "Incorrect format of VPD 0x83 NAA(3) string: '%s', "
+ "expecting 32 or 16 lower case hex characters" %
_vpd83)
self._vpd83 = _vpd83 # SCSI page 83 unique ID
self._block_size = _block_size # Block size

[Gris Ge]

* After review SPC-3 and SPC-4 document and udev source code, I noticed the
incorrect definition of our Volume.vpd83.

Gris Ge (4):
Library: Fix incorrect Volume.vpd83 definition.
C Unit Test: Fix incorrect lsm_volume_vpd83_verify test.
Plugin Test: Fix incorrect VPD83 test.
SMI-S Plugin: Use Volume.vpd83_verify() to verify volume VPD83 string.

c_binding/lsm_mgmt.cpp | 19 ++++++++++++-------
plugin/smispy/dmtf.py | 6 ------
plugin/smispy/smis_vol.py | 9 +++++----
python_binding/lsm/_data.py | 6 +++---
test/plugin_test.py | 20 +++++++++++---------
test/tester.c | 12 ++++++++++--
6 files changed, 41 insertions(+), 31 deletions(-)

[Gris Ge]

* The Volume.vpd83 property is designed to match Linux SCSI ID.
In practice, it should match "ID_WWN" value of udev information.
It could be check via command:

udevadm info --query=all /dev/sda

* In udev code, the "ID_WWN" is VPD 0x83 NAA (identifier type 3) information.

* In SPC-3 and later version, the VPD 0x83 NAA is allowed in these formats:

Type | Type Name | Size | Code Set
-----+--------------------------+-------+--------------
2h | IEEE Extended | 08h | 1h(binary)
3h | Local Assigned | 08h | 1h(binary)
5h | IEEE Registered | 08h | 1h(binary)
6h | IEEE Registered Extended | 10h | 1h(binary)

Note: The '3h(local assigned)' is defined in SPC-4.
In spc4r37.pdf, page 757, section 7.8.6.6 NAA designator format.

* It's clear our VPD 0x83 string verification method is incorrect.
This patch is to enforce above limitation:

1. String start with 6 should with string size 32.
2. String start with 2,3,5 should with string size 16.
3. String not start with 2,3,5,6 is illegal.

Changes in V2:
* Fix the incorrect python regex:

(?:^6[0-9a-f]{31})|(?:^[2356][0-9a-f]{15})
^
|--- So remove 6.

Changes in V3:
* Fix incorrect C syntax:

(strlen(vpd83) == 16 && vpd83[0] == '5' ) {
^
|--- Missing a parentheses.

Signed-off-by: Gris Ge <***@redhat.com>
---
c_binding/lsm_mgmt.cpp | 19 ++++++++++++-------
python_binding/lsm/_data.py | 6 +++---
2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/c_binding/lsm_mgmt.cpp b/c_binding/lsm_mgmt.cpp
index cbe57fe..1741a0a 100644
--- a/c_binding/lsm_mgmt.cpp
+++ b/c_binding/lsm_mgmt.cpp
@@ -118,15 +118,20 @@ int lsm_volume_vpd83_verify( const char *vpd83 )
int rc = LSM_ERR_INVALID_ARGUMENT;
int i;

- if( vpd83 && strlen(vpd83) == 32 ) {
- for(i = 0; i < 32; ++i) {
- char v = vpd83[i];
- // 0-9 || a-f is OK
- if( !((v >= 48 && v <= 57) || (v >= 97 && v <= 102)) ) {
- return rc;
+ if( vpd83 ){
+ if( (strlen(vpd83) == 32 && vpd83[0] == '6' ) ||
+ (strlen(vpd83) == 16 && vpd83[0] == '2' ) ||
+ (strlen(vpd83) == 16 && vpd83[0] == '3' ) ||
+ (strlen(vpd83) == 16 && vpd83[0] == '5' )) {
+ for(i = 0; i < strlen(vpd83); ++i) {
+ char v = vpd83[i];
+ // 0-9 || a-f is OK
+ if( !((v >= 48 && v <= 57) || (v >= 97 && v <= 102)) ) {
+ return rc;
+ }
}
+ rc = LSM_ERR_OK;
}
- rc = LSM_ERR_OK;
}
return rc;
}
diff --git a/python_binding/lsm/_data.py b/python_binding/lsm/_data.py
index 0e16311..ce133a2 100644
--- a/python_binding/lsm/_data.py
+++ b/python_binding/lsm/_data.py
@@ -232,7 +232,7 @@ def __str__(self):

# Lets do this once outside of the class to minimize the number of
# times it needs to be compiled.
-_vol_regex_vpd83 = re.compile('^[0-9a-f]{32}$')
+_vol_regex_vpd83 = re.compile('(?:^6[0-9a-f]{31})|(?:^[235][0-9a-f]{15})$')


@default_property('id', doc="Unique identifier")
@@ -314,8 +314,8 @@ def __init__(self, _id, _name, _vpd83, _block_size, _num_of_blocks,
self._name = _name # Human recognisable name
if _vpd83 and not Volume.vpd83_verify(_vpd83):
raise LsmError(ErrorNumber.INVALID_ARGUMENT,
- "Incorrect format of VPD 0x83 string: '%s', "
- "expecting 32 lower case hex characters" %
+ "Incorrect format of VPD 0x83 NAA(3) string: '%s', "
+ "expecting 32 or 16 lower case hex characters" %
_vpd83)
self._vpd83 = _vpd83 # SCSI page 83 unique ID
self._block_size = _block_size # Block size

[Gris Ge]

* Fix the incorrect lsm_volume_vpd83_verify test by using correct
VPD 83 NAA strings.

Signed-off-by: Gris Ge <***@redhat.com>
---
test/tester.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/test/tester.c b/test/tester.c
index a106e9c..4c84aab 100644
--- a/test/tester.c
+++ b/test/tester.c
@@ -2853,8 +2853,16 @@ START_TEST(test_volume_vpd_check)
F(rc, lsm_volume_vpd83_verify, "012345678901234567890123456789ag");
F(rc, lsm_volume_vpd83_verify, "1234567890123456789012345abcdef");
F(rc, lsm_volume_vpd83_verify, "01234567890123456789012345abcdefa");
-
- G(rc, lsm_volume_vpd83_verify, "01234567890123456789012345abcdef");
+ F(rc, lsm_volume_vpd83_verify, "01234567890123456789012345abcdef");
+ F(rc, lsm_volume_vpd83_verify, "55cd2e404beec32e0");
+ F(rc, lsm_volume_vpd83_verify, "55cd2e404beec32ex");
+ F(rc, lsm_volume_vpd83_verify, "55cd2e404beec32A");
+ F(rc, lsm_volume_vpd83_verify, "35cd2e404beec32A");
+
+ G(rc, lsm_volume_vpd83_verify, "61234567890123456789012345abcdef");
+ G(rc, lsm_volume_vpd83_verify, "55cd2e404beec32e");
+ G(rc, lsm_volume_vpd83_verify, "35cd2e404beec32e");
+ G(rc, lsm_volume_vpd83_verify, "25cd2e404beec32e");
}
END_TEST

[Gris Ge]

* Use library method to verify VPD 83 string.

* Remove unused constants from dmtf.py:
* VOL_NAME_FORMAT_OTHER
* VOL_NAME_FORMAT_EUI64
* VOL_NAME_FORMAT_T10VID
* VOL_NAME_SPACE_OTHER
* VOL_NAME_SPACE_VPD83_TYPE2
* VOL_NAME_SPACE_VPD83_TYPE1

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/dmtf.py | 6 ------
plugin/smispy/smis_vol.py | 9 +++++----
2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/plugin/smispy/dmtf.py b/plugin/smispy/dmtf.py
index c44db2f..f3fe91a 100644
--- a/plugin/smispy/dmtf.py
+++ b/plugin/smispy/dmtf.py
@@ -201,16 +201,10 @@ def op_status_list_conv(conv_dict, dmtf_op_status_list,
REPLICA_MODE_ASYNC = Uint16(3)

# CIM_StorageVolume['NameFormat']
-VOL_NAME_FORMAT_OTHER = 1
VOL_NAME_FORMAT_NNA = 9
-VOL_NAME_FORMAT_EUI64 = 10
-VOL_NAME_FORMAT_T10VID = 11

# CIM_StorageVolume['NameNamespace']
-VOL_NAME_SPACE_OTHER = 1
VOL_NAME_SPACE_VPD83_TYPE3 = 2
-VOL_NAME_SPACE_VPD83_TYPE2 = 3
-VOL_NAME_SPACE_VPD83_TYPE1 = 4

# CIM_ReplicationServiceCapabilities['SupportedAsynchronousActions']
# or CIM_ReplicationServiceCapabilities['SupportedSynchronousActions']
diff --git a/plugin/smispy/smis_vol.py b/plugin/smispy/smis_vol.py
index f8c727c..cc5a7b0 100644
--- a/plugin/smispy/smis_vol.py
+++ b/plugin/smispy/smis_vol.py
@@ -158,8 +158,7 @@ def _vpd83_netapp(cim_vol):

def _vpd83_of_cim_vol(cim_vol):
"""
- Extract VPD83 string from CIMInstanceName and convert to LSM format:
- ^6[a-f0-9]{31}$
+ Extract VPD83 NAA string from CIMInstanceName and convert to LSM format.
"""
vpd_83 = _vpd83_in_cim_vol_name(cim_vol)
if vpd_83 is None:
@@ -167,8 +166,10 @@ def _vpd83_of_cim_vol(cim_vol):
if vpd_83 is None:
vpd_83 = _vpd83_netapp(cim_vol)

- if vpd_83 and re.match('^6[a-fA-F0-9]{31}$', vpd_83):
- return vpd_83.lower()
+ vpd_83 = vpd_83.lower()
+
+ if vpd_83 and Volume.vpd83_verify(vpd_83):
+ return vpd_83
else:
return ''

[Gris Ge]

* After review SPC-3 and SPC-4 document and udev source code, I noticed the
incorrect definition of our Volume.vpd83.

Changes in V4:

* Patch 4/4: Fix None.lower() error in SMI-S plugin.

Gris Ge (4):
Library: Fix incorrect Volume.vpd83 definition.
C Unit Test: Fix incorrect lsm_volume_vpd83_verify test.
Plugin Test: Fix incorrect VPD83 test.
SMI-S Plugin: Use Volume.vpd83_verify() to verify volume VPD83 string.

c_binding/lsm_mgmt.cpp | 19 ++++++++++++-------
plugin/smispy/dmtf.py | 6 ------
plugin/smispy/smis_vol.py | 10 ++++++----
python_binding/lsm/_data.py | 6 +++---
test/plugin_test.py | 20 +++++++++++---------
test/tester.c | 12 ++++++++++--
6 files changed, 42 insertions(+), 31 deletions(-)

[Gris Ge]

* The Volume.vpd83 property is designed to match Linux SCSI ID.
In practice, it should match "ID_WWN" value of udev information.
It could be check via command:

udevadm info --query=all /dev/sda

* In udev code, the "ID_WWN" is VPD 0x83 NAA (identifier type 3) information.

* In SPC-3 and later version, the VPD 0x83 NAA is allowed in these formats:

Type | Type Name | Size | Code Set
-----+--------------------------+-------+--------------
2h | IEEE Extended | 08h | 1h(binary)
3h | Local Assigned | 08h | 1h(binary)
5h | IEEE Registered | 08h | 1h(binary)
6h | IEEE Registered Extended | 10h | 1h(binary)

Note: The '3h(local assigned)' is defined in SPC-4.
In spc4r37.pdf, page 757, section 7.8.6.6 NAA designator format.

* It's clear our VPD 0x83 string verification method is incorrect.
This patch is to enforce above limitation:

1. String start with 6 should with string size 32.
2. String start with 2,3,5 should with string size 16.
3. String not start with 2,3,5,6 is illegal.

Changes in V2:
* Fix the incorrect python regex:

(?:^6[0-9a-f]{31})|(?:^[2356][0-9a-f]{15})
^
|--- So remove 6.

Changes in V3:
* Fix incorrect C syntax:

(strlen(vpd83) == 16 && vpd83[0] == '5' ) {
^
|--- Missing a parentheses.

Signed-off-by: Gris Ge <***@redhat.com>
---
c_binding/lsm_mgmt.cpp | 19 ++++++++++++-------
python_binding/lsm/_data.py | 6 +++---
2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/c_binding/lsm_mgmt.cpp b/c_binding/lsm_mgmt.cpp
index cbe57fe..1741a0a 100644
--- a/c_binding/lsm_mgmt.cpp
+++ b/c_binding/lsm_mgmt.cpp
@@ -118,15 +118,20 @@ int lsm_volume_vpd83_verify( const char *vpd83 )
int rc = LSM_ERR_INVALID_ARGUMENT;
int i;

- if( vpd83 && strlen(vpd83) == 32 ) {
- for(i = 0; i < 32; ++i) {
- char v = vpd83[i];
- // 0-9 || a-f is OK
- if( !((v >= 48 && v <= 57) || (v >= 97 && v <= 102)) ) {
- return rc;
+ if( vpd83 ){
+ if( (strlen(vpd83) == 32 && vpd83[0] == '6' ) ||
+ (strlen(vpd83) == 16 && vpd83[0] == '2' ) ||
+ (strlen(vpd83) == 16 && vpd83[0] == '3' ) ||
+ (strlen(vpd83) == 16 && vpd83[0] == '5' )) {
+ for(i = 0; i < strlen(vpd83); ++i) {
+ char v = vpd83[i];
+ // 0-9 || a-f is OK
+ if( !((v >= 48 && v <= 57) || (v >= 97 && v <= 102)) ) {
+ return rc;
+ }
}
+ rc = LSM_ERR_OK;
}
- rc = LSM_ERR_OK;
}
return rc;
}
diff --git a/python_binding/lsm/_data.py b/python_binding/lsm/_data.py
index 0e16311..ce133a2 100644
--- a/python_binding/lsm/_data.py
+++ b/python_binding/lsm/_data.py
@@ -232,7 +232,7 @@ def __str__(self):

# Lets do this once outside of the class to minimize the number of
# times it needs to be compiled.
-_vol_regex_vpd83 = re.compile('^[0-9a-f]{32}$')
+_vol_regex_vpd83 = re.compile('(?:^6[0-9a-f]{31})|(?:^[235][0-9a-f]{15})$')


@default_property('id', doc="Unique identifier")
@@ -314,8 +314,8 @@ def __init__(self, _id, _name, _vpd83, _block_size, _num_of_blocks,
self._name = _name # Human recognisable name
if _vpd83 and not Volume.vpd83_verify(_vpd83):
raise LsmError(ErrorNumber.INVALID_ARGUMENT,
- "Incorrect format of VPD 0x83 string: '%s', "
- "expecting 32 lower case hex characters" %
+ "Incorrect format of VPD 0x83 NAA(3) string: '%s', "
+ "expecting 32 or 16 lower case hex characters" %
_vpd83)
self._vpd83 = _vpd83 # SCSI page 83 unique ID
self._block_size = _block_size # Block size

[Gris Ge]

* Fix the incorrect lsm_volume_vpd83_verify test by using correct
VPD 83 NAA strings.

Signed-off-by: Gris Ge <***@redhat.com>
---
test/tester.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/test/tester.c b/test/tester.c
index a106e9c..4c84aab 100644
--- a/test/tester.c
+++ b/test/tester.c
@@ -2853,8 +2853,16 @@ START_TEST(test_volume_vpd_check)
F(rc, lsm_volume_vpd83_verify, "012345678901234567890123456789ag");
F(rc, lsm_volume_vpd83_verify, "1234567890123456789012345abcdef");
F(rc, lsm_volume_vpd83_verify, "01234567890123456789012345abcdefa");
-
- G(rc, lsm_volume_vpd83_verify, "01234567890123456789012345abcdef");
+ F(rc, lsm_volume_vpd83_verify, "01234567890123456789012345abcdef");
+ F(rc, lsm_volume_vpd83_verify, "55cd2e404beec32e0");
+ F(rc, lsm_volume_vpd83_verify, "55cd2e404beec32ex");
+ F(rc, lsm_volume_vpd83_verify, "55cd2e404beec32A");
+ F(rc, lsm_volume_vpd83_verify, "35cd2e404beec32A");
+
+ G(rc, lsm_volume_vpd83_verify, "61234567890123456789012345abcdef");
+ G(rc, lsm_volume_vpd83_verify, "55cd2e404beec32e");
+ G(rc, lsm_volume_vpd83_verify, "35cd2e404beec32e");
+ G(rc, lsm_volume_vpd83_verify, "25cd2e404beec32e");
}
END_TEST

[Gris Ge]

* Use library method to verify VPD 83 string.

* Remove unused constants from dmtf.py:
* VOL_NAME_FORMAT_OTHER
* VOL_NAME_FORMAT_EUI64
* VOL_NAME_FORMAT_T10VID
* VOL_NAME_SPACE_OTHER
* VOL_NAME_SPACE_VPD83_TYPE2
* VOL_NAME_SPACE_VPD83_TYPE1

Changes in V4 (this patch introduced in V3):

* Fix AttributeError when None.lower().

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/dmtf.py | 6 ------
plugin/smispy/smis_vol.py | 10 ++++++----
2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/plugin/smispy/dmtf.py b/plugin/smispy/dmtf.py
index c44db2f..f3fe91a 100644
--- a/plugin/smispy/dmtf.py
+++ b/plugin/smispy/dmtf.py
@@ -201,16 +201,10 @@ def op_status_list_conv(conv_dict, dmtf_op_status_list,
REPLICA_MODE_ASYNC = Uint16(3)

# CIM_StorageVolume['NameFormat']
-VOL_NAME_FORMAT_OTHER = 1
VOL_NAME_FORMAT_NNA = 9
-VOL_NAME_FORMAT_EUI64 = 10
-VOL_NAME_FORMAT_T10VID = 11

# CIM_StorageVolume['NameNamespace']
-VOL_NAME_SPACE_OTHER = 1
VOL_NAME_SPACE_VPD83_TYPE3 = 2
-VOL_NAME_SPACE_VPD83_TYPE2 = 3
-VOL_NAME_SPACE_VPD83_TYPE1 = 4

# CIM_ReplicationServiceCapabilities['SupportedAsynchronousActions']
# or CIM_ReplicationServiceCapabilities['SupportedSynchronousActions']
diff --git a/plugin/smispy/smis_vol.py b/plugin/smispy/smis_vol.py
index f8c727c..a2f15c9 100644
--- a/plugin/smispy/smis_vol.py
+++ b/plugin/smispy/smis_vol.py
@@ -158,8 +158,7 @@ def _vpd83_netapp(cim_vol):

def _vpd83_of_cim_vol(cim_vol):
"""
- Extract VPD83 string from CIMInstanceName and convert to LSM format:
- ^6[a-f0-9]{31}$
+ Extract VPD83 NAA string from CIMInstanceName and convert to LSM format.
"""
vpd_83 = _vpd83_in_cim_vol_name(cim_vol)
if vpd_83 is None:
@@ -167,8 +166,11 @@ def _vpd83_of_cim_vol(cim_vol):
if vpd_83 is None:
vpd_83 = _vpd83_netapp(cim_vol)

- if vpd_83 and re.match('^6[a-fA-F0-9]{31}$', vpd_83):
- return vpd_83.lower()
+ if vpd_83:
+ vpd_83 = vpd_83.lower()
+
+ if vpd_83 and Volume.vpd83_verify(vpd_83):
+ return vpd_83
else:
return ''

[Gris Ge]

* Fix the incorrect lsm_volume_vpd83_verify test by using correct
VPD 83 NAA strings.

Signed-off-by: Gris Ge <***@redhat.com>
---
test/tester.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/test/tester.c b/test/tester.c
index a106e9c..4c84aab 100644
--- a/test/tester.c
+++ b/test/tester.c
@@ -2853,8 +2853,16 @@ START_TEST(test_volume_vpd_check)
F(rc, lsm_volume_vpd83_verify, "012345678901234567890123456789ag");
F(rc, lsm_volume_vpd83_verify, "1234567890123456789012345abcdef");
F(rc, lsm_volume_vpd83_verify, "01234567890123456789012345abcdefa");
-
- G(rc, lsm_volume_vpd83_verify, "01234567890123456789012345abcdef");
+ F(rc, lsm_volume_vpd83_verify, "01234567890123456789012345abcdef");
+ F(rc, lsm_volume_vpd83_verify, "55cd2e404beec32e0");
+ F(rc, lsm_volume_vpd83_verify, "55cd2e404beec32ex");
+ F(rc, lsm_volume_vpd83_verify, "55cd2e404beec32A");
+ F(rc, lsm_volume_vpd83_verify, "35cd2e404beec32A");
+
+ G(rc, lsm_volume_vpd83_verify, "61234567890123456789012345abcdef");
+ G(rc, lsm_volume_vpd83_verify, "55cd2e404beec32e");
+ G(rc, lsm_volume_vpd83_verify, "35cd2e404beec32e");
+ G(rc, lsm_volume_vpd83_verify, "25cd2e404beec32e");
}
END_TEST

[Gris Ge]

* The Volume.vpd83 property is designed to match Linux SCSI ID.
In practice, it should match "ID_WWN" value of udev information.
It could be check via command:

udevadm info --query=all /dev/sda

* In udev code, the "ID_WWN" is VPD 0x83 NAA (identifier type 3) information.

* In SPC-3 and later version, the VPD 0x83 NAA is allowed in these formats:

Type | Type Name | Size | Code Set
-----+--------------------------+-------+--------------
2h | IEEE Extended | 08h | 1h(binary)
3h | Local Assigned | 08h | 1h(binary)
5h | IEEE Registered | 08h | 1h(binary)
6h | IEEE Registered Extended | 10h | 1h(binary)

Note: The '3h(local assigned)' is defined in SPC-4.
In spc4r37.pdf, page 757, section 7.8.6.6 NAA designator format.

* It's clear our VPD 0x83 string verification method is incorrect.
This patch is to enforce above limitation:

1. String start with 6 should with string size 32.
2. String start with 2,3,5 should with string size 16.
3. String not start with 2,3,5,6 is illegal.

Changes in V2:
* Fix the incorrect python regex:

(?:^6[0-9a-f]{31})|(?:^[2356][0-9a-f]{15})
^
|--- So remove 6.

Changes in V3:
* Fix incorrect C syntax:

(strlen(vpd83) == 16 && vpd83[0] == '5' ) {
^
|--- Missing a parentheses.

Changes in V5:

* Remove extra strlen() by save its value to a variable in
lsm_volume_vpd83_verify() of lsm_mgmt.cpp

Signed-off-by: Gris Ge <***@redhat.com>
---
c_binding/lsm_mgmt.cpp | 23 +++++++++++++++--------
python_binding/lsm/_data.py | 6 +++---
2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/c_binding/lsm_mgmt.cpp b/c_binding/lsm_mgmt.cpp
index cbe57fe..6b195eb 100644
--- a/c_binding/lsm_mgmt.cpp
+++ b/c_binding/lsm_mgmt.cpp
@@ -117,16 +117,23 @@ int lsm_volume_vpd83_verify( const char *vpd83 )
{
int rc = LSM_ERR_INVALID_ARGUMENT;
int i;
-
- if( vpd83 && strlen(vpd83) == 32 ) {
- for(i = 0; i < 32; ++i) {
- char v = vpd83[i];
- // 0-9 || a-f is OK
- if( !((v >= 48 && v <= 57) || (v >= 97 && v <= 102)) ) {
- return rc;
+ size_t vpd83_len;
+
+ if( vpd83 ){
+ vpd83_len = strlen(vpd83);
+ if( (vpd83_len == 32 && vpd83[0] == '6' ) ||
+ (vpd83_len == 16 && vpd83[0] == '2' ) ||
+ (vpd83_len == 16 && vpd83[0] == '3' ) ||
+ (vpd83_len == 16 && vpd83[0] == '5' )) {
+ for(i = 0; i < vpd83_len; ++i) {
+ char v = vpd83[i];
+ // 0-9 || a-f is OK
+ if( !((v >= 48 && v <= 57) || (v >= 97 && v <= 102)) ) {
+ return rc;
+ }
}
+ rc = LSM_ERR_OK;
}
- rc = LSM_ERR_OK;
}
return rc;
}
diff --git a/python_binding/lsm/_data.py b/python_binding/lsm/_data.py
index 0e16311..ce133a2 100644
--- a/python_binding/lsm/_data.py
+++ b/python_binding/lsm/_data.py
@@ -232,7 +232,7 @@ def __str__(self):

# Lets do this once outside of the class to minimize the number of
# times it needs to be compiled.
-_vol_regex_vpd83 = re.compile('^[0-9a-f]{32}$')
+_vol_regex_vpd83 = re.compile('(?:^6[0-9a-f]{31})|(?:^[235][0-9a-f]{15})$')


@default_property('id', doc="Unique identifier")
@@ -314,8 +314,8 @@ def __init__(self, _id, _name, _vpd83, _block_size, _num_of_blocks,
self._name = _name # Human recognisable name
if _vpd83 and not Volume.vpd83_verify(_vpd83):
raise LsmError(ErrorNumber.INVALID_ARGUMENT,
- "Incorrect format of VPD 0x83 string: '%s', "
- "expecting 32 lower case hex characters" %
+ "Incorrect format of VPD 0x83 NAA(3) string: '%s', "
+ "expecting 32 or 16 lower case hex characters" %
_vpd83)
self._vpd83 = _vpd83 # SCSI page 83 unique ID
self._block_size = _block_size # Block size

[Gris Ge]

* Fix test_volume_vpd83_verify test.

* Replace _vpd_correct() with lsm.Volume.vpd83_verify().

Signed-off-by: Gris Ge <***@redhat.com>
---
test/plugin_test.py | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/test/plugin_test.py b/test/plugin_test.py
index 2147d43..df78243 100755
--- a/test/plugin_test.py
+++ b/test/plugin_test.py
@@ -321,12 +321,6 @@ def test_pools_list(self):
pools_list = self.c.pools()
self.assertTrue(len(pools_list) > 0, "We need at least 1 pool to test")

- @staticmethod
- def _vpd_correct(vpd):
- if vpd and re.match('^[a-f0-9]{32}$', vpd):
- return True
- return False
-
def _find_or_create_volumes(self):
"""
Find existing volumes, if not found, try to create one.
@@ -357,7 +351,7 @@ def test_volume_vpd83(self):
(volumes, flag_created) = self._find_or_create_volumes()
self.assertTrue(len(volumes) > 0, "We need at least 1 volume to test")
for v in volumes:
- self.assertTrue(TestPlugin._vpd_correct(v.vpd83),
+ self.assertTrue(lsm.Volume.vpd83_verify(v.vpd83),
"VPD is not as expected '%s' for volume id: '%s'" %
(v.vpd83, v.id))
if flag_created:
@@ -1122,13 +1116,21 @@ def test_volume_vpd83_verify(self):
"012345678901234567890123456789ax",
"012345678901234567890123456789ag",
"1234567890123456789012345abcdef",
- "01234567890123456789012345abcdefa"]
+ "01234567890123456789012345abcdefa",
+ "55cd2e404beec32e0", "55cd2e404beec32ex",
+ "35cd2e404beec32A"]

for f in failing:
self.assertFalse(lsm.Volume.vpd83_verify(f))

self.assertTrue(
- lsm.Volume.vpd83_verify("01234567890123456789012345abcdef"))
+ lsm.Volume.vpd83_verify("61234567890123456789012345abcdef"))
+ self.assertTrue(
+ lsm.Volume.vpd83_verify("55cd2e404beec32e"))
+ self.assertTrue(
+ lsm.Volume.vpd83_verify("35cd2e404beec32e"))
+ self.assertTrue(
+ lsm.Volume.vpd83_verify("25cd2e404beec32e"))

def test_available_plugins(self):
plugins = self.c.available_plugins(':')

[Gris Ge]

* Use library method to verify VPD 83 string.

* Remove unused constants from dmtf.py:
* VOL_NAME_FORMAT_OTHER
* VOL_NAME_FORMAT_EUI64
* VOL_NAME_FORMAT_T10VID
* VOL_NAME_SPACE_OTHER
* VOL_NAME_SPACE_VPD83_TYPE2
* VOL_NAME_SPACE_VPD83_TYPE1

Changes in V4 (this patch introduced in V3):

* Fix AttributeError when None.lower().

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/dmtf.py | 6 ------
plugin/smispy/smis_vol.py | 10 ++++++----
2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/plugin/smispy/dmtf.py b/plugin/smispy/dmtf.py
index c44db2f..f3fe91a 100644
--- a/plugin/smispy/dmtf.py
+++ b/plugin/smispy/dmtf.py
@@ -201,16 +201,10 @@ def op_status_list_conv(conv_dict, dmtf_op_status_list,
REPLICA_MODE_ASYNC = Uint16(3)

# CIM_StorageVolume['NameFormat']
-VOL_NAME_FORMAT_OTHER = 1
VOL_NAME_FORMAT_NNA = 9
-VOL_NAME_FORMAT_EUI64 = 10
-VOL_NAME_FORMAT_T10VID = 11

# CIM_StorageVolume['NameNamespace']
-VOL_NAME_SPACE_OTHER = 1
VOL_NAME_SPACE_VPD83_TYPE3 = 2
-VOL_NAME_SPACE_VPD83_TYPE2 = 3
-VOL_NAME_SPACE_VPD83_TYPE1 = 4

# CIM_ReplicationServiceCapabilities['SupportedAsynchronousActions']
# or CIM_ReplicationServiceCapabilities['SupportedSynchronousActions']
diff --git a/plugin/smispy/smis_vol.py b/plugin/smispy/smis_vol.py
index f8c727c..a2f15c9 100644
--- a/plugin/smispy/smis_vol.py
+++ b/plugin/smispy/smis_vol.py
@@ -158,8 +158,7 @@ def _vpd83_netapp(cim_vol):

def _vpd83_of_cim_vol(cim_vol):
"""
- Extract VPD83 string from CIMInstanceName and convert to LSM format:
- ^6[a-f0-9]{31}$
+ Extract VPD83 NAA string from CIMInstanceName and convert to LSM format.
"""
vpd_83 = _vpd83_in_cim_vol_name(cim_vol)
if vpd_83 is None:
@@ -167,8 +166,11 @@ def _vpd83_of_cim_vol(cim_vol):
if vpd_83 is None:
vpd_83 = _vpd83_netapp(cim_vol)

- if vpd_83 and re.match('^6[a-fA-F0-9]{31}$', vpd_83):
- return vpd_83.lower()
+ if vpd_83:
+ vpd_83 = vpd_83.lower()
+
+ if vpd_83 and Volume.vpd83_verify(vpd_83):
+ return vpd_83
else:
return ''

[Gris Ge]

* After review SPC-3 and SPC-4 document and udev source code, I noticed the
incorrect definition of our Volume.vpd83.

Changes in V4:

* Patch 4/4: Fix None.lower() error in SMI-S plugin.

Changes in V5:

* Patch 1/4: Remove extra strlen() call by save its return in
lsm_volume_vpd83_verify() of lsm_mgmt.cpp.

Changes in V6:

* Patch 1/4: Fix git comment about VPD83 which should be ID_WWN_WITH_EXTENSION.

Gris Ge (4):
Library: Fix incorrect Volume.vpd83 definition.
C Unit Test: Fix incorrect lsm_volume_vpd83_verify test.
Plugin Test: Fix incorrect VPD83 test.
SMI-S Plugin: Use Volume.vpd83_verify() to verify volume VPD83 string.

c_binding/lsm_mgmt.cpp | 23 +++++++++++++++--------
plugin/smispy/dmtf.py | 6 ------
plugin/smispy/smis_vol.py | 10 ++++++----
python_binding/lsm/_data.py | 6 +++---
test/plugin_test.py | 20 +++++++++++---------
test/tester.c | 12 ++++++++++--
6 files changed, 45 insertions(+), 32 deletions(-)

[Gris Ge]

* The Volume.vpd83 property is designed to match Linux SCSI ID.
In practice, it should match "ID_WWN_WITH_EXTENSION" value of udev
information which is used to create
"/dev/disk/by-id/wwn-0x<ID_WWN_WITH_EXTENSION>" disk link.
It could be check via command:

udevadm info --query=all /dev/sda | grep ID_WWN_WITH_EXTENSION
or
sudo sg_inq -p 0x83 /dev/sda # the 'designator_type: NAA' section

* In udev code, the "ID_WWN_WITH_EXTENSION" is SCSI VPD 0x83 NAA
(identifier type 3) information.

* In SPC-3 and later version, the VPD 0x83 NAA is allowed in these formats:

Type | Type Name | Size | Code Set
-----+--------------------------+-------+--------------
2h | IEEE Extended | 08h | 1h(binary)
3h | Local Assigned | 08h | 1h(binary)
5h | IEEE Registered | 08h | 1h(binary)
6h | IEEE Registered Extended | 10h | 1h(binary)

Note: The '3h(local assigned)' is defined in SPC-4.
In spc4r37.pdf, page 757, section 7.8.6.6 NAA designator format.

* It's clear our VPD 0x83 string verification method is incorrect.
This patch is to enforce above limitation:

1. String start with 6 should with string size 32.
2. String start with 2,3,5 should with string size 16.
3. String not start with 2,3,5,6 is illegal.

Changes in V2:
* Fix the incorrect python regex:

(?:^6[0-9a-f]{31})|(?:^[2356][0-9a-f]{15})
^
|--- So remove 6.

Changes in V3:
* Fix incorrect C syntax:

(strlen(vpd83) == 16 && vpd83[0] == '5' ) {
^
|--- Missing a parentheses.

Changes in V5:

* Remove extra strlen() by save its value to a variable in
lsm_volume_vpd83_verify() of lsm_mgmt.cpp

Changes in V6:

* Fix git comment about VPD83 which should be ID_WWN_WITH_EXTENSION.

Signed-off-by: Gris Ge <***@redhat.com>
---
c_binding/lsm_mgmt.cpp | 23 +++++++++++++++--------
python_binding/lsm/_data.py | 6 +++---
2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/c_binding/lsm_mgmt.cpp b/c_binding/lsm_mgmt.cpp
index 0e61541..841545d 100644
--- a/c_binding/lsm_mgmt.cpp
+++ b/c_binding/lsm_mgmt.cpp
@@ -117,16 +117,23 @@ int lsm_volume_vpd83_verify( const char *vpd83 )
{
int rc = LSM_ERR_INVALID_ARGUMENT;
int i;
-
- if( vpd83 && strlen(vpd83) == 32 ) {
- for(i = 0; i < 32; ++i) {
- char v = vpd83[i];
- // 0-9 || a-f is OK
- if( !((v >= 48 && v <= 57) || (v >= 97 && v <= 102)) ) {
- return rc;
+ size_t vpd83_len;
+
+ if( vpd83 ){
+ vpd83_len = strlen(vpd83);
+ if( (vpd83_len == 32 && vpd83[0] == '6' ) ||
+ (vpd83_len == 16 && vpd83[0] == '2' ) ||
+ (vpd83_len == 16 && vpd83[0] == '3' ) ||
+ (vpd83_len == 16 && vpd83[0] == '5' )) {
+ for(i = 0; i < vpd83_len; ++i) {
+ char v = vpd83[i];
+ // 0-9 || a-f is OK
+ if( !((v >= 48 && v <= 57) || (v >= 97 && v <= 102)) ) {
+ return rc;
+ }
}
+ rc = LSM_ERR_OK;
}
- rc = LSM_ERR_OK;
}
return rc;
}
diff --git a/python_binding/lsm/_data.py b/python_binding/lsm/_data.py
index 0e16311..ce133a2 100644
--- a/python_binding/lsm/_data.py
+++ b/python_binding/lsm/_data.py
@@ -232,7 +232,7 @@ def __str__(self):

# Lets do this once outside of the class to minimize the number of
# times it needs to be compiled.
-_vol_regex_vpd83 = re.compile('^[0-9a-f]{32}$')
+_vol_regex_vpd83 = re.compile('(?:^6[0-9a-f]{31})|(?:^[235][0-9a-f]{15})$')


@default_property('id', doc="Unique identifier")
@@ -314,8 +314,8 @@ def __init__(self, _id, _name, _vpd83, _block_size, _num_of_blocks,
self._name = _name # Human recognisable name
if _vpd83 and not Volume.vpd83_verify(_vpd83):
raise LsmError(ErrorNumber.INVALID_ARGUMENT,
- "Incorrect format of VPD 0x83 string: '%s', "
- "expecting 32 lower case hex characters" %
+ "Incorrect format of VPD 0x83 NAA(3) string: '%s', "
+ "expecting 32 or 16 lower case hex characters" %
_vpd83)
self._vpd83 = _vpd83 # SCSI page 83 unique ID
self._block_size = _block_size # Block size

[Gris Ge]

* Fix the incorrect lsm_volume_vpd83_verify test by using correct
VPD 83 NAA strings.

Signed-off-by: Gris Ge <***@redhat.com>
---
test/tester.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/test/tester.c b/test/tester.c
index a106e9c..4c84aab 100644
--- a/test/tester.c
+++ b/test/tester.c
@@ -2853,8 +2853,16 @@ START_TEST(test_volume_vpd_check)
F(rc, lsm_volume_vpd83_verify, "012345678901234567890123456789ag");
F(rc, lsm_volume_vpd83_verify, "1234567890123456789012345abcdef");
F(rc, lsm_volume_vpd83_verify, "01234567890123456789012345abcdefa");
-
- G(rc, lsm_volume_vpd83_verify, "01234567890123456789012345abcdef");
+ F(rc, lsm_volume_vpd83_verify, "01234567890123456789012345abcdef");
+ F(rc, lsm_volume_vpd83_verify, "55cd2e404beec32e0");
+ F(rc, lsm_volume_vpd83_verify, "55cd2e404beec32ex");
+ F(rc, lsm_volume_vpd83_verify, "55cd2e404beec32A");
+ F(rc, lsm_volume_vpd83_verify, "35cd2e404beec32A");
+
+ G(rc, lsm_volume_vpd83_verify, "61234567890123456789012345abcdef");
+ G(rc, lsm_volume_vpd83_verify, "55cd2e404beec32e");
+ G(rc, lsm_volume_vpd83_verify, "35cd2e404beec32e");
+ G(rc, lsm_volume_vpd83_verify, "25cd2e404beec32e");
}
END_TEST

[Gris Ge]

* Fix test_volume_vpd83_verify test.

* Replace _vpd_correct() with lsm.Volume.vpd83_verify().

Signed-off-by: Gris Ge <***@redhat.com>
---
test/plugin_test.py | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/test/plugin_test.py b/test/plugin_test.py
index 2147d43..df78243 100755
--- a/test/plugin_test.py
+++ b/test/plugin_test.py
@@ -321,12 +321,6 @@ def test_pools_list(self):
pools_list = self.c.pools()
self.assertTrue(len(pools_list) > 0, "We need at least 1 pool to test")

- @staticmethod
- def _vpd_correct(vpd):
- if vpd and re.match('^[a-f0-9]{32}$', vpd):
- return True
- return False
-
def _find_or_create_volumes(self):
"""
Find existing volumes, if not found, try to create one.
@@ -357,7 +351,7 @@ def test_volume_vpd83(self):
(volumes, flag_created) = self._find_or_create_volumes()
self.assertTrue(len(volumes) > 0, "We need at least 1 volume to test")
for v in volumes:
- self.assertTrue(TestPlugin._vpd_correct(v.vpd83),
+ self.assertTrue(lsm.Volume.vpd83_verify(v.vpd83),
"VPD is not as expected '%s' for volume id: '%s'" %
(v.vpd83, v.id))
if flag_created:
@@ -1122,13 +1116,21 @@ def test_volume_vpd83_verify(self):
"012345678901234567890123456789ax",
"012345678901234567890123456789ag",
"1234567890123456789012345abcdef",
- "01234567890123456789012345abcdefa"]
+ "01234567890123456789012345abcdefa",
+ "55cd2e404beec32e0", "55cd2e404beec32ex",
+ "35cd2e404beec32A"]

for f in failing:
self.assertFalse(lsm.Volume.vpd83_verify(f))

self.assertTrue(
- lsm.Volume.vpd83_verify("01234567890123456789012345abcdef"))
+ lsm.Volume.vpd83_verify("61234567890123456789012345abcdef"))
+ self.assertTrue(
+ lsm.Volume.vpd83_verify("55cd2e404beec32e"))
+ self.assertTrue(
+ lsm.Volume.vpd83_verify("35cd2e404beec32e"))
+ self.assertTrue(
+ lsm.Volume.vpd83_verify("25cd2e404beec32e"))

def test_available_plugins(self):
plugins = self.c.available_plugins(':')

[Gris Ge]

* Use library method to verify VPD 83 string.

* Remove unused constants from dmtf.py:
* VOL_NAME_FORMAT_OTHER
* VOL_NAME_FORMAT_EUI64
* VOL_NAME_FORMAT_T10VID
* VOL_NAME_SPACE_OTHER
* VOL_NAME_SPACE_VPD83_TYPE2
* VOL_NAME_SPACE_VPD83_TYPE1

Changes in V4 (this patch introduced in V3):

* Fix AttributeError when None.lower().

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/smispy/dmtf.py | 6 ------
plugin/smispy/smis_vol.py | 10 ++++++----
2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/plugin/smispy/dmtf.py b/plugin/smispy/dmtf.py
index c44db2f..f3fe91a 100644
--- a/plugin/smispy/dmtf.py
+++ b/plugin/smispy/dmtf.py
@@ -201,16 +201,10 @@ def op_status_list_conv(conv_dict, dmtf_op_status_list,
REPLICA_MODE_ASYNC = Uint16(3)

# CIM_StorageVolume['NameFormat']
-VOL_NAME_FORMAT_OTHER = 1
VOL_NAME_FORMAT_NNA = 9
-VOL_NAME_FORMAT_EUI64 = 10
-VOL_NAME_FORMAT_T10VID = 11

# CIM_StorageVolume['NameNamespace']
-VOL_NAME_SPACE_OTHER = 1
VOL_NAME_SPACE_VPD83_TYPE3 = 2
-VOL_NAME_SPACE_VPD83_TYPE2 = 3
-VOL_NAME_SPACE_VPD83_TYPE1 = 4

# CIM_ReplicationServiceCapabilities['SupportedAsynchronousActions']
# or CIM_ReplicationServiceCapabilities['SupportedSynchronousActions']
diff --git a/plugin/smispy/smis_vol.py b/plugin/smispy/smis_vol.py
index f8c727c..a2f15c9 100644
--- a/plugin/smispy/smis_vol.py
+++ b/plugin/smispy/smis_vol.py
@@ -158,8 +158,7 @@ def _vpd83_netapp(cim_vol):

def _vpd83_of_cim_vol(cim_vol):
"""
- Extract VPD83 string from CIMInstanceName and convert to LSM format:
- ^6[a-f0-9]{31}$
+ Extract VPD83 NAA string from CIMInstanceName and convert to LSM format.
"""
vpd_83 = _vpd83_in_cim_vol_name(cim_vol)
if vpd_83 is None:
@@ -167,8 +166,11 @@ def _vpd83_of_cim_vol(cim_vol):
if vpd_83 is None:
vpd_83 = _vpd83_netapp(cim_vol)

- if vpd_83 and re.match('^6[a-fA-F0-9]{31}$', vpd_83):
- return vpd_83.lower()
+ if vpd_83:
+ vpd_83 = vpd_83.lower()
+
+ if vpd_83 and Volume.vpd83_verify(vpd_83):
+ return vpd_83
else:
return ''

[Gris Ge]

* Fix test_volume_vpd83_verify test.

* Replace _vpd_correct() with lsm.Volume.vpd83_verify().

Signed-off-by: Gris Ge <***@redhat.com>
---
test/plugin_test.py | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/test/plugin_test.py b/test/plugin_test.py
index 2147d43..df78243 100755
--- a/test/plugin_test.py
+++ b/test/plugin_test.py
@@ -321,12 +321,6 @@ def test_pools_list(self):
pools_list = self.c.pools()
self.assertTrue(len(pools_list) > 0, "We need at least 1 pool to test")

- @staticmethod
- def _vpd_correct(vpd):
- if vpd and re.match('^[a-f0-9]{32}$', vpd):
- return True
- return False
-
def _find_or_create_volumes(self):
"""
Find existing volumes, if not found, try to create one.
@@ -357,7 +351,7 @@ def test_volume_vpd83(self):
(volumes, flag_created) = self._find_or_create_volumes()
self.assertTrue(len(volumes) > 0, "We need at least 1 volume to test")
for v in volumes:
- self.assertTrue(TestPlugin._vpd_correct(v.vpd83),
+ self.assertTrue(lsm.Volume.vpd83_verify(v.vpd83),
"VPD is not as expected '%s' for volume id: '%s'" %
(v.vpd83, v.id))
if flag_created:
@@ -1122,13 +1116,21 @@ def test_volume_vpd83_verify(self):
"012345678901234567890123456789ax",
"012345678901234567890123456789ag",
"1234567890123456789012345abcdef",
- "01234567890123456789012345abcdefa"]
+ "01234567890123456789012345abcdefa",
+ "55cd2e404beec32e0", "55cd2e404beec32ex",
+ "35cd2e404beec32A"]

for f in failing:
self.assertFalse(lsm.Volume.vpd83_verify(f))

self.assertTrue(
- lsm.Volume.vpd83_verify("01234567890123456789012345abcdef"))
+ lsm.Volume.vpd83_verify("61234567890123456789012345abcdef"))
+ self.assertTrue(
+ lsm.Volume.vpd83_verify("55cd2e404beec32e"))
+ self.assertTrue(
+ lsm.Volume.vpd83_verify("35cd2e404beec32e"))
+ self.assertTrue(
+ lsm.Volume.vpd83_verify("25cd2e404beec32e"))

def test_available_plugins(self):
plugins = self.c.available_plugins(':')