https://bugzilla.redhat.com/show_bug.cgi?id=1146837 Resolve a crash in libvirtd resulting from commit id 'a4bd62ad' (1.0.6) which added parentaddr and unique_id to allow unique identification of a scsi_host, but assumed that all the pool entries and the incoming definition would be similarly defined. If the existing pool uses the 'name' attribute and an incoming pool is using the parentaddr/unique_id, then the code will attempt to compare the existing name string against the incoming name string which doesn't exist (is NULL) and results in a core (STREQ). Conversely, if the existing pool used the parentaddr/unique_id and the to be defined pool used the name, then the comparison would be against the parentaddr, but since the incoming pool doesn't have one - that would leave the comparison against a parentaddr of all 0's and a unique_id of 0, which will always comparison to fail. This means someone could define the same source adapter for two pools This patch resolves that. Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> --- src/conf/storage_conf.c | 67 ++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 61 insertions(+), 6 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 74267bd..c17565e 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2062,7 +2062,7 @@ virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, return ret; } -static bool +static bool ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) matchSCSIAdapterName(const char *pool_name, const char *def_name) { @@ -2105,16 +2105,63 @@ matchSCSIAdapterParent(virStoragePoolSourceAdapter poolAdapter, return false; } -static bool +static int matchSCSIAdapter(virStoragePoolSourceAdapter poolAdapter, virStoragePoolSourceAdapter defAdapter) { - if (poolAdapter.data.scsi_host.name) { + char *name = NULL; + char *parentaddr = NULL; + const char *cmpname; + unsigned int unique_id; + virDevicePCIAddressPtr poolPCIptr; + bool retval = -1; + + /* simple cases - both use name or both use parentaddr */ + if (poolAdapter.data.scsi_host.name && + defAdapter.data.scsi_host.name) { return matchSCSIAdapterName(poolAdapter.data.scsi_host.name, defAdapter.data.scsi_host.name); - } else { + } else if (poolAdapter.data.scsi_host.has_parent && + defAdapter.data.scsi_host.has_parent) { return matchSCSIAdapterParent(poolAdapter, defAdapter); } + + if (poolAdapter.data.scsi_host.has_parent) { + /* If the poolAdapter is using parentaddr, but incoming is using the + * "scsi_host#" or "host#", then we need to formulate the "host#" + * string of the poolAdapter for comparison. We cannot save that + * during startup in the pooldef since when/if we write out the + * XML for the pool, if we find the name field filled in we use that + * defeating the purpose of having parentaddr/unique_id + */ + cmpname = defAdapter.data.scsi_host.name; + unique_id = poolAdapter.data.scsi_host.unique_id; + poolPCIptr = &poolAdapter.data.scsi_host.parentaddr; + } else { + /* Otherwise the pool is using "host#" or "scsi_host#" and the incoming + * def is using the "parentaddr" and "uniqueid" - so formulate the + * host# name of the incoming def and make sure it doesn't match the + * current pool definition + */ + cmpname = poolAdapter.data.scsi_host.name; + unique_id = defAdapter.data.scsi_host.unique_id; + poolPCIptr = &defAdapter.data.scsi_host.parentaddr; + } + + if (virAsprintf(&parentaddr, "%04x:%02x:%02x.%01x", + poolPCIptr->domain, poolPCIptr->bus, + poolPCIptr->slot, poolPCIptr->function) < 0) + goto cleanup; + + if (!(name = virFindSCSIHostByPCI(NULL, parentaddr, unique_id))) + goto cleanup; + + retval = matchSCSIAdapterName(name, cmpname); + + cleanup: + VIR_FREE(name); + VIR_FREE(parentaddr); + return retval; } int @@ -2163,8 +2210,12 @@ virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools, VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST && def->source.adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) { - if (matchSCSIAdapter(pool->def->source.adapter, - def->source.adapter)) + int rc; + rc = matchSCSIAdapter(pool->def->source.adapter, + def->source.adapter); + if (rc < 0) + goto error; /* Error reported during matchSCSIAdapter */ + if (rc) matchpool = pool; } break; @@ -2205,6 +2256,10 @@ virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools, ret = -1; } return ret; + + error: + virStoragePoolObjUnlock(pool); + return -1; } void -- 1.9.3 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list