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.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/libvirt.c | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c index 7e665b6cba..a12a72a31b 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -567,19 +567,53 @@ int virRegisterConnectDriver(virConnectDriverPtr driver, bool setSharedDrivers) { - VIR_DEBUG("driver=%p name=%s", driver, - driver ? NULLSTR(driver->hypervisorDriver->name) : "(null)"); + const char *driver_name; + + driver_name = driver ? NULLSTR(driver->hypervisorDriver->name) : "(null)"; + VIR_DEBUG("driver=%p name=%s", driver, driver_name);
virCheckNonNullArgReturn(driver, -1); if (virConnectDriverTabCount >= MAX_DRIVERS) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Too many drivers, cannot register %s"), - driver->hypervisorDriver->name); + driver_name); return -1; }
+ /* Check for drivers failing to provide a modern counterpart to an + * older API */ +#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.
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. Failing during "make check" is preferable to printf at runtime. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|