Nir Soffer has posted comments on this change. Change subject: JsonRPC: make Image.prepare to return info. ......................................................................
Patch Set 2: (5 comments) https://gerrit.ovirt.org/#/c/51054/2//COMMIT_MSG Commit Message: Line 12: Line 13: After preparing an image, the caller expects to get some valuable Line 14: information regarding it. Line 15: Line 16: This patch makes jsonRPC call to "Image.prepare" procedure to return jsonRPC - JSONRPC, procedure -> verb same for all other instances Line 17: the relevant info returned by calling this procedure via XMLRpc Line 18: Line 19: Change-Id: Icc03568b35c14e1f5c7239c8c3df8f74acf3ec51 Line 13: After preparing an image, the caller expects to get some valuable Line 14: information regarding it. Line 15: Line 16: This patch makes jsonRPC call to "Image.prepare" procedure to return Line 17: the relevant info returned by calling this procedure via XMLRpc same, XMLRpc -> XMLRPC Same for all other instances Line 18: Line 19: Change-Id: Icc03568b35c14e1f5c7239c8c3df8f74acf3ec51 https://gerrit.ovirt.org/#/c/51054/2/lib/api/vdsmapi-schema.json File lib/api/vdsmapi-schema.json: Line 5224: ## Line 5225: {'command': {'class': 'Image', 'name': 'prepare'}, Line 5226: 'data': {'storagepoolID': 'UUID', 'storagedomainID': 'UUID', Line 5227: 'imageID': 'UUID', 'volumeID': 'UUID'} Line 5228: 'returns': 'PrepareImageInfo'}} This should be a type specifying the return value, which need much more work, since it is nested structure. We don't have to promise anything that vdsm returns now, just the stuff that we consider as public and may be needed. I would also call this ImageInfo and not PrepareImageInfo, since this is information about an image, like VolumeInfo, which is information about a volume. Check the current return value and decide what is the minimal info that we like to make public. imaged and hosted engine agent/setup needs only the path now. In the future we may like to get the path of the lease for each volume in the chain, so external code can use sanlock leases, but currently we don't have such need, so we may defer this. Line 5229: Line 5230: ## Line 5231: # @Image.teardown: Line 5232: # https://gerrit.ovirt.org/#/c/51054/2/vdsm/rpc/Bridge.py File vdsm/rpc/Bridge.py: Line 393: """ Line 394: return {'truesize': ret['truesize'], 'apparentsize': ret['apparentsize']} Line 395: Line 396: Line 397: def Prepare_image_Ret(ret): Looking in other translators, this should be ClassName_methodName_Ret, so it should be named "Image_prepare_Ret". This convention is the most ugly I can imagine, but we should be consistent with the current names. Line 398: return {'path': ret['path'], 'info': ret['info'], Line 399: 'imgVolumesInfo': ret['imgVolumesInfo']} Line 400: Line 401: Line 395: Line 396: Line 397: def Prepare_image_Ret(ret): Line 398: return {'path': ret['path'], 'info': ret['info'], Line 399: 'imgVolumesInfo': ret['imgVolumesInfo']} Pleae format this one line per item, in the same order as the schema describe this type. Line 400: Line 401: Line 402: ## Line 403: # Possible ways to override a command: -- To view, visit https://gerrit.ovirt.org/51054 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icc03568b35c14e1f5c7239c8c3df8f74acf3ec51 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram <aavi...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Greg Padgett <gpadg...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Simone Tiraboschi <stira...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
