[libvirt] [PATCH 6/8] util: Alter virNodeDevice{Create|Delete}Vport to use nodedev APIs
Laine Stump
laine at laine.org
Mon Feb 27 18:15:54 UTC 2017
On 02/20/2017 08:18 AM, John Ferlan wrote:
> If we have a connection pointer there's no sense walking through the
> sysfs in order to create/destroy the vHBA. Instead, let's make use of
> the node device create/destroy API's.
>
> Since we don't have to rewrite all the various parent options for
> the test driver in order to test whether the storage pool creation
> works as the node device creation has been tested already, let's just
> use the altered API to test the storage pool paths.
>
> Fix a "bug" in the storage pool test driver code which "assumed"
> testStoragePoolObjSetDefaults should fill in the configFile for
> both the Define/Create (persistent) and CreateXML (transient) pools
> by just VIR_FREE() of the pool during CreateXML. Because the
> configFile was filled in, during Destroy, the pool wouldn't be
> free causing a test using the same name pool to fail.
>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
> src/conf/node_device_conf.c | 121 ++++++++++++++++++++++++++++++++++++++++++++
> src/test/test_driver.c | 6 +++
> tests/fchosttest.c | 15 ++++++
> 3 files changed, 142 insertions(+)
>
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index 68ed5ad..1d76096 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c
> @@ -2390,6 +2390,82 @@ nodeDeviceCheckParent(virConnectPtr conn,
>
> /**
> * @conn: Connection pointer
> + * @fchost: Pointer to the vHBA adapter
> + *
> + * If we have a valid connection, then use the node device create
> + * XML API rather than traversing through the sysfs to create the vHBA.
> + * Generate the Node Device XML using the Storage vHBA Adapter providing
> + * either the parent name, parent wwnn/wwpn, or parent fabric_name if
> + * available to the Node Device code. Since the Storage XML processing
> + * requires the wwnn/wwpn to be used for the vHBA to be provided (and
> + * not generated), we can use that as the fc_host wwnn/wwpn. This will
> + * allow for easier search later when we need it.
> + *
> + * Returns vHBA name on success, NULL on failure with an error message set
> + */
> +static char *
> +nodeDeviceCreateNodeDeviceVport(virConnectPtr conn,
> + virStorageAdapterFCHostPtr fchost)
> +{
> + virBuffer buf = VIR_BUFFER_INITIALIZER;
> + char *vhbaStr = NULL;
> + virNodeDevicePtr dev = NULL;
> + char *name;
> + bool created = false;
> +
> + /* If the nodedev already knows about this vHBA, then we're not
> + * managing it - we'll just use it. */
> + if ((dev = virNodeDeviceLookupSCSIHostByWWN(conn, fchost->wwnn,
> + fchost->wwpn, 0)))
I get nervous anytime I see a call to a toplevel public libvirt API
inside some low level function even when it's in one of the driver
directories. But this is in the conf directory. Is there a precedent for
doing that? It doesn't seem like a good idea - if it's called from
within any other nodedev function it could end up in a deadlock of the
driver-wide lock.
Is there some other organization that could make this cleaner? (haven't
thought about what yet)
More information about the libvir-list
mailing list