Re: [libvirt] [PATCH 1/4] latency: Introduce new members for _virDomainBlockStats
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org