6:49 a.m.
(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)
- 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)
- python3 effort to cover all asyncProc usage, and allowing utils import
without having python3-cpopen - https://gerrit.ovirt.org/51421
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.*
6:57 a.m.
Thanks for the summary!
See one comment inline.
On Sun, Jan 10, 2016 at 2:49 PM, Yaniv Bronheim <ybronhei(a)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.
- 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)
- python3 effort to cover all asyncProc usage, and allowing utils import
without having python3-cpopen - https://gerrit.ovirt.org/51421
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(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel
7:37 a.m.
On Sun, Jan 10, 2016 at 2:57 PM, Oved Ourfali <oourfali(a)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> 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.
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?
>
> - 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)
>
> - python3 effort to cover all asyncProc usage, and allowing utils import
> without having python3-cpopen - https://gerrit.ovirt.org/51421
> 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(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
_______________________________________________
Devel mailing list
Devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel
7:56 a.m.
On Jan 10, 2016 3:37 PM, "Nir Soffer" <nsoffer(a)redhat.com> wrote:
That's great. As said earlier, if you know of others please open bugs.
> 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.
I agree. But again, 3.6 clusters will require jsonrpc. That's why in 4.0
xmlrpc will no longer be relevant.
> 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?
> > >
>> - 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
> >> - Exception patches -
still on progress, please review
> >> (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/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(a)ovirt.org
> >> http://lists.ovirt.org/mailman/listinfo/devel
> > > > >
_______________________________________________
> > Devel mailing list
> > Devel(a)ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/devel
On Sun, Jan 10, 2016 at 2:57 PM, Oved Ourfali <oourfali(a)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>
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.
No we don't. And we won't. We had it around for one version for this
reason, but no need for more.
We had a bug about that, and communicated it to whomever is relevant.
We want to get rid of it entirely in 4.0, so 3.6 cluster won't work with
it.
If you know of any issue then please fix it now, or open a bug about it.
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.
>> major version (nir already managed to run most of the tests
with it,
and
> >> stated few gaps)
> >
1:51 p.m.
On Sun, Jan 10, 2016 at 2:56 PM, Oved Ourfali <oourfali(a)redhat.com> wrote:
On Jan 10, 2016 3:37 PM, "Nir Soffer" <nsoffer(a)redhat.com> wrote:
>
> On Sun, Jan 10, 2016 at 2:57 PM, Oved Ourfali <oourfali(a)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>
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.
>
No we don't. And we won't. We had it around for one version for this
reason, but no need for more.
We had a bug about that, and communicated it to whomever is relevant.
We want to get rid of it entirely in 4.0, so 3.6 cluster won't work with
it.
If you know of any issue then please fix it now, or open a bug about it.
> 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
>
That's great. As said earlier, if you know of others please open bugs.
> 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.
I agree. But again, 3.6 clusters will require jsonrpc. That's why in 4.0
xmlrpc will no longer be relevant.
>
> 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?
>
I would love to get rid of it but we need to make sure that none of the
code is using
xmlrpc. I think that we need to prioritize tasks before we can stop using
it.
It would be great to change format of the schema for new client and
drop/replace
code which is still using xmlrpc.
> >>
> >> - 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)
> >>
> >> - python3 effort to cover all asyncProc usage, and allowing utils
import
> >> without having python3-cpopen - https://gerrit.ovirt.org/51421
> >> 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(a)ovirt.org
> >> http://lists.ovirt.org/mailman/listinfo/devel
> >
> >
> >
> > _______________________________________________
> > Devel mailing list
> > Devel(a)ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/devel
1:18 p.m.
------=_Part_10719691_722243383.1452539910403
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
----- Original Message -----
On Sun, Jan 10, 2016 at 2:56 PM, Oved Ourfali <
oourfali(a)redhat.com > wrote:
> On Jan 10, 2016 3:37 PM, "Nir Soffer" <
nsoffer(a)redhat.com > wrote:
> >
> > On Sun, Jan 10, 2016 at 2:57 PM, Oved Ourfali < oourfali(a)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
>
> > > 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.
> >
> No we don't. And we won't. We had it around for one
version for this
> reason,
> but no need for more.
> We had a bug about that, and communicated it to whomever is relevant.
> We want to get rid of it entirely in 4.0, so 3.6 cluster won't work with
> it.
> If you know of any issue then please fix it now, or open a bug about it.
> > 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
> >
> That's great. As said earlier, if you know of others please
open bugs.
> > 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.
> I agree. But again, 3.6 clusters will require jsonrpc.
That's why in 4.0
> xmlrpc will no longer be relevant.
> >
> > 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?
> >
I would love to get rid of it but we need to make sure that none of
the code
is using
xmlrpc. I think that we need to prioritize tasks before we can stop using it.
It would be great to change format of the schema for new client and
drop/replace
code which is still using xmlrpc.
I agree, it does not make sense to maintain both protocols.
Let's think for a minute how it will work with the customers.
If the move from RHEV 3 to 4 would be similar as from 2 to 3, when 4 is a complete new
system and some migration path would be needed, I think it is ok to say that we stop
supporting all the old hosts running RHEL6 that can do XML-RPC only. But 3.6 definitely
should still support XML RPC, because some customers would continue running old hosts in
old compatibility mode clusters. Because we support it.
https://access.redhat.com/support/policy/updates/rhev/
When working on a command line client, also keep in mind that customers would attempt
using it on their production environment, even if it may create inconsistency with their
engine database. It would be good to enable some kind of production mode, where the
customer would have to hit a confirmation yes, if the command may affect the global setup
(all storage operations). There is at least one customer that I know of that is using
vdsClient in their production system (I protested as much as I could).
> > >>
> > >> - 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 )
> > >>
> > >> - python3 effort to cover all asyncProc usage, and allowing utils
> > >> import
> > >> without having python3-cpopen - https://gerrit.ovirt.org/51421
> > >> 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(a)ovirt.org
> > >> http://lists.ovirt.org/mailman/listinfo/devel
> > >
> > >
> > >
> > > _______________________________________________
> > > Devel mailing list
> > > Devel(a)ovirt.org
> > > http://lists.ovirt.org/mailman/listinfo/devel
--
--
mku
------=_Part_10719691_722243383.1452539910403
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: quoted-printable
<html><body><div style=3D"font-family: Verdana; font-size: 12pt;
color: #00=
0066"><div><br></div><div><br></div><hr
id=3D"zwchr"><blockquote style=3D"b=
order-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;fo=
nt-weight:normal;font-style:normal;text-decoration:none;font-family:Helveti=
ca,Arial,sans-serif;font-size:12pt;" data-mce-style=3D"border-left: 2px sol=
id #1010FF; margin-left: 5px; padding-left: 5px; color: #000; font-weight: =
normal; font-style: normal; text-decoration: none; font-family: Helvetica,A=
rial,sans-serif; font-size: 12pt;"><div
dir=3D"ltr"><br><div class=3D"gmail=
_extra"><br><div class=3D"gmail_quote">On Sun, Jan 10, 2016
at 2:56 PM, Ove=
d Ourfali <span dir=3D"ltr"><<a
href=3D"mailto:oourfali@redhat.com" targ=
et=3D"_blank"
data-mce-href=3D"mailto:oourfali@redhat.com">oourfali@redhat.=
com</a>></span> wrote:<br><blockquote
class=3D"gmail_quote" style=3D"mar=
gin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex" data-mce-style=
=3D"margin: 0 0 0 .8ex; border-left: 1px #ccc solid; padding-left:
1ex;"><p=
dir=3D"ltr"><br> On Jan 10, 2016 3:37 PM, "Nir Soffer"
<<a href=3D"mail=
to:nsoffer@redhat.com" target=3D"_blank"
data-mce-href=3D"mailto:nsoffer@re=
dhat.com">nsoffer(a)redhat.com</a>&gt; wrote:<br> ><br>
> On Sun, Jan 1=
0, 2016 at 2:57 PM, Oved Ourfali <<a
href=3D"mailto:oourfali@redhat.com"=
target=3D"_blank"
data-mce-href=3D"mailto:oourfali@redhat.com">oourfali@re=
dhat.com</a>> wrote:<br> > > Thanks for the
summary!<br> > >=
See one comment inline.<br> > ><br> > > On Sun,
Jan 10, 2016 a=
t 2:49 PM, Yaniv Bronheim <<a href=3D"mailto:ybronhei@redhat.com"
target=
=3D"_blank"
data-mce-href=3D"mailto:ybronhei@redhat.com">ybronhei@redhat.co=
m</a>> wrote:<br> > >><br> >
>><br> > >> (fro=
mani, nsoffer, ybronhei, alitke)<br> > >><br> >
>> =
- Removing xmlrpc for good - who should accept it? where do we stand with<b=
r> > >> full jsonrpc client ? (we didn't get to any
conclusions an=
d said that we'll<br> > >> reraise this topic next week
with piote=
r)<br> > >><br> > ><br> >
> With regards to that, in o=
rder 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 x=
mlrpc option in 3.6, as a backup for<br> > jsonrpc issues.<br>
></p><=
p dir=3D"ltr">No we don't. And we won't. We had it around for one
version f=
or this reason, but no need for more. <br> We had a bug about that, and com=
municated it to whomever is relevant. <br> We want to get rid of it entirel=
y 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=3D"ltr">> We j=
ust fixed couple of jsonrpc verbs that were returning True instead of<br> &=
gt; the correct return value (caused by incorrect schema).<br> > - <a hr=
ef=3D"https://github.com/oVirt/vdsm/commit/0ca680700596564b4d6b0ef01...
7c488de7" target=3D"_blank"
data-mce-href=3D"https://github.com/oVirt/vdsm/=
commit/0ca680700596564b4d6b0ef01ed4b0ae7c488de7">https://github.c...
dsm/commit/0ca680700596564b4d6b0ef01ed4b0ae7c488de7</a><br> > - <a
href=
=3D"https://gerrit.ovirt.org/40402" target=3D"_blank"
data-mce-href=3D"http=
s://gerrit.ovirt.org/40402">https://gerrit.ovirt.org/40402</a>
VM.getDiskAl=
ignment cannot be used in jsonrpc<br> ></p><p
dir=3D"ltr">That's great. =
As said earlier, if you know of others please open bugs.</p><div><div
class=
=3D"h5"><p dir=3D"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
inv=
est time in adding xmlrpc and vdsClient support.<br> ><br> >
Example =
new verb merged recently:<br> > <a
href=3D"https://github.com/oVirt/vdsm=
/commit/bbbb72a192d8b54d21c8d65f6a10278404a966db" target=3D"_blank"
data-mc=
e-href=3D"https://github.com/oVirt/vdsm/commit/bbbb72a192d8b54d21c8d...
278404a966db">https://github.com/oVirt/vdsm/commit/bbbb72a192d8b5...
6a10278404a966db</a><br> ><br> > In the new verbs, we
cleaned up the =
api, so integer values are passed<br> > as integers,<br> > not
as str=
ings. Previously we use to require strings since xmlrpc did<br> > not su=
pport large<br> > numbers (> 2**31 - 1).<br> ><br>
> So in the =
schema, we require now a uint:<br> ><br> > +##<br>
> +# @CreateVol=
umeInfo:<br> > +#<br> > ...<br> > +# @virtual_size:
The Volume siz=
e in bytes<br> > ...<br> > +# @initial_size: #optional If
specified, =
the initial allocated size of volume<br> > +# in bytes. Allowed only whe=
n creating a thinly provisioned<br> > +# volume on block storage.<br>
&g=
t; +#<br> > +# Since: 4.18<br> > +##<br> >
+{'type': 'CreateVolume=
Info',<br> > + 'data': {'sd_id': 'UUID',
'img_id': 'UUID', 'vol_id': 'UU=
ID',<br> > + 'virtual_size': 'uint',
'vol_format': 'VolumeFormat',<br> &=
gt; + 'disk_type': 'DiskType', 'description':
'str',<br> > + '*parent_im=
g_id': 'UUID', '*parent_vol_id': 'UUID',<br> > +
'*initial_size': 'uint'=
}}<br> ><br> > To support xmlrpc, we added this ugly code in
bindingx=
mlrpc.py:<br> ><br> > + def sdm_create_volume(self,
args):<br> > +=
validateArgTypes(args, [str, parse_json_obj])<br> > +<br> > + #
Conv=
ert large integers to strings. The server's xmlrpc binding will<br> > +
=
# restore them to their proper int types.<br> > + vol_info =3D args[1]<b=
r> > + for param in 'virtual_size', 'initial_size':<br>
> + if param =
in vol_info:<br> > + vol_info[param] =3D str(vol_info[param])<br>
> +=
<br> > + res =3D self.s.sdm_create_volume(*args)<br> > + if
res['stat=
us']['code']:<br> > + return
res['status']['code'], res['status']['messa=
ge']<br> > +<br> > + return 0, ''<br> >
+<br> ><br> > To sup=
port vdsClient, we added this:<br> ><br> > + def
sdm_create_volume(se=
lf, job_id, vol_info):<br> > + sdm =3D 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[para=
m] =3D int(vol_info[param])<br> > +<br> > + return
sdm.create_volume(=
job_id, vol_info)<br> > +<br> ><br> > All this work
is waste effor=
t on our side.<br> ><br> > We should make a decision now - do we
supp=
ort xmlrpc in vdsm?<br> ><br> > I think we should not support
it, aft=
er two version we support both<br> > xmlrpc and
jsonrpc.</p></div></div>=
<p dir=3D"ltr">I agree. But again, 3.6 clusters will require jsonrpc.
That'=
s why in 4.0 xmlrpc will no longer be relevant.</p><div
class=3D"HOEnZb"><d=
iv class=3D"h5"><p dir=3D"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 hr=
ef=3D"https://gerrit.ovirt.org/35181/" target=3D"_blank"
data-mce-href=3D"h=
ttps://gerrit.ovirt.org/35181/">https://gerrit.ovirt.org/35181/&l...
>=
;<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 Ah=
aron - supporting both xmlrpc and jsonrpc means we need to test<br> > ev=
erything twice, I don't think Aharion will like to do that.<br>
><=
br> > Piotr, Francesco, Dan, Adam: your thoughts?<br>
><br></p></div>=
</div></blockquote><div>I would love to get rid of it but we need to
make s=
ure that none of the code is using<br></div><div>xmlrpc. I think that we
ne=
ed to prioritize tasks before we can stop using
it.<br><div><br></div></div=
<div>It would be great to change format of the schema for new
client and d=
rop/replace<br></div><div>code which is still using
xmlrpc.</div></div></di=
v></div></blockquote><div>I agree, it does not make sense to maintain
both =
protocols.<br></div><div>Let's think for a minute how it will work
with the=
customers.<br></div><div>If the move from RHEV 3 to 4 would be similar
as =
from 2 to 3, when 4 is a complete new system and some migration path would =
be needed, I think it is ok to say that we stop supporting all the old host=
s running RHEL6 that can do XML-RPC only. But 3.6 definitely should still s=
upport XML RPC, because some customers would continue running old hosts in =
old compatibility mode clusters. Because we support it.
<br></div><div>http=
s://access.redhat.com/support/policy/updates/rhev/<br></div>&...
<div>When working on a command line client, also keep in mind that customer=
s would attempt using it on their production environment, even if it may cr=
eate inconsistency with their engine database. It would be good to enable s=
ome kind of production mode, where the customer would have to hit a confirm=
ation yes, if the command may affect the global setup (all storage operatio=
ns). There is at least one customer that I know of that is using vdsClient =
in their production system (I protested as much as I
could).<br></div><bloc=
kquote style=3D"border-left:2px solid #1010FF;margin-left:5px;padding-left:=
5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;fo=
nt-family:Helvetica,Arial,sans-serif;font-size:12pt;" data-mce-style=3D"bor=
der-left: 2px solid #1010FF; margin-left: 5px; padding-left: 5px; color: #0=
00; font-weight: normal; font-style: normal; text-decoration: none; font-fa=
mily: Helvetica,Arial,sans-serif; font-size: 12pt;"><div
dir=3D"ltr"><div c=
lass=3D"gmail_extra"><div
class=3D"gmail_quote"><div><br></div><blockquote =
class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc
solid=
;padding-left:1ex" data-mce-style=3D"margin: 0 0 0 .8ex; border-left: 1px #=
ccc solid; padding-left: 1ex;"><div class=3D"HOEnZb"><div
class=3D"h5"><p d=
ir=3D"ltr">> >><br> >
>> - Moving from nose to pyte=
st - 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, a=
nd<br> > >> stated few gaps)<br> >
>><br> > >>&n=
bsp; - Exception patches - still on progress, please review<br> >
>&g=
t; (<a href=3D"https://gerrit.ovirt.org/48868" target=3D"_blank"
data-mce-h=
ref=3D"https://gerrit.ovirt.org/48868">https://gerrit.ovirt....
br> > >><br> > >> - python3
effort to cover all asy=
ncProc usage, and allowing utils import<br> > >> without having
py=
thon3-cpopen - <a href=3D"https://gerrit.ovirt.org/51421"
target=3D"_blank"=
data-mce-href=3D"https://gerrit.ovirt.org/51421">https://ger...
51421</a><br> > >> <a
href=3D"https://gerrit.ovirt.org/49441" targ=
et=3D"_blank"
data-mce-href=3D"https://gerrit.ovirt.org/49441">https://gerr=
it.ovirt.org/49441</a> . still under review<br> >
>><br> > >=
> We didn't take notes during that talk, so if I forgot to mention somet=
hing<br> > >> I apologize. Feel free to reply and raise
it<br> >=
; >><br> > >> Greetings,<br> >
>><br> > >>=
--<br> > >> Yaniv Bronhaim.<br> >
>><br> > >> _=
______________________________________________<br> > >> Devel
mail=
ing list<br> > >> <a
href=3D"mailto:Devel@ovirt.org" target=3D"_bl=
ank"
data-mce-href=3D"mailto:Devel@ovirt.org">Devel@ovirt.org</a><br>
> =
>> <a href=3D"http://lists.ovirt.org/mailman/listinfo/devel"
target=
=3D"_blank"
data-mce-href=3D"http://lists.ovirt.org/mailman/listinfo/devel"=
http://lists.ovirt.org/mailman/listinfo/devel</a><br>
> ><br> > &=
gt;<br> >
><br> > > ___________________________________________=
____<br> > > Devel mailing list<br> > > <a
href=3D"mailto:Devel=
@ovirt.org" target=3D"_blank"
data-mce-href=3D"mailto:Devel@ovirt.org">Deve=
l(a)ovirt.org</a><br> > > <a
href=3D"http://lists.ovirt.org/mailman/lis=
tinfo/devel" target=3D"_blank"
data-mce-href=3D"http://lists.ovirt.org/mail=
man/listinfo/devel">http://lists.ovirt.org/mailman/listinfo/devel...
p></div></div></blockquote></div><br></div></div></blockquote><div><br><div=
<br></div></div><div><br></div><div>--
<br></div><div><span name=3D"x"></s=
pan>--<br> mku<span
name=3D"x"></span><br></div></div></body></html>
------=_Part_10719691_722243383.1452539910403--
1:55 a.m.
On Mon, Jan 11, 2016 at 8:18 PM, Marina Kalinin <mkalinin(a)redhat.com> wrote:
------------------------------
On Sun, Jan 10, 2016 at 2:56 PM, Oved Ourfali <oourfali(a)redhat.com> wrote:
>
> On Jan 10, 2016 3:37 PM, "Nir Soffer" <nsoffer(a)redhat.com> wrote:
> >
> > On Sun, Jan 10, 2016 at 2:57 PM, Oved Ourfali <oourfali(a)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>
> 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.
> >
>
> No we don't. And we won't. We had it around for one version for this
> reason, but no need for more.
> We had a bug about that, and communicated it to whomever is relevant.
> We want to get rid of it entirely in 4.0, so 3.6 cluster won't work with
> it.
> If you know of any issue then please fix it now, or open a bug about it.
>
> > 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
> >
>
> That's great. As said earlier, if you know of others please open bugs.
>
> > 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.
>
> I agree. But again, 3.6 clusters will require jsonrpc. That's why in 4.0
> xmlrpc will no longer be relevant.
>
> >
> > 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?
> >
>
I would love to get rid of it but we need to make sure that none of the
code is using
xmlrpc. I think that we need to prioritize tasks before we can stop using
it.
It would be great to change format of the schema for new client and
drop/replace
code which is still using xmlrpc.
I agree, it does not make sense to maintain both protocols.
Let's think for a minute how it will work with the customers.
If the move from RHEV 3 to 4 would be similar as from 2 to 3, when 4 is a
complete new system and some migration path would be needed, I think it is
ok to say that we stop supporting all the old hosts running RHEL6 that can
do XML-RPC only. But 3.6 definitely should still support XML RPC, because
some customers would continue running old hosts in old compatibility mode
clusters. Because we support it.
https://access.redhat.com/support/policy/updates/rhev/
Xmlrpc is can be still used for clusters lower than 3.6. There are no
changes and people can still use the old hosts. With 3.6 cluster we ask to
use jsonrpc so it is required to change protocol prior to cluster upgrade.
When working on a command line client, also keep in mind that
customers
would attempt using it on their production environment, even if it may
create inconsistency with their engine database. It would be good to enable
some kind of production mode, where the customer would have to hit a
confirmation yes, if the command may affect the global setup (all storage
operations). There is at least one customer that I know of that is using
vdsClient in their production system (I protested as much as I could).
> >>
> > >> - 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)
> > >>
> > >> - python3 effort to cover all asyncProc usage, and allowing utils
> import
> > >> without having python3-cpopen - https://gerrit.ovirt.org/51421
> > >> 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(a)ovirt.org
> > >> http://lists.ovirt.org/mailman/listinfo/devel
> > >
> > >
> > >
> > > _______________________________________________
> > > Devel mailing list
> > > Devel(a)ovirt.org
> > > http://lists.ovirt.org/mailman/listinfo/devel
>
--
--
mku
3:26 a.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--
3020
days inactive
3022
days old
7 comments
6 participants
participants (6)
-
Marina Kalinin -
Michal Skrivanek -
Nir Soffer -
Oved Ourfali -
Piotr Kliczewski -
Yaniv Bronheim