Nir Soffer has posted comments on this change.
Change subject: jsonrpcvdscli: fix results of several commands
......................................................................
Patch Set 3:
(6 comments)
Unfortunately, this change the behavior in incompatible way. I did not check the schema,
but even if it describes the new format this change is wrong since all clients are
depending on the actual data returned from vdsm.
More: This is not a change in jsonrpsvdscli, but in the Bridge - it effects all jsonrpc
clients.
https://gerrit.ovirt.org/#/c/63483/3/lib/vdsm/rpc/Bridge.py
File lib/vdsm/rpc/Bridge.py:
Line 345
Line 346
Line 347
Line 348
Line 349
This returned before a list of pool...
Line 345: 'Host_fenceNode': {'ret': Host_fenceNode_Ret},
Line 346: 'Host_getAllTasksInfo': {'ret': 'allTasksInfo'},
Line 347: 'Host_getAllTasksStatuses': {'ret':
'allTasksStatus'},
Line 348: 'Host_getCapabilities': {'ret': Host_getCapabilities_Ret},
Line 349: 'Host_getConnectedStoragePools': {'ret':
remove_status_from_ret},
Now we get a dict with a 'poollist' key - schema conflict - and worse breaking all
client of this api, expecting list of pools.
Line 350: 'Host_getDeviceList': {'ret': 'devList'},
Line 351: 'Host_getDevicesVisibility': {'ret': 'visible'},
Line 352: 'Host_getExternalVMs': {'ret': 'vmList'},
Line 353: 'Host_getExternalVMNames': {'ret': 'vmNames'},
Line 355: 'Host_getConvertedVm': {'ret': 'ovf'},
Line 356: 'Host_getHardwareInfo': {'ret': 'info'},
Line 357: 'Host_getLVMVolumeGroups': {'ret': 'vglist'},
Line 358: 'Host_getStats': {'ret': 'info'},
Line 359: 'Host_getStorageDomains': {'ret': remove_status_from_ret},
Same
Line 360: 'Host_getStorageRepoStats': {'ret':
Host_getStorageRepoStats_Ret},
Line 361: 'Host_hostdevListByCaps': {'ret': 'deviceList'},
Line 362: 'Host_getVMList': {'call': Host_getVMList_Call,
Line 363: 'ret': remove_status_from_ret},
Line 380: 'ISCSIConnection_discoverSendTargets': {'ret':
'fullTargets'},
Line 381: 'LVMVolumeGroup_create': {'ret': 'uuid'},
Line 382: 'LVMVolumeGroup_getInfo': {'ret': 'info'},
Line 383: 'StorageDomain_getFileStats': {'ret': 'fileStats'},
Line 384: 'StorageDomain_getImages': {'ret': remove_status_from_ret},
Same
Line 385: 'StorageDomain_getInfo': {'ret': 'info'},
Line 386: 'StorageDomain_getStats': {'ret': 'stats'},
Line 387: 'StorageDomain_getVolumes': {'ret':
remove_status_from_ret},
Line 388: 'StorageDomain_resizePV': {'ret': 'size'},
Line 383: 'StorageDomain_getFileStats': {'ret': 'fileStats'},
Line 384: 'StorageDomain_getImages': {'ret': remove_status_from_ret},
Line 385: 'StorageDomain_getInfo': {'ret': 'info'},
Line 386: 'StorageDomain_getStats': {'ret': 'stats'},
Line 387: 'StorageDomain_getVolumes': {'ret':
remove_status_from_ret},
Same
Line 388: 'StorageDomain_resizePV': {'ret': 'size'},
Line 389: 'StoragePool_connectStorageServer': {'ret':
'statuslist'},
Line 390: 'StoragePool_disconnectStorageServer': {'ret':
'statuslist'},
Line 391: 'StoragePool_fence': {'ret': 'spm_st'},
Line 427: 'VM_updateDevice': {'ret': 'vmList'},
Line 428: 'Volume_copy': {'ret': 'uuid'},
Line 429: 'Volume_create': {'ret': 'uuid'},
Line 430: 'Volume_delete': {'ret': 'uuid'},
Line 431: 'Volume_getInfo': {'ret': remove_status_from_ret},
Same
Line 432: 'Volume_getPath': {'ret': 'path'},
Line 433: 'Volume_getSize': {'ret': Volume_getsize_Ret},
Line 434: 'Volume_extendSize': {'ret': 'uuid'},
Line 435: 'Host_getAllTasks': {'ret': 'tasks'},
--
To view, visit
https://gerrit.ovirt.org/63483
To unsubscribe, visit
https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f6b5862a69ea3d5140352fce7dda51bb95bab9c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes