[libvirt] [PATCH 00/23] Take 64 gnulib modules, eliminate 14, to give 50 remaining

Daniel P. Berrangé posted 23 patches 4 years, 3 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200102145357.6724-1-berrange@redhat.com
.travis.yml                             |  15 ++
bootstrap.conf                          | 173 +++++++++++++++---------
build-aux/syntax-check.mk               |   4 +-
config-post.h                           |  18 ++-
docs/platforms.html.in                  |  13 +-
include/libvirt/libvirt-event.h         |   4 +
m4/virt-glib.m4                         |   2 +-
src/conf/domain_conf.c                  |  32 ++---
src/conf/moment_conf.c                  |   7 +-
src/conf/node_device_conf.c             |   6 +-
src/esx/esx_vi_types.c                  |  71 +---------
src/internal.h                          |   4 +-
src/libvirt.c                           |  25 +---
src/libvirt_private.syms                |   2 +
src/libxl/libxl_driver.c                |  21 +--
src/locking/lock_driver_sanlock.c       |   6 +-
src/node_device/node_device_udev.c      |  12 +-
src/nwfilter/nwfilter_dhcpsnoop.c       |   2 +-
src/qemu/qemu_backup.c                  |   5 +-
src/qemu/qemu_command.c                 |   1 -
src/qemu/qemu_domain.c                  |  14 +-
src/qemu/qemu_driver.c                  |   7 +-
src/qemu/qemu_firmware.c                |   4 +-
src/qemu/qemu_tpm.c                     |  33 +----
src/remote/libvirtd.conf.in             |   8 +-
src/rpc/virnetsaslcontext.c             |  11 +-
src/rpc/virnetsocket.c                  |   5 +-
src/rpc/virnettlscontext.c              |  10 +-
src/security/security_dac.c             |   2 +-
src/security/virt-aa-helper.c           |   8 +-
src/storage/storage_backend_disk.c      |   9 +-
src/storage/storage_driver.c            |   1 -
src/storage/storage_util.c              |  11 +-
src/test/test_driver.c                  |  51 ++-----
src/util/glibcompat.c                   |  13 ++
src/util/glibcompat.h                   |   6 +
src/util/iohelper.c                     |   2 +-
src/util/vircgroupv1.c                  |   4 +-
src/util/virevent.c                     |   7 +
src/util/virfile.c                      |  49 ++++---
src/util/virfile.h                      |   2 +
src/util/virlog.c                       |  22 +--
src/util/virmdev.c                      |  12 +-
src/util/virnetdev.c                    |   5 +-
src/util/virpci.c                       |  17 ++-
src/util/virstoragefile.c               |  13 +-
src/util/virsystemd.c                   |   4 +-
src/util/virtime.c                      |  22 +--
src/util/virutil.c                      |  12 +-
src/vbox/vbox_XPCOMCGlue.c              |   4 +-
src/vmware/vmware_conf.c                |   7 +-
src/vz/vz_sdk.c                         |  10 +-
tests/eventtest.c                       |  18 +--
tests/libxlxml2domconfigtest.c          |   4 +-
tests/lxcxml2xmltest.c                  |   2 +-
tests/qemudomaincheckpointxml2xmltest.c |   2 +-
tests/qemudomainsnapshotxml2xmltest.c   |   2 +-
tests/qemufirmwaretest.c                |   2 +-
tests/qemuhotplugtest.c                 |   2 +-
tests/qemumemlocktest.c                 |   2 +-
tests/qemusecuritytest.c                |   4 +-
tests/qemuvhostusertest.c               |   2 +-
tests/qemuxml2argvtest.c                |  24 ++--
tests/qemuxml2xmltest.c                 |   4 +-
tests/securityselinuxhelper.c           |   4 +-
tests/testutils.c                       |  15 +-
tests/testutils.h                       |   6 +-
tests/testutilsqemu.c                   |   4 +-
tests/vircgrouptest.c                   |  16 +--
tests/virconfdata/libvirtd.conf         |   8 +-
tests/virconfdata/libvirtd.out          |   8 +-
tests/virfilemock.c                     |  12 +-
tests/virfiletest.c                     |   4 +-
tests/virhostdevtest.c                  |   2 +-
tests/virnettlscontexttest.c            |   2 +-
tests/virnettlssessiontest.c            |   2 +-
tests/virpcimock.c                      |   3 +-
tests/virpcitest.c                      |   2 +-
tests/virportallocatortest.c            |   2 +-
tests/virstoragetest.c                  |   1 -
tests/virsystemdtest.c                  |  46 +++----
tests/virtimetest.c                     |   4 +-
tools/virsh-domain.c                    |  10 +-
tools/virt-login-shell-helper.c         |  17 ++-
tools/vsh.c                             |  27 +---
tools/vsh.h                             |   1 -
86 files changed, 470 insertions(+), 602 deletions(-)
[libvirt] [PATCH 00/23] Take 64 gnulib modules, eliminate 14, to give 50 remaining
Posted by Daniel P. Berrangé 4 years, 3 months ago
In the last days before the xmas break I took some time to
eliminate 14 more gnulib modules, bringing us down to just
50 left to go. They're getting harder to eliminate as we go
on, but to give some hints, I've annotated every module in
bootstrap.conf with a suggested replacement strategy.

