From e364883416785d51ff8eb132b63bd802ab0ccfe9 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/8] 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 | 74 ++++++++-
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, 469 insertions(+), 153 deletions(-)
diff --git a/src/lib/plugin_apis/lvm.api b/src/lib/plugin_apis/lvm.api
index 563c104..62f602f 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 144551f..d1726ed 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"
@@ -247,6 +249,14 @@ 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
@@ -385,6 +395,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);
@@ -522,6 +534,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;
@@ -543,8 +556,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);
@@ -565,6 +578,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);
@@ -2651,6 +2669,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 2be1dbd..c0d8198 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;
@@ -2018,6 +2041,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 2162d76..8063693 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 08e44fd..efd17bd 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 4882da8..35ace37 100644
--- a/tests/lvm_dbus_tests.py
+++ b/tests/lvm_dbus_tests.py
@@ -33,6 +33,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")
@@ -44,8 +49,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)
@@ -227,6 +231,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 eb94c91..b37a879 100644
--- a/tests/lvm_test.py
+++ b/tests/lvm_test.py
@@ -22,10 +22,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):
@@ -39,6 +46,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)
@@ -213,6 +222,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"""
@@ -1335,6 +1382,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 8e7f5a5..d3faf3c 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.31.1
From bebd74962db6fb7b5314be411a4d02b21554d80f 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/8] 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 62f602f..bce2920 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 d1726ed..44d2794 100644
--- a/src/plugins/lvm-dbus.c
+++ b/src/plugins/lvm-dbus.c
@@ -3938,3 +3938,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 c0d8198..94c6a22 100644
--- a/src/plugins/lvm.c
+++ b/src/plugins/lvm.c
@@ -3235,3 +3235,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 8063693..5ca2a9d 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 f768c8b..715a262 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 35ace37..fb1a9ed 100644
--- a/tests/lvm_dbus_tests.py
+++ b/tests/lvm_dbus_tests.py
@@ -10,7 +10,7 @@ import subprocess
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
@@ -1696,3 +1696,38 @@ class LVMVDOTest(LVMTestCase):
full_stats = BlockDev.lvm_vdo_get_stats_full("testVDOVG", "vdoPool")
self.assertIn("writeAmplificationRatio", full_stats.keys())
+
+
+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 b37a879..786434f 100644
--- a/tests/lvm_test.py
+++ b/tests/lvm_test.py
@@ -9,7 +9,7 @@ import shutil
import subprocess
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
@@ -1682,3 +1682,38 @@ class LVMVDOTest(LVMTestCase):
full_stats = BlockDev.lvm_vdo_get_stats_full("testVDOVG", "vdoPool")
self.assertIn("writeAmplificationRatio", full_stats.keys())
+
+
+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.31.1
From 8d8cbe7169cb94b01e7064a0d00b7d86baf5e652 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/8] 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 bce2920..b96bcfd 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 44d2794..22204d5 100644
--- a/src/plugins/lvm-dbus.c
+++ b/src/plugins/lvm-dbus.c
@@ -3939,6 +3939,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
@@ -3957,6 +4015,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;
@@ -3983,6 +4047,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 94c6a22..605fcb0 100644
--- a/src/plugins/lvm.c
+++ b/src/plugins/lvm.c
@@ -3236,6 +3236,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
@@ -3254,6 +3302,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;
@@ -3280,6 +3334,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 5ca2a9d..fabf091 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 fb1a9ed..c411c9e 100644
--- a/tests/lvm_dbus_tests.py
+++ b/tests/lvm_dbus_tests.py
@@ -1731,3 +1731,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 786434f..315dd07 100644
--- a/tests/lvm_test.py
+++ b/tests/lvm_test.py
@@ -1717,3 +1717,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.31.1
From 81df85e7ea6e129e78074b6967f80c505d1b08f0 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/8] 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 c411c9e..9cfc647 100644
--- a/tests/lvm_dbus_tests.py
+++ b/tests/lvm_dbus_tests.py
@@ -1711,6 +1711,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)
@@ -1732,6 +1738,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 315dd07..ea3b7f8 100644
--- a/tests/lvm_test.py
+++ b/tests/lvm_test.py
@@ -1697,6 +1697,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)
@@ -1718,6 +1724,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.31.1
From 01237b62bb8ad67def7c937185c42152503fbc6f 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/8] 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 9cfc647..d422869 100644
--- a/tests/lvm_dbus_tests.py
+++ b/tests/lvm_dbus_tests.py
@@ -1711,7 +1711,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 }")
@@ -1738,13 +1738,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 ea3b7f8..882cdf2 100644
--- a/tests/lvm_test.py
+++ b/tests/lvm_test.py
@@ -1697,7 +1697,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 }")
@@ -1724,13 +1724,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.31.1
From 2f33f2af18efa0b337f8383cb6f137d6211fe7fb 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/8] 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.
---
src/plugins/lvm-dbus.c | 5 ++++-
src/plugins/lvm.c | 5 ++++-
tests/lvm_dbus_tests.py | 12 ++++++++++++
tests/lvm_test.py | 12 ++++++++++++
4 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/src/plugins/lvm-dbus.c b/src/plugins/lvm-dbus.c
index 22204d5..b7bd019 100644
--- a/src/plugins/lvm-dbus.c
+++ b/src/plugins/lvm-dbus.c
@@ -2644,7 +2644,10 @@ gboolean bd_lvm_set_global_config (const gchar *new_config, GError **error UNUSE
g_free (global_config_str);
/* now store the new one */
- global_config_str = g_strdup (new_config);
+ if (!new_config || g_strcmp0 (new_config, "") == 0)
+ global_config_str = NULL;
+ else
+ global_config_str = g_strdup (new_config);
g_mutex_unlock (&global_config_lock);
return TRUE;
diff --git a/src/plugins/lvm.c b/src/plugins/lvm.c
index 605fcb0..124fce7 100644
--- a/src/plugins/lvm.c
+++ b/src/plugins/lvm.c
@@ -2016,7 +2016,10 @@ gboolean bd_lvm_set_global_config (const gchar *new_config, GError **error UNUSE
g_free (global_config_str);
/* now store the new one */
- global_config_str = g_strdup (new_config);
+ if (!new_config || g_strcmp0 (new_config, "") == 0)
+ global_config_str = NULL;
+ else
+ global_config_str = g_strdup (new_config);
g_mutex_unlock (&global_config_lock);
return TRUE;
diff --git a/tests/lvm_dbus_tests.py b/tests/lvm_dbus_tests.py
index d422869..5516afe 100644
--- a/tests/lvm_dbus_tests.py
+++ b/tests/lvm_dbus_tests.py
@@ -1754,3 +1754,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 882cdf2..e349817 100644
--- a/tests/lvm_test.py
+++ b/tests/lvm_test.py
@@ -1740,3 +1740,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.31.1
From 2a4e610027a2c2a315054b84a323ce973939ca2d Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
Date: Tue, 16 Mar 2021 12:05:37 +0100
Subject: [PATCH 7/8] vdo: Do not use g_memdup in bd_vdo_stats_copy
g_memdup is deprecated and the replacement g_memdup2 is not yet
available so lets just do the copy manually.
---
src/lib/plugin_apis/vdo.api | 17 ++++++++++++++++-
src/plugins/vdo.c | 17 ++++++++++++++++-
2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/src/lib/plugin_apis/vdo.api b/src/lib/plugin_apis/vdo.api
index 936f8e0..312de4e 100644
--- a/src/lib/plugin_apis/vdo.api
+++ b/src/lib/plugin_apis/vdo.api
@@ -170,7 +170,22 @@ void bd_vdo_stats_free (BDVDOStats *stats) {
* Deprecated: 2.24: Use LVM-VDO integration instead.
*/
BDVDOStats* bd_vdo_stats_copy (BDVDOStats *stats) {
- return g_memdup (stats, sizeof (BDVDOStats));
+ if (stats == NULL)
+ return NULL;
+
+ BDVDOStats *new_stats = g_new0 (BDVDOStats, 1);
+
+ new_stats->block_size = stats->block_size;
+ new_stats->logical_block_size = stats->logical_block_size;
+ new_stats->physical_blocks = stats->physical_blocks;
+ new_stats->data_blocks_used = stats->data_blocks_used;
+ new_stats->overhead_blocks_used = stats->overhead_blocks_used;
+ new_stats->logical_blocks_used = stats->logical_blocks_used;
+ new_stats->used_percent = stats->used_percent;
+ new_stats->saving_percent = stats->saving_percent;
+ new_stats->write_amplification_ratio = stats->write_amplification_ratio;
+
+ return new_stats;
}
GType bd_vdo_stats_get_type () {
diff --git a/src/plugins/vdo.c b/src/plugins/vdo.c
index 2352394..d443099 100644
--- a/src/plugins/vdo.c
+++ b/src/plugins/vdo.c
@@ -81,7 +81,22 @@ void bd_vdo_stats_free (BDVDOStats *stats) {
}
BDVDOStats* bd_vdo_stats_copy (BDVDOStats *stats) {
- return g_memdup (stats, sizeof (BDVDOStats));
+ if (stats == NULL)
+ return NULL;
+
+ BDVDOStats *new_stats = g_new0 (BDVDOStats, 1);
+
+ new_stats->block_size = stats->block_size;
+ new_stats->logical_block_size = stats->logical_block_size;
+ new_stats->physical_blocks = stats->physical_blocks;
+ new_stats->data_blocks_used = stats->data_blocks_used;
+ new_stats->overhead_blocks_used = stats->overhead_blocks_used;
+ new_stats->logical_blocks_used = stats->logical_blocks_used;
+ new_stats->used_percent = stats->used_percent;
+ new_stats->saving_percent = stats->saving_percent;
+ new_stats->write_amplification_ratio = stats->write_amplification_ratio;
+
+ return new_stats;
}
--
2.31.1
From 577ea466e3b7af464137e087907ba980ad3994ee Mon Sep 17 00:00:00 2001
From: Vojtech Trefny <vtrefny@redhat.com>
Date: Fri, 26 Nov 2021 15:19:55 +0100
Subject: [PATCH 8/8] 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 b7bd019..825c5e9 100644
--- a/src/plugins/lvm-dbus.c
+++ b/src/plugins/lvm-dbus.c
@@ -3955,9 +3955,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 124fce7..21320f3 100644
--- a/src/plugins/lvm.c
+++ b/src/plugins/lvm.c
@@ -3251,9 +3251,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.31.1