[libvirt] [PATCH 3/4] storage_conf: Fix the scsi_host.name comparison

John Ferlan jferlan at redhat.com
Tue Sep 30 21:35:26 UTC 2014


Since the 'scsi_host#' concept was introduced in commit id '9f781da6' it
is possible to provide either "name='host#'" or "name='scsi_host#'" to
define/name the scsi_host adapter source address.  The concept being
that 'name#' is/was legacy and 'scsi_host#' is what's seen in the
'virsh nodedev-list [--cap scsi_host]' output.  However, in doing the
comparison for a to be defined scsi_host, a latent bug allows the same
source to be defined twice, e.g. "name='host5'" is not distiguished
from "name='scsi_host5'", although in reality they eventually become
the same thing in commit id 'c1f63a9b' with the introduction of the
getHostNumber() API.

So this change will ensure that no one can use 'host5' and 'scsi_host5'
(or whatever #) to resolve to the same source adapter when going through
the define the source adapter processing and checking for duplicates. Doing
so will now result in an error (assuming that an existing pool is using
'host5' and the to be defined pool tries 'scsi_host5'):

$ virsh pool-define scsi-pool-use-scsi_host.xml
error: Failed to define pool from scsi-pool-use-scsi_host.xml
error: operation failed: Storage source conflict with pool: 'scsi-pool-use-host
$

Signed-off-by: John Ferlan <jferlan at redhat.com>
---
 src/conf/storage_conf.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index ac41307..74267bd 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -2063,6 +2063,27 @@ virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools,
 }
 
 static bool
+matchSCSIAdapterName(const char *pool_name,
+                     const char *def_name)
+{
+    /* Names can be either "scsi_host#" or just "host#", where
+     * "host#" is the back-compat format, but both equate to
+     * the same source adapter.  First check if both pool and def
+     * are using same format (easier) - if so, then compare
+     */
+    if ((STRPREFIX(pool_name, "scsi_") && STRPREFIX(def_name, "scsi")) ||
+        (STRPREFIX(pool_name, "host") && STRPREFIX(def_name, "host")))
+        return STREQ(pool_name, def_name);
+
+    /* If pool uses "scsi_host#" and def uses "host#", deal with that here */
+    if (STRPREFIX(pool_name, "scsi_"))
+        return STREQ(&pool_name[5], def_name);
+
+    /* Otherwise we have pool with "host#" and def with "scsi_host#" */
+    return STREQ(pool_name, &def_name[5]);
+}
+
+static bool
 matchSCSIAdapterParent(virStoragePoolSourceAdapter poolAdapter,
                        virStoragePoolSourceAdapter defAdapter)
 {
@@ -2089,8 +2110,8 @@ matchSCSIAdapter(virStoragePoolSourceAdapter poolAdapter,
                  virStoragePoolSourceAdapter defAdapter)
 {
     if (poolAdapter.data.scsi_host.name) {
-        return STREQ(poolAdapter.data.scsi_host.name,
-                     defAdapter.data.scsi_host.name);
+        return matchSCSIAdapterName(poolAdapter.data.scsi_host.name,
+                                    defAdapter.data.scsi_host.name);
     } else {
         return matchSCSIAdapterParent(poolAdapter, defAdapter);
     }
-- 
1.9.3




More information about the libvir-list mailing list