[libvirt] [PATCH 4/9] storage: Create matchPoolSourceHost

[John Ferlan]

On 04/13/2015 06:18 AM, Peter Krempa wrote:
> On Thu, Apr 02, 2015 at 13:39:41 -0400, John Ferlan wrote:
>> Split out the nhost == 1 and hosts[0].name logic into a separate routine
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  src/conf/storage_conf.c | 17 ++++++++++++-----
>>  1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
>> index e4cb54b..b3e930b 100644
>> --- a/src/conf/storage_conf.c
>> +++ b/src/conf/storage_conf.c
>> @@ -2290,6 +2290,17 @@ matchSCSIAdapterParent(virStoragePoolObjPtr pool,
>>      return false;
>>  }
>>  
>> +static bool
>> +matchPoolSourceHost(virStoragePoolSourcePtr poolsrc,
> 
> Same compliant for the function name as in 1/9.
> 
So how about: virStoragePoolSourceMatchSingleHost

As you probably eventually realized I punted on the multiple host case
of NBD - although it probably needs similar logic, but I was intent on
doing the cases where only one host was used by the backends.

BTW: Where this is headed is we currently only match the host[0] by
string, but that's not good enough for networks where one can use a
'name' or a 'number' (and then all sorts of fun with ipv4 v ipv6).

John

>> +                    virStoragePoolSourcePtr defsrc)
>> +{
>> +    /* NB: nhost cannot be > 1 */
>> +    if (poolsrc->nhost == 0 || defsrc->nhost == 0)
>> +        return false;
> 
> And this condition can be made explicitly state the same without the
> need for the comment.
> 
> ACK with the name and condition changed.
> 
> Peter
> 

So up through this point the diff to master would be:

+static bool
+virStoragePoolSourceMatchSingleHost(virStoragePoolSourcePtr poolsrc,
+                                    virStoragePoolSourcePtr defsrc)
+{
+    if (poolsrc->nhost != 1 && defsrc->nhost != 1)
+        return false;
+
+    return STREQ(poolsrc->hosts[0].name, defsrc->hosts[0].name);
+}
+
+
+static bool
+virStoragePoolSourceISCSIMatch(virStoragePoolObjPtr matchpool,
+                               virStoragePoolDefPtr def)
+{
+    virStoragePoolSourcePtr poolsrc = &matchpool->def->source;
+    virStoragePoolSourcePtr defsrc = &def->source;
+
+    if (!virStoragePoolSourceMatchSingleHost(poolsrc, defsrc))
+        return false;
+
+    if (STRNEQ_NULLABLE(poolsrc->initiator.iqn, defsrc->initiator.iqn))
+        return false;
+
+    return true;
+}
+

 int
 virStoragePoolSourceFindDuplicate(virConnectPtr conn,
@@ -2505,17 +2532,8 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn,
         case VIR_STORAGE_POOL_ISCSI:
             matchpool = virStoragePoolSourceFindDuplicateDevices(pool,
def);
             if (matchpool) {
-                if (matchpool->def->source.nhost == 1 &&
def->source.nhost == 1) {
-                    if (STREQ(matchpool->def->source.hosts[0].name,
def->source.hosts[0].name)) {
-                        if ((matchpool->def->source.initiator.iqn) &&
(def->source.initiator.iqn)) {
-                            if
(STREQ(matchpool->def->source.initiator.iqn, def->source.initiator.iqn))
-                                break;
-                            matchpool = NULL;
-                        }
-                        break;
-                    }
-                }
-                matchpool = NULL;
+                if (!virStoragePoolSourceISCSIMatch(matchpool, def))
+                    matchpool = NULL;
             }
             break;
         case VIR_STORAGE_POOL_FS: