Blacklist Perl and Shell code in favour of Python for sake of readability and portability. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- docs/hacking.html.in | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 90376968c3..edf2f54ce3 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -343,6 +343,36 @@ Richard Jones' guide to working with open source projects</a>. </p> + <h2><a id="lang">Language Usage</a></h2> + + <p> + The libvirt repository makes use of a large number of programming + languages. It is anticipated that in the future libvirt will adopt + use of other new languages. To reduce the overall burden on developers, + there is thus a general desire to phase out usage of some of the + existing languages. + </p> + + <p> + The preferred languages at this time are: + </p> + + <ul> + <li>C - for the main libvirt codebase. Dialect supported by + GCC/CLang only.</li> + <li>Python - for supporting build scripts / tools. Code must + run with both version 2.7 and 3.x at this time.</li> + </ul> + + <p> + Languages that should not be used for any new contributions: + </p> + + <ul> + <li>Perl - build scripts must be written in Python instead.</li> + <li>Shell - build scripts must be written in Python instead.</li> + </ul> + <h2><a id="tooling">Tooling</a></h2> <p> -- 2.21.0

As part of an goal to eliminate Perl from libvirt build tools, rewrite the augeas-gentest.pl tool in Python. This was 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. The use of $(AUG_GENTEST) as a dependancy in the makefiles needed to be fixed, because this was assumed to be the filename of the script, but is in fact a full shell command line. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- Makefile.am | 2 +- build-aux/augeas-gentest.pl | 60 --------------------------- build-aux/augeas-gentest.py | 72 +++++++++++++++++++++++++++++++++ src/Makefile.am | 3 +- src/bhyve/Makefile.inc.am | 4 +- src/interface/Makefile.inc.am | 2 +- src/libxl/Makefile.inc.am | 4 +- src/locking/Makefile.inc.am | 6 +-- src/logging/Makefile.inc.am | 2 +- src/lxc/Makefile.inc.am | 4 +- src/network/Makefile.inc.am | 2 +- src/node_device/Makefile.inc.am | 2 +- src/nwfilter/Makefile.inc.am | 2 +- src/qemu/Makefile.inc.am | 4 +- src/remote/Makefile.inc.am | 4 +- src/secret/Makefile.inc.am | 2 +- src/storage/Makefile.inc.am | 2 +- src/vbox/Makefile.inc.am | 2 +- src/vz/Makefile.inc.am | 2 +- 19 files changed, 97 insertions(+), 84 deletions(-) delete mode 100755 build-aux/augeas-gentest.pl create mode 100755 build-aux/augeas-gentest.py diff --git a/Makefile.am b/Makefile.am index 711f365504..17448a914e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -41,7 +41,7 @@ EXTRA_DIST = \ run.in \ README.md \ AUTHORS.in \ - build-aux/augeas-gentest.pl \ + build-aux/augeas-gentest.py \ build-aux/check-spacing.pl \ build-aux/gitlog-to-changelog \ build-aux/header-ifdef.pl \ diff --git a/build-aux/augeas-gentest.pl b/build-aux/augeas-gentest.pl deleted file mode 100755 index 65834b533b..0000000000 --- a/build-aux/augeas-gentest.pl +++ /dev/null @@ -1,60 +0,0 @@ -#!/usr/bin/env perl -# -# augeas-gentest.pl: Generate an augeas test file, from an -# example config file + test file template -# -# 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; - -die "syntax: $0 CONFIG TEMPLATE\n" unless @ARGV == 2; - -my $config = shift @ARGV; -my $template = shift @ARGV; - -open CONFIG, "<", $config or die "cannot read $config: $!"; -open TEMPLATE, "<", $template or die "cannot read $template: $!"; - -my $group = 0; -while (<TEMPLATE>) { - if (/\@CONFIG\@/) { - my $group = 0; - print " let conf = \""; - while (<CONFIG>) { - if (/^#\w/) { - s/^#//; - s/\"/\\\"/g; - print $_; - $group = /\[\s$/; - } elsif ($group) { - s/\"/\\\"/g; - if (/#\s*\]/) { - $group = 0; - } - if (/^#/) { - s/^#//; - print $_; - } - } - } - print "\"\n"; - } else { - print $_; - } -} - -close TEMPLATE; -close CONFIG; diff --git a/build-aux/augeas-gentest.py b/build-aux/augeas-gentest.py new file mode 100755 index 0000000000..18c6490a08 --- /dev/null +++ b/build-aux/augeas-gentest.py @@ -0,0 +1,72 @@ +#!/usr/bin/env python +# +# Copyright (C) 2012-2019 Red Hat, Inc. +# +# augeas-gentest.py: Generate an augeas test file, from an +# example config file + test file template +# +# 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 + +if len(sys.argv) != 3: + print("syntax: %s CONFIG TEMPLATE" % sys.argv[0], file=sys.stderr) + sys.exit(1) + +config = sys.argv[1] +template = sys.argv[2] + + +def expand_config(config): + with open(config) as fh: + optprog = re.compile(r'''^#\w.*''') + groupstartprog = re.compile(r'''.*\[\s$''') + groupendprog = re.compile(r'''#\s*\].*$''') + + group = False + for line in fh: + if optprog.match(line) is not None: + line = line[1:] + line = line.replace('"', '\\"') + print(line, end='') + if groupstartprog.match(line): + group = True + elif group: + line = line.replace('"', '\\"') + + if groupendprog.match(line): + group = False + + if line[0] == '#': + line = line[1:] + print(line, end='') + + +def expand_template(template, config): + with open(template) as fh: + markerprog = re.compile(r'''\s*@CONFIG@\s*''') + for line in fh: + if markerprog.match(line) is not None: + print(' let conf = "', end='') + expand_config(config) + print('"') + else: + print(line, end='') + + +expand_template(template, config) diff --git a/src/Makefile.am b/src/Makefile.am index cd955ee552..2956e4bf35 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -421,7 +421,8 @@ check-augeas: $(augeas_DATA) $(augeastest_DATA) fi .PHONY: check-augeas -AUG_GENTEST = $(PERL) $(top_srcdir)/build-aux/augeas-gentest.pl +AUG_GENTEST_SCRIPT = $(top_srcdir)/build-aux/augeas-gentest.py +AUG_GENTEST = $(RUNUTF8) $(PYTHON) $(AUG_GENTEST_SCRIPT) # diff --git a/src/bhyve/Makefile.inc.am b/src/bhyve/Makefile.inc.am index 195069872a..7e49a8bb5c 100644 --- a/src/bhyve/Makefile.inc.am +++ b/src/bhyve/Makefile.inc.am @@ -77,7 +77,7 @@ bhyve/virtbhyved.aug: remote/libvirtd.aug.in $< > $@ bhyve/test_virtbhyved.aug: remote/test_libvirtd.aug.in \ - bhyve/virtbhyved.conf $(AUG_GENTEST) + bhyve/virtbhyved.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) bhyve/virtbhyved.conf \ $(srcdir)/remote/test_libvirtd.aug.in | \ $(SED) \ @@ -91,7 +91,7 @@ augeas_DATA += bhyve/libvirtd_bhyve.aug augeastest_DATA += bhyve/test_libvirtd_bhyve.aug bhyve/test_libvirtd_bhyve.aug: bhyve/test_libvirtd_bhyve.aug.in \ - $(srcdir)/bhyve/bhyve.conf $(AUG_GENTEST) + $(srcdir)/bhyve/bhyve.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) $(srcdir)/bhyve/bhyve.conf $< > $@ endif WITH_BHYVE diff --git a/src/interface/Makefile.inc.am b/src/interface/Makefile.inc.am index baa85b4ba9..7f161c487f 100644 --- a/src/interface/Makefile.inc.am +++ b/src/interface/Makefile.inc.am @@ -97,7 +97,7 @@ interface/virtinterfaced.aug: remote/libvirtd.aug.in $< > $@ interface/test_virtinterfaced.aug: remote/test_libvirtd.aug.in \ - interface/virtinterfaced.conf $(AUG_GENTEST) + interface/virtinterfaced.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) interface/virtinterfaced.conf \ $(srcdir)/remote/test_libvirtd.aug.in | \ $(SED) \ diff --git a/src/libxl/Makefile.inc.am b/src/libxl/Makefile.inc.am index c53396b7f3..3f35d1f2eb 100644 --- a/src/libxl/Makefile.inc.am +++ b/src/libxl/Makefile.inc.am @@ -111,7 +111,7 @@ libxl/virtxend.aug: remote/libvirtd.aug.in $< > $@ libxl/test_virtxend.aug: remote/test_libvirtd.aug.in \ - libxl/virtxend.conf $(AUG_GENTEST) + libxl/virtxend.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) libxl/virtxend.conf \ $(srcdir)/remote/test_libvirtd.aug.in | \ $(SED) \ @@ -125,7 +125,7 @@ augeas_DATA += libxl/libvirtd_libxl.aug augeastest_DATA += libxl/test_libvirtd_libxl.aug libxl/test_libvirtd_libxl.aug: libxl/test_libvirtd_libxl.aug.in \ - $(srcdir)/libxl/libxl.conf $(AUG_GENTEST) + $(srcdir)/libxl/libxl.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) $(srcdir)/libxl/libxl.conf $< > $@ INSTALL_DATA_DIRS += libxl diff --git a/src/locking/Makefile.inc.am b/src/locking/Makefile.inc.am index fae92a6e45..df0c51aaf6 100644 --- a/src/locking/Makefile.inc.am +++ b/src/locking/Makefile.inc.am @@ -219,7 +219,7 @@ endif WITH_SANLOCK if WITH_SANLOCK if WITH_QEMU locking/test_libvirt_sanlock.aug: locking/test_libvirt_sanlock.aug.in \ - locking/qemu-sanlock.conf $(AUG_GENTEST) + locking/qemu-sanlock.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) locking/qemu-sanlock.conf $< > $@ endif WITH_QEMU @@ -227,12 +227,12 @@ endif WITH_SANLOCK if WITH_QEMU locking/test_libvirt_lockd.aug: locking/test_libvirt_lockd.aug.in \ - locking/qemu-lockd.conf $(AUG_GENTEST) + locking/qemu-lockd.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) locking/qemu-lockd.conf $< > $@ endif WITH_QEMU locking/test_virtlockd.aug: locking/test_virtlockd.aug.in \ - locking/virtlockd.conf $(AUG_GENTEST) + locking/virtlockd.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) $(srcdir)/locking/virtlockd.conf $< > $@ endif WITH_LIBVIRTD diff --git a/src/logging/Makefile.inc.am b/src/logging/Makefile.inc.am index 7e441dbffb..ce777e278a 100644 --- a/src/logging/Makefile.inc.am +++ b/src/logging/Makefile.inc.am @@ -101,7 +101,7 @@ augeas_DATA += logging/virtlogd.aug augeastest_DATA += logging/test_virtlogd.aug logging/test_virtlogd.aug: logging/test_virtlogd.aug.in \ - logging/virtlogd.conf $(AUG_GENTEST) + logging/virtlogd.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) $(srcdir)/logging/virtlogd.conf $< > $@ endif WITH_LIBVIRTD diff --git a/src/lxc/Makefile.inc.am b/src/lxc/Makefile.inc.am index b4d560702c..0e12800886 100644 --- a/src/lxc/Makefile.inc.am +++ b/src/lxc/Makefile.inc.am @@ -164,7 +164,7 @@ lxc/virtlxcd.aug: remote/libvirtd.aug.in $< > $@ lxc/test_virtlxcd.aug: remote/test_libvirtd.aug.in \ - lxc/virtlxcd.conf $(AUG_GENTEST) + lxc/virtlxcd.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) lxc/virtlxcd.conf \ $(srcdir)/remote/test_libvirtd.aug.in | \ $(SED) \ @@ -222,7 +222,7 @@ augeas_DATA += lxc/libvirtd_lxc.aug augeastest_DATA += lxc/test_libvirtd_lxc.aug lxc/test_libvirtd_lxc.aug: lxc/test_libvirtd_lxc.aug.in \ - $(srcdir)/lxc/lxc.conf $(AUG_GENTEST) + $(srcdir)/lxc/lxc.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) $(srcdir)/lxc/lxc.conf $< > $@ INSTALL_DATA_DIRS += lxc diff --git a/src/network/Makefile.inc.am b/src/network/Makefile.inc.am index 17467a65ad..0ac0e03957 100644 --- a/src/network/Makefile.inc.am +++ b/src/network/Makefile.inc.am @@ -104,7 +104,7 @@ network/virtnetworkd.aug: remote/libvirtd.aug.in $< > $@ network/test_virtnetworkd.aug: remote/test_libvirtd.aug.in \ - network/virtnetworkd.conf $(AUG_GENTEST) + network/virtnetworkd.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) network/virtnetworkd.conf \ $(srcdir)/remote/test_libvirtd.aug.in | \ $(SED) \ diff --git a/src/node_device/Makefile.inc.am b/src/node_device/Makefile.inc.am index eac7f92e88..a42cf8bc6a 100644 --- a/src/node_device/Makefile.inc.am +++ b/src/node_device/Makefile.inc.am @@ -120,7 +120,7 @@ node_device/virtnodedevd.aug: remote/libvirtd.aug.in $< > $@ node_device/test_virtnodedevd.aug: remote/test_libvirtd.aug.in \ - node_device/virtnodedevd.conf $(AUG_GENTEST) + node_device/virtnodedevd.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) node_device/virtnodedevd.conf \ $(srcdir)/remote/test_libvirtd.aug.in | \ $(SED) \ diff --git a/src/nwfilter/Makefile.inc.am b/src/nwfilter/Makefile.inc.am index 6acb45705c..45be065f7c 100644 --- a/src/nwfilter/Makefile.inc.am +++ b/src/nwfilter/Makefile.inc.am @@ -105,7 +105,7 @@ nwfilter/virtnwfilterd.aug: remote/libvirtd.aug.in $< > $@ nwfilter/test_virtnwfilterd.aug: remote/test_libvirtd.aug.in \ - nwfilter/virtnwfilterd.conf $(AUG_GENTEST) + nwfilter/virtnwfilterd.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) nwfilter/virtnwfilterd.conf \ $(srcdir)/remote/test_libvirtd.aug.in | \ $(SED) \ diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 48fd0332ec..e9794520cf 100644 --- a/src/qemu/Makefile.inc.am +++ b/src/qemu/Makefile.inc.am @@ -169,7 +169,7 @@ qemu/virtqemud.aug: remote/libvirtd.aug.in $< > $@ qemu/test_virtqemud.aug: remote/test_libvirtd.aug.in \ - qemu/virtqemud.conf $(AUG_GENTEST) + qemu/virtqemud.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) qemu/virtqemud.conf \ $(srcdir)/remote/test_libvirtd.aug.in | \ $(SED) \ @@ -184,7 +184,7 @@ augeas_DATA += qemu/libvirtd_qemu.aug augeastest_DATA += qemu/test_libvirtd_qemu.aug qemu/test_libvirtd_qemu.aug: qemu/test_libvirtd_qemu.aug.in \ - $(srcdir)/qemu/qemu.conf $(AUG_GENTEST) + $(srcdir)/qemu/qemu.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) $(srcdir)/qemu/qemu.conf $< > $@ INSTALL_DATA_DIRS += qemu diff --git a/src/remote/Makefile.inc.am b/src/remote/Makefile.inc.am index 5a5c90a922..b42b700488 100644 --- a/src/remote/Makefile.inc.am +++ b/src/remote/Makefile.inc.am @@ -285,7 +285,7 @@ remote/virtproxyd.aug: remote/libvirtd.aug.in $< > $@ remote/test_libvirtd.aug: remote/test_libvirtd.aug.in \ - remote/libvirtd.conf $(AUG_GENTEST) + remote/libvirtd.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) remote/libvirtd.conf \ $(srcdir)/remote/test_libvirtd.aug.in | \ $(SED) \ @@ -298,7 +298,7 @@ remote/test_libvirtd.aug: remote/test_libvirtd.aug.in \ remote/test_virtproxyd.aug: remote/test_libvirtd.aug.in \ - remote/virtproxyd.conf $(AUG_GENTEST) + remote/virtproxyd.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) remote/virtproxyd.conf \ $(srcdir)/remote/test_libvirtd.aug.in | \ $(SED) \ diff --git a/src/secret/Makefile.inc.am b/src/secret/Makefile.inc.am index 76bc67418c..1020fb64c8 100644 --- a/src/secret/Makefile.inc.am +++ b/src/secret/Makefile.inc.am @@ -93,7 +93,7 @@ secret/virtsecretd.aug: remote/libvirtd.aug.in $< > $@ secret/test_virtsecretd.aug: remote/test_libvirtd.aug.in \ - secret/virtsecretd.conf $(AUG_GENTEST) + secret/virtsecretd.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) secret/virtsecretd.conf \ $(srcdir)/remote/test_libvirtd.aug.in | \ $(SED) \ diff --git a/src/storage/Makefile.inc.am b/src/storage/Makefile.inc.am index 4dccb14ac1..28ddfeec72 100644 --- a/src/storage/Makefile.inc.am +++ b/src/storage/Makefile.inc.am @@ -197,7 +197,7 @@ storage/virtstoraged.aug: remote/libvirtd.aug.in $< > $@ storage/test_virtstoraged.aug: remote/test_libvirtd.aug.in \ - storage/virtstoraged.conf $(AUG_GENTEST) + storage/virtstoraged.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) storage/virtstoraged.conf \ $(srcdir)/remote/test_libvirtd.aug.in | \ $(SED) \ diff --git a/src/vbox/Makefile.inc.am b/src/vbox/Makefile.inc.am index 178c360b99..1f2bcb5488 100644 --- a/src/vbox/Makefile.inc.am +++ b/src/vbox/Makefile.inc.am @@ -117,7 +117,7 @@ vbox/virtvboxd.aug: remote/libvirtd.aug.in $< > $@ vbox/test_virtvboxd.aug: remote/test_libvirtd.aug.in \ - vbox/virtvboxd.conf $(AUG_GENTEST) + vbox/virtvboxd.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) vbox/virtvboxd.conf \ $(srcdir)/remote/test_libvirtd.aug.in | \ $(SED) \ diff --git a/src/vz/Makefile.inc.am b/src/vz/Makefile.inc.am index f56fceb8f7..fa7b8b79dd 100644 --- a/src/vz/Makefile.inc.am +++ b/src/vz/Makefile.inc.am @@ -91,7 +91,7 @@ vz/virtvzd.aug: remote/libvirtd.aug.in $< > $@ vz/test_virtvzd.aug: remote/test_libvirtd.aug.in \ - vz/virtvzd.conf $(AUG_GENTEST) + vz/virtvzd.conf $(AUG_GENTEST_SCRIPT) $(AM_V_GEN)$(AUG_GENTEST) vz/virtvzd.conf \ $(srcdir)/remote/test_libvirtd.aug.in | \ $(SED) \ -- 2.21.0

