On 09/03/2014 03:28 PM, John Ferlan wrote:
>
> [found this in my "probably should look at this one some day" pile..]
>
> On 06/21/2014 12:57 PM, Pradipta Kr. Banerjee wrote:
>> libvirtd crashes when there is an existing SCSI pool
>> with adapter type as 'scsi_host' and defining a new SCSI pool with adapter
>> type as 'fc_host' and parent attribute missing.
>>
>> For eg when defining a storage-pool with the following XML will crash libvirtd
>> if there already exists a SCSI pool with adapter type 'scsi_host'
>>
>> <pool type='scsi'>
>> <name>TEST_SCSI_FC_POOL</name>
>> <source>
>> <adapter type='fc_host' wwnn='1234567890abcdef' wwpn='abcdef1234567890'/>
>> </source>
>> <target>
>> <path>/dev/disk/by-path</path>
>> </target>
>> </pool>
>>
>> This happens because for fc_host, adapter 'name' is not relevant whereas
>> for scsi_host its mandatory attribute. However the check in libvirt for
>> finding duplicate storage pools doesn't take that into account while comparing,
>> resulting into crash
>>
>> This patch fixes the issue
>
> Going to need to clean the above up, but if you also provided the existing
> scsi/scsi_host pool def that would have been good!
>
>>
>> Signed-off-by: Pradipta Kr. Banerjee <bpradip@xxxxxxxxxx>
>> ---
>> src/conf/storage_conf.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
>> index 8b6fd79..54a4589 100644
>> --- a/src/conf/storage_conf.c
>> +++ b/src/conf/storage_conf.c
>> @@ -2126,8 +2126,10 @@ virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr pools,
>> STREQ(pool->def->source.adapter.data.fchost.wwpn,
>> def->source.adapter.data.fchost.wwpn))
>> matchpool = pool;
>> - } else if (pool->def->source.adapter.type ==
>> - VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST){
>> + } else if ((pool->def->source.adapter.type ==
>> + VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST)\
>> + && (def->source.adapter.type ==
>> + VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST)) {
>
> My assumption is this issue is a result of commit id '9f781da'
>
> Beyond the - it needs a bit a refresh due to other changes
> since that time - I'm wondering if you've only solved half
> the potential problem
You are right.
>
> What if the fc_host is defined and you're coming in with
> a 'scsi_host' def->source.adapter.type? (see a few lines
> earlier)
This will result in crash too.. I'll send a V2 based on your suggestion
>
>
>> if (STREQ(pool->def->source.adapter.data.name,
>> def->source.adapter.data.name))
>> matchpool = pool;
>>
>
> So instead of what's here, how about the following diff:
>
>
> break;
> case VIR_STORAGE_POOL_SCSI:
> if (pool->def->source.adapter.type ==
> + VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST &&
> + def->source.adapter.type ==
> VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
> if (STREQ(pool->def->source.adapter.data.fchost.wwnn,
> def->source.adapter.data.fchost.wwnn) &&
> @@ -2124,6 +2126,8 @@ virStoragePoolSourceFindDuplicate(virStoragePoolObjListPtr
> def->source.adapter.data.fchost.wwpn))
> matchpool = pool;
> } else if (pool->def->source.adapter.type ==
> + VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST &&
> + def->source.adapter.type ==
> VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
> if (pool->def->source.adapter.data.scsi_host.name) {
> if (STREQ(pool->def->source.adapter.data.scsi_host.name,
>
>
> John
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
Regards,
Pradipta Kumar B(bpradip@xxxxxxxxxx)
IBM Systems & Technology Labs,
India.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list