[libvirt] [PATCH 5/6] libvirt: Ensure modern APIs are implemented

[Eric Blake]

On 7/9/19 4:36 AM, Daniel P. Berrangé wrote:
> On Tue, Jul 09, 2019 at 11:02:03AM +0200, Peter Krempa wrote:
>> On Mon, Jul 08, 2019 at 22:37:02 -0500, Eric Blake wrote:
>>> As shown in recent patches, several drivers provided only an older
>>> counterpart of an API, making it harder to uniformly use the newer
>>> preferred API form. We can prevent future instances of this by failing
>>> the driver at initialization time if a modern API is forgotten when an
>>> older API is present.  For now, the list includes any interface with a
>>> Flags counterpart, except virDomainBlockStatsFlags which is a bit more
>>> complex than virDomainBlockStats.
>>>

>>> +#define REQUIRE_API(old, new) \
>>> +    do { \
>>> +        if (driver->hypervisorDriver->old && \
>>> +            !driver->hypervisorDriver->new) { \
>>> +            fprintf(stderr, " ***FIXME!: driver %s is broken on %s\n", \
>>> +                    driver ? NULLSTR(driver->hypervisorDriver->name) : "(null)", #new); \
>>
>> fprintf in a library function is really wrong.

Whoops, leftovers while writing the other patches, that I meant to
remove. That said,

>>
>> Also I don't think this really requires a runtime check as the APIs
>> aren't really going to just disappear.
> 
> Yeah, I think we can easily do this validation via a make check rule
> using static analysis.
> 
> We could hack either check-driverimpls.pl or check-drivername.pl to
> mandate that both the old + new method are always provided as a pair,
> or just create a new dedicated check script for that.

this is indeed a much nicer idea. I'll do v2 along those lines.

Other ideas: we have few enough hypervisor drivers that the work of
duplicating an API in each driver is still doable - but if we really
wanted to worry about scaling, a better approach might be to rewrite
src/libvirt*.c to do automatic fallback any time we have paired APIs.
After all, each API already has central code that checks if the
hypervisor has a callback and supplies a sane error if not, as well as
doing global argument sanity checking; it would be easy enough to expand
the central code for each paired API to do automatic fallback to the
other half of the pair before completely failing.  This can be done in
both directions: if a driver provides only the new and not the old
interface, we can synthesize the old Foo(args) by calling the new
FooFlags(args, 0). Or if only the new interface is lacking, the new one
can do virCheckFlags(0, RET) and then call into the old.  It would let
us remove quite a bit of code from the various drivers, AND it has the
benefit that we don't need a syntax check, but it may make it harder for
the web page listing which interfaces were supported in which release
for a given driver.  Is that worth pursuing instead?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190709/a01a23fe/attachment-0001.sig>