Change in vdsm[master]: storage: Add path cap. & PV size to getDeviceList

nsoffer at redhat.com nsoffer at redhat.com
Fri Mar 13 12:18:08 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: storage: Add path cap. & PV size to getDeviceList
......................................................................


Patch Set 1:

(4 comments)

Code is fine, commit message should give less details more context.

https://gerrit.ovirt.org/#/c/38675/1//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2015-03-12 10:40:43 +0200
Line 4: Commit:     Fred Rolland <frolland at redhat.com>
Line 5: CommitDate: 2015-03-12 17:33:21 +0200
Line 6: 
Line 7: storage: Add path cap. & PV size to getDeviceList
- No shortcuts in title - avoid unneeded details instead.
- "storage:" is too general.

How about:

    hsm: Report information needed for refreshing devices
Line 8: 
Line 9: Adding capacity of path in the path status section in
Line 10: getDeviceList.
Line 11: 


Line 20: 
Line 21: See output:
Line 22:   'productID': 'fred2',
Line 23:   'pvUUID': 'KsX3rW-UcXk-G4iO-TQUT-WFHe-9eP2-n0T1A0',
Line 24:   'pvsize': '53687091200',
It is not clear where these keys are - we need more context like you show for pathStatus.
Line 25: 
Line 26: This information is important to discover difference between
Line 27: the LUN size, path size and the PV size.
Line 28: 


Line 23:   'pvUUID': 'KsX3rW-UcXk-G4iO-TQUT-WFHe-9eP2-n0T1A0',
Line 24:   'pvsize': '53687091200',
Line 25: 
Line 26: This information is important to discover difference between
Line 27: the LUN size, path size and the PV size.
- "is important" -> "is required"
- "discover difference between the LUN size, path size and the PV size" -> "discover devices that were resized on the storage server and need to be resized on the host."
Line 28: 
Line 29: Relates-To: https://bugzilla.redhat.com/609689
Line 30: Change-Id: Icfa5cf4321202362f5a00dad7c9a0347ba52ec71


Line 24:   'pvsize': '53687091200',
Line 25: 
Line 26: This information is important to discover difference between
Line 27: the LUN size, path size and the PV size.
Line 28: 
This describes mostly the changes, which we can see in the code, but
we need to document the reason for this change.

Please describe why we need both pv size and paths size and why we
want to detect changes in device size.

Showing the new output is nice, but it would be nicer as complete 
device info in getDeviceList output (maybe with unrelated lines
stripped).
Line 29: Relates-To: https://bugzilla.redhat.com/609689
Line 30: Change-Id: Icfa5cf4321202362f5a00dad7c9a0347ba52ec71


-- 
To view, visit https://gerrit.ovirt.org/38675
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icfa5cf4321202362f5a00dad7c9a0347ba52ec71
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list