1:31 p.m.
On 05/14/2013 01:42 PM, Osier Yang wrote:
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
Thanks!
Gene