9:24 a.m.
Hello Fred Rolland,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/45093
to review the following change.
Change subject: hsm: Support testPvCreate param in getDeviceList
......................................................................
hsm: Support testPvCreate param in getDeviceList
Add optional 'testPvCreate' boolean parameter to getDeviceList.
By default it will be True to keep same behavior as before.
If specified as False, the PV create test will be skipped and the
'status' field will not be populated.
Examples:
getDeviceList
return all devices
getDeviceList FCP
return only FCP devices
getDeviceList FCP True
return only FCP devices and perform PV create test
getDeviceList ISCSI False guid1 guid2
return info for guid1 and guid2, assuming ISCSI type
without performing PV create test
Change-Id: Ic28954708f2fd7c7b721aa7f9a0fb6e1a6019597
Bug-Url: https://bugzilla.redhat.com/1217401
Signed-off-by: Fred Rolland <frolland(a)redhat.com>
---
M client/vdsClient.py
M vdsm/API.py
M vdsm/rpc/bindingxmlrpc.py
M vdsm/rpc/vdsmapi-schema.json
M vdsm/storage/hsm.py
5 files changed, 46 insertions(+), 27 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/93/45093/1
diff --git a/client/vdsClient.py b/client/vdsClient.py
index ce40974..c62321d 100755
--- a/client/vdsClient.py
+++ b/client/vdsClient.py
@@ -732,8 +732,11 @@
def getDeviceList(self, args):
if len(args) == 0:
res = self.s.getDeviceList()
+ elif len(args) == 1:
+ res = self.s.getDeviceList(args[0])
else:
- res = self.s.getDeviceList(args[0], args[1:])
+ res = self.s.getDeviceList(args[0],
+ utils.tobool(args[1]), args[2:])
if res['status']['code']:
return res['status']['code'],
res['status']['message']
@@ -2277,16 +2280,22 @@
)),
'getDeviceList': (serv.getDeviceList,
('[storageType]',
+ '[testPvCreate]',
'[<devlist>]',
'List of all block devices (optionally - matching '
- 'storageType, optionally - of each device listed).',
+ 'storageType, optionally - perform PV create test '
+ 'optionally - of each device listed).',
' getDeviceList',
' return all devices',
' getDeviceList FCP',
' return only FCP devices',
- ' getDeviceList ISCSI guid1 guid2',
+ ' getDeviceList FCP True',
+ ' return only FCP devices and perform ',
+ ' PV creation test',
+ ' getDeviceList ISCSI False guid1 guid2',
' return info for guid1 and guid2',
- ' , assuming ISCSI type'
+ ' , assuming ISCSI type and skip PV ',
+ ' creation test'
)),
'getDevicesVisibility': (serv.getDevicesVisibility,
('<devlist>',
diff --git a/vdsm/API.py b/vdsm/API.py
index 66822a5..bbc3f03 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -1679,8 +1679,8 @@
def getLVMVolumeGroups(self, storageType=None):
return self._irs.getVGList(storageType)
- def getDeviceList(self, storageType=None, guids=()):
- return self._irs.getDeviceList(storageType, guids)
+ def getDeviceList(self, storageType=None, testPvCreate=True, guids=()):
+ return self._irs.getDeviceList(storageType, testPvCreate, guids)
def getDevicesVisibility(self, guidList):
return self._irs.getDevicesVisibility(guidList)
diff --git a/vdsm/rpc/bindingxmlrpc.py b/vdsm/rpc/bindingxmlrpc.py
index 7f1da4f..b23643b 100644
--- a/vdsm/rpc/bindingxmlrpc.py
+++ b/vdsm/rpc/bindingxmlrpc.py
@@ -1029,9 +1029,10 @@
api = API.Global()
return api.getLVMVolumeGroups(storageType)
- def devicesGetList(self, storageType=None, guids=(), options=None):
+ def devicesGetList(self, storageType=None, testPvCreate=True,
+ guids=(), options=None):
api = API.Global()
- res = api.getDeviceList(storageType, guids)
+ res = api.getDeviceList(storageType, testPvCreate, guids)
return unprotect_passwords(res)
def devicesGetVisibility(self, guids, options=None):
diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json
index 4cac5ae..42b9c91 100644
--- a/vdsm/rpc/vdsmapi-schema.json
+++ b/vdsm/rpc/vdsmapi-schema.json
@@ -1477,6 +1477,9 @@
#
# @storageType: #optional Only return devices of this type
#
+# @testPvCreate: #optional Indicates if PV creation test should be performed
+# (new in version 4.17)
+#
# @guids: #optional Only return info on specific list of block device
# GUIDs (new in version 4.17)
#
@@ -1487,6 +1490,7 @@
##
{'command': {'class': 'Host', 'name':
'getDeviceList'},
'data': {'*storageType': 'BlockDeviceType',
+ '*testPvCreate': 'bool',
'*guids': ['str']},
'returns': ['BlockDeviceInfo']}
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index 0806abb..b5e2b7f 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -1957,12 +1957,15 @@
masterVersion, leaseParams)
@public
- def getDeviceList(self, storageType=None, guids=(), options={}):
+ def getDeviceList(self, storageType=None, testPvCreate=True,
+ guids=(), options={}):
"""
List all Block Devices.
:param storageType: Filter by storage type.
:type storageType: Some enum?
+ :param testPvCreate: Test PV creation.
+ :type bool: if true PV creation will be tested for the given devices
:param guids: List of device GUIDs to retrieve info.
:type guids: list
:param options: ?
@@ -1972,10 +1975,11 @@
:rtype: dict
"""
vars.task.setDefaultException(se.BlockDeviceActionError())
- devices = self._getDeviceList(storageType=storageType, guids=guids)
+ devices = self._getDeviceList(storageType=storageType,
+ testPvCreate=testPvCreate, guids=guids)
return dict(devList=devices)
- def _getDeviceList(self, storageType=None, guids=()):
+ def _getDeviceList(self, storageType=None, testPvCreate=True, guids=()):
sdCache.refreshStorage()
typeFilter = lambda dev: True
if storageType:
@@ -2030,22 +2034,23 @@
'physicalblocksize':
dev.get("physicalblocksize", "")}
devices.append(devInfo)
- # Look for devices that will probably fail if pvcreated.
- devNamesToPVTest = tuple(dev["GUID"] for dev in devices)
- unusedDevs, usedDevs = lvm.testPVCreate(
- devNamesToPVTest, metadataSize=blockSD.VG_METADATASIZE)
- # Assuming that unusables v unusables = None
- free = tuple(os.path.basename(d) for d in unusedDevs)
- used = tuple(os.path.basename(d) for d in usedDevs)
- for dev in devices:
- guid = dev['GUID']
- if guid in free:
- dev['status'] = "free"
- elif guid in used:
- dev['status'] = "used"
- else:
- raise KeyError("pvcreate response foresight is "
- "can not be determined for %s", dev)
+ if testPvCreate:
+ # Look for devices that will probably fail if pvcreated.
+ devNamesToPVTest = tuple(dev["GUID"] for dev in devices)
+ unusedDevs, usedDevs = lvm.testPVCreate(
+ devNamesToPVTest, metadataSize=blockSD.VG_METADATASIZE)
+ # Assuming that unusables v unusables = None
+ free = tuple(os.path.basename(d) for d in unusedDevs)
+ used = tuple(os.path.basename(d) for d in usedDevs)
+ for dev in devices:
+ guid = dev['GUID']
+ if guid in free:
+ dev['status'] = "free"
+ elif guid in used:
+ dev['status'] = "used"
+ else:
+ raise KeyError("pvcreate response foresight is "
+ "can not be determined for %s", dev)
return devices
--
To view, visit https://gerrit.ovirt.org/45093
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic28954708f2fd7c7b721aa7f9a0fb6e1a6019597
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
9:24 a.m.
automation(a)ovirt.org has posted comments on this change.
Change subject: hsm: Support testPvCreate param in getDeviceList
......................................................................
Patch Set 1:
* Update tracker::#1217401::OK
* Check Bug-Url::OK
* Check Public Bug::#1217401::OK, public bug
* Check Product::#1217401::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])
--
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: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
12:57 p.m.
Nir Soffer has posted comments on this change.
Change subject: hsm: Support testPvCreate param in getDeviceList
......................................................................
Patch Set 1:
(8 comments)
https://gerrit.ovirt.org/#/c/45093/1//COMMIT_MSG
Commit Message:
Line 6:
Line 7: hsm: Support testPvCreate param in getDeviceList
Line 8:
Line 9: Add optional 'testPvCreate' boolean parameter to getDeviceList.
Line 10: By default it will be True to keep same behavior as before.
Please explain why this is needed, and provide some measurements how this effects the user
experience on a system with lot of devices.
Line 11:
Line 12: If specified as False, the PV create test will be skipped and the
Line 13: 'status' field will not be populated.
Line 14:
https://gerrit.ovirt.org/#/c/45093/1/client/vdsClient.py
File client/vdsClient.py:
Line 735: elif len(args) == 1:
Line 736: res = self.s.getDeviceList(args[0])
Line 737: else:
Line 738: res = self.s.getDeviceList(args[0],
Line 739: utils.tobool(args[1]), args[2:])
Check if utils.tobool(args[1]) can be on the first line, this indentation looks strange.
Line 740:
Line 741: if res['status']['code']:
Line 742: return res['status']['code'],
res['status']['message']
Line 743: pp.pprint(res['devList'])
Line 2282: ('[storageType]',
Line 2283: '[testPvCreate]',
Line 2284: '[<devlist>]',
Line 2285: 'List of all block devices (optionally -
matching '
Line 2286: 'storageType, optionally - perform PV create
test '
update name
Line 2287: 'optionally - of each device listed).',
Line 2288: ' getDeviceList',
Line 2289: ' return all devices',
Line 2290: ' getDeviceList FCP',
https://gerrit.ovirt.org/#/c/45093/1/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 1487
Line 1488
Line 1489
Line 1490
Line 1491
This type should be updated now, as status is optional, or it may return third value when
status was not checked.
Line 1477: #
Line 1478: # @storageType: #optional Only return devices of this type
Line 1479: #
Line 1480: # @testPvCreate: #optional Indicates if PV creation test should be performed
Line 1481: # (new in version 4.17)
Update to describe what this does instead of how.
Line 1482: #
Line 1483: # @guids: #optional Only return info on specific list of block device
Line 1484: # GUIDs (new in version 4.17)
Line 1485: #
https://gerrit.ovirt.org/#/c/45093/1/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:
Line 1956: return pool.reconstructMaster(hostId, poolName, masterDom, domDict,
Line 1957: masterVersion, leaseParams)
Line 1958:
Line 1959: @public
Line 1960: def getDeviceList(self, storageType=None, testPvCreate=True,
This is too specific, what we want is to check the device status, so lets call this
parameter checkStatus (or another name that explain what it does, and not how it does
it).
Line 1961: guids=(), options={}):
Line 1962: """
Line 1963: List all Block Devices.
Line 1964:
Line 1978: devices = self._getDeviceList(storageType=storageType,
Line 1979: testPvCreate=testPvCreate, guids=guids)
Line 1980: return dict(devList=devices)
Line 1981:
Line 1982: def _getDeviceList(self, storageType=None, testPvCreate=True, guids=()):
Update to new parameter name.
Line 1983: sdCache.refreshStorage()
Line 1984: typeFilter = lambda dev: True
Line 1985: if storageType:
Line 1986: if sd.storageType(storageType) == sd.type2name(sd.ISCSI_DOMAIN):
Line 2046: guid = dev['GUID']
Line 2047: if guid in free:
Line 2048: dev['status'] = "free"
Line 2049: elif guid in used:
Line 2050: dev['status'] = "used"
This will not report device status now when new engine call with checkStatus=False, so the
schema should be update to explain this behavior.
Or, you can report dev["status"] = "unknown", which simplifies the
schema, and probably makes the client code simpler.
Line 2051: else:
Line 2052: raise KeyError("pvcreate response foresight is "
Line 2053: "can not be determined for %s",
dev)
Line 2054:
--
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: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
8:16 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
automation(a)ovirt.org has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 2:
* Update tracker::#1217401::OK
* Check Bug-Url::OK
* Check Public Bug::#1217401::OK, public bug
* Check Product::#1217401::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])
--
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 <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
8:29 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Freddy Rolland has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 2: Verified+1
--
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 <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
8:29 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Freddy Rolland has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 1:
(7 comments)
https://gerrit.ovirt.org/#/c/45093/1/client/vdsClient.py
File client/vdsClient.py:
Line 735: elif len(args) == 1:
Line 736: res = self.s.getDeviceList(args[0])
Line 737: else:
Line 738: res = self.s.getDeviceList(args[0],
Line 739: utils.tobool(args[1]), args[2:])
Check if utils.tobool(args[1]) can be on the first line, this
indentation l
Done
Line 740:
Line 741: if res['status']['code']:
Line 742: return res['status']['code'],
res['status']['message']
Line 743: pp.pprint(res['devList'])
Line 2282: ('[storageType]',
Line 2283: '[testPvCreate]',
Line 2284: '[<devlist>]',
Line 2285: 'List of all block devices (optionally -
matching '
Line 2286: 'storageType, optionally - perform PV create
test '
update name
Done
Line 2287: 'optionally - of each device listed).',
Line 2288: ' getDeviceList',
Line 2289: ' return all devices',
Line 2290: ' getDeviceList FCP',
https://gerrit.ovirt.org/#/c/45093/1/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 1487
Line 1488
Line 1489
Line 1490
Line 1491
This type should be updated now, as status is optional, or it may
return th
I added a description for status enum that was missing, and added a third
value
Line 1477: #
Line 1478: # @storageType: #optional Only return devices of this type
Line 1479: #
Line 1480: # @testPvCreate: #optional Indicates if PV creation test should be performed
Line 1481: # (new in version 4.17)
Update to describe what this does instead of how.
Done
Line 1482: #
Line 1483: # @guids: #optional Only return info on specific list of block device
Line 1484: # GUIDs (new in version 4.17)
Line 1485: #
https://gerrit.ovirt.org/#/c/45093/1/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:
Line 1956: return pool.reconstructMaster(hostId, poolName, masterDom, domDict,
Line 1957: masterVersion, leaseParams)
Line 1958:
Line 1959: @public
Line 1960: def getDeviceList(self, storageType=None, testPvCreate=True,
This is too specific, what we want is to check the device status, so
lets c
Done
Line 1961: guids=(), options={}):
Line 1962: """
Line 1963: List all Block Devices.
Line 1964:
Line 1978: devices = self._getDeviceList(storageType=storageType,
Line 1979: testPvCreate=testPvCreate, guids=guids)
Line 1980: return dict(devList=devices)
Line 1981:
Line 1982: def _getDeviceList(self, storageType=None, testPvCreate=True, guids=()):
Update to new parameter name.
Done
Line 1983: sdCache.refreshStorage()
Line 1984: typeFilter = lambda dev: True
Line 1985: if storageType:
Line 1986: if sd.storageType(storageType) == sd.type2name(sd.ISCSI_DOMAIN):
Line 2046: guid = dev['GUID']
Line 2047: if guid in free:
Line 2048: dev['status'] = "free"
Line 2049: elif guid in used:
Line 2050: dev['status'] = "used"
This will not report device status now when new engine call with
checkStatu
Done
Line 2051: else:
Line 2052: raise KeyError("pvcreate response foresight is "
Line 2053: "can not be determined for %s",
dev)
Line 2054:
--
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: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
9:26 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Nir Soffer has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 2: Code-Review-1
(11 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.
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!
But the text does not explain anything. We must be very specific what is the meaning of
"used".
I think that "used" means devices recognized by lvm as pvs (containing a pv
header), but maybe I'm wrong and we only pvs which are part of a vg are considered
used - please check the underlying implementation to understand the behavior of the
current code.
Additionally, I think that devides with a partition table are considered as used.
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
(New in version 4.17.?)
Check with Dan which version should appear here.
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
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 version where this
patch should be available.
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
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?
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.
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 this in the
documentation, so we don't have to change it if we change the implementation.
Also explain that checkStatus is an expensive operation and it should be used only with
specific devices using the guids argument.
Also explain why the default is True (we just explained why it should be actually False).
Line 1969: :param guids: List of device GUIDs to retrieve info.
Line 1970: :type guids: list
Line 1971: :param options: ?
Line 1972:
Line 2037: if checkStatus:
Line 2038: # Look for devices that will probably fail if pvcreated.
Line 2039: devNamesToPVTest = tuple(dev["GUID"] for dev in devices)
Line 2040: unusedDevs, usedDevs = lvm.testPVCreate(
Line 2041: devNamesToPVTest, metadataSize=blockSD.VG_METADATASIZE)
It seems that this is only a partial fix, since it we seem to call pvCreate on devices
which are known pvs (has pv uuid) and maybe even part of a vg (have vg uuid).
If we change the logic to check only devices which are not pvs (lvm does not recognize
them as pvs), we can get nice performance improvement for older
engine without changing the engine side.
Lets tackle this problem in another patch.
Line 2042: # Assuming that unusables v unusables = None
Line 2043: free = tuple(os.path.basename(d) for d in unusedDevs)
Line 2044: used = tuple(os.path.basename(d) for d in usedDevs)
Line 2045: for dev in devices:
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 dict is
created.
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 <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
4:47 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
automation(a)ovirt.org has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 3:
* Update tracker::#1217401::OK
* Check Bug-Url::OK
* Check Public Bug::#1217401::OK, public bug
* Check Product::#1217401::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])
--
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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
6:34 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Nir Soffer has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 3: Code-Review-1
(4 comments)
https://gerrit.ovirt.org/#/c/45093/3/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 1426: # @used: The device is in use.
Line 1427: # A device will be considered used on the following cases:
Line 1428: # - a PV exists on this device and is part of a VG
Line 1429: # - a partition table is found on the device
Line 1430: # - The device is not accessible
This does not make sense - why do we consider inaccessible device as used?
Line 1431: #
Line 1432: # @unknown: The device status is unknown
Line 1433: # (new in version 4.17)
Line 1434: #
Line 1429: # - a partition table is found on the device
Line 1430: # - The device is not accessible
Line 1431: #
Line 1432: # @unknown: The device status is unknown
Line 1433: # (new in version 4.17)
Did you check if old engine can handle this response?
Line 1434: #
Line 1435: # Since: 4.10.0
Line 1436: ##
Line 1437: {'enum': 'BlockDeviceStatus', 'data': ['free',
'used', 'unknown']}
https://gerrit.ovirt.org/#/c/45093/3/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:
Line 1967: :param checkStatus: Check the status of the devices. This operation is
Line 1968: an expensive operation and should be used only
with
Line 1969: specific devices using the guids argument.
Line 1970: The default is True for backward compatibility.
Line 1971: :type bool: if true the status will be checked for the given devices.
:type checkStatus: bool
:type ...: sections should not include documentation, only type names. The line "if
true..." should be part of the :param ...: section.
Line 1972: :param guids: List of device GUIDs to retrieve info.
Line 1973: :type guids: list
Line 1974: :param options: ?
Line 1975:
Line 2023: pvsize = ""
Line 2024: vguuid = ""
Line 2025:
Line 2026: devInfo = {'GUID': dev.get("guid", ""),
'pvUUID': pvuuid,
Line 2027: 'status': 'unknown',
This may break old engine - please check that old engine can cope with this value.
For example, if engine look only for "used" or "free", it will treat
"unknown" as the other value, which may be correct or not.
Line 2028: 'pvsize': str(pvsize),
Line 2029: 'vgUUID': vguuid, 'vendorID':
dev.get("vendor", ""),
Line 2030: 'productID': dev.get("product",
""),
Line 2031: 'fwrev': dev.get("fwrev",
""),
--
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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
7:28 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
automation(a)ovirt.org has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 4:
* Update tracker::#1217401::OK
* Check Bug-Url::OK
* Check Public Bug::#1217401::OK, public bug
* Check Product::#1217401::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])
--
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: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
7:28 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Freddy Rolland has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 3:
(4 comments)
https://gerrit.ovirt.org/#/c/45093/3/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 1426: # @used: The device is in use.
Line 1427: # A device will be considered used on the following cases:
Line 1428: # - a PV exists on this device and is part of a VG
Line 1429: # - a partition table is found on the device
Line 1430: # - The device is not accessible
This does not make sense - why do we consider inaccessible device as
used?
Pvcreate will fail as in the example below.
Do we have a way to filter these out ?
# pvcreate --test /dev/mapper/36001405d37fe1b74e554233915d2c45e
TEST MODE: Metadata will NOT be updated and volumes will not be (de)activated.
/dev/mapper/36001405d37fe1b74e554233915d2c45e: read failed after 0 of 4096 at 0:
Input/output error
/dev/mapper/36001405d37fe1b74e554233915d2c45e: read failed after 0 of 4096 at
22548512768: Input/output error
/dev/mapper/36001405d37fe1b74e554233915d2c45e: read failed after 0 of 4096 at
22548570112: Input/output error
/dev/mapper/36001405d37fe1b74e554233915d2c45e: read failed after 0 of 4096 at 4096:
Input/output error
Can't open /dev/mapper/36001405d37fe1b74e554233915d2c45e exclusively. Mounted
filesystem?
Line 1431: #
Line 1432: # @unknown: The device status is unknown
Line 1433: # (new in version 4.17)
Line 1434: #
Line 1429: # - a partition table is found on the device
Line 1430: # - The device is not accessible
Line 1431: #
Line 1432: # @unknown: The device status is unknown
Line 1433: # (new in version 4.17)
Did you check if old engine can handle this response?
Old
engine will not get this value as it will call getDeviceList without checkStatus. Since
the default of thecheckStatus is True, it will be only used/free.
Line 1434: #
Line 1435: # Since: 4.10.0
Line 1436: ##
Line 1437: {'enum': 'BlockDeviceStatus', 'data': ['free',
'used', 'unknown']}
https://gerrit.ovirt.org/#/c/45093/3/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:
Line 1967: :param checkStatus: Check the status of the devices. This operation is
Line 1968: an expensive operation and should be used only
with
Line 1969: specific devices using the guids argument.
Line 1970: The default is True for backward compatibility.
Line 1971: :type bool: if true the status will be checked for the given devices.
:type checkStatus: bool
Done
Line 1972: :param guids: List of device GUIDs to retrieve info.
Line 1973: :type guids: list
Line 1974: :param options: ?
Line 1975:
Line 2023: pvsize = ""
Line 2024: vguuid = ""
Line 2025:
Line 2026: devInfo = {'GUID': dev.get("guid", ""),
'pvUUID': pvuuid,
Line 2027: 'status': 'unknown',
This may break old engine - please check that old engine can cope
with this
Since old engine will always have "checkstatus",
'unknown' will not be returned.
Line 2028: 'pvsize': str(pvsize),
Line 2029: 'vgUUID': vguuid, 'vendorID':
dev.get("vendor", ""),
Line 2030: 'productID': dev.get("product",
""),
Line 2031: 'fwrev': dev.get("fwrev",
""),
--
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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
4:28 p.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Nir Soffer has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 3:
(3 comments)
https://gerrit.ovirt.org/#/c/45093/3/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 1426: # @used: The device is in use.
Line 1427: # A device will be considered used on the following cases:
Line 1428: # - a PV exists on this device and is part of a VG
Line 1429: # - a partition table is found on the device
Line 1430: # - The device is not accessible
Pvcreate will fail as in the example below.
"Can't
open /dev/mapper/36001405d37fe1b74e554233915d2c45e exclusively. Mounted filesystem?"
does not mean disk is not accessible. It means that the disk may have a mounted file
system, so lets update the text to describe this situation.
Line 1431: #
Line 1432: # @unknown: The device status is unknown
Line 1433: # (new in version 4.17)
Line 1434: #
https://gerrit.ovirt.org/#/c/45093/3/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:
Line 2026
Line 2027
Line 2028
Line 2029
Line 2030
For example, we can add the status here:
if not checkStatus:
devInfo["status"] = "unknown"
Line 2023: pvsize = ""
Line 2024: vguuid = ""
Line 2025:
Line 2026: devInfo = {'GUID': dev.get("guid", ""),
'pvUUID': pvuuid,
Line 2027: 'status': 'unknown',
Since old engine will always have "checkstatus",
'unknown' will not be retu
Correct, but I wonder if we should make it more
clear like by adding "status": "unknown" only if checkStatus is
False.
Line 2028: 'pvsize': str(pvsize),
Line 2029: 'vgUUID': vguuid, 'vendorID':
dev.get("vendor", ""),
Line 2030: 'productID': dev.get("product",
""),
Line 2031: 'fwrev': dev.get("fwrev",
""),
--
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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
4:31 p.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Nir Soffer has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 4: Code-Review-1
(1 comment)
https://gerrit.ovirt.org/#/c/45093/4//COMMIT_MSG
Commit Message:
Line 22: sys 0m0.028s
Line 23:
Line 24: real 0m57.769s
Line 25: user 0m0.265s
Line 26: sys 0m0.024s
Use spaces, tabs are formatted differently in the shell and in the editor. The only place
we use tabs in makefiles rules (tabs are required there).
Line 27:
Line 28: Examples:
Line 29: getDeviceList
Line 30: return all devices
--
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: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
3:15 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
automation(a)ovirt.org has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 5:
* Update tracker::#1217401::OK
* Check Bug-Url::OK
* Check Public Bug::#1217401::OK, public bug
* Check Product::#1217401::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])
--
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: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
3:15 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Freddy Rolland has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 4:
(1 comment)
https://gerrit.ovirt.org/#/c/45093/4//COMMIT_MSG
Commit Message:
Line 22: sys 0m0.028s
Line 23:
Line 24: real 0m57.769s
Line 25: user 0m0.265s
Line 26: sys 0m0.024s
Use spaces, tabs are formatted differently in the shell and in the
editor.
Done
Line 27:
Line 28: Examples:
Line 29: getDeviceList
Line 30: return all devices
--
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: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
3:15 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Freddy Rolland has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 3:
(2 comments)
https://gerrit.ovirt.org/#/c/45093/3/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:
Line 2026
Line 2027
Line 2028
Line 2029
Line 2030
For example, we can add the status here:
Done
Line 2023: pvsize = ""
Line 2024: vguuid = ""
Line 2025:
Line 2026: devInfo = {'GUID': dev.get("guid", ""),
'pvUUID': pvuuid,
Line 2027: 'status': 'unknown',
Correct, but I wonder if we should make it more clear like by adding
"statu
Done
Line 2028: 'pvsize': str(pvsize),
Line 2029: 'vgUUID': vguuid, 'vendorID':
dev.get("vendor", ""),
Line 2030: 'productID': dev.get("product",
""),
Line 2031: 'fwrev': dev.get("fwrev",
""),
--
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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
3:15 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
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 <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
5:23 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Nir Soffer has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 5: Code-Review-1
(1 comment)
https://gerrit.ovirt.org/#/c/45093/5/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 1426: # @used: The device is in use.
Line 1427: # A device will be considered used on the following cases:
Line 1428: # - a PV exists on this device and is part of a VG
Line 1429: # - a partition table is found on the device
Line 1430: # - The device may have a mounted system
mounted file system
Line 1431: #
Line 1432: # @unknown: The device status is unknown
Line 1433: # (new in version 4.17)
Line 1434: #
--
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: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
5:52 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Freddy Rolland has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 5:
(1 comment)
https://gerrit.ovirt.org/#/c/45093/5/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 1426: # @used: The device is in use.
Line 1427: # A device will be considered used on the following cases:
Line 1428: # - a PV exists on this device and is part of a VG
Line 1429: # - a partition table is found on the device
Line 1430: # - The device may have a mounted system
mounted file system
Done
Line 1431: #
Line 1432: # @unknown: The device status is unknown
Line 1433: # (new in version 4.17)
Line 1434: #
--
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: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
5:52 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
automation(a)ovirt.org has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 6:
* Update tracker::#1217401::OK
* Check Bug-Url::OK
* Check Public Bug::#1217401::OK, public bug
* Check Product::#1217401::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])
--
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
6:25 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Nir Soffer has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 6:
(1 comment)
Looks good, but the commit is missing the big picture.
https://gerrit.ovirt.org/#/c/45093/6//COMMIT_MSG
Commit Message:
Line 22: sys 0m0.028s
Line 23:
Line 24: real 0m57.769s
Line 25: user 0m0.265s
Line 26: sys 0m0.024s
This does not explain the flow, and the reader may think we are checking any more the
device status because it is too slow, while we do check the status.
The motivation for this patch is avoiding paying for the expensive test when we don't
have to.
The flow before this patch was:
- system get device list, perform expensive status check for each device, possibly times
out
- user select couple of devices (typically one)
- system warn user about used devices
The flow with this patch is:
- system get device list, skipping status check
- user select couple of devices (typically one)
- system perform status check for select devices
- system warn user about used devices
Line 27:
Line 28: Examples:
Line 29: getDeviceList
Line 30: return all devices
--
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
5:38 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
automation(a)ovirt.org has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 7:
* Update tracker::#1217401::OK
* Check Bug-Url::OK
* Check Public Bug::#1217401::OK, public bug
* Check Product::#1217401::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])
--
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: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
5:54 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Freddy Rolland has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 6:
(1 comment)
https://gerrit.ovirt.org/#/c/45093/6//COMMIT_MSG
Commit Message:
Line 22: sys 0m0.028s
Line 23:
Line 24: real 0m57.769s
Line 25: user 0m0.265s
Line 26: sys 0m0.024s
This does not explain the flow, and the reader may think we are
checking an
Done
Line 27:
Line 28: Examples:
Line 29: getDeviceList
Line 30: return all devices
--
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
6:08 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Nir Soffer has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 7: Code-Review-1
(3 comments)
Looks good except the backward incompatibility caused by the new parameter, should be
trivial fix.
https://gerrit.ovirt.org/#/c/45093/7/client/vdsClient.py
File client/vdsClient.py:
Line 735: elif len(args) == 1:
Line 736: res = self.s.getDeviceList(args[0])
Line 737: else:
Line 738: res = self.s.getDeviceList(args[0], utils.tobool(args[1]),
Line 739: args[2:])
guids be be sent before checkStatus - xmlrpc is using positional arguments.
The vdsclient api is not backward compatible, but it is not supported api, and nobody is
using it, so there is no problem with that.
The current vdsClient is pure junk. In the future we will have better user interface using
standard options that will avoid this ordering issue. For example, all this will work:
vdsctl device list --check-status
vdsctl device list guid1 guid2 --check-status guid3 ...
vdsctl device list --device-type=FCP --check-status guid ...
vdsctl device list -t=FCP -c guid ...
Line 740:
Line 741: if res['status']['code']:
Line 742: return res['status']['code'],
res['status']['message']
Line 743: pp.pprint(res['devList'])
https://gerrit.ovirt.org/#/c/45093/7/vdsm/rpc/bindingxmlrpc.py
File vdsm/rpc/bindingxmlrpc.py:
Line 1029: api = API.Global()
Line 1030: return api.getLVMVolumeGroups(storageType)
Line 1031:
Line 1032: def devicesGetList(self, storageType=None, checkStatus=True,
Line 1033: guids=(), options=None):
There is one more isssue - you added the new parameter in a non-backward compatible way,
so once this patch is merged, xmlrpc is broken with current engine, since the xmlrpc
request is using positional args. When current engine will call this with guids list, the
guides will end up in checkStatus and API will get an empty guid list.
If we are unlucky, we will release this vdsm before engine patch is merged, and this error
will experienced by real users.
We cannot add new apis in this way; the api must be backward compatible.
The fix:
- Move checkStatus after guids here and in all other calls to keep the call uniform - we
don't like to have different parameter order in various parts of the code base.
- Keep vdsclient api as is, but call the arguments in the correct order
see my comment in vdsClient
Line 1034: api = API.Global()
Line 1035: res = api.getDeviceList(storageType, checkStatus, guids)
Line 1036: return unprotect_passwords(res)
Line 1037:
https://gerrit.ovirt.org/#/c/45093/7/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 1499: #
Line 1500: # @storageType: #optional Only return devices of this type
Line 1501: #
Line 1502: # @checkStatus: #optional Indicates if device status should be checked
Line 1503: # (new in version 4.17)
Must be after guids for backward compatibility.
Line 1504: #
Line 1505: # @guids: #optional Only return info on specific list of block device
Line 1506: # GUIDs (new in version 4.17)
Line 1507: #
--
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: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
6:57 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
automation(a)ovirt.org has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 8:
* Update tracker::#1217401::OK
* Check Bug-Url::OK
* Check Public Bug::#1217401::OK, public bug
* Check Product::#1217401::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])
--
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: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
6:57 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Freddy Rolland has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 7:
(3 comments)
https://gerrit.ovirt.org/#/c/45093/7/client/vdsClient.py
File client/vdsClient.py:
Line 735: elif len(args) == 1:
Line 736: res = self.s.getDeviceList(args[0])
Line 737: else:
Line 738: res = self.s.getDeviceList(args[0], utils.tobool(args[1]),
Line 739: args[2:])
guids be be sent before checkStatus - xmlrpc is using positional
arguments.
Done
Line 740:
Line 741: if res['status']['code']:
Line 742: return res['status']['code'],
res['status']['message']
Line 743: pp.pprint(res['devList'])
https://gerrit.ovirt.org/#/c/45093/7/vdsm/rpc/bindingxmlrpc.py
File vdsm/rpc/bindingxmlrpc.py:
Line 1029: api = API.Global()
Line 1030: return api.getLVMVolumeGroups(storageType)
Line 1031:
Line 1032: def devicesGetList(self, storageType=None, checkStatus=True,
Line 1033: guids=(), options=None):
There is one more isssue - you added the new parameter in a
non-backward co
Done
Line 1034: api = API.Global()
Line 1035: res = api.getDeviceList(storageType, checkStatus, guids)
Line 1036: return unprotect_passwords(res)
Line 1037:
https://gerrit.ovirt.org/#/c/45093/7/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 1499: #
Line 1500: # @storageType: #optional Only return devices of this type
Line 1501: #
Line 1502: # @checkStatus: #optional Indicates if device status should be checked
Line 1503: # (new in version 4.17)
Must be after guids for backward compatibility.
Done
Line 1504: #
Line 1505: # @guids: #optional Only return info on specific list of block device
Line 1506: # GUIDs (new in version 4.17)
Line 1507: #
--
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: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
10:34 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Nir Soffer has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 8: Code-Review-1
(4 comments)
Few minor fixes needed.
https://gerrit.ovirt.org/#/c/45093/8/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 1448
Line 1449
Line 1450
Line 1451
Line 1452
4.17.x - in another patch
Line 1477
Line 1478
Line 1479
Line 1480
Line 1481
4.17.x
Align with the #:
#optional
GUIDs ...
Line 1429: # - a partition table is found on the device
Line 1430: # - The device may have a mounted file system
Line 1431: #
Line 1432: # @unknown: The device status is unknown
Line 1433: # (new in version 4.17)
4.17.3
Line 1434: #
Line 1435: # Since: 4.10.0
Line 1436: ##
Line 1437: {'enum': 'BlockDeviceStatus', 'data': ['free',
'used', 'unknown']}
Line 1502: # @guids: #optional Only return info on specific list of block device
Line 1503: # GUIDs (new in version 4.17)
Line 1504: #
Line 1505: # @checkStatus: #optional Indicates if device status should be checked
Line 1506: # (new in version 4.17)
4.17.x
Same
Line 1507: #
Line 1508: # Returns:
Line 1509: # An array of @BlockDeviceInfo
Line 1510: #
--
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: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
10:58 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
automation(a)ovirt.org has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 9:
* Update tracker::#1217401::OK
* Check Bug-Url::OK
* Check Public Bug::#1217401::OK, public bug
* Check Product::#1217401::OK, Correct product Red Hat Enterprise Virtualization Manager
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])
--
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: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
10:58 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Freddy Rolland has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 8:
(2 comments)
https://gerrit.ovirt.org/#/c/45093/8/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 1429: # - a partition table is found on the device
Line 1430: # - The device may have a mounted file system
Line 1431: #
Line 1432: # @unknown: The device status is unknown
Line 1433: # (new in version 4.17)
4.17.3
Done
Line 1434: #
Line 1435: # Since: 4.10.0
Line 1436: ##
Line 1437: {'enum': 'BlockDeviceStatus', 'data': ['free',
'used', 'unknown']}
Line 1502: # @guids: #optional Only return info on specific list of block device
Line 1503: # GUIDs (new in version 4.17)
Line 1504: #
Line 1505: # @checkStatus: #optional Indicates if device status should be checked
Line 1506: # (new in version 4.17)
4.17.x
Done
Line 1507: #
Line 1508: # Returns:
Line 1509: # An array of @BlockDeviceInfo
Line 1510: #
--
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: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
10:58 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Freddy Rolland has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 9: Verified+1
--
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: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
12:53 p.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Nir Soffer has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 9: Code-Review+2
--
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: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
5:24 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Allon Mureinik has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 9: Code-Review+1
--
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: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
10:58 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Freddy Rolland has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 9:
With 200 devices on targetcli:
# time vdsClient -s 0 getDeviceList Iscsi False >/dev/null
real 0m5.525s
user 0m0.413s
sys 0m0.041s
# time vdsClient -s 0 getDeviceList Iscsi True >/dev/null
real 6m9.429s
user 0m0.438s
sys 0m0.021s
--
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: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
10:36 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Dan Kenigsberg has submitted this change and it was merged.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
hsm: Support checkStatus param in getDeviceList
In order to populate the 'status' field, the getDeviceList
verb perform a create PV test. This operation is expensive, and in
setups with large number of devices it will cause performance issue.
However, the 'status' field is not always needed. This patch
add an optional 'checkStatus' boolean parameter to getDeviceList.
By default it will be True to keep same behavior as before.
If specified as False, the PV create test will be skipped and the
'status' field will be populated as 'unknown'.
The flow before this patch was:
- Engine calls getDeviceList, VDSM perform status check for each
devices, possibly times out
- User select a few devices (typically one)
- Engine warn user about used devices
The flow with this patch:
- Engine calls getDeviceList, skipping status check
- User select a few devices (typically one)
- Engine calls getDeviceList, checking status only for selected
- Engine warn user about used devices
On a setup of 100 ISCSI devices, the command took 3s without
the PV create test against 58s with the test.
real 0m2.987s
user 0m0.263s
sys 0m0.028s
real 0m57.769s
user 0m0.265s
sys 0m0.024s
Examples:
getDeviceList
return all devices
getDeviceList FCP
return only FCP devices
getDeviceList FCP True
return only FCP devices and perform PV create test
getDeviceList ISCSI False guid1 guid2
return info for guid1 and guid2, assuming ISCSI type
without performing PV create test
Change-Id: Ic28954708f2fd7c7b721aa7f9a0fb6e1a6019597
Bug-Url: https://bugzilla.redhat.com/1217401
Signed-off-by: Fred Rolland <frolland(a)redhat.com>
Reviewed-on: https://gerrit.ovirt.org/45093
Continuous-Integration: Jenkins CI
Reviewed-by: Nir Soffer <nsoffer(a)redhat.com>
Reviewed-by: Allon Mureinik <amureini(a)redhat.com>
---
M client/vdsClient.py
M vdsm/API.py
M vdsm/rpc/bindingxmlrpc.py
M vdsm/rpc/vdsmapi-schema.json
M vdsm/storage/hsm.py
5 files changed, 77 insertions(+), 29 deletions(-)
Approvals:
Nir Soffer: Looks good to me, approved
Jenkins CI: Passed CI tests
Allon Mureinik: Looks good to me, but someone else must approve
Freddy Rolland: Verified
--
To view, visit https://gerrit.ovirt.org/45093
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic28954708f2fd7c7b721aa7f9a0fb6e1a6019597
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
10:36 a.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
automation(a)ovirt.org has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 10:
* Update tracker::#1217401::OK
* Set MODIFIED::bug 1217401::::#1217401::::IGNORE, not oVirt prod but Red Hat Enterprise
Virtualization Manager
--
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: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
6:59 p.m.
New subject: Change in vdsm[master]: hsm: Support checkStatus param in getDeviceList
Nir Soffer has posted comments on this change.
Change subject: hsm: Support checkStatus param in getDeviceList
......................................................................
Patch Set 10:
Fred, please backport this to 3.6.
Next time add also:
Backport-To: 3.6
--
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: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
3182
days inactive
3198
days old
vdsm-patches@lists.fedorahosted.org
35 comments
5 participants
participants (5)
-
Allon Mureinik -
automation＠ovirt.org -
Dan Kenigsberg -
frolland＠redhat.com -
Nir Soffer