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

Sun Jan 10 19:51:37 UTC 2016

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

>
> On Jan 10, 2016 3:37 PM, "Nir Soffer" <nsoffer at redhat.com> wrote:
> >
> > On Sun, Jan 10, 2016 at 2:57 PM, Oved Ourfali <oourfali at redhat.com>
> wrote:
> > > Thanks for the summary!
> > > See one comment inline.
> > >
> > > On Sun, Jan 10, 2016 at 2:49 PM, Yaniv Bronheim <ybronhei at 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/bbbb72a192d8b54d21c8d65f6a10278404a966db
> >
> > 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 at ovirt.org
> > >> http://lists.ovirt.org/mailman/listinfo/devel
> > >
> > >
> > >
> > > _______________________________________________
> > > Devel mailing list
> > > Devel at ovirt.org
> > > http://lists.ovirt.org/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/devel/attachments/20160110/c5df83cb/attachment-0001.html>