Fix sg_inq parsing the WWN during the device discovery.

[tzurE]

Internal ticket UB-1030.

Validate the WWN of the discovered device done by sg_inq. But we have a bug there that the parsing sg_inq output is not working well for XIV because different outline of the sg_inq output.

(was found while testing Ubiquity on zLinux)

---

This change is 

[coveralls]

Coverage remained the same at 54.731% when pulling ea8d059 on fix/sg_inq_parsing_zlinux into 7c17ff8 on dev.

[shay-berman]

Review status: 0 of 2 files reviewed at latest revision, all discussions resolved.

---

remote/mounter/block_device_utils/block_device_utils_test.go, line 145 at r1 (raw file):

			Expect(args).To(Equal([]string{"-p",  "0x83", "/dev/mapper/mpath"}))
		})
                It("Discover returns path for volume on z systems", func() {

Tzur
there is no need to add unit test for Discover function, since the discover is one function above GetWwnByScsiInq.
Instead you need to do the unit test only on the function GetWwnByScsiInq.

---

remote/mounter/block_device_utils/mpath.go, line 188 at r1 (raw file):

		return "", b.logger.ErrorRet(err, "failed")
	}
	pattern := "(?i)" + "Vendor Specific (Identifier|Extension) (Identifier|Extension):"

please add a comment in the code here - that zlinux sg_inq return "Vendor Specific Extension Identifier" while x86 return "Vendor Specific Identifier Extension"
so it will be clear why we improved the regex

---

Comments from Reviewable

[alonm]

remote/mounter/block_device_utils/mpath.go, line 188 at r1 (raw file):

		return "", b.logger.ErrorRet(err, "failed")
	}
	pattern := "(?i)" + "Vendor Specific (Identifier|Extension) (Identifier|Extension):"

The following would be more accurate "Vendor Specific (Identifier Extension|Extension Identifier):"

---

Comments from Reviewable

[tzurE]

Review status: 0 of 2 files reviewed at latest revision, 3 unresolved discussions.

---

remote/mounter/block_device_utils/block_device_utils_test.go, line 145 at r1 (raw file):

Previously, shay-berman wrote…

Tzur
there is no need to add unit test for Discover function, since the discover is one function above GetWwnByScsiInq.
Instead you need to do the unit test only on the function GetWwnByScsiInq.

I think it should be added in both. But I can move it to GetWwnByScsiInq if you say it's better there.

---

remote/mounter/block_device_utils/mpath.go, line 188 at r1 (raw file):

Previously, shay-berman wrote…

please add a comment in the code here - that zlinux sg_inq return "Vendor Specific Extension Identifier" while x86 return "Vendor Specific Identifier Extension"
so it will be clear why we improved the regex

done

---

remote/mounter/block_device_utils/mpath.go, line 188 at r1 (raw file):

Previously, alonm wrote…

The following would be more accurate "Vendor Specific (Identifier Extension|Extension Identifier):"

Thought of that a bit later. you are correct of course, it'll prevent us from receiving false lines.

---

Comments from Reviewable

[alonm]

---

Reviewed 2 of 2 files at r1.
Review status: 0 of 2 files reviewed at latest revision, 3 unresolved discussions.

---

Comments from Reviewable

[shay-berman]

NOTE :
we should merge to dev only if this issue related also impact XIV system.
So please wait with the merging until testing finish. If its not related to XIV, we will merge it after GA 1.1.

---

Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[shay-berman]

Updated the title of the PR and the description according to the finding today. We should merge it tomorrow to dev branch after sanity test.