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

Osier Yang jyang at redhat.com
Mon May 13 09:32:07 UTC 2013


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.




More information about the libvir-list mailing list