Daniel P. Berrangé (23):
  build: set min version for CLang to 3.4 / XCode CLang to 5.1
  docs: expand macOS platform support coverage
  travis: add macOS Xcode 11.3 testing
  util: add note about event file descriptors on Windows
  src: always pull in glib/gstdio.h header
  src: switch to use g_setenv/g_unsetenv
  util: add compat wrapper for g_fsync
  src: use g_fsync for portability
  util: introduce virFileDataSync
  src: use g_lstat() instead of lstat()
  src: switch from fnmatch to g_pattern_match_simple
  src: replace clock_gettime()/gettimeofday() with g_get_real_time()
  src: replace last_component() with g_path_get_basename()
  util: replace IS_ABSOLUTE_FILE_NAME with g_path_is_absolute
  src: replace mdir_name() with g_path_get_dirname()
  src: remove unused imports of dirname.h
  src: replace getcwd() with g_get_current_dir()
  util: use realpath/g_canonicalize_filename
  util: replace gethostname() with g_get_hostname()
  src: replace WSAStartup with g_networking_init()
  src: replace strptime()/timegm()/mktime() with GDateTime APIs set
  bootstrap: remove now unused gnulib modules
  bootstrap: annotate with info about desired replacement

 .travis.yml                             |  15 ++
 bootstrap.conf                          | 173 +++++++++++++++---------
 build-aux/syntax-check.mk               |   4 +-
 config-post.h                           |  18 ++-
 docs/platforms.html.in                  |  13 +-
 include/libvirt/libvirt-event.h         |   4 +
 m4/virt-glib.m4                         |   2 +-
 src/conf/domain_conf.c                  |  32 ++---
 src/conf/moment_conf.c                  |   7 +-
 src/conf/node_device_conf.c             |   6 +-
 src/esx/esx_vi_types.c                  |  71 +---------
 src/internal.h                          |   4 +-
 src/libvirt.c                           |  25 +---
 src/libvirt_private.syms                |   2 +
 src/libxl/libxl_driver.c                |  21 +--
 src/locking/lock_driver_sanlock.c       |   6 +-
 src/node_device/node_device_udev.c      |  12 +-
 src/nwfilter/nwfilter_dhcpsnoop.c       |   2 +-
 src/qemu/qemu_backup.c                  |   5 +-
 src/qemu/qemu_command.c                 |   1 -
 src/qemu/qemu_domain.c                  |  14 +-
 src/qemu/qemu_driver.c                  |   7 +-
 src/qemu/qemu_firmware.c                |   4 +-
 src/qemu/qemu_tpm.c                     |  33 +----
 src/remote/libvirtd.conf.in             |   8 +-
 src/rpc/virnetsaslcontext.c             |  11 +-
 src/rpc/virnetsocket.c                  |   5 +-
 src/rpc/virnettlscontext.c              |  10 +-
 src/security/security_dac.c             |   2 +-
 src/security/virt-aa-helper.c           |   8 +-
 src/storage/storage_backend_disk.c      |   9 +-
 src/storage/storage_driver.c            |   1 -
 src/storage/storage_util.c              |  11 +-
 src/test/test_driver.c                  |  51 ++-----
 src/util/glibcompat.c                   |  13 ++
 src/util/glibcompat.h                   |   6 +
 src/util/iohelper.c                     |   2 +-
 src/util/vircgroupv1.c                  |   4 +-
 src/util/virevent.c                     |   7 +
 src/util/virfile.c                      |  49 ++++---
 src/util/virfile.h                      |   2 +
 src/util/virlog.c                       |  22 +--
 src/util/virmdev.c                      |  12 +-
 src/util/virnetdev.c                    |   5 +-
 src/util/virpci.c                       |  17 ++-
 src/util/virstoragefile.c               |  13 +-
 src/util/virsystemd.c                   |   4 +-
 src/util/virtime.c                      |  22 +--
 src/util/virutil.c                      |  12 +-
 src/vbox/vbox_XPCOMCGlue.c              |   4 +-
 src/vmware/vmware_conf.c                |   7 +-
 src/vz/vz_sdk.c                         |  10 +-
 tests/eventtest.c                       |  18 +--
 tests/libxlxml2domconfigtest.c          |   4 +-
 tests/lxcxml2xmltest.c                  |   2 +-
 tests/qemudomaincheckpointxml2xmltest.c |   2 +-
 tests/qemudomainsnapshotxml2xmltest.c   |   2 +-
 tests/qemufirmwaretest.c                |   2 +-
 tests/qemuhotplugtest.c                 |   2 +-
 tests/qemumemlocktest.c                 |   2 +-
 tests/qemusecuritytest.c                |   4 +-
 tests/qemuvhostusertest.c               |   2 +-
 tests/qemuxml2argvtest.c                |  24 ++--
 tests/qemuxml2xmltest.c                 |   4 +-
 tests/securityselinuxhelper.c           |   4 +-
 tests/testutils.c                       |  15 +-
 tests/testutils.h                       |   6 +-
 tests/testutilsqemu.c                   |   4 +-
 tests/vircgrouptest.c                   |  16 +--
 tests/virconfdata/libvirtd.conf         |   8 +-
 tests/virconfdata/libvirtd.out          |   8 +-
 tests/virfilemock.c                     |  12 +-
 tests/virfiletest.c                     |   4 +-
 tests/virhostdevtest.c                  |   2 +-
 tests/virnettlscontexttest.c            |   2 +-
 tests/virnettlssessiontest.c            |   2 +-
 tests/virpcimock.c                      |   3 +-
 tests/virpcitest.c                      |   2 +-
 tests/virportallocatortest.c            |   2 +-
 tests/virstoragetest.c                  |   1 -
 tests/virsystemdtest.c                  |  46 +++----
 tests/virtimetest.c                     |   4 +-
 tools/virsh-domain.c                    |  10 +-
 tools/virt-login-shell-helper.c         |  17 ++-
 tools/vsh.c                             |  27 +---
 tools/vsh.h                             |   1 -
 86 files changed, 470 insertions(+), 602 deletions(-)

