[libvirt] [PATCH 04/11] nodedev: Add the ability to create vHBA by parent wwnn/wwpn or fabric_wwn
Boris Fiuczynski
fiuczy at linux.vnet.ibm.com
Tue Dec 13 15:58:36 UTC 2016
John,
I have a concern regarding usage of the parent_fabric_name.
As far I have been told there are a lot of fc_host attributes in Linux
that are optional and left to the low-level driver to decide if
implemented. fabric_name apparently happens to be one of these
attributes and I know an architecture that has a driver (zfcp) that does
not provide it.
Up until this patch the fabric_name was collected on the host during
detection of the fc_host capability and provided during a nodedev dumpxml.
I would like to propose making the existence of fabric_name option in
the detection of the fc_host capability. In doing so it would also be
required make the fabric_name xml tag optional in the xml of the nodedev
dump. I am aware that this is a creating a slightly incompatible output
but how else could that be fixed than making the tag optional? I can
send a patch for this if you agree to the solution.
The implications regarding the search by parent_fabric_wwn should be
obvious.
On 11/18/2016 03:26 PM, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1349696
>
> When creating a vHBA, the process is to feed XML to nodeDeviceCreateXML
> that lists the <parent> scsi_hostX to use to create the vHBA. However,
> between reboots, it's possible that the <parent> changes its scsi_hostX
> to scsi_hostY and saved XML to perform the creation will either fail or
> create a vHBA using the wrong parent.
>
> So add the ability to provide <parent_wwnn> and <parent_wwpn> or
> <parent_fabric_wwn> in order to use those values as more consistent
> search parameters. For some providing the wwnn/wwpn pair will provide
> the most specific search option, while for others providing a fabric_wwn
> will at least ensure usage of the same SAN.
>
> This patch will add the new fields to the nodedev.rng for input purposes
> only since the input XML is essentially thrown away, no need to Format
> the values since they'd already be printed as part of the scsi_host
> data block.
>
> New API virNodeDeviceGetParentHostByWWNs will take the parent_wwnn and
> parent_wwpn in order to search the list of devices for matching capability
> data fields wwnn and wwpn.
>
> New API virNodeDeviceGetParentHostByFabricWWN will take the parent_fabric_wwn
> in order to search the list of devices for matching capability data field
> fabric_wwn.
>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
> docs/schemas/nodedev.rng | 15 +++++
> src/conf/node_device_conf.c | 120 +++++++++++++++++++++++++++++++++++
> src/conf/node_device_conf.h | 14 ++++
> src/libvirt_private.syms | 2 +
> src/node_device/node_device_driver.c | 13 ++++
> 5 files changed, 164 insertions(+)
>
> diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
> index 93a88d8..6fe49a3 100644
> --- a/docs/schemas/nodedev.rng
> +++ b/docs/schemas/nodedev.rng
> @@ -18,6 +18,21 @@
> <optional>
> <element name="parent"><text/></element>
> </optional>
> + <optional>
> + <element name="parent_wwnn">
> + <ref name='wwn'/>
> + </element>
> + </optional>
> + <optional>
> + <element name="parent_wwpn">
> + <ref name='wwn'/>
> + </element>
> + </optional>
> + <optional>
> + <element name="parent_fabric_wwn">
> + <ref name='wwn'/>
> + </element>
> + </optional>
>
> <optional>
> <element name="driver">
> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> index 3aa77cf..cecd915 100644
> --- a/src/conf/node_device_conf.c
> +++ b/src/conf/node_device_conf.c
> @@ -92,6 +92,30 @@ int virNodeDeviceHasCap(const virNodeDeviceObj *dev, const char *cap)
> }
>
>
> +/* virNodeDeviceFindFCCapDef:
> + * @dev: Pointer to current device
> + *
> + * Search the device object 'caps' array for fc_host capability.
> + *
> + * Returns:
> + * Pointer to the caps or NULL if not found
> + */
> +static virNodeDevCapsDefPtr
> +virNodeDeviceFindFCCapDef(const virNodeDeviceObj *dev)
> +{
> + virNodeDevCapsDefPtr caps = dev->def->caps;
> +
> + while (caps) {
> + if (caps->data.type == VIR_NODE_DEV_CAP_SCSI_HOST &&
> + (caps->data.scsi_host.flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST))
> + break;
> +
> + caps = caps->next;
> + }
> + return caps;
> +}
> +
> +
> /* virNodeDeviceFindVPORTCapDef:
> * @dev: Pointer to current device
> *
> @@ -152,6 +176,46 @@ virNodeDeviceObjPtr virNodeDeviceFindByName(virNodeDeviceObjListPtr devs,
>
>
> static virNodeDeviceObjPtr
> +virNodeDeviceFindByWWNs(virNodeDeviceObjListPtr devs,
> + const char *parent_wwnn,
> + const char *parent_wwpn)
> +{
> + size_t i;
> +
> + for (i = 0; i < devs->count; i++) {
> + virNodeDevCapsDefPtr cap;
> + virNodeDeviceObjLock(devs->objs[i]);
> + if ((cap = virNodeDeviceFindFCCapDef(devs->objs[i])) &&
> + STREQ_NULLABLE(cap->data.scsi_host.wwnn, parent_wwnn) &&
> + STREQ_NULLABLE(cap->data.scsi_host.wwpn, parent_wwpn))
> + return devs->objs[i];
> + virNodeDeviceObjUnlock(devs->objs[i]);
> + }
> +
> + return NULL;
> +}
> +
> +
> +static virNodeDeviceObjPtr
> +virNodeDeviceFindByFabricWWN(virNodeDeviceObjListPtr devs,
> + const char *parent_fabric_wwn)
> +{
> + size_t i;
> +
> + for (i = 0; i < devs->count; i++) {
> + virNodeDevCapsDefPtr cap;
> + virNodeDeviceObjLock(devs->objs[i]);
> + if ((cap = virNodeDeviceFindFCCapDef(devs->objs[i])) &&
> + STREQ_NULLABLE(cap->data.scsi_host.fabric_wwn, parent_fabric_wwn))
> + return devs->objs[i];
> + virNodeDeviceObjUnlock(devs->objs[i]);
> + }
> +
> + return NULL;
> +}
> +
> +
> +static virNodeDeviceObjPtr
> virNodeDeviceFindByCap(virNodeDeviceObjListPtr devs,
> const char *cap)
> {
> @@ -177,6 +241,9 @@ void virNodeDeviceDefFree(virNodeDeviceDefPtr def)
>
> VIR_FREE(def->name);
> VIR_FREE(def->parent);
> + VIR_FREE(def->parent_wwnn);
> + VIR_FREE(def->parent_wwpn);
> + VIR_FREE(def->parent_fabric_wwn);
> VIR_FREE(def->driver);
> VIR_FREE(def->sysfs_path);
> VIR_FREE(def->parent_sysfs_path);
> @@ -1652,6 +1719,10 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
>
> /* Extract device parent, if any */
> def->parent = virXPathString("string(./parent[1])", ctxt);
> + def->parent_wwnn = virXPathString("string(./parent_wwnn[1])", ctxt);
> + def->parent_wwpn = virXPathString("string(./parent_wwpn[1])", ctxt);
> + def->parent_fabric_wwn = virXPathString("string(./parent_fabric_wwn[1])",
> + ctxt);
>
> /* Parse device capabilities */
> nodes = NULL;
> @@ -1847,6 +1918,55 @@ virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs,
>
>
> int
> +virNodeDeviceGetParentHostByWWNs(virNodeDeviceObjListPtr devs,
> + const char *dev_name,
> + const char *parent_wwnn,
> + const char *parent_wwpn,
> + int *parent_host)
> +{
> + virNodeDeviceObjPtr parent = NULL;
> + int ret;
> +
> + if (!(parent = virNodeDeviceFindByWWNs(devs, parent_wwnn, parent_wwpn))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not find parent device for '%s'"),
> + dev_name);
> + return -1;
> + }
> +
> + ret = virNodeDeviceFindFCParentHost(parent, parent_host);
> +
> + virNodeDeviceObjUnlock(parent);
> +
> + return ret;
> +}
> +
> +
> +int
> +virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs,
> + const char *dev_name,
> + const char *parent_fabric_wwn,
> + int *parent_host)
> +{
> + virNodeDeviceObjPtr parent = NULL;
> + int ret;
> +
> + if (!(parent = virNodeDeviceFindByFabricWWN(devs, parent_fabric_wwn))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not find parent device for '%s'"),
> + dev_name);
> + return -1;
> + }
> +
> + ret = virNodeDeviceFindFCParentHost(parent, parent_host);
> +
> + virNodeDeviceObjUnlock(parent);
> +
> + return ret;
> +}
> +
> +
> +int
> virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs,
> int *parent_host)
> {
> diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
> index 2b2aed7..1634483 100644
> --- a/src/conf/node_device_conf.h
> +++ b/src/conf/node_device_conf.h
> @@ -200,6 +200,9 @@ struct _virNodeDeviceDef {
> char *sysfs_path; /* udev name/sysfs path */
> char *parent; /* optional parent device name */
> char *parent_sysfs_path; /* udev parent name/sysfs path */
> + char *parent_wwnn; /* optional parent wwnn */
> + char *parent_wwpn; /* optional parent wwpn */
> + char *parent_fabric_wwn; /* optional parent fabric_wwn */
> char *driver; /* optional driver name */
> virNodeDevCapsDefPtr caps; /* optional device capabilities */
> };
> @@ -273,6 +276,17 @@ int virNodeDeviceGetParentHost(virNodeDeviceObjListPtr devs,
> const char *parent_name,
> int *parent_host);
>
> +int virNodeDeviceGetParentHostByWWNs(virNodeDeviceObjListPtr devs,
> + const char *dev_name,
> + const char *parent_wwnn,
> + const char *parent_wwpn,
> + int *parent_host);
> +
> +int virNodeDeviceGetParentHostByFabricWWN(virNodeDeviceObjListPtr devs,
> + const char *dev_name,
> + const char *parent_fabric_wwn,
> + int *parent_host);
> +
> int virNodeDeviceFindVportParentHost(virNodeDeviceObjListPtr devs,
> int *parent_host);
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index de14a7e..5673bda 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -701,6 +701,8 @@ virNodeDeviceFindByName;
> virNodeDeviceFindBySysfsPath;
> virNodeDeviceFindVportParentHost;
> virNodeDeviceGetParentHost;
> +virNodeDeviceGetParentHostByFabricWWN;
> +virNodeDeviceGetParentHostByWWNs;
> virNodeDeviceGetWWNs;
> virNodeDeviceHasCap;
> virNodeDeviceObjListExport;
> diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
> index 0e091fe..b7bec65 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -590,6 +590,19 @@ nodeDeviceCreateXML(virConnectPtr conn,
> def->parent,
> &parent_host) < 0)
> goto cleanup;
> + } else if (def->parent_wwnn && def->parent_wwpn) {
> + if (virNodeDeviceGetParentHostByWWNs(&driver->devs,
> + def->name,
> + def->parent_wwnn,
> + def->parent_wwpn,
> + &parent_host) < 0)
> + goto cleanup;
> + } else if (def->parent_fabric_wwn) {
> + if (virNodeDeviceGetParentHostByFabricWWN(&driver->devs,
> + def->name,
> + def->parent_fabric_wwn,
> + &parent_host) < 0)
> + goto cleanup;
> } else {
> /* Try to find "a" vport capable scsi_host when no parent supplied */
> if (virNodeDeviceFindVportParentHost(&driver->devs, &parent_host) < 0)
>
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
More information about the libvir-list
mailing list