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.
---
 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',
+    '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";
+                    $status = 1;
+                }
+            }
             $intable = 0;
             $table = undef;
             $mainprefix = undef;
+            %apis = ();
         } elsif (/\.(\w+)\s*=\s*(\w+),?/) {
             my $api = $1;
             my $impl = $2;
@@ -39,9 +66,11 @@ while (<>) {
             next if $api eq "name";
             next if $impl eq "NULL";

+            $apis{$api} = 1;
+
             my $suffix = $impl;
             my $prefix = $impl;
-            $prefix =~ s/^([a-z]+(?:Unified)?)(.*?)$/$1/;
+            $prefix =~ s/^([a-z]+)(.*?)$/$1/;

             if (defined $mainprefix) {
                 if ($mainprefix ne $prefix) {
@@ -53,7 +82,7 @@ while (<>) {
             }

             if ($api !~ /^$mainprefix/) {
-                $suffix =~ s/^[a-z]+(?:Unified)?//;
+                $suffix =~ s/^[a-z]+//;
                 $suffix =~ s/^([A-Z]+)/lc $1/e;
             }

-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list