On Tue, Sep 24, 2019 at 03:58:44PM +0100, Daniel P. Berrangé wrote: I guess the fact !$fuzzy was missing in this condition was a bug that the new python code doesn't suffer from, right? question 1) what's the benefit of compiling a regex and using it only once? Btw python does cache every pattern passed to re.match (and friends) so compilation IMO hardly ever makes sense unless you're doing 1000s of searches for the same pattern in which case the latency would naturally accumulate. question 2) why do we need the '''.* and .*''' parts compared to the original perl regex? I'll go ahead and assume it's because re.match matches at the beginning of a string by default, in which case, shouldn't we use re.search which matches anywhere (that's what perl does by default) instead? Erik

As part of an goal to eliminate Perl from libvirt build tools, rewrite the prohibit-duplicate-header.pl tool in Python. This was 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. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- Makefile.am | 2 +- build-aux/prohibit-duplicate-header.pl | 26 ------------ build-aux/prohibit-duplicate-header.py | 57 ++++++++++++++++++++++++++ cfg.mk | 4 +- 4 files changed, 60 insertions(+), 29 deletions(-) delete mode 100644 build-aux/prohibit-duplicate-header.pl create mode 100644 build-aux/prohibit-duplicate-header.py diff --git a/Makefile.am b/Makefile.am index 8f688d40d0..a7d8da7146 100644 --- a/Makefile.am +++ b/Makefile.am @@ -47,7 +47,7 @@ EXTRA_DIST = \ build-aux/header-ifdef.pl \ build-aux/minimize-po.py \ build-aux/mock-noinline.pl \ - build-aux/prohibit-duplicate-header.pl \ + build-aux/prohibit-duplicate-header.py \ build-aux/useless-if-before-free \ build-aux/vc-list-files \ ci/Makefile \ diff --git a/build-aux/prohibit-duplicate-header.pl b/build-aux/prohibit-duplicate-header.pl deleted file mode 100644 index 4a2ea65665..0000000000 --- a/build-aux/prohibit-duplicate-header.pl +++ /dev/null @@ -1,26 +0,0 @@ -#!/usr/bin/env perl - -use strict; - -my $file = " "; -my $ret = 0; -my %includes = ( ); -my $lineno = 0; - -while (<>) { - if (not $file eq $ARGV) { - %includes = ( ); - $file = $ARGV; - $lineno = 0; - } - $lineno++; - if (/^# *include *[<"]([^>"]*\.h)[">]/) { - $includes{$1}++; - if ($includes{$1} == 2) { - $ret = 1; - print STDERR "$ARGV:$lineno: $_"; - print STDERR "Do not include a header more than once per file\n"; - } - } -} -exit $ret; diff --git a/build-aux/prohibit-duplicate-header.py b/build-aux/prohibit-duplicate-header.py new file mode 100644 index 0000000000..5ed3f82ba7 --- /dev/null +++ b/build-aux/prohibit-duplicate-header.py @@ -0,0 +1,57 @@ +#!/usr/bin/env python +# +# Copyright (C) 2016-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 + + +def check_file(filename): + includes = {} + lineno = 0 + errs = False + with open(filename, "r") as fh: + headerprog = re.compile(r'''^# *include *[<"]([^>"]*\.h)[">]\s*$''') + for line in fh: + lineno = lineno + 1 + + headermatch = headerprog.match(line) + if headermatch is not None: + inc = headermatch.group(1) + + if inc in includes: + print("%s:%d: %s" % (filename, lineno, inc), + file=sys.stderr) + errs = True + else: + includes[inc] = True + + return errs + + +ret = 0 + +for filename in sys.argv[1:]: + if check_file(filename): + ret = 1 + +if ret == 1: + print("Do not include a header more than once per file", file=sys.stderr) + +sys.exit(ret) diff --git a/cfg.mk b/cfg.mk index 3731a2830b..bf4a8270d8 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1152,8 +1152,8 @@ endif # Don't include duplicate header in the source (either *.c or *.h) prohibit-duplicate-header: - $(AM_V_GEN)$(VC_LIST_EXCEPT) | $(GREP) '\.[chx]$$' | xargs \ - $(PERL) -W $(top_srcdir)/build-aux/prohibit-duplicate-header.pl + $(AM_V_GEN)$(VC_LIST_EXCEPT) | $(GREP) '\.[chx]$$' | $(RUNUTF8) xargs \ + $(PYTHON) $(top_srcdir)/build-aux/prohibit-duplicate-header.py spacing-check: $(AM_V_GEN)$(VC_LIST) | $(GREP) '\.c$$' | xargs \ -- 2.21.0

On Thu, Sep 26, 2019 at 02:46:50PM +0200, Erik Skultety wrote: I'm biased against referencing global variables from within methods, because I feel it leads to harder to understand code when there's a mix of local & global vars used. It especially makes later refactoring of code more error prone as code being refactored interacts with global state in unpredictable ways. Now 'errs' is quite a simple usage pattern, but I'd still prefer to avoid mixing in globals. 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 :|

On Tue, Sep 24, 2019 at 03:58:46PM +0100, Daniel P. Berrangé wrote: I think even with descriptive names for the regexes and the Python rewrite, this script is hard to read and comes too close to be a C parser. Also, the execution time goes from 1.5s to 6.5s, which is longer than all the other checks combined run on my 8-core machine. Can we get rid of it completely in favor of some proper formatting tool? I have played with clang-format to try to match our style, the main problems seem to be: * pre-processor directives are indented by the same offset as code (I see that cppi is specifically intended to add just one space) * function calls sometimes leave an empty opening parenthesis * always rewrapping function arguments might create unnecessary churn * parameters wrapping might not be smart enough, e.g. we like to do virReportError(VIR_ERR_CODE, "%s", _("string")); in a lot of places. On the plus side, we would be able to properly check for spacing after casts. Jano

On Fri, Sep 27, 2019 at 10:33:45AM +0100, Daniel P. Berrangé wrote: Another tool is "uncrustify" which seems to have even more configurability than clang-format does. On my SSD it takes 30 seconds to run over all the .c file. 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 :|

On Fri, Sep 27, 2019 at 11:42:28AM +0100, Daniel P. Berrangé wrote: The attached lv.cfg file is an uncrustify config that is reasonably close to our current style. To see what it does, run this in your source tree: find src/ -name '*.c' | xargs uncrustify --replace -c uncrustify.cfg There's quite a few real mistakes it is correcting for us. Most of the big stuff is due to us having followed inconsistent rules in different parts of the source tree. No matter which tool we pick will suffer this as we have to pick one style switch/case indents are the big one - sometimes we line up case + switch, sometimes we don't. src/access/viraccessdriverpolkit.c | 16 +-- src/admin/admin_server_dispatch.c | 4 +- src/bhyve/bhyve_capabilities.c | 2 +- src/bhyve/bhyve_command.c | 66 ++++++------- src/bhyve/bhyve_driver.c | 14 +-- src/bhyve/bhyve_monitor.c | 4 +- src/bhyve/bhyve_parse_command.c | 96 +++++++++--------- src/bhyve/bhyve_process.c | 32 +++--- src/conf/capabilities.c | 2 +- src/conf/checkpoint_conf.c | 2 +- src/conf/cpu_conf.c | 8 +- src/conf/device_conf.c | 18 ++-- src/conf/domain_addr.c | 10 +- src/conf/domain_audit.c | 8 +- src/conf/domain_capabilities.c | 6 +- src/conf/domain_conf.c | 414 +++++++++++++++++++++++++++++++++++++++--------------------------------------- src/conf/domain_event.c | 434 +++++++++++++++++++++++++++++++++++++++++----------------------------------------- src/conf/interface_conf.c | 182 +++++++++++++++++------------------ src/conf/netdev_vlan_conf.c | 2 +- src/conf/network_conf.c | 60 ++++++------ src/conf/network_event.c | 20 ++-- src/conf/node_device_conf.c | 20 ++-- src/conf/node_device_event.c | 30 +++--- src/conf/node_device_util.c | 2 +- src/conf/numa_conf.c | 4 +- src/conf/nwfilter_conf.c | 684 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------- src/conf/nwfilter_ipaddrmap.c | 2 +- src/conf/nwfilter_params.c | 12 +-- src/conf/secret_event.c | 30 +++--- src/conf/snapshot_conf.c | 8 +- src/conf/storage_adapter_conf.c | 6 +- src/conf/storage_conf.c | 76 ++++++--------- src/conf/storage_event.c | 32 +++--- src/conf/virchrdev.c | 32 +++--- src/conf/virinterfaceobj.c | 9 +- src/conf/virnetworkobj.c | 18 ++-- src/conf/virnetworkportdef.c | 12 +-- src/conf/virnodedeviceobj.c | 21 ++-- src/conf/virnwfilterobj.c | 2 +- src/conf/virsecretobj.c | 9 +- src/conf/virstorageobj.c | 24 +++-- src/cpu/cpu_ppc64.c | 8 +- src/cpu/cpu_s390.c | 8 +- src/cpu/cpu_x86.c | 76 +++++++-------- src/datatypes.c | 2 +- src/esx/esx_driver.c | 144 +++++++++++++-------------- src/esx/esx_interface_driver.c | 2 +- src/esx/esx_network_driver.c | 62 ++++++------ src/esx/esx_storage_backend_iscsi.c | 14 +-- src/esx/esx_storage_backend_vmfs.c | 30 +++--- src/esx/esx_storage_driver.c | 4 +- src/esx/esx_util.c | 6 +- src/esx/esx_vi.c | 332 +++++++++++++++++++++++++++++++-------------------------------- src/esx/esx_vi_methods.c | 58 +++++------ src/esx/esx_vi_types.c | 402 ++++++++++++++++++++++++++++++++++++++-------------------------------------- src/hyperv/hyperv_driver.c | 84 ++++++++-------- src/hyperv/hyperv_wmi.c | 374 +++++++++++++++++++++++++++++++++++----------------------------------- src/interface/interface_backend_netcf.c | 80 +++++++-------- src/interface/interface_backend_udev.c | 126 ++++++++++++------------ src/keycodemapdb | 0 src/libvirt-domain.c | 110 ++++++++++----------- src/libvirt-host.c | 4 +- src/libvirt-interface.c | 6 +- src/libvirt-lxc.c | 4 +- src/libvirt-network.c | 2 +- src/libvirt-nodedev.c | 4 +- src/libvirt-storage.c | 10 +- src/libvirt.c | 18 ++-- src/libxl/libxl_capabilities.c | 11 ++- src/libxl/libxl_conf.c | 404 ++++++++++++++++++++++++++++++++++++++-------------------------------------- src/libxl/libxl_domain.c | 40 ++++---- src/libxl/libxl_driver.c | 636 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------ src/libxl/libxl_migration.c | 30 +++--- src/libxl/xen_common.c | 38 ++++---- src/libxl/xen_xl.c | 184 +++++++++++++++++------------------ src/libxl/xen_xm.c | 54 +++++------ src/locking/domain_lock.c | 24 ++--- src/locking/lock_daemon.c | 126 ++++++++++++------------ src/locking/lock_manager.c | 4 +- src/logging/log_daemon.c | 122 +++++++++++------------ src/lxc/lxc_cgroup.c | 5 +- src/lxc/lxc_container.c | 14 +-- src/lxc/lxc_controller.c | 6 +- src/lxc/lxc_domain.c | 16 +-- src/lxc/lxc_driver.c | 94 +++++++++--------- src/lxc/lxc_fuse.c | 2 +- src/lxc/lxc_native.c | 18 ++-- src/lxc/lxc_process.c | 34 +++---- src/network/bridge_driver.c | 52 +++++----- src/node_device/node_device_hal.c | 6 +- src/node_device/node_device_udev.c | 6 +- src/nwfilter/nwfilter_dhcpsnoop.c | 104 ++++++++++---------- src/nwfilter/nwfilter_driver.c | 12 +-- src/nwfilter/nwfilter_ebiptables_driver.c | 160 +++++++++++++++--------------- src/nwfilter/nwfilter_gentech_driver.c | 2 +- src/nwfilter/nwfilter_learnipaddr.c | 42 ++++---- src/openvz/openvz_conf.c | 10 +- src/openvz/openvz_driver.c | 25 +++-- src/phyp/phyp_driver.c | 80 +++++++-------- src/qemu/qemu_agent.c | 32 +++--- src/qemu/qemu_alias.c | 4 +- src/qemu/qemu_block.c | 27 +++--- src/qemu/qemu_blockjob.c | 8 +- src/qemu/qemu_capabilities.c | 20 ++-- src/qemu/qemu_cgroup.c | 22 ++--- src/qemu/qemu_command.c | 574 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------ src/qemu/qemu_conf.c | 54 +++++------ src/qemu/qemu_domain.c | 378 +++++++++++++++++++++++++++++++++++------------------------------------ src/qemu/qemu_domain_address.c | 36 +++---- src/qemu/qemu_driver.c | 526 +++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------- src/qemu/qemu_hostdev.c | 2 +- src/qemu/qemu_hotplug.c | 72 +++++++------- src/qemu/qemu_interface.c | 14 +-- src/qemu/qemu_migration.c | 104 ++++++++++---------- src/qemu/qemu_migration_cookie.c | 16 +-- src/qemu/qemu_migration_params.c | 4 +- src/qemu/qemu_monitor.c | 12 +-- src/qemu/qemu_monitor_json.c | 42 ++++---- src/qemu/qemu_monitor_text.c | 6 +- src/qemu/qemu_process.c | 94 +++++++++--------- src/qemu/qemu_security.c | 8 +- src/qemu/qemu_tpm.c | 34 +++---- src/remote/remote_daemon.c | 148 ++++++++++++++-------------- src/remote/remote_daemon_config.c | 6 +- src/remote/remote_daemon_dispatch.c | 22 ++--- src/remote/remote_driver.c | 28 +++--- src/rpc/virnetclient.c | 44 ++++----- src/rpc/virnetclientstream.c | 8 +- src/rpc/virnetlibsshsession.c | 4 +- src/rpc/virnetserverclient.c | 2 +- src/rpc/virnetserverprogram.c | 2 +- src/rpc/virnetserverservice.c | 2 +- src/rpc/virnetsocket.c | 32 +++--- src/rpc/virnetsshsession.c | 8 +- src/rpc/virnettlscontext.c | 4 +- src/secret/secret_driver.c | 2 +- src/secret/secret_util.c | 6 +- src/security/security_apparmor.c | 28 +++--- src/security/security_dac.c | 8 +- src/security/security_manager.c | 16 +-- src/security/security_selinux.c | 6 +- src/security/security_util.c | 4 +- src/security/virt-aa-helper.c | 204 +++++++++++++++++++-------------------- src/storage/storage_backend_disk.c | 92 +++++++++--------- src/storage/storage_backend_fs.c | 4 +- src/storage/storage_backend_gluster.c | 5 +- src/storage/storage_backend_logical.c | 28 +++--- src/storage/storage_backend_mpath.c | 4 +- src/storage/storage_backend_rbd.c | 42 ++++---- src/storage/storage_driver.c | 58 +++++------ src/storage/storage_util.c | 16 +-- src/test/test_driver.c | 456 +++++++++++++++++++++++++++++++++++++++++++------------------------------------------- src/util/viralloc.c | 4 +- src/util/virauthconfig.c | 4 +- src/util/virbuffer.c | 2 +- src/util/vircgroup.c | 22 ++--- src/util/vircommand.c | 6 +- src/util/virconf.c | 84 ++++++++-------- src/util/vircrypto.c | 4 +- src/util/virdbus.c | 4 +- src/util/virdnsmasq.c | 6 +- src/util/virerror.c | 357 +++++++++++++++++++++++++++++++++++++++++++------------------------ src/util/virevent.c | 12 +-- src/util/vireventpoll.c | 4 +- src/util/virfdstream.c | 8 +- src/util/virfile.c | 12 +-- src/util/virfirewall.c | 2 +- src/util/virfirewalld.c | 6 +- src/util/virhook.c | 28 +++--- src/util/virhostdev.c | 14 +-- src/util/virinitctl.c | 46 ++++----- src/util/virlease.c | 4 +- src/util/virlog.c | 38 ++++---- src/util/virmacaddr.c | 2 +- src/util/virnetdev.c | 140 +++++++++++++-------------- src/util/virnetdevbandwidth.c | 6 +- src/util/virnetdevbridge.c | 14 +-- src/util/virnetdevip.c | 8 +- src/util/virnetdevmacvlan.c | 14 +-- src/util/virnetdevvportprofile.c | 14 +-- src/util/virnetlink.c | 14 +-- src/util/virnodesuspend.c | 8 +- src/util/virnuma.c | 2 +- src/util/virpci.c | 20 ++-- src/util/virprocess.c | 22 ++--- src/util/virresctrl.c | 2 +- src/util/virrotatingfile.c | 2 +- src/util/virscsivhost.c | 2 +- src/util/virsocketaddr.c | 14 +-- src/util/virstorageencryption.c | 4 +- src/util/virstoragefile.c | 93 +++++++++--------- src/util/virsysinfo.c | 6 +- src/util/virsystemd.c | 12 +-- src/util/virtime.c | 10 +- src/util/virtpm.c | 8 +- src/util/virtypedparam.c | 18 ++-- src/util/viruri.c | 4 +- src/util/virusb.c | 8 +- src/util/virutil.c | 2 +- src/util/virvhba.c | 14 +-- src/util/virxml.c | 9 +- src/vbox/vbox_V5_0.c | 2 +- src/vbox/vbox_V5_1.c | 2 +- src/vbox/vbox_V5_2.c | 2 +- src/vbox/vbox_common.c | 250 +++++++++++++++++++++++------------------------ src/vbox/vbox_network.c | 2 +- src/vbox/vbox_snapshot_conf.c | 106 ++++++++++---------- src/vbox/vbox_storage.c | 2 +- src/vbox/vbox_tmpl.c | 86 ++++++++--------- src/vmware/vmware_conf.c | 68 ++++++------- src/vmware/vmware_driver.c | 8 +- src/vmx/vmx.c | 652 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------------------- src/vz/vz_driver.c | 64 ++++++------ src/vz/vz_sdk.c | 94 +++++++++--------- src/vz/vz_utils.c | 8 +- 216 files changed, 6371 insertions(+), 6256 deletions(-) If anyone fancies taking this further feel free. I'm not going to work on uncrustify right now. I'll just drop this whitespace patch rewrite and focus on the other things as a priority. 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 :|

