On 11/11/19 9:38 AM, Daniel P. Berrangé wrote: > As part of an goal to eliminate Perl from libvirt build tools, > rewrite the check-drivername.pl tool in Python. > > This was mostly a straight conversion, manually going line-by-line > to change the syntax from Perl to Python. Thus the overall structure > of the file and approach is the same. > > In testing though it was discovered the existing code was broken > since it hadn't been updated after driver.h was split into many > files. Since the old code is being thrown away, the fix was done > as part of the rewrite rather than split into a separate commit. > > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx> > --- > Makefile.am | 1 + > scripts/check-drivername.py | 114 ++++++++++++++++++++++++++++++++++++ > src/Makefile.am | 18 ++++-- > src/check-drivername.pl | 83 -------------------------- > 4 files changed, 129 insertions(+), 87 deletions(-) > create mode 100644 scripts/check-drivername.py > delete mode 100755 src/check-drivername.pl > > diff --git a/Makefile.am b/Makefile.am > index afed409c94..7155ab6ce9 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -47,6 +47,7 @@ EXTRA_DIST = \ > AUTHORS.in \ > scripts/augeas-gentest.py \ > scripts/check-aclperms.py \ > + scripts/check-drivername.py \ > scripts/check-spacing.py \ > scripts/check-symfile.py \ > scripts/check-symsorting.py \ > diff --git a/scripts/check-drivername.py b/scripts/check-drivername.py > new file mode 100644 > index 0000000000..3226ee7962 > --- /dev/null > +++ b/scripts/check-drivername.py > @@ -0,0 +1,114 @@ > +#!/usr/bin/env python > +# > +# Copyright (C) 2013-2019 Red Hat, Inc. > +# > +# This library is free software; you can redistribute it and/or > +# modify it under the terms of the GNU Lesser General Public > +# License as published by the Free Software Foundation; either > +# version 2.1 of the License, or (at your option) any later version. > +# > +# This library is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > +# Lesser General Public License for more details. > +# > +# You should have received a copy of the GNU Lesser General Public > +# License along with this library. If not, see > +# <http://www.gnu.org/licenses/>. > +# > + > +from __future__ import print_function > + > +import re > +import sys > + > +drvfiles = [] > +symfiles = [] > +for arg in sys.argv: > + if arg.endswith(".h"): > + drvfiles.append(arg) > + else: > + symfiles.append(arg) > + > +symbols = {} > + > +for symfile in symfiles: > + with open(symfile, "r") as fh: > + for line in fh: > + m = re.search(r'''^\s*(vir\w+)\s*;\s*$''', line) > + if m is not None: > + symbols[m.group(1)] = True > + > +status = 0 > +for drvfile in drvfiles: > + with open(drvfile, "r") as fh: > + for line in fh: > + if line.find("virDrvConnectSupportsFeature") != -1: > + continue > + I think this bit is just another mechanism for the skip list below. I can't really figure out what other purpose it could be serving > + m = re.search(r'''\*(virDrv\w+)\s*\)''', line) > + if m is not None: > + drv = m.group(1) > + > + skip = [ > + "virDrvStateInitialize", > + "virDrvStateCleanup", > + "virDrvStateReload", > + "virDrvStateStop", > + "virDrvConnectURIProbe", > + "virDrvDomainMigratePrepare", > + "virDrvDomainMigratePrepare2", > + "virDrvDomainMigratePrepare3", > + "virDrvDomainMigratePrepare3Params", > + "virDrvDomainMigratePrepareTunnel", > + "virDrvDomainMigratePrepareTunnelParams", > + "virDrvDomainMigratePrepareTunnel3", > + "virDrvDomainMigratePrepareTunnel3Params", > + "virDrvDomainMigratePerform", > + "virDrvDomainMigratePerform3", > + "virDrvDomainMigratePerform3Params", > + "virDrvDomainMigrateConfirm", > + "virDrvDomainMigrateConfirm3", > + "virDrvDomainMigrateConfirm3Params", > + "virDrvDomainMigrateBegin", > + "virDrvDomainMigrateBegin3", > + "virDrvDomainMigrateBegin3Params", > + "virDrvDomainMigrateFinish", > + "virDrvDomainMigrateFinish2", > + "virDrvDomainMigrateFinish3", > + "virDrvDomainMigrateFinish3Params", > + "virDrvStreamInData", > + ] > + if drv in skip: > + continue > + > + sym = drv.replace("virDrv", "vir") > + > + if sym not in symbols: > + print("Driver method name %s doesn't match public API" % > + drv) Missing status = 1 here > + continue > + > + m = re.search(r'''^\*(vir\w+)\s*\)''', line) > + if m is not None: > + name = m.group(1) > + print("Bogus name %s" % name) > + status = 1 > + continue > + Not quite sure what this condition is supposed to be catching. It's trying to match lines that start with '*vir'. I guess it's trying to warning against anything that doesn't start with 'virDrv' and instead plain 'vir', but the anchor usage is messed up. But it looks pre-existing. Anyways, the two other error messages trigger correctly Tested-by: Cole Robinson <crobinso@xxxxxxxxxx> > + m = re.search(r'''^\s*(virDrv\w+)\s+(\w+);\s*''', line) > + if m is not None: > + drv = m.group(1) > + field = m.group(2) > + > + tmp = drv.replace("virDrv", "") > + if tmp.startswith("NWFilter"): > + tmp = "nwfilter" + tmp[8:] > + tmp = tmp[0:1].lower() + tmp[1:] > + > + if tmp != field: > + print("Driver struct field %s should be named %s" % > + (field, tmp)) > + status = 1 > + > +sys.exit(status) > diff --git a/src/Makefile.am b/src/Makefile.am > index b0ca9d3442..55ad51abf1 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -330,15 +330,25 @@ check-protocol: > endif !WITH_REMOTE > EXTRA_DIST += $(PROTOCOL_STRUCTS) > > +DRIVERS = \ > + $(srcdir)/driver-hypervisor.h \ > + $(srcdir)/driver-interface.h \ > + $(srcdir)/driver-network.h \ > + $(srcdir)/driver-nodedev.h \ > + $(srcdir)/driver-nwfilter.h \ > + $(srcdir)/driver-secret.h \ > + $(srcdir)/driver-state.h \ > + $(srcdir)/driver-storage.h \ > + $(srcdir)/driver-stream.h \ > + $(NULL) > + > check-drivername: > - $(AM_V_GEN)$(PERL) $(srcdir)/check-drivername.pl \ > - $(srcdir)/driver.h \ > + $(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(top_srcdir)/scripts/check-drivername.py \ > + $(DRIVERS) \ > $(srcdir)/libvirt_public.syms \ > $(srcdir)/libvirt_qemu.syms \ > $(srcdir)/libvirt_lxc.syms > > -EXTRA_DIST += check-drivername.pl > - > check-driverimpls: > $(AM_V_GEN)$(PERL) $(srcdir)/check-driverimpls.pl \ > $(filter /%,$(DRIVER_SOURCE_FILES)) \ > diff --git a/src/check-drivername.pl b/src/check-drivername.pl > deleted file mode 100755 > index 3a62193e33..0000000000 > --- a/src/check-drivername.pl > +++ /dev/null > @@ -1,83 +0,0 @@ > -#!/usr/bin/env perl > -# > -# Copyright (C) 2013 Red Hat, Inc. > -# > -# This library is free software; you can redistribute it and/or > -# modify it under the terms of the GNU Lesser General Public > -# License as published by the Free Software Foundation; either > -# version 2.1 of the License, or (at your option) any later version. > -# > -# This library is distributed in the hope that it will be useful, > -# but WITHOUT ANY WARRANTY; without even the implied warranty of > -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > -# Lesser General Public License for more details. > -# > -# You should have received a copy of the GNU Lesser General Public > -# License along with this library. If not, see > -# <http://www.gnu.org/licenses/>. > -# > - > -use strict; > -use warnings; > - > -my $drvfile = shift; > -my @symfiles = @ARGV; > - > -my %symbols; > - > -foreach my $symfile (@symfiles) { > - open SYMFILE, "<", $symfile > - or die "cannot read $symfile: $!"; > - while (<SYMFILE>) { > - if (/^\s*(vir\w+)\s*;\s*$/) { > - $symbols{$1} = 1; > - } > - } > - > - close SYMFILE; > -} > - > -open DRVFILE, "<", $drvfile > - or die "cannot read $drvfile: $!"; > - > -my $status = 0; > - > -while (<DRVFILE>) { > - next if /virDrvConnectSupportsFeature/; > - if (/\*(virDrv\w+)\s*\)/) { > - > - my $drv = $1; > - > - next if $drv =~ /virDrvState/; > - next if $drv =~ /virDrvDomainMigrate(Prepare|Perform|Confirm|Begin|Finish)/; > - > - my $sym = $drv; > - $sym =~ s/virDrv/vir/; > - > - unless (exists $symbols{$sym}) { > - print "Driver method name $drv doesn't match public API name\n"; > - $status = 1; > - } > - } elsif (/^\*(vir\w+)\s*\)/) { > - my $name = $1; > - print "Bogus name $1\n"; > - $status = 1; > - } elsif (/^\s*(virDrv\w+)\s+(\w+);\s*/) { > - my $drv = $1; > - my $field = $2; > - > - my $tmp = $drv; > - $tmp =~ s/virDrv//; > - $tmp =~ s/^NWFilter/nwfilter/; > - $tmp =~ s/^(\w)/lc $1/e; > - > - unless ($tmp eq $field) { > - print "Driver struct field $field should be named $tmp\n"; > - $status = 1; > - } > - } > -} > - > -close DRVFILE; > - > -exit $status; > - Cole -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
