<p dir="ltr"><br>
On Jan 10, 2016 3:37 PM, "Nir Soffer" <<a href="mailto:nsoffer@redhat.com">nsoffer@redhat.com</a>> wrote:<br>
><br>
> On Sun, Jan 10, 2016 at 2:57 PM, Oved Ourfali <<a href="mailto:oourfali@redhat.com">oourfali@redhat.com</a>> wrote:<br>
> > Thanks for the summary!<br>
> > See one comment inline.<br>
> ><br>
> > On Sun, Jan 10, 2016 at 2:49 PM, Yaniv Bronheim <<a href="mailto:ybronhei@redhat.com">ybronhei@redhat.com</a>> wrote:<br>
> >><br>
> >><br>
> >> (fromani, nsoffer, ybronhei, alitke)<br>
> >><br>
> >> - Removing xmlrpc for good - who should accept it? where do we stand with<br>
> >> full jsonrpc client ? (we didn't get to any conclusions and said that we'll<br>
> >> reraise this topic next week with pioter)<br>
> >><br>
> ><br>
> > With regards to that, in order to move to 3.6 cluster level, you MUST have<br>
> > all hosts in jsonrpc protocol. So, we just need to make sure no piece of<br>
> > code uses that explicitly, and if so move that to jsonrpc as well.<br>
><br>
> I don't remember that this was discussed here, and storage never<br>
> approved this change<br>
> for 3.6. We need to keep the xmlrpc option in 3.6, as a backup for<br>
> jsonrpc issues.<br>
></p>
<p dir="ltr">No we don't. And we won't. We had it around for one version for this reason, but no need for more. <br>
We had a bug about that, and communicated it to whomever is relevant. <br>
We want to get rid of it entirely in 4.0, so 3.6 cluster won't work with it. <br>
If you know of any issue then please fix it now, or open a bug about it. </p>
<p dir="ltr">> We just fixed couple of jsonrpc verbs that were returning True instead of<br>
> the correct return value (caused by incorrect schema).<br>
> - <a href="https://github.com/oVirt/vdsm/commit/0ca680700596564b4d6b0ef01ed4b0ae7c488de7">https://github.com/oVirt/vdsm/commit/0ca680700596564b4d6b0ef01ed4b0ae7c488de7</a><br>
> - <a href="https://gerrit.ovirt.org/40402">https://gerrit.ovirt.org/40402</a> VM.getDiskAlignment cannot be used in jsonrpc<br>
></p>
<p dir="ltr">That's great. As said earlier, if you know of others please open bugs. </p>
<p dir="ltr">> However this is not the topic of the discussion, we are discussion the next<br>
> version (3.7/4.0). We are adding lot of new verbs as part of removing the spm,<br>
> and we don't want to invest time in adding xmlrpc and vdsClient support.<br>
><br>
> Example new verb merged recently:<br>
> <a href="https://github.com/oVirt/vdsm/commit/bbbb72a192d8b54d21c8d65f6a10278404a966db">https://github.com/oVirt/vdsm/commit/bbbb72a192d8b54d21c8d65f6a10278404a966db</a><br>
><br>
> In the new verbs, we cleaned up the api, so integer values are passed<br>
> as integers,<br>
> not as strings. Previously we use to require strings since xmlrpc did<br>
> not support large<br>
> numbers (> 2**31 - 1).<br>
><br>
> So in the schema, we require now a uint:<br>
><br>
> +##<br>
> +# @CreateVolumeInfo:<br>
> +#<br>
> ...<br>
> +# @virtual_size: The Volume size in bytes<br>
> ...<br>
> +# @initial_size: #optional If specified, the initial allocated size of volume<br>
> +# in bytes. Allowed only when creating a thinly provisioned<br>
> +# volume on block storage.<br>
> +#<br>
> +# Since: 4.18<br>
> +##<br>
> +{'type': 'CreateVolumeInfo',<br>
> + 'data': {'sd_id': 'UUID', 'img_id': 'UUID', 'vol_id': 'UUID',<br>
> + 'virtual_size': 'uint', 'vol_format': 'VolumeFormat',<br>
> + 'disk_type': 'DiskType', 'description': 'str',<br>
> + '*parent_img_id': 'UUID', '*parent_vol_id': 'UUID',<br>
> + '*initial_size': 'uint'}}<br>
><br>
> To support xmlrpc, we added this ugly code in bindingxmlrpc.py:<br>
><br>
> + def sdm_create_volume(self, args):<br>
> + validateArgTypes(args, [str, parse_json_obj])<br>
> +<br>
> + # Convert large integers to strings. The server's xmlrpc binding will<br>
> + # restore them to their proper int types.<br>
> + vol_info = args[1]<br>
> + for param in 'virtual_size', 'initial_size':<br>
> + if param in vol_info:<br>
> + vol_info[param] = str(vol_info[param])<br>
> +<br>
> + res = self.s.sdm_create_volume(*args)<br>
> + if res['status']['code']:<br>
> + return res['status']['code'], res['status']['message']<br>
> +<br>
> + return 0, ''<br>
> +<br>
><br>
> To support vdsClient, we added this:<br>
><br>
> + def sdm_create_volume(self, job_id, vol_info):<br>
> + sdm = API.SDM()<br>
> +<br>
> + # As a workaround for the 32bit signed integer limitation of xmlrpc,<br>
> + # allow large integers to be passed as strings. We convert them back<br>
> + # to the correct type here.<br>
> + for param in 'virtual_size', 'initial_size':<br>
> + if param in vol_info:<br>
> + vol_info[param] = int(vol_info[param])<br>
> +<br>
> + return sdm.create_volume(job_id, vol_info)<br>
> +<br>
><br>
> All this work is waste effort on our side.<br>
><br>
> We should make a decision now - do we support xmlrpc in vdsm?<br>
><br>
> I think we should not support it, after two version we support both<br>
> xmlrpc and jsonrpc.</p>
<p dir="ltr">I agree. But again, 3.6 clusters will require jsonrpc. That's why in 4.0 xmlrpc will no longer be relevant. </p>
<p dir="ltr">><br>
> If we stop supporting xmlrpc, we must have a replacement for vdsClient,<br>
><br>
> We need to have commnad line client, both for development, and for sos<br>
> plugin. I think this patch, owned now by Piotr, is the best direction:<br>
> <a href="https://gerrit.ovirt.org/35181/">https://gerrit.ovirt.org/35181/</a><br>
><br>
> But we need to give this high priority, as having a command line client is<br>
> a must for developing vdsm.<br>
><br>
> Adding Aharon - supporting both xmlrpc and jsonrpc means we need to test<br>
> everything twice, I don't think Aharion will like to do that.<br>
><br>
> Piotr, Francesco, Dan, Adam: your thoughts?<br>
><br>
> >><br>
> >> - Moving from nose to pytest - generally good approach to achieve. It<br>
> >> requires some changes in current testlib.py code. must be an item for next<br>
> >> major version (nir already managed to run most of the tests with it, and<br>
> >> stated few gaps)<br>
> >><br>
> >> - Exception patches - still on progress, please review<br>
> >> (<a href="https://gerrit.ovirt.org/48868">https://gerrit.ovirt.org/48868</a>)<br>
> >><br>
> >> - python3 effort to cover all asyncProc usage, and allowing utils import<br>
> >> without having python3-cpopen - <a href="https://gerrit.ovirt.org/51421">https://gerrit.ovirt.org/51421</a><br>
> >> <a href="https://gerrit.ovirt.org/49441">https://gerrit.ovirt.org/49441</a> . still under review<br>
> >><br>
> >> We didn't take notes during that talk, so if I forgot to mention something<br>
> >> I apologize. Feel free to reply and raise it<br>
> >><br>
> >> Greetings,<br>
> >><br>
> >> --<br>
> >> Yaniv Bronhaim.<br>
> >><br>
> >> _______________________________________________<br>
> >> Devel mailing list<br>
> >> <a href="mailto:Devel@ovirt.org">Devel@ovirt.org</a><br>
> >> <a href="http://lists.ovirt.org/mailman/listinfo/devel">http://lists.ovirt.org/mailman/listinfo/devel</a><br>
> ><br>
> ><br>
> ><br>
> > _______________________________________________<br>
> > Devel mailing list<br>
> > <a href="mailto:Devel@ovirt.org">Devel@ovirt.org</a><br>
> > <a href="http://lists.ovirt.org/mailman/listinfo/devel">http://lists.ovirt.org/mailman/listinfo/devel</a><br>
</p>