As part of an goal to eliminate Perl from libvirt build tools, rewrite the header-ifdef.pl tool in Python. This was 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. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- Makefile.am | 2 +- build-aux/header-ifdef.pl | 182 ------------------------------ build-aux/header-ifdef.py | 231 ++++++++++++++++++++++++++++++++++++++ cfg.mk | 4 +- 4 files changed, 234 insertions(+), 185 deletions(-) delete mode 100644 build-aux/header-ifdef.pl create mode 100644 build-aux/header-ifdef.py diff --git a/Makefile.am b/Makefile.am index db77da890c..4ba729d3f7 100644 --- a/Makefile.am +++ b/Makefile.am @@ -44,7 +44,7 @@ EXTRA_DIST = \ build-aux/augeas-gentest.py \ build-aux/check-spacing.py \ build-aux/gitlog-to-changelog \ - build-aux/header-ifdef.pl \ + build-aux/header-ifdef.py \ build-aux/minimize-po.py \ build-aux/mock-noinline.py \ build-aux/prohibit-duplicate-header.py \ diff --git a/build-aux/header-ifdef.pl b/build-aux/header-ifdef.pl deleted file mode 100644 index dba3dbcbdc..0000000000 --- a/build-aux/header-ifdef.pl +++ /dev/null @@ -1,182 +0,0 @@ -#!/usr/bin/perl -# -# Validate that header files follow a standard layout: -# -# /* -# ...copyright header... -# */ -# <one blank line> -# #pragma once -# ....content.... -# -#--- -# -# For any file ending priv.h, before the #pragma once -# We will have a further section -# -# #ifndef SYMBOL_ALLOW -# # error .... -# #endif /* SYMBOL_ALLOW */ -# <one blank line> -# -#--- -# -# For public headers (files in include/), use the standard header guard instead of #pragma once: -# #ifndef SYMBOL -# # define SYMBOL -# ....content.... -# #endif /* SYMBOL */ - -use strict; -use warnings; - -my $STATE_COPYRIGHT_COMMENT = 0; -my $STATE_COPYRIGHT_BLANK = 1; -my $STATE_PRIV_START = 2; -my $STATE_PRIV_ERROR = 3; -my $STATE_PRIV_END = 4; -my $STATE_PRIV_BLANK = 5; -my $STATE_GUARD_START = 6; -my $STATE_GUARD_DEFINE = 7; -my $STATE_GUARD_END = 8; -my $STATE_EOF = 9; -my $STATE_PRAGMA = 10; - -my $file = " "; -my $ret = 0; -my $ifdef = ""; -my $ifdefpriv = ""; -my $publicheader = 0; - -my $state = $STATE_EOF; -my $mistake = 0; - -sub mistake { - my $msg = shift; - warn $msg; - $mistake = 1; - $ret = 1; -} - -while (<>) { - if (not $file eq $ARGV) { - if ($state == $STATE_COPYRIGHT_COMMENT) { - &mistake("$file: missing copyright comment"); - } elsif ($state == $STATE_COPYRIGHT_BLANK) { - &mistake("$file: missing blank line after copyright header"); - } elsif ($state == $STATE_PRIV_START) { - &mistake("$file: missing '#ifndef $ifdefpriv'"); - } elsif ($state == $STATE_PRIV_ERROR) { - &mistake("$file: missing '# error ...priv allow...'"); - } elsif ($state == $STATE_PRIV_END) { - &mistake("$file: missing '#endif /* $ifdefpriv */'"); - } elsif ($state == $STATE_PRIV_BLANK) { - &mistake("$file: missing blank line after priv header check"); - } elsif ($state == $STATE_GUARD_START) { - if ($publicheader) { - &mistake("$file: missing '#ifndef $ifdef'"); - } else { - &mistake("$file: missing '#pragma once' header guard"); - } - } elsif ($state == $STATE_GUARD_DEFINE) { - &mistake("$file: missing '# define $ifdef'"); - } elsif ($state == $STATE_GUARD_END) { - &mistake("$file: missing '#endif /* $ifdef */'"); - } - - $ifdef = uc $ARGV; - $ifdef =~ s,.*/,,; - $ifdef =~ s,[^A-Z0-9],_,g; - $ifdef =~ s,__+,_,g; - unless ($ifdef =~ /^LIBVIRT_/ && $ARGV !~ /libvirt_internal.h/) { - $ifdef = "LIBVIRT_" . $ifdef; - } - $ifdefpriv = $ifdef . "_ALLOW"; - - $file = $ARGV; - $state = $STATE_COPYRIGHT_COMMENT; - $mistake = 0; - $publicheader = ($ARGV =~ /include\//); - } - - if ($mistake || - $ARGV =~ /config-post\.h$/ || - $ARGV =~ /vbox_(CAPI|XPCOM)/) { - $state = $STATE_EOF; - next; - } - - if ($state == $STATE_COPYRIGHT_COMMENT) { - if (m,\*/,) { - $state = $STATE_COPYRIGHT_BLANK; - } - } elsif ($state == $STATE_COPYRIGHT_BLANK) { - if (! /^$/) { - &mistake("$file: missing blank line after copyright header"); - } - if ($ARGV =~ /priv\.h$/) { - $state = $STATE_PRIV_START; - } else { - $state = $STATE_GUARD_START; - } - } elsif ($state == $STATE_PRIV_START) { - if (/^$/) { - &mistake("$file: too many blank lines after copyright header"); - } elsif (/#ifndef $ifdefpriv$/) { - $state = $STATE_PRIV_ERROR; - } else { - &mistake("$file: missing '#ifndef $ifdefpriv'"); - } - } elsif ($state == $STATE_PRIV_ERROR) { - if (/# error ".*"$/) { - $state = $STATE_PRIV_END; - } else { - &mistake("$file: missing '# error ...priv allow...'"); - } - } elsif ($state == $STATE_PRIV_END) { - if (m,#endif /\* $ifdefpriv \*/,) { - $state = $STATE_PRIV_BLANK; - } else { - &mistake("$file: missing '#endif /* $ifdefpriv */'"); - } - } elsif ($state == $STATE_PRIV_BLANK) { - if (! /^$/) { - &mistake("$file: missing blank line after priv guard"); - } - $state = $STATE_GUARD_START; - } elsif ($state == $STATE_GUARD_START) { - if (/^$/) { - &mistake("$file: too many blank lines after copyright header"); - } - if ($publicheader) { - if (/#ifndef $ifdef$/) { - $state = $STATE_GUARD_DEFINE; - } else { - &mistake("$file: missing '#ifndef $ifdef'"); - } - } else { - if (/#pragma once/) { - $state = $STATE_PRAGMA; - } else { - &mistake("$file: missing '#pragma once' header guard"); - } - } - } elsif ($state == $STATE_GUARD_DEFINE) { - if (/# define $ifdef$/) { - $state = $STATE_GUARD_END; - } else { - &mistake("$file: missing '# define $ifdef'"); - } - } elsif ($state == $STATE_GUARD_END) { - if (m,#endif /\* $ifdef \*/$,) { - $state = $STATE_EOF; - } - } elsif ($state == $STATE_PRAGMA) { - next; - } elsif ($state == $STATE_EOF) { - die "$file: unexpected content after '#endif /* $ifdef */'"; - } else { - die "$file: unexpected state $state"; - } -} -exit $ret; diff --git a/build-aux/header-ifdef.py b/build-aux/header-ifdef.py new file mode 100644 index 0000000000..a192e01bc4 --- /dev/null +++ b/build-aux/header-ifdef.py @@ -0,0 +1,231 @@ +#!/usr/bin/env python +# +# Copyright (C) 2018-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/>;. +# +# Validate that header files follow a standard layout: +# +# /* +# ...copyright header... +# */ +# <one blank line> +# #pragma once +# ....content.... +# +# --- +# +# For any file ending priv.h, before the #pragma once +# We will have a further section +# +# #ifndef SYMBOL_ALLOW +# # error .... +# #endif /* SYMBOL_ALLOW */ +# <one blank line> +# +# --- +# +# For public headers (files in include/), use the standard +# header guard instead of #pragma once: +# #ifndef SYMBOL +# # define SYMBOL +# ....content.... +# #endif /* SYMBOL */ + +from __future__ import print_function + +import os.path +import re +import sys + +STATE_COPYRIGHT_COMMENT = 0 +STATE_COPYRIGHT_BLANK = 1 +STATE_PRIV_START = 2 +STATE_PRIV_ERROR = 3 +STATE_PRIV_END = 4 +STATE_PRIV_BLANK = 5 +STATE_GUARD_START = 6 +STATE_GUARD_DEFINE = 7 +STATE_GUARD_END = 8 +STATE_EOF = 9 +STATE_PRAGMA = 10 + + +def check_header(filename): + ifdef = "" + ifdefpriv = "" + + state = STATE_EOF + + ifdef = os.path.basename(filename).upper() + ifdef = re.sub(r"""[^A-Z0-9]""", "_", ifdef) + ifdef = re.sub(r"""__+""", "_", ifdef) + + if (not ifdef.startswith("LIBVIRT_") or + filename.find("libvirt_internal.h") != -1): + ifdef = "LIBVIRT_" + ifdef + + ifdefpriv = ifdef + "_ALLOW" + + state = STATE_COPYRIGHT_COMMENT + publicheader = False + if filename.find("include/") != -1: + publicheader = True + + with open(filename, "r") as fh: + for line in fh: + line = line.rstrip("\n") + if state == STATE_COPYRIGHT_COMMENT: + if line.find("*/") != -1: + state = STATE_COPYRIGHT_BLANK + elif state == STATE_COPYRIGHT_BLANK: + if line != "": + print("%s: missing blank line after copyright header" % + filename, file=sys.stderr) + return True + + if filename.endswith("priv.h"): + state = STATE_PRIV_START + else: + state = STATE_GUARD_START + elif state == STATE_PRIV_START: + if line == "": + print("%s: too many blank lines after copyright header" % + filename, file=sys.stderr) + return True + elif re.match(r"""#ifndef %s$""" % ifdefpriv, line): + state = STATE_PRIV_ERROR + else: + print("%s: missing '#ifndef %s'" % (filename, ifdefpriv), + file=sys.stderr) + return True + elif state == STATE_PRIV_ERROR: + if re.match(r"""# error ".*"$""", line): + state = STATE_PRIV_END + else: + print("%s: missing '# error ...priv allow...'" % + filename, file=sys.stderr) + return True + elif state == STATE_PRIV_END: + if re.match(r"""#endif /\* %s \*/""" % ifdefpriv, line): + state = STATE_PRIV_BLANK + else: + print("%s: missing '#endif /* %s */'" % + (filename, ifdefpriv), file=sys.stderr) + return True + elif state == STATE_PRIV_BLANK: + if line != "": + print("%s: missing blank line after priv guard" % + filename, file=sys.stderr) + return True + state = STATE_GUARD_START + elif state == STATE_GUARD_START: + if line == "": + print("%s: too many blank lines after copyright header" % + filename, file=sys.stderr) + return True + if publicheader: + if re.match(r"""#ifndef %s$""" % ifdef, line): + state = STATE_GUARD_DEFINE + else: + print("%s: missing '#ifndef %s'" % + (filename, ifdef), file=sys.stderr) + return True + else: + if re.match(r"""#pragma once""", line): + state = STATE_PRAGMA + else: + print("%s: missing '#pragma once' header guard" % + filename, file=sys.stderr) + return True + elif state == STATE_GUARD_DEFINE: + if re.match(r"""# define %s$""" % ifdef, line): + state = STATE_GUARD_END + else: + print("%s: missing '# define %s'" % + (filename, ifdef), file=sys.stderr) + return True + elif state == STATE_GUARD_END: + if re.match(r"""#endif /\* %s \*/$""" % ifdef, line): + state = STATE_EOF + elif state == STATE_PRAGMA: + next + elif state == STATE_EOF: + print("%s: unexpected content after '#endif /* %s */'" % + (filename, ifdef), file=sys.stderr) + return True + else: + print("%s: unexpected state $state" % + filename, file=sys.stderr) + return True + + if state == STATE_COPYRIGHT_COMMENT: + print("%s: missing copyright comment" % + filename, file=sys.stderr) + return True + elif state == STATE_COPYRIGHT_BLANK: + print("%s: missing blank line after copyright header" % + filename, file=sys.stderr) + return True + elif state == STATE_PRIV_START: + print("%s: missing '#ifndef %s'" % + (filename, ifdefpriv), file=sys.stderr) + return True + elif state == STATE_PRIV_ERROR: + print("%s: missing '# error ...priv allow...'" % + filename, file=sys.stderr) + return True + elif state == STATE_PRIV_END: + print("%s: missing '#endif /* %s */'" % + (filename, ifdefpriv), file=sys.stderr) + return True + elif state == STATE_PRIV_BLANK: + print("%s: missing blank line after priv header check" % + filename, file=sys.stderr) + return True + elif state == STATE_GUARD_START: + if publicheader: + print("%s: missing '#ifndef %s'" % + (filename, ifdef), file=sys.stderr) + return True + else: + print("%s: missing '#pragma once' header guard" % + filename, file=sys.stderr) + return True + elif state == STATE_GUARD_DEFINE: + print("%s: missing '# define %s'" % + (filename, ifdef), file=sys.stderr) + return True + elif state == STATE_GUARD_END: + print("%s: missing '#endif /* %s */'" % + (filename, ifdef), file=sys.stderr) + return True + + return False + + +ret = 0 + +for filename in sys.argv[1:]: + if filename.find("config-post.h") != -1: + continue + if filename.find("vbox_CAPI") != -1: + continue + if filename.find("vbox_XPCOM") != -1: + continue + if check_header(filename): + ret = 1 + +sys.exit(ret) diff --git a/cfg.mk b/cfg.mk index b73d3ae1bf..f0aed0365a 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1165,8 +1165,8 @@ mock-noinline: $(PYTHON) $(top_srcdir)/build-aux/mock-noinline.py header-ifdef: - $(AM_V_GEN)$(VC_LIST) | $(GREP) '\.[h]$$' | xargs \ - $(PERL) $(top_srcdir)/build-aux/header-ifdef.pl + $(AM_V_GEN)$(VC_LIST) | $(GREP) '\.[h]$$' | $(RUNUTF8) xargs \ + $(PYTHON) $(top_srcdir)/build-aux/header-ifdef.py test-wrap-argv: $(AM_V_GEN)$(VC_LIST) | $(GREP) -E '\.(ldargs|args)' | xargs \ -- 2.21.0

