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