-- 
2.24.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/23] Take 64 gnulib modules, eliminate 14, to give 50 remaining
Posted by Fabiano Fidêncio 4 years, 3 months ago
Daniel,

On Thu, Jan 2, 2020 at 3:59 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> In the last days before the xmas break I took some time to
> eliminate 14 more gnulib modules, bringing us down to just
> 50 left to go. They're getting harder to eliminate as we go
> on, but to give some hints, I've annotated every module in
> bootstrap.conf with a suggested replacement strategy.
>
> Daniel P. Berrangé (23):
>   build: set min version for CLang to 3.4 / XCode CLang to 5.1
>   docs: expand macOS platform support coverage
>   travis: add macOS Xcode 11.3 testing
>   util: add note about event file descriptors on Windows
>   src: always pull in glib/gstdio.h header
>   src: switch to use g_setenv/g_unsetenv

In the patch above I'd also take the opportunity and use TRUE/FALSE
instead of 1/0 in g_setenv().
Feel free to ignore as this is just a minor nitpick;

>   util: add compat wrapper for g_fsync
>   src: use g_fsync for portability
>   util: introduce virFileDataSync

What would be the impact of using g_fsync() on Linuxes as well?

>   src: use g_lstat() instead of lstat()
>   src: switch from fnmatch to g_pattern_match_simple

This one worries me a little bit about possible breakages, mainly
related to wildcards used in libvirtd.conf.

>   src: replace clock_gettime()/gettimeofday() with g_get_real_time()
>   src: replace last_component() with g_path_get_basename()

Please, take a look at this patch's reply.

>   util: replace IS_ABSOLUTE_FILE_NAME with g_path_is_absolute
>   src: replace mdir_name() with g_path_get_dirname()
>   src: remove unused imports of dirname.h
>   src: replace getcwd() with g_get_current_dir()
>   util: use realpath/g_canonicalize_filename
>   util: replace gethostname() with g_get_hostname()

Please, take a look at this patch's reply.

>   src: replace WSAStartup with g_networking_init()
>   src: replace strptime()/timegm()/mktime() with GDateTime APIs set
>   bootstrap: remove now unused gnulib modules
>   bootstrap: annotate with info about desired replacement
>

All the comments are more suggestions than blockers. The questions are
not blockers in any way.
Knowing that, for the series:

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

[snip]


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/23] Take 64 gnulib modules, eliminate 14, to give 50 remaining
Posted by Daniel P. Berrangé 4 years, 3 months ago
On Thu, Jan 02, 2020 at 05:40:22PM +0100, Fabiano Fidêncio wrote:
> Daniel,
> 
> On Thu, Jan 2, 2020 at 3:59 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > In the last days before the xmas break I took some time to
> > eliminate 14 more gnulib modules, bringing us down to just
> > 50 left to go. They're getting harder to eliminate as we go
> > on, but to give some hints, I've annotated every module in
> > bootstrap.conf with a suggested replacement strategy.
> >
> > Daniel P. Berrangé (23):
> >   build: set min version for CLang to 3.4 / XCode CLang to 5.1
> >   docs: expand macOS platform support coverage
> >   travis: add macOS Xcode 11.3 testing
> >   util: add note about event file descriptors on Windows
> >   src: always pull in glib/gstdio.h header
> >   src: switch to use g_setenv/g_unsetenv
> 
> In the patch above I'd also take the opportunity and use TRUE/FALSE
> instead of 1/0 in g_setenv().
> Feel free to ignore as this is just a minor nitpick;

Yes, it should use TRUE/FALSE to match the API parameter type.

> >   util: add compat wrapper for g_fsync
> >   src: use g_fsync for portability
> >   util: introduce virFileDataSync
> 
> What would be the impact of using g_fsync() on Linuxes as well?

fsync is stronger & so worse for performance

[quote fdatasync(2)]
       fdatasync()  is similar to fsync(), but does not flush modified metadata
       unless that metadata is needed in  order  to  allow  a  subsequent  data
       retrieval  to be correctly handled. 
       ...snip...
       The aim of fdatasync() is to reduce disk activity for applications  that
       do not require all metadata to be synchronized with the disk.
[/quote]

> 
> >   src: use g_lstat() instead of lstat()
> >   src: switch from fnmatch to g_pattern_match_simple
> 
> This one worries me a little bit about possible breakages, mainly
> related to wildcards used in libvirtd.conf.

Yes, there is a risk here, however, the config files are not part of the
API/ABI guarantee and the risk is reasonably low. It'll need a release
note of course.


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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list