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

Re: [libvirt] [PATCH 7/7] tests: Add tests for fc_host



On 15/05/13 00:35, Gene Czarcinski wrote:
On 05/13/2013 05:32 AM, Osier Yang wrote:
On 08/05/13 23:03, Osier Yang wrote:
On 08/05/13 21:53, John Ferlan wrote:
On 05/06/2013 08:45 AM, Osier Yang wrote:
Since the NPIV machine is not easy to get, it's very likely to
introduce regressions when doing changes on the existing code.
This patch dumps part of the sysfs files (the necessary ones)
of fc_host as test input data, to test the related util functions.
It could be extended for more fc_host related testing in future.
---
  tests/Makefile.am                                |   5 +
  tests/fchostdata/fc_host/host4/fabric_name       |   1 +
  tests/fchostdata/fc_host/host4/max_npiv_vports   |   1 +
  tests/fchostdata/fc_host/host4/node_name         |   1 +
  tests/fchostdata/fc_host/host4/npiv_vports_inuse |   1 +
  tests/fchostdata/fc_host/host4/port_name         |   1 +
  tests/fchostdata/fc_host/host4/port_state        |   1 +
  tests/fchostdata/fc_host/host4/vport_create      |   0
  tests/fchostdata/fc_host/host4/vport_delete      |   0
  tests/fchostdata/fc_host/host5/fabric_name       |   1 +
  tests/fchostdata/fc_host/host5/max_npiv_vports   |   1 +
  tests/fchostdata/fc_host/host5/node_name         |   1 +
  tests/fchostdata/fc_host/host5/npiv_vports_inuse |   1 +
  tests/fchostdata/fc_host/host5/port_name         |   1 +
  tests/fchostdata/fc_host/host5/port_state        |   1 +
  tests/fchostdata/fc_host/host5/vport_create      |   0
  tests/fchostdata/fc_host/host5/vport_delete      |   0
tests/fchosttest.c | 175 +++++++++++++++++++++++
  18 files changed, 192 insertions(+)
  create mode 100644 tests/fchostdata/fc_host/host4/fabric_name
  create mode 100644 tests/fchostdata/fc_host/host4/max_npiv_vports
  create mode 100644 tests/fchostdata/fc_host/host4/node_name
  create mode 100644 tests/fchostdata/fc_host/host4/npiv_vports_inuse
  create mode 100644 tests/fchostdata/fc_host/host4/port_name
  create mode 100644 tests/fchostdata/fc_host/host4/port_state
  create mode 100644 tests/fchostdata/fc_host/host4/vport_create
  create mode 100644 tests/fchostdata/fc_host/host4/vport_delete
  create mode 100644 tests/fchostdata/fc_host/host5/fabric_name
  create mode 100644 tests/fchostdata/fc_host/host5/max_npiv_vports
  create mode 100644 tests/fchostdata/fc_host/host5/node_name
  create mode 100644 tests/fchostdata/fc_host/host5/npiv_vports_inuse
  create mode 100644 tests/fchostdata/fc_host/host5/port_name
  create mode 100644 tests/fchostdata/fc_host/host5/port_state
  create mode 100644 tests/fchostdata/fc_host/host5/vport_create
  create mode 100644 tests/fchostdata/fc_host/host5/vport_delete
  create mode 100644 tests/fchosttest.c

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 68dbb27..48c4e4a 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -107,6 +107,7 @@ test_programs = virshtest sockettest \
          virportallocatortest \
      sysinfotest \
      virstoragetest \
+    fchosttest \
      $(NULL)
    if WITH_GNUTLS
@@ -525,6 +526,10 @@ nodeinfotest_SOURCES = \
      nodeinfotest.c testutils.h testutils.c
  nodeinfotest_LDADD = $(LDADDS)
  +fchosttest_SOURCES = \
+    fchosttest.c testutils.h testutils.c
+fchosttest_LDADD = $(LDADDS)
+
  commandtest_SOURCES = \
      commandtest.c testutils.h testutils.c
  commandtest_LDADD = $(LDADDS)
