[libvirt] [PATCH 2/8] tests: Add createVHBAByStoragePool-by-parent to fchosttest

[Michal Privoznik]

On 20.02.2017 14:18, John Ferlan wrote:
> Add a new test to fchosttest in order to test creation of our vHBA
> via the Storage Pool logic.  Unlike the real code, we cannot yet use
> the virVHBA* API's because they (currently) traverse the file system
> in order to get the parent vport capable scsi_host. Besides there's
> no "real" NPIV device here - so we have to take some liberties, at
> least for now.
> 
> Instead, we'll follow the node device tests partially in order to
> create and destroy the vHBA with the test node devices.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/test/test_driver.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  tests/fchosttest.c     | 64 ++++++++++++++++++++++++++++++++++
>  2 files changed, 157 insertions(+), 2 deletions(-)
> 
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 5fef3f1..4dff0f1 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -4365,6 +4365,32 @@ testConnectFindStoragePoolSources(virConnectPtr conn ATTRIBUTE_UNUSED,
>  }
>  
>  
> +static virNodeDeviceDefPtr
> +testNodeDeviceMockCreateVport(virConnectPtr conn,
> +                              const char *wwnn,
> +                              const char *wwpn);
> +static int
> +testCreateVport(virConnectPtr conn,
> +                const char *wwnn,
> +                const char *wwpn)
> +{
> +    /* The storage_backend_scsi createVport() will use the input adapter
> +     * fields parent name, parent_wwnn/parent_wwpn, or parent_fabric_wwn
> +     * in order to determine whether the provided parent can be used to
> +     * create a vHBA or will find "an available vport capable" to create
> +     * a vHBA. In order to do this, it uses the virVHBA* API's which traverse
> +     * the sysfs looking at various fields (rather than going via nodedev).
> +     *
> +     * Since the test environ doesn't have the sysfs for the storage pool
> +     * test, at least for now use the node device test infrastructure to
> +     * create the vHBA. In the long run the result is the same. */
> +    if (!testNodeDeviceMockCreateVport(conn, wwnn, wwpn))
> +        return -1;
> +
> +    return 0;
> +}
> +
> +
>  static virStoragePoolPtr
>  testStoragePoolCreateXML(virConnectPtr conn,
>                           const char *xml,
> @@ -4395,6 +4421,24 @@ testStoragePoolCreateXML(virConnectPtr conn,
>          goto cleanup;
>      def = NULL;
>  
> +    if (pool->def->source.adapter.type ==
> +        VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
> +        /* In the real code, we'd call virVHBAManageVport followed by
> +         * find_new_device, but we cannot do that here since we're not
> +         * mocking udev. The mock routine will copy an existing vHBA and
> +         * rename a few fields to mock that. So in order to allow that to
> +         * work properly, we need to drop our lock */
> +        testDriverUnlock(privconn);
> +        if (testCreateVport(conn, pool->def->source.adapter.data.fchost.wwnn,
> +                            pool->def->source.adapter.data.fchost.wwpn) < 0) {
> +            virStoragePoolObjRemove(&privconn->pools, pool);
> +            pool = NULL;
> +            testDriverLock(privconn);
> +            goto cleanup;
> +        }
> +        testDriverLock(privconn);

So we need this testDriverLock() and Unlock() calls because
testCreateVport() calls testNodeDeviceMockCreateVport() which then call
top level APIs for looking up a nodedev and fetching its XML. Pardon my
language but that looks stup^Wweird. Mind fixing that?

> +    }
> +

Otherwise looking good.

Michal