[libvirt] [PATCH 04/11] nodedev: Add the ability to create vHBA by parent wwnn/wwpn or fabric_wwn
John Ferlan
jferlan at redhat.com
Tue Jan 3 21:46:43 UTC 2017
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 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>
>
> 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
More information about the libvir-list
mailing list