The multipath storage backend used the device path (e.g.,
/dev/mapper/mpathX) as the volume key. Device paths can change
across reboots when devices are enumerated in a different order.

Use dm_task_get_uuid() to retrieve the device-mapper UUID, which
is derived from the underlying hardware identifier and remains
stable. Fall back to the device path when a UUID is not available.

Signed-off-by: Lucas Amaral <[email protected]>
---
Build-tested and passed full test suite on CentOS Stream 9
(226 OK, 0 failures).

Verified dm_task_get_uuid() behavior using a standalone C
program that mimics the patched vol->key logic, run against
simulated device-mapper devices in a privileged container:

  $ dmsetup create testmpath0 \
      -u "mpath-3600508b4000c4a5d0000300000490000" \
      --table "0 1000 zero"
  $ dmsetup create testmpath1 --table "0 1000 zero"

Results:

  Device: testmpath0
    vol->key = mpath-3600508b4000c4a5d0000300000490000  (UUID)

  Device: testmpath1 (no UUID set)
    vol->key = /dev/mapper/testmpath1                   (path fallback)

  Device: nonexistent
    vol->key = /dev/mapper/nonexistent                  (path fallback)

All three cases (UUID present, UUID absent, device missing)
behave correctly. Not tested with a running libvirtd and a
real mpath pool.
---
 src/storage/storage_backend_mpath.c | 48 +++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/src/storage/storage_backend_mpath.c 
b/src/storage/storage_backend_mpath.c
index 9fc3983c52..a36400502f 100644
--- a/src/storage/storage_backend_mpath.c
+++ b/src/storage/storage_backend_mpath.c
@@ -40,13 +40,43 @@
 
 VIR_LOG_INIT("storage.storage_backend_mpath");
 
+static int
+virStorageBackendGetDeviceUUID(const char *dev_name, char **uuid)
+{
+    int ret = -1;
+    struct dm_task *dmt;
+    const char *dm_uuid;
+
+    if (!(dmt = dm_task_create(DM_DEVICE_INFO)))
+        goto out;
+
+    if (!dm_task_set_name(dmt, dev_name))
+        goto out;
+
+    if (!dm_task_run(dmt))
+        goto out;
+
+    if (!(dm_uuid = dm_task_get_uuid(dmt)) || !*dm_uuid)
+        goto out;
+
+    *uuid = g_strdup(dm_uuid);
+    ret = 0;
+
+ out:
+    if (dmt != NULL)
+        dm_task_destroy(dmt);
+    return ret;
+}
+
+
 static int
 virStorageBackendMpathNewVol(virStoragePoolObj *pool,
                              const int devnum,
-                             const char *dev)
+                             const char *dev_name)
 {
     virStoragePoolDef *def = virStoragePoolObjGetDef(pool);
     g_autoptr(virStorageVolDef) vol = NULL;
+    g_autofree char *dev_uuid = NULL;
 
     vol = g_new0(virStorageVolDef, 1);
 
@@ -54,15 +84,18 @@ virStorageBackendMpathNewVol(virStoragePoolObj *pool,
 
     (vol->name) = g_strdup_printf("dm-%u", devnum);
 
-    vol->target.path = g_strdup_printf("/dev/%s", dev);
+    vol->target.path = g_strdup_printf("/dev/mapper/%s", dev_name);
 
     if (virStorageBackendUpdateVolInfo(vol, true,
                                        VIR_STORAGE_VOL_OPEN_DEFAULT, 0) < 0) {
         return -1;
     }
 
-    /* XXX should use logical unit's UUID instead */
-    vol->key = g_strdup(vol->target.path);
+    virStorageBackendGetDeviceUUID(dev_name, &dev_uuid);
+    if (dev_uuid)
+        vol->key = g_steal_pointer(&dev_uuid);
+    else
+        vol->key = g_strdup(vol->target.path);
 
     if (virStoragePoolObjAddVol(pool, vol) < 0)
         return -1;
@@ -151,7 +184,6 @@ virStorageBackendCreateVols(virStoragePoolObj *pool,
 {
     uint32_t minor = -1;
     uint32_t next;
-    g_autofree char *map_device = NULL;
 
     do {
         int is_mpath = virStorageBackendIsMultipath(names->name);
@@ -161,8 +193,6 @@ virStorageBackendCreateVols(virStoragePoolObj *pool,
 
         if (is_mpath == 1) {
 
-            map_device = g_strdup_printf("mapper/%s", names->name);
-
             if (virStorageBackendGetMinorNumber(names->name, &minor) < 0) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                _("Failed to get %1$s minor number"),
@@ -170,10 +200,8 @@ virStorageBackendCreateVols(virStoragePoolObj *pool,
                 return -1;
             }
 
-            if (virStorageBackendMpathNewVol(pool, minor, map_device) < 0)
+            if (virStorageBackendMpathNewVol(pool, minor, names->name) < 0)
                 return -1;
-
-            VIR_FREE(map_device);
         }
 
         /* Given the way libdevmapper returns its data, I don't see
-- 
2.52.0

Reply via email to