[libvirt PATCH v3 0/7] the gnulib saga: the season finale

Daniel P. Berrangé posted 7 patches 2 weeks ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200205171858.2694632-1-berrange@redhat.com
.color_coded.in                 |    2 -
.gitignore                      |    9 +-
.gitmodules                     |    3 -
.gnulib                         |    1 -
.ycm_extra_conf.py.in           |    2 -
Makefile.am                     |    2 +-
README-hacking                  |    9 +-
autogen.sh                      |  219 +------
bootstrap                       | 1073 -------------------------------
bootstrap.conf                  |  100 ---
build-aux/syntax-check.mk       |  160 +----
ci/build.sh                     |    4 +-
config-post.h                   |    5 +-
configure.ac                    |   11 +-
docs/compiling.html.in          |   25 -
docs/hacking.html.in            |    5 +-
gnulib/lib/Makefile.am          |   30 -
libvirt.spec.in                 |    2 -
m4/virt-compile-warnings.m4     |   18 +-
po/POTFILES.in                  |    1 -
src/Makefile.am                 |    7 +-
src/admin/Makefile.inc.am       |    1 -
src/bhyve/Makefile.inc.am       |    1 -
src/interface/Makefile.inc.am   |    1 -
src/libvirt_private.syms        |   14 +-
src/libvirt_probes.d            |   14 +
src/libxl/Makefile.inc.am       |    1 -
src/locking/Makefile.inc.am     |    3 -
src/logging/Makefile.inc.am     |    1 -
src/lxc/Makefile.inc.am         |    2 -
src/network/Makefile.inc.am     |    3 +-
src/node_device/Makefile.inc.am |    2 -
src/nwfilter/Makefile.inc.am    |    1 -
src/qemu/Makefile.inc.am        |    1 -
src/remote/Makefile.inc.am      |    1 -
src/rpc/virnetclient.c          |  222 ++++---
src/rpc/virnetsocket.c          |    6 -
src/secret/Makefile.inc.am      |    1 -
src/security/Makefile.inc.am    |    1 -
src/storage/Makefile.inc.am     |   16 -
src/util/Makefile.inc.am        |    6 +-
src/util/viralloc.h             |    3 +-
src/util/virbitmap.c            |    4 +-
src/util/virevent.c             |   21 +-
src/util/vireventglib.c         |  499 ++++++++++++++
src/util/vireventglib.h         |   28 +
src/util/vireventglibwatch.c    |  249 +++++++
src/util/vireventglibwatch.h    |   48 ++
src/util/vireventpoll.c         |  772 ----------------------
src/util/vireventpoll.h         |  126 ----
src/util/virfile.c              |    7 +-
src/util/virsocket.h            |   15 -
src/vbox/Makefile.inc.am        |    1 -
src/vz/Makefile.inc.am          |    1 -
tests/Makefile.am               |   21 +-
tests/virstringtest.c           |    3 +-
tools/Makefile.am               |    9 +-
tools/virsh-domain.c            |  388 ++++++-----
tools/virsh.h                   |    3 +-
59 files changed, 1268 insertions(+), 2916 deletions(-)
delete mode 160000 .gnulib
delete mode 100755 bootstrap
delete mode 100644 bootstrap.conf
delete mode 100644 gnulib/lib/Makefile.am
create mode 100644 src/util/vireventglib.c
create mode 100644 src/util/vireventglib.h
create mode 100644 src/util/vireventglibwatch.c
create mode 100644 src/util/vireventglibwatch.h
delete mode 100644 src/util/vireventpoll.c
delete mode 100644 src/util/vireventpoll.h

[libvirt PATCH v3 0/7] the gnulib saga: the season finale

Posted by Daniel P. Berrangé 2 weeks ago
This is a followup to

  v1: https://www.redhat.com/archives/libvir-list/2020-January/msg00900.html
  v2: https://www.redhat.com/archives/libvir-list/2020-January/msg01158.html

At the end of this series we have 100% eliminated use of GNULIB
from libvirt.

Some things to note

 - I have build tested this on Travis platforms and manually
   via FreeBSD 11/12. This covers make, make syntax-check &
   make check

 - I've validated that virsh still works with mingw64 builds
   on Windows 2008r2.

 - I've done basic functional testing on Fedora 31, starting
   and stopping VMs & other other simple APIs

The config.h we generate is much much smaller than before as we
eliminated alot of gnulib macros.

The risk here is that we are no longer setting some HAVE_XXX
in config.h that we rely on. To mitigate this I did a diff
of config.h before & after this series to determinw which
HAVE_XXX we no longer set. I then grepped the source to see
if we actually use any of them. This identified a few mistakes
which I fixed in testing this series.

The builds times for libvirt after applying this series have
some significant gains, improving speed of all stages (autogen,
configure & make).

Overall while this was time consuming work (due to massive number
of builds for testing each step), it is surprising just how easy
it was eliminate need for GNULIB.  GLib helped a little bit in
this respect, but the biggest factor is simply that a large
number of issues GNULIB fixes only matter for ancient / obsolete
OS platforms.

