From 25e8336a95e00e629c1afcd56b043c44bc5fd90c Mon Sep 17 00:00:00 2001 From: Marius Vollmer Date: Fri, 8 Apr 2022 16:46:33 +0300 Subject: [PATCH] lvm: Enhancements for LVM2 RAID support This exposes the "missing" attribute of PVs and allows for repairs. --- docs/libblockdev-sections.txt | 1 + src/lib/plugin_apis/lvm.api | 17 +++++++ src/plugins/lvm-dbus.c | 20 ++++++++ src/plugins/lvm.c | 47 ++++++++++++++++-- src/plugins/lvm.h | 2 + tests/lvm_dbus_tests.py | 69 ++++++++++++++++++++++++++ tests/lvm_test.py | 92 +++++++++++++++++++++++++++++++++++ 7 files changed, 244 insertions(+), 4 deletions(-) diff --git a/docs/libblockdev-sections.txt b/docs/libblockdev-sections.txt index 34de14ee3..99db9b178 100644 --- a/docs/libblockdev-sections.txt +++ b/docs/libblockdev-sections.txt @@ -344,6 +344,7 @@ bd_lvm_lvcreate bd_lvm_lvremove bd_lvm_lvrename bd_lvm_lvresize +bd_lvm_lvrepair bd_lvm_lvactivate bd_lvm_lvdeactivate bd_lvm_lvsnapshotcreate diff --git a/src/lib/plugin_apis/lvm.api b/src/lib/plugin_apis/lvm.api index f6fd507ea..b7bd70030 100644 --- a/src/lib/plugin_apis/lvm.api +++ b/src/lib/plugin_apis/lvm.api @@ -123,6 +123,7 @@ typedef struct BDLVMPVdata { guint64 vg_free_count; guint64 vg_pv_count; gchar **pv_tags; + gboolean missing; } BDLVMPVdata; /** @@ -151,6 +152,7 @@ BDLVMPVdata* bd_lvm_pvdata_copy (BDLVMPVdata *data) { new_data->vg_free_count = data->vg_free_count; new_data->vg_pv_count = data->vg_pv_count; new_data->pv_tags = g_strdupv (data->pv_tags); + new_data->missing = data->missing; return new_data; } @@ -1114,6 +1116,21 @@ gboolean bd_lvm_lvrename (const gchar *vg_name, const gchar *lv_name, const gcha */ gboolean bd_lvm_lvresize (const gchar *vg_name, const gchar *lv_name, guint64 size, const BDExtraArg **extra, GError **error); +/** + * bd_lvm_lvrepair: + * @vg_name: name of the VG containing the to-be-repaired LV + * @lv_name: name of the to-be-repaired LV + * @pv_list: (array zero-terminated=1): list of PVs to be used for the repair + * @extra: (nullable) (array zero-terminated=1): extra options for the LV repair + * (just passed to LVM as is) + * @error: (out) (optional): place to store error (if any) + * + * Returns: whether the @vg_name/@lv_name LV was successfully repaired or not + * + * Tech category: %BD_LVM_TECH_BASIC-%BD_LVM_TECH_MODE_MODIFY + */ +gboolean bd_lvm_lvrepair (const gchar *vg_name, const gchar *lv_name, const gchar **pv_list, const BDExtraArg **extra, GError **error); + /** * bd_lvm_lvactivate: * @vg_name: name of the VG containing the to-be-activated LV diff --git a/src/plugins/lvm-dbus.c b/src/plugins/lvm-dbus.c index c37218138..efea64d39 100644 --- a/src/plugins/lvm-dbus.c +++ b/src/plugins/lvm-dbus.c @@ -989,6 +989,7 @@ static BDLVMPVdata* get_pv_data_from_props (GVariant *props, GError **error) { g_variant_dict_lookup (&dict, "FreeBytes", "t", &(data->pv_free)); g_variant_dict_lookup (&dict, "SizeBytes", "t", &(data->pv_size)); g_variant_dict_lookup (&dict, "PeStart", "t", &(data->pe_start)); + g_variant_dict_lookup (&dict, "Missing", "b", &(data->missing)); value = g_variant_dict_lookup_value (&dict, "Tags", (GVariantType*) "as"); if (value) { @@ -2445,6 +2446,25 @@ gboolean bd_lvm_lvresize (const gchar *vg_name, const gchar *lv_name, guint64 si return call_lv_method_sync (vg_name, lv_name, "Resize", params, NULL, extra, TRUE, error); } +/** + * bd_lvm_lvrepair: + * @vg_name: name of the VG containing the to-be-repaired LV + * @lv_name: name of the to-be-repaired LV + * @pv_list: (array zero-terminated=1): list of PVs to be used for the repair + * @extra: (nullable) (array zero-terminated=1): extra options for the LV repair + * (just passed to LVM as is) + * @error: (out) (optional): place to store error (if any) + * + * Returns: whether the @vg_name/@lv_name LV was successfully repaired or not + * + * Tech category: %BD_LVM_TECH_BASIC-%BD_LVM_TECH_MODE_MODIFY + */ +gboolean bd_lvm_lvrepair (const gchar *vg_name UNUSED, const gchar *lv_name UNUSED, const gchar **pv_list UNUSED, const BDExtraArg **extra UNUSED, GError **error) { + g_set_error (error, BD_LVM_ERROR, BD_LVM_ERROR_TECH_UNAVAIL, + "lvrepair is not supported by this plugin implementation."); + return FALSE; +} + /** * bd_lvm_lvactivate: * @vg_name: name of the VG containing the to-be-activated LV diff --git a/src/plugins/lvm.c b/src/plugins/lvm.c index ddbc9487d..4bd790fd3 100644 --- a/src/plugins/lvm.c +++ b/src/plugins/lvm.c @@ -89,6 +89,7 @@ BDLVMPVdata* bd_lvm_pvdata_copy (BDLVMPVdata *data) { new_data->vg_free_count = data->vg_free_count; new_data->vg_pv_count = data->vg_pv_count; new_data->pv_tags = g_strdupv (data->pv_tags); + new_data->missing = data->missing; return new_data; } @@ -543,6 +544,9 @@ static BDLVMPVdata* get_pv_data_from_table (GHashTable *table, gboolean free_tab else data->pv_tags = NULL; + value = (gchar*) g_hash_table_lookup (table, "LVM2_PV_MISSING"); + data->missing = (g_strcmp0 (value, "missing") == 0); + if (free_table) g_hash_table_destroy (table); @@ -1175,7 +1179,7 @@ BDLVMPVdata* bd_lvm_pvinfo (const gchar *device, GError **error) { const gchar *args[10] = {"pvs", "--unit=b", "--nosuffix", "--nameprefixes", "--unquoted", "--noheadings", "-o", "pv_name,pv_uuid,pv_free,pv_size,pe_start,vg_name,vg_uuid,vg_size," \ - "vg_free,vg_extent_size,vg_extent_count,vg_free_count,pv_count,pv_tags", + "vg_free,vg_extent_size,vg_extent_count,vg_free_count,pv_count,pv_tags,pv_missing", device, NULL}; GHashTable *table = NULL; gboolean success = FALSE; @@ -1194,7 +1198,7 @@ BDLVMPVdata* bd_lvm_pvinfo (const gchar *device, GError **error) { for (lines_p = lines; *lines_p; lines_p++) { table = parse_lvm_vars ((*lines_p), &num_items); - if (table && (num_items == 14)) { + if (table && (num_items == 15)) { g_clear_error (error); g_strfreev (lines); return get_pv_data_from_table (table, TRUE); @@ -1222,7 +1226,7 @@ BDLVMPVdata** bd_lvm_pvs (GError **error) { const gchar *args[9] = {"pvs", "--unit=b", "--nosuffix", "--nameprefixes", "--unquoted", "--noheadings", "-o", "pv_name,pv_uuid,pv_free,pv_size,pe_start,vg_name,vg_uuid,vg_size," \ - "vg_free,vg_extent_size,vg_extent_count,vg_free_count,pv_count,pv_tags", + "vg_free,vg_extent_size,vg_extent_count,vg_free_count,pv_count,pv_tags,pv_missing", NULL}; GHashTable *table = NULL; gboolean success = FALSE; @@ -1256,7 +1260,7 @@ BDLVMPVdata** bd_lvm_pvs (GError **error) { for (lines_p = lines; *lines_p; lines_p++) { table = parse_lvm_vars ((*lines_p), &num_items); - if (table && (num_items == 14)) { + if (table && (num_items == 15)) { /* valid line, try to parse and record it */ pvdata = get_pv_data_from_table (table, TRUE); if (pvdata) @@ -1743,6 +1747,41 @@ gboolean bd_lvm_lvresize (const gchar *vg_name, const gchar *lv_name, guint64 si return success; } +/** + * bd_lvm_lvrepair: + * @vg_name: name of the VG containing the to-be-repaired LV + * @lv_name: name of the to-be-repaired LV + * @pv_list: (array zero-terminated=1): list of PVs to be used for the repair + * @extra: (nullable) (array zero-terminated=1): extra options for the LV repair + * (just passed to LVM as is) + * @error: (out) (optional): place to store error (if any) + * + * Returns: whether the @vg_name/@lv_name LV was successfully repaired or not + * + * Tech category: %BD_LVM_TECH_BASIC-%BD_LVM_TECH_MODE_MODIFY + */ +gboolean bd_lvm_lvrepair (const gchar *vg_name, const gchar *lv_name, const gchar **pv_list, const BDExtraArg **extra, GError **error) { + guint i = 0; + guint pv_list_len = pv_list ? g_strv_length ((gchar **) pv_list) : 0; + const gchar **argv = g_new0 (const gchar*, pv_list_len + 5); + gboolean success = FALSE; + + argv[0] = "lvconvert"; + argv[1] = "--repair"; + argv[2] = "--yes"; + argv[3] = g_strdup_printf ("%s/%s", vg_name, lv_name); + for (i=4; i < (pv_list_len + 4); i++) { + argv[i] = pv_list[i-4]; + } + argv[i] = NULL; + + success = call_lvm_and_report_error (argv, extra, TRUE, error); + g_free ((gchar *) argv[3]); + g_free (argv); + + return success; +} + /** * bd_lvm_lvactivate: * @vg_name: name of the VG containing the to-be-activated LV diff --git a/src/plugins/lvm.h b/src/plugins/lvm.h index 5536bbd93..1045eb507 100644 --- a/src/plugins/lvm.h +++ b/src/plugins/lvm.h @@ -106,6 +106,7 @@ typedef struct BDLVMPVdata { guint64 vg_free_count; guint64 vg_pv_count; gchar **pv_tags; + gboolean missing; } BDLVMPVdata; void bd_lvm_pvdata_free (BDLVMPVdata *data); @@ -270,6 +271,7 @@ gboolean bd_lvm_lvcreate (const gchar *vg_name, const gchar *lv_name, guint64 si gboolean bd_lvm_lvremove (const gchar *vg_name, const gchar *lv_name, gboolean force, const BDExtraArg **extra, GError **error); gboolean bd_lvm_lvrename (const gchar *vg_name, const gchar *lv_name, const gchar *new_name, const BDExtraArg **extra, GError **error); gboolean bd_lvm_lvresize (const gchar *vg_name, const gchar *lv_name, guint64 size, const BDExtraArg **extra, GError **error); +gboolean bd_lvm_lvrepair (const gchar *vg_name, const gchar *lv_name, const gchar **pv_list, const BDExtraArg **extra, GError **error); gboolean bd_lvm_lvactivate (const gchar *vg_name, const gchar *lv_name, gboolean ignore_skip, const BDExtraArg **extra, GError **error); gboolean bd_lvm_lvdeactivate (const gchar *vg_name, const gchar *lv_name, const BDExtraArg **extra, GError **error); gboolean bd_lvm_lvsnapshotcreate (const gchar *vg_name, const gchar *origin_name, const gchar *snapshot_name, guint64 size, const BDExtraArg **extra, GError **error); diff --git a/tests/lvm_dbus_tests.py b/tests/lvm_dbus_tests.py index 38e052ecd..145f78fdc 100644 --- a/tests/lvm_dbus_tests.py +++ b/tests/lvm_dbus_tests.py @@ -1172,6 +1172,75 @@ def _clean_up(self): LvmPVVGTestCase._clean_up(self) +@unittest.skipUnless(lvm_dbus_running, "LVM DBus not running") +class LvmTestPartialLVs(LvmPVVGLVTestCase): + # the mirror halves are actually written to during sync-up and the + # default sparse_size of 1Gig is too much for a regular /tmp, so + # let's use smaller ones here. + # + _sparse_size = 20*1024**2 + + @tag_test(TestTags.CORE) + def test_lvpartial(self): + """Verify that missing PVs are detected and can be dealt with""" + + succ = BlockDev.lvm_pvcreate(self.loop_dev, 0, 0, None) + self.assertTrue(succ) + + succ = BlockDev.lvm_pvcreate(self.loop_dev2, 0, 0, None) + self.assertTrue(succ) + + succ = BlockDev.lvm_vgcreate("testVG", [self.loop_dev, self.loop_dev2], 0, None) + self.assertTrue(succ) + + info = BlockDev.lvm_pvinfo(self.loop_dev2) + self.assertTrue(info) + self.assertFalse(info.missing) + self.assertEqual(info.vg_name, "testVG") + loop_dev2_pv_uuid = info.pv_uuid + + # Create a mirrored LV on the first two PVs + with wait_for_sync("testVG", "testLV"): + succ = BlockDev.lvm_lvcreate("testVG", "testLV", 5 * 1024**2, "raid1", + [self.loop_dev, self.loop_dev2], None) + self.assertTrue(succ) + + info = BlockDev.lvm_lvinfo("testVG", "testLV") + self.assertTrue(info) + self.assertEqual(info.attr[8], "-") + + # Disconnect the second PV, this should cause it to be flagged + # as missing, and testLV to be reported as "partial". + delete_lio_device(self.loop_dev2) + + # Kick lvmdbusd so that it notices the missing PV. + dbus.SystemBus().call_blocking('com.redhat.lvmdbus1', '/com/redhat/lvmdbus1/Manager', + 'com.redhat.lvmdbus1.Manager', 'Refresh', '', []) + + pvs = BlockDev.lvm_pvs() + found = False + for pv in pvs: + if pv.pv_uuid == loop_dev2_pv_uuid: + found = True + self.assertTrue(pv.missing) + self.assertEqual(pv.vg_name, "testVG") + self.assertTrue(found) + + info = BlockDev.lvm_lvinfo("testVG", "testLV") + self.assertTrue(info) + self.assertEqual(info.attr[8], "p") + + # remove records of missing PVs + succ = BlockDev.lvm_vgreduce("testVG", None, None) + self.assertTrue(succ) + + pvs = BlockDev.lvm_pvs() + found = False + for pv in pvs: + if pv.pv_uuid == loop_dev2_pv_uuid: + found = True + self.assertFalse(found) + @unittest.skipUnless(lvm_dbus_running, "LVM DBus not running") class LvmTestLVsAll(LvmPVVGthpoolTestCase): def test_lvs_all(self): diff --git a/tests/lvm_test.py b/tests/lvm_test.py index 7b3f9985a..4d43b7581 100644 --- a/tests/lvm_test.py +++ b/tests/lvm_test.py @@ -332,6 +332,7 @@ def setUp(self): self.addCleanup(self._clean_up) self.dev_file = create_sparse_tempfile("lvm_test", self._sparse_size) self.dev_file2 = create_sparse_tempfile("lvm_test", self._sparse_size) + self.dev_file3 = create_sparse_tempfile("lvm_test", self._sparse_size) try: self.loop_dev = create_lio_device(self.dev_file) except RuntimeError as e: @@ -340,6 +341,10 @@ def setUp(self): self.loop_dev2 = create_lio_device(self.dev_file2) except RuntimeError as e: raise RuntimeError("Failed to setup loop device for testing: %s" % e) + try: + self.loop_dev3 = create_lio_device(self.dev_file3) + except RuntimeError as e: + raise RuntimeError("Failed to setup loop device for testing: %s" % e) def _clean_up(self): try: @@ -352,6 +357,11 @@ def _clean_up(self): except: pass + try: + BlockDev.lvm_pvremove(self.loop_dev3, None) + except: + pass + try: delete_lio_device(self.loop_dev) except RuntimeError: @@ -366,6 +376,13 @@ def _clean_up(self): pass os.unlink(self.dev_file2) + try: + delete_lio_device(self.loop_dev3) + except RuntimeError: + # just move on, we can do no better here + pass + os.unlink(self.dev_file3) + class LvmTestPVcreateRemove(LvmPVonlyTestCase): @tag_test(TestTags.CORE) def test_pvcreate_and_pvremove(self): @@ -775,6 +792,81 @@ def test_lvcreate_lvremove(self): with self.assertRaises(GLib.GError): BlockDev.lvm_lvremove("testVG", "testLV", True, None) +class LvmTestPartialLVs(LvmPVVGLVTestCase): + # the mirror halves are actually written to during sync-up and the + # default sparse_size of 1Gig is too much for a regular /tmp, so + # let's use smaller ones here. + # + _sparse_size = 20*1024**2 + + @tag_test(TestTags.CORE) + def test_lvpartial(self): + """Verify that missing PVs are detected and can be dealt with""" + + succ = BlockDev.lvm_pvcreate(self.loop_dev, 0, 0, None) + self.assertTrue(succ) + + succ = BlockDev.lvm_pvcreate(self.loop_dev2, 0, 0, None) + self.assertTrue(succ) + + succ = BlockDev.lvm_pvcreate(self.loop_dev3, 0, 0, None) + self.assertTrue(succ) + + succ = BlockDev.lvm_vgcreate("testVG", [self.loop_dev, self.loop_dev2, self.loop_dev3], 0, None) + self.assertTrue(succ) + + info = BlockDev.lvm_pvinfo(self.loop_dev2) + self.assertTrue(info) + self.assertFalse(info.missing) + self.assertEqual(info.vg_name, "testVG") + loop_dev2_pv_uuid = info.pv_uuid + + # Create a mirrored LV on the first two PVs + with wait_for_sync("testVG", "testLV"): + succ = BlockDev.lvm_lvcreate("testVG", "testLV", 5 * 1024**2, "raid1", + [self.loop_dev, self.loop_dev2], None) + self.assertTrue(succ) + + info = BlockDev.lvm_lvinfo("testVG", "testLV") + self.assertTrue(info) + self.assertEqual(info.attr[8], "-") + + # Disconnect the second PV, this should cause it to be flagged + # as missing, and testLV to be reported as "partial". + delete_lio_device(self.loop_dev2) + + pvs = BlockDev.lvm_pvs() + found = False + for pv in pvs: + if pv.pv_uuid == loop_dev2_pv_uuid: + found = True + self.assertTrue(pv.missing) + self.assertEqual(pv.vg_name, "testVG") + self.assertTrue(found) + + info = BlockDev.lvm_lvinfo("testVG", "testLV") + self.assertTrue(info) + self.assertEqual(info.attr[8], "p") + + # repair testLV with the third PV + with wait_for_sync("testVG", "testLV"): + succ = BlockDev.lvm_lvrepair("testVG", "testLV", [self.loop_dev3]) + self.assertTrue(succ) + + info = BlockDev.lvm_lvinfo("testVG", "testLV") + self.assertEqual(info.attr[8], "-") + + # remove records of missing PVs + succ = BlockDev.lvm_vgreduce("testVG", None, None) + self.assertTrue(succ) + + pvs = BlockDev.lvm_pvs() + found = False + for pv in pvs: + if pv.pv_uuid == loop_dev2_pv_uuid: + found = True + self.assertFalse(found) + class LvmTestLVcreateWithExtra(LvmPVVGLVTestCase): def __init__(self, *args, **kwargs): LvmPVVGLVTestCase.__init__(self, *args, **kwargs)