[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] API break from the VMware player driver



On Tue, Dec 21, 2010 at 08:50:04PM +0100, Matthias Bolte wrote:
> 2010/12/21 Eric Blake <eblake redhat com>:
> > On 12/21/2010 11:34 AM, Chris Lalancette wrote:
> >> All,
> >>      I'll preface this by saying that I'm not 100% sure I'm correct.  But I
> >> still think there may be an API break that was introduced with the VMware
> >> player driver.  In include/libvirt/virterror.h, VIR_FROM_VMWARE was added to
> >> the *middle* of the enum for virErrorDomain.  If any clients of libvirt happen
> >> to be using hardcoded numbers, then they will now have the wrong number for
> >> everything after VIR_FROM_VMWARE.
> >
> > virterror.h is public, we should fix it now before the next release,

  Absolutely !
BTW I will send a mail about next release suggestion later today, now that
my move is over and I'm back online.

> > because these values are passed over the wire in RPC calls, and if the
> > server on one machine uses different values than the client on another
> > machine, then the client can misinterpret the error values.
> >
> 
> We might explicit write out all values of the enum members. That'll
> make it more obvious that adding in the middle is a bad idea.

  agreed too,

> Can we add a make syntax-check rule to avoid future breakage, like we
> have for the XDR protocol?
> 
> PS: Sorry, for not noticing this during review of the VMware driver.

  Well it happens, but yes an automated extra check for all enums in
the public APIs would be good, I think this could be also managed with
libvirt-api.xml extracted values, assuming we put the numbers in.

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]