[libvirt] [PATCH 1/4] latency: Introduce new members for _virDomainBlockStats

[Eric Blake]

On 08/17/2011 10:10 AM, Daniel P. Berrange wrote:
>> 2. recompiling an existing client against newer libvirt.h with no
>> other changes will automatically pick up the larger struct.  The
>> sizeof() argument will change.  Newer servers will recognize the
>> larger struct and automatically call the newer RPC, getting all the
>> new fields. However, older servers will now reject the larger sizeof
>> as too large - this is a subtle limitation, if clients are
>> recompiled without being aware of the difference it will cause!
>
> That last point is a significant problem. While we have technically
> preserved "ABI" compatibility, we have not preserved actual real
> world application API usage compatibility.
>
> I don't really like the idea of having this kind of special case
> functionality in just one of our APIs, where our current policy
> is always to add new APIs. While this is a no doubt a very neat
> trick in terms of ABI compatibility, I think we should go for
> clarity&  simplicity.

OK, both DV and you have convinced me that API compatibility is 
important, and that we must not get in the situation of point 2 - that 
is, a solution is viable only if any application that uses the type 
virDomainBlockStatsPtr is always referring to the v1 struct size, and 
the only way to get the larger struct is to use a new struct name.

But I'm unclear from your answer whether you are opposed to the overall 
idea of reusing virDomainBlockStats() with the size argument as the key 
on which struct was passed in (in which case, we need to rethink 
everything about my proposal - and rebasing to a newer .so is the only 
way to take advantage of the new fields), or just the idea of an API 
break by how I first described the changes (in which case I am confident 
that we can tweak libvirt.h.in to guarantee a naming choice that 
preserves API compatibility across recompilation, while most of my 
proposal stands as-is where it remains possible to backport the new 
fields to older libvirtd installations since no .so entry points are 
changed).  DV also seemed to be in favor of reusing the existing 
function call with a new struct, as long as there is no API break for 
existing code.  And personally, I see value in being able to add a new 
struct without requiring a rebase to a new ABI.


I just audited libvirt.c, and found that the following libvirt 
interfaces that take a size parameter, and could be extensible by use of 
the size field:
virDomainBlockStats
virDomainInterfaceStats

The following libvirt interfaces take a flags parameter, and could be 
extensible by setting a flag bit that says a larger struct is in use:
virDomainGetControlInfo
virNodeGetCPUStats
virNodeGetMemoryStats
virDomainMemoryStats
virDomainGetBlockInfo
virDomainGetBlockJobInfo

Meanwhile, the following APIs which take a pointer to a struct are 
non-extensible (neither size nor flags as an escape hatch to indicate 
explicit use of a larger struct), extending any of these would require a 
new API function:
virDomainGetInfo
virNodeGetInfo
virDomainGetVcpus
virDomainGetSecurityLabel
virDomainGetSecurityModel
virStoragePoolGetInfo
virStorageVolGetInfo
virDomainGetJobInfo



As a final question for this mail, if we decide that reusing the 
existing ABI with a new struct size is not desirable, then would you 
rather that the new virDomainBlockStats2 API (or whatever we name the 
new function) would also take a (non-extensible) struct, or would it be 
better to rewrite it in terms of 'virTypedParameterPtr params, int 
*nparams' so that all future extensibility is covered by adding 
name-type-value tuples rather than dealing with future struct sizing? 
I'd rather go through the pain of an ABI addition at most once, rather 
than once per time that we identify new fields to add.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org