Re: [ovirt-devel] Vdsm sync 5/1 summary after short delay

On Sun, Jan 10, 2016 at 2:57 PM, Oved Ourfali <oourfali(a)redhat.com&gt; wrote: I don't remember that this was discussed here, and storage never approved this change for 3.6. We need to keep the xmlrpc option in 3.6, as a backup for jsonrpc issues. We just fixed couple of jsonrpc verbs that were returning True instead of the correct return value (caused by incorrect schema). - https://github.com/oVirt/vdsm/commit/0ca680700596564b4d6b0ef01ed4b0ae7c48... - https://gerrit.ovirt.org/40402 VM.getDiskAlignment cannot be used in jsonrpc However this is not the topic of the discussion, we are discussion the next version (3.7/4.0). We are adding lot of new verbs as part of removing the spm, and we don't want to invest time in adding xmlrpc and vdsClient support. Example new verb merged recently: https://github.com/oVirt/vdsm/commit/bbbb72a192d8b54d21c8d65f6a10278404a9... In the new verbs, we cleaned up the api, so integer values are passed as integers, not as strings. Previously we use to require strings since xmlrpc did not support large numbers (> 2**31 - 1). So in the schema, we require now a uint: +## +# @CreateVolumeInfo: +# ... +# @virtual_size: The Volume size in bytes ... +# @initial_size: #optional If specified, the initial allocated size of volume +# in bytes. Allowed only when creating a thinly provisioned +# volume on block storage. +# +# Since: 4.18 +## +{'type': 'CreateVolumeInfo', + 'data': {'sd_id': 'UUID', 'img_id': 'UUID', 'vol_id': 'UUID', + 'virtual_size': 'uint', 'vol_format': 'VolumeFormat', + 'disk_type': 'DiskType', 'description': 'str', + '*parent_img_id': 'UUID', '*parent_vol_id': 'UUID', + '*initial_size': 'uint'}} To support xmlrpc, we added this ugly code in bindingxmlrpc.py: + def sdm_create_volume(self, args): + validateArgTypes(args, [str, parse_json_obj]) + + # Convert large integers to strings. The server's xmlrpc binding will + # restore them to their proper int types. + vol_info = args[1] + for param in 'virtual_size', 'initial_size': + if param in vol_info: + vol_info[param] = str(vol_info[param]) + + res = self.s.sdm_create_volume(*args) + if res['status']['code']: + return res['status']['code'], res['status']['message'] + + return 0, '' + To support vdsClient, we added this: + def sdm_create_volume(self, job_id, vol_info): + sdm = API.SDM() + + # As a workaround for the 32bit signed integer limitation of xmlrpc, + # allow large integers to be passed as strings. We convert them back + # to the correct type here. + for param in 'virtual_size', 'initial_size': + if param in vol_info: + vol_info[param] = int(vol_info[param]) + + return sdm.create_volume(job_id, vol_info) + All this work is waste effort on our side. We should make a decision now - do we support xmlrpc in vdsm? I think we should not support it, after two version we support both xmlrpc and jsonrpc. If we stop supporting xmlrpc, we must have a replacement for vdsClient, We need to have commnad line client, both for development, and for sos plugin. I think this patch, owned now by Piotr, is the best direction: https://gerrit.ovirt.org/35181/ But we need to give this high priority, as having a command line client is a must for developing vdsm. Adding Aharon - supporting both xmlrpc and jsonrpc means we need to test everything twice, I don't think Aharion will like to do that. Piotr, Francesco, Dan, Adam: your thoughts?