3 a.m.
Recent attempt to add a lot of meson options to specify different
runtime paths motivated me enough to cleanup this from meson.
Changes in v3:
- some patches were already pushed
- removed patch that moved virFindFileInPath from testutilsqemu.c
Changes in v2:
- split and rework patch 16/17 to address review comments
- added a new patch to cleanup libvirt.spec.in file
Pavel Hrdina (8):
virfile: introduce virFindFileInPathFull()
qemu_conf: use virFindFileInPathFull for runtime binaries
meson: drop check for runtime binary dependencies
meson: move iscsiadm check into storage_iscsi condition
meson: stop setting runtime binaries defines during compilation
meson: use runtime binaries to only resolve features with "auto" value
meson: optional_programs should be used only for building libvirt
libvirt.spec: drop no longer required build dependencies
libvirt.spec.in | 31 ----
meson.build | 214 +++++++++----------------
src/bhyve/bhyve_command.c | 4 +
src/libvirt_private.syms | 2 +-
src/locking/lock_driver_lockd.c | 12 +-
src/network/bridge_driver.c | 2 +
src/node_device/node_device_driver.c | 2 +
src/qemu/qemu_conf.c | 23 ++-
src/storage/storage_backend_logical.c | 13 ++
src/storage/storage_backend_sheepdog.c | 2 +
src/storage/storage_backend_zfs.c | 3 +
src/storage/storage_util.c | 2 +
src/storage/storage_util.h | 6 +
src/util/virdnsmasq.c | 1 +
src/util/virfile.c | 16 +-
src/util/virfile.h | 5 +-
src/util/virfirewall.h | 4 +
src/util/viriscsi.h | 2 +
src/util/virkmod.h | 3 +
src/util/virnetdevbandwidth.h | 2 +
src/util/virnetdevip.c | 2 +
src/util/virnetdevmidonet.c | 2 +
src/util/virnetdevopenvswitch.c | 2 +
src/util/virnuma.c | 1 +
src/util/virsysinfo.c | 1 +
src/util/virutil.c | 2 +
tests/testutilsqemu.c | 3 +-
tests/virfirewallmock.c | 3 +-
28 files changed, 173 insertions(+), 192 deletions(-)
--
2.31.1
3 a.m.
New subject: [PATCH RESEND v3 1/8] virfile: introduce virFindFileInPathFull()
Extend virFindFileInPath to search in custom extra paths as well. Some
binaries that libvirt needs are not usually in $PATH so we need to have
a way to look for these as well.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/libvirt_private.syms | 2 +-
src/util/virfile.c | 16 ++++++++++++++--
src/util/virfile.h | 5 ++++-
tests/testutilsqemu.c | 3 ++-
tests/virfirewallmock.c | 3 ++-
5 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e1aef5267e..2ab43d5ffb 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2259,7 +2259,7 @@ virFileWrapperFdClose;
virFileWrapperFdFree;
virFileWrapperFdNew;
virFileWriteStr;
-virFindFileInPath;
+virFindFileInPathFull;
# util/virfilecache.h
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 03a7725dd3..7922fda2e5 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1654,13 +1654,15 @@ virFileIsLink(const char *linkpath)
}
/*
- * Finds a requested executable file in the PATH env. e.g.:
+ * Finds a requested executable file in the paths provided by @extra_paths
+ * argument or in PATH env. e.g.:
* "qemu-img" will return "/usr/bin/qemu-img"
*
* You must free the result
*/
char *
-virFindFileInPath(const char *file)
+virFindFileInPathFull(const char *file,
+ const GStrv extra_paths)
{
const char *origpath = NULL;
g_auto(GStrv) paths = NULL;
@@ -1692,6 +1694,16 @@ virFindFileInPath(const char *file)
return abspath;
}
+ /* First search in paths provided by caller.
+ */
+ if (extra_paths) {
+ for (pathiter = extra_paths; *pathiter; pathiter++) {
+ g_autofree char *fullpath = g_strdup_printf("%s/%s", *pathiter,
file);
+ if (virFileIsExecutable(fullpath))
+ return g_steal_pointer(&fullpath);
+ }
+ }
+
/* copy PATH env so we can tweak it */
origpath = getenv("PATH");
if (!origpath)
diff --git a/src/util/virfile.h b/src/util/virfile.h
index b9f7b1766f..eee27c2efc 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -184,9 +184,12 @@ int virFileResolveAllLinks(const char *linkpath,
int virFileIsLink(const char *linkpath)
ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT;
-char *virFindFileInPath(const char *file)
+char *virFindFileInPathFull(const char *file,
+ const GStrv extra_paths)
G_GNUC_NO_INLINE;
+#define virFindFileInPath(file) virFindFileInPathFull(file, NULL)
+
char *virFileFindResource(const char *filename,
const char *builddir,
const char *installdir)
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 1444abc401..1ceceef4a7 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -118,7 +118,8 @@ static const char *qemu_default_ram_id[VIR_ARCH_LAST] = {
};
char *
-virFindFileInPath(const char *file)
+virFindFileInPathFull(const char *file,
+ const GStrv extra_args G_GNUC_UNUSED)
{
if (g_str_has_prefix(file, "qemu-system") ||
g_str_equal(file, "qemu-kvm")) {
diff --git a/tests/virfirewallmock.c b/tests/virfirewallmock.c
index 6b096701c9..a047e56bd9 100644
--- a/tests/virfirewallmock.c
+++ b/tests/virfirewallmock.c
@@ -20,7 +20,8 @@
#include "virfile.h"
char *
-virFindFileInPath(const char *file)
+virFindFileInPathFull(const char *file,
+ const GStrv extra_args G_GNUC_UNUSED)
{
if (file &&
(g_strrstr(file, "ebtables") ||
--
2.31.1
4:03 a.m.
New subject: [PATCH RESEND v3 1/8] virfile: introduce virFindFileInPathFull()
On Tue, May 25, 2021 at 10:00:40AM +0200, Pavel Hrdina wrote:
Extend virFindFileInPath to search in custom extra paths as well.
Some
binaries that libvirt needs are not usually in $PATH so we need to have
a way to look for these as well.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/libvirt_private.syms | 2 +-
src/util/virfile.c | 16 ++++++++++++++--
src/util/virfile.h | 5 ++++-
tests/testutilsqemu.c | 3 ++-
tests/virfirewallmock.c | 3 ++-
5 files changed, 23 insertions(+), 6 deletions(-)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
3 a.m.
New subject: [PATCH RESEND v3 2/8] qemu_conf: use virFindFileInPathFull for runtime binaries
Following patches will stop detecting the full path during compilation
so we will need to do it here.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_conf.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 916a3d36ee..437b3ce2be 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -109,6 +109,12 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
const char *root)
{
g_autoptr(virQEMUDriverConfig) cfg = NULL;
+ const char *qemu_extra_paths[] = {
+ "/usr/libexec",
+ "/usr/lib/qemu",
+ "/usr/lib",
+ NULL,
+ };
if (virQEMUConfigInitialize() < 0)
return NULL;
@@ -267,10 +273,14 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
return NULL;
}
- cfg->bridgeHelperName = g_strdup(QEMU_BRIDGE_HELPER);
- cfg->prHelperName = g_strdup(QEMU_PR_HELPER);
- cfg->slirpHelperName = g_strdup(QEMU_SLIRP_HELPER);
- cfg->dbusDaemonName = g_strdup(QEMU_DBUS_DAEMON);
+ cfg->bridgeHelperName = virFindFileInPathFull(QEMU_BRIDGE_HELPER,
+ (GStrv) qemu_extra_paths);
+ cfg->prHelperName = virFindFileInPathFull(QEMU_PR_HELPER,
+ (GStrv) qemu_extra_paths);
+ cfg->slirpHelperName = virFindFileInPathFull(QEMU_SLIRP_HELPER,
+ (GStrv) qemu_extra_paths);
+ cfg->dbusDaemonName = virFindFileInPathFull(QEMU_DBUS_DAEMON,
+ (GStrv) qemu_extra_paths);
cfg->securityDefaultConfined = true;
cfg->securityRequireConfined = false;
--
2.31.1
4:05 a.m.
New subject: [PATCH RESEND v3 2/8] qemu_conf: use virFindFileInPathFull for runtime binaries
On Tue, May 25, 2021 at 10:00:41AM +0200, Pavel Hrdina wrote:
Following patches will stop detecting the full path during
compilation
so we will need to do it here.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_conf.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
3 a.m.
New subject: [PATCH RESEND v3 3/8] meson: drop check for runtime binary dependencies
These binaries are used only during runtime so technically there is no
need to check for them while compiling libvirt.
Usually the location is the same while compiling and running but it may
not be true. In addition they are not strictly required to compile the
code so this way developers don't have to install it or create fake
binaries in order to compile the code.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
meson.build | 63 ----------------------------
src/network/bridge_driver.c | 2 +
src/node_device/node_device_driver.c | 2 +
src/qemu/qemu_conf.c | 5 +++
src/storage/storage_util.c | 2 +
src/util/virdnsmasq.c | 1 +
src/util/virfirewall.h | 4 ++
src/util/virkmod.h | 3 ++
src/util/virnetdevbandwidth.h | 2 +
src/util/virnetdevip.c | 2 +
src/util/virnetdevmidonet.c | 2 +
src/util/virnetdevopenvswitch.c | 2 +
src/util/virsysinfo.c | 1 +
src/util/virutil.c | 2 +
14 files changed, 30 insertions(+), 63 deletions(-)
diff --git a/meson.build b/meson.build
index 4f23f9104e..ed4afd2052 100644
--- a/meson.build
+++ b/meson.build
@@ -823,24 +823,9 @@ endforeach
optional_programs = [
'augparse',
- 'dmidecode',
- 'dnsmasq',
- 'ebtables',
'flake8',
- 'ip',
- 'ip6tables',
- 'iptables',
'iscsiadm',
- 'mdevctl',
- 'mm-ctl',
- 'modprobe',
- 'ovs-vsctl',
'pdwtags',
- 'radvd',
- 'rmmod',
- 'scrub',
- 'tc',
- 'udevadm',
]
foreach name : optional_programs
@@ -1646,54 +1631,6 @@ if not get_option('driver_qemu').disabled()
endif
conf.set_quoted('QEMU_USER', qemu_user)
conf.set_quoted('QEMU_GROUP', qemu_group)
-
- qemu_bridge_prog = find_program(
- 'qemu-bridge-helper',
- dirs: [ '/usr/libexec', '/usr/lib/qemu', '/usr/lib' ],
- required: false
- )
- if qemu_bridge_prog.found()
- qemu_bridge_path = qemu_bridge_prog.path()
- else
- qemu_bridge_path = '/usr/libexec/qemu-bridge-helper'
- endif
- conf.set_quoted('QEMU_BRIDGE_HELPER', qemu_bridge_path)
-
- qemu_pr_prog = find_program(
- 'qemu-pr-helper',
- dirs: [ '/usr/bin', '/usr/libexec' ],
- required: false
- )
- if qemu_pr_prog.found()
- qemu_pr_path = qemu_pr_prog.path()
- else
- qemu_pr_path = '/usr/bin/qemu-pr-helper'
- endif
- conf.set_quoted('QEMU_PR_HELPER', qemu_pr_path)
-
- qemu_slirp_prog = find_program(
- 'slirp-helper',
- dirs: [ '/usr/bin', '/usr/libexec' ],
- required: false
- )
- if qemu_slirp_prog.found()
- qemu_slirp_path = qemu_slirp_prog.path()
- else
- qemu_slirp_path = '/usr/bin/slirp-helper'
- endif
- conf.set_quoted('QEMU_SLIRP_HELPER', qemu_slirp_path)
-
- qemu_dbus_daemon_prog = find_program(
- 'dbus-daemon',
- dirs: [ '/usr/bin', '/usr/libexec' ],
- required: false
- )
- if qemu_dbus_daemon_prog.found()
- qemu_dbus_daemon_path = qemu_dbus_daemon_prog.path()
- else
- qemu_dbus_daemon_path = '/usr/bin/dbus-daemon'
- endif
- conf.set_quoted('QEMU_DBUS_DAEMON', qemu_dbus_daemon_path)
endif
endif
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index a711b34c48..81d0c06ec5 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -74,6 +74,8 @@
#define VIR_FROM_THIS VIR_FROM_NETWORK
#define MAX_BRIDGE_ID 256
+#define RADVD "radvd"
+
static virMutex bridgeNameValidateMutex = VIR_MUTEX_INITIALIZER;
/**
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
index a9f605715b..72edbc3fa0 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -48,6 +48,8 @@
VIR_LOG_INIT("node_device.node_device_driver");
+#define MDEVCTL "mdevctl"
+
virNodeDeviceDriverState *driver;
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 437b3ce2be..d228504d8c 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -55,6 +55,11 @@
VIR_LOG_INIT("qemu.qemu_conf");
+#define QEMU_BRIDGE_HELPER "qemu-bridge-helper"
+#define QEMU_PR_HELPER "qemu-pr-helper"
+#define QEMU_SLIRP_HELPER "slirp-helper"
+#define QEMU_DBUS_DAEMON "dbus-daemon"
+
/* These are only defaults, they can be changed now in qemu.conf and
* explicitly specified port is checked against these two (makes
* sense to limit the values).
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index a7c9355bf9..b43ea7b757 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -85,6 +85,8 @@ VIR_LOG_INIT("storage.storage_util");
# define S_IRWXUGO (S_IRWXU | S_IRWXG | S_IRWXO)
#endif
+#define SCRUB "scrub"
+
/* virStorageBackendNamespaceInit:
* @poolType: virStoragePoolType
* @xmlns: Storage Pool specific namespace callback methods
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index f2f606913f..b666c2f9d3 100644
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -48,6 +48,7 @@ VIR_LOG_INIT("util.dnsmasq");
#define DNSMASQ_HOSTSFILE_SUFFIX "hostsfile"
#define DNSMASQ_ADDNHOSTSFILE_SUFFIX "addnhosts"
+#define DNSMASQ "dnsmasq"
static void
dhcphostFreeContent(dnsmasqDhcpHost *host)
diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h
index 169d99fe2b..a3954c5ed0 100644
--- a/src/util/virfirewall.h
+++ b/src/util/virfirewall.h
@@ -22,6 +22,10 @@
#include "internal.h"
+#define EBTABLES_PATH "ebtables"
+#define IPTABLES_PATH "iptables"
+#define IP6TABLES_PATH "ip6tables"
+
typedef struct _virFirewall virFirewall;
typedef struct _virFirewallRule virFirewallRule;
diff --git a/src/util/virkmod.h b/src/util/virkmod.h
index bb043d4876..58311909eb 100644
--- a/src/util/virkmod.h
+++ b/src/util/virkmod.h
@@ -23,6 +23,9 @@
#include "internal.h"
+#define MODPROBE "modprobe"
+#define RMMOD "rmmod"
+
char *virKModLoad(const char *)
ATTRIBUTE_NONNULL(1);
char *virKModUnload(const char *)
diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h
index 3d520721f6..6c91bd0f33 100644
--- a/src/util/virnetdevbandwidth.h
+++ b/src/util/virnetdevbandwidth.h
@@ -21,6 +21,8 @@
#include "internal.h"
#include "virmacaddr.h"
+#define TC "tc"
+
typedef struct _virNetDevBandwidthRate virNetDevBandwidthRate;
struct _virNetDevBandwidthRate {
unsigned long long average; /* kbytes/s */
diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
index 0ce8f5b536..f4cf6ac39f 100644
--- a/src/util/virnetdevip.c
+++ b/src/util/virnetdevip.c
@@ -50,6 +50,8 @@
#define VIR_FROM_THIS VIR_FROM_NONE
+#define IP_PATH "ip"
+
VIR_LOG_INIT("util.netdevip");
#if defined(WITH_LIBNL)
diff --git a/src/util/virnetdevmidonet.c b/src/util/virnetdevmidonet.c
index 9061f1516f..735607e5df 100644
--- a/src/util/virnetdevmidonet.c
+++ b/src/util/virnetdevmidonet.c
@@ -26,6 +26,8 @@
#define VIR_FROM_THIS VIR_FROM_NONE
+#define MM_CTL "mm-ctl"
+
/**
* virNetDevMidonetBindPort:
* @ifname: the network interface name
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index 21ee4bdd42..a05128dbca 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -35,6 +35,8 @@
VIR_LOG_INIT("util.netdevopenvswitch");
+#define OVS_VSCTL "ovs-vsctl"
+
/*
* Set openvswitch default timeout
*/
diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c
index af9e03c5ac..8bbda9ea69 100644
--- a/src/util/virsysinfo.c
+++ b/src/util/virsysinfo.c
@@ -53,6 +53,7 @@ static const char *sysinfoCpuinfo = "/proc/cpuinfo";
#define SYSINFO sysinfoSysinfo
#define CPUINFO sysinfoCpuinfo
#define CPUINFO_FILE_LEN (1024*1024) /* 1MB limit for /proc/cpuinfo file */
+#define DMIDECODE "dmidecode"
void
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 311cbbf93a..757fb1439d 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -57,6 +57,8 @@
#include "virutil.h"
#include "virsocket.h"
+#define UDEVADM "udevadm"
+
G_STATIC_ASSERT(sizeof(gid_t) <= sizeof(unsigned int) &&
sizeof(uid_t) <= sizeof(unsigned int));
--
2.31.1
4:20 a.m.
New subject: [PATCH RESEND v3 3/8] meson: drop check for runtime binary dependencies
On Tue, May 25, 2021 at 10:00:42AM +0200, Pavel Hrdina wrote:
+#define EBTABLES_PATH "ebtables"
+#define IPTABLES_PATH "iptables"
+#define IP6TABLES_PATH "ip6tables"
These are no longer paths, so you should rename them accordingly in a
preparatory patch.
+#define IP_PATH "ip"
Same here.
--
Andrea Bolognani / Red Hat / Virtualization
3 a.m.
New subject: [PATCH RESEND v3 4/8] meson: move iscsiadm check into storage_iscsi condition
This requires to define the binary name in code because we compile
src/util/viriscsi.c unconditionally.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
meson.build | 22 ++++++++++++++++------
src/util/viriscsi.h | 2 ++
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/meson.build b/meson.build
index ed4afd2052..7eeaa689f9 100644
--- a/meson.build
+++ b/meson.build
@@ -824,7 +824,6 @@ endforeach
optional_programs = [
'augparse',
'flake8',
- 'iscsiadm',
'pdwtags',
]
@@ -1742,11 +1741,22 @@ if conf.has('WITH_LIBVIRTD')
error('Need glusterfs (libgfapi) for gluster storage driver')
endif
- if not get_option('storage_iscsi').disabled() and iscsiadm_prog.found()
- use_storage = true
- conf.set('WITH_STORAGE_ISCSI', 1)
- elif get_option('storage_iscsi').enabled()
- error('We need iscsiadm for iSCSI storage driver')
+ if not get_option('storage_iscsi').disabled()
+ iscsi_enable = true
+ iscsiadm_prog = find_program('iscsiadm', required: false, dirs:
libvirt_sbin_path)
+
+ if not iscsiadm_prog.found()
+ if get_option('storage_iscsi').enabled()
+ error('We need iscsiadm for iSCSI storage driver')
+ else
+ iscsi_enable = false
+ endif
+ endif
+
+ if iscsi_enable
+ use_storage = true
+ conf.set('WITH_STORAGE_ISCSI', 1)
+ endif
endif
if not get_option('storage_iscsi_direct').disabled() and libiscsi_dep.found()
diff --git a/src/util/viriscsi.h b/src/util/viriscsi.h
index 6d17d139eb..fa7ff280c2 100644
--- a/src/util/viriscsi.h
+++ b/src/util/viriscsi.h
@@ -23,6 +23,8 @@
#include "internal.h"
+#define ISCSIADM "iscsiadm"
+
char *
virISCSIGetSession(const char *devpath,
bool probe)
--
2.31.1
4:36 a.m.
New subject: [PATCH RESEND v3 4/8] meson: move iscsiadm check into storage_iscsi condition
On Tue, May 25, 2021 at 10:00:43AM +0200, Pavel Hrdina wrote:
+ if not get_option('storage_iscsi').disabled()
+ iscsi_enable = true
+ iscsiadm_prog = find_program('iscsiadm', required: false, dirs:
libvirt_sbin_path)
+
+ if not iscsiadm_prog.found()
+ if get_option('storage_iscsi').enabled()
+ error('We need iscsiadm for iSCSI storage driver')
+ else
+ iscsi_enable = false
+ endif
+ endif
I believe this could be rewritten as
if not get_option('storage_iscsi').disabled()
iscsi_enable = true
iscsiadm_prog = find_program('iscsiadm', required:
get_option('storage_iscsi'), dirs: libvirt_sbin_path)
if not iscsiadm_prog.found()
iscsi_enable = false
endif
I see this pattern used elsewhere in the file and it's more concise
while achieving the same result AFAICT.
--
Andrea Bolognani / Red Hat / Virtualization
4:53 a.m.
New subject: [PATCH RESEND v3 4/8] meson: move iscsiadm check into storage_iscsi condition
On Tue, May 25, 2021 at 02:36:04AM -0700, Andrea Bolognani wrote:
On Tue, May 25, 2021 at 10:00:43AM +0200, Pavel Hrdina wrote:
> + if not get_option('storage_iscsi').disabled()
> + iscsi_enable = true
> + iscsiadm_prog = find_program('iscsiadm', required: false, dirs:
libvirt_sbin_path)
> +
> + if not iscsiadm_prog.found()
> + if get_option('storage_iscsi').enabled()
> + error('We need iscsiadm for iSCSI storage driver')
> + else
> + iscsi_enable = false
> + endif
> + endif
I believe this could be rewritten as
if not get_option('storage_iscsi').disabled()
iscsi_enable = true
iscsiadm_prog = find_program('iscsiadm', required:
get_option('storage_iscsi'), dirs: libvirt_sbin_path)
if not iscsiadm_prog.found()
iscsi_enable = false
endif
I see this pattern used elsewhere in the file and it's more concise
while achieving the same result AFAICT.
I just realized that you rewrite this check again in 6/8, so no
matter what it looks like in this commit
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
3 a.m.
New subject: [PATCH RESEND v3 5/8] meson: stop setting runtime binaries defines during compilation
Technically the location of these binaries may be different when
compiling libvirt or running it. This will allow users to change $PATH
to use different binaries as well.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
meson.build | 41 ++++----------------------
src/bhyve/bhyve_command.c | 4 +++
src/locking/lock_driver_lockd.c | 12 ++------
src/storage/storage_backend_logical.c | 13 ++++++++
src/storage/storage_backend_sheepdog.c | 2 ++
src/storage/storage_backend_zfs.c | 3 ++
src/storage/storage_util.h | 6 ++++
src/util/virnuma.c | 1 +
8 files changed, 36 insertions(+), 46 deletions(-)
diff --git a/meson.build b/meson.build
index 7eeaa689f9..2900b6e1a8 100644
--- a/meson.build
+++ b/meson.build
@@ -979,9 +979,7 @@ libparted_version = '1.8.0'
libparted_dep = dependency('libparted', version: '>=' +
libparted_version, required: false)
if libparted_dep.found()
parted_prog = find_program('parted', required: false, dirs: libvirt_sbin_path)
- if parted_prog.found()
- conf.set_quoted('PARTED', parted_prog.path())
- else
+ if not parted_prog.found()
libparted_dep = dependency('', required: false)
endif
endif
@@ -1428,9 +1426,6 @@ if not get_option('driver_bhyve').disabled() and
host_machine.system() == 'freeb
if bhyve_prog.found() and bhyvectl_prog.found() and bhyveload_prog.found()
conf.set('WITH_BHYVE', 1)
- conf.set_quoted('BHYVE', bhyve_prog.path())
- conf.set_quoted('BHYVECTL', bhyvectl_prog.path())
- conf.set_quoted('BHYVELOAD', bhyveload_prog.path())
endif
elif get_option('driver_bhyve').enabled()
error('The bhyve driver cannot be enabled')
@@ -1719,18 +1714,7 @@ if conf.has('WITH_LIBVIRTD')
if fs_enable
use_storage = true
-
conf.set('WITH_STORAGE_FS', 1)
- conf.set_quoted('MOUNT', mount_prog.path())
- conf.set_quoted('UMOUNT', umount_prog.path())
- conf.set_quoted('MKFS', mkfs_prog.path())
-
- showmount_prog = find_program('showmount', required: false, dirs:
libvirt_sbin_path)
- showmount_path = ''
- if showmount_prog.found()
- showmount_path = showmount_prog.path()
- endif
- conf.set_quoted('SHOWMOUNT', showmount_path)
endif
endif
@@ -1775,11 +1759,8 @@ if conf.has('WITH_LIBVIRTD')
'pvs', 'vgs', 'lvs',
]
foreach name : lvm_progs
- set_variable(
- '@0(a)_prog'.format(name),
- find_program(name, required: get_option('storage_lvm'), dirs:
libvirt_sbin_path)
- )
- if not get_variable('@0(a)_prog'.format(name)).found()
+ lvm_prog = find_program(name, required: get_option('storage_lvm'), dirs:
libvirt_sbin_path)
+ if not lvm_prog.found()
lvm_enable = false
endif
endforeach
@@ -1787,10 +1768,6 @@ if conf.has('WITH_LIBVIRTD')
if lvm_enable
use_storage = true
conf.set('WITH_STORAGE_LVM', 1)
-
- foreach name : lvm_progs
- conf.set_quoted(name.to_upper(),
get_variable('@0(a)_prog'.format(name)).path())
- endforeach
endif
endif
@@ -1819,7 +1796,6 @@ if conf.has('WITH_LIBVIRTD')
if sheepdogcli_prog.found()
use_storage = true
conf.set('WITH_STORAGE_SHEEPDOG', 1)
- conf.set_quoted('SHEEPDOGCLI', sheepdogcli_prog.path())
endif
endif
@@ -1842,11 +1818,8 @@ if conf.has('WITH_LIBVIRTD')
if not get_option('storage_zfs').disabled()
zfs_enable = true
foreach name : ['zfs', 'zpool']
- set_variable(
- '@0(a)_prog'.format(name),
- find_program(name, required: get_option('storage_zfs'), dirs:
libvirt_sbin_path)
- )
- if not get_variable('@0(a)_prog'.format(name)).found()
+ zfs_prog = find_program(name, required: get_option('storage_zfs'), dirs:
libvirt_sbin_path)
+ if not zfs_prog.found()
zfs_enable = false
endif
endforeach
@@ -1854,9 +1827,6 @@ if conf.has('WITH_LIBVIRTD')
if zfs_enable
use_storage = true
conf.set('WITH_STORAGE_ZFS', 1)
- foreach name : ['zfs', 'zpool']
- conf.set_quoted(name.to_upper(),
get_variable('@0(a)_prog'.format(name)).path())
- endforeach
endif
endif
endif
@@ -1970,7 +1940,6 @@ if not get_option('numad').disabled() and
numactl_dep.found()
numad_prog = find_program('numad', required: get_option('numad'), dirs:
libvirt_sbin_path)
if numad_prog.found()
conf.set('WITH_NUMAD', 1)
- conf.set_quoted('NUMAD', numad_prog.path())
endif
elif get_option('numad').enabled()
error('You must have numactl enabled for numad support.')
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index ab9d3026cc..71293e039f 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -37,6 +37,10 @@
#define VIR_FROM_THIS VIR_FROM_BHYVE
+#define BHYVE "bhyve"
+#define BHYVECTL "bhyvectl"
+#define BHYVELOAD "bhyveload"
+
VIR_LOG_INIT("bhyve.bhyve_command");
static int
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index 823b918db3..ea787bf87d 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -454,7 +454,8 @@ static int virLockManagerLockDaemonNew(virLockManager *lock,
}
-#ifdef LVS
+#define LVS "lvs"
+
static int
virLockManagerGetLVMKey(const char *path,
char **key)
@@ -508,15 +509,6 @@ virLockManagerGetLVMKey(const char *path,
return ret;
}
-#else
-static int
-virLockManagerGetLVMKey(const char *path,
- char **key G_GNUC_UNUSED)
-{
- virReportSystemError(ENOSYS, _("Unable to get LVM key for %s"), path);
- return -1;
-}
-#endif
static int virLockManagerLockDaemonAddResource(virLockManager *lock,
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index ed8e47d880..4bbb585d07 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -40,6 +40,19 @@
VIR_LOG_INIT("storage.storage_backend_logical");
+#define PVCREATE "pvcreate"
+#define PVREMOVE "pvremove"
+#define PVS "pvs"
+#define VGCREATE "vgcreate"
+#define VGREMOVE "vgremove"
+#define VGCHANGE "vgchange"
+#define VGSCAN "vgscan"
+#define VGS "vgs"
+#define LVCREATE "lvcreate"
+#define LVREMOVE "lvremove"
+#define LVCHANGE "lvchange"
+#define LVS "lvs"
+
static int
virStorageBackendLogicalSetActive(virStoragePoolObj *pool,
diff --git a/src/storage/storage_backend_sheepdog.c
b/src/storage/storage_backend_sheepdog.c
index 6490dfae52..beb6445e1e 100644
--- a/src/storage/storage_backend_sheepdog.c
+++ b/src/storage/storage_backend_sheepdog.c
@@ -35,6 +35,8 @@
#define VIR_FROM_THIS VIR_FROM_STORAGE
+#define SHEEPDOGCLI "dog"
+
static int virStorageBackendSheepdogRefreshVol(virStoragePoolObj *pool,
virStorageVolDef *vol);
diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_zfs.c
index 6fabeade11..2a5d74357d 100644
--- a/src/storage/storage_backend_zfs.c
+++ b/src/storage/storage_backend_zfs.c
@@ -33,6 +33,9 @@
VIR_LOG_INIT("storage.storage_backend_zfs");
+#define ZFS "zfs"
+#define ZPOOL "zpool"
+
/*
* Some common flags of zfs and zpool commands we use:
* -H -- don't print headers and separate fields by tab
diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h
index aa3c25e9fc..487345ff22 100644
--- a/src/storage/storage_util.h
+++ b/src/storage/storage_util.h
@@ -25,6 +25,12 @@
#include "storage_driver.h"
#include "storage_backend.h"
+#define PARTED "parted"
+#define MOUNT "mount"
+#define UMOUNT "umount"
+#define MKFS "mkfs"
+#define SHOWMOUNT "showmount"
+
/* Storage Pool Namespace options to share w/ storage_backend_fs.c and
* the virStorageBackendFileSystemMountCmd method */
typedef struct _virStoragePoolFSMountOptionsDef virStoragePoolFSMountOptionsDef;
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 31f65d8902..d3cdc3ba53 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -52,6 +52,7 @@
VIR_LOG_INIT("util.numa");
+#define NUMAD "numad"
#if WITH_NUMAD
char *
--
2.31.1
4:43 a.m.
New subject: [PATCH RESEND v3 5/8] meson: stop setting runtime binaries defines during compilation
On Tue, May 25, 2021 at 10:00:44AM +0200, Pavel Hrdina wrote:
Technically the location of these binaries may be different when
compiling libvirt or running it. This will allow users to change $PATH
to use different binaries as well.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
meson.build | 41 ++++----------------------
src/bhyve/bhyve_command.c | 4 +++
src/locking/lock_driver_lockd.c | 12 ++------
src/storage/storage_backend_logical.c | 13 ++++++++
src/storage/storage_backend_sheepdog.c | 2 ++
src/storage/storage_backend_zfs.c | 3 ++
src/storage/storage_util.h | 6 ++++
src/util/virnuma.c | 1 +
8 files changed, 36 insertions(+), 46 deletions(-)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
3 a.m.
New subject: [PATCH RESEND v3 6/8] meson: use runtime binaries to only resolve features with "auto" value
This way meson will try to do the right thing by default but we will
allow users to change this behavior by using -Dname=enabled. This comes
with two benefits compared to the previous behavior:
- no need to install the binaries if developers would like to check
that the code compiles correctly
- package maintainers can drop some build dependencies
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
meson.build | 102 ++++++++++++++++++++++++++++++++--------------------
1 file changed, 63 insertions(+), 39 deletions(-)
diff --git a/meson.build b/meson.build
index 2900b6e1a8..8e8d31953d 100644
--- a/meson.build
+++ b/meson.build
@@ -977,12 +977,6 @@ endif
libparted_version = '1.8.0'
libparted_dep = dependency('libparted', version: '>=' +
libparted_version, required: false)
-if libparted_dep.found()
- parted_prog = find_program('parted', required: false, dirs: libvirt_sbin_path)
- if not parted_prog.found()
- libparted_dep = dependency('', required: false)
- endif
-endif
libpcap_version = '1.5.0'
if not get_option('libpcap').disabled()
@@ -1420,11 +1414,17 @@ if not get_option('driver_libvirtd').disabled()
endif
if not get_option('driver_bhyve').disabled() and host_machine.system() ==
'freebsd'
- bhyve_prog = find_program('bhyve', required:
get_option('driver_bhyve'))
- bhyvectl_prog = find_program('bhyvectl', required:
get_option('driver_bhyve'))
- bhyveload_prog = find_program('bhyveload', required:
get_option('driver_bhyve'))
+ bhyve_enable = true
- if bhyve_prog.found() and bhyvectl_prog.found() and bhyveload_prog.found()
+ if get_option('driver_bhyve').auto()
+ bhyve_prog = find_program('bhyve', required: false)
+ bhyvectl_prog = find_program('bhyvectl', required: false)
+ bhyveload_prog = find_program('bhyveload', required: false)
+
+ bhyve_enable = bhyve_prog.found() and bhyvectl_prog.found() and
bhyveload_prog.found()
+ endif
+
+ if bhyve_enable
conf.set('WITH_BHYVE', 1)
endif
elif get_option('driver_bhyve').enabled()
@@ -1680,8 +1680,17 @@ if conf.has('WITH_LIBVIRTD')
endif
if not get_option('storage_disk').disabled() and devmapper_dep.found() and
libparted_dep.found()
- use_storage = true
- conf.set('WITH_STORAGE_DISK', 1)
+ disk_enable = true
+
+ if get_option('storage_disk').auto()
+ parted_prog = find_program('parted', required: false, dirs:
libvirt_sbin_path)
+ disk_enable = parted_prog.found()
+ endif
+
+ if disk_enable
+ use_storage = true
+ conf.set('WITH_STORAGE_DISK', 1)
+ endif
elif get_option('storage_disk').enabled()
error('You must install libparted and libdevmapper to compile libvirt with disk
storage driver')
endif
@@ -1702,10 +1711,10 @@ if conf.has('WITH_LIBVIRTD')
endif
endif
- if fs_enable
- mount_prog = find_program('mount', required:
get_option('storage_fs'), dirs: libvirt_sbin_path)
- umount_prog = find_program('umount', required:
get_option('storage_fs'), dirs: libvirt_sbin_path)
- mkfs_prog = find_program('mkfs', required:
get_option('storage_fs'), dirs: libvirt_sbin_path)
+ if fs_enable and get_option('storage_fs').auto()
+ mount_prog = find_program('mount', required: false, dirs:
libvirt_sbin_path)
+ umount_prog = find_program('umount', required: false, dirs:
libvirt_sbin_path)
+ mkfs_prog = find_program('mkfs', required: false, dirs: libvirt_sbin_path)
if not mount_prog.found() or not umount_prog.found() or not mkfs_prog.found()
fs_enable = false
@@ -1727,14 +1736,11 @@ if conf.has('WITH_LIBVIRTD')
if not get_option('storage_iscsi').disabled()
iscsi_enable = true
- iscsiadm_prog = find_program('iscsiadm', required: false, dirs:
libvirt_sbin_path)
- if not iscsiadm_prog.found()
- if get_option('storage_iscsi').enabled()
- error('We need iscsiadm for iSCSI storage driver')
- else
- iscsi_enable = false
- endif
+ if get_option('storage_iscsi').auto()
+ iscsiadm_prog = find_program('iscsiadm', required: false, dirs:
libvirt_sbin_path)
+
+ iscsi_enable = iscsiadm_prog.found()
endif
if iscsi_enable
@@ -1758,12 +1764,15 @@ if conf.has('WITH_LIBVIRTD')
'lvchange', 'vgchange', 'vgscan',
'pvs', 'vgs', 'lvs',
]
- foreach name : lvm_progs
- lvm_prog = find_program(name, required: get_option('storage_lvm'), dirs:
libvirt_sbin_path)
- if not lvm_prog.found()
- lvm_enable = false
- endif
- endforeach
+
+ if get_option('storage_lvm').auto()
+ foreach name : lvm_progs
+ lvm_prog = find_program(name, required: false, dirs: libvirt_sbin_path)
+ if not lvm_prog.found()
+ lvm_enable = false
+ endif
+ endforeach
+ endif
if lvm_enable
use_storage = true
@@ -1791,9 +1800,15 @@ if conf.has('WITH_LIBVIRTD')
endif
if not get_option('storage_sheepdog').disabled()
- sheepdogcli_prog = find_program('dog', required:
get_option('storage_sheepdog'), dirs: libvirt_sbin_path)
+ sheepdog_enable = true
- if sheepdogcli_prog.found()
+ if get_option('storage_sheepdog').auto()
+ sheepdogcli_prog = find_program('dog', required: false, dirs:
libvirt_sbin_path)
+
+ sheepdog_enable = sheepdogcli_prog.found()
+ endif
+
+ if sheepdog_enable
use_storage = true
conf.set('WITH_STORAGE_SHEEPDOG', 1)
endif
@@ -1817,12 +1832,15 @@ if conf.has('WITH_LIBVIRTD')
if not get_option('storage_zfs').disabled()
zfs_enable = true
- foreach name : ['zfs', 'zpool']
- zfs_prog = find_program(name, required: get_option('storage_zfs'), dirs:
libvirt_sbin_path)
- if not zfs_prog.found()
- zfs_enable = false
- endif
- endforeach
+
+ if get_option('storage_zfs').auto()
+ foreach name : ['zfs', 'zpool']
+ zfs_prog = find_program(name, required: false, dirs: libvirt_sbin_path)
+ if not zfs_prog.found()
+ zfs_enable = false
+ endif
+ endforeach
+ endif
if zfs_enable
use_storage = true
@@ -1937,8 +1955,14 @@ if not get_option('nss').disabled()
endif
if not get_option('numad').disabled() and numactl_dep.found()
- numad_prog = find_program('numad', required: get_option('numad'), dirs:
libvirt_sbin_path)
- if numad_prog.found()
+ numad_enable = true
+
+ if get_option('numad').auto()
+ numad_prog = find_program('numad', required: false, dirs: libvirt_sbin_path)
+ numad_enable = numad_prog.found()
+ endif
+
+ if numad_enable
conf.set('WITH_NUMAD', 1)
endif
elif get_option('numad').enabled()
--
2.31.1
4:54 a.m.
New subject: [PATCH RESEND v3 6/8] meson: use runtime binaries to only resolve features with "auto" value
On Tue, May 25, 2021 at 10:00:45AM +0200, Pavel Hrdina wrote:
This way meson will try to do the right thing by default but we will
allow users to change this behavior by using -Dname=enabled. This comes
with two benefits compared to the previous behavior:
- no need to install the binaries if developers would like to check
that the code compiles correctly
- package maintainers can drop some build dependencies
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
meson.build | 102 ++++++++++++++++++++++++++++++++--------------------
1 file changed, 63 insertions(+), 39 deletions(-)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
3 a.m.
New subject: [PATCH RESEND v3 7/8] meson: optional_programs should be used only for building libvirt
Drop code that creates defines with program paths and update the comment
to reflect current usage of optional_programs.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
meson.build | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/meson.build b/meson.build
index 8e8d31953d..1c0de2f408 100644
--- a/meson.build
+++ b/meson.build
@@ -819,7 +819,7 @@ foreach item : required_programs_groups
endforeach
-# optional programs
+# optional programs used while building libvirt
optional_programs = [
'augparse',
@@ -830,14 +830,6 @@ optional_programs = [
foreach name : optional_programs
prog = find_program(name, required: false, dirs: libvirt_sbin_path)
varname = name.underscorify()
- if prog.found()
- prog_path = prog.path()
- else
- prog_path = name
- endif
-
- conf.set_quoted(varname.to_upper(), prog_path)
- conf.set_quoted('@0(a)_PATH'.format(varname.to_upper()), prog_path)
set_variable('@0(a)_prog'.format(varname), prog)
endforeach
--
2.31.1
4:55 a.m.
New subject: [PATCH RESEND v3 7/8] meson: optional_programs should be used only for building libvirt
On Tue, May 25, 2021 at 10:00:46AM +0200, Pavel Hrdina wrote:
Drop code that creates defines with program paths and update the
comment
to reflect current usage of optional_programs.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
meson.build | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
3 a.m.
New subject: [PATCH RESEND v3 8/8] libvirt.spec: drop no longer required build dependencies
These are no longer required to build libvirt as they are used during
compilation only by meson to detect if some "auto" features should be
enabled or not but in spec file we explicitly enable/disable all libvirt
features.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
libvirt.spec.in | 31 -------------------------------
1 file changed, 31 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index f421828d16..a49cd58e38 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -260,10 +260,6 @@ BuildRequires: sanlock-devel >= 2.4
BuildRequires: libpcap-devel >= 1.5.0
BuildRequires: libnl3-devel
BuildRequires: libselinux-devel
-BuildRequires: dnsmasq >= 2.41
-BuildRequires: iptables
-BuildRequires: radvd
-BuildRequires: ebtables
BuildRequires: module-init-tools
BuildRequires: cyrus-sasl-devel
BuildRequires: polkit >= 0.112
@@ -272,13 +268,7 @@ BuildRequires: util-linux
%if %{with_qemu}
# For managing ACLs
BuildRequires: libacl-devel
-# From QEMU RPMs
-BuildRequires: /usr/bin/qemu-img
%endif
-# For LVM drivers
-BuildRequires: lvm2
-# For pool type=iscsi
-BuildRequires: iscsi-initiator-utils
%if %{with_storage_iscsi_direct}
# For pool type=iscsi-direct
BuildRequires: libiscsi-devel
@@ -297,15 +287,6 @@ BuildRequires: librbd-devel
BuildRequires: glusterfs-api-devel >= 3.4.1
BuildRequires: glusterfs-devel >= 3.4.1
%endif
-%if %{with_storage_sheepdog}
-BuildRequires: sheepdog
-%endif
-%if %{with_storage_zfs}
-# Support any conforming implementation of zfs. On stock Fedora
-# this is zfs-fuse, but could be zfsonlinux upstream RPMs
-BuildRequires: /sbin/zfs
-BuildRequires: /sbin/zpool
-%endif
%if %{with_numactl}
# For QEMU/LXC numa info
BuildRequires: numactl-devel
@@ -330,21 +311,9 @@ BuildRequires: audit-libs-devel
# we need /usr/sbin/dtrace
BuildRequires: systemtap-sdt-devel
-# For mount/umount in FS driver
-BuildRequires: util-linux
-# For showmount in FS driver (netfs discovery)
-BuildRequires: nfs-utils
-
# Fedora build root suckage
BuildRequires: gawk
-# For storage wiping with different algorithms
-BuildRequires: scrub
-
-%if %{with_numad}
-BuildRequires: numad
-%endif
-
%if %{with_wireshark}
BuildRequires: wireshark-devel
%endif
--
2.31.1
5 a.m.
New subject: [PATCH RESEND v3 8/8] libvirt.spec: drop no longer required build dependencies
On Tue, May 25, 2021 at 10:00:47AM +0200, Pavel Hrdina wrote:
-# For mount/umount in FS driver
-BuildRequires: util-linux
This BuildRequires is duplicated, so please drop the other instance
as well.
-# For storage wiping with different algorithms
-BuildRequires: scrub
Turns out we're actually missing the runtime dependency on scrub! Can
you please take care of addressing that issue in a separate patch?
With the second BuildRequires on util-linux removed,
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
10:06 a.m.
New subject: [PATCH RESEND v3 8/8] libvirt.spec: drop no longer required build dependencies
On 5/25/21 12:00 PM, Andrea Bolognani wrote:
On Tue, May 25, 2021 at 10:00:47AM +0200, Pavel Hrdina wrote:
> -# For mount/umount in FS driver
> -BuildRequires: util-linux
This BuildRequires is duplicated, so please drop the other instance
as well.
> -# For storage wiping with different algorithms
> -BuildRequires: scrub
Turns out we're actually missing the runtime dependency on scrub! Can
you please take care of addressing that issue in a separate patch?
Are we? scrub is needed if and only if you want to pass a special
algorithm to virStorageVolWipePattern(). Does that justify a runtime
dependency? We fail gracefully if scrub isn't installed. We aren't even
requiring qemu binary for daemon-driver-qemu package.
Wasn't the whole idea to drop dependencies?
Michal
10:33 a.m.
New subject: [PATCH RESEND v3 8/8] libvirt.spec: drop no longer required build dependencies
On Tue, May 25, 2021 at 05:06:41PM +0200, Michal Prívozník wrote:
On 5/25/21 12:00 PM, Andrea Bolognani wrote:
> On Tue, May 25, 2021 at 10:00:47AM +0200, Pavel Hrdina wrote:
>> -# For storage wiping with different algorithms
>> -BuildRequires: scrub
>
> Turns out we're actually missing the runtime dependency on scrub! Can
> you please take care of addressing that issue in a separate patch?
Are we? scrub is needed if and only if you want to pass a special
algorithm to virStorageVolWipePattern(). Does that justify a runtime
dependency? We fail gracefully if scrub isn't installed. We aren't even
requiring qemu binary for daemon-driver-qemu package.
Wasn't the whole idea to drop dependencies?
The point was to drop **build** dependencies, specifically
problematic ones such as ZFS. When it comes to runtime dependencies,
our RPMs generally try to be as featureful as possible, so adding a
dependency on scrub is the right thing to do IMO.
--
Andrea Bolognani / Red Hat / Virtualization
9:19 p.m.
New subject: [PATCH RESEND v3 8/8] libvirt.spec: drop no longer required build dependencies
On Tue, May 25, 2021 at 11:34 AM Andrea Bolognani <abologna(a)redhat.com> wrote:
On Tue, May 25, 2021 at 05:06:41PM +0200, Michal Prívozník wrote:
> On 5/25/21 12:00 PM, Andrea Bolognani wrote:
> > On Tue, May 25, 2021 at 10:00:47AM +0200, Pavel Hrdina wrote:
> >> -# For storage wiping with different algorithms
> >> -BuildRequires: scrub
> >
> > Turns out we're actually missing the runtime dependency on scrub! Can
> > you please take care of addressing that issue in a separate patch?
>
> Are we? scrub is needed if and only if you want to pass a special
> algorithm to virStorageVolWipePattern(). Does that justify a runtime
> dependency? We fail gracefully if scrub isn't installed. We aren't even
> requiring qemu binary for daemon-driver-qemu package.
>
> Wasn't the whole idea to drop dependencies?
The point was to drop **build** dependencies, specifically
problematic ones such as ZFS. When it comes to runtime dependencies,
our RPMs generally try to be as featureful as possible, so adding a
dependency on scrub is the right thing to do IMO.
Why is ZFS problematic? The zfs-fuse package is totally fine for us to
depend on...
--
真実はいつも一つ！/ Always, there's only one truth!
2:50 a.m.
New subject: [PATCH RESEND v3 8/8] libvirt.spec: drop no longer required build dependencies
On Tue, May 25, 2021 at 10:19:28PM -0400, Neal Gompa wrote:
On Tue, May 25, 2021 at 11:34 AM Andrea Bolognani
<abologna(a)redhat.com> wrote:
> On Tue, May 25, 2021 at 05:06:41PM +0200, Michal Prívozník wrote:
> > Wasn't the whole idea to drop dependencies?
>
> The point was to drop **build** dependencies, specifically
> problematic ones such as ZFS. When it comes to runtime dependencies,
> our RPMs generally try to be as featureful as possible, so adding a
> dependency on scrub is the right thing to do IMO.
Why is ZFS problematic? The zfs-fuse package is totally fine for us to
depend on...
I'm not too familiar with the ZFS situation in Fedora / RHEL, but at
least in Debian we're patching meson.build specifically to enable ZFS
support without needing a ZFS implementation available at build time.
In general, using the build time availability of programs to guess
whether features should be enabled is a good, user-friendly behavior,
but we shouldn't second-guess the information provided by the user:
if they passed -Dstorage_zfs=enabled to Meson, then we should enable
ZFS support whether or not ZFS happens to be currently installed.
--
Andrea Bolognani / Red Hat / Virtualization
1272
days inactive
1273
days old
21 comments
4 participants
participants (4)
-
Andrea Bolognani -
Michal Prívozník -
Neal Gompa -
Pavel Hrdina