Re: [libvirt] [PATCH][1.2.10] Relax duplicate SCSI host pool checking
On 10/31/2014 12:01 PM, Ján Tomko wrote:
Since commit 3f99d64 no new scsi_host pools can be defined
if one of the already defined scsi_host pools does not refer
to an accessible scsi_host adapter.
Relax the check by skipping over these inaccessible pools
when checking for duplicates. If both of them are defined by
their parent, only compare their address and unique_id with
the re-introduced matchSCSIAdapterParent function.
---
This would be nice to get into 1.2.10, as the commit mentioned
above has not yet been released.
I see no issue with that...
My only query/thought is whether this should be two patches?
#1 reintroduce match function
#2 adjust/fix second check
src/conf/storage_conf.c | 35 ++++++++++++++++++++++++++++++-----
1 file changed, 30 insertions(+), 5 deletions(-)
OK - so this makes sense.
Removal of match parent code was based mostly on the premise we have a
valid scsi_host data being looked at. I guess it seems odd to me someone
would go through the iterations necessary in order to define/create one
that wouldn't be checked to be working first before making their second
attempt. Nonetheless what you have works, so...
ACK
John
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 19c452b..afd6cd4 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -2094,6 +2094,28 @@ getSCSIHostNumber(virStoragePoolSourceAdapter adapter,
VIR_FREE(name);
return ret;
}
+static bool
+matchSCSIAdapterParent(virStoragePoolObjPtr pool,
+ virStoragePoolDefPtr def)
+{
+ virDevicePCIAddressPtr pooladdr =
+ &pool->def->source.adapter.data.scsi_host.parentaddr;
+ virDevicePCIAddressPtr defaddr =
+ &def->source.adapter.data.scsi_host.parentaddr;
+ int pool_unique_id =
+ pool->def->source.adapter.data.scsi_host.unique_id;
+ int def_unique_id =
+ def->source.adapter.data.scsi_host.unique_id;
+ if (pooladdr->domain == defaddr->domain &&
+ pooladdr->bus == defaddr->bus &&
+ pooladdr->slot == defaddr->slot &&
+ pooladdr->function == defaddr->function &&
+ pool_unique_id == def_unique_id) {
+ return true;
+ }
+ return false;
+}
+
int
virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools,
@@ -2143,10 +2165,17 @@ virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr
pools,
VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
unsigned int pool_hostnum, def_hostnum;
+ if (pool->def->source.adapter.data.scsi_host.has_parent
&&
+ def->source.adapter.data.scsi_host.has_parent &&
+ matchSCSIAdapterParent(pool, def)) {
+ matchpool = pool;
+ break;
+ }
+
if (getSCSIHostNumber(pool->def->source.adapter,
&pool_hostnum) < 0 ||
getSCSIHostNumber(def->source.adapter, &def_hostnum) < 0)
- goto error;
+ break;
if (pool_hostnum == def_hostnum)
matchpool = pool;
}
@@ -2188,10 +2217,6 @@ virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools,
ret = -1;
}
return ret;
-
- error:
- virStoragePoolObjUnlock(pool);
- return -1;
}
void