On Wed, Jan 25, 2017 at 03:27:33PM -0500, John Ferlan wrote:
> Move the function and rename to simply virVHBAGetParent
>
> Since I'm changing related code, rather than have a Vhba named function
> in storage_backend_scsi rename it to simply checkParent as it's already
> within the scope of checking VHBA related features.
This is Vhba named function but it isn't only VHBA related code.
With some modification it could be generic node device function to
get a parent of a device specified by it's name. The only VHBA related
code in this function is construction of the node device name.
Even without this context we should not import anything from src/conf/*.h
in the src/util/* code.
NACK
Pavel
>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> src/conf/storage_conf.c | 61 +----------------------------------
> src/conf/storage_conf.h | 5 ---
> src/libvirt_private.syms | 2 +-
> src/storage/storage_backend_scsi.c | 12 +++----
> src/util/virvhba.c | 65 ++++++++++++++++++++++++++++++++++++++
> src/util/virvhba.h | 4 +++
> 6 files changed, 77 insertions(+), 72 deletions(-)
>
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index f8b5228..5e13bbf 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -2264,64 +2264,6 @@ virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools,
> return ret;
> }
>
> -/*
> - * virStoragePoolGetVhbaSCSIHostParent:
> - *
> - * Using the Node Device Driver, find the host# name found via wwnn/wwpn
> - * lookup in the fc_host sysfs tree (e.g. virGetFCHostNameByWWN) to get
> - * the parent 'scsi_host#'.
> - *
> - * @conn: Connection pointer (must be non-NULL on entry)
> - * @name: Pointer a string from a virGetFCHostNameByWWN (e.g., "host#")
> - *
> - * Returns a "scsi_host#" string of the parent of the vHBA
> - */
> -char *
> -virStoragePoolGetVhbaSCSIHostParent(virConnectPtr conn,
> - const char *name)
> -{
> - char *nodedev_name = NULL;
> - virNodeDevicePtr device = NULL;
> - char *xml = NULL;
> - virNodeDeviceDefPtr def = NULL;
> - char *vhba_parent = NULL;
> -
> - VIR_DEBUG("conn=%p, name=%s", conn, name);
> -
> - /* We get passed "host#" from the return from virGetFCHostNameByWWN,
> - * so we need to adjust that to what the nodedev lookup expects
> - */
> - if (virAsprintf(&nodedev_name, "scsi_%s", name) < 0)
> - goto cleanup;
> -
> - /* Compare the scsi_host for the name with the provided parent
> - * if not the same, then fail
> - */
> - if (!(device = virNodeDeviceLookupByName(conn, nodedev_name))) {
> - virReportError(VIR_ERR_XML_ERROR,
> - _("Cannot find '%s' in node device database"),
> - nodedev_name);
> - goto cleanup;
> - }
> -
> - if (!(xml = virNodeDeviceGetXMLDesc(device, 0)))
> - goto cleanup;
> -
> - if (!(def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL)))
> - goto cleanup;
> -
> - /* The caller checks whether the returned value is NULL or not
> - * before continuing
> - */
> - ignore_value(VIR_STRDUP(vhba_parent, def->parent));
> -
> - cleanup:
> - VIR_FREE(nodedev_name);
> - virNodeDeviceDefFree(def);
> - VIR_FREE(xml);
> - virObjectUnref(device);
> - return vhba_parent;
> -}
>
> static int
> getSCSIHostNumber(virStoragePoolSourceAdapter adapter,
> @@ -2404,8 +2346,7 @@ matchFCHostToSCSIHost(virConnectPtr conn,
> * have a match.
> */
> if (conn && !fc_adapter.data.fchost.parent) {
> - parent_name = virStoragePoolGetVhbaSCSIHostParent(conn, name);
> - if (parent_name) {
> + if ((parent_name = virVHBAGetParent(conn, name))) {
> if (virGetSCSIHostNumber(parent_name, &fc_hostnum) == 0 &&
> scsi_hostnum == fc_hostnum) {
> VIR_FREE(parent_name);
> diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
> index b35471d..426e8b8 100644
> --- a/src/conf/storage_conf.h
> +++ b/src/conf/storage_conf.h
> @@ -30,7 +30,6 @@
> # include "virbitmap.h"
> # include "virthread.h"
> # include "device_conf.h"
> -# include "node_device_conf.h"
> # include "object_event.h"
>
> # include <libxml/tree.h>
> @@ -417,10 +416,6 @@ int virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools,
> virStoragePoolDefPtr def,
> unsigned int check_active);
>
> -char *virStoragePoolGetVhbaSCSIHostParent(virConnectPtr conn,
> - const char *name)
> - ATTRIBUTE_NONNULL(1);
> -
> int virStoragePoolSourceFindDuplicate(virConnectPtr conn,
> virStoragePoolObjListPtr pools,
> virStoragePoolDefPtr def);
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 429c133..b58742d 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -887,7 +887,6 @@ virStoragePoolFormatDiskTypeToString;
> virStoragePoolFormatFileSystemNetTypeToString;
> virStoragePoolFormatFileSystemTypeToString;
> virStoragePoolFormatLogicalTypeToString;
> -virStoragePoolGetVhbaSCSIHostParent;
> virStoragePoolLoadAllConfigs;
> virStoragePoolLoadAllState;
> virStoragePoolObjAssignDef;
> @@ -2744,6 +2743,7 @@ virVHBAFindVportHost;
> virVHBAGetConfig;
> virVHBAGetHostByFabricWWN;
> virVHBAGetHostByWWN;
> +virVHBAGetParent;
> virVHBAIsVportCapable;
> virVHBAManageVport;
> virVHBAPathExists;
> diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
> index e037a93..2da15dd 100644
> --- a/src/storage/storage_backend_scsi.c
> +++ b/src/storage/storage_backend_scsi.c
> @@ -214,9 +214,9 @@ getAdapterName(virStoragePoolSourceAdapter adapter)
> * sysfs tree to get the parent 'scsi_host#' to ensure it matches.
> */
> static bool
> -checkVhbaSCSIHostParent(virConnectPtr conn,
> - const char *name,
> - const char *parent_name)
> +checkParent(virConnectPtr conn,
> + const char *name,
> + const char *parent_name)
> {
> char *vhba_parent = NULL;
> bool retval = false;
> @@ -227,7 +227,7 @@ checkVhbaSCSIHostParent(virConnectPtr conn,
> if (!conn)
> return true;
>
> - if (!(vhba_parent = virStoragePoolGetVhbaSCSIHostParent(conn, name)))
> + if (!(vhba_parent = virVHBAGetParent(conn, name)))
> goto cleanup;
>
> if (STRNEQ(parent_name, vhba_parent)) {
> @@ -276,7 +276,7 @@ createVport(virConnectPtr conn,
> * retrieved has the same parent
> */
> if (adapter->data.fchost.parent &&
> - checkVhbaSCSIHostParent(conn, name, adapter->data.fchost.parent))
> + checkParent(conn, name, adapter->data.fchost.parent))
> ret = 0;
>
> goto cleanup;
> @@ -416,7 +416,7 @@ deleteVport(virConnectPtr conn,
> if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0)
> goto cleanup;
> } else {
> - if (!(vhba_parent = virStoragePoolGetVhbaSCSIHostParent(conn, name)))
> + if (!(vhba_parent = virVHBAGetParent(conn, name)))
> goto cleanup;
>
> if (virGetSCSIHostNumber(vhba_parent, &parent_host) < 0)
> diff --git a/src/util/virvhba.c b/src/util/virvhba.c
> index e5637d7..8c4da3a 100644
> --- a/src/util/virvhba.c
> +++ b/src/util/virvhba.c
> @@ -18,6 +18,8 @@
>
> #include <config.h>
>
> +#include "node_device_conf.h"
> +
> #include "viralloc.h"
> #include "virerror.h"
> #include "virfile.h"
> @@ -530,3 +532,66 @@ virVHBAGetHostByFabricWWN(const char *sysfs_prefix ATTRIBUTE_UNUSED,
> }
>
> #endif /* __linux__ */
> +
> +
> +/* The following will be more generic - using the Node Device driver in
> + * order to perform the lookup rather than looking through the file system
> + * in order to find the answer */
> +/*
> + * virVHBAGetParent:
> + *
> + * Using the Node Device Driver, find the host# name found via wwnn/wwpn
> + * lookup in the fc_host sysfs tree (e.g. virGetFCHostNameByWWN) to get
> + * the parent 'scsi_host#'.
> + *
> + * @conn: Connection pointer (must be non-NULL on entry)
> + * @name: Pointer a string from a virGetFCHostNameByWWN (e.g., "host#")
> + *
> + * Returns a "scsi_host#" string of the parent of the vHBA
> + */
> +char *
> +virVHBAGetParent(virConnectPtr conn,
> + const char *name)
> +{
> + char *nodedev_name = NULL;
> + virNodeDevicePtr device = NULL;
> + char *xml = NULL;
> + virNodeDeviceDefPtr def = NULL;
> + char *vhba_parent = NULL;
> +
> + VIR_DEBUG("conn=%p, name=%s", conn, name);
> +
> + /* We get passed "host#" from the return from virGetFCHostNameByWWN,
> + * so we need to adjust that to what the nodedev lookup expects
> + */
> + if (virAsprintf(&nodedev_name, "scsi_%s", name) < 0)
> + goto cleanup;
> +
> + /* Compare the scsi_host for the name with the provided parent
> + * if not the same, then fail
> + */
> + if (!(device = virNodeDeviceLookupByName(conn, nodedev_name))) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("Cannot find '%s' in node device database"),
> + nodedev_name);
> + goto cleanup;
> + }
> +
> + if (!(xml = virNodeDeviceGetXMLDesc(device, 0)))
> + goto cleanup;
> +
> + if (!(def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL)))
> + goto cleanup;
> +
> + /* The caller checks whether the returned value is NULL or not
> + * before continuing
> + */
> + ignore_value(VIR_STRDUP(vhba_parent, def->parent));
> +
> + cleanup:
> + VIR_FREE(nodedev_name);
> + virNodeDeviceDefFree(def);
> + VIR_FREE(xml);
> + virObjectUnref(device);
> + return vhba_parent;
> +}
> diff --git a/src/util/virvhba.h b/src/util/virvhba.h
> index e338f96..b8d73ab 100644
> --- a/src/util/virvhba.h
> +++ b/src/util/virvhba.h
> @@ -52,4 +52,8 @@ char *virVHBAGetHostByFabricWWN(const char *sysfs_prefix,
> const char *fabric_wwn)
> ATTRIBUTE_NONNULL(2);
>
> +char *virVHBAGetParent(virConnectPtr conn,
> + const char *name)
> + ATTRIBUTE_NONNULL(1);
> +
> #endif /* __VIR_VBHA_H__ */
> --
> 2.7.4
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
