On 04/13/2015 05:27 AM, Peter Krempa wrote:
On Thu, Apr 02, 2015 at 13:39:38 -0400, John Ferlan wrote:
Create a separate iSCSI Source matching subroutine. Makes the calling code a bit cleaner as well as sets up for future patches which need to do better source hosts[0].name processing/checking
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/storage_conf.c | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 8b1898b..4a38416 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2291,6 +2291,28 @@ matchSCSIAdapterParent(virStoragePoolObjPtr pool, }
+static bool +matchISCSISource(virStoragePoolObjPtr matchpool,
Please use the virStorageConf... prefix for the new function.
How about "virStoragePoolSourceISCSIMatch"
+ virStoragePoolDefPtr def) +{ + 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)) + return true; + else + return false;
Um, how about return STREQ(... ?
myopia, but in the long run it won't matter as I agree with your view to merge patches 1-3 (something I probably thought along the way but didn't type as I was trying to show the transformation for at least the review) John
+ } + return true; + } + } + return false; +} + + int virStoragePoolSourceFindDuplicate(virConnectPtr conn, virStoragePoolObjListPtr pools, @@ -2390,17 +2412,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 (!matchISCSISource(matchpool, def)) + matchpool = NULL; } break; case VIR_STORAGE_POOL_FS:
ACK if you rename the function and remove the redundant if.
Peter