On Thu, Oct 18, 2012 at 12:03:02PM -0400, Saggi Mizrahi wrote:
> currently getVdsCaps() does a lot of unrelated things most of them have no 
> relation to capabilites.
> This was done because of HTTP overhead. Instead of calling multiple commands 
> we will call one that does everything.
> 
> I agree with the suggestion that getVdsCaps() will actually return the 
> capabilities.

I am not sure anyone has suggested changing the behavior of
getVdsCapabilities().  What is being suggested is a new API that returns static
version and capabilities information.  This information is very fast to assemble
and does not require any slow querying of current state.

> Capabilities being:
> - storage core version supported domain formats
> - VM core version and supported host capabilites.
> - network core and capabilities.
> - etc...
> 
> These all should be mostly static and set at boot.

Agreed.

> As to the query API. I personally dislike the idea of a "bag" API. Now that we
> are moving away from HTTP, call overhead is no longer an issue so we can have
> multiple verbs and call them sequentially. In actuality we already do.
> Internally getVdsCaps() just aggregates other APIs.  This makes return values
> of the method easier to handle and makes changing the results of an API call
> not affect users that don't care about that change.  This also has better
> performance as storage APIs tend to slow the response and sending multiple
> commands would mean that you can get the Network stats even though the storage
> server is down.

I agree that getVdsCapabilities is a less than ideal API.  I would not consider
my suggested capabilities approach to be a "bag" API.  What is returned is a
list of enums to indicate whether a particular capability is supported.  Again,
these would all be defined in the vdsm source code and would be easy to
regurgitate on demand.

> 
> ----- Original Message -----
> > From: "Dan Kenigsberg" <dan...@redhat.com> To: "Adam Litke" 
> > <a...@us.ibm.com>
> > Cc: vdsm-devel@lists.fedorahosted.org, "Michal Skrivanek"
> > <mskri...@redhat.com> Sent: Thursday, October 18, 2012 4:38:16 AM Subject:
> > Re: [vdsm] new API verb: getVersionInfo()
> > 
> > On Wed, Oct 17, 2012 at 10:07:43AM -0500, Adam Litke wrote:
> > > Thanks for posting your idea on the list here.  I like the idea of a more
> > > fine-grained version query API.  getVdsCapabilities has become too much of
> > > a catch-all and I agree that something lighter is useful.  I do think vdsm
> > > will want to add a real capabilities mechanism and it could probably go
> > > here as well.
> > > 
> > > As we work to make the vdsm API evolve in a stable, compatible manner,
> > > capabilities/feature-bits will come into play.  Since you're proposing a
> > > structure return value, we can easily add the capabilities field to it in
> > > a future release, but it might make sense to have it there now to reduce
> > > client-side complexity of figuring out if the return value has a
> > > capabilities item.
> > > 
> > > To avoid the bloat that we have with the current getVdsCapabilities API, I
> > > propose a simple format for the new capabilities:
> > > 
> > > {'enum': 'Capabilities', 'data': ['StorageDomain_30', 'StorageDomain_22',
> > > 'Sanlock', ...]}
> > > 
> > > and then add the following to the return type for your new API:
> > > 
> > > 'capabilities': ['Capabilities']
> > > 
> > > This is essentially an expandable bitmask of features where a feature is
> > > present by its presense in the 'capabilities' array.  This will be
> > > extensible by simply adding new capabilities to the enum as we find them
> > > to be necessary.
> > > 
> > > Thoughts on this?  The reason I am bringing it up now is it would be nice
> > > to restrict the pain of migrating to this new version API to just one
> > > time.
> > 
> > I fully agree - that's what I've ment in my
> > http://gerrit.ovirt.org/#/c/8431/4/vdsm_api/vdsmapi-schema.json comment on a
> > "bag" of capability flags.
> > 
> > > 
> > > 
> > > On Wed, Oct 17, 2012 at 01:37:08PM +0200, Peter V. Saveliev wrote:
> > > > …
> > > > 
> > > > New verb proposal: getVersionInfo()
> > > > 
> > > > 
> > > >   Background
> > > > 
> > > > Right now VDSM has only one possibility to discover the peer VDSM
> > > > version — it is to call getVdsCapabilities(). All would be nice, but the
> > > > verb does a lot of stuff, including disk I/O (rpm data query).  It is a
> > > > serious issue for high-loaded hosts, that can even trigger call timeout.
> > > > 
> > > > 
> > > >   Rationale
> > > > 
> > > > Working in an environment with multiple VDSM versions, it is inevitable
> > > > to fall in a simple choice:
> > > > 
> > > > * always operate with one API, described once and forever * use
> > > > different protocol versions.
> > > > 
> > > > It is a common practice to reserve something in a protocol, that will
> > > > represent the protocol version. Any protocols w/o version info sooner or
> > > > later face the need to guess a version, that is much worse.
> > > > 
> > > > On the other hand, involving rpm queries and CPU topology calculation
> > > > into the protocol version discovery is an overkill.  So the simplest way
> > > > is to reserve a new verb for it.
> > > > 
> > > > 
> > > >   Usecases
> > > > 
> > > > It can be used in the future in *any* VDSM communication that can expose
> > > > version difference.
> > > > 
> > > > 
> > > >   Implementation
> > > > 
> > > > Obviously, the usage of a new verb in the current release, e.g.
> > > > RHEV-3.1 can be done only in try/catch way, 'cause RHEV-3.0 does not
> > > > support it. But to be able to use it in RHEV-3.2, we should already have
> > > > it in 3.1. Even if we will not use it yet, the future usecases are
> > > > pretty straightforward.
> > > > 
> > > > So pls comment it: http://gerrit.ovirt.org/#/c/8431/
> > > > 
> > > > -- Peter V. Saveliev
> > > > 
> > > > _______________________________________________ vdsm-devel mailing list
> > > > vdsm-devel@lists.fedorahosted.org
> > > > https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
> > > 
> > > -- Adam Litke <a...@us.ibm.com> IBM Linux Technology Center
> > > 
> > > _______________________________________________ vdsm-devel mailing list
> > > vdsm-devel@lists.fedorahosted.org
> > > https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
> > _______________________________________________ vdsm-devel mailing list
> > vdsm-devel@lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
> > 
> 

-- 
Adam Litke <a...@us.ibm.com>
IBM Linux Technology Center

_______________________________________________
vdsm-devel mailing list
vdsm-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel

Reply via email to