[libvirt] [PATCH v5 10/23] src: rewrite driver name checker in Python
Daniel P. Berrangé
berrange at redhat.com
Mon Nov 18 17:44:04 UTC 2019
On Fri, Nov 15, 2019 at 04:50:57PM -0500, Cole Robinson wrote:
> 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
Yes, this is just an artifact of the original code. It can be merged
below.
>
>
>
> > + 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
It matches if you define an API without the 'Drv' prefix. I just
need to drop the "^"
eg if i did
diff --git a/src/driver-secret.h b/src/driver-secret.h
index 125238fe7b..ee93eaba68 100644
--- a/src/driver-secret.h
+++ b/src/driver-secret.h
@@ -35,7 +35,7 @@ typedef virSecretPtr
const unsigned char *uuid);
typedef virSecretPtr
-(*virDrvSecretLookupByUsage)(virConnectPtr conn,
+(*virFishSecretLookupByUsage)(virConnectPtr conn,
int usageType,
const char *usageID);
then it would report
Bogus name *virFishSecretLookupByUsage
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 :|
More information about the libvir-list
mailing list