As part of an goal to eliminate Perl from libvirt build tools, rewrite the check-symsorting.pl tool in Python. This was 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. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/Makefile.am | 6 +- src/check-symsorting.pl | 106 ----------------------------------- src/check-symsorting.py | 119 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 109 deletions(-) delete mode 100755 src/check-symsorting.pl create mode 100755 src/check-symsorting.py diff --git a/src/Makefile.am b/src/Makefile.am index 71858ea07f..7c3e9ab981 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -284,12 +284,12 @@ check-symfile: check-admin-symfile: endif ! WITH_LINUX check-symsorting: - $(AM_V_GEN)$(PERL) $(srcdir)/check-symsorting.pl \ + $(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(srcdir)/check-symsorting.py \ $(srcdir) $(SYM_FILES) check-admin-symsorting: - $(AM_V_GEN)$(PERL) $(srcdir)/check-symsorting.pl \ + $(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(srcdir)/check-symsorting.py \ $(srcdir) $(ADMIN_SYM_FILES) -EXTRA_DIST += check-symfile.pl check-symsorting.pl +EXTRA_DIST += check-symfile.pl check-symsorting.py # Keep this list synced with RPC_PROBE_FILES PROTOCOL_STRUCTS = \ diff --git a/src/check-symsorting.pl b/src/check-symsorting.pl deleted file mode 100755 index 51e38bdedb..0000000000 --- a/src/check-symsorting.pl +++ /dev/null @@ -1,106 +0,0 @@ -#!/usr/bin/env perl - -# Copyright (C) 2012-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; - -die "syntax: $0 SRCDIR SYMFILE..." unless int(@ARGV) >= 2; - -my $ret = 0; -my $srcdir = shift; -my $lastgroup = undef; -foreach my $symfile (@ARGV) { - open SYMFILE, $symfile or die "cannot read $symfile: $!"; - - my $line = 0; - my $groupfile = ""; - my @group; - - while (<SYMFILE>) { - chomp; - - if (/^#\s*((\w+\/)*(\w+\.h))\s*$/) { - $groupfile = $1; - } elsif (/^#/) { - # Ignore comments - } elsif (/^\s*$/) { - if (@group) { - &check_sorting(\@group, $symfile, $line, $groupfile); - } - @group = (); - $line = $.; - } else { - $_ =~ s/;//; - push @group, $_; - } - } - - close SYMFILE; - if (@group) { - &check_sorting(\@group, $symfile, $line, $groupfile); - } - $lastgroup = undef; -} - -sub check_sorting { - my $group = shift; - my $symfile = shift; - my $line = shift; - my $groupfile = shift; - - my @group = @{$group}; - my @sorted = sort { lc $a cmp lc $b } @group; - my $sorted = 1; - my $first; - my $last; - - # Check that groups are in order and groupfile exists - if (defined $lastgroup && lc $lastgroup ge lc $groupfile) { - print "Symbol block at $symfile:$line: block not sorted\n"; - print "Move $groupfile block before $lastgroup block\n"; - print "\n"; - $ret = 1; - } - if (! -e "$srcdir/$groupfile") { - print "Symbol block at $symfile:$line: $groupfile not found\n"; - print "\n"; - $ret = 1; - } - $lastgroup = $groupfile; - - # Check that symbols within a group are in order - for (my $i = 0 ; $i <= $#sorted ; $i++) { - if ($sorted[$i] ne $group[$i]) { - $first = $i unless defined $first; - $last = $i; - $sorted = 0; - } - } - if (!$sorted) { - @group = splice @group, $first, ($last-$first+1); - @sorted = splice @sorted, $first, ($last-$first+1); - print "Symbol block at $symfile:$line: symbols not sorted\n"; - print map { " " . $_ . "\n" } @group; - print "Correct ordering\n"; - print map { " " . $_ . "\n" } @sorted; - print "\n"; - $ret = 1; - } -} - -exit $ret; diff --git a/src/check-symsorting.py b/src/check-symsorting.py new file mode 100755 index 0000000000..a25175cac8 --- /dev/null +++ b/src/check-symsorting.py @@ -0,0 +1,119 @@ +#!/usr/bin/env python + +# Copyright (C) 2012-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 os.path +import re +import sys + +if len(sys.argv) < 3: + print("syntax: %s SRCDIR SYMFILE..." % sys.argv[0], file=sys.stderr) + sys.exit(1) + + +def check_sorting(group, symfile, line, groupfile, lastgroup): + sortedgroup = sorted(group, key=str.lower) + issorted = True + first = None + last = None + + err = False + # Check that groups are in order and groupfile exists + if lastgroup is not None and lastgroup.lower() > groupfile.lower(): + print("Symbol block at %s:%s: block not sorted" % + (symfile, line), file=sys.stderr) + print("Move %s block before %s block" % + (groupfile, lastgroup), file=sys.stderr) + print("", file=sys.stderr) + err = True + + if not os.path.exists(os.path.join(srcdir, groupfile)): + print("Symbol block at %s:%s: %s not found" % + (symfile, line, groupfile), file=sys.stderr) + print("", file=sys.stderr) + err = True + + # Check that symbols within a group are in order + for i in range(len(group)): + if sortedgroup[i] != group[i]: + if first is None: + first = i + + last = i + issorted = False + + if not issorted: + actual = group[first:(last-first+1)] + expect = sortedgroup[first:(last-first+1)] + print("Symbol block at %s:%s: symbols not sorted" % + (symfile, line), file=sys.stderr) + for g in actual: + print(" %s" % g, file=sys.stderr) + print("Correct ordering", file=sys.stderr) + for g in expect: + print(" %s" % g, file=sys.stderr) + print("", file=sys.stderr) + err = True + + return err + + +ret = 0 +srcdir = sys.argv[1] +lastgroup = None +for symfile in sys.argv[2:]: + with open(symfile, "r") as fh: + lineno = 0 + groupfile = "" + group = [] + thisline = 0 + + filenameprog = re.compile(r'''^#\s*((\w+\/)*(\w+\.h))\s*$''') + + for line in fh: + thisline = thisline + 1 + line = line.strip() + + filenamematch = filenameprog.match(line) + if filenamematch is not None: + groupfile = filenamematch.group(1) + elif line == "": + if len(group) > 0: + if check_sorting(group, symfile, lineno, + groupfile, lastgroup): + ret = 1 + + group = [] + lineno = thisline + lastgroup = groupfile + elif line[0] == '#': + # Ignore comments + pass + else: + line = line.strip(";") + group.append(line) + + if len(group) > 0: + if check_sorting(group, symfile, lineno, + groupfile, lastgroup): + ret = 1 + + lastgroup = None + +sys.exit(ret) -- 2.21.0

As part of an goal to eliminate Perl from libvirt build tools, rewrite the dtrace2systemtap.pl tool in Python. This was 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. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/Makefile.am | 9 ++- src/dtrace2systemtap.pl | 130 ------------------------------------ src/dtrace2systemtap.py | 143 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 147 insertions(+), 135 deletions(-) delete mode 100755 src/dtrace2systemtap.pl create mode 100755 src/dtrace2systemtap.py diff --git a/src/Makefile.am b/src/Makefile.am index d10bfdf448..5a1df7a93c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -604,7 +604,6 @@ nodist_libvirt_la_SOURCES = libvirt_probes.h if WITH_REMOTE nodist_libvirt_driver_remote_la_SOURCES = libvirt_probes.h endif WITH_REMOTE -DTRACE2SYSTEMTAP_FLAGS = --with-modules BUILT_SOURCES += libvirt_probes.h libvirt_probes.stp libvirt_functions.stp @@ -639,10 +638,10 @@ RPC_PROBE_FILES += $(srcdir)/rpc/virnetprotocol.x \ libvirt_functions.stp: $(RPC_PROBE_FILES) $(srcdir)/rpc/gensystemtap.pl $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gensystemtap.pl $(RPC_PROBE_FILES) > $@ -%_probes.stp: %_probes.d $(srcdir)/dtrace2systemtap.pl \ +%_probes.stp: %_probes.d $(srcdir)/dtrace2systemtap.py \ $(top_builddir)/config.status - $(AM_V_GEN)$(PERL) -w $(srcdir)/dtrace2systemtap.pl \ - $(DTRACE2SYSTEMTAP_FLAGS) $(bindir) $(sbindir) $(libdir) $< > $@ + $(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(srcdir)/dtrace2systemtap.py \ + $(bindir) $(sbindir) $(libdir) $< > $@ CLEANFILES += libvirt_probes.h libvirt_probes.o libvirt_probes.lo \ libvirt_functions.stp libvirt_probes.stp @@ -763,7 +762,7 @@ endif LIBVIRT_INIT_SCRIPT_SYSTEMD endif WITH_LIBVIRTD -EXTRA_DIST += dtrace2systemtap.pl +EXTRA_DIST += dtrace2systemtap.py if WITH_LIBVIRTD diff --git a/src/dtrace2systemtap.pl b/src/dtrace2systemtap.pl deleted file mode 100755 index c5fce248b4..0000000000 --- a/src/dtrace2systemtap.pl +++ /dev/null @@ -1,130 +0,0 @@ -#!/usr/bin/env perl -# -# Copyright (C) 2011-2012 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/>;. -# -# -# Generate a set of systemtap probe definitions corresponding to -# DTrace probe markers in libvirt.so -# -# perl dtrace2systemtap.pl probes.d > libvirt_probes.stp -# - -use strict; -use warnings; - -my $file; -my @files; -my %files; - -my $with_modules = 0; -if ($ARGV[0] eq "--with-modules") { - # set if we want to honor the "module" setting in the .d file - $with_modules = 1; - shift @ARGV; -} - -my $bindir = shift @ARGV; -my $sbindir = shift @ARGV; -my $libdir = shift @ARGV; - -my $probe; -my $args; - -# Read the DTraceprobes definition -while (<>) { - next if m,^\s*$,; - - next if /^\s*provider\s+\w+\s*{\s*$/; - next if /^\s*};\s*$/; - - if (m,^\s*\#,) { - if (m,^\s*\#\s*file:\s*(\S+)\s*$,) { - $file = $1; - push @files, $file; - $files{$file} = { prefix => undef, probes => [] }; - } elsif (m,^\s*\#\s*prefix:\s*(\S+)\s*$,) { - $files{$file}->{prefix} = $1; - } elsif (m,^\s*\#\s*binary:\s*(\S+)\s*$,) { - $files{$file}->{binary} = $1; - } elsif (m,^\s*\#\s*module:\s*(\S+)\s*$,) { - $files{$file}->{module} = $1; - } else { - # ignore unknown comments - } - } else { - if (m,\s*probe\s+([a-zA-Z0-9_]+)\((.*?)(\);)?$,) { - $probe = $1; - $args = $2; - if ($3) { - push @{$files{$file}->{probes}}, [$probe, $args]; - $probe = $args = undef; - } - } elsif ($probe) { - if (m,^(.*?)(\);)?$,) { - $args .= $1; - if ($2) { - push @{$files{$file}->{probes}}, [$probe, $args]; - $probe = $args = undef; - } - } else { - die "unexpected data $_ on line $."; - } - } else { - die "unexpected data $_ on line $."; - } - } -} - -# Write out the SystemTap probes -foreach my $file (@files) { - my $prefix = $files{$file}->{prefix}; - my @probes = @{$files{$file}->{probes}}; - - print "# $file\n\n"; - foreach my $probe (@probes) { - my $name = $probe->[0]; - my $args = $probe->[1]; - - my $pname = $name; - $pname =~ s/${prefix}_/libvirt.$prefix./; - - my $binary = "$libdir/libvirt.so"; - if (exists $files{$file}->{binary}) { - $binary = $sbindir . "/" . $files{$file}->{binary}; - } - if ($with_modules && exists $files{$file}->{module}) { - $binary = $libdir . "/" . $files{$file}->{module}; - } - - print "probe $pname = process(\"$binary\").mark(\"$name\") {\n"; - - my @args = split /,/, $args; - for (my $i = 0 ; $i <= $#args ; $i++) { - my $arg = $args[$i]; - my $isstr = $arg =~ /char\s+\*/; - $arg =~ s/^.*\s\*?(\S+)$/$1/; - - if ($isstr) { - print " $arg = user_string(\$arg", $i + 1, ");\n"; - } else { - print " $arg = \$arg", $i + 1, ";\n"; - } - } - print "}\n\n"; - } - print "\n"; -} diff --git a/src/dtrace2systemtap.py b/src/dtrace2systemtap.py new file mode 100755 index 0000000000..8ee5632d6b --- /dev/null +++ b/src/dtrace2systemtap.py @@ -0,0 +1,143 @@ +#!/usr/bin/env python +# +# Copyright (C) 2011-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/>;. +# +# +# Generate a set of systemtap probe definitions corresponding to +# DTrace probe markers in libvirt.so +# +# python dtrace2systemtap.py probes.d > libvirt_probes.stp +# + +from __future__ import print_function + +import re +import sys + +file = None +filelist = [] +files = {} + +bindir = sys.argv[1] +sbindir = sys.argv[2] +libdir = sys.argv[3] +dtrace = sys.argv[4] + +probe = None +args = None + +# Read the DTraceprobes definition + +with open(dtrace, "r") as fh: + lineno = 0 + for line in fh: + lineno = lineno + 1 + line = line.strip() + if line == "": + continue + if line.find("provider ") != -1 and line.find("{") != -1: + continue + if line.find("};") != -1: + continue + + if line.startswith("#"): + m = re.match(r'''^\#\s*file:\s*(\S+)$''', line) + if m is not None: + file = m.group(1) + filelist.append(file) + files[file] = {"prefix": None, "probes": []} + continue + + m = re.match(r'''^\#\s*prefix:\s*(\S+)$''', line) + if m is not None: + files[file]["prefix"] = m.group(1) + continue + + m = re.match(r'''^\#\s*binary:\s*(\S+)$''', line) + if m is not None: + files[file]["binary"] = m.group(1) + continue + + m = re.match(r'''^\#\s*module:\s*(\S+)$''', line) + if m is not None: + files[file]["module"] = m.group(1) + + # ignore unknown comments + else: + m = re.match(r'''probe\s+([a-zA-Z0-9_]+)\((.*?)(\);)?$''', line) + if m is not None: + probe = m.group(1) + args = m.group(2) + if m.group(3) is not None: + files[file]["probes"].append([probe, args]) + probe = None + args = None + elif probe is not None: + m = re.match(r'''^(.*?)(\);)?$''', line) + if m is not None: + args = args + m.group(1) + if m.group(2) is not None: + files[file]["probes"].append([probe, args]) + probe = None + args = None + else: + raise Exception("unexpected data %s on line %d" % + (line, lineno)) + else: + raise Exception("unexpected data %s on line %d" % + (line, lineno)) + +# Write out the SystemTap probes +for file in filelist: + prefix = files[file]["prefix"] + probes = files[file]["probes"] + + print("# %s\n" % file) + for probe in probes: + name = probe[0] + args = probe[1] + + pname = name.replace(prefix + "_", "libvirt." + prefix + ".") + + binary = libdir + "/libvirt.so" + if "binary" in files[file]: + binary = sbindir + "/" + files[file]["binary"] + if "module" in files[file]: + binary = libdir + "/" + files[file]["module"] + + print("probe %s = process(\"%s\").mark(\"%s\") {" % + (pname, binary, name)) + + argbits = args.split(",") + for idx in range(len(argbits)): + arg = argbits[idx] + isstr = False + if arg.find("char *") != -1: + isstr = True + + m = re.match(r'''^.*\s\*?(\S+)$''', arg) + if m is not None: + arg = m.group(1) + else: + raise Exception("Malformed arg %s" % arg) + + if isstr: + print(" %s = user_string($arg%d);" % (arg, idx + 1)) + else: + print(" %s = $arg%d;" % (arg, idx + 1)) + print("}\n") + print("") -- 2.21.0

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(a)redhat.com&gt; --- src/Makefile.am | 18 +++++-- src/check-drivername.pl | 83 ----------------------------- src/check-drivername.py | 114 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 86 deletions(-) delete mode 100755 src/check-drivername.pl create mode 100644 src/check-drivername.py diff --git a/src/Makefile.am b/src/Makefile.am index 5226871a73..0d387a1335 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -335,15 +335,27 @@ 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) $(srcdir)/check-drivername.py \ + $(DRIVERS) \ $(srcdir)/libvirt_public.syms \ $(srcdir)/libvirt_admin_public.syms \ $(srcdir)/libvirt_qemu.syms \ $(srcdir)/libvirt_lxc.syms -EXTRA_DIST += check-drivername.pl +EXTRA_DIST += check-drivername.py check-driverimpls: $(AM_V_GEN)$(PERL) $(srcdir)/check-driverimpls.pl \ 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; diff --git a/src/check-drivername.py b/src/check-drivername.py new file mode 100644 index 0000000000..2f024f5dee --- /dev/null +++ b/src/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.match(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 + + m = re.match(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) + continue + + m = re.match(r'''^\*(vir\w+)\s*\)''', line) + if m is not None: + name = m.group(1) + print("Bogus name %s" % name) + status = 1 + continue + + m = re.match(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) -- 2.21.0

As part of an goal to eliminate Perl from libvirt build tools, rewrite the check-aclrules.pl tool in Python. This was 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. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/Makefile.am | 4 +- src/check-aclrules.pl | 252 ---------------------------------------- src/check-aclrules.py | 263 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 265 insertions(+), 254 deletions(-) delete mode 100755 src/check-aclrules.pl create mode 100755 src/check-aclrules.py diff --git a/src/Makefile.am b/src/Makefile.am index 1066f7f7cc..ed1e1ca32b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -365,7 +365,7 @@ check-driverimpls: $(filter-out /%,$(DRIVER_SOURCE_FILES)))) check-aclrules: - $(AM_V_GEN)$(PERL) $(srcdir)/check-aclrules.pl \ + $(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(srcdir)/check-aclrules.py \ $(REMOTE_PROTOCOL) \ $(addprefix $(srcdir)/,$(filter-out /%,$(STATEFUL_DRIVER_SOURCE_FILES))) @@ -374,7 +374,7 @@ check-aclperms: $(srcdir)/access/viraccessperm.h \ $(srcdir)/access/viraccessperm.c -EXTRA_DIST += check-driverimpls.py check-aclrules.pl check-aclperms.py +EXTRA_DIST += check-driverimpls.py check-aclrules.py check-aclperms.py check-local: check-protocol check-symfile check-symsorting \ check-drivername check-driverimpls check-aclrules \ diff --git a/src/check-aclrules.pl b/src/check-aclrules.pl deleted file mode 100755 index 0d4cac17ca..0000000000 --- a/src/check-aclrules.pl +++ /dev/null @@ -1,252 +0,0 @@ -#!/usr/bin/env perl -# -# Copyright (C) 2013-2014 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/>;. -# -# This script validates that the driver implementation of any -# public APIs contain ACL checks. -# -# As the script reads each source file, it attempts to identify -# top level function names. -# -# When reading the body of the functions, it looks for anything -# that looks like an API called named XXXEnsureACL. It will -# validate that the XXX prefix matches the name of the function -# it occurs in. -# -# When it later finds the virDriverPtr table, for each entry -# point listed, it will validate if there was a previously -# detected EnsureACL call recorded. -# -use strict; -use warnings; - -my $status = 0; - -my $brace = 0; -my $maybefunc; -my $intable = 0; -my $table; - -my %acls; -my %aclfilters; - -my %whitelist = ( - "connectClose" => 1, - "connectIsEncrypted" => 1, - "connectIsSecure" => 1, - "connectIsAlive" => 1, - "networkOpen" => 1, - "networkClose" => 1, - "nwfilterOpen" => 1, - "nwfilterClose" => 1, - "secretOpen" => 1, - "secretClose" => 1, - "storageOpen" => 1, - "storageClose" => 1, - "interfaceOpen" => 1, - "interfaceClose" => 1, - "connectURIProbe" => 1, - "localOnly" => 1, - "domainQemuAttach" => 1, - ); - -# XXX this vzDomainMigrateConfirm3Params looks -# bogus - determine why it doesn't have a valid -# ACL check. -my %implwhitelist = ( - "vzDomainMigrateConfirm3Params" => 1, - ); - -my $lastfile; - -sub fixup_name { - my $name = shift; - - $name =~ s/Nwfilter/NWFilter/; - $name =~ s/Xml$/XML/; - $name =~ s/Uri$/URI/; - $name =~ s/Uuid$/UUID/; - $name =~ s/Id$/ID/; - $name =~ s/Mac$/MAC/; - $name =~ s/Cpu$/CPU/; - $name =~ s/Os$/OS/; - $name =~ s/Nmi$/NMI/; - $name =~ s/Pm/PM/; - $name =~ s/Fstrim$/FSTrim/; - $name =~ s/Scsi/SCSI/; - $name =~ s/Wwn$/WWN/; - - return $name; -} - -sub name_to_ProcName { - my $name = shift; - - my @elems; - if ($name =~ /_/ || (lc $name) eq "open" || (lc $name) eq "close") { - @elems = split /_/, $name; - @elems = map lc, @elems; - @elems = map ucfirst, @elems; - } else { - @elems = $name; - } - @elems = map { fixup_name($_) } @elems; - my $procname = join "", @elems; - - $procname =~ s/^([A-Z])/lc $1/e; - - return $procname; -} - - -my $proto = shift @ARGV; - -open PROTO, "<$proto" or die "cannot read $proto"; - -my %filtered; -my $incomment = 0; -my $filtered = 0; -while (<PROTO>) { - if (m,/\*\*,) { - $incomment = 1; - $filtered = 0; - } elsif ($incomment) { - if (m,\*\s\@aclfilter,) { - $filtered = 1; - } elsif ($filtered && - m,REMOTE_PROC_(.*)\s+=\s*\d+,) { - my $api = name_to_ProcName($1); - # Event filtering is handled in daemon/remote.c instead of drivers - if (! m,_EVENT_REGISTER,) { - $filtered{$api} = 1; - } - $incomment = 0; - } - } -} - -close PROTO; - -while (<>) { - if (!defined $lastfile || - $lastfile ne $ARGV) { - %acls = (); - $brace = 0; - $maybefunc = undef; - $lastfile = $ARGV; - } - if ($brace == 0) { - # Looks for anything which appears to be a function - # body name. Doesn't matter if we pick up bogus stuff - # here, as long as we don't miss valid stuff - if (m,\b(\w+)\(,) { - $maybefunc = $1; - } - } elsif ($brace > 0) { - if (m,(\w+)EnsureACL,) { - # Record the fact that maybefunc contains an - # ACL call, and make sure it is the right call! - my $func = $1; - $func =~ s/^vir//; - if (!defined $maybefunc) { - print "$ARGV:$. Unexpected check '$func' outside function\n"; - $status = 1; - } else { - unless ($maybefunc =~ /$func$/i) { - print "$ARGV:$. Mismatch check 'vir${func}EnsureACL' for function '$maybefunc'\n"; - $status = 1; - } - } - $acls{$maybefunc} = 1; - } elsif (m,(\w+)CheckACL,) { - # Record the fact that maybefunc contains an - # ACL filter call, and make sure it is the right call! - my $func = $1; - $func =~ s/^vir//; - if (!defined $maybefunc) { - print "$ARGV:$. Unexpected check '$func' outside function\n"; - $status = 1; - } else { - unless ($maybefunc =~ /$func$/i) { - print "$ARGV:$. Mismatch check 'vir${func}CheckACL' for function '$maybefunc'\n"; - $status = 1; - } - } - $aclfilters{$maybefunc} = 1; - } elsif (m,\b(\w+)\(,) { - # Handles case where we replaced an API with a new - # one which adds new parameters, and we're left with - # a simple stub calling the new API. - my $callfunc = $1; - if (exists $acls{$callfunc}) { - $acls{$maybefunc} = 1; - } - if (exists $aclfilters{$callfunc}) { - $aclfilters{$maybefunc} = 1; - } - } - } - - # Pass the vir*DriverPtr tables and make sure that - # every func listed there, has an impl which calls - # an ACL function - if ($intable) { - if (/\}/) { - $intable = 0; - $table = undef; - } elsif (/\.(\w+)\s*=\s*(\w+),?/) { - my $api = $1; - my $impl = $2; - - next if $impl eq "NULL"; - - if ($api ne "no" && - $api ne "name" && - $table ne "virStateDriver" && - !exists $acls{$impl} && - !exists $whitelist{$api} && - !exists $implwhitelist{$impl}) { - print "$ARGV:$. Missing ACL check in function '$impl' for '$api'\n"; - $status = 1; - } - - if (exists $filtered{$api} && - !exists $aclfilters{$impl}) { - print "$ARGV:$. Missing ACL filter in function '$impl' for '$api'\n"; - $status = 1; - } - } - } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+/) { - if ($1 ne "virNWFilterCallbackDriver" && - $1 ne "virNWFilterTechDriver" && - $1 ne "virDomainConfNWFilterDriver") { - $intable = 1; - $table = $1; - } - } - - - my $count; - $count = s/{//g; - $brace += $count; - $count = s/}//g; - $brace -= $count; -} continue { - close ARGV if eof; -} - -exit $status; diff --git a/src/check-aclrules.py b/src/check-aclrules.py new file mode 100755 index 0000000000..a7ccc794be --- /dev/null +++ b/src/check-aclrules.py @@ -0,0 +1,263 @@ +#!/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/>;. +# +# This script validates that the driver implementation of any +# public APIs contain ACL checks. +# +# As the script reads each source file, it attempts to identify +# top level function names. +# +# When reading the body of the functions, it looks for anything +# that looks like an API called named XXXEnsureACL. It will +# validate that the XXX prefix matches the name of the function +# it occurs in. +# +# When it later finds the virDriverPtr table, for each entry +# point listed, it will validate if there was a previously +# detected EnsureACL call recorded. +# + +from __future__ import print_function + +import re +import sys + +whitelist = { + "connectClose": True, + "connectIsEncrypted": True, + "connectIsSecure": True, + "connectIsAlive": True, + "networkOpen": True, + "networkClose": True, + "nwfilterOpen": True, + "nwfilterClose": True, + "secretOpen": True, + "secretClose": True, + "storageOpen": True, + "storageClose": True, + "interfaceOpen": True, + "interfaceClose": True, + "connectURIProbe": True, + "localOnly": True, + "domainQemuAttach": True, +} + +# XXX this vzDomainMigrateConfirm3Params looks +# bogus - determine why it doesn't have a valid +# ACL check. +implwhitelist = { + "vzDomainMigrateConfirm3Params": True, +} + +lastfile = None + + +def fixup_name(name): + name.replace("Nwfilter", "NWFilter") + name.replace("Pm", "PM") + name.replace("Scsi", "SCSI") + if name.endswith("Xml"): + name = name[:-3] + "XML" + elif name.endswith("Uri"): + name = name[:-3] + "URI" + elif name.endswith("Uuid"): + name = name[:-4] + "UUID" + elif name.endswith("Id"): + name = name[:-2] + "ID" + elif name.endswith("Mac"): + name = name[:-3] + "MAC" + elif name.endswith("Cpu"): + name = name[:-3] + "MAC" + elif name.endswith("Os"): + name = name[:-2] + "OS" + elif name.endswith("Nmi"): + name = name[:-3] + "NMI" + elif name.endswith("Fstrim"): + name = name[:-6] + "FSTrim" + elif name.endswith("Wwn"): + name = name[:-3] + "WWN" + + return name + + +def name_to_ProcName(name): + elems = [] + if name.find("_") != -1 or name.lower() in ["open", "close"]: + elems = [n.lower().capitalize() for n in name.split("_")] + else: + elems = [name] + + elems = [fixup_name(n) for n in elems] + procname = "".join(elems) + + return procname[0:1].lower() + procname[1:] + + +proto = sys.argv[1] + +filteredmap = {} +with open(proto, "r") as fh: + incomment = False + filtered = False + + for line in fh: + if line.find("/**") != -1: + incomment = True + filtered = False + elif incomment: + if line.find("* @aclfilter") != -1: + filtered = True + elif filtered: + m = re.match(r'''.*REMOTE_PROC_(.*)\s+=\s*\d+.*''', line) + if m is not None: + api = name_to_ProcName(m.group(1)) + # Event filtering is handled in daemon/remote.c + # instead of drivers + if line.find("_EVENT_REGISTER") == -1: + filteredmap[api] = True + incomment = False + + +def process_file(filename): + brace = 0 + maybefunc = None + intable = False + table = None + + acls = {} + aclfilters = {} + errs = False + with open(filename, "r") as fh: + lineno = 0 + for line in fh: + lineno = lineno + 1 + if brace == 0: + # Looks for anything which appears to be a function + # body name. Doesn't matter if we pick up bogus stuff + # here, as long as we don't miss valid stuff + m = re.match(r'''.*\b(\w+)\(.*''', line) + if m is not None: + maybefunc = m.group(1) + elif brace > 0: + ensureacl = re.match(r'''.*?(\w+)EnsureACL.*''', line) + checkacl = re.match(r'''.*?(\w+)CheckACL.*''', line) + stub = re.match(r'''.*?\b(\w+)\(.*''', line) + if ensureacl is not None: + # Record the fact that maybefunc contains an + # ACL call, and make sure it is the right call! + func = ensureacl.group(1) + if func.startswith("vir"): + func = func[3:] + + if maybefunc is None: + print("%s:%d Unexpected check '%s' outside function" % + (filename, lineno, func), file=sys.stderr) + errs = True + else: + if not maybefunc.lower().endswith(func.lower()): + print("%s:%d Mismatch check 'vir%sEnsureACL'" + + "for function '%s'" % + (filename, lineno, func, maybefunc), + file=sys.stderr) + errs = True + acls[maybefunc] = True + elif checkacl: + # Record the fact that maybefunc contains an + # ACL filter call, and make sure it is the right call! + func = checkacl.group(1) + if func.startswith("vir"): + func = func[3:] + + if maybefunc is None: + print("%s:%d Unexpected check '%s' outside function" % + (filename, lineno, func), file=sys.stderr) + errs = True + else: + if not maybefunc.lower().endswith(func.lower()): + print("%s:%d Mismatch check 'vir%sEnsureACL' " + + "for function '%s'" % + (filename, lineno, func, maybefunc), + file=sys.stderr) + errs = True + aclfilters[maybefunc] = True + elif stub: + # Handles case where we replaced an API with a new + # one which adds new parameters, and we're left with + # a simple stub calling the new API. + callfunc = stub.group(1) + if callfunc in acls: + acls[maybefunc] = True + + if callfunc in aclfilters: + aclfilters[maybefunc] = True + + # Pass the vir*DriverPtr tables and make sure that + # every func listed there, has an impl which calls + # an ACL function + if intable: + assign = re.match(r'''.*?\.(\w+)\s*=\s*(\w+),?.*''', line) + if line.find("}") != -1: + intable = False + table = None + elif assign is not None: + api = assign.group(1) + impl = assign.group(2) + + if (impl != "NULL" and + api not in ["no", "name"] and + table != "virStateDriver"): + if (impl not in acls and + api not in whitelist and + impl not in implwhitelist): + print("%s:%d Missing ACL check in " + + "function '%s' for '%s'" % + (filename, lineno, impl, api), + file=sys.stderr) + errs = True + + if api in filteredmap and impl not in aclfilters: + print("%s:%d Missing ACL filter in " + + "function '%s' for '%s'" % + (filename, lineno, impl, api), + file=sys.stderr) + errs = True + else: + m = re.match(r'''^(?:static\s+)?(vir(?:\w+)?Driver)\s+''', + line) + if m is not None: + name = m.group(1) + if name not in ["virNWFilterCallbackDriver", + "virNWFilterTechDriver", + "virDomainConfNWFilterDriver"]: + intable = True + table = name + + if line.find("{") != -1: + brace = brace + 1 + if line.find("}") != -1: + brace = brace - 1 + + return errs + + +status = 0 +for filename in sys.argv[2:]: + if process_file(filename): + status = 1 + +sys.exit(status) -- 2.21.0

As part of an goal to eliminate Perl from libvirt build tools, rewrite the pdwtags processing script in Python. The original inline shell and perl code was completely unintelligible. The new python code is a manual conversion that attempts todo basically the same thing. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- cfg.mk | 3 +- src/Makefile.am | 99 ++++--------------------- src/check-remote-protocol.py | 137 +++++++++++++++++++++++++++++++++++ 3 files changed, 154 insertions(+), 85 deletions(-) create mode 100644 src/check-remote-protocol.py diff --git a/cfg.mk b/cfg.mk index 18bab6e9a4..66df1448b4 100644 --- a/cfg.mk +++ b/cfg.mk @@ -353,6 +353,7 @@ sc_prohibit_mkstemp: # access with F_OK or R_OK is okay, though. sc_prohibit_access_xok: @prohibit='access(at)? *\(.*X_OK' \ + in_vc_files='\.[ch]$$' \ halt='use virFileIsExecutable instead of access(,X_OK)' \ $(_sc_search_regexp) @@ -1227,7 +1228,7 @@ exclude_file_name_regexp--sc_prohibit_PATH_MAX = \ ^cfg\.mk$$ exclude_file_name_regexp--sc_prohibit_access_xok = \ - ^(cfg\.mk|src/util/virutil\.c)$$ + ^(src/util/virutil\.c)$$ exclude_file_name_regexp--sc_prohibit_asprintf = \ ^(cfg\.mk|bootstrap.conf$$|examples/|src/util/virstring\.[ch]$$|tests/vircgroupmock\.c|tools/virt-login-shell\.c|tools/nss/libvirt_nss\.c$$) diff --git a/src/Makefile.am b/src/Makefile.am index ed1e1ca32b..05ad832668 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -192,84 +192,6 @@ DRIVER_SOURCES += \ -# Ensure that we don't change the struct or member names or member ordering -# in remote_protocol.x The embedded perl below needs a few comments, and -# presumes you know what pdwtags output looks like: -# * use -0777 -n to slurp the entire file into $_. -# * the "split" splits on the /* DD */ comments, so that $p iterates -# through the struct definitions. -# * process only "struct remote_..." entries -# * remove comments and preceding TAB throughout -# * remove empty lines throughout -# * remove white space at end of buffer - -# With pdwtags 1.8, --verbose output includes separators like these: -# /* 93 */ -# /* <0> (null):0 */ -# with the second line omitted for intrinsic types. -# Whereas with pdwtags 1.3, they look like this: -# /* <2d2> /usr/include/libio.h:180 */ -# The alternation of the following regexps matches both cases. -r1 = /\* \d+ \*/ -r2 = /\* <[[:xdigit:]]+> \S+:\d+ \*/ -libs_prefix = remote_|qemu_|lxc_|admin_ -other_prefix = keepalive|vir(Net|LockSpace|LXCMonitor) -struct_prefix = ($(libs_prefix)|$(other_prefix)) - -# Depending on configure options, libtool creates one or both of -# remote/{,.libs/}libvirt_driver_remote_la-remote_protocol.o. We want -# the newest of the two, in case configure options changed and a stale -# file is left around from an earlier build. -# The pdwtags output is completely different when building with clang -# which causes the comparison against expected output to fail, so skip -# if using clang as CC. -PDWTAGS = \ - $(AM_V_GEN)if $(CC) -v 2>&1 | grep -q clang; then \ - echo 'WARNING: skipping pdwtags test with Clang' >&2; \ - exit 0; \ - fi; \ - if (pdwtags --help) > /dev/null 2>&1; then \ - o=`ls -t $(<:.lo=.$(OBJEXT)) \ - $(subst /,/.libs/,$(<:.lo=.$(OBJEXT))) \ - 2>/dev/null | sed -n 1p`; \ - test -f "$$o" || { echo ".o for $< not found" >&2; exit 1; }; \ - pdwtags --verbose $$o > $(@F)-t1 2> $(@F)-t2; \ - if test ! -s $(@F)-t1 && test -s $(@F)-t2; then \ - rm -rf $(@F)-t?; \ - echo 'WARNING: pdwtags appears broken; skipping the $@ test' >&2;\ - else \ - $(PERL) -0777 -n \ - -e 'foreach my $$p (split m!\n*(?:$(r1)|$(r2))\n!) {' \ - -e ' if ($$p =~ /^(struct|enum) $(struct_prefix)/ ||' \ - -e ' $$p =~ /^enum \{/) {' \ - -e ' $$p =~ s!\t*/\*.*?\*/!!sg;' \ - -e ' $$p =~ s!\s+\n!\n!sg;' \ - -e ' $$p =~ s!\s+$$!!;' \ - -e ' $$p =~ s!\t! !g;' \ - -e ' print "$$p\n";' \ - -e ' $$n++;' \ - -e ' }' \ - -e '}' \ - -e 'BEGIN {' \ - -e ' print "/* -*- c -*- */\n";' \ - -e '}' \ - -e 'END {' \ - -e ' if ($$n < 1) {' \ - -e ' warn "WARNING: your pdwtags program is too old\n";' \ - -e ' warn "WARNING: skipping the $@ test\n";' \ - -e ' warn "WARNING: install dwarves-1.3 or newer\n";' \ - -e ' exit 8;' \ - -e ' }' \ - -e '}' \ - < $(@F)-t1 > $(@F)-t3; \ - case $$? in 8) rm -f $(@F)-t?; exit 0;; 0) ;; *) exit 1;; esac;\ - diff -u $(@)s $(@F)-t3; st=$$?; rm -f $(@F)-t?; exit $$st; \ - fi; \ - else \ - echo 'WARNING: you lack pdwtags; skipping the $@ test' >&2; \ - echo 'WARNING: install the dwarves package to get pdwtags' >&2; \ - fi - # .libs/libvirt.so is built by libtool as a side-effect of the Makefile # rule for libvirt.la. However, checking symbols relies on Linux ELF layout if WITH_LINUX @@ -306,34 +228,43 @@ PROTOCOL_STRUCTS = \ if WITH_REMOTE check-protocol: $(PROTOCOL_STRUCTS) $(PROTOCOL_STRUCTS:structs=struct) +# Ensure that we don't change the struct or member names or member ordering +# in remote_protocol.x The process-pdwtags.py post-processes output to +# extract the bits we want. + # The .o file that pdwtags parses is created as a side effect of running # libtool; but from make's perspective we depend on the .lo file. $(srcdir)/remote_protocol-struct \ $(srcdir)/qemu_protocol-struct \ $(srcdir)/lxc_protocol-struct: \ $(srcdir)/%-struct: remote/libvirt_driver_remote_la-%.lo - $(PDWTAGS) + $(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(srcdir)/check-remote-protocol.py \ + "$(CC)" "$(OBJEXT)" $< $(@)s $(srcdir)/virnetprotocol-struct $(srcdir)/virkeepaliveprotocol-struct: \ $(srcdir)/%-struct: rpc/libvirt_net_rpc_la-%.lo - $(PDWTAGS) + $(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(srcdir)/check-remote-protocol.py \ + "$(CC)" "$(OBJEXT)" $< $(@)s if WITH_LXC $(srcdir)/lxc_monitor_protocol-struct: \ $(srcdir)/%-struct: lxc/libvirt_driver_lxc_impl_la-%.lo - $(PDWTAGS) + $(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(srcdir)/check-remote-protocol.py \ + "$(CC)" "$(OBJEXT)" $< $(@)s endif WITH_LXC $(srcdir)/lock_protocol-struct: \ $(srcdir)/%-struct: locking/lockd_la-%.lo - $(PDWTAGS) + $(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(srcdir)/check-remote-protocol.py \ + "$(CC)" "$(OBJEXT)" $< $(@)s $(srcdir)/admin_protocol-struct: \ $(srcdir)/%-struct: admin/libvirt_admin_la-%.lo - $(PDWTAGS) + $(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(srcdir)/check-remote-protocol.py \ + "$(CC)" "$(OBJEXT)" $< $(@)s else !WITH_REMOTE # The $(PROTOCOL_STRUCTS) files must live in git, because they cannot be # re-generated when configured --without-remote. check-protocol: endif !WITH_REMOTE -EXTRA_DIST += $(PROTOCOL_STRUCTS) +EXTRA_DIST += $(PROTOCOL_STRUCTS) check-remote-protocol.py DRIVERS = \ $(srcdir)/driver-hypervisor.h \ diff --git a/src/check-remote-protocol.py b/src/check-remote-protocol.py new file mode 100644 index 0000000000..3b7b915e88 --- /dev/null +++ b/src/check-remote-protocol.py @@ -0,0 +1,137 @@ +#!/usr/bin/env python +# +# Copyright (C) 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/>;. +# +# This uses pdwtags to check remote protocol defs +# +# * the "split" splits on the /* DD */ comments, so that $p iterates +# through the struct definitions. +# * process only "struct remote_..." entries +# * remove comments and preceding TAB throughout +# * remove empty lines throughout +# * remove white space at end of buffer + +from __future__ import print_function + +import os +import os.path +import re +import subprocess +import sys + +cc = sys.argv[1] +objext = sys.argv[2] +proto_lo = sys.argv[3] +expected = sys.argv[4] + +proto_lo = proto_lo.replace("/", "/.libs/") + +ccargv = cc.split(" ") +ccargv.append("-v") +ccproc = subprocess.Popen(ccargv, stdout=subprocess.PIPE, + stderr=subprocess.STDOUT) +out, err = ccproc.communicate() +out = out.decode("utf-8") +if out.find("clang") != -1: + print("WARNING: skipping pdwtags test with Clang", file=sys.stderr) + sys.exit(0) + + +def which(program): + def is_exe(fpath): + return (os.path.isfile(fpath) and + os.access(fpath, os.X_OK)) + + fpath, fname = os.path.split(program) + if fpath: + if is_exe(program): + return program + else: + for path in os.environ["PATH"].split(os.pathsep): + exe_file = os.path.join(path, program) + if is_exe(exe_file): + return exe_file + + return None + + +pdwtags = which("pdwtags") +if pdwtags is None: + print("WARNING: you lack pdwtags; skipping the protocol test", + file=sys.stderr) + print("WARNING: install the dwarves package to get pdwtags", + file=sys.stderr) + sys.exit(0) + +proto_o = proto_lo.replace(".lo", ".o") + +if not os.path.exists(proto_o): + raise Exception("Missing %s", proto_o) + +pdwtagsproc = subprocess.Popen(["pdwtags", "--verbose", proto_o], + stdout=subprocess.PIPE, stderr=subprocess.PIPE) +out, err = pdwtagsproc.communicate() +out = out.decode("utf-8") +err = err.decode("utf-8") + +if out == "" and err != "": + print("WARNING: no output, pdwtags appears broken:", file=sys.stderr) + for l in err.strip().split("\n"): + print("WARNING: %s" % l, file=sys.stderr) + print("WARNING: skipping the remote protocol test", file=sys.stderr) + sys.exit(0) + +# With pdwtags 1.8, --verbose output includes separators like these: +# /* 93 */ +# /* <0> (null):0 */ +# with the second line omitted for intrinsic types. +# Whereas with pdwtags 1.3, they look like this: +# /* <2d2> /usr/include/libio.h:180 */ +# The alternation of the following regexps matches both cases. +r1 = r'''/\* \d+ \*/''' +r2 = r'''/\* <[0-9a-fA-F]+> \S+:\d+ \*/''' + +libs_prefix = "remote_|qemu_|lxc_|admin_" +other_prefix = "keepalive|vir(Net|LockSpace|LXCMonitor)" +struct_prefix = "(" + libs_prefix + "|" + other_prefix + ")" + +n = 0 +bits = re.split(r'''\n*(?:%s|%s)\n''' % (r1, r2), out) +actual = ["/* -*- c -*- */"] + +for bit in bits: + if (re.match(r'''^(struct|enum)\s+''' + struct_prefix, bit) or + bit.startswith("enum {")): + bit = re.sub(r'''\t*/\*.*?\*/''', "", bit) + bit = re.sub(r'''\s+\n''', '''\n''', bit) + bit = re.sub(r'''\s+$''', "", bit) + bit = re.sub(r'''\t''', " ", bit) + actual.append(bit) + n = n + 1 + +if n < 1: + print("WARNING: No structs/enums matched. Your", file=sys.stderr) + print("WARNING: pdwtags program is probably too old", file=sys.stderr) + print("WARNING: skipping the remote protocol test", file=sys.stderr) + print("WARNING: install dwarves-1.3 or newer", file=sys.stderr) + sys.exit(8) + +diff = subprocess.Popen(["diff", "-u", expected, "-"], stdin=subprocess.PIPE) +actualstr = "\n".join(actual) + "\n" +diff.communicate(input=actualstr.encode("utf-8")) + +sys.exit(diff.returncode) -- 2.21.0

As part of an goal to eliminate Perl from libvirt build tools, rewrite the group-qemu-caps.pl tool in Python. This was 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. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- cfg.mk | 3 +- tests/group-qemu-caps.pl | 124 --------------------------------------- tests/group-qemu-caps.py | 123 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 125 deletions(-) delete mode 100755 tests/group-qemu-caps.pl create mode 100755 tests/group-qemu-caps.py diff --git a/cfg.mk b/cfg.mk index d5d93e2c1f..25785423a2 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1175,7 +1175,8 @@ test-wrap-argv: $(PYTHON) $(top_srcdir)/tests/test-wrap-argv.py --check group-qemu-caps: - $(AM_V_GEN)$(PERL) $(top_srcdir)/tests/group-qemu-caps.pl --check $(top_srcdir)/ + $(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(top_srcdir)/tests/group-qemu-caps.py \ + --check --prefix $(top_srcdir)/ # sc_po_check can fail if generated files are not built first sc_po_check: \ diff --git a/tests/group-qemu-caps.pl b/tests/group-qemu-caps.pl deleted file mode 100755 index 829e63a562..0000000000 --- a/tests/group-qemu-caps.pl +++ /dev/null @@ -1,124 +0,0 @@ -#!/usr/bin/env perl -# -# 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/>;. -# -# -# Regroup array values into smaller groups separated by numbered comments. -# -# If --check is the first parameter, the script will return -# a non-zero value if a file is not grouped correctly. -# Otherwise the files are regrouped in place. - -use strict; -use warnings; - -my $check = 0; - -if (defined $ARGV[0] && $ARGV[0] eq "--check") { - $check = 1; - shift @ARGV; -} - -my $prefix = ''; -if (defined $ARGV[0]) { - $prefix = $ARGV[0]; - shift @ARGV; -} - -my $ret = 0; -if (&regroup_caps($prefix . 'src/qemu/qemu_capabilities.c', - 'virQEMUCaps grouping marker', - '\);', - 0, - " ") < 0) { - $ret = 1; -} -if (&regroup_caps($prefix . 'src/qemu/qemu_capabilities.h', - 'virQEMUCapsFlags grouping marker', - 'QEMU_CAPS_LAST \/\* this must', - 1, - " ") < 0) { - $ret = 1; -} - -exit $ret; - -sub regroup_caps { - my $filename = shift; - my $start_regex = shift; - my $end_regex = shift; - my $trailing_newline = shift; - my $counter_prefix = shift; - my $step = 5; - - open FILE, '<', $filename or die "cannot open $filename: $!"; - my @original = <FILE>; - close FILE; - - my @fixed; - my $game_on = 0; - my $counter = 0; - foreach (@original) { - if ($game_on) { - next if ($_ =~ '/\* [0-9]+ \*/'); - next if (/^\s+$/); - if ($counter % $step == 0) { - if ($counter != 0) { - push @fixed, "\n"; - } - push @fixed, "$counter_prefix/* $counter */\n"; - } - if (!($_ =~ '/\*' && !($_ =~ '\*/'))) { - # count two-line comments as one line - $counter++; - } - } - if (/$start_regex/) { - $game_on = 1; - } elsif ($game_on && $_ =~ /$end_regex/) { - if (($counter -1) % $step == 0) { - pop @fixed; # /* $counter */ - if ($counter != 1) { - pop @fixed; # \n - } - } - if ($trailing_newline) { - push @fixed, "\n"; - } - $game_on = 0; - } - push @fixed, $_; - } - - if ($check) { - my $nl = join('', @fixed); - my $ol = join('', @original); - unless ($nl eq $ol) { - open DIFF, "| diff -u $filename -" or die "cannot run diff: $!"; - print DIFF $nl; - close DIFF; - - print STDERR "Incorrect array grouping in $filename\n"; - print STDERR "Use group-qemu-caps.pl to group long array members\n"; - return -1; - } - } else { - open FILE, '>', $filename or die "cannot open $filename: $!"; - foreach my $line (@fixed) { - print FILE $line; - } - close FILE; - } -} diff --git a/tests/group-qemu-caps.py b/tests/group-qemu-caps.py new file mode 100755 index 0000000000..f73a94b998 --- /dev/null +++ b/tests/group-qemu-caps.py @@ -0,0 +1,123 @@ +#!/usr/bin/env python +# +# 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/>;. +# +# +# Regroup array values into smaller groups separated by numbered comments. +# +# If --check is the first parameter, the script will return +# a non-zero value if a file is not grouped correctly. +# Otherwise the files are regrouped in place. + +from __future__ import print_function + +import argparse +import re +import subprocess +import sys + + +def regroup_caps(check, filename, start_regex, end_regex, + trailing_newline, counter_prefix): + step = 5 + + original = [] + with open(filename, "r") as fh: + for line in fh: + original.append(line) + + fixed = [] + game_on = False + counter = 0 + for line in original: + line = line.rstrip("\n") + if game_on: + if re.match(r'''.*/\* [0-9]+ \*/.*''', line): + continue + if re.match(r'''^\s*$''', line): + continue + if counter % step == 0: + if counter != 0: + fixed.append("\n") + fixed.append("%s/* %d */\n" % (counter_prefix, counter)) + + if not (line.find("/*") != -1 and line.find("*/") == -1): + # count two-line comments as one line + counter = counter + 1 + + if re.match(start_regex, line): + game_on = True + elif game_on and re.match(end_regex, line): + if (counter - 1) % step == 0: + fixed = fixed[:-1] # /* $counter */ + if counter != 1: + fixed = fixed[:-1] # \n + + if trailing_newline: + fixed.append("\n") + + game_on = False + + fixed.append(line + "\n") + + if check: + orig = "".join(original) + new = "".join(fixed) + if new != orig: + diff = subprocess.Popen(["diff", "-u", filename, "-"], + stdin=subprocess.PIPE) + diff.communicate(input=new.encode('utf-8')) + + print("Incorrect line wrapping in $file", + file=sys.stderr) + print("Use test-wrap-argv.py to wrap test data files", + file=sys.stderr) + return False + else: + with open(filename, "w") as fh: + for line in fixed: + print(line, file=fh, end='') + + return True + + +parser = argparse.ArgumentParser(description='Test arg line wrapper') +parser.add_argument('--check', action="store_true", + help='check existing files only') +parser.add_argument('--prefix', default='', + help='source code tree prefix') +args = parser.parse_args() + +errs = False + +if not regroup_caps(args.check, + args.prefix + 'src/qemu/qemu_capabilities.c', + r'.*virQEMUCaps grouping marker.*', + r'.*\);.*', + 0, + " "): + errs = True + +if not regroup_caps(args.check, + args.prefix + 'src/qemu/qemu_capabilities.h', + r'.*virQEMUCapsFlags grouping marker.*', + r'.*QEMU_CAPS_LAST \/\* this must.*', + 1, + " "): + errs = True + +if errs: + sys.exit(1) +sys.exit(0) -- 2.21.0

As part of an goal to eliminate Perl from libvirt build tools, rewrite the hvsupport.pl tool in Python. This was 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. The new impl generates byte-for-byte identical output to the old impl. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- docs/Makefile.am | 8 +- docs/hvsupport.pl | 458 ---------------------------------------- docs/hvsupport.py | 516 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 520 insertions(+), 462 deletions(-) delete mode 100755 docs/hvsupport.pl create mode 100755 docs/hvsupport.py diff --git a/docs/Makefile.am b/docs/Makefile.am index 8b8074c3c0..b9d69db8c8 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -172,7 +172,7 @@ EXTRA_DIST= \ $(internals_html_in) $(internals_html) $(fonts) \ $(kbase_html_in) $(kbase_html) \ aclperms.htmlinc \ - hvsupport.pl \ + hvsupport.py \ $(schema_DATA) acl_generated = aclperms.htmlinc @@ -207,12 +207,12 @@ web: $(dot_html) $(internals_html) $(kbase_html) \ hvsupport.html: $(srcdir)/hvsupport.html.in -$(srcdir)/hvsupport.html.in: $(srcdir)/hvsupport.pl $(api_DATA) \ +$(srcdir)/hvsupport.html.in: $(srcdir)/hvsupport.py $(api_DATA) \ $(top_srcdir)/src/libvirt_public.syms \ $(top_srcdir)/src/libvirt_qemu.syms $(top_srcdir)/src/libvirt_lxc.syms \ $(top_srcdir)/src/driver.h - $(AM_V_GEN)$(PERL) $(srcdir)/hvsupport.pl $(top_srcdir)/src > $@ \ - || { rm $@ && exit 1; } + $(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(srcdir)/hvsupport.py \ + $(top_srcdir)/src > $@ || { rm $@ && exit 1; } news.html.in: \ $(srcdir)/news.xml \ diff --git a/docs/hvsupport.pl b/docs/hvsupport.pl deleted file mode 100755 index 494b8a27ec..0000000000 --- a/docs/hvsupport.pl +++ /dev/null @@ -1,458 +0,0 @@ -#!/usr/bin/env perl - -use strict; -use warnings; - -use File::Find; - -die "syntax: $0 SRCDIR\n" unless int(@ARGV) == 1; - -my $srcdir = shift @ARGV; - -my $symslibvirt = "$srcdir/libvirt_public.syms"; -my $symsqemu = "$srcdir/libvirt_qemu.syms"; -my $symslxc = "$srcdir/libvirt_lxc.syms"; -my @drivertable = ( - "$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", - ); - -my %groupheaders = ( - "virHypervisorDriver" => "Hypervisor APIs", - "virNetworkDriver" => "Virtual Network APIs", - "virInterfaceDriver" => "Host Interface APIs", - "virNodeDeviceDriver" => "Host Device APIs", - "virStorageDriver" => "Storage Pool APIs", - "virSecretDriver" => "Secret APIs", - "virNWFilterDriver" => "Network Filter APIs", - ); - - -my @srcs; -find({ - wanted => sub { - if (m!$srcdir/.*/\w+_(driver|common|tmpl|monitor|hal|udev)\.c$!) { - push @srcs, $_ if $_ !~ /vbox_driver\.c/; - } - }, no_chdir => 1}, $srcdir); - -# Map API functions to the header and documentation files they're in -# so that we can generate proper hyperlinks to their documentation. -# -# The function names are grep'd from the XML output of apibuild.py. -sub getAPIFilenames { - my $filename = shift; - - my %files; - my $line; - - open FILE, "<", $filename or die "cannot read $filename: $!"; - - while (defined($line = <FILE>)) { - if ($line =~ /function name='([^']+)' file='([^']+)'/) { - $files{$1} = $2; - } - } - - close FILE; - - if (keys %files == 0) { - die "No functions found in $filename. Has the apibuild.py output changed?"; - } - return \%files; -} - -sub parseSymsFile { - my $apisref = shift; - my $prefix = shift; - my $filename = shift; - my $xmlfilename = shift; - - my $line; - my $vers; - my $prevvers; - - my $filenames = getAPIFilenames($xmlfilename); - - open FILE, "<$filename" - or die "cannot read $filename: $!"; - - while (defined($line = <FILE>)) { - chomp $line; - next if $line =~ /^\s*#/; - next if $line =~ /^\s*$/; - next if $line =~ /^\s*(global|local):/; - if ($line =~ /^\s*${prefix}_(\d+\.\d+\.\d+)\s*{\s*$/) { - if (defined $vers) { - die "malformed syms file"; - } - $vers = $1; - } elsif ($line =~ /\s*}\s*;\s*$/) { - if (defined $prevvers) { - die "malformed syms file"; - } - $prevvers = $vers; - $vers = undef; - } elsif ($line =~ /\s*}\s*${prefix}_(\d+\.\d+\.\d+)\s*;\s*$/) { - if ($1 ne $prevvers) { - die "malformed syms file $1 != $vers"; - } - $prevvers = $vers; - $vers = undef; - } elsif ($line =~ /\s*(\w+)\s*;\s*$/) { - $$apisref{$1} = {}; - $$apisref{$1}->{vers} = $vers; - $$apisref{$1}->{file} = $$filenames{$1}; - } else { - die "unexpected data $line\n"; - } - } - - close FILE; -} - -my %apis; -# Get the list of all public APIs and their corresponding version -parseSymsFile(\%apis, "LIBVIRT", $symslibvirt, "$srcdir/../docs/libvirt-api.xml"); - -# And the same for the QEMU specific APIs -parseSymsFile(\%apis, "LIBVIRT_QEMU", $symsqemu, "$srcdir/../docs/libvirt-qemu-api.xml"); - -# And the same for the LXC specific APIs -parseSymsFile(\%apis, "LIBVIRT_LXC", $symslxc, "$srcdir/../docs/libvirt-lxc-api.xml"); - - -# Some special things which aren't public APIs, -# but we want to report -$apis{virConnectSupportsFeature}->{vers} = "0.3.2"; -$apis{virDomainMigratePrepare}->{vers} = "0.3.2"; -$apis{virDomainMigratePerform}->{vers} = "0.3.2"; -$apis{virDomainMigrateFinish}->{vers} = "0.3.2"; -$apis{virDomainMigratePrepare2}->{vers} = "0.5.0"; -$apis{virDomainMigrateFinish2}->{vers} = "0.5.0"; -$apis{virDomainMigratePrepareTunnel}->{vers} = "0.7.2"; - -$apis{virDomainMigrateBegin3}->{vers} = "0.9.2"; -$apis{virDomainMigratePrepare3}->{vers} = "0.9.2"; -$apis{virDomainMigratePrepareTunnel3}->{vers} = "0.9.2"; -$apis{virDomainMigratePerform3}->{vers} = "0.9.2"; -$apis{virDomainMigrateFinish3}->{vers} = "0.9.2"; -$apis{virDomainMigrateConfirm3}->{vers} = "0.9.2"; - -$apis{virDomainMigrateBegin3Params}->{vers} = "1.1.0"; -$apis{virDomainMigratePrepare3Params}->{vers} = "1.1.0"; -$apis{virDomainMigratePrepareTunnel3Params}->{vers} = "1.1.0"; -$apis{virDomainMigratePerform3Params}->{vers} = "1.1.0"; -$apis{virDomainMigrateFinish3Params}->{vers} = "1.1.0"; -$apis{virDomainMigrateConfirm3Params}->{vers} = "1.1.0"; - - - -# Now we want to get the mapping between public APIs -# and driver struct fields. This lets us later match -# update the driver impls with the public APis. - -my $line; - -# Group name -> hash of APIs { fields -> api name } -my %groups; -my $ingrp; -foreach my $drivertable (@drivertable) { - open FILE, "<$drivertable" - or die "cannot read $drivertable: $!"; - - while (defined($line = <FILE>)) { - if ($line =~ /struct _(vir\w*Driver)/) { - my $grp = $1; - if ($grp ne "virStateDriver" && - $grp ne "virStreamDriver") { - $ingrp = $grp; - $groups{$ingrp} = { apis => {}, drivers => {} }; - } - } elsif ($ingrp) { - if ($line =~ /^\s*vir(?:Drv)(\w+)\s+(\w+);\s*$/) { - my $field = $2; - my $name = $1; - - my $api; - if (exists $apis{"vir$name"}) { - $api = "vir$name"; - } elsif ($name =~ /\w+(Open|Close|URIProbe)/) { - next; - } else { - die "driver $name does not have a public API"; - } - $groups{$ingrp}->{apis}->{$field} = $api; - } elsif ($line =~ /};/) { - $ingrp = undef; - } - } - } - - close FILE; -} - - -# Finally, we read all the primary driver files and extract -# the driver API tables from each one. - -foreach my $src (@srcs) { - open FILE, "<$src" or - die "cannot read $src: $!"; - - my $groups_regex = join("|", keys %groups); - $ingrp = undef; - my $impl; - while (defined($line = <FILE>)) { - if (!$ingrp) { - # skip non-matching lines early to save time - next if not $line =~ /$groups_regex/; - - if ($line =~ /^\s*(?:static\s+)?($groups_regex)\s+(\w+)\s*=\s*{/ || - $line =~ /^\s*(?:static\s+)?($groups_regex)\s+NAME\(\w+\)\s*=\s*{/) { - $ingrp = $1; - $impl = $src; - - if ($impl =~ m,.*/node_device_(\w+)\.c,) { - $impl = $1; - } else { - $impl =~ s,.*/(\w+?)_((\w+)_)?(\w+)\.c,$1,; - } - - if ($groups{$ingrp}->{drivers}->{$impl}) { - die "Group $ingrp already contains $impl"; - } - - $groups{$ingrp}->{drivers}->{$impl} = {}; - } - - } else { - if ($line =~ m!\s*\.(\w+)\s*=\s*(\w+)\s*,?\s*(?:/\*\s*(\d+\.\d+\.\d+)\s*(?:-\s*(\d+\.\d+\.\d+))?\s*\*/\s*)?$!) { - my $api = $1; - my $meth = $2; - my $vers = $3; - my $deleted = $4; - - next if $api eq "no" || $api eq "name"; - - if ($meth eq "NULL" && !defined $deleted) { - die "Method impl for $api is NULL, but no deleted version is provided"; - } - if ($meth ne "NULL" && defined $deleted) { - die "Method impl for $api is non-NULL, but deleted version is provided"; - } - - die "Method $meth in $src is missing version" unless defined $vers || $api eq "connectURIProbe"; - - if (!exists($groups{$ingrp}->{apis}->{$api})) { - next if $api =~ /\w(Open|Close|URIProbe)/; - - die "Found unexpected method $api in $ingrp\n"; - } - - $groups{$ingrp}->{drivers}->{$impl}->{$api} = {}; - $groups{$ingrp}->{drivers}->{$impl}->{$api}->{vers} = $vers; - $groups{$ingrp}->{drivers}->{$impl}->{$api}->{deleted} = $deleted; - if ($api eq "domainMigratePrepare" || - $api eq "domainMigratePrepare2" || - $api eq "domainMigratePrepare3") { - if (!$groups{$ingrp}->{drivers}->{$impl}->{"domainMigrate"}) { - $groups{$ingrp}->{drivers}->{$impl}->{"domainMigrate"} = {}; - $groups{$ingrp}->{drivers}->{$impl}->{"domainMigrate"}->{vers} = $vers; - } - } - - } elsif ($line =~ /}/) { - $ingrp = undef; - } - } - } - - close FILE; -} - - -# The '.open' driver method is used for 3 public APIs, so we -# have a bit of manual fixup todo with the per-driver versioning -# and support matrix - -$groups{virHypervisorDriver}->{apis}->{"openAuth"} = "virConnectOpenAuth"; -$groups{virHypervisorDriver}->{apis}->{"openReadOnly"} = "virConnectOpenReadOnly"; -$groups{virHypervisorDriver}->{apis}->{"domainMigrate"} = "virDomainMigrate"; - -my $openAuthVers = (0 * 1000 * 1000) + (4 * 1000) + 0; - -foreach my $drv (keys %{$groups{"virHypervisorDriver"}->{drivers}}) { - my $openVersStr = $groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"connectOpen"}->{vers}; - my $openVers; - if ($openVersStr =~ /(\d+)\.(\d+)\.(\d+)/) { - $openVers = ($1 * 1000 * 1000) + ($2 * 1000) + $3; - } - - # virConnectOpenReadOnly always matches virConnectOpen version - $groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"connectOpenReadOnly"} = - $groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"connectOpen"}; - - $groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"connectOpenAuth"} = {}; - - # virConnectOpenAuth is always 0.4.0 if the driver existed - # before this time, otherwise it matches the version of - # the driver's virConnectOpen entry - if ($openVersStr eq "Y" || - $openVers >= $openAuthVers) { - $groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"connectOpenAuth"}->{vers} = $openVersStr; - } else { - $groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"connectOpenAuth"}->{vers} = "0.4.0"; - } -} - - -# Another special case for the virDomainCreateLinux which was replaced -# with virDomainCreateXML -$groups{virHypervisorDriver}->{apis}->{"domainCreateLinux"} = "virDomainCreateLinux"; - -my $createAPIVers = (0 * 1000 * 1000) + (0 * 1000) + 3; - -foreach my $drv (keys %{$groups{"virHypervisorDriver"}->{drivers}}) { - my $createVersStr = $groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"domainCreateXML"}->{vers}; - next unless defined $createVersStr; - my $createVers; - if ($createVersStr =~ /(\d+)\.(\d+)\.(\d+)/) { - $createVers = ($1 * 1000 * 1000) + ($2 * 1000) + $3; - } - - $groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"domainCreateLinux"} = {}; - - # virCreateLinux is always 0.0.3 if the driver existed - # before this time, otherwise it matches the version of - # the driver's virCreateXML entry - if ($createVersStr eq "Y" || - $createVers >= $createAPIVers) { - $groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"domainCreateLinux"}->{vers} = $createVersStr; - } else { - $groups{"virHypervisorDriver"}->{drivers}->{$drv}->{"domainCreateLinux"}->{vers} = "0.0.3"; - } -} - - -# Finally we generate the HTML file with the tables - -print <<EOF; -<?xml version="1.0" encoding="UTF-8"?> -<!DOCTYPE html> -<html xmlns="http://www.w3.org/1999/xhtml"> -<body class="hvsupport"> -<h1>libvirt API support matrix</h1> - -<ul id="toc"></ul> - -<p> -This page documents which <a href="html/">libvirt calls</a> work on -which libvirt drivers / hypervisors, and which version the API appeared -in. If a hypervisor driver later dropped support for the API, the version -when it was removed is also mentioned (highlighted in -<span class="removedhv">dark red</span>). -</p> - -EOF - - foreach my $grp (sort { $a cmp $b } keys %groups) { - print "<h2><a id=\"$grp\">", $groupheaders{$grp}, "</a></h2>\n"; - print <<EOF; -<table class="top_table"> -<thead> -<tr> -<th>API</th> -<th>Version</th> -EOF - - foreach my $drv (sort { $a cmp $b } keys %{$groups{$grp}->{drivers}}) { - print " <th>$drv</th>\n"; - } - - print <<EOF; -</tr> -</thead> -<tbody> -EOF - - my $row = 0; - foreach my $field (sort { - $groups{$grp}->{apis}->{$a} - cmp - $groups{$grp}->{apis}->{$b} - } keys %{$groups{$grp}->{apis}}) { - my $api = $groups{$grp}->{apis}->{$field}; - my $vers = $apis{$api}->{vers}; - my $htmlgrp = $apis{$api}->{file}; - print <<EOF; -<tr> -<td> -EOF - - if (defined $htmlgrp) { - print <<EOF; -<a href=\"html/libvirt-$htmlgrp.html#$api\">$api</a> -EOF - - } else { - print $api; - } - print <<EOF; -</td> -<td>$vers</td> -EOF - - foreach my $drv (sort {$a cmp $b } keys %{$groups{$grp}->{drivers}}) { - print "<td>"; - if (exists $groups{$grp}->{drivers}->{$drv}->{$field}) { - if ($groups{$grp}->{drivers}->{$drv}->{$field}->{vers}) { - print $groups{$grp}->{drivers}->{$drv}->{$field}->{vers}; - } - if ($groups{$grp}->{drivers}->{$drv}->{$field}->{deleted}) { - print " - <span class=\"removedhv\">", $groups{$grp}->{drivers}->{$drv}->{$field}->{deleted}, "</span>"; - } - } - print "</td>\n"; - } - - print <<EOF; -</tr> -EOF - - $row++; - if (($row % 15) == 0) { - print <<EOF; -<tr> -<th>API</th> -<th>Version</th> -EOF - - foreach my $drv (sort { $a cmp $b } keys %{$groups{$grp}->{drivers}}) { - print " <th>$drv</th>\n"; - } - - print <<EOF; -</tr> -EOF - } - - } - - print <<EOF; -</tbody> -</table> -EOF -} - -print <<EOF; -</body> -</html> -EOF diff --git a/docs/hvsupport.py b/docs/hvsupport.py new file mode 100755 index 0000000000..cc05cce565 --- /dev/null +++ b/docs/hvsupport.py @@ -0,0 +1,516 @@ +#!/usr/bin/env python +# +# Copyright (C) 2011-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 sys +import os.path +import re + +if len(sys.argv) != 2: + print("syntax: %s SRCDIR\n" % sys.argv[0], file=sys.stderr) + +srcdir = sys.argv[1] + +symslibvirt = os.path.join(srcdir, "libvirt_public.syms") +symsqemu = os.path.join(srcdir, "libvirt_qemu.syms") +symslxc = os.path.join(srcdir, "libvirt_lxc.syms") +drivertablefiles = [ + os.path.join(srcdir, "driver-hypervisor.h"), + os.path.join(srcdir, "driver-interface.h"), + os.path.join(srcdir, "driver-network.h"), + os.path.join(srcdir, "driver-nodedev.h"), + os.path.join(srcdir, "driver-nwfilter.h"), + os.path.join(srcdir, "driver-secret.h"), + os.path.join(srcdir, "driver-state.h"), + os.path.join(srcdir, "driver-storage.h"), + os.path.join(srcdir, "driver-stream.h"), +] + +groupheaders = { + "virHypervisorDriver": "Hypervisor APIs", + "virNetworkDriver": "Virtual Network APIs", + "virInterfaceDriver": "Host Interface APIs", + "virNodeDeviceDriver": "Host Device APIs", + "virStorageDriver": "Storage Pool APIs", + "virSecretDriver": "Secret APIs", + "virNWFilterDriver": "Network Filter APIs", +} + + +srcs = [] +for root, dirs, files in os.walk(srcdir): + for file in files: + if ((file.endswith("driver.c") and + not file.endswith("vbox_driver.c")) or + file.endswith("common.c") or + file.endswith("tmpl.c") or + file.endswith("monitor.c") or + file.endswith("hal.c") or + file.endswith("udev.c")): + srcs.append(os.path.join(root, file)) + + +# Map API functions to the header and documentation files they're in +# so that we can generate proper hyperlinks to their documentation. +# +# The function names are grep'd from the XML output of apibuild.py. +def getAPIFilenames(filename): + files = {} + + with open(filename) as fh: + prog = re.compile(r"\s*<function name='([^']+)' file='([^']+)'.*") + for line in fh: + res = prog.match(line) + if res is not None: + files[res.group(1)] = res.group(2) + + if len(files) == 0: + raise Exception(("No functions found in %s. " + + "Has the apibuild.py output changed?") % + filename) + + return files + + +def parseSymsFile(apisref, prefix, filename, xmlfilename): + vers = None + prevvers = None + + filenames = getAPIFilenames(xmlfilename) + + with open(filename) as fh: + groupstartprog = re.compile(r"^\s*%s_(\d+\.\d+\.\d+)\s*{\s*$" % + prefix) + groupendprog1 = re.compile(r"^\s*}\s*;\s*$") + groupendprog2 = re.compile(r"^\s*}\s*%s_(\d+\.\d+\.\d+)\s*;\s*$" % + prefix) + symbolprog = re.compile(r"^\s*(\w+)\s*;\s*$") + for line in fh: + line = line.strip() + + if line == "": + continue + if line[0] == '#': + continue + if line.startswith("global:"): + continue + if line.startswith("local:"): + continue + + groupstartmatch = groupstartprog.match(line) + groupendmatch1 = groupendprog1.match(line) + groupendmatch2 = groupendprog2.match(line) + symbolmatch = symbolprog.match(line) + if groupstartmatch is not None: + if vers is not None: + raise Exception("malformed syms file when starting group") + + vers = groupstartmatch.group(1) + elif groupendmatch1 is not None: + if prevvers is not None: + raise Exception("malformed syms file when ending group") + + prevvers = vers + vers = None + elif groupendmatch2 is not None: + if groupendmatch2.group(1) != prevvers: + raise Exception("malformed syms file %s != %s " + + "when ending group" % + (groupendmatch2.group(1), prevvers)) + + prevvers = vers + vers = None + elif symbolmatch is not None: + name = symbolmatch.group(1) + apisref[name] = { + "vers": vers, + "file": filenames.get(name), + } + else: + raise Exception("unexpected data %s" % line) + + +apis = {} +# Get the list of all public APIs and their corresponding version +parseSymsFile(apis, "LIBVIRT", symslibvirt, + os.path.join(srcdir, "../docs/libvirt-api.xml")) + +# And the same for the QEMU specific APIs +parseSymsFile(apis, "LIBVIRT_QEMU", symsqemu, + os.path.join(srcdir, "../docs/libvirt-qemu-api.xml")) + +# And the same for the LXC specific APIs +parseSymsFile(apis, "LIBVIRT_LXC", symslxc, + os.path.join(srcdir, "../docs/libvirt-lxc-api.xml")) + + +# Some special things which aren't public APIs, +# but we want to report +apis["virConnectSupportsFeature"] = { + "vers": "0.3.2" +} +apis["virDomainMigratePrepare"] = { + "vers": "0.3.2" +} +apis["virDomainMigratePerform"] = { + "vers": "0.3.2" +} +apis["virDomainMigrateFinish"] = { + "vers": "0.3.2" +} +apis["virDomainMigratePrepare2"] = { + "vers": "0.5.0" +} +apis["virDomainMigrateFinish2"] = { + "vers": "0.5.0" +} +apis["virDomainMigratePrepareTunnel"] = { + "vers": "0.7.2" +} + +apis["virDomainMigrateBegin3"] = { + "vers": "0.9.2" +} +apis["virDomainMigratePrepare3"] = { + "vers": "0.9.2" +} +apis["virDomainMigratePrepareTunnel3"] = { + "vers": "0.9.2" +} +apis["virDomainMigratePerform3"] = { + "vers": "0.9.2" +} +apis["virDomainMigrateFinish3"] = { + "vers": "0.9.2" +} +apis["virDomainMigrateConfirm3"] = { + "vers": "0.9.2" +} + +apis["virDomainMigrateBegin3Params"] = { + "vers": "1.1.0" +} +apis["virDomainMigratePrepare3Params"] = { + "vers": "1.1.0" +} +apis["virDomainMigratePrepareTunnel3Params"] = { + "vers": "1.1.0" +} +apis["virDomainMigratePerform3Params"] = { + "vers": "1.1.0" +} +apis["virDomainMigrateFinish3Params"] = { + "vers": "1.1.0" +} +apis["virDomainMigrateConfirm3Params"] = { + "vers": "1.1.0" +} + + +# Now we want to get the mapping between public APIs +# and driver struct fields. This lets us later match +# update the driver impls with the public APis. + +# Group name -> hash of APIs { fields -> api name } +groups = {} +ingrp = None +for drivertablefile in drivertablefiles: + with open(drivertablefile) as fh: + starttableprog = re.compile(r"struct _(vir\w*Driver)") + endtableprog = re.compile(r"};") + callbackprog = re.compile(r"^\s*vir(?:Drv)(\w+)\s+(\w+);\s*$") + ignoreapiprog = re.compile(r"\w+(Open|Close|URIProbe)") + for line in fh: + starttablematch = starttableprog.match(line) + if starttablematch is not None: + grp = starttablematch.group(1) + if grp != "virStateDriver" and grp != "virStreamDriver": + ingrp = grp + groups[ingrp] = { + "apis": {}, + "drivers": {} + } + elif ingrp is not None: + callbackmatch = callbackprog.match(line) + if callbackmatch is not None: + name = callbackmatch.group(1) + field = callbackmatch.group(2) + + api = "vir" + name + if api in apis: + groups[ingrp]["apis"][field] = api + elif ignoreapiprog.match(api) is not None: + continue + else: + raise Exception(("driver %s does not have " + + "a public API") % name) + elif endtableprog.match(line): + ingrp = None + + +# Finally, we read all the primary driver files and extract +# the driver API tables from each one. + +for src in srcs: + with open(src) as fh: + groupsre = "|".join(groups.keys()) + groupsprog2 = re.compile(r"^\s*(static\s+)?(" + + groupsre + + r")\s+(\w+)\s*=\s*{") + groupsprog3 = re.compile(r"^\s*(static\s+)?(" + + groupsre + + r")\s+NAME\(\w+\)\s*=\s*{") + nodedevimplprog = re.compile(r".*/node_device_(\w+)\.c") + miscimplprog = re.compile(r".*/(\w+?)_((\w+)_)?(\w+)\.c") + callbackprog = re.compile(r"\s*\.(\w+)\s*=\s*(\w+)\s*,?\s*" + + r"(?:/\*\s*(\d+\.\d+\.\d+)\s*" + + r"(?:-\s*(\d+\.\d+\.\d+))?\s*\*/\s*)?$") + skipapiprog = re.compile(r"\w+(Open|Close|URIProbe)") + + ingrp = None + impl = None + for line in fh: + if ingrp is None: + m = groupsprog2.match(line) + if m is None: + m = groupsprog3.match(line) + if m is not None: + ingrp = m.group(2) + impl = src + + implmatch = nodedevimplprog.match(impl) + if implmatch is None: + implmatch = miscimplprog.match(impl) + if implmatch is None: + raise Exception("Unexpected impl format '%s'" % impl) + impl = implmatch.group(1) + + if impl in groups[ingrp]["drivers"]: + raise Exception( + "Group %s already contains %s" % (ingrp, impl)) + + groups[ingrp]["drivers"][impl] = {} + else: + callbackmatch = callbackprog.match(line) + if callbackmatch is not None: + api = callbackmatch.group(1) + meth = callbackmatch.group(2) + vers = callbackmatch.group(3) + deleted = callbackmatch.group(4) + + if api == "no" or api == "name": + continue + + if meth == "NULL" and deleted is None: + raise Exception( + ("Method impl for %s is NULL, but " + + "no deleted version is provided") % api) + + if meth != "NULL" and deleted is not None: + raise Exception( + ("Method impl for %s is non-NULL, but " + + "deleted version is provided") % api) + + if vers is None and api != "connectURIProbe": + raise Exception( + "Method %s in %s is missing version" % + (meth, src)) + + if api not in groups[ingrp]["apis"]: + if skipapiprog.match(api): + continue + + raise Exception("Found unexpected method " + + "%s in %s" % (api, ingrp)) + + groups[ingrp]["drivers"][impl][api] = { + "vers": vers, + "deleted": deleted, + } + + if (api == "domainMigratePrepare" or + api == "domainMigratePrepare2" or + api == "domainMigratePrepare3"): + if ("domainMigrate" not in + groups[ingrp]["drivers"][impl]): + groups[ingrp]["drivers"][impl]["domainMigrate"] = { + "vers": vers, + } + elif line.find("}") != -1: + ingrp = None + + +# The '.open' driver method is used for 3 public APIs, so we +# have a bit of manual fixup todo with the per-driver versioning +# and support matrix + +groups["virHypervisorDriver"]["apis"]["openAuth"] = \ + "virConnectOpenAuth" +groups["virHypervisorDriver"]["apis"]["openReadOnly"] = \ + "virConnectOpenReadOnly" +groups["virHypervisorDriver"]["apis"]["domainMigrate"] = \ + "virDomainMigrate" + +openAuthVers = (0 * 1000 * 1000) + (4 * 1000) + 0 + +drivers = groups["virHypervisorDriver"]["drivers"] +for drv in drivers.keys(): + openVersStr = drivers[drv]["connectOpen"]["vers"] + openVers = 0 + if openVersStr != "Y": + openVersBits = openVersStr.split(".") + if len(openVersBits) != 3: + raise Exception("Expected 3 digit version for %s" % openVersStr) + openVers = ((int(openVersBits[0]) * 1000 * 1000) + + (int(openVersBits[1]) * 1000) + + int(openVersBits[2])) + + # virConnectOpenReadOnly always matches virConnectOpen version + drivers[drv]["connectOpenReadOnly"] = \ + drivers[drv]["connectOpen"] + + # virConnectOpenAuth is always 0.4.0 if the driver existed + # before this time, otherwise it matches the version of + # the driver's virConnectOpen entry + if openVersStr == "Y" or openVers >= openAuthVers: + vers = openVersStr + else: + vers = "0.4.0" + drivers[drv]["connectOpenAuth"] = { + "vers": vers, + } + + +# Another special case for the virDomainCreateLinux which was replaced +# with virDomainCreateXML +groups["virHypervisorDriver"]["apis"]["domainCreateLinux"] = \ + "virDomainCreateLinux" + +createAPIVers = (0 * 1000 * 1000) + (0 * 1000) + 3 + +for drv in drivers.keys(): + if "domainCreateXML" not in drivers[drv]: + continue + createVersStr = drivers[drv]["domainCreateXML"]["vers"] + createVers = 0 + if createVersStr != "Y": + createVersBits = createVersStr.split(".") + if len(createVersBits) != 3: + raise Exception("Expected 3 digit version for %s" % createVersStr) + createVers = ((int(createVersBits[0]) * 1000 * 1000) + + (int(createVersBits[1]) * 1000) + + int(createVersBits[2])) + + # virCreateLinux is always 0.0.3 if the driver existed + # before this time, otherwise it matches the version of + # the driver's virCreateXML entry + if createVersStr == "Y" or createVers >= createAPIVers: + vers = createVersStr + else: + vers = "0.0.3" + + drivers[drv]["domainCreateLinux"] = { + "vers": vers, + } + + +# Finally we generate the HTML file with the tables + +print('''<?xml version="1.0" encoding="UTF-8"?> +<!DOCTYPE html> +<html xmlns="http://www.w3.org/1999/xhtml"> +<body class="hvsupport"> +<h1>libvirt API support matrix</h1> + +<ul id="toc"></ul> + +<p> +This page documents which <a href="html/">libvirt calls</a> work on +which libvirt drivers / hypervisors, and which version the API appeared +in. If a hypervisor driver later dropped support for the API, the version +when it was removed is also mentioned (highlighted in +<span class="removedhv">dark red</span>). +</p> +''') + +for grp in sorted(groups.keys()): + print("<h2><a id=\"%s\">%s</a></h2>" % (grp, groupheaders[grp])) + print('''<table class="top_table"> +<thead> +<tr> +<th>API</th> +<th>Version</th>''') + + for drv in sorted(groups[grp]["drivers"].keys()): + print(" <th>%s</th>" % drv) + + print('''</tr> +</thead> +<tbody>''') + + row = 0 + + def sortkey(field): + return groups[grp]["apis"][field] + + for field in sorted(groups[grp]["apis"].keys(), key=sortkey): + api = groups[grp]["apis"][field] + vers = apis[api]["vers"] + htmlgrp = apis[api].get("file") + print("<tr>") + + if htmlgrp is not None: + print(('''<td>\n<a href=\"html/libvirt-%s.html#%s\">''' + + '''%s</a>\n</td>''') % + (htmlgrp, api, api)) + else: + print("<td>\n%s</td>" % api) + + print("<td>%s</td>" % vers) + + for drv in sorted(groups[grp]["drivers"].keys()): + info = "" + if field in groups[grp]["drivers"][drv]: + vers = groups[grp]["drivers"][drv][field]["vers"] + if vers is not None: + info = info + vers + + deleted = groups[grp]["drivers"][drv][field].get("deleted") + if deleted is not None: + info = info + (''' - <span class="removedhv">''' + + '''%s</span>''' % deleted) + + print("<td>%s</td>" % info) + + print("</tr>") + + row = row + 1 + if (row % 15) == 0: + print('''<tr> +<th>API</th> +<th>Version</th>''') + + for drv in sorted(groups[grp]["drivers"].keys()): + print(" <th>%s</th>" % drv) + + print("</tr>") + + print("</tbody>\n</table>") + +print("</body>\n</html>") -- 2.21.0