[Libstoragemgmt-devel] [PATCH] Targetd Plugin: Fix vpd83 of targetd volume

[Gris Ge]

* Just convert UUID(vpd 0x80) to WWID(vpd 0x83) base on the LIO kernel code.

Signed-off-by: Gris Ge <***@redhat.com>
---
plugin/targetd/targetd.py | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/plugin/targetd/targetd.py b/plugin/targetd/targetd.py
index 26537a5..a39d7b9 100644
--- a/plugin/targetd/targetd.py
+++ b/plugin/targetd/targetd.py
@@ -29,6 +29,7 @@ import json
import time
import urlparse
import socket
+import re

DEFAULT_USER = "admin"
DEFAULT_PORT = 18700
@@ -157,14 +158,32 @@ class TargetdStorage(IStorageAreaNetwork, INfs):
def job_free(self, job_id, flags=0):
raise LsmError(ErrorNumber.NO_SUPPORT, "Not supported")

+ @staticmethod
+ def _lio_uuid_to_vpd83(uuid):
+ """
+ This convension is based on linux kernel code:
+ drivers/target/target_core_spc.c:
+ spc_emulate_evpd_83()
+ spc_parse_naa_6h_vendor_specific()
+ Convert UUID (VPD 0x80) to VPD 0x83 32 hex digitals.
+ """
+ vpd83 = '6001405'
+ vpd83 += re.sub(r"""[^a-f0-9]""", '', uuid.lower())[0:25]
+ print vpd83
+ # Fill up with zero.
+ if len(vpd83) <= 32:
+ vpd83 += '0' * ( 32 - len(vpd83))
+ return vpd83
+
@handle_errors
def volumes(self, search_key=None, search_value=None, flags=0):
volumes = []
for p_name in (p['name'] for p in self._jsonrequest("pool_list") if
p['type'] == 'block'):
for vol in self._jsonrequest("vol_list", dict(pool=p_name)):
+ vpd83 = TargetdStorage._lio_uuid_to_vpd83(vol['uuid'])
volumes.append(
- Volume(vol['uuid'], vol['name'], vol['uuid'], 512,
+ Volume(vol['uuid'], vol['name'], vpd83, 512,
long(vol['size'] / 512),
Volume.ADMIN_STATE_ENABLED,
self.system.id, p_name))

[Andy Grover]

Post by Gris Ge
* Just convert UUID(vpd 0x80) to WWID(vpd 0x83) base on the LIO kernel code.

Better commit msg please. See below. This is where you could point to
where you got the conversion method from, not in a src comment.

Post by Gris Ge
---
plugin/targetd/targetd.py | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/plugin/targetd/targetd.py b/plugin/targetd/targetd.py
index 26537a5..a39d7b9 100644
--- a/plugin/targetd/targetd.py
+++ b/plugin/targetd/targetd.py
@@ -29,6 +29,7 @@ import json
import time
import urlparse
import socket
+import re
DEFAULT_USER = "admin"
DEFAULT_PORT = 18700
raise LsmError(ErrorNumber.NO_SUPPORT, "Not supported")

Nothing else in the file has "lio" prefix, remove. In fact, this uuid
comes from LVM, not LIO.

Post by Gris Ge
+ """
+ spc_emulate_evpd_83()
+ spc_parse_naa_6h_vendor_specific()
+ Convert UUID (VPD 0x80) to VPD 0x83 32 hex digitals.

"convention". "digits".

Don't refer to other code, although you can use the comments there as a
basis for the comments here.

Post by Gris Ge
+ """
+ vpd83 = '6001405'

See above, you can reference specs but not other code to document where
this number comes from, because what if the other code changes?

Post by Gris Ge
+ vpd83 += re.sub(r"""[^a-f0-9]""", '', uuid.lower())[0:25]

[0:25] can be replaced with [:25]

You shouldn't need triple quotes, or even the r, since you're not using
regexp escape values.

If all this is just to get rid of the dashes, maybe a:
uuid.replace("-", "").lower() is all you need, and you don't even need
re module.

Post by Gris Ge
+ print vpd83

delete this

Post by Gris Ge
+ # Fill up with zero.

<= ? Shouldn't this be < ?

Although you don't even need the if, because the code in the conditional
still does the right thing in all cases.

Post by Gris Ge
+ vpd83 += '0' * ( 32 - len(vpd83))

No whitespace between ( and 32.

Post by Gris Ge
+ return vpd83
+
@handle_errors
volumes = []
for p_name in (p['name'] for p in self._jsonrequest("pool_list") if
+ vpd83 = TargetdStorage._lio_uuid_to_vpd83(vol['uuid'])
volumes.append(
- Volume(vol['uuid'], vol['name'], vol['uuid'], 512,
+ Volume(vol['uuid'], vol['name'], vpd83, 512,
long(vol['size'] / 512),
Volume.ADMIN_STATE_ENABLED,
self.system.id, p_name))

HTH -- Regards -- Andy

[Tony Asleson]

Gris,

Did you test this to make sure that the value you return matches the
values in /dev/disk/by-id/scsi-<...> ?

Thanks!

Regards,
Tony

Post by Andy Grover

Post by Gris Ge
* Just convert UUID(vpd 0x80) to WWID(vpd 0x83) base on the LIO kernel code.

Better commit msg please. See below. This is where you could point to
where you got the conversion method from, not in a src comment.

Post by Gris Ge
---
plugin/targetd/targetd.py | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/plugin/targetd/targetd.py b/plugin/targetd/targetd.py
index 26537a5..a39d7b9 100644
--- a/plugin/targetd/targetd.py
+++ b/plugin/targetd/targetd.py
@@ -29,6 +29,7 @@ import json
import time
import urlparse
import socket
+import re
DEFAULT_USER = "admin"
DEFAULT_PORT = 18700
raise LsmError(ErrorNumber.NO_SUPPORT, "Not supported")

Nothing else in the file has "lio" prefix, remove. In fact, this uuid
comes from LVM, not LIO.

Post by Gris Ge
+ """
+ spc_emulate_evpd_83()
+ spc_parse_naa_6h_vendor_specific()
+ Convert UUID (VPD 0x80) to VPD 0x83 32 hex digitals.

"convention". "digits".
Don't refer to other code, although you can use the comments there as a
basis for the comments here.

Post by Gris Ge
+ """
+ vpd83 = '6001405'

See above, you can reference specs but not other code to document where
this number comes from, because what if the other code changes?

Post by Gris Ge
+ vpd83 += re.sub(r"""[^a-f0-9]""", '', uuid.lower())[0:25]

[0:25] can be replaced with [:25]
You shouldn't need triple quotes, or even the r, since you're not using
regexp escape values.
uuid.replace("-", "").lower() is all you need, and you don't even need
re module.

Post by Gris Ge
+ print vpd83

delete this

Post by Gris Ge
+ # Fill up with zero.

<= ? Shouldn't this be < ?
Although you don't even need the if, because the code in the conditional
still does the right thing in all cases.

Post by Gris Ge
+ vpd83 += '0' * ( 32 - len(vpd83))

No whitespace between ( and 32.

Post by Gris Ge
+ return vpd83
+
@handle_errors
volumes = []
for p_name in (p['name'] for p in self._jsonrequest("pool_list") if
+ vpd83 = TargetdStorage._lio_uuid_to_vpd83(vol['uuid'])
volumes.append(
- Volume(vol['uuid'], vol['name'], vol['uuid'], 512,
+ Volume(vol['uuid'], vol['name'], vpd83, 512,
long(vol['size'] / 512),
Volume.ADMIN_STATE_ENABLED,
self.system.id, p_name))

HTH -- Regards -- Andy
------------------------------------------------------------------------------
Slashdot TV.
Video for Nerds. Stuff that matters.
http://tv.slashdot.org/
_______________________________________________
Libstoragemgmt-devel mailing list
https://lists.sourceforge.net/lists/listinfo/libstoragemgmt-devel

[Gris Ge]

Post by Tony Asleson
Gris,
Did you test this to make sure that the value you return matches the
values in /dev/disk/by-id/scsi-<...> ?
Thanks!
Regards,
Tony

Hi Tony,

I tested with commands:
sg_vpd --page=0x83 /dev/sda
and:
udevadm info --name=sda --query=all |grep "ID_SERIAL="

Best regards.

[Gris Ge]

Post by Andy Grover

Post by Gris Ge
* Just convert UUID(vpd 0x80) to WWID(vpd 0x83) base on the LIO kernel code.

Better commit msg please. See below. This is where you could point to
where you got the conversion method from, not in a src comment.

Thanks. I will submit a updated patch for this base on your comments and
help.

Best regards.

Post by Andy Grover
HTH -- Regards -- Andy