Re: [libvirt] [PATCH v3 01/18] conf: Ensure both parent_wwnn/parent_wwpn provided
On 03/10/2017 04:10 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1428209
Commit id 'bb74a7ffe' neglected to check that both the parent_wwnn
parent_wwpn are in the XML if one or the other is similar to how
the node device code checked (commit id '2b13361bc').
If only one is provided, the "default" is to use a vHBA capable
adapter (see commit id '78be2e8b'), so the vHBA could start, but
perhaps not on the expected adapter.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/storage_conf.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index a52eeba..e4d89dd 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -941,6 +941,17 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
if (!virValidateWWN(ret->source.adapter.data.fchost.wwnn) ||
!virValidateWWN(ret->source.adapter.data.fchost.wwpn))
goto error;
+
+ if ((ret->source.adapter.data.fchost.parent_wwnn &&
+ !ret->source.adapter.data.fchost.parent_wwpn) ||
+ (!ret->source.adapter.data.fchost.parent_wwnn &&
+ ret->source.adapter.data.fchost.parent_wwpn)) {
If you want to make this more compact, you could use an XOR. Since there isn't a
logical XOR operator in C, you could do this:
if (!!ret->source.adapter.data.fchost.parent_wwnn ^
!!ret->source.adapter.data.fchost.parent_wwpn)
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("must supply both parent_wwnn and "
+ "parent_wwpn not just one or the other"));
+ goto error;
+ }
+
Is there any other identifiable info available at this point that could give more useful
context to the log message? If nothing else, maybe log whichever of the two values is set
so there would be something for the user to search on when looking for the offending xml.
(I see the error log in commit 2b13361bc has the same problem).
} else if (ret->source.adapter.type ==
VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
if (!ret->source.adapter.data.scsi_host.name &&
ACK, but I'd prefer if the error log gave more identifiable information.