[libvirt] [PATCH 4/4] conf: Check for storage conflicts across pool types

[John Ferlan]

https://bugzilla.redhat.com/show_bug.cgi?id=1233129

The virStoragePoolObjSourceFindDuplicate logic used by PoolCreateXML
and PoolDefineXML avoids comparing the new definition against "other"
pool types. This can cause unexpected corruption if two different pool
source types used the same source device path. For example, a 'disk'
pool using source type device=/dev/sdc could be unwittingly overwritten
by using /dev/sdc for a 'logical' pool which also uses the source
device path.

So rather than blindly ignoring those checks when def->type !=
pool->def->type - have the pool->def->type switch logic handle the
check for which def->type's should be checked.

Signed-off-by: John Ferlan <jferlan at redhat.com>
---
 src/conf/virstorageobj.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 1623162..ae327d9 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -794,6 +794,9 @@ virStoragePoolObjSourceMatchTypeDEVICE(virStoragePoolObjPtr pool,
     virStoragePoolObjPtr matchpool = NULL;
 
     if (pool->def->type == VIR_STORAGE_POOL_ISCSI) {
+        if (def->type != VIR_STORAGE_POOL_ISCSI)
+            return NULL;
+
         if ((matchpool = virStoragePoolSourceFindDuplicateDevices(pool, def))) {
             if (!virStoragePoolSourceISCSIMatch(matchpool, def))
                 return NULL;
@@ -801,6 +804,9 @@ virStoragePoolObjSourceMatchTypeDEVICE(virStoragePoolObjPtr pool,
         return matchpool;
     }
 
+    if (def->type == VIR_STORAGE_POOL_ISCSI)
+        return NULL;
+
     /* VIR_STORAGE_POOL_FS
      * VIR_STORAGE_POOL_LOGICAL
      * VIR_STORAGE_POOL_DISK
@@ -822,8 +828,6 @@ virStoragePoolObjSourceFindDuplicate(virConnectPtr conn,
     /* Check the pool list for duplicate underlying storage */
     for (i = 0; i < pools->count; i++) {
         pool = pools->objs[i];
-        if (def->type != pool->def->type)
-            continue;
 
         /* Don't match against ourself if re-defining existing pool ! */
         if (STREQ(pool->def->name, def->name))
@@ -835,11 +839,14 @@ virStoragePoolObjSourceFindDuplicate(virConnectPtr conn,
         case VIR_STORAGE_POOL_DIR:
         case VIR_STORAGE_POOL_GLUSTER:
         case VIR_STORAGE_POOL_NETFS:
-            matchpool = virStoragePoolObjSourceMatchTypeDIR(pool, def);
+            if (def->type == pool->def->type)
+                matchpool = virStoragePoolObjSourceMatchTypeDIR(pool, def);
             break;
 
         case VIR_STORAGE_POOL_SCSI:
-            matchpool = virStoragePoolObjSourceMatchTypeISCSI(pool, def, conn);
+            if (def->type == pool->def->type)
+                matchpool = virStoragePoolObjSourceMatchTypeISCSI(pool, def,
+                                                                  conn);
             break;
 
         case VIR_STORAGE_POOL_ISCSI:
@@ -847,22 +854,33 @@ virStoragePoolObjSourceFindDuplicate(virConnectPtr conn,
         case VIR_STORAGE_POOL_LOGICAL:
         case VIR_STORAGE_POOL_DISK:
         case VIR_STORAGE_POOL_ZFS:
-            matchpool = virStoragePoolObjSourceMatchTypeDEVICE(pool, def);
+            if (def->type == VIR_STORAGE_POOL_ISCSI ||
+                def->type == VIR_STORAGE_POOL_FS ||
+                def->type == VIR_STORAGE_POOL_LOGICAL ||
+                def->type == VIR_STORAGE_POOL_DISK ||
+                def->type == VIR_STORAGE_POOL_ZFS)
+                matchpool = virStoragePoolObjSourceMatchTypeDEVICE(pool, def);
             break;
 
         case VIR_STORAGE_POOL_SHEEPDOG:
-            if (virStoragePoolSourceMatchSingleHost(&pool->def->source,
+            if (def->type == pool->def->type &&
+                virStoragePoolSourceMatchSingleHost(&pool->def->source,
                                                     &def->source))
                 matchpool = pool;
             break;
+
         case VIR_STORAGE_POOL_MPATH:
             /* Only one mpath pool is valid per host */
-            matchpool = pool;
+            if (def->type == pool->def->type)
+                matchpool = pool;
             break;
+
         case VIR_STORAGE_POOL_VSTORAGE:
-            if (STREQ(pool->def->source.name, def->source.name))
+            if (def->type == pool->def->type &&
+                STREQ(pool->def->source.name, def->source.name))
                 matchpool = pool;
             break;
+
         case VIR_STORAGE_POOL_RBD:
         case VIR_STORAGE_POOL_LAST:
             break;
-- 
2.9.3