From 94d707dd225104ba14422eeb43c73b1f742b12da Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
Date: Tue, 13 Jul 2021 13:22:05 +0200
Subject: [PATCH 1/7] lvm: Allow configuring global "device filter" for LVM
commands
Starting with 2.03.12 LVM introduces a new system for telling LVM
which devices it should use. The old device filters in config are
no longer working and we need to use either the system.devices
config file in /etc/lvm/devices (default behaviour) or specify
all allowed devices using the new --devices option. Because this
option must be specified for every call which might be incovenient
for our users, this commit introduces a new function to configure
this globally, which we already do for the --config option.
---
src/lib/plugin_apis/lvm.api | 23 +++
src/plugins/lvm-dbus.c | 75 ++++++++-
src/plugins/lvm.c | 97 ++++++++++--
src/plugins/lvm.h | 4 +
tests/library_test.py | 304 ++++++++++++++++++++----------------
tests/lvm_dbus_tests.py | 47 +++++-
tests/lvm_test.py | 50 ++++++
tests/overrides_test.py | 23 ++-
8 files changed, 470 insertions(+), 153 deletions(-)
diff --git a/src/lib/plugin_apis/lvm.api b/src/lib/plugin_apis/lvm.api
index c695c111..23f68b81 100644
--- a/src/lib/plugin_apis/lvm.api
+++ b/src/lib/plugin_apis/lvm.api
@@ -601,6 +601,7 @@ typedef enum {
BD_LVM_TECH_CACHE_CALCS,
BD_LVM_TECH_GLOB_CONF,
BD_LVM_TECH_VDO,
+ BD_LVM_TECH_DEVICES,
} BDLVMTech;
typedef enum {
@@ -1214,6 +1215,28 @@ gboolean bd_lvm_set_global_config (const gchar *new_config, GError **error);
*/
gchar* bd_lvm_get_global_config (GError **error);
+/**
+ * bd_lvm_set_devices_filter:
+ * @devices: (allow-none) (array zero-terminated=1): list of devices for lvm commands to work on
+ * @error: (out): place to store error (if any)
+ *
+ * Returns: whether the devices filter was successfully set or not
+ *
+ * Tech category: %BD_LVM_TECH_DEVICES no mode (it is ignored)
+ */
+gboolean bd_lvm_set_devices_filter (const gchar **devices, GError **error);
+
+/**
+ * bd_lvm_get_devices_filter:
+ * @error: (out): place to store error (if any)
+ *
+ * Returns: (transfer full) (array zero-terminated=1): a copy of a string representation of
+ * the currently set LVM devices filter
+ *
+ * Tech category: %BD_LVM_TECH_DEVICES no mode (it is ignored)
+ */
+gchar** bd_lvm_get_devices_filter (GError **error);
+
/**
* bd_lvm_cache_get_default_md_size:
* @cache_size: size of the cache to determine MD size for
diff --git a/src/plugins/lvm-dbus.c b/src/plugins/lvm-dbus.c
index 51572c9a..b47ed0ef 100644
--- a/src/plugins/lvm-dbus.c
+++ b/src/plugins/lvm-dbus.c
@@ -35,6 +35,8 @@
static GMutex global_config_lock;
static gchar *global_config_str = NULL;
+static gchar *global_devices_str = NULL;
+
#define LVM_BUS_NAME "com.redhat.lvmdbus1"
#define LVM_OBJ_PREFIX "/com/redhat/lvmdbus1"
#define MANAGER_OBJ "/com/redhat/lvmdbus1/Manager"
@@ -241,11 +243,20 @@ static gboolean setup_dbus_connection (GError **error) {
return TRUE;
}
+static volatile guint avail_deps = 0;
static volatile guint avail_dbus_deps = 0;
static volatile guint avail_features = 0;
static volatile guint avail_module_deps = 0;
static GMutex deps_check_lock;
+#define DEPS_LVMDEVICES 0
+#define DEPS_LVMDEVICES_MASK (1 << DEPS_LVMDEVICES)
+#define DEPS_LAST 1
+
+static const UtilDep deps[DEPS_LAST] = {
+ {"lvmdevices", NULL, NULL, NULL},
+};
+
#define DBUS_DEPS_LVMDBUSD 0
#define DBUS_DEPS_LVMDBUSD_MASK (1 << DBUS_DEPS_LVMDBUSD)
#define DBUS_DEPS_LAST 1
@@ -378,6 +389,8 @@ gboolean bd_lvm_is_tech_avail (BDLVMTech tech, guint64 mode, GError **error) {
return check_dbus_deps (&avail_dbus_deps, DBUS_DEPS_LVMDBUSD_MASK, dbus_deps, DBUS_DEPS_LAST, &deps_check_lock, error) &&
check_features (&avail_features, FEATURES_VDO_MASK, features, FEATURES_LAST, &deps_check_lock, error) &&
check_module_deps (&avail_module_deps, MODULE_DEPS_VDO_MASK, module_deps, MODULE_DEPS_LAST, &deps_check_lock, error);
+ case BD_LVM_TECH_DEVICES:
+ return check_deps (&avail_deps, DEPS_LVMDEVICES_MASK, deps, DEPS_LAST, &deps_check_lock, error);
default:
/* everything is supported by this implementation of the plugin */
return check_dbus_deps (&avail_dbus_deps, DBUS_DEPS_LVMDBUSD_MASK, dbus_deps, DBUS_DEPS_LAST, &deps_check_lock, error);
@@ -515,6 +528,7 @@ static gboolean unbox_params_and_add (GVariant *params, GVariantBuilder *builder
static GVariant* call_lvm_method (const gchar *obj, const gchar *intf, const gchar *method, GVariant *params, GVariant *extra_params, const BDExtraArg **extra_args, guint64 *task_id, guint64 *progress_id, gboolean lock_config, GError **error) {
GVariant *config = NULL;
+ GVariant *devices = NULL;
GVariant *param = NULL;
GVariantIter iter;
GVariantBuilder builder;
@@ -536,8 +550,8 @@ static GVariant* call_lvm_method (const gchar *obj, const gchar *intf, const gch
if (lock_config)
g_mutex_lock (&global_config_lock);
- if (global_config_str || extra_params || extra_args) {
- if (global_config_str || extra_args) {
+ if (global_config_str || global_devices_str || extra_params || extra_args) {
+ if (global_config_str || global_devices_str || extra_args) {
/* add the global config to the extra_params */
g_variant_builder_init (&extra_builder, G_VARIANT_TYPE_DICTIONARY);
@@ -558,6 +572,11 @@ static GVariant* call_lvm_method (const gchar *obj, const gchar *intf, const gch
g_variant_builder_add (&extra_builder, "{sv}", "--config", config);
added_extra = TRUE;
}
+ if (global_devices_str) {
+ devices = g_variant_new ("s", global_devices_str);
+ g_variant_builder_add (&extra_builder, "{sv}", "--devices", devices);
+ added_extra = TRUE;
+ }
if (added_extra)
config_extra_params = g_variant_builder_end (&extra_builder);
@@ -2654,6 +2673,58 @@ gchar* bd_lvm_get_global_config (GError **error UNUSED) {
return ret;
}
+/**
+ * bd_lvm_set_devices_filter:
+ * @devices: (allow-none) (array zero-terminated=1): list of devices for lvm commands to work on
+ * @error: (out): place to store error (if any)
+ *
+ * Returns: whether the devices filter was successfully set or not
+ *
+ * Tech category: %BD_LVM_TECH_DEVICES no mode (it is ignored)
+ */
+gboolean bd_lvm_set_devices_filter (const gchar **devices, GError **error) {
+ if (!bd_lvm_is_tech_avail (BD_LVM_TECH_DEVICES, 0, error))
+ return FALSE;
+
+ g_mutex_lock (&global_config_lock);
+
+ /* first free the old value */
+ g_free (global_devices_str);
+
+ /* now store the new one */
+ if (!devices || !(*devices))
+ global_devices_str = NULL;
+ else
+ global_devices_str = g_strjoinv (",", (gchar **) devices);
+
+ g_mutex_unlock (&global_config_lock);
+ return TRUE;
+}
+
+/**
+ * bd_lvm_get_devices_filter:
+ * @error: (out): place to store error (if any)
+ *
+ * Returns: (transfer full) (array zero-terminated=1): a copy of a string representation of
+ * the currently set LVM devices filter
+ *
+ * Tech category: %BD_LVM_TECH_DEVICES no mode (it is ignored)
+ */
+gchar** bd_lvm_get_devices_filter (GError **error UNUSED) {
+ gchar **ret = NULL;
+
+ g_mutex_lock (&global_config_lock);
+
+ if (global_devices_str)
+ ret = g_strsplit (global_devices_str, ",", -1);
+ else
+ ret = NULL;
+
+ g_mutex_unlock (&global_config_lock);
+
+ return ret;
+}
+
/**
* bd_lvm_cache_get_default_md_size:
* @cache_size: size of the cache to determine MD size for
diff --git a/src/plugins/lvm.c b/src/plugins/lvm.c
index 26af0d19..42ee0f90 100644
--- a/src/plugins/lvm.c
+++ b/src/plugins/lvm.c
@@ -34,6 +34,8 @@
static GMutex global_config_lock;
static gchar *global_config_str = NULL;
+static gchar *global_devices_str = NULL;
+
/**
* SECTION: lvm
* @short_description: plugin for operations with LVM
@@ -212,10 +214,13 @@ static GMutex deps_check_lock;
#define DEPS_LVM 0
#define DEPS_LVM_MASK (1 << DEPS_LVM)
-#define DEPS_LAST 1
+#define DEPS_LVMDEVICES 1
+#define DEPS_LVMDEVICES_MASK (1 << DEPS_LVMDEVICES)
+#define DEPS_LAST 2
static const UtilDep deps[DEPS_LAST] = {
{"lvm", LVM_MIN_VERSION, "version", "LVM version:\\s+([\\d\\.]+)"},
+ {"lvmdevices", NULL, NULL, NULL},
};
#define FEATURES_VDO 0
@@ -327,6 +332,8 @@ gboolean bd_lvm_is_tech_avail (BDLVMTech tech, guint64 mode, GError **error) {
case BD_LVM_TECH_VDO:
return check_features (&avail_features, FEATURES_VDO_MASK, features, FEATURES_LAST, &deps_check_lock, error) &&
check_module_deps (&avail_module_deps, MODULE_DEPS_VDO_MASK, module_deps, MODULE_DEPS_LAST, &deps_check_lock, error);
+ case BD_LVM_TECH_DEVICES:
+ return check_deps (&avail_deps, DEPS_LVMDEVICES_MASK, deps, DEPS_LAST, &deps_check_lock, error);
default:
/* everything is supported by this implementation of the plugin */
return check_deps (&avail_deps, DEPS_LVM_MASK, deps, DEPS_LAST, &deps_check_lock, error);
@@ -337,6 +344,8 @@ static gboolean call_lvm_and_report_error (const gchar **args, const BDExtraArg
gboolean success = FALSE;
guint i = 0;
guint args_length = g_strv_length ((gchar **) args);
+ g_autofree gchar *config_arg = NULL;
+ g_autofree gchar *devices_arg = NULL;
if (!check_deps (&avail_deps, DEPS_LVM_MASK, deps, DEPS_LAST, &deps_check_lock, error))
return FALSE;
@@ -345,20 +354,26 @@ static gboolean call_lvm_and_report_error (const gchar **args, const BDExtraArg
if (lock_config)
g_mutex_lock (&global_config_lock);
- /* allocate enough space for the args plus "lvm", "--config" and NULL */
- const gchar **argv = g_new0 (const gchar*, args_length + 3);
+ /* allocate enough space for the args plus "lvm", "--config", "--devices" and NULL */
+ const gchar **argv = g_new0 (const gchar*, args_length + 4);
/* construct argv from args with "lvm" prepended */
argv[0] = "lvm";
for (i=0; i < args_length; i++)
argv[i+1] = args[i];
- argv[args_length + 1] = global_config_str ? g_strdup_printf("--config=%s", global_config_str) : NULL;
- argv[args_length + 2] = NULL;
+ if (global_config_str) {
+ config_arg = g_strdup_printf("--config=%s", global_config_str);
+ argv[++args_length] = config_arg;
+ }
+ if (global_devices_str) {
+ devices_arg = g_strdup_printf("--devices=%s", global_devices_str);
+ argv[++args_length] = devices_arg;
+ }
+ argv[++args_length] = NULL;
success = bd_utils_exec_and_report_error (argv, extra, error);
if (lock_config)
g_mutex_unlock (&global_config_lock);
- g_free ((gchar *) argv[args_length + 1]);
g_free (argv);
return success;
@@ -368,6 +383,8 @@ static gboolean call_lvm_and_capture_output (const gchar **args, const BDExtraAr
gboolean success = FALSE;
guint i = 0;
guint args_length = g_strv_length ((gchar **) args);
+ g_autofree gchar *config_arg = NULL;
+ g_autofree gchar *devices_arg = NULL;
if (!check_deps (&avail_deps, DEPS_LVM_MASK, deps, DEPS_LAST, &deps_check_lock, error))
return FALSE;
@@ -375,19 +392,25 @@ static gboolean call_lvm_and_capture_output (const gchar **args, const BDExtraAr
/* don't allow global config string changes during the run */
g_mutex_lock (&global_config_lock);
- /* allocate enough space for the args plus "lvm", "--config" and NULL */
- const gchar **argv = g_new0 (const gchar*, args_length + 3);
+ /* allocate enough space for the args plus "lvm", "--config", "--devices" and NULL */
+ const gchar **argv = g_new0 (const gchar*, args_length + 4);
/* construct argv from args with "lvm" prepended */
argv[0] = "lvm";
for (i=0; i < args_length; i++)
argv[i+1] = args[i];
- argv[args_length + 1] = global_config_str ? g_strdup_printf("--config=%s", global_config_str) : NULL;
- argv[args_length + 2] = NULL;
+ if (global_config_str) {
+ config_arg = g_strdup_printf("--config=%s", global_config_str);
+ argv[++args_length] = config_arg;
+ }
+ if (global_devices_str) {
+ devices_arg = g_strdup_printf("--devices=%s", global_devices_str);
+ argv[++args_length] = devices_arg;
+ }
+ argv[++args_length] = NULL;
success = bd_utils_exec_and_capture_output (argv, extra, output, error);
g_mutex_unlock (&global_config_lock);
- g_free ((gchar *) argv[args_length + 1]);
g_free (argv);
return success;
@@ -2033,6 +2056,58 @@ gchar* bd_lvm_get_global_config (GError **error UNUSED) {
return ret;
}
+/**
+ * bd_lvm_set_devices_filter:
+ * @devices: (allow-none) (array zero-terminated=1): list of devices for lvm commands to work on
+ * @error: (out): place to store error (if any)
+ *
+ * Returns: whether the devices filter was successfully set or not
+ *
+ * Tech category: %BD_LVM_TECH_DEVICES no mode (it is ignored)
+ */
+gboolean bd_lvm_set_devices_filter (const gchar **devices, GError **error) {
+ if (!bd_lvm_is_tech_avail (BD_LVM_TECH_DEVICES, 0, error))
+ return FALSE;
+
+ g_mutex_lock (&global_config_lock);
+
+ /* first free the old value */
+ g_free (global_devices_str);
+
+ /* now store the new one */
+ if (!devices || !(*devices))
+ global_devices_str = NULL;
+ else
+ global_devices_str = g_strjoinv (",", (gchar **) devices);
+
+ g_mutex_unlock (&global_config_lock);
+ return TRUE;
+}
+
+/**
+ * bd_lvm_get_devices_filter:
+ * @error: (out): place to store error (if any)
+ *
+ * Returns: (transfer full) (array zero-terminated=1): a copy of a string representation of
+ * the currently set LVM devices filter
+ *
+ * Tech category: %BD_LVM_TECH_DEVICES no mode (it is ignored)
+ */
+gchar** bd_lvm_get_devices_filter (GError **error UNUSED) {
+ gchar **ret = NULL;
+
+ g_mutex_lock (&global_config_lock);
+
+ if (global_devices_str)
+ ret = g_strsplit (global_devices_str, ",", -1);
+ else
+ ret = NULL;
+
+ g_mutex_unlock (&global_config_lock);
+
+ return ret;
+}
+
/**
* bd_lvm_cache_get_default_md_size:
* @cache_size: size of the cache to determine MD size for
diff --git a/src/plugins/lvm.h b/src/plugins/lvm.h
index 2162d769..8063693f 100644
--- a/src/plugins/lvm.h
+++ b/src/plugins/lvm.h
@@ -216,6 +216,7 @@ typedef enum {
BD_LVM_TECH_CACHE_CALCS,
BD_LVM_TECH_GLOB_CONF,
BD_LVM_TECH_VDO,
+ BD_LVM_TECH_DEVICES,
} BDLVMTech;
typedef enum {
@@ -289,6 +290,9 @@ gboolean bd_lvm_thsnapshotcreate (const gchar *vg_name, const gchar *origin_name
gboolean bd_lvm_set_global_config (const gchar *new_config, GError **error);
gchar* bd_lvm_get_global_config (GError **error);
+gboolean bd_lvm_set_devices_filter (const gchar **devices, GError **error);
+gchar** bd_lvm_get_devices_filter (GError **error);
+
guint64 bd_lvm_cache_get_default_md_size (guint64 cache_size, GError **error);
const gchar* bd_lvm_cache_get_mode_str (BDLVMCacheMode mode, GError **error);
BDLVMCacheMode bd_lvm_cache_get_mode_from_str (const gchar *mode_str, GError **error);
diff --git a/tests/library_test.py b/tests/library_test.py
index 08e44fdc..efd17bd2 100644
--- a/tests/library_test.py
+++ b/tests/library_test.py
@@ -13,18 +13,178 @@ class LibraryOpsTestCase(unittest.TestCase):
# all plugins except for 'btrfs', 'fs' and 'mpath' -- these don't have all
# the dependencies on CentOS/Debian and we don't need them for this test
requested_plugins = BlockDev.plugin_specs_from_names(("crypto", "dm",
- "kbd", "loop", "lvm",
+ "kbd", "loop",
"mdraid", "part", "swap"))
+ @classmethod
+ def setUpClass(cls):
+ if not BlockDev.is_initialized():
+ BlockDev.init(cls.requested_plugins, None)
+ else:
+ BlockDev.reinit(cls.requested_plugins, True, None)
+
+ @classmethod
+ def tearDownClass(cls):
+ BlockDev.switch_init_checks(True)
+
+ def my_log_func(self, level, msg):
+ # not much to verify here
+ self.assertTrue(isinstance(level, int))
+ self.assertTrue(isinstance(msg, str))
+
+ self.log += msg + "\n"
+
+ @tag_test(TestTags.CORE)
+ def test_logging_setup(self):
+ """Verify that setting up logging works as expected"""
+
+ self.assertTrue(BlockDev.reinit(self.requested_plugins, False, self.my_log_func))
+
+ succ = BlockDev.utils_exec_and_report_error(["true"])
+ self.assertTrue(succ)
+
+ # reinit with no logging function should change nothing about logging
+ self.assertTrue(BlockDev.reinit(self.requested_plugins, False, None))
+
+ succ, out = BlockDev.utils_exec_and_capture_output(["echo", "hi"])
+ self.assertTrue(succ)
+ self.assertEqual(out, "hi\n")
+
+ match = re.search(r'Running \[(\d+)\] true', self.log)
+ self.assertIsNot(match, None)
+ task_id1 = match.group(1)
+ match = re.search(r'Running \[(\d+)\] echo hi', self.log)
+ self.assertIsNot(match, None)
+ task_id2 = match.group(1)
+
+ self.assertIn("...done [%s] (exit code: 0)" % task_id1, self.log)
+ self.assertIn("stdout[%s]:" % task_id1, self.log)
+ self.assertIn("stderr[%s]:" % task_id1, self.log)
+
+ self.assertIn("stdout[%s]: hi" % task_id2, self.log)
+ self.assertIn("stderr[%s]:" % task_id2, self.log)
+ self.assertIn("...done [%s] (exit code: 0)" % task_id2, self.log)
+
+ @tag_test(TestTags.CORE)
+ def test_require_plugins(self):
+ """Verify that loading only required plugins works as expected"""
+
+ ps = BlockDev.PluginSpec()
+ ps.name = BlockDev.Plugin.SWAP
+ ps.so_name = ""
+ self.assertTrue(BlockDev.reinit([ps], True, None))
+ self.assertEqual(BlockDev.get_available_plugin_names(), ["swap"])
+ self.assertTrue(BlockDev.reinit(self.requested_plugins, True, None))
+
+ @tag_test(TestTags.CORE)
+ def test_not_implemented(self):
+ """Verify that unloaded/unimplemented functions report errors"""
+
+ # should be loaded and working
+ self.assertTrue(BlockDev.md_canonicalize_uuid("3386ff85:f5012621:4a435f06:1eb47236"))
+
+ ps = BlockDev.PluginSpec()
+ ps.name = BlockDev.Plugin.SWAP
+ ps.so_name = ""
+ self.assertTrue(BlockDev.reinit([ps], True, None))
+ self.assertEqual(BlockDev.get_available_plugin_names(), ["swap"])
+
+ # no longer loaded
+ with self.assertRaises(GLib.GError):
+ BlockDev.md_canonicalize_uuid("3386ff85:f5012621:4a435f06:1eb47236")
+
+ self.assertTrue(BlockDev.reinit(self.requested_plugins, True, None))
+
+ # loaded again
+ self.assertTrue(BlockDev.md_canonicalize_uuid("3386ff85:f5012621:4a435f06:1eb47236"))
+
+ def test_ensure_init(self):
+ """Verify that ensure_init just returns when already initialized"""
+
+ # the library is already initialized, ensure_init() shonuld do nothing
+ avail_plugs = BlockDev.get_available_plugin_names()
+ self.assertTrue(BlockDev.ensure_init(self.requested_plugins, None))
+ self.assertEqual(avail_plugs, BlockDev.get_available_plugin_names())
+
+ # reinit with a subset of plugins
+ plugins = BlockDev.plugin_specs_from_names(["swap", "part"])
+ self.assertTrue(BlockDev.reinit(plugins, True, None))
+ self.assertEqual(set(BlockDev.get_available_plugin_names()), set(["swap", "part"]))
+
+ # ensure_init with the same subset -> nothing should change
+ self.assertTrue(BlockDev.ensure_init(plugins, None))
+ self.assertEqual(set(BlockDev.get_available_plugin_names()), set(["swap", "part"]))
+
+ # ensure_init with more plugins -> extra plugins should be loaded
+ plugins = BlockDev.plugin_specs_from_names(["swap", "part", "crypto"])
+ self.assertTrue(BlockDev.ensure_init(plugins, None))
+ self.assertEqual(set(BlockDev.get_available_plugin_names()), set(["swap", "part", "crypto"]))
+
+ # reinit to unload all plugins
+ self.assertTrue(BlockDev.reinit([], True, None))
+ self.assertEqual(BlockDev.get_available_plugin_names(), [])
+
+ # ensure_init to load all plugins back
+ self.assertTrue(BlockDev.ensure_init(self.requested_plugins, None))
+ self.assertGreaterEqual(len(BlockDev.get_available_plugin_names()), 7)
+
+ def test_try_reinit(self):
+ """Verify that try_reinit() works as expected"""
+
+ # try reinitializing with only some utilities being available and thus
+ # only some plugins able to load
+ with fake_path("tests/lib_missing_utils", keep_utils=["swapon", "swapoff", "mkswap", "swaplabel"]):
+ succ, loaded = BlockDev.try_reinit(self.requested_plugins, True, None)
+ self.assertFalse(succ)
+ for plug_name in ("swap", "crypto"):
+ self.assertIn(plug_name, loaded)
+
+ # reset back to all plugins
+ self.assertTrue(BlockDev.reinit(self.requested_plugins, True, None))
+
+ # now the same with a subset of plugins requested
+ plugins = BlockDev.plugin_specs_from_names(["swap", "crypto"])
+ with fake_path("tests/lib_missing_utils", keep_utils=["swapon", "swapoff", "mkswap", "swaplabel"]):
+ succ, loaded = BlockDev.try_reinit(plugins, True, None)
+ self.assertTrue(succ)
+ self.assertEqual(set(loaded), set(["swap", "crypto"]))
+
+ def test_non_en_init(self):
+ """Verify that the library initializes with lang different from en_US"""
+
+ orig_lang = os.environ.get("LANG")
+ os.environ["LANG"] = "cs.CZ_UTF-8"
+ self.assertTrue(BlockDev.reinit(self.requested_plugins, True, None))
+ if orig_lang:
+ os.environ["LANG"] = orig_lang
+ else:
+ del os.environ["LANG"]
+ self.assertTrue(BlockDev.reinit(self.requested_plugins, True, None))
+
+
+class PluginsTestCase(unittest.TestCase):
+ # only LVM plugin for this test
+ requested_plugins = BlockDev.plugin_specs_from_names(("lvm",))
+
orig_config_dir = ""
@classmethod
def setUpClass(cls):
+ BlockDev.switch_init_checks(False)
if not BlockDev.is_initialized():
BlockDev.init(cls.requested_plugins, None)
else:
BlockDev.reinit(cls.requested_plugins, True, None)
+ try:
+ cls.devices_avail = BlockDev.lvm_is_tech_avail(BlockDev.LVMTech.DEVICES, 0)
+ except:
+ cls.devices_avail = False
+
+ @classmethod
+ def tearDownClass(cls):
+ BlockDev.switch_init_checks(True)
+
def setUp(self):
self.orig_config_dir = os.environ.get("LIBBLOCKDEV_CONFIG_DIR", "")
self.addCleanup(self._clean_up)
@@ -185,6 +345,12 @@ class LibraryOpsTestCase(unittest.TestCase):
def test_plugin_fallback(self):
"""Verify that fallback when loading plugins works as expected"""
+ if not self.devices_avail:
+ self.skipTest("skipping plugin fallback test: missing some LVM dependencies")
+
+ BlockDev.switch_init_checks(True)
+ self.addCleanup(BlockDev.switch_init_checks, False)
+
# library should be successfully initialized
self.assertTrue(BlockDev.is_initialized())
@@ -206,7 +372,7 @@ class LibraryOpsTestCase(unittest.TestCase):
# now reinit the library with the config preferring the new build
orig_conf_dir = os.environ.get("LIBBLOCKDEV_CONFIG_DIR")
- os.environ["LIBBLOCKDEV_CONFIG_DIR"] = "tests/plugin_prio_conf.d"
+ os.environ["LIBBLOCKDEV_CONFIG_DIR"] = "tests/test_configs/plugin_prio_conf.d"
self.assertTrue(BlockDev.reinit(self.requested_plugins, True, None))
# the original plugin should be loaded because the new one should fail
@@ -243,139 +409,9 @@ class LibraryOpsTestCase(unittest.TestCase):
self.assertEqual(BlockDev.lvm_get_max_lv_size(), orig_max_size)
- def my_log_func(self, level, msg):
- # not much to verify here
- self.assertTrue(isinstance(level, int))
- self.assertTrue(isinstance(msg, str))
-
- self.log += msg + "\n"
-
- @tag_test(TestTags.CORE)
- def test_logging_setup(self):
- """Verify that setting up logging works as expected"""
-
- self.assertTrue(BlockDev.reinit(self.requested_plugins, False, self.my_log_func))
-
- succ = BlockDev.utils_exec_and_report_error(["true"])
- self.assertTrue(succ)
-
- # reinit with no logging function should change nothing about logging
- self.assertTrue(BlockDev.reinit(self.requested_plugins, False, None))
-
- succ, out = BlockDev.utils_exec_and_capture_output(["echo", "hi"])
- self.assertTrue(succ)
- self.assertEqual(out, "hi\n")
-
- match = re.search(r'Running \[(\d+)\] true', self.log)
- self.assertIsNot(match, None)
- task_id1 = match.group(1)
- match = re.search(r'Running \[(\d+)\] echo hi', self.log)
- self.assertIsNot(match, None)
- task_id2 = match.group(1)
-
- self.assertIn("...done [%s] (exit code: 0)" % task_id1, self.log)
- self.assertIn("stdout[%s]:" % task_id1, self.log)
- self.assertIn("stderr[%s]:" % task_id1, self.log)
-
- self.assertIn("stdout[%s]: hi" % task_id2, self.log)
- self.assertIn("stderr[%s]:" % task_id2, self.log)
- self.assertIn("...done [%s] (exit code: 0)" % task_id2, self.log)
-
- @tag_test(TestTags.CORE)
- def test_require_plugins(self):
- """Verify that loading only required plugins works as expected"""
-
- ps = BlockDev.PluginSpec()
- ps.name = BlockDev.Plugin.SWAP
- ps.so_name = ""
- self.assertTrue(BlockDev.reinit([ps], True, None))
- self.assertEqual(BlockDev.get_available_plugin_names(), ["swap"])
- self.assertTrue(BlockDev.reinit(self.requested_plugins, True, None))
-
- @tag_test(TestTags.CORE)
- def test_not_implemented(self):
- """Verify that unloaded/unimplemented functions report errors"""
-
- # should be loaded and working
- self.assertTrue(BlockDev.lvm_get_max_lv_size() > 0)
- ps = BlockDev.PluginSpec()
- ps.name = BlockDev.Plugin.SWAP
- ps.so_name = ""
- self.assertTrue(BlockDev.reinit([ps], True, None))
- self.assertEqual(BlockDev.get_available_plugin_names(), ["swap"])
-
- # no longer loaded
- with self.assertRaises(GLib.GError):
- BlockDev.lvm_get_max_lv_size()
-
- self.assertTrue(BlockDev.reinit(self.requested_plugins, True, None))
-
- # loaded again
- self.assertTrue(BlockDev.lvm_get_max_lv_size() > 0)
-
- def test_ensure_init(self):
- """Verify that ensure_init just returns when already initialized"""
-
- # the library is already initialized, ensure_init() shonuld do nothing
- avail_plugs = BlockDev.get_available_plugin_names()
- self.assertTrue(BlockDev.ensure_init(self.requested_plugins, None))
- self.assertEqual(avail_plugs, BlockDev.get_available_plugin_names())
-
- # reinit with a subset of plugins
- plugins = BlockDev.plugin_specs_from_names(["swap", "lvm"])
- self.assertTrue(BlockDev.reinit(plugins, True, None))
- self.assertEqual(set(BlockDev.get_available_plugin_names()), set(["swap", "lvm"]))
-
- # ensure_init with the same subset -> nothing should change
- self.assertTrue(BlockDev.ensure_init(plugins, None))
- self.assertEqual(set(BlockDev.get_available_plugin_names()), set(["swap", "lvm"]))
-
- # ensure_init with more plugins -> extra plugins should be loaded
- plugins = BlockDev.plugin_specs_from_names(["swap", "lvm", "crypto"])
- self.assertTrue(BlockDev.ensure_init(plugins, None))
- self.assertEqual(set(BlockDev.get_available_plugin_names()), set(["swap", "lvm", "crypto"]))
-
- # reinit to unload all plugins
- self.assertTrue(BlockDev.reinit([], True, None))
- self.assertEqual(BlockDev.get_available_plugin_names(), [])
-
- # ensure_init to load all plugins back
- self.assertTrue(BlockDev.ensure_init(self.requested_plugins, None))
- self.assertGreaterEqual(len(BlockDev.get_available_plugin_names()), 8)
-
- def test_try_reinit(self):
- """Verify that try_reinit() works as expected"""
-
- # try reinitializing with only some utilities being available and thus
- # only some plugins able to load
- with fake_path("tests/lib_missing_utils", keep_utils=["swapon", "swapoff", "mkswap", "lvm", "swaplabel"]):
- succ, loaded = BlockDev.try_reinit(self.requested_plugins, True, None)
- self.assertFalse(succ)
- for plug_name in ("swap", "lvm", "crypto"):
- self.assertIn(plug_name, loaded)
-
- # reset back to all plugins
- self.assertTrue(BlockDev.reinit(self.requested_plugins, True, None))
-
- # now the same with a subset of plugins requested
- plugins = BlockDev.plugin_specs_from_names(["lvm", "swap", "crypto"])
- with fake_path("tests/lib_missing_utils", keep_utils=["swapon", "swapoff", "mkswap", "lvm","swaplabel"]):
- succ, loaded = BlockDev.try_reinit(plugins, True, None)
- self.assertTrue(succ)
- self.assertEqual(set(loaded), set(["swap", "lvm", "crypto"]))
-
- def test_non_en_init(self):
- """Verify that the library initializes with lang different from en_US"""
-
- orig_lang = os.environ.get("LANG")
- os.environ["LANG"] = "cs.CZ_UTF-8"
- self.assertTrue(BlockDev.reinit(self.requested_plugins, True, None))
- if orig_lang:
- os.environ["LANG"] = orig_lang
- else:
- del os.environ["LANG"]
- self.assertTrue(BlockDev.reinit(self.requested_plugins, True, None))
+class DepChecksTestCase(unittest.TestCase):
+ requested_plugins = BlockDev.plugin_specs_from_names(( "swap",))
def test_dep_checks_disabled(self):
"""Verify that disabling runtime dep checks works"""
diff --git a/tests/lvm_dbus_tests.py b/tests/lvm_dbus_tests.py
index 3fb7946a..ae26c6d2 100644
--- a/tests/lvm_dbus_tests.py
+++ b/tests/lvm_dbus_tests.py
@@ -50,6 +50,11 @@ class LVMTestCase(unittest.TestCase):
else:
BlockDev.reinit([cls.ps, cls.ps2], True, None)
+ try:
+ cls.devices_avail = BlockDev.lvm_is_tech_avail(BlockDev.LVMTech.DEVICES, 0)
+ except:
+ cls.devices_avail = False
+
@classmethod
def _get_lvm_version(cls):
_ret, out, _err = run_command("lvm version")
@@ -61,8 +66,7 @@ class LVMTestCase(unittest.TestCase):
@unittest.skipUnless(lvm_dbus_running, "LVM DBus not running")
class LvmNoDevTestCase(LVMTestCase):
- def __init__(self, *args, **kwargs):
- super(LvmNoDevTestCase, self).__init__(*args, **kwargs)
+ def setUp(self):
self._log = ""
@tag_test(TestTags.NOSTORAGE)
@@ -250,6 +254,45 @@ class LvmNoDevTestCase(LVMTestCase):
succ = BlockDev.lvm_set_global_config(None)
self.assertTrue(succ)
+ @tag_test(TestTags.NOSTORAGE)
+ def test_get_set_global_devices_filter(self):
+ """Verify that getting and setting LVM devices filter works as expected"""
+ if not self.devices_avail:
+ self.skipTest("skipping LVM devices filter test: not supported")
+
+ # setup logging
+ self.assertTrue(BlockDev.reinit([self.ps], False, self._store_log))
+
+ # no global config set initially
+ self.assertListEqual(BlockDev.lvm_get_devices_filter(), [])
+
+ # set and try to get back
+ succ = BlockDev.lvm_set_devices_filter(["/dev/sda"])
+ self.assertTrue(succ)
+ self.assertListEqual(BlockDev.lvm_get_devices_filter(), ["/dev/sda"])
+
+ # reset and try to get back
+ succ = BlockDev.lvm_set_devices_filter(None)
+ self.assertTrue(succ)
+ self.assertListEqual(BlockDev.lvm_get_devices_filter(), [])
+
+ # set twice and try to get back twice
+ succ = BlockDev.lvm_set_devices_filter(["/dev/sda"])
+ self.assertTrue(succ)
+ succ = BlockDev.lvm_set_devices_filter(["/dev/sdb"])
+ self.assertTrue(succ)
+ self.assertEqual(BlockDev.lvm_get_devices_filter(), ["/dev/sdb"])
+
+ # set something sane and check it's really used
+ succ = BlockDev.lvm_set_devices_filter(["/dev/sdb", "/dev/sdc"])
+ BlockDev.lvm_pvscan()
+ self.assertIn("'--devices'", self._log)
+ self.assertIn("'/dev/sdb,/dev/sdc'", self._log)
+
+ # reset back to default
+ succ = BlockDev.lvm_set_devices_filter(None)
+ self.assertTrue(succ)
+
@tag_test(TestTags.NOSTORAGE)
def test_cache_get_default_md_size(self):
"""Verify that default cache metadata size is calculated properly"""
diff --git a/tests/lvm_test.py b/tests/lvm_test.py
index 7be8f1ab..11d8c94e 100644
--- a/tests/lvm_test.py
+++ b/tests/lvm_test.py
@@ -39,10 +39,17 @@ class LVMTestCase(unittest.TestCase):
ps.so_name = "libbd_lvm.so.2"
cls.requested_plugins = [ps]
+ BlockDev.switch_init_checks(False)
if not BlockDev.is_initialized():
BlockDev.init(cls.requested_plugins, None)
else:
BlockDev.reinit(cls.requested_plugins, True, None)
+ BlockDev.switch_init_checks(True)
+
+ try:
+ cls.devices_avail = BlockDev.lvm_is_tech_avail(BlockDev.LVMTech.DEVICES, 0)
+ except:
+ cls.devices_avail = False
@classmethod
def _get_lvm_version(cls):
@@ -56,6 +63,8 @@ class LVMTestCase(unittest.TestCase):
class LvmNoDevTestCase(LVMTestCase):
def __init__(self, *args, **kwargs):
super(LvmNoDevTestCase, self).__init__(*args, **kwargs)
+
+ def setUp(self):
self._log = ""
@tag_test(TestTags.NOSTORAGE)
@@ -236,6 +245,44 @@ class LvmNoDevTestCase(LVMTestCase):
succ = BlockDev.lvm_set_global_config(None)
self.assertTrue(succ)
+ @tag_test(TestTags.NOSTORAGE)
+ def test_get_set_global_devices_filter(self):
+ """Verify that getting and setting LVM devices filter works as expected"""
+ if not self.devices_avail:
+ self.skipTest("skipping LVM devices filter test: not supported")
+
+ # setup logging
+ self.assertTrue(BlockDev.reinit(self.requested_plugins, False, self._store_log))
+
+ # no global config set initially
+ self.assertListEqual(BlockDev.lvm_get_devices_filter(), [])
+
+ # set and try to get back
+ succ = BlockDev.lvm_set_devices_filter(["/dev/sda"])
+ self.assertTrue(succ)
+ self.assertListEqual(BlockDev.lvm_get_devices_filter(), ["/dev/sda"])
+
+ # reset and try to get back
+ succ = BlockDev.lvm_set_devices_filter(None)
+ self.assertTrue(succ)
+ self.assertListEqual(BlockDev.lvm_get_devices_filter(), [])
+
+ # set twice and try to get back twice
+ succ = BlockDev.lvm_set_devices_filter(["/dev/sda"])
+ self.assertTrue(succ)
+ succ = BlockDev.lvm_set_devices_filter(["/dev/sdb"])
+ self.assertTrue(succ)
+ self.assertEqual(BlockDev.lvm_get_devices_filter(), ["/dev/sdb"])
+
+ # set something sane and check it's really used
+ succ = BlockDev.lvm_set_devices_filter(["/dev/sdb", "/dev/sdc"])
+ BlockDev.lvm_lvs(None)
+ self.assertIn("--devices=/dev/sdb,/dev/sdc", self._log)
+
+ # reset back to default
+ succ = BlockDev.lvm_set_devices_filter(None)
+ self.assertTrue(succ)
+
@tag_test(TestTags.NOSTORAGE)
def test_cache_get_default_md_size(self):
"""Verify that default cache metadata size is calculated properly"""
@@ -1406,6 +1453,9 @@ class LvmPVVGcachedThpoolstatsTestCase(LvmPVVGLVTestCase):
class LVMUnloadTest(LVMTestCase):
def setUp(self):
+ if not self.devices_avail:
+ self.skipTest("skipping LVM unload test: missing some LVM dependencies")
+
# make sure the library is initialized with all plugins loaded for other
# tests
self.addCleanup(BlockDev.reinit, self.requested_plugins, True, None)
diff --git a/tests/overrides_test.py b/tests/overrides_test.py
index 8e7f5a5a..d3faf3cf 100644
--- a/tests/overrides_test.py
+++ b/tests/overrides_test.py
@@ -15,10 +15,12 @@ class OverridesTest(unittest.TestCase):
@classmethod
def setUpClass(cls):
+ BlockDev.switch_init_checks(False)
if not BlockDev.is_initialized():
BlockDev.init(cls.requested_plugins, None)
else:
BlockDev.reinit(cls.requested_plugins, True, None)
+ BlockDev.switch_init_checks(True)
class OverridesTestCase(OverridesTest):
@tag_test(TestTags.NOSTORAGE, TestTags.CORE)
@@ -65,7 +67,20 @@ class OverridesTestCase(OverridesTest):
self.assertEqual(BlockDev.lvm_get_thpool_padding(11 * 1024**2),
expected_padding)
-class OverridesUnloadTestCase(OverridesTest):
+class OverridesUnloadTestCase(unittest.TestCase):
+ # all plugins except for 'btrfs', 'fs' and 'mpath' -- these don't have all
+ # the dependencies on CentOS/Debian and we don't need them for this test
+ requested_plugins = BlockDev.plugin_specs_from_names(("crypto", "dm",
+ "kbd", "loop",
+ "mdraid", "part", "swap"))
+
+ @classmethod
+ def setUpClass(cls):
+ if not BlockDev.is_initialized():
+ BlockDev.init(cls.requested_plugins, None)
+ else:
+ BlockDev.reinit(cls.requested_plugins, True, None)
+
def tearDown(self):
# make sure the library is initialized with all plugins loaded for other
# tests
@@ -80,7 +95,7 @@ class OverridesUnloadTestCase(OverridesTest):
# no longer loaded
with self.assertRaises(BlockDev.BlockDevNotImplementedError):
- BlockDev.lvm.get_max_lv_size()
+ BlockDev.md.canonicalize_uuid("3386ff85:f5012621:4a435f06:1eb47236")
# load the plugins back
self.assertTrue(BlockDev.reinit(self.requested_plugins, True, None))
@@ -92,9 +107,9 @@ class OverridesUnloadTestCase(OverridesTest):
# the exception should be properly inherited from two classes
with self.assertRaises(NotImplementedError):
- BlockDev.lvm.get_max_lv_size()
+ BlockDev.md.canonicalize_uuid("3386ff85:f5012621:4a435f06:1eb47236")
with self.assertRaises(BlockDev.BlockDevError):
- BlockDev.lvm.get_max_lv_size()
+ BlockDev.md.canonicalize_uuid("3386ff85:f5012621:4a435f06:1eb47236")
# load the plugins back
self.assertTrue(BlockDev.reinit(self.requested_plugins, True, None))
--
2.37.3
From 707de091b8848b95cc78faa4299119844aab4172 Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
Date: Tue, 13 Jul 2021 13:27:32 +0200
Subject: [PATCH 2/7] lvm: Add functions for managing LVM devices file
Currently covers only --adddev and --deldev from the lvmdevices
command.
---
src/lib/plugin_apis/lvm.api | 26 +++++++++++++++
src/plugins/lvm-dbus.c | 52 +++++++++++++++++++++++++++++
src/plugins/lvm.c | 52 +++++++++++++++++++++++++++++
src/plugins/lvm.h | 3 ++
src/python/gi/overrides/BlockDev.py | 15 +++++++++
tests/lvm_dbus_tests.py | 37 +++++++++++++++++++-
tests/lvm_test.py | 37 +++++++++++++++++++-
7 files changed, 220 insertions(+), 2 deletions(-)
diff --git a/src/lib/plugin_apis/lvm.api b/src/lib/plugin_apis/lvm.api
index 23f68b81..b869afcc 100644
--- a/src/lib/plugin_apis/lvm.api
+++ b/src/lib/plugin_apis/lvm.api
@@ -1685,4 +1685,30 @@ GHashTable* bd_lvm_vdo_get_stats_full (const gchar *vg_name, const gchar *pool_n
*/
BDLVMVDOStats* bd_lvm_vdo_get_stats (const gchar *vg_name, const gchar *pool_name, GError **error);
+/**
+ * bd_lvm_devices_add:
+ * @device: device (PV) to add to the devices file
+ * @devices_file: (allow-none): LVM devices file or %NULL for default
+ * @extra: (allow-none) (array zero-terminated=1): extra options for the lvmdevices command
+ * @error: (out): place to store error (if any)
+ *
+ * Returns: whether the @device was successfully added to @devices_file or not
+ *
+ * Tech category: %BD_LVM_TECH_DEVICES no mode (it is ignored)
+ */
+gboolean bd_lvm_devices_add (const gchar *device, const gchar *devices_file, const BDExtraArg **extra, GError **error);
+
+/**
+ * bd_lvm_devices_delete:
+ * @device: device (PV) to delete from the devices file
+ * @devices_file: (allow-none): LVM devices file or %NULL for default
+ * @extra: (allow-none) (array zero-terminated=1): extra options for the lvmdevices command
+ * @error: (out): place to store error (if any)
+ *
+ * Returns: whether the @device was successfully removed from @devices_file or not
+ *
+ * Tech category: %BD_LVM_TECH_DEVICES no mode (it is ignored)
+ */
+gboolean bd_lvm_devices_delete (const gchar *device, const gchar *devices_file, const BDExtraArg **extra, GError **error);
+
#endif /* BD_LVM_API */
diff --git a/src/plugins/lvm-dbus.c b/src/plugins/lvm-dbus.c
index b47ed0ef..86ca28ca 100644
--- a/src/plugins/lvm-dbus.c
+++ b/src/plugins/lvm-dbus.c
@@ -3950,3 +3950,55 @@ BDLVMVDOStats* bd_lvm_vdo_get_stats (const gchar *vg_name, const gchar *pool_nam
return stats;
}
+
+/**
+ * bd_lvm_devices_add:
+ * @device: device (PV) to add to the devices file
+ * @devices_file: (allow-none): LVM devices file or %NULL for default
+ * @extra: (allow-none) (array zero-terminated=1): extra options for the lvmdevices command
+ * @error: (out): place to store error (if any)
+ *
+ * Returns: whether the @device was successfully added to @devices_file or not
+ *
+ * Tech category: %BD_LVM_TECH_DEVICES no mode (it is ignored)
+ */
+gboolean bd_lvm_devices_add (const gchar *device, const gchar *devices_file, const BDExtraArg **extra, GError **error) {
+ const gchar *args[5] = {"lvmdevices", "--adddev", device, NULL, NULL};
+ g_autofree gchar *devfile = NULL;
+
+ if (!bd_lvm_is_tech_avail (BD_LVM_TECH_DEVICES, 0, error))
+ return FALSE;
+
+ if (devices_file) {
+ devfile = g_strdup_printf ("--devicesfile=%s", devices_file);
+ args[3] = devfile;
+ }
+
+ return bd_utils_exec_and_report_error (args, extra, error);
+}
+
+/**
+ * bd_lvm_devices_delete:
+ * @device: device (PV) to delete from the devices file
+ * @devices_file: (allow-none): LVM devices file or %NULL for default
+ * @extra: (allow-none) (array zero-terminated=1): extra options for the lvmdevices command
+ * @error: (out): place to store error (if any)
+ *
+ * Returns: whether the @device was successfully removed from @devices_file or not
+ *
+ * Tech category: %BD_LVM_TECH_DEVICES no mode (it is ignored)
+ */
+gboolean bd_lvm_devices_delete (const gchar *device, const gchar *devices_file, const BDExtraArg **extra, GError **error) {
+ const gchar *args[5] = {"lvmdevices", "--deldev", device, NULL, NULL};
+ g_autofree gchar *devfile = NULL;
+
+ if (!bd_lvm_is_tech_avail (BD_LVM_TECH_DEVICES, 0, error))
+ return FALSE;
+
+ if (devices_file) {
+ devfile = g_strdup_printf ("--devicesfile=%s", devices_file);
+ args[3] = devfile;
+ }
+
+ return bd_utils_exec_and_report_error (args, extra, error);
+}
diff --git a/src/plugins/lvm.c b/src/plugins/lvm.c
index 42ee0f90..3bd8fae1 100644
--- a/src/plugins/lvm.c
+++ b/src/plugins/lvm.c
@@ -3250,3 +3250,55 @@ BDLVMVDOStats* bd_lvm_vdo_get_stats (const gchar *vg_name, const gchar *pool_nam
return stats;
}
+
+/**
+ * bd_lvm_devices_add:
+ * @device: device (PV) to add to the devices file
+ * @devices_file: (allow-none): LVM devices file or %NULL for default
+ * @extra: (allow-none) (array zero-terminated=1): extra options for the lvmdevices command
+ * @error: (out): place to store error (if any)
+ *
+ * Returns: whether the @device was successfully added to @devices_file or not
+ *
+ * Tech category: %BD_LVM_TECH_DEVICES no mode (it is ignored)
+ */
+gboolean bd_lvm_devices_add (const gchar *device, const gchar *devices_file, const BDExtraArg **extra, GError **error) {
+ const gchar *args[5] = {"lvmdevices", "--adddev", device, NULL, NULL};
+ g_autofree gchar *devfile = NULL;
+
+ if (!bd_lvm_is_tech_avail (BD_LVM_TECH_DEVICES, 0, error))
+ return FALSE;
+
+ if (devices_file) {
+ devfile = g_strdup_printf ("--devicesfile=%s", devices_file);
+ args[3] = devfile;
+ }
+
+ return bd_utils_exec_and_report_error (args, extra, error);
+}
+
+/**
+ * bd_lvm_devices_delete:
+ * @device: device (PV) to delete from the devices file
+ * @devices_file: (allow-none): LVM devices file or %NULL for default
+ * @extra: (allow-none) (array zero-terminated=1): extra options for the lvmdevices command
+ * @error: (out): place to store error (if any)
+ *
+ * Returns: whether the @device was successfully removed from @devices_file or not
+ *
+ * Tech category: %BD_LVM_TECH_DEVICES no mode (it is ignored)
+ */
+gboolean bd_lvm_devices_delete (const gchar *device, const gchar *devices_file, const BDExtraArg **extra, GError **error) {
+ const gchar *args[5] = {"lvmdevices", "--deldev", device, NULL, NULL};
+ g_autofree gchar *devfile = NULL;
+
+ if (!bd_lvm_is_tech_avail (BD_LVM_TECH_DEVICES, 0, error))
+ return FALSE;
+
+ if (devices_file) {
+ devfile = g_strdup_printf ("--devicesfile=%s", devices_file);
+ args[3] = devfile;
+ }
+
+ return bd_utils_exec_and_report_error (args, extra, error);
+}
diff --git a/src/plugins/lvm.h b/src/plugins/lvm.h
index 8063693f..5ca2a9d7 100644
--- a/src/plugins/lvm.h
+++ b/src/plugins/lvm.h
@@ -333,4 +333,7 @@ BDLVMVDOWritePolicy bd_lvm_get_vdo_write_policy_from_str (const gchar *policy_st
BDLVMVDOStats* bd_lvm_vdo_get_stats (const gchar *vg_name, const gchar *pool_name, GError **error);
GHashTable* bd_lvm_vdo_get_stats_full (const gchar *vg_name, const gchar *pool_name, GError **error);
+gboolean bd_lvm_devices_add (const gchar *device, const gchar *devices_file, const BDExtraArg **extra, GError **error);
+gboolean bd_lvm_devices_delete (const gchar *device, const gchar *devices_file, const BDExtraArg **extra, GError **error);
+
#endif /* BD_LVM */
diff --git a/src/python/gi/overrides/BlockDev.py b/src/python/gi/overrides/BlockDev.py
index ea059060..8574ab04 100644
--- a/src/python/gi/overrides/BlockDev.py
+++ b/src/python/gi/overrides/BlockDev.py
@@ -724,6 +724,21 @@ def lvm_vdo_pool_convert(vg_name, lv_name, pool_name, virtual_size, index_memory
return _lvm_vdo_pool_convert(vg_name, lv_name, pool_name, virtual_size, index_memory, compression, deduplication, write_policy, extra)
__all__.append("lvm_vdo_pool_convert")
+_lvm_devices_add = BlockDev.lvm_devices_add
+@override(BlockDev.lvm_devices_add)
+def lvm_devices_add(device, devices_file=None, extra=None, **kwargs):
+ extra = _get_extra(extra, kwargs)
+ return _lvm_devices_add(device, devices_file, extra)
+__all__.append("lvm_devices_add")
+
+_lvm_devices_delete = BlockDev.lvm_devices_delete
+@override(BlockDev.lvm_devices_delete)
+def lvm_devices_delete(device, devices_file=None, extra=None, **kwargs):
+ extra = _get_extra(extra, kwargs)
+ return _lvm_devices_delete(device, devices_file, extra)
+__all__.append("lvm_devices_delete")
+
+
_md_get_superblock_size = BlockDev.md_get_superblock_size
@override(BlockDev.md_get_superblock_size)
def md_get_superblock_size(size, version=None):
diff --git a/tests/lvm_dbus_tests.py b/tests/lvm_dbus_tests.py
index ae26c6d2..82e4761d 100644
--- a/tests/lvm_dbus_tests.py
+++ b/tests/lvm_dbus_tests.py
@@ -12,7 +12,7 @@ from contextlib import contextmanager
from distutils.version import LooseVersion
from itertools import chain
-from utils import create_sparse_tempfile, create_lio_device, delete_lio_device, run_command, TestTags, tag_test
+from utils import create_sparse_tempfile, create_lio_device, delete_lio_device, run_command, TestTags, tag_test, read_file
from gi.repository import BlockDev, GLib
import dbus
@@ -1785,3 +1785,38 @@ class LvmConfigTestPvremove(LvmPVonlyTestCase):
BlockDev.lvm_set_global_config("")
succ = BlockDev.lvm_pvremove(self.loop_dev)
self.assertTrue(succ)
+
+
+class LvmTestDevicesFile(LvmPVonlyTestCase):
+ devicefile = "bd_lvm_dbus_tests.devices"
+
+ @classmethod
+ def tearDownClass(cls):
+ shutil.rmtree("/etc/lvm/devices/" + cls.devicefile, ignore_errors=True)
+
+ super(LvmTestDevicesFile, cls).tearDownClass()
+
+ def test_devices_add_delete(self):
+ if not self.devices_avail:
+ self.skipTest("skipping LVM devices file test: not supported")
+
+ succ = BlockDev.lvm_pvcreate(self.loop_dev)
+ self.assertTrue(succ)
+
+ with self.assertRaises(GLib.GError):
+ BlockDev.lvm_devices_add("/non/existing/device", self.devicefile)
+
+ with self.assertRaises(GLib.GError):
+ BlockDev.lvm_devices_delete(self.loop_dev, self.devicefile)
+
+ succ = BlockDev.lvm_devices_add(self.loop_dev, self.devicefile)
+ self.assertTrue(succ)
+
+ dfile = read_file("/etc/lvm/devices/" + self.devicefile)
+ self.assertIn(self.loop_dev, dfile)
+
+ succ = BlockDev.lvm_devices_delete(self.loop_dev, self.devicefile)
+ self.assertTrue(succ)
+
+ dfile = read_file("/etc/lvm/devices/" + self.devicefile)
+ self.assertNotIn(self.loop_dev, dfile)
diff --git a/tests/lvm_test.py b/tests/lvm_test.py
index 11d8c94e..6ddeaa6a 100644
--- a/tests/lvm_test.py
+++ b/tests/lvm_test.py
@@ -11,7 +11,7 @@ import time
from contextlib import contextmanager
from distutils.version import LooseVersion
-from utils import create_sparse_tempfile, create_lio_device, delete_lio_device, fake_utils, fake_path, TestTags, tag_test, run_command
+from utils import create_sparse_tempfile, create_lio_device, delete_lio_device, fake_utils, fake_path, TestTags, tag_test, run_command, read_file
from gi.repository import BlockDev, GLib
@@ -1765,3 +1765,38 @@ class LvmConfigTestPvremove(LvmPVonlyTestCase):
BlockDev.lvm_set_global_config("")
succ = BlockDev.lvm_pvremove(self.loop_dev)
self.assertTrue(succ)
+
+
+class LvmTestDevicesFile(LvmPVonlyTestCase):
+ devicefile = "bd_lvm_test.devices"
+
+ @classmethod
+ def tearDownClass(cls):
+ shutil.rmtree("/etc/lvm/devices/" + cls.devicefile, ignore_errors=True)
+
+ super(LvmTestDevicesFile, cls).tearDownClass()
+
+ def test_devices_add_delete(self):
+ if not self.devices_avail:
+ self.skipTest("skipping LVM devices file test: not supported")
+
+ succ = BlockDev.lvm_pvcreate(self.loop_dev)
+ self.assertTrue(succ)
+
+ with self.assertRaises(GLib.GError):
+ BlockDev.lvm_devices_add("/non/existing/device", self.devicefile)
+
+ with self.assertRaises(GLib.GError):
+ BlockDev.lvm_devices_delete(self.loop_dev, self.devicefile)
+
+ succ = BlockDev.lvm_devices_add(self.loop_dev, self.devicefile)
+ self.assertTrue(succ)
+
+ dfile = read_file("/etc/lvm/devices/" + self.devicefile)
+ self.assertIn(self.loop_dev, dfile)
+
+ succ = BlockDev.lvm_devices_delete(self.loop_dev, self.devicefile)
+ self.assertTrue(succ)
+
+ dfile = read_file("/etc/lvm/devices/" + self.devicefile)
+ self.assertNotIn(self.loop_dev, dfile)
--
2.37.3
From 4c832576df8918c269db8fe2cb7eb74e45628d6c Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
Date: Fri, 15 Oct 2021 13:18:54 +0200
Subject: [PATCH 3/7] lvm: Report special error when system.devices file is not
enabled
This can be disabled either in LVM by a compile time option or
by a lvm.conf option so we should report a specific error for this
case so users can distinguish between the feature not being enabled
and not being supported at all.
---
src/lib/plugin_apis/lvm.api | 1 +
src/plugins/lvm-dbus.c | 70 +++++++++++++++++++++++++++++++++++++
src/plugins/lvm.c | 60 +++++++++++++++++++++++++++++++
src/plugins/lvm.h | 1 +
tests/lvm_dbus_tests.py | 15 ++++++++
tests/lvm_test.py | 15 ++++++++
6 files changed, 162 insertions(+)
diff --git a/src/lib/plugin_apis/lvm.api b/src/lib/plugin_apis/lvm.api
index b869afcc..b8cde70b 100644
--- a/src/lib/plugin_apis/lvm.api
+++ b/src/lib/plugin_apis/lvm.api
@@ -44,6 +44,7 @@ typedef enum {
BD_LVM_ERROR_FAIL,
BD_LVM_ERROR_NOT_SUPPORTED,
BD_LVM_ERROR_VDO_POLICY_INVAL,
+ BD_LVM_ERROR_DEVICES_DISABLED,
} BDLVMError;
typedef enum {
diff --git a/src/plugins/lvm-dbus.c b/src/plugins/lvm-dbus.c
index 86ca28ca..7f48e422 100644
--- a/src/plugins/lvm-dbus.c
+++ b/src/plugins/lvm-dbus.c
@@ -3951,6 +3951,64 @@ BDLVMVDOStats* bd_lvm_vdo_get_stats (const gchar *vg_name, const gchar *pool_nam
return stats;
}
+/* check whether the LVM devices file is enabled by LVM
+ * we use the existence of the "lvmdevices" command to check whether the feature is available
+ * or not, but this can still be disabled either in LVM or in lvm.conf
+ */
+static gboolean _lvm_devices_enabled () {
+ const gchar *args[6] = {"lvmconfig", "--typeconfig", NULL, "devices/use_devicesfile", NULL, NULL};
+ gboolean ret = FALSE;
+ GError *loc_error = NULL;
+ gchar *output = NULL;
+ gboolean enabled = FALSE;
+ gint scanned = 0;
+ g_autofree gchar *config_arg = NULL;
+
+ /* try current config first -- if we get something from this it means the feature is
+ explicitly enabled or disabled by system lvm.conf or using the --config option */
+ args[2] = "current";
+
+ /* make sure to include the global config from us when getting the current config value */
+ g_mutex_lock (&global_config_lock);
+ if (global_config_str) {
+ config_arg = g_strdup_printf ("--config=%s", global_config_str);
+ args[4] = config_arg;
+ }
+
+ ret = bd_utils_exec_and_capture_output (args, NULL, &output, &loc_error);
+ g_mutex_unlock (&global_config_lock);
+ if (ret) {
+ scanned = sscanf (output, "use_devicesfile=%u", &enabled);
+ g_free (output);
+ if (scanned != 1)
+ return FALSE;
+
+ return enabled;
+ } else {
+ g_clear_error (&loc_error);
+ g_free (output);
+ }
+
+ output = NULL;
+
+ /* now try default */
+ args[2] = "default";
+ ret = bd_utils_exec_and_capture_output (args, NULL, &output, &loc_error);
+ if (ret) {
+ scanned = sscanf (output, "# use_devicesfile=%u", &enabled);
+ g_free (output);
+ if (scanned != 1)
+ return FALSE;
+
+ return enabled;
+ } else {
+ g_clear_error (&loc_error);
+ g_free (output);
+ }
+
+ return FALSE;
+}
+
/**
* bd_lvm_devices_add:
* @device: device (PV) to add to the devices file
@@ -3969,6 +4027,12 @@ gboolean bd_lvm_devices_add (const gchar *device, const gchar *devices_file, con
if (!bd_lvm_is_tech_avail (BD_LVM_TECH_DEVICES, 0, error))
return FALSE;
+ if (!_lvm_devices_enabled ()) {
+ g_set_error (error, BD_LVM_ERROR, BD_LVM_ERROR_DEVICES_DISABLED,
+ "LVM devices file not enabled.");
+ return FALSE;
+ }
+
if (devices_file) {
devfile = g_strdup_printf ("--devicesfile=%s", devices_file);
args[3] = devfile;
@@ -3995,6 +4059,12 @@ gboolean bd_lvm_devices_delete (const gchar *device, const gchar *devices_file,
if (!bd_lvm_is_tech_avail (BD_LVM_TECH_DEVICES, 0, error))
return FALSE;
+ if (!_lvm_devices_enabled ()) {
+ g_set_error (error, BD_LVM_ERROR, BD_LVM_ERROR_DEVICES_DISABLED,
+ "LVM devices file not enabled.");
+ return FALSE;
+ }
+
if (devices_file) {
devfile = g_strdup_printf ("--devicesfile=%s", devices_file);
args[3] = devfile;
diff --git a/src/plugins/lvm.c b/src/plugins/lvm.c
index 3bd8fae1..73d5005f 100644
--- a/src/plugins/lvm.c
+++ b/src/plugins/lvm.c
@@ -3251,6 +3251,54 @@ BDLVMVDOStats* bd_lvm_vdo_get_stats (const gchar *vg_name, const gchar *pool_nam
return stats;
}
+/* check whether the LVM devices file is enabled by LVM
+ * we use the existence of the "lvmdevices" command to check whether the feature is available
+ * or not, but this can still be disabled either in LVM or in lvm.conf
+ */
+static gboolean _lvm_devices_enabled () {
+ const gchar *args[5] = {"config", "--typeconfig", NULL, "devices/use_devicesfile", NULL};
+ gboolean ret = FALSE;
+ GError *loc_error = NULL;
+ gchar *output = NULL;
+ gboolean enabled = FALSE;
+ gint scanned = 0;
+
+ /* try current config first -- if we get something from this it means the feature is
+ explicitly enabled or disabled by system lvm.conf or using the --config option */
+ args[2] = "current";
+ ret = call_lvm_and_capture_output (args, NULL, &output, &loc_error);
+ if (ret) {
+ scanned = sscanf (output, "use_devicesfile=%u", &enabled);
+ g_free (output);
+ if (scanned != 1)
+ return FALSE;
+
+ return enabled;
+ } else {
+ g_clear_error (&loc_error);
+ g_free (output);
+ }
+
+ output = NULL;
+
+ /* now try default */
+ args[2] = "default";
+ ret = call_lvm_and_capture_output (args, NULL, &output, &loc_error);
+ if (ret) {
+ scanned = sscanf (output, "# use_devicesfile=%u", &enabled);
+ g_free (output);
+ if (scanned != 1)
+ return FALSE;
+
+ return enabled;
+ } else {
+ g_clear_error (&loc_error);
+ g_free (output);
+ }
+
+ return FALSE;
+}
+
/**
* bd_lvm_devices_add:
* @device: device (PV) to add to the devices file
@@ -3269,6 +3317,12 @@ gboolean bd_lvm_devices_add (const gchar *device, const gchar *devices_file, con
if (!bd_lvm_is_tech_avail (BD_LVM_TECH_DEVICES, 0, error))
return FALSE;
+ if (!_lvm_devices_enabled ()) {
+ g_set_error (error, BD_LVM_ERROR, BD_LVM_ERROR_DEVICES_DISABLED,
+ "LVM devices file not enabled.");
+ return FALSE;
+ }
+
if (devices_file) {
devfile = g_strdup_printf ("--devicesfile=%s", devices_file);
args[3] = devfile;
@@ -3295,6 +3349,12 @@ gboolean bd_lvm_devices_delete (const gchar *device, const gchar *devices_file,
if (!bd_lvm_is_tech_avail (BD_LVM_TECH_DEVICES, 0, error))
return FALSE;
+ if (!_lvm_devices_enabled ()) {
+ g_set_error (error, BD_LVM_ERROR, BD_LVM_ERROR_DEVICES_DISABLED,
+ "LVM devices file not enabled.");
+ return FALSE;
+ }
+
if (devices_file) {
devfile = g_strdup_printf ("--devicesfile=%s", devices_file);
args[3] = devfile;
diff --git a/src/plugins/lvm.h b/src/plugins/lvm.h
index 5ca2a9d7..fabf091f 100644
--- a/src/plugins/lvm.h
+++ b/src/plugins/lvm.h
@@ -53,6 +53,7 @@ typedef enum {
BD_LVM_ERROR_FAIL,
BD_LVM_ERROR_NOT_SUPPORTED,
BD_LVM_ERROR_VDO_POLICY_INVAL,
+ BD_LVM_ERROR_DEVICES_DISABLED,
} BDLVMError;
typedef enum {
diff --git a/tests/lvm_dbus_tests.py b/tests/lvm_dbus_tests.py
index 82e4761d..792c1cc8 100644
--- a/tests/lvm_dbus_tests.py
+++ b/tests/lvm_dbus_tests.py
@@ -1820,3 +1820,18 @@ class LvmTestDevicesFile(LvmPVonlyTestCase):
dfile = read_file("/etc/lvm/devices/" + self.devicefile)
self.assertNotIn(self.loop_dev, dfile)
+
+ def test_devices_enabled(self):
+ if not self.devices_avail:
+ self.skipTest("skipping LVM devices file test: not supported")
+
+ self.addCleanup(BlockDev.lvm_set_global_config, "")
+
+ # checking if the feature is enabled or disabled is hard so lets just disable
+ # the devices file using the global config and check lvm_devices_add fails
+ # with the correct exception message
+ succ = BlockDev.lvm_set_global_config("devices { use_devicesfile=0 }")
+ self.assertTrue(succ)
+
+ with self.assertRaisesRegex(GLib.GError, "LVM devices file not enabled."):
+ BlockDev.lvm_devices_add("", self.devicefile)
diff --git a/tests/lvm_test.py b/tests/lvm_test.py
index 6ddeaa6a..73fb1030 100644
--- a/tests/lvm_test.py
+++ b/tests/lvm_test.py
@@ -1800,3 +1800,18 @@ class LvmTestDevicesFile(LvmPVonlyTestCase):
dfile = read_file("/etc/lvm/devices/" + self.devicefile)
self.assertNotIn(self.loop_dev, dfile)
+
+ def test_devices_enabled(self):
+ if not self.devices_avail:
+ self.skipTest("skipping LVM devices file test: not supported")
+
+ self.addCleanup(BlockDev.lvm_set_global_config, "")
+
+ # checking if the feature is enabled or disabled is hard so lets just disable
+ # the devices file using the global config and check lvm_devices_add fails
+ # with the correct exception message
+ succ = BlockDev.lvm_set_global_config("devices { use_devicesfile=0 }")
+ self.assertTrue(succ)
+
+ with self.assertRaisesRegex(GLib.GError, "LVM devices file not enabled."):
+ BlockDev.lvm_devices_add("", self.devicefile)
--
2.37.3
From 2fdec5f7e42de869d4b2ec80dce597d22dd57617 Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
Date: Fri, 15 Oct 2021 14:21:03 +0200
Subject: [PATCH 4/7] lvm: Force enable LVM devices file for LvmTestDevicesFile
This feauture might be disabled in lvm.conf so to be able to test
it we need to override this. The correct handling of the disabled
state is checked in a separate test case.
---
tests/lvm_dbus_tests.py | 8 ++++++++
tests/lvm_test.py | 8 ++++++++
2 files changed, 16 insertions(+)
diff --git a/tests/lvm_dbus_tests.py b/tests/lvm_dbus_tests.py
index 792c1cc8..e55535cc 100644
--- a/tests/lvm_dbus_tests.py
+++ b/tests/lvm_dbus_tests.py
@@ -1800,6 +1800,12 @@ class LvmTestDevicesFile(LvmPVonlyTestCase):
if not self.devices_avail:
self.skipTest("skipping LVM devices file test: not supported")
+ self.addCleanup(BlockDev.lvm_set_global_config, "")
+
+ # force-enable the feature, it might be disabled by default
+ succ = BlockDev.lvm_set_global_config("devices { use_devicesfile=1 }")
+ self.assertTrue(succ)
+
succ = BlockDev.lvm_pvcreate(self.loop_dev)
self.assertTrue(succ)
@@ -1821,6 +1827,8 @@ class LvmTestDevicesFile(LvmPVonlyTestCase):
dfile = read_file("/etc/lvm/devices/" + self.devicefile)
self.assertNotIn(self.loop_dev, dfile)
+ BlockDev.lvm_set_global_config("")
+
def test_devices_enabled(self):
if not self.devices_avail:
self.skipTest("skipping LVM devices file test: not supported")
diff --git a/tests/lvm_test.py b/tests/lvm_test.py
index 73fb1030..907b4f59 100644
--- a/tests/lvm_test.py
+++ b/tests/lvm_test.py
@@ -1780,6 +1780,12 @@ class LvmTestDevicesFile(LvmPVonlyTestCase):
if not self.devices_avail:
self.skipTest("skipping LVM devices file test: not supported")
+ self.addCleanup(BlockDev.lvm_set_global_config, "")
+
+ # force-enable the feature, it might be disabled by default
+ succ = BlockDev.lvm_set_global_config("devices { use_devicesfile=1 }")
+ self.assertTrue(succ)
+
succ = BlockDev.lvm_pvcreate(self.loop_dev)
self.assertTrue(succ)
@@ -1801,6 +1807,8 @@ class LvmTestDevicesFile(LvmPVonlyTestCase):
dfile = read_file("/etc/lvm/devices/" + self.devicefile)
self.assertNotIn(self.loop_dev, dfile)
+ BlockDev.lvm_set_global_config("")
+
def test_devices_enabled(self):
if not self.devices_avail:
self.skipTest("skipping LVM devices file test: not supported")
--
2.37.3
From 1809a41c0b2b99c8d6a077b5aa70834686980181 Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
Date: Fri, 12 Nov 2021 14:51:39 +0100
Subject: [PATCH 5/7] tests: Fix resetting global LVM config after LVM devices
file test
We need to set the config to None/NULL not to an empty string.
---
tests/lvm_dbus_tests.py | 6 +++---
tests/lvm_test.py | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tests/lvm_dbus_tests.py b/tests/lvm_dbus_tests.py
index e55535cc..8ae670d5 100644
--- a/tests/lvm_dbus_tests.py
+++ b/tests/lvm_dbus_tests.py
@@ -1800,7 +1800,7 @@ class LvmTestDevicesFile(LvmPVonlyTestCase):
if not self.devices_avail:
self.skipTest("skipping LVM devices file test: not supported")
- self.addCleanup(BlockDev.lvm_set_global_config, "")
+ self.addCleanup(BlockDev.lvm_set_global_config, None)
# force-enable the feature, it might be disabled by default
succ = BlockDev.lvm_set_global_config("devices { use_devicesfile=1 }")
@@ -1827,13 +1827,13 @@ class LvmTestDevicesFile(LvmPVonlyTestCase):
dfile = read_file("/etc/lvm/devices/" + self.devicefile)
self.assertNotIn(self.loop_dev, dfile)
- BlockDev.lvm_set_global_config("")
+ BlockDev.lvm_set_global_config(None)
def test_devices_enabled(self):
if not self.devices_avail:
self.skipTest("skipping LVM devices file test: not supported")
- self.addCleanup(BlockDev.lvm_set_global_config, "")
+ self.addCleanup(BlockDev.lvm_set_global_config, None)
# checking if the feature is enabled or disabled is hard so lets just disable
# the devices file using the global config and check lvm_devices_add fails
diff --git a/tests/lvm_test.py b/tests/lvm_test.py
index 907b4f59..095e4bac 100644
--- a/tests/lvm_test.py
+++ b/tests/lvm_test.py
@@ -1780,7 +1780,7 @@ class LvmTestDevicesFile(LvmPVonlyTestCase):
if not self.devices_avail:
self.skipTest("skipping LVM devices file test: not supported")
- self.addCleanup(BlockDev.lvm_set_global_config, "")
+ self.addCleanup(BlockDev.lvm_set_global_config, None)
# force-enable the feature, it might be disabled by default
succ = BlockDev.lvm_set_global_config("devices { use_devicesfile=1 }")
@@ -1807,13 +1807,13 @@ class LvmTestDevicesFile(LvmPVonlyTestCase):
dfile = read_file("/etc/lvm/devices/" + self.devicefile)
self.assertNotIn(self.loop_dev, dfile)
- BlockDev.lvm_set_global_config("")
+ BlockDev.lvm_set_global_config(None)
def test_devices_enabled(self):
if not self.devices_avail:
self.skipTest("skipping LVM devices file test: not supported")
- self.addCleanup(BlockDev.lvm_set_global_config, "")
+ self.addCleanup(BlockDev.lvm_set_global_config, None)
# checking if the feature is enabled or disabled is hard so lets just disable
# the devices file using the global config and check lvm_devices_add fails
--
2.37.3
From 1c2f1d20a3cfa522b78ab007e8e4f9a5a4bb579d Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
Date: Fri, 12 Nov 2021 15:10:45 +0100
Subject: [PATCH 6/7] lvm: Do not set global config to and empty string
If we set it to an empty string we end up running "--config"
without a parameter and lvm will use whatever is next parameter
like the device path for pvremove.
---
tests/lvm_dbus_tests.py | 12 ++++++++++++
tests/lvm_test.py | 12 ++++++++++++
2 files changed, 24 insertions(+)
diff --git a/tests/lvm_dbus_tests.py b/tests/lvm_dbus_tests.py
index 8ae670d5..61c898c1 100644
--- a/tests/lvm_dbus_tests.py
+++ b/tests/lvm_dbus_tests.py
@@ -1843,3 +1843,15 @@ class LvmTestDevicesFile(LvmPVonlyTestCase):
with self.assertRaisesRegex(GLib.GError, "LVM devices file not enabled."):
BlockDev.lvm_devices_add("", self.devicefile)
+
+
+class LvmConfigTestPvremove(LvmPVonlyTestCase):
+
+ @tag_test(TestTags.REGRESSION)
+ def test_set_empty_config(self):
+ succ = BlockDev.lvm_pvcreate(self.loop_dev)
+ self.assertTrue(succ)
+
+ BlockDev.lvm_set_global_config("")
+ succ = BlockDev.lvm_pvremove(self.loop_dev)
+ self.assertTrue(succ)
diff --git a/tests/lvm_test.py b/tests/lvm_test.py
index 095e4bac..36ff10ec 100644
--- a/tests/lvm_test.py
+++ b/tests/lvm_test.py
@@ -1823,3 +1823,15 @@ class LvmTestDevicesFile(LvmPVonlyTestCase):
with self.assertRaisesRegex(GLib.GError, "LVM devices file not enabled."):
BlockDev.lvm_devices_add("", self.devicefile)
+
+
+class LvmConfigTestPvremove(LvmPVonlyTestCase):
+
+ @tag_test(TestTags.REGRESSION)
+ def test_set_empty_config(self):
+ succ = BlockDev.lvm_pvcreate(self.loop_dev)
+ self.assertTrue(succ)
+
+ BlockDev.lvm_set_global_config("")
+ succ = BlockDev.lvm_pvremove(self.loop_dev)
+ self.assertTrue(succ)
--
2.37.3
From 05cfb84777c5472550673a1f2150ca357718b3f2 Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
Date: Fri, 26 Nov 2021 15:19:55 +0100
Subject: [PATCH 7/7] lvm: Use "lvmconfig full" to get valid config instead of
"current"
"lvmconfig current" doesn't work together with --config even if we
don't override the "use_devicefile" key. "lvmconfig full" seems to
be working in all cases.
---
src/plugins/lvm-dbus.c | 4 ++--
src/plugins/lvm.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/plugins/lvm-dbus.c b/src/plugins/lvm-dbus.c
index 7f48e422..d4b542e2 100644
--- a/src/plugins/lvm-dbus.c
+++ b/src/plugins/lvm-dbus.c
@@ -3964,9 +3964,9 @@ static gboolean _lvm_devices_enabled () {
gint scanned = 0;
g_autofree gchar *config_arg = NULL;
- /* try current config first -- if we get something from this it means the feature is
+ /* try full config first -- if we get something from this it means the feature is
explicitly enabled or disabled by system lvm.conf or using the --config option */
- args[2] = "current";
+ args[2] = "full";
/* make sure to include the global config from us when getting the current config value */
g_mutex_lock (&global_config_lock);
diff --git a/src/plugins/lvm.c b/src/plugins/lvm.c
index 73d5005f..03211f8a 100644
--- a/src/plugins/lvm.c
+++ b/src/plugins/lvm.c
@@ -3263,9 +3263,9 @@ static gboolean _lvm_devices_enabled () {
gboolean enabled = FALSE;
gint scanned = 0;
- /* try current config first -- if we get something from this it means the feature is
+ /* try full config first -- if we get something from this it means the feature is
explicitly enabled or disabled by system lvm.conf or using the --config option */
- args[2] = "current";
+ args[2] = "full";
ret = call_lvm_and_capture_output (args, NULL, &output, &loc_error);
if (ret) {
scanned = sscanf (output, "use_devicesfile=%u", &enabled);
--
2.37.3