[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

On 10.11.2014 23:45, John Ferlan wrote:

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'>
         <adapter type='fc_host' parent='scsi_host2' wwnn='5001a4aaf3ca174b' wwpn='5001a4a77192b864'/>

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 redhat com>
  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)
          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)


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]