On 01/02/2017 09:25 AM, Ján Tomko wrote:
On Fri, Nov 18, 2016 at 09:26:30AM -0500, 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
These are all flattening the XML and duplicating the "parent" in their name.
Can we no longer put subelements in <parent> after we've used its contents as text?
I took the easy way out, but it seems the desire is one of the following: <parent>scsi_hostN</parent> <parent wwnn='' wwpn=''/> <parent fabric_name=''/> This appears to be "doable" (again apologies in advance for the cut-n-paste and email reader issues... I can repost the entire series, but figured I'd at least take a stab first to make sure we're on the same page): diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index f76204e..4666c9c 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -16,20 +16,7 @@ <element name="path"><text/></element> </optional> <optional> - <element name="parent"><text/></element> - </optional> - <optional> - <element name="parent_wwnn"> - <ref name='wwn'/> - </element> - <element name="parent_wwpn"> - <ref name='wwn'/> - </element> - </optional> - <optional> - <element name="parent_fabric_wwn"> - <ref name='wwn'/> - </element> + <ref name="parent"/> </optional> <optional> @@ -44,6 +31,28 @@ </element> </define> + <element name='parent'> + <optional> + <attribute name='wwnn'> + <ref name='wwn'/> + </attribute> + <attribute name='wwpn'> + <ref name='wwn'/> + </attribute> + </optional> + <optional> + <attribute name='fabric_wwn'> + <ref name='wwn'/> + </attribute> + </optional> + <choice> + <text/> + <empty/> + </choice> + </element> + </define> + <define name='capability'> <element name="capability"> <choice> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index ad7b699..4d3268f 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1724,15 +1724,15 @@ 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_wwnn = virXPathString("string(./parent[1]/@wwnn)", ctxt); + def->parent_wwpn = virXPathString("string(./parent[1]/@wwpn)", ctxt); if ((def->parent_wwnn && !def->parent_wwpn) || (!def->parent_wwnn && def->parent_wwpn)) { virReportError(VIR_ERR_XML_ERROR, "%s", _("must supply both wwnn and wwpn for parent")); goto error; } - def->parent_fabric_wwn = virXPathString("string(./parent_fabric_wwn[1])", + def->parent_fabric_wwn = virXPathString("string(./parent[1]/@fabric_wwn)", ctxt); /* Parse device capabilities */
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@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>
This allows either of {nn,pn} to be present, but the code expects both. Please put them in a single <optional> block if omitting either does not make sense.
OK
+ <optional> + <element name="parent_fabric_wwn"> + <ref name='wwn'/> + </element> + </optional>
<optional> <element name="driver">
@@ -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);
Should we error out if only one was provided?
Sure - I can add that - I did it in a separate local commit before making the above change... John
+ def->parent_fabric_wwn = virXPathString("string(./parent_fabric_wwn[1])", + ctxt);
/* Parse device capabilities */ nodes = NULL;
Weak ACK, I'd rather see the XML nested (if possible).
Jan