Re: [libvirt] [PATCH v2 1/5] storage: Check for valid fc_host parent at startup

On 10.11.2014 23:45, John Ferlan wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1160565 > > If a 'parent' attribute is provided for the fchost, then at startup > time check to ensure it is a vport capable scsi_host. If the parent > is not vport capable, then disallow the startup. The following is the > expected results: > > error: Failed to start pool fc_pool > error: XML error: parent 'scsi_host2' specified for vHBA is not vport capable > > where the XML for the fc_pool is: > > <pool type='scsi'> > <name>fc_pool</name> > <source> > <adapter type='fc_host' parent='scsi_host2' wwnn='5001a4aaf3ca174b' wwpn='5001a4a77192b864'/> > </source> > ... > > and 'scsi_host2' is not vport capable. > > Providing an incorrect parent and a correct wwnn/wwpn could lead to > failures at shutdown (deleteVport) where the assumption is the parent > is for the fchost. > > NOTE: If the provided wwnn/wwpn doesn't resolve to an existing scsi_host, > then we will be creating one with code (virManageVport) which > assumes the parent is vport capable. > > Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; > --- > src/storage/storage_backend_scsi.c | 22 ++++++++++++++++++---- > 1 file changed, 18 insertions(+), 4 deletions(-) > > diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c > index 02160bc..549d8db 100644 > --- a/src/storage/storage_backend_scsi.c > +++ b/src/storage/storage_backend_scsi.c > @@ -556,6 +556,20 @@ createVport(virStoragePoolSourceAdapter adapter) > if (adapter.type != VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) > return 0; > > + /* If a parent was provided, then let's make sure it's vhost capable */ > + if (adapter.data.fchost.parent) { > + if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0) > + return -1;

> + > + if (!virIsCapableFCHost(NULL, parent_host)) { > + virReportError(VIR_ERR_XML_ERROR, > + _("parent '%s' specified for vHBA " > + "is not vport capable"), > + adapter.data.fchost.parent); > + return -1; > + } > + } > + > /* This filters either HBA or already created vHBA */ > if ((name = virGetFCHostNameByWWN(NULL, adapter.data.fchost.wwnn, > adapter.data.fchost.wwpn))) { > @@ -565,14 +579,14 @@ createVport(virStoragePoolSourceAdapter adapter) > > if (!adapter.data.fchost.parent && > !(adapter.data.fchost.parent = virFindFCHostCapableVport(NULL))) { > - virReportError(VIR_ERR_XML_ERROR, "%s", > + virReportError(VIR_ERR_XML_ERROR, "%s", > _("'parent' for vHBA not specified, and " > "cannot find one on this host")); > return -1; > - } > > - if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0) > - return -1; > + if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0) > + return -1; > + } This chunk seems odd. After the error is reported, the control jumps out from the function, so virGetSCSIHostNumer is not called at all. Did you forget to commit something?

> > if (virManageVport(parent_host, adapter.data.fchost.wwpn, > adapter.data.fchost.wwnn, VPORT_CREATE) < 0) > Michal