Re: [libvirt] [PATCH 4/9] storage: Create matchPoolSourceHost
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(a)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: