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

[Marina Kalinin]

- Original Message -

> On Sun, Jan 10, 2016 at 2:56 PM, Oved Ourfali < oourf...@redhat.com > wrote:

> > On Jan 10, 2016 3:37 PM, "Nir Soffer" < nsof...@redhat.com > wrote:
> 
> > >
> 
> > > On Sun, Jan 10, 2016 at 2:57 PM, Oved Ourfali < oourf...@redhat.com >
> > > wrote:
> 
> > > > Thanks for the summary!
> 
> > > > See one comment inline.
> 
> > > >
> 
> > > > On Sun, Jan 10, 2016 at 2:49 PM, Yaniv Bronheim < ybron...@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/0ca680700596564b4d6b0ef01ed4b0ae7c488de7
> 
> > > - 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/72a192d8b54d21c8d65f6a10278404a966db
> 
> > >
> 
> > > 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 n

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

[Michal Skrivanek]

> On 10 Jan 2016, at 14:56, Oved Ourfali  wrote:
> 
> 
> On Jan 10, 2016 3:37 PM, "Nir Soffer"  > wrote:
> >
> > On Sun, Jan 10, 2016 at 2:57 PM, Oved Ourfali  > > wrote:
> > > Thanks for the summary!
> > > See one comment inline.
> > >
> > > On Sun, Jan 10, 2016 at 2:49 PM, Yaniv Bronheim  > > > 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/0ca680700596564b4d6b0ef01ed4b0ae7c488de7
> >  
> > 
> > - 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.
> 
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’m aware of migrationCreate and destroy, but there are likely 
more. 

Thanks,
michal

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1188543 
> 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/72a192d8b54d21c8d65f6a10278404a966db
> >  
> > 
> >
> > 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 

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

[Piotr Kliczewski]

On Mon, Jan 11, 2016 at 8:18 PM, Marina Kalinin  wrote:

>
>
> --
>
>
>
> On Sun, Jan 10, 2016 at 2:56 PM, Oved Ourfali  wrote:
>
>>
>> On Jan 10, 2016 3:37 PM, "Nir Soffer"  wrote:
>> >
>> > On Sun, Jan 10, 2016 at 2:57 PM, Oved Ourfali 
>> wrote:
>> > > Thanks for the summary!
>> > > See one comment inline.
>> > >
>> > > On Sun, Jan 10, 2016 at 2:49 PM, Yaniv Bronheim 
>> 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/0ca680700596564b4d6b0ef01ed4b0ae7c488de7
>> > - 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/72a192d8b54d21c8d65f6a10278404a966db
>> >
>> > 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/
>> >
>> > B

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

[Piotr Kliczewski]

On Sun, Jan 10, 2016 at 2:56 PM, Oved Ourfali  wrote:

>
> On Jan 10, 2016 3:37 PM, "Nir Soffer"  wrote:
> >
> > On Sun, Jan 10, 2016 at 2:57 PM, Oved Ourfali 
> wrote:
> > > Thanks for the summary!
> > > See one comment inline.
> > >
> > > On Sun, Jan 10, 2016 at 2:49 PM, Yaniv Bronheim 
> 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/0ca680700596564b4d6b0ef01ed4b0ae7c488de7
> > - 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/72a192d8b54d21c8d65f6a10278404a966db
> >
> > 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

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

[Oved Ourfali]

On Jan 10, 2016 3:37 PM, "Nir Soffer"  wrote:
>
> On Sun, Jan 10, 2016 at 2:57 PM, Oved Ourfali  wrote:
> > Thanks for the summary!
> > See one comment inline.
> >
> > On Sun, Jan 10, 2016 at 2:49 PM, Yaniv Bronheim 
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/0ca680700596564b4d6b0ef01ed4b0ae7c488de7
> - 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/72a192d8b54d21c8d65f6a10278404a966db
>
> 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
> >> major version (nir already managed to run most of the tests with it,
and
> >> stated few gaps)
> >>
> >>  - Excep

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

[Nir Soffer]

On Sun, Jan 10, 2016 at 2:57 PM, Oved Ourfali  wrote:
> Thanks for the summary!
> See one comment inline.
>
> On Sun, Jan 10, 2016 at 2:49 PM, Yaniv Bronheim  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/0ca680700596564b4d6b0ef01ed4b0ae7c488de7
- 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/72a192d8b54d21c8d65f6a10278404a966db

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@ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/devel
>
>
>
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
__

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

[Oved Ourfali]

Thanks for the summary!
See one comment inline.

On Sun, Jan 10, 2016 at 2:49 PM, Yaniv Bronheim  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@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
>
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

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

[Yaniv Bronheim]

(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.*
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel