[PATCH 00/20] Finish the work to stop hardcoding paths at build time

Daniel P. Berrangé via Devel posted 20 patches 4 months, 1 week ago
Failed in applying to current master (apply log)
build-aux/Makefile.in                 |   1 +
build-aux/meson.build                 |   1 +
docs/meson.build                      |   1 -
meson.build                           |  92 +++------------------
src/bhyve/bhyve_command.c             |   6 +-
src/locking/lock_driver_lockd.c       |   4 +-
src/node_device/node_device_driver.c  |  14 ++--
src/openvz/openvz_conf.h              |   6 +-
src/storage/storage_backend_fs.c      |  17 +---
src/storage/storage_backend_logical.c |  24 +++---
src/util/virfirewall.h                |   1 +
src/util/viriscsi.c                   |  55 +++++++------
src/util/virkmod.c                    |   4 +-
src/util/virnetdevbandwidth.c         |  36 ++++-----
src/util/virnetdevip.c                |  14 ++--
src/util/virnetdevmidonet.c           |   4 +-
src/util/virnetdevopenvswitch.c       |   4 +-
src/util/virnuma.c                    |   2 +-
src/util/virpolkit.c                  |   8 +-
src/util/virpolkit.h                  |   2 -
src/util/virsysinfo.c                 |   4 +-
tests/storagepoolxml2argvtest.c       |  12 +--
tests/viriscsitest.c                  |  16 ++--
tests/virkmodtest.c                   |   4 +-
tests/virnetdevbandwidthtest.c        | 110 +++++++++++++-------------
tests/virnetdevopenvswitchtest.c      |  74 ++++++++---------
26 files changed, 216 insertions(+), 300 deletions(-)
[PATCH 00/20] Finish the work to stop hardcoding paths at build time
Posted by Daniel P. Berrangé via Devel 4 months, 1 week ago
Over the years we've made various changes to stop hardcoding paths at
build time, but the work is incomplete.

This bit us significantly in Fedora 42 where they have merged the 'sbin'
and 'bin' dirs together. In the build environment we have a clean
install where '/sbin' and '/usr/sbin' are simply symlinks to '/usr/bin'.

Our meson.build logic preferentially looks in '/sbin' and '/usr/sbin'
to find programs, and thus it finds *all* binaries in '/sbin' and then
hardcodes this path. This happens even for binaries that have always
been in '/usr/bin' and NEVER '/sbin' or '/usr/sbin'

On fresh Fedora installs this is fine as the symlinks match the build
environment.

On upgraded Fedora installs, however, '/sbin' and '/usr/sbin' remain
separate directories and inside they symlink individual binaries,
if-and-only-if the binary was in '/sbin' in Fedora 41.

Thus many of the binaries we found in /sbin don't actually exist and
libvirt thus breaks.

There is really no reason why we should be harcoding any paths at
build time. virCommand happily uses virFindFileInPath for any paths
which are not already absolute. $PATH includes '/sbin' and '/usr/sbin'
when we're running privileged, and when unprivileged we shouldn't need
to run binaries from there.

Daniel P. Berrangé (20):
  storage: stop hardcoding paths for mkfs, mount, umount
  tests: storage hardcoding paths for mount & vgchange
  util: stop hardcoding numad path
  util: stop hardcoding bhyve, bhyvectl, bhyveload paths
  util: stop hardcoding 'ifconfig' path
  docs: stop setting vars for docs tools
  build-aux: add missing definition of PERL variable
  meson: stop setting conf var for required programs
  util: remove use hardcoded DMIDECODE path
  meson: remove check for 'ip' program
  util: remove hardcoded ISCSIADM command path
  nodedev: remove use hardcoded MDEVCTL path
  util: remove use hardcoded MM_CTL path
  util: remove use hardcoded MODPROBE/RMMOD paths
  util: remove use hardcoded OVS_VSCTL path
  util: remove use hardcoded TC path
  meson: stop setting conf var for optional programs
  storage: stop hardcoding LVM tool paths
  util: stop hardcoding pkttyagent path
  openvz: stop hardcoding vzlist/vzctl/vzmigrate paths

 build-aux/Makefile.in                 |   1 +
 build-aux/meson.build                 |   1 +
 docs/meson.build                      |   1 -
 meson.build                           |  92 +++------------------
 src/bhyve/bhyve_command.c             |   6 +-
 src/locking/lock_driver_lockd.c       |   4 +-
 src/node_device/node_device_driver.c  |  14 ++--
 src/openvz/openvz_conf.h              |   6 +-
 src/storage/storage_backend_fs.c      |  17 +---
 src/storage/storage_backend_logical.c |  24 +++---
 src/util/virfirewall.h                |   1 +
 src/util/viriscsi.c                   |  55 +++++++------
 src/util/virkmod.c                    |   4 +-
 src/util/virnetdevbandwidth.c         |  36 ++++-----
 src/util/virnetdevip.c                |  14 ++--
 src/util/virnetdevmidonet.c           |   4 +-
 src/util/virnetdevopenvswitch.c       |   4 +-
 src/util/virnuma.c                    |   2 +-
 src/util/virpolkit.c                  |   8 +-
 src/util/virpolkit.h                  |   2 -
 src/util/virsysinfo.c                 |   4 +-
 tests/storagepoolxml2argvtest.c       |  12 +--
 tests/viriscsitest.c                  |  16 ++--
 tests/virkmodtest.c                   |   4 +-
 tests/virnetdevbandwidthtest.c        | 110 +++++++++++++-------------
 tests/virnetdevopenvswitchtest.c      |  74 ++++++++---------
 26 files changed, 216 insertions(+), 300 deletions(-)

-- 
2.49.0
Re: [PATCH 00/20] Finish the work to stop hardcoding paths at build time
Posted by Ján Tomko via Devel 4 months, 1 week ago
On a Tuesday in 2025, Daniel P. Berrangé via Devel wrote:
>Over the years we've made various changes to stop hardcoding paths at
>build time, but the work is incomplete.
>
>This bit us significantly in Fedora 42 where they have merged the 'sbin'
>and 'bin' dirs together. In the build environment we have a clean
>install where '/sbin' and '/usr/sbin' are simply symlinks to '/usr/bin'.
>
>Our meson.build logic preferentially looks in '/sbin' and '/usr/sbin'
>to find programs, and thus it finds *all* binaries in '/sbin' and then
>hardcodes this path. This happens even for binaries that have always
>been in '/usr/bin' and NEVER '/sbin' or '/usr/sbin'
>
>On fresh Fedora installs this is fine as the symlinks match the build
>environment.
>
>On upgraded Fedora installs, however, '/sbin' and '/usr/sbin' remain
>separate directories and inside they symlink individual binaries,
>if-and-only-if the binary was in '/sbin' in Fedora 41.
>
>Thus many of the binaries we found in /sbin don't actually exist and
>libvirt thus breaks.
>
>There is really no reason why we should be harcoding any paths at
>build time. virCommand happily uses virFindFileInPath for any paths
>which are not already absolute. $PATH includes '/sbin' and '/usr/sbin'
>when we're running privileged, and when unprivileged we shouldn't need
>to run binaries from there.
>

Does anyone happen to remember the original reason?

The only thing I can think of is saving the lookup time, but that has to
be negligible compared to the cost of fork and the command actually
doing something

Jano
Re: [PATCH 00/20] Finish the work to stop hardcoding paths at build time
Posted by Daniel P. Berrangé via Devel 4 months, 1 week ago
On Tue, Apr 29, 2025 at 02:57:13PM +0200, Ján Tomko wrote:
> On a Tuesday in 2025, Daniel P. Berrangé via Devel wrote:
> > Over the years we've made various changes to stop hardcoding paths at
> > build time, but the work is incomplete.
> > 
> > This bit us significantly in Fedora 42 where they have merged the 'sbin'
> > and 'bin' dirs together. In the build environment we have a clean
> > install where '/sbin' and '/usr/sbin' are simply symlinks to '/usr/bin'.
> > 
> > Our meson.build logic preferentially looks in '/sbin' and '/usr/sbin'
> > to find programs, and thus it finds *all* binaries in '/sbin' and then
> > hardcodes this path. This happens even for binaries that have always
> > been in '/usr/bin' and NEVER '/sbin' or '/usr/sbin'
> > 
> > On fresh Fedora installs this is fine as the symlinks match the build
> > environment.
> > 
> > On upgraded Fedora installs, however, '/sbin' and '/usr/sbin' remain
> > separate directories and inside they symlink individual binaries,
> > if-and-only-if the binary was in '/sbin' in Fedora 41.
> > 
> > Thus many of the binaries we found in /sbin don't actually exist and
> > libvirt thus breaks.
> > 
> > There is really no reason why we should be harcoding any paths at
> > build time. virCommand happily uses virFindFileInPath for any paths
> > which are not already absolute. $PATH includes '/sbin' and '/usr/sbin'
> > when we're running privileged, and when unprivileged we shouldn't need
> > to run binaries from there.
> > 
> 
> Does anyone happen to remember the original reason?
> 
> The only thing I can think of is saving the lookup time, but that has to
> be negligible compared to the cost of fork and the command actually
> doing something

I've not checked, but I expect that a very very very long time ago virCommand
would not lookup binaries in $PATH, and then we just cargo-culted the
configure.ac checks, later meson.build checks, without thinking about it
more.

Yes, we've got the cost of a few stat() calls, but as you say, it isn't
worth worrying about that in the context of a heavy fork+exec.


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