diff --git a/tests/fchostdata/fc_host/host4/fabric_name b/tests/fchostdata/fc_host/host4/fabric_name
new file mode 100644
index 0000000..f587b68
--- /dev/null
+++ b/tests/fchostdata/fc_host/host4/fabric_name
@@ -0,0 +1 @@
+0xffffffffffffffff
diff --git a/tests/fchostdata/fc_host/host4/max_npiv_vports b/tests/fchostdata/fc_host/host4/max_npiv_vports
new file mode 100644
index 0000000..c75acbe
--- /dev/null
+++ b/tests/fchostdata/fc_host/host4/max_npiv_vports
@@ -0,0 +1 @@
+127
diff --git a/tests/fchostdata/fc_host/host4/node_name b/tests/fchostdata/fc_host/host4/node_name
new file mode 100644
index 0000000..e8c1e1a
--- /dev/null
+++ b/tests/fchostdata/fc_host/host4/node_name
@@ -0,0 +1 @@
+0x2000001b3289da4e
diff --git a/tests/fchostdata/fc_host/host4/npiv_vports_inuse b/tests/fchostdata/fc_host/host4/npiv_vports_inuse
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/fchostdata/fc_host/host4/npiv_vports_inuse
@@ -0,0 +1 @@
+0
diff --git a/tests/fchostdata/fc_host/host4/port_name b/tests/fchostdata/fc_host/host4/port_name
new file mode 100644
index 0000000..ee2d399
--- /dev/null
+++ b/tests/fchostdata/fc_host/host4/port_name
@@ -0,0 +1 @@
+0x2100001b3289da4e
diff --git a/tests/fchostdata/fc_host/host4/port_state b/tests/fchostdata/fc_host/host4/port_state
new file mode 100644
index 0000000..bd1e6d5
--- /dev/null
+++ b/tests/fchostdata/fc_host/host4/port_state
@@ -0,0 +1 @@
+Linkdown
diff --git a/tests/fchostdata/fc_host/host4/vport_create b/tests/fchostdata/fc_host/host4/vport_create
new file mode 100644
index 0000000..e69de29
diff --git a/tests/fchostdata/fc_host/host4/vport_delete b/tests/fchostdata/fc_host/host4/vport_delete
new file mode 100644
index 0000000..e69de29
diff --git a/tests/fchostdata/fc_host/host5/fabric_name b/tests/fchostdata/fc_host/host5/fabric_name
new file mode 100644
index 0000000..4128070
--- /dev/null
+++ b/tests/fchostdata/fc_host/host5/fabric_name
@@ -0,0 +1 @@
+0x2001000dec9877c1
diff --git a/tests/fchostdata/fc_host/host5/max_npiv_vports b/tests/fchostdata/fc_host/host5/max_npiv_vports
new file mode 100644
index 0000000..c75acbe
--- /dev/null
+++ b/tests/fchostdata/fc_host/host5/max_npiv_vports
@@ -0,0 +1 @@
+127
diff --git a/tests/fchostdata/fc_host/host5/node_name b/tests/fchostdata/fc_host/host5/node_name
new file mode 100644
index 0000000..91a0a05
--- /dev/null
+++ b/tests/fchostdata/fc_host/host5/node_name
@@ -0,0 +1 @@
+0x2001001b32a9da4e
diff --git a/tests/fchostdata/fc_host/host5/npiv_vports_inuse b/tests/fchostdata/fc_host/host5/npiv_vports_inuse
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/fchostdata/fc_host/host5/npiv_vports_inuse
@@ -0,0 +1 @@
+0
diff --git a/tests/fchostdata/fc_host/host5/port_name b/tests/fchostdata/fc_host/host5/port_name
new file mode 100644
index 0000000..c37f618
--- /dev/null
+++ b/tests/fchostdata/fc_host/host5/port_name
@@ -0,0 +1 @@
+0x2101001b32a9da4e
diff --git a/tests/fchostdata/fc_host/host5/port_state b/tests/fchostdata/fc_host/host5/port_state
new file mode 100644
index 0000000..b73dd46
--- /dev/null
+++ b/tests/fchostdata/fc_host/host5/port_state
@@ -0,0 +1 @@
+Online
diff --git a/tests/fchostdata/fc_host/host5/vport_create b/tests/fchostdata/fc_host/host5/vport_create
new file mode 100644
index 0000000..e69de29
diff --git a/tests/fchostdata/fc_host/host5/vport_delete b/tests/fchostdata/fc_host/host5/vport_delete
new file mode 100644
index 0000000..e69de29
diff --git a/tests/fchosttest.c b/tests/fchosttest.c
new file mode 100644
index 0000000..035f721
--- /dev/null
+++ b/tests/fchosttest.c
@@ -0,0 +1,175 @@
+/*
+ * Copyright (C) 2013 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <config.h>
+
+#include "virutil.h"
+#include "testutils.h"
+
+#define TEST_FC_HOST_PREFIX "./fchostdata/fc_host/"
+#define TEST_FC_HOST_NUM 5
+
+/* Test virIsCapableFCHost */
+static int
+test1(const void *data ATTRIBUTE_UNUSED)
+{
+    if (virIsCapableFCHost(TEST_FC_HOST_PREFIX,
+                           TEST_FC_HOST_NUM))
+        return 0;
+
+    return -1;
+}
+
+/* Test virIsCapableVport */
+static int
+test2(const void *data ATTRIBUTE_UNUSED)
+{
+    if (virIsCapableVport(TEST_FC_HOST_PREFIX,
+                          TEST_FC_HOST_NUM))
+        return 0;
+
+    return -1;
+}
+
+/* Test virReadFCHost */
+static int
+test3(const void *data ATTRIBUTE_UNUSED)
+{
+    const char *expect_wwnn = "2001001b32a9da4e";
+    const char *expect_wwpn = "2101001b32a9da4e";
+    const char *expect_fabric_wwn = "2001000dec9877c1";
+    const char *expect_max_vports = "127";
+    const char *expect_vports = "0";
+    char *wwnn = NULL;
+    char *wwpn = NULL;
+    char *fabric_wwn = NULL;
+    char *max_vports = NULL;
+    char *vports = NULL;
+    int ret = -1;
+
+    if (virReadFCHost(TEST_FC_HOST_PREFIX,
+                      TEST_FC_HOST_NUM,
+                      "node_name",
+                      &wwnn) < 0)
+        return -1;
NIT: s/-1/ret  or goto cleanup;

Not sure if there's a norm though

+
+    if (virReadFCHost(TEST_FC_HOST_PREFIX,
+                      TEST_FC_HOST_NUM,
+                      "port_name",
+                      &wwpn) < 0)
+        goto cleanup;
+
+    if (virReadFCHost(TEST_FC_HOST_PREFIX,
+                      TEST_FC_HOST_NUM,
+                      "fabric_name",
+                      &fabric_wwn) < 0)
+        goto cleanup;
+
+    if (virReadFCHost(TEST_FC_HOST_PREFIX,
+                      TEST_FC_HOST_NUM,
+                      "max_npiv_vports",
+                      &max_vports) < 0)
+        goto cleanup;
+
+
+    if (virReadFCHost(TEST_FC_HOST_PREFIX,
+                      TEST_FC_HOST_NUM,
+                      "npiv_vports_inuse",
+                      &vports) < 0)
+        goto cleanup;
+
+    if (STRNEQ(expect_wwnn, wwnn) ||
+        STRNEQ(expect_wwpn, wwpn) ||
+        STRNEQ(expect_fabric_wwn, fabric_wwn) ||
+        STRNEQ(expect_max_vports, max_vports) ||
+        STRNEQ(expect_vports, vports))
+        goto cleanup;
+
+    ret = 0;
+cleanup:
+    VIR_FREE(wwnn);
+    VIR_FREE(wwpn);
+    VIR_FREE(fabric_wwn);
+    VIR_FREE(max_vports);
+    VIR_FREE(vports);
+    return ret;
+}
+
+/* Test virGetFCHostNameByWWN */
+static int
+test4(const void *data ATTRIBUTE_UNUSED)
+{
+    const char *expect_hostname = "host5";
+    char *hostname = NULL;
+    int ret = -1;
+
+    if (!(hostname = virGetFCHostNameByWWN(TEST_FC_HOST_PREFIX,
+ "2001001b32a9da4e",
+ "2101001b32a9da4e")))
+        return -1;
Same NIT here.

+
+    if (STRNEQ(hostname, expect_hostname))
+        goto cleanup;
+
+    ret = 0;
+cleanup:
+    VIR_FREE(hostname);
+    return ret;
+}
+
+/* Test virFindFCHostCapableVport (host4 is not Online) */
+static int
+test5(const void *data ATTRIBUTE_UNUSED)
+{
+    const char *expect_hostname = "host5";
+    char *hostname = NULL;
+    int ret = -1;
+
+ if (!(hostname = virFindFCHostCapableVport(TEST_FC_HOST_PREFIX)))
+        return -1;
Same NIT here.

+
+    if (STRNEQ(hostname, expect_hostname))
+        goto cleanup;
+
+    ret = 0;
+cleanup:
+    VIR_FREE(hostname);
+    return ret;
+}
+
+static int
+mymain(void)
+{
+    int ret = 0;
+
+    if (virtTestRun("test1", 1, test1, NULL) < 0)
+        ret = -1;
+    if (virtTestRun("test2", 1, test2, NULL) < 0)
+        ret = -1;
+    if (virtTestRun("test3", 1, test3, NULL) < 0)
+        ret = -1;
+    if (virtTestRun("test4", 1, test4, NULL) < 0)
+        ret = -1;
+    if (virtTestRun("test5", 1, test5, NULL) < 0)
+        ret = -1;
+
+    return ret;
+}
+
+VIRT_TEST_MAIN(mymain)

ACK - again, not sure of "coding style" norm on those returns. They
accomplish the same thing so I'm fine with things the way they are.


As replied in 4/7, personally I prefer to use the "return 0/-1" if
there is no need to goto cleanup, it's more clear to let one
known what's the return value is, especially when the code
body of a function is long enough.

I will send v2 to add the patches to cleanup the macros not
used anymore in src/node_device_driver.h, and use the enum
in the utils.

I misunderstood your meaning in 2/7, you meant the macros defined
for string, pushed the set and will post independant patches.
NOTE!!

Your test adds data but DOES NOT update Makefile.am

Therefore. when you create a dist tarball and run the tests, the fchosttest will fail because it cannot find its data because the data is NOT included in the tarball.

Fixed by commit 1cc8259bfe17be51eb1


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