Re: [libvirt] [PATCH 1/1] libvirtd crash when defining scsi storage pool
[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(a)in.ibm.com>
---
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
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)
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