2:27 p.m.
The first patch is related to review comments from my Converge Storage
Pool vHBA logic Patch1 w/r/t a weak error message
The second patch is pulled from Patch17 of that series and is just an
adjustment in the storage pool create xml logic
Neither of these require news.xml updates...
John Ferlan (2):
conf: Alter error message for vHBA creation using parent wwnn/wwpn
test: Don't assume a configFile exists for Storage Pool tests
src/conf/node_device_conf.c | 17 +++++++++++++----
src/test/test_driver.c | 6 ++++++
2 files changed, 19 insertions(+), 4 deletions(-)
--
2.9.3
2:27 p.m.
New subject: [libvirt] [PATCH 1/2] conf: Alter error message for vHBA creation using parent wwnn/wwpn
Commit id 'bb74a7ffe' added a fairly non specific message when providing
only the <parent wwnn='xxx'/> or <parent wwpn='xxx'/> instead
of providing
both wwnn and wwpn. This patch just modifies the message to be more specific
about which was missing.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/node_device_conf.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 3565aec..2b49f5c 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -1615,10 +1615,19 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
def->parent = virXPathString("string(./parent[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"));
+ if (def->parent_wwnn && !def->parent_wwpn) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("when providing parent wwnn='%s', the "
+ "wwpn must also be provided"),
+ def->parent_wwnn);
+ goto error;
+ }
+
+ if (!def->parent_wwnn && def->parent_wwpn) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("when providing parent wwpn='%s', the "
+ "wwnn must also be provided"),
+ def->parent_wwpn);
goto error;
}
def->parent_fabric_wwn =
virXPathString("string(./parent[1]/@fabric_wwn)",
--
2.9.3
5:01 p.m.
New subject: [libvirt] [PATCH 1/2] conf: Alter error message for vHBA creation using parent wwnn/wwpn
On 03/15/2017 03:27 PM, John Ferlan wrote:
Commit id 'bb74a7ffe' added a fairly non specific message
when providing
only the <parent wwnn='xxx'/> or <parent wwpn='xxx'/> instead
of providing
both wwnn and wwpn. This patch just modifies the message to be more specific
about which was missing.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
ACK.
2:27 p.m.
New subject: [libvirt] [PATCH 2/2] test: Don't assume a configFile exists for Storage Pool tests
Fix a "bug" in the storage pool test driver code which "assumed"
testStoragePoolObjSetDefaults should fill in the configFile for
both the Define/Create (persistent) and CreateXML (transient) pools
by just VIR_FREE()'ing it during CreateXML. Because the configFile
was filled in, during Destroy the pool wouldn't be free'd which
could cause issues for future patches which add tests to validate
vHBA creation for the storage pool using the same name.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/test/test_driver.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index cf7820a..361d62e 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4440,6 +4440,12 @@ testStoragePoolCreateXML(virConnectPtr conn,
pool = NULL;
goto cleanup;
}
+
+ /* *SetDefaults fills this in for the persistent pools, but this
+ * would be a transient pool so remove it; otherwise, the Destroy
+ * code will not Remove the pool */
+ VIR_FREE(pool->configFile);
+
pool->active = 1;
event = virStoragePoolEventLifecycleNew(pool->def->name,
pool->def->uuid,
--
2.9.3
5:05 p.m.
New subject: [libvirt] [PATCH 2/2] test: Don't assume a configFile exists for Storage Pool tests
On 03/15/2017 03:27 PM, John Ferlan wrote:
Fix a "bug" in the storage pool test driver code which
"assumed"
testStoragePoolObjSetDefaults should fill in the configFile for
both the Define/Create (persistent) and CreateXML (transient) pools
by just VIR_FREE()'ing it during CreateXML. Because the configFile
was filled in, during Destroy the pool wouldn't be free'd which
could cause issues for future patches which add tests to validate
vHBA creation for the storage pool using the same name.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/test/test_driver.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index cf7820a..361d62e 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4440,6 +4440,12 @@ testStoragePoolCreateXML(virConnectPtr conn,
pool = NULL;
goto cleanup;
}
+
+ /* *SetDefaults fills this in for the persistent pools, but this
+ * would be a transient pool so remove it; otherwise, the Destroy
+ * code will not Remove the pool */
+ VIR_FREE(pool->configFile);
+
pool->active = 1;
event = virStoragePoolEventLifecycleNew(pool->def->name,
pool->def->uuid,
After looking at the context, this makes sense.
ACK.
2840
days inactive
2840
days old
4 comments
2 participants
participants (2)
-
John Ferlan -
Laine Stump