On Fri, Nov 06, 2009 at 04:28:02AM +0100, Matthias Bolte wrote:
phypNumDomainsGeneric() and phypListDomainsGeneric() return 0 in
case
of an error. This makes it impossible to distinguish between an actual
error and no domains being defined on the hypervisor. It also turn the
no domains situation into an error. Return -1 in case of an error to
fix this problem.
---
src/phyp/phyp_driver.c | 27 ++++++++++++++++++++++-----
1 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index b94d0fa..8d54ae7 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -980,7 +980,7 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type)
err:
VIR_FREE(cmd);
VIR_FREE(ret);
- return 0;
+ return -1;
}
static int
@@ -1065,7 +1065,7 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids,
err:
VIR_FREE(cmd);
VIR_FREE(ret);
- return 0;
+ return -1;
}
static int
@@ -1130,7 +1130,7 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int
nnames)
VIR_FREE(names[i]);
VIR_FREE(cmd);
VIR_FREE(ret);
- return 0;
+ return -1;
}
static virDomainPtr
@@ -1843,17 +1843,34 @@ phypUUIDTable_Init(virConnectPtr conn)
int *ids = NULL;
unsigned int i = 0;
- if ((nids = phypNumDomainsGeneric(conn, 2)) == 0)
+ if ((nids = phypNumDomainsGeneric(conn, 2)) < 0)
goto err;
+ /* exit early if there are no domains */
+ if (nids == 0)
+ return 0;
+
if (VIR_ALLOC_N(ids, nids) < 0) {
virReportOOMError(conn);
goto err;
}
- if (phypListDomainsGeneric(conn, ids, nids, 1) == 0)
+ if ((nids = phypListDomainsGeneric(conn, ids, nids, 1)) < 0)
goto err;
+ /* exit early if there are no domains */
+ /* FIXME: phypNumDomainsGeneric() returned > 0 but phypListDomainsGeneric()
+ * returned 0. indicates this an error condition?
+ * an even stricter check would be to treat
+ *
+ * phypNumDomainsGeneric() != phypListDomainsGeneric()
+ *
+ * as an error */
+ if (nids == 0) {
+ VIR_FREE(ids);
+ return 0;
+ }
+
phyp_driver = conn->privateData;
uuid_table = phyp_driver->uuid_table;
uuid_table->nlpars = nids;
Yes I think it's needed to fix the semantic of phypListDefinedDomains
and phypNumDefinedDomains which are just wrappers around this static
function, and those are public entry points,
ACK
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit
http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine
http://rpmfind.net/
http://veillard.com/ | virtualization library
http://libvirt.org/