Change in vdsm[master]: vdsm: Missing hostId parameter on reconstructMaster verb
by Nir Soffer
Nir Soffer has posted comments on this change.
Change subject: vdsm: Missing hostId parameter on reconstructMaster verb
......................................................................
Patch Set 1: Code-Review+1
(2 comments)
Just need more detailed commit message explaining the messs.
http://gerrit.ovirt.org/#/c/29510/1//COMMIT_MSG
Commit Message:
Line 6:
Line 7: vdsm: Missing hostId parameter on reconstructMaster verb
Line 8:
Line 9: Json schema was not consistent with API.py and there was missing
Line 10: hostId parameter.
Please describe that schema document the classes in API and the argument order is different then in the xmlrpc, which is *not* documented by the schema.
Line 11:
Line 12:
Line 13: Change-Id: I0695a21601b8f0765bc24c3cf273f1ba161a40a9
Line 14: Signed-off-by: pkliczewski <piotr.kliczewski(a)gmail.com>
http://gerrit.ovirt.org/#/c/29510/1/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 5234: # @StoragePool.reconstructMaster:
Line 5235: #
Line 5236: # Recover a Storage Pool by reconstructing its Storage Domains.
Line 5237: #
Line 5238: # @storagepoolID: The UUID of the Storage Pool
> As I can see in API.py:
You are right, our code is more horrible that I thought.
Line 5239: #
Line 5240: # @hostId: Host Id used by San lock.
Line 5241: #
Line 5242: # @name: A human-readable name for the Storage Pool
--
To view, visit http://gerrit.ovirt.org/29510
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0695a21601b8f0765bc24c3cf273f1ba161a40a9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9 years, 9 months
Change in vdsm[master]: vdsm: Missing hostId parameter on reconstructMaster verb
by Nir Soffer
Nir Soffer has posted comments on this change.
Change subject: vdsm: Missing hostId parameter on reconstructMaster verb
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.ovirt.org/#/c/29510/1/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 5234: # @StoragePool.reconstructMaster:
Line 5235: #
Line 5236: # Recover a Storage Pool by reconstructing its Storage Domains.
Line 5237: #
Line 5238: # @storagepoolID: The UUID of the Storage Pool
> You are correct - the API code is horrible.
Sadly this will not work - look at the xmlrpc code:
def poolReconstructMaster(self, spUUID, poolName, masterDom, domDict,
masterVersion, lockPolicy=None,
lockRenewalIntervalSec=None, leaseTimeSec=None,
ioOpTimeoutSec=None, leaseRetries=None,
hostId=None, options=None):
pool = API.StoragePool(spUUID)
return pool.reconstructMaster(
hostId, poolName, masterDom, masterVersion, domDict,
lockRenewalIntervalSec, leaseTimeSec, ioOpTimeoutSec, leaseRetries)
poolReconstructMaster accepts:
poolName, masterDom, domDict, masterVersion... hostId, options
But pool.reconstructMaster accepts:
hostId, poolName, masterDom, masterVersion, domDict... options
So if you send the arguments in the correct order according the schema, you cannot take the first argument out and call the poll.reconstructMaster with the rest of the arguments.
And the hostId argument is expected before the last options argument, so it cannot be first.
In short, you have to provide the same function as xmlrpc, converting poolReconstructMaster call to pool.reconstructMaster.
Or fix the call chain so arguments are passed in the same order.
This issue may exists in other calls too. Any verb that you did not explicitly checked may be broken.
Line 5239: #
Line 5240: # @hostId: Host Id used by San lock.
Line 5241: #
Line 5242: # @name: A human-readable name for the Storage Pool
--
To view, visit http://gerrit.ovirt.org/29510
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0695a21601b8f0765bc24c3cf273f1ba161a40a9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9 years, 9 months
Change in vdsm[master]: vdsm: Missing hostId parameter on reconstructMaster verb
by Nir Soffer
Nir Soffer has posted comments on this change.
Change subject: vdsm: Missing hostId parameter on reconstructMaster verb
......................................................................
Patch Set 1: Code-Review-1
It will not work - the order of the other parameters is wrong.
--
To view, visit http://gerrit.ovirt.org/29510
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0695a21601b8f0765bc24c3cf273f1ba161a40a9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
9 years, 9 months
Change in vdsm[master]: vdsm: Missing hostId parameter on reconstructMaster verb
by Nir Soffer
Nir Soffer has posted comments on this change.
Change subject: vdsm: Missing hostId parameter on reconstructMaster verb
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.ovirt.org/#/c/29510/1/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 5234: # @StoragePool.reconstructMaster:
Line 5235: #
Line 5236: # Recover a Storage Pool by reconstructing its Storage Domains.
Line 5237: #
Line 5238: # @storagepoolID: The UUID of the Storage Pool
> As I can see in API.py storagepoolID is constructor argument which is still
You are correct - the API code is horrible.
Line 5239: #
Line 5240: # @hostId: Host Id used by San lock.
Line 5241: #
Line 5242: # @name: A human-readable name for the Storage Pool
--
To view, visit http://gerrit.ovirt.org/29510
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0695a21601b8f0765bc24c3cf273f1ba161a40a9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9 years, 9 months
Change in vdsm[master]: vdsm: Missing hostId parameter on reconstructMaster verb
by Nir Soffer
Nir Soffer has posted comments on this change.
Change subject: vdsm: Missing hostId parameter on reconstructMaster verb
......................................................................
Patch Set 1: Code-Review-1
(2 comments)
http://gerrit.ovirt.org/#/c/29510/1/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:
Line 5234: # @StoragePool.reconstructMaster:
Line 5235: #
Line 5236: # Recover a Storage Pool by reconstructing its Storage Domains.
Line 5237: #
Line 5238: # @storagepoolID: The UUID of the Storage Pool
This should be removed - hostId should replace storagePoolID
Line 5239: #
Line 5240: # @hostId: Host Id used by San lock.
Line 5241: #
Line 5242: # @name: A human-readable name for the Storage Pool
Line 5264: #
Line 5265: # Since: 4.10.0
Line 5266: ##
Line 5267: {'command': {'class': 'StoragePool', 'name': 'reconstructMaster'},
Line 5268: 'data': {'storagepoolID': 'UUID', 'hostId': 'int', 'name': 'str',
Remove storagePoolID
Line 5269: 'masterSdUUID': 'UUID', 'masterVersion': 'int',
Line 5270: 'domainDict': 'StorageDomainStatusMap',
Line 5271: 'lockRenewalIntervalSec': 'int', 'leaseTimeSec': 'int',
Line 5272: 'ioOpTimeoutSec': 'int', 'leaseRetries': 'int'}}}
--
To view, visit http://gerrit.ovirt.org/29510
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0695a21601b8f0765bc24c3cf273f1ba161a40a9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
9 years, 9 months
