[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
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list