[libvirt] [PATCH v2 3/9] maint: Enhance check-driverimpls.pl to check for API pairing

[Peter Krempa]

On Tue, Jul 09, 2019 at 12:46:32 -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
> enhancing 'make syntax-check' to flag any time a modern API is
> forgotten when an older API is present.  It also flags if a modern API
> is provided without an old counterpart; but thankfully, that situation
> didn't flag, which gives us some room for future patches to confine
> the magic of API pairs to just src/libvirt*.c and the remote driver.
> 
> Also, drop support for special-casing xenUnified, since 1dac5fbbbb0
> dropped support for that naming scheme.

Please put it in a separate patch.

> ---
>  src/check-driverimpls.pl | 33 +++++++++++++++++++++++++++++++--
>  1 file changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/src/check-driverimpls.pl b/src/check-driverimpls.pl
> index b175e710f1..34273ddbba 100755
> --- a/src/check-driverimpls.pl
> +++ b/src/check-driverimpls.pl
> @@ -23,14 +23,41 @@ use warnings;
>  my $intable = 0;
>  my $table;
>  my $mainprefix;
> +my %apis;
> +
> +# API pairs where a driver should provide both or neither alternative.
> +my %pairs = (
> +    'domainShutdown' => 'domainShutdownFlags',
> +    'domainDestroy' => 'domainDestroyFlags',
> +    'domainSetMemory' => 'domainSetMemoryFlags',

domainSetMaxMemory is in the same relationship here as it just implies
VIR_DOMAIN_MEM_MAXIMUM passed to the *Flags API.

> +    'domainSave' => 'domainSaveFlags',
> +    'domainRestore' => 'domainRestoreFlags',
> +    'domainSetVcpus' => 'domainSetVcpusFlags',
> +    'domainPinVcpu' => 'domainPinVcpuFlags',
> +    'domainCreate' => 'domainCreateWithFlags',
> +    'domainDefineXML' => 'domainDefineXMLFlags',
> +    'domainUndefine' => 'domainUndefineFlags',
> +    'domainAttachDevice' => 'domainAttachDeviceFlags',
> +    'domainDetachDevice' => 'domainDetachDeviceFlags',
> +    'domainGetSchedulerParameters' => 'domainGetSchedulerParametersFlags',
> +    'domainSetSchedulerParameters' => 'domainSetSchedulerParametersFlags',
> +    'nodeDeviceDettach' => 'nodeDeviceDetachFlags',
> +);
> 
>  my $status = 0;
>  while (<>) {
>      if ($intable) {
>          if (/}/) {
> +            while (my ($old, $new) = each %pairs) {
> +                if (exists $apis{$old} != exists $apis{$new}) {
> +                    print "$ARGV:$. Inconsistent paired API '$old' vs. '$new'\n";

Without the context of the patch the message does not seem to explain
what's happening to the unindoctrinated. I don't have a better
suggestion though.

> +                    $status = 1;
> +                }
> +            }
>              $intable = 0;
>              $table = undef;
>              $mainprefix = undef;

ACK if you split out the "Also fix this" stuff. Your call whether
domainSetMaxMemory is worth covering or the error message needs
changing.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190710/2f8ec45f/attachment-0001.sig>