[libvirt] [PATCH v5 10/23] src: rewrite driver name checker in Python
Cole Robinson
crobinso at redhat.com
Fri Nov 15 21:50:57 UTC 2019
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 at redhat.com>
> ---
> 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 at redhat.com>
> + 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
More information about the libvir-list
mailing list