12:26 p.m.
--Apple-Mail=_058DD6D0-D8BF-486E-80A4-9F57BA3E401E
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
charset=utf-8
On 10 Jan 2016, at 14:56, Oved Ourfali <oourfali(a)redhat.com>
wrote:
=20
=20
On Jan 10, 2016 3:37 PM, "Nir Soffer" <nsoffer(a)redhat.com =
<mailto:nsoffer@redhat.com>> wrote:
>
> On Sun, Jan 10, 2016 at 2:57 PM, Oved Ourfali <oourfali(a)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(a)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 =
that we'll
> >> 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 =
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>
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
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
> 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 =
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 =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 =
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>
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(a)ovirt.org <mailto:Devel@ovirt.org>
> >> http://lists.ovirt.org/mailman/listinfo/devel =
<http://lists.ovirt.org/mailman/listinfo/devel>
> >
> >
> >
> > _______________________________________________
> > Devel mailing list
> > Devel(a)ovirt.org <mailto:Devel@ovirt.org>
> > http://lists.ovirt.org/mailman/listinfo/devel =
<http://lists.ovirt.org/mailman/listinfo/devel>
_______________________________________________
Devel mailing list
Devel(a)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(a)redhat.com</a>&gt;=
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(a)redhat.com</a>&gt; =
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(a)redhat.com</a>&gt;=
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(a)redhat.com</a>&gt;=
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/0ca680700596564b4d6b0ef...
0ae7c488de7" =
class=3D"">https://github.com/oVirt/vdsm/commit/0ca680700596...
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=3D118...
<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/bbbb72a192d8b54d21c8d65...
78404a966db" =
class=3D"">https://github.com/oVirt/vdsm/commit/bbbb72a192d8...
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(a)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<... =
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(a)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<... =
class=3D"">
</p>
_______________________________________________<br class=3D"">Devel =
mailing list<br class=3D""><a href=3D"mailto:Devel@ovirt.org"
=
class=3D"">Devel(a)ovirt.org</a><br =
class=3D"">http://lists.ovirt.org/mailman/listinfo/devel<...
</div><br class=3D""></body></html>=
--Apple-Mail=_058DD6D0-D8BF-486E-80A4-9F57BA3E401E--