Freddy Rolland has posted comments on this change.

Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................


Patch Set 2:

(10 comments)

https://gerrit.ovirt.org/#/c/45093/2/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:

Line 1477
Line 1478
Line 1479
Line 1480
Line 1481
> In another patch, fix this version to 4.17.0.
OK


Line 1416:  'data': {'connection': 'str', 'port': 'str', 'iqn': 'str', 
'portal': 'str',
Line 1417:           'initiatorname': 'str', '*username': 'str', '*password': 
'str'}}
Line 1418: 
Line 1419: ##
Line 1420: # @BlockDeviceStatus:
> Thanks nice addition!
I added the reasons I know that pvcreate will fail.
Line 1421: #
Line 1422: # Enumeration of possible status for a block device.
Line 1423: #
Line 1424: # @free:  The device is free


Line 1424: # @free:  The device is free
Line 1425: #
Line 1426: # @used:  The device is in use
Line 1427: #
Line 1428: # @unknown:  The device is in use
> The device status is unknown
I will ask Dan and update in another patch
Line 1429: #
Line 1430: # Since: 4.10.0
Line 1431: ##
Line 1432: {'enum': 'BlockDeviceStatus', 'data': ['free', 'used', 'unknown']}


Line 1493: # Get information about all block devices.
Line 1494: #
Line 1495: # @storageType:  #optional Only return devices of this type
Line 1496: #
Line 1497: # @checkStatus: #optional Indicates if device status should be 
checked
> Align with previous line
Done
Line 1498: #                 (new in version 4.17)
Line 1499: #
Line 1500: # @guids:        #optional Only return info on specific list of 
block device
Line 1501: #                 GUIDs  (new in version 4.17)


Line 1494: #
Line 1495: # @storageType:  #optional Only return devices of this type
Line 1496: #
Line 1497: # @checkStatus: #optional Indicates if device status should be 
checked
Line 1498: #                 (new in version 4.17)
> Version should be more specific, check with Dan what will be the next versi
I will ask Dan and update in another patch
Line 1499: #
Line 1500: # @guids:        #optional Only return info on specific list of 
block device
Line 1501: #                 GUIDs  (new in version 4.17)
Line 1502: #


https://gerrit.ovirt.org/#/c/45093/2/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 2026
Line 2027
Line 2028
Line 2029
Line 2030
> Add 'status': 'unknown' here
Done


Line 1957:                                       masterVersion, leaseParams)
Line 1958: 
Line 1959:     @public
Line 1960:     def getDeviceList(self, storageType=None, checkStatus=True,
Line 1961:                       guids=(), options={}):
> Are you sure guids=() does not fit in the previous line?
Done
Line 1962:         """
Line 1963:         List all Block Devices.
Line 1964: 
Line 1965:         :param storageType: Filter by storage type.


Line 1963:         List all Block Devices.
Line 1964: 
Line 1965:         :param storageType: Filter by storage type.
Line 1966:         :type storageType: Some enum?
Line 1967:         :param testPvCreate: Test PV creation.
> Please update the name here and elsewhere.
Done
Line 1968:         :type bool: if true PV creation will be tested for the given 
devices
Line 1969:         :param guids: List of device GUIDs to retrieve info.
Line 1970:         :type guids: list
Line 1971:         :param options: ?


Line 1964: 
Line 1965:         :param storageType: Filter by storage type.
Line 1966:         :type storageType: Some enum?
Line 1967:         :param testPvCreate: Test PV creation.
Line 1968:         :type bool: if true PV creation will be tested for the given 
devices
> if True, device status will be checked.  Please do not promise how we check
Done
Line 1969:         :param guids: List of device GUIDs to retrieve info.
Line 1970:         :type guids: list
Line 1971:         :param options: ?
Line 1972: 


Line 2052:                     raise KeyError("pvcreate response foresight is "
Line 2053:                                    "can not be determined for %s", 
dev)
Line 2054:         else:
Line 2055:             for dev in devices:
Line 2056:                 dev['status'] = "unknown"
> This works, but ugly - better initialize status to 'unknown' when the dev d
Done
Line 2057: 
Line 2058:         return devices
Line 2059: 
Line 2060:     @public


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic28954708f2fd7c7b721aa7f9a0fb6e1a6019597
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <[email protected]>
Gerrit-Reviewer: Fred Rolland <[email protected]>
Gerrit-Reviewer: Freddy Rolland <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to