With libvirt only targetting modern Linux, FreeBSD, macOS & MinGW,
the only really hard stuff where GNULIB was a big help is the
Windows sockets portability.

GNULIB was a pretty valuable approach when there were countless
flavours of UNIX to worry about with poor levels of POSIX API
compatibility. With a typical modern set of platforms, I think
it is better to just use a library like GLib and deal with any
other portability problems explicitly.

Almost certainly someone will appear after next release and
complain that libvirt no longer builds on some platform that
we don't officially support. My expectation is that when this
happens it will be reasonably easy to fix whatever problem
they report. Also at that time we can also consider whether
the platform needs to be added to CI.

Changed in v3:

 - Merged all the already acked patches which didn't
   have ordering dependencies
 - Rewrite way virsh monitor long running jobs to
   avoid non-portable pipe usage
 - Improve debug messages in glib event loop
 - Add dtrace probes to glib event loop
 - Remove main context acquire/release steps
 - Fix buck passing mistakes in RPC client
   event loop conversion
 - Purge more from syntax-check
 - Improve event watch to GIOCondition conversions
 - Fix leak of windows event object HANDLE

Daniel P. Berrangé (7):
  tools: rewrite interactive job monitoring logic
  src: introduce helper API for creating GSource for socket
  rpc: convert RPC client to use GMainLoop instead of poll
  util: import an event loop impl based on GMainContext
  util: switch to use the GLib event loop impl
  util: delete the poll() based event loop impl
  gnulib: delete all gnulib integration

 .color_coded.in                 |    2 -
 .gitignore                      |    9 +-
 .gitmodules                     |    3 -
 .gnulib                         |    1 -
 .ycm_extra_conf.py.in           |    2 -
 Makefile.am                     |    2 +-
 README-hacking                  |    9 +-
 autogen.sh                      |  219 +------
 bootstrap                       | 1073 -------------------------------
 bootstrap.conf                  |  100 ---
 build-aux/syntax-check.mk       |  160 +----
 ci/build.sh                     |    4 +-
 config-post.h                   |    5 +-
 configure.ac                    |   11 +-
 docs/compiling.html.in          |   25 -
 docs/hacking.html.in            |    5 +-
 gnulib/lib/Makefile.am          |   30 -
 libvirt.spec.in                 |    2 -
 m4/virt-compile-warnings.m4     |   18 +-
 po/POTFILES.in                  |    1 -
 src/Makefile.am                 |    7 +-
 src/admin/Makefile.inc.am       |    1 -
 src/bhyve/Makefile.inc.am       |    1 -
 src/interface/Makefile.inc.am   |    1 -
 src/libvirt_private.syms        |   14 +-
 src/libvirt_probes.d            |   14 +
 src/libxl/Makefile.inc.am       |    1 -
 src/locking/Makefile.inc.am     |    3 -
 src/logging/Makefile.inc.am     |    1 -
 src/lxc/Makefile.inc.am         |    2 -
 src/network/Makefile.inc.am     |    3 +-
 src/node_device/Makefile.inc.am |    2 -
 src/nwfilter/Makefile.inc.am    |    1 -
 src/qemu/Makefile.inc.am        |    1 -
 src/remote/Makefile.inc.am      |    1 -
 src/rpc/virnetclient.c          |  222 ++++---
 src/rpc/virnetsocket.c          |    6 -
 src/secret/Makefile.inc.am      |    1 -
 src/security/Makefile.inc.am    |    1 -
 src/storage/Makefile.inc.am     |   16 -
 src/util/Makefile.inc.am        |    6 +-
 src/util/viralloc.h             |    3 +-
 src/util/virbitmap.c            |    4 +-
 src/util/virevent.c             |   21 +-
 src/util/vireventglib.c         |  499 ++++++++++++++
 src/util/vireventglib.h         |   28 +
 src/util/vireventglibwatch.c    |  249 +++++++
 src/util/vireventglibwatch.h    |   48 ++
 src/util/vireventpoll.c         |  772 ----------------------
 src/util/vireventpoll.h         |  126 ----
 src/util/virfile.c              |    7 +-
 src/util/virsocket.h            |   15 -
 src/vbox/Makefile.inc.am        |    1 -
 src/vz/Makefile.inc.am          |    1 -
 tests/Makefile.am               |   21 +-
 tests/virstringtest.c           |    3 +-
 tools/Makefile.am               |    9 +-
 tools/virsh-domain.c            |  388 ++++++-----
 tools/virsh.h                   |    3 +-
 59 files changed, 1268 insertions(+), 2916 deletions(-)
 delete mode 160000 .gnulib
 delete mode 100755 bootstrap
 delete mode 100644 bootstrap.conf
 delete mode 100644 gnulib/lib/Makefile.am
 create mode 100644 src/util/vireventglib.c
 create mode 100644 src/util/vireventglib.h
 create mode 100644 src/util/vireventglibwatch.c
 create mode 100644 src/util/vireventglibwatch.h
 delete mode 100644 src/util/vireventpoll.c
 delete mode 100644 src/util/vireventpoll.h

-- 
2.24.1