lvm: Enhancements for LVM2 RAID support

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

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

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

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

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);

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

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)