11:35 a.m.
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.
Gene