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 <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to