[libvirt] [PATCH 0/9] Several bug fixes for the Power Hypervisor driver

After the last large patch has been applied I reviewed the whole driver code and found several bugs and issues. Most important memory leaks and two potential infinite loops. I've no Power Hypervisor at hand, so I can't test this changes with an actual system. Eduardo could you test this patches to make sure they don't break existing functionality? Matthias

* configure.in: add -L$libssh2_path to LIBS for the AC_TRY_LINK check --- configure.in | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/configure.in b/configure.in index 7afbc7c..7ad1a90 100644 --- a/configure.in +++ b/configure.in @@ -977,7 +977,7 @@ if test "$with_phyp" = "check"; then if test "$with_phyp" != "no"; then saved_libs="$LIBS" - LIBS="$LIBS -lssh2" + LIBS="$LIBS -L$libssh2_path -lssh2" AC_TRY_LINK([#include <libssh2.h>], [ (void) libssh2_session_block_directions(NULL); ], [ -- 1.6.0.4

On Fri, Nov 06, 2009 at 04:27:59AM +0100, Matthias Bolte wrote:
* configure.in: add -L$libssh2_path to LIBS for the AC_TRY_LINK check --- configure.in | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/configure.in b/configure.in index 7afbc7c..7ad1a90 100644 --- a/configure.in +++ b/configure.in @@ -977,7 +977,7 @@ if test "$with_phyp" = "check"; then
if test "$with_phyp" != "no"; then saved_libs="$LIBS" - LIBS="$LIBS -lssh2" + LIBS="$LIBS -L$libssh2_path -lssh2" AC_TRY_LINK([#include <libssh2.h>], [ (void) libssh2_session_block_directions(NULL); ], [
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

--- src/phyp/phyp_driver.c | 285 ++++++++++++++++++++++++++++------------------- src/phyp/phyp_driver.h | 2 + 2 files changed, 172 insertions(+), 115 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 8e199ee..5379cd3 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -71,7 +71,7 @@ phypOpen(virConnectPtr conn, { LIBSSH2_SESSION *session = NULL; ConnectionData *connection_data = NULL; - char *string; + char *string = NULL; size_t len = 0; int internal_socket; uuid_tablePtr uuid_table = NULL; @@ -128,19 +128,17 @@ phypOpen(virConnectPtr conn, goto failure; } - if (VIR_ALLOC_N(managed_system, len) < 0) { + /* need to shift one byte in order to remove the first "/" of URI component */ + if (conn->uri->path[0] == '/') + managed_system = strdup(conn->uri->path + 1); + else + managed_system = strdup(conn->uri->path); + + if (!managed_system) { virReportOOMError(conn); goto failure; } - managed_system = strdup(conn->uri->path); - if (!managed_system) - goto failure; - - /* need to shift one byte in order to remove the first "/" of URI component */ - if (managed_system[0] == '/') - managed_system++; - /* here we are handling only the first component of the path, * so skipping the second: * */ @@ -187,11 +185,19 @@ phypOpen(virConnectPtr conn, return VIR_DRV_OPEN_SUCCESS; failure: - virCapabilitiesFree(phyp_driver->caps); - VIR_FREE(phyp_driver->managed_system); - VIR_FREE(phyp_driver); - VIR_FREE(uuid_table); - VIR_FREE(uuid_table->lpars); + if (phyp_driver != NULL) { + virCapabilitiesFree(phyp_driver->caps); + VIR_FREE(phyp_driver->managed_system); + VIR_FREE(phyp_driver); + } + + phypUUIDTable_Free(uuid_table); + + if (session != NULL) { + libssh2_session_disconnect(session, "Disconnecting..."); + libssh2_session_free(session); + } + VIR_FREE(connection_data); VIR_FREE(string); @@ -209,8 +215,7 @@ phypClose(virConnectPtr conn) libssh2_session_free(session); virCapabilitiesFree(phyp_driver->caps); - VIR_FREE(phyp_driver->uuid_table); - VIR_FREE(phyp_driver->uuid_table->lpars); + phypUUIDTable_Free(phyp_driver->uuid_table); VIR_FREE(phyp_driver->managed_system); VIR_FREE(phyp_driver); VIR_FREE(connection_data); @@ -281,7 +286,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", _("Failure establishing SSH session.")); - goto err; + goto disconnect; } /* Trying authentication by pubkey */ @@ -305,7 +310,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", _("No authentication callback provided.")); - goto err; + goto disconnect; } for (i = 0; i < auth->ncredtype; i++) { @@ -317,7 +322,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", _("Required credentials are not supported.")); - goto err; + goto disconnect; } int res = @@ -327,7 +332,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", _("Unable to fetch credentials.")); - goto err; + goto disconnect; } if (creds[0].result) { @@ -336,9 +341,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", _("Unable to get password certificates")); - libssh2_session_disconnect(session, "Disconnecting..."); - libssh2_session_free(session); - goto err; + goto disconnect; } while ((rc = @@ -350,16 +353,19 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", _("Authentication failed")); - libssh2_session_disconnect(session, "Disconnecting"); - libssh2_session_free(session); - goto err; + goto disconnect; } else goto exit; } + disconnect: + libssh2_session_disconnect(session, "Disconnecting..."); + libssh2_session_free(session); err: + VIR_FREE(password); return NULL; exit: + VIR_FREE(password); return session; } @@ -456,7 +462,8 @@ phypGetLparID(LIBSSH2_SESSION * session, const char *managed_system, int exit_status = 0; int lpar_id = 0; char *char_ptr; - char *cmd; + char *cmd = NULL; + char *ret = NULL; if (virAsprintf(&cmd, "lssyscfg -r lpar -m %s --filter lpar_names=%s -F lpar_id", @@ -465,7 +472,7 @@ phypGetLparID(LIBSSH2_SESSION * session, const char *managed_system, goto err; } - const char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn); if (exit_status < 0 || ret == NULL) goto err; @@ -474,10 +481,12 @@ phypGetLparID(LIBSSH2_SESSION * session, const char *managed_system, goto err; VIR_FREE(cmd); + VIR_FREE(ret); return lpar_id; err: VIR_FREE(cmd); + VIR_FREE(ret); return -1; } @@ -486,7 +495,8 @@ static char * phypGetLparNAME(LIBSSH2_SESSION * session, const char *managed_system, unsigned int lpar_id, virConnectPtr conn) { - char *cmd; + char *cmd = NULL; + char *ret = NULL; int exit_status = 0; if (virAsprintf(&cmd, @@ -496,7 +506,7 @@ phypGetLparNAME(LIBSSH2_SESSION * session, const char *managed_system, goto err; } - char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn); if (ret == NULL) goto err; @@ -514,6 +524,7 @@ phypGetLparNAME(LIBSSH2_SESSION * session, const char *managed_system, err: VIR_FREE(cmd); + VIR_FREE(ret); return NULL; } @@ -553,7 +564,8 @@ phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id, { ConnectionData *connection_data = conn->networkPrivateData; LIBSSH2_SESSION *session = connection_data->session; - char *cmd; + char *cmd = NULL; + char *ret = NULL; char *char_ptr; int memory = 0; int exit_status = 0; @@ -579,7 +591,7 @@ phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id, } } - char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn); if (ret == NULL) goto err; @@ -596,10 +608,12 @@ phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id, goto err; VIR_FREE(cmd); + VIR_FREE(ret); return memory; err: VIR_FREE(cmd); + VIR_FREE(ret); return 0; } @@ -625,7 +639,8 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, { ConnectionData *connection_data = conn->networkPrivateData; LIBSSH2_SESSION *session = connection_data->session; - char *cmd; + char *cmd = NULL; + char *ret = NULL; int exit_status = 0; int vcpus = 0; @@ -646,7 +661,7 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, goto err; } } - char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn); if (ret == NULL) goto err; @@ -663,10 +678,12 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, goto err; VIR_FREE(cmd); + VIR_FREE(ret); return (unsigned long) vcpus; err: VIR_FREE(cmd); + VIR_FREE(ret); return 0; } @@ -676,7 +693,8 @@ phypGetRemoteSlot(virConnectPtr conn, const char *managed_system, { ConnectionData *connection_data = conn->networkPrivateData; LIBSSH2_SESSION *session = connection_data->session; - char *cmd; + char *cmd = NULL; + char *ret = NULL; char *char_ptr; int remote_slot = 0; int exit_status = 0; @@ -688,7 +706,7 @@ phypGetRemoteSlot(virConnectPtr conn, const char *managed_system, virReportOOMError(conn); goto err; } - char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn); if (ret == NULL) goto err; @@ -705,10 +723,12 @@ phypGetRemoteSlot(virConnectPtr conn, const char *managed_system, goto err; VIR_FREE(cmd); + VIR_FREE(ret); return remote_slot; err: VIR_FREE(cmd); + VIR_FREE(ret); return -1; } @@ -718,9 +738,12 @@ phypGetBackingDevice(virConnectPtr conn, const char *managed_system, { ConnectionData *connection_data = conn->networkPrivateData; LIBSSH2_SESSION *session = connection_data->session; - char *cmd; + char *cmd = NULL; + char *ret = NULL; int remote_slot = 0; int exit_status = 0; + char *char_ptr; + char *backing_device = NULL; if ((remote_slot = phypGetRemoteSlot(conn, managed_system, lpar_name)) == -1) @@ -734,9 +757,9 @@ phypGetBackingDevice(virConnectPtr conn, const char *managed_system, goto err; } - char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn); - if (ret == NULL) + if (exit_status < 0 || ret == NULL) goto err; /* here is a little trick to deal returns of this kind: @@ -746,31 +769,38 @@ phypGetBackingDevice(virConnectPtr conn, const char *managed_system, * the information we really need is only lv01, so we * need to skip a lot of things on the string. * */ - char *backing_device = strchr(ret, '/'); + char_ptr = strchr(ret, '/'); - if (backing_device) { - backing_device++; - if (backing_device[0] == '/') - backing_device++; + if (char_ptr) { + char_ptr++; + if (char_ptr[0] == '/') + char_ptr++; else goto err; + + backing_device = strdup(char_ptr); + + if (backing_device == NULL) { + virReportOOMError(conn); + goto err; + } } else { backing_device = ret; + ret = NULL; } - char *char_ptr = strchr(backing_device, '\n'); + char_ptr = strchr(backing_device, '\n'); if (char_ptr) *char_ptr = '\0'; - if (exit_status < 0 || backing_device == NULL) - goto err; - VIR_FREE(cmd); + VIR_FREE(ret); return backing_device; err: VIR_FREE(cmd); + VIR_FREE(ret); return NULL; } @@ -781,44 +811,41 @@ phypGetLparState(virConnectPtr conn, unsigned int lpar_id) ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; LIBSSH2_SESSION *session = connection_data->session; - char *cmd; + char *cmd = NULL; + char *ret = NULL; int exit_status = 0; char *char_ptr = NULL; char *managed_system = phyp_driver->managed_system; + int state = VIR_DOMAIN_NOSTATE; if (virAsprintf(&cmd, "lssyscfg -r lpar -m %s -F state --filter lpar_ids=%d", managed_system, lpar_id) < 0) { virReportOOMError(conn); - goto err; + goto cleanup; } - char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn); - if (ret == NULL) - goto err; + if (exit_status < 0 || ret == NULL) + goto cleanup; char_ptr = strchr(ret, '\n'); if (char_ptr) *char_ptr = '\0'; - if (exit_status < 0 || ret == NULL) - goto err; - - VIR_FREE(cmd); if (STREQ(ret, "Running")) - return VIR_DOMAIN_RUNNING; + state = VIR_DOMAIN_RUNNING; else if (STREQ(ret, "Not Activated")) - return VIR_DOMAIN_SHUTOFF; + state = VIR_DOMAIN_SHUTOFF; else if (STREQ(ret, "Shutting Down")) - return VIR_DOMAIN_SHUTDOWN; - else - goto err; + state = VIR_DOMAIN_SHUTDOWN; - err: + cleanup: VIR_FREE(cmd); - return VIR_DOMAIN_NOSTATE; + VIR_FREE(ret); + return state; } int @@ -827,7 +854,8 @@ phypGetVIOSPartitionID(virConnectPtr conn) ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; LIBSSH2_SESSION *session = connection_data->session; - char *cmd; + char *cmd = NULL; + char *ret = NULL; int exit_status = 0; int id = -1; char *char_ptr; @@ -840,7 +868,7 @@ phypGetVIOSPartitionID(virConnectPtr conn) goto err; } - char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn); if (exit_status < 0 || ret == NULL) goto err; @@ -848,10 +876,13 @@ phypGetVIOSPartitionID(virConnectPtr conn) if (virStrToLong_i(ret, &char_ptr, 10, &id) == -1) goto err; + VIR_FREE(cmd); + VIR_FREE(ret); return id; err: VIR_FREE(cmd); + VIR_FREE(ret); return -1; } @@ -861,44 +892,41 @@ phypDiskType(virConnectPtr conn, char *backing_device) phyp_driverPtr phyp_driver = conn->privateData; ConnectionData *connection_data = conn->networkPrivateData; LIBSSH2_SESSION *session = connection_data->session; - char *cmd; + char *cmd = NULL; + char *ret = NULL; int exit_status = 0; char *char_ptr; char *managed_system = phyp_driver->managed_system; int vios_id = phyp_driver->vios_id; + int disk_type = -1; if (virAsprintf(&cmd, "viosvrcmd -m %s -p %d -c \"lssp -field name type " "-fmt , -all|grep %s|sed -e 's/^.*,//g'\"", managed_system, vios_id, backing_device) < 0) { virReportOOMError(conn); - goto err; + goto cleanup; } - char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn); - if (ret == NULL) - goto err; + if (exit_status < 0 || ret == NULL) + goto cleanup; char_ptr = strchr(ret, '\n'); if (char_ptr) *char_ptr = '\0'; - if (exit_status < 0 || ret == NULL) - goto err; - - VIR_FREE(cmd); if (STREQ(ret, "LVPOOL")) - return VIR_DOMAIN_DISK_TYPE_BLOCK; + disk_type = VIR_DOMAIN_DISK_TYPE_BLOCK; else if (STREQ(ret, "FBPOOL")) - return VIR_DOMAIN_DISK_TYPE_FILE; - else - goto err; + disk_type = VIR_DOMAIN_DISK_TYPE_FILE; - err: + cleanup: VIR_FREE(cmd); - return -1; + VIR_FREE(ret); + return disk_type; } /* This is a generic function that won't be used directly by @@ -918,7 +946,8 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type) int exit_status = 0; int ndom = 0; char *char_ptr; - char *cmd; + char *cmd = NULL; + char *ret = NULL; char *managed_system = phyp_driver->managed_system; const char *state; @@ -936,7 +965,7 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type) goto err; } - char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn); if (exit_status < 0 || ret == NULL) goto err; @@ -945,10 +974,12 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type) goto err; VIR_FREE(cmd); + VIR_FREE(ret); return ndom; err: VIR_FREE(cmd); + VIR_FREE(ret); return 0; } @@ -984,7 +1015,8 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids, char *char_ptr; unsigned int i = 0, j = 0; char id_c[10]; - char *cmd; + char *cmd = NULL; + char *ret = NULL; const char *state; if (type == 0) @@ -1001,7 +1033,7 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids, virReportOOMError(conn); goto err; } - char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn); /* I need to parse the textual return in order to get the ret */ if (exit_status < 0 || ret == NULL) @@ -1023,10 +1055,12 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids, } VIR_FREE(cmd); + VIR_FREE(ret); return got; err: VIR_FREE(cmd); + VIR_FREE(ret); return 0; } @@ -1045,8 +1079,11 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int nnames) char *managed_system = phyp_driver->managed_system; int exit_status = 0; int got = 0; - char *cmd; - char *domains; + int i; + char *cmd = NULL; + char *ret = NULL; + char *domains = NULL; + char *char_ptr2 = NULL; if (virAsprintf (&cmd, @@ -1056,42 +1093,37 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int nnames) goto err; } - char *ret = phypExec(session, cmd, &exit_status, conn); - - if (VIR_ALLOC(domains) < 0) - virReportOOMError(conn); - - domains = strdup(ret); - if (!domains) - goto err; + ret = phypExec(session, cmd, &exit_status, conn); - char *char_ptr2 = NULL; /* I need to parse the textual return in order to get the domains */ - if (exit_status < 0 || domains == NULL) + if (exit_status < 0 || ret == NULL) goto err; else { + domains = ret; + while (got < nnames) { char_ptr2 = strchr(domains, '\n'); if (char_ptr2) { *char_ptr2 = '\0'; - if (!strdup(domains)) + if ((names[got++] = strdup(domains)) == NULL) { + virReportOOMError(conn); goto err; - names[got] = strdup(domains); + } char_ptr2++; domains = char_ptr2; - got++; } } } - VIR_FREE(domains); VIR_FREE(cmd); VIR_FREE(ret); return got; err: - VIR_FREE(domains); + for (i = 0; i < got; i++) + VIR_FREE(names[i]); + VIR_FREE(cmd); VIR_FREE(ret); return 0; } @@ -1242,7 +1274,8 @@ phypDomainResume(virDomainPtr dom) LIBSSH2_SESSION *session = connection_data->session; char *managed_system = phyp_driver->managed_system; int exit_status = 0; - char *cmd; + char *cmd = NULL; + char *ret = NULL; if (virAsprintf (&cmd, @@ -1252,7 +1285,7 @@ phypDomainResume(virDomainPtr dom) goto err; } - char *ret = phypExec(session, cmd, &exit_status, dom->conn); + ret = phypExec(session, cmd, &exit_status, dom->conn); if (exit_status < 0) goto err; @@ -1275,7 +1308,8 @@ phypDomainShutdown(virDomainPtr dom) LIBSSH2_SESSION *session = connection_data->session; char *managed_system = phyp_driver->managed_system; int exit_status = 0; - char *cmd; + char *cmd = NULL; + char *ret = NULL; if (virAsprintf (&cmd, @@ -1285,7 +1319,7 @@ phypDomainShutdown(virDomainPtr dom) goto err; } - char *ret = phypExec(session, cmd, &exit_status, dom->conn); + ret = phypExec(session, cmd, &exit_status, dom->conn); if (exit_status < 0) goto err; @@ -1331,7 +1365,8 @@ phypDomainDestroy(virDomainPtr dom) LIBSSH2_SESSION *session = connection_data->session; char *managed_system = phyp_driver->managed_system; int exit_status = 0; - char *cmd; + char *cmd = NULL; + char *ret = NULL; if (virAsprintf (&cmd, @@ -1340,7 +1375,7 @@ phypDomainDestroy(virDomainPtr dom) goto err; } - char *ret = phypExec(session, cmd, &exit_status, dom->conn); + ret = phypExec(session, cmd, &exit_status, dom->conn); if (exit_status < 0) goto err; @@ -1406,7 +1441,8 @@ phypDomainCreateAndStart(virConnectPtr conn, err: virDomainDefFree(def); - VIR_FREE(dom); + if (dom) + virUnrefDomain(dom); return NULL; } @@ -1474,7 +1510,8 @@ phypDomainSetCPU(virDomainPtr dom, unsigned int nvcpus) LIBSSH2_SESSION *session = connection_data->session; char *managed_system = phyp_driver->managed_system; int exit_status = 0; - char *cmd; + char *cmd = NULL; + char *ret = NULL; char operation; unsigned long ncpus = 0; unsigned int amount = 0; @@ -1507,7 +1544,7 @@ phypDomainSetCPU(virDomainPtr dom, unsigned int nvcpus) goto err; } - char *ret = phypExec(session, cmd, &exit_status, dom->conn); + ret = phypExec(session, cmd, &exit_status, dom->conn); if (exit_status < 0) { VIR_ERROR("%s", @@ -1606,7 +1643,8 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) phyp_driverPtr phyp_driver = conn->privateData; LIBSSH2_SESSION *session = connection_data->session; char *managed_system = phyp_driver->managed_system; - char *cmd; + char *cmd = NULL; + char *ret = NULL; int exit_status = 0; if (virAsprintf @@ -1620,7 +1658,7 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) goto err; } - char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn); if (exit_status < 0) { VIR_ERROR("%s\"%s\"", "Unable to create LPAR. Reason: ", ret); @@ -1728,8 +1766,10 @@ phypUUIDTable_ReadFile(virConnectPtr conn) rc = read(fd, buffer, sizeof(int)); if (rc == sizeof(int)) { - if (VIR_ALLOC(uuid_table->lpars[i]) < 0) + if (VIR_ALLOC(uuid_table->lpars[i]) < 0) { virReportOOMError(conn); + goto err; + } uuid_table->lpars[i]->id = (*buffer); } else { VIR_WARN("%s", @@ -1851,6 +1891,21 @@ phypUUIDTable_Init(virConnectPtr conn) return -1; } +void +phypUUIDTable_Free(uuid_tablePtr uuid_table) +{ + int i; + + if (uuid_table == NULL) + return; + + for (i = 0; i < uuid_table->nlpars; i++) + VIR_FREE(uuid_table->lpars[i]); + + VIR_FREE(uuid_table->lpars); + VIR_FREE(uuid_table); +} + int phypUUIDTable_Push(virConnectPtr conn) { diff --git a/src/phyp/phyp_driver.h b/src/phyp/phyp_driver.h index 5ec12be..d05f184 100644 --- a/src/phyp/phyp_driver.h +++ b/src/phyp/phyp_driver.h @@ -92,6 +92,8 @@ int phypUUIDTable_Push(virConnectPtr conn); int phypUUIDTable_Init(virConnectPtr conn); +void phypUUIDTable_Free(uuid_tablePtr uuid_table); + int escape_specialcharacters(char *src, char *dst, size_t dstlen); int waitsocket(int socket_fd, LIBSSH2_SESSION * session); -- 1.6.0.4

On Fri, Nov 06, 2009 at 04:28:00AM +0100, Matthias Bolte wrote:
--- src/phyp/phyp_driver.c | 285 ++++++++++++++++++++++++++++------------------- src/phyp/phyp_driver.h | 2 + 2 files changed, 172 insertions(+), 115 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 8e199ee..5379cd3 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -71,7 +71,7 @@ phypOpen(virConnectPtr conn, { LIBSSH2_SESSION *session = NULL; ConnectionData *connection_data = NULL; - char *string; + char *string = NULL; size_t len = 0; int internal_socket; uuid_tablePtr uuid_table = NULL; @@ -128,19 +128,17 @@ phypOpen(virConnectPtr conn, goto failure; }
- if (VIR_ALLOC_N(managed_system, len) < 0) { + /* need to shift one byte in order to remove the first "/" of URI component */ + if (conn->uri->path[0] == '/') + managed_system = strdup(conn->uri->path + 1); + else + managed_system = strdup(conn->uri->path); + + if (!managed_system) { virReportOOMError(conn); goto failure; }
- managed_system = strdup(conn->uri->path); - if (!managed_system) - goto failure; - - /* need to shift one byte in order to remove the first "/" of URI component */ - if (managed_system[0] == '/') - managed_system++; - /* here we are handling only the first component of the path, * so skipping the second: * */ @@ -187,11 +185,19 @@ phypOpen(virConnectPtr conn, return VIR_DRV_OPEN_SUCCESS;
failure: - virCapabilitiesFree(phyp_driver->caps); - VIR_FREE(phyp_driver->managed_system); - VIR_FREE(phyp_driver); - VIR_FREE(uuid_table); - VIR_FREE(uuid_table->lpars); + if (phyp_driver != NULL) { + virCapabilitiesFree(phyp_driver->caps); + VIR_FREE(phyp_driver->managed_system); + VIR_FREE(phyp_driver); + } + + phypUUIDTable_Free(uuid_table); + + if (session != NULL) { + libssh2_session_disconnect(session, "Disconnecting..."); + libssh2_session_free(session); + } + VIR_FREE(connection_data); VIR_FREE(string);
@@ -209,8 +215,7 @@ phypClose(virConnectPtr conn) libssh2_session_free(session);
virCapabilitiesFree(phyp_driver->caps); - VIR_FREE(phyp_driver->uuid_table); - VIR_FREE(phyp_driver->uuid_table->lpars); + phypUUIDTable_Free(phyp_driver->uuid_table); VIR_FREE(phyp_driver->managed_system); VIR_FREE(phyp_driver); VIR_FREE(connection_data); @@ -281,7 +286,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", _("Failure establishing SSH session.")); - goto err; + goto disconnect; }
/* Trying authentication by pubkey */ @@ -305,7 +310,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", _("No authentication callback provided.")); - goto err; + goto disconnect; }
for (i = 0; i < auth->ncredtype; i++) { @@ -317,7 +322,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", _("Required credentials are not supported.")); - goto err; + goto disconnect; }
int res = @@ -327,7 +332,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", _("Unable to fetch credentials.")); - goto err; + goto disconnect; }
if (creds[0].result) { @@ -336,9 +341,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", _("Unable to get password certificates")); - libssh2_session_disconnect(session, "Disconnecting..."); - libssh2_session_free(session); - goto err; + goto disconnect; }
while ((rc = @@ -350,16 +353,19 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", _("Authentication failed")); - libssh2_session_disconnect(session, "Disconnecting"); - libssh2_session_free(session); - goto err; + goto disconnect; } else goto exit; } + disconnect: + libssh2_session_disconnect(session, "Disconnecting..."); + libssh2_session_free(session); err: + VIR_FREE(password); return NULL;
exit: + VIR_FREE(password); return session; }
@@ -456,7 +462,8 @@ phypGetLparID(LIBSSH2_SESSION * session, const char *managed_system, int exit_status = 0; int lpar_id = 0; char *char_ptr; - char *cmd; + char *cmd = NULL; + char *ret = NULL;
if (virAsprintf(&cmd, "lssyscfg -r lpar -m %s --filter lpar_names=%s -F lpar_id", @@ -465,7 +472,7 @@ phypGetLparID(LIBSSH2_SESSION * session, const char *managed_system, goto err; }
- const char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn);
if (exit_status < 0 || ret == NULL) goto err; @@ -474,10 +481,12 @@ phypGetLparID(LIBSSH2_SESSION * session, const char *managed_system, goto err;
VIR_FREE(cmd); + VIR_FREE(ret); return lpar_id;
err: VIR_FREE(cmd); + VIR_FREE(ret); return -1; }
@@ -486,7 +495,8 @@ static char * phypGetLparNAME(LIBSSH2_SESSION * session, const char *managed_system, unsigned int lpar_id, virConnectPtr conn) { - char *cmd; + char *cmd = NULL; + char *ret = NULL; int exit_status = 0;
if (virAsprintf(&cmd, @@ -496,7 +506,7 @@ phypGetLparNAME(LIBSSH2_SESSION * session, const char *managed_system, goto err; }
- char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn);
if (ret == NULL) goto err; @@ -514,6 +524,7 @@ phypGetLparNAME(LIBSSH2_SESSION * session, const char *managed_system,
err: VIR_FREE(cmd); + VIR_FREE(ret); return NULL; }
@@ -553,7 +564,8 @@ phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id, { ConnectionData *connection_data = conn->networkPrivateData; LIBSSH2_SESSION *session = connection_data->session; - char *cmd; + char *cmd = NULL; + char *ret = NULL; char *char_ptr; int memory = 0; int exit_status = 0; @@ -579,7 +591,7 @@ phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id, } }
- char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn);
if (ret == NULL) goto err; @@ -596,10 +608,12 @@ phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id, goto err;
VIR_FREE(cmd); + VIR_FREE(ret); return memory;
err: VIR_FREE(cmd); + VIR_FREE(ret); return 0;
} @@ -625,7 +639,8 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, { ConnectionData *connection_data = conn->networkPrivateData; LIBSSH2_SESSION *session = connection_data->session; - char *cmd; + char *cmd = NULL; + char *ret = NULL; int exit_status = 0; int vcpus = 0;
@@ -646,7 +661,7 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, goto err; } } - char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn);
if (ret == NULL) goto err; @@ -663,10 +678,12 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, goto err;
VIR_FREE(cmd); + VIR_FREE(ret); return (unsigned long) vcpus;
err: VIR_FREE(cmd); + VIR_FREE(ret); return 0; }
@@ -676,7 +693,8 @@ phypGetRemoteSlot(virConnectPtr conn, const char *managed_system, { ConnectionData *connection_data = conn->networkPrivateData; LIBSSH2_SESSION *session = connection_data->session; - char *cmd; + char *cmd = NULL; + char *ret = NULL; char *char_ptr; int remote_slot = 0; int exit_status = 0; @@ -688,7 +706,7 @@ phypGetRemoteSlot(virConnectPtr conn, const char *managed_system, virReportOOMError(conn); goto err; } - char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn);
if (ret == NULL) goto err; @@ -705,10 +723,12 @@ phypGetRemoteSlot(virConnectPtr conn, const char *managed_system, goto err;
VIR_FREE(cmd); + VIR_FREE(ret); return remote_slot;
err: VIR_FREE(cmd); + VIR_FREE(ret); return -1; }
@@ -718,9 +738,12 @@ phypGetBackingDevice(virConnectPtr conn, const char *managed_system, { ConnectionData *connection_data = conn->networkPrivateData; LIBSSH2_SESSION *session = connection_data->session; - char *cmd; + char *cmd = NULL; + char *ret = NULL; int remote_slot = 0; int exit_status = 0; + char *char_ptr; + char *backing_device = NULL;
if ((remote_slot = phypGetRemoteSlot(conn, managed_system, lpar_name)) == -1) @@ -734,9 +757,9 @@ phypGetBackingDevice(virConnectPtr conn, const char *managed_system, goto err; }
- char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn);
- if (ret == NULL) + if (exit_status < 0 || ret == NULL) goto err;
/* here is a little trick to deal returns of this kind: @@ -746,31 +769,38 @@ phypGetBackingDevice(virConnectPtr conn, const char *managed_system, * the information we really need is only lv01, so we * need to skip a lot of things on the string. * */ - char *backing_device = strchr(ret, '/'); + char_ptr = strchr(ret, '/');
- if (backing_device) { - backing_device++; - if (backing_device[0] == '/') - backing_device++; + if (char_ptr) { + char_ptr++; + if (char_ptr[0] == '/') + char_ptr++; else goto err; + + backing_device = strdup(char_ptr); + + if (backing_device == NULL) { + virReportOOMError(conn); + goto err; + } } else { backing_device = ret; + ret = NULL; }
- char *char_ptr = strchr(backing_device, '\n'); + char_ptr = strchr(backing_device, '\n');
if (char_ptr) *char_ptr = '\0';
- if (exit_status < 0 || backing_device == NULL) - goto err; - VIR_FREE(cmd); + VIR_FREE(ret); return backing_device;
err: VIR_FREE(cmd); + VIR_FREE(ret); return NULL;
} @@ -781,44 +811,41 @@ phypGetLparState(virConnectPtr conn, unsigned int lpar_id) ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; LIBSSH2_SESSION *session = connection_data->session; - char *cmd; + char *cmd = NULL; + char *ret = NULL; int exit_status = 0; char *char_ptr = NULL; char *managed_system = phyp_driver->managed_system; + int state = VIR_DOMAIN_NOSTATE;
if (virAsprintf(&cmd, "lssyscfg -r lpar -m %s -F state --filter lpar_ids=%d", managed_system, lpar_id) < 0) { virReportOOMError(conn); - goto err; + goto cleanup; }
- char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn);
- if (ret == NULL) - goto err; + if (exit_status < 0 || ret == NULL) + goto cleanup;
char_ptr = strchr(ret, '\n');
if (char_ptr) *char_ptr = '\0';
- if (exit_status < 0 || ret == NULL) - goto err; - - VIR_FREE(cmd); if (STREQ(ret, "Running")) - return VIR_DOMAIN_RUNNING; + state = VIR_DOMAIN_RUNNING; else if (STREQ(ret, "Not Activated")) - return VIR_DOMAIN_SHUTOFF; + state = VIR_DOMAIN_SHUTOFF; else if (STREQ(ret, "Shutting Down")) - return VIR_DOMAIN_SHUTDOWN; - else - goto err; + state = VIR_DOMAIN_SHUTDOWN;
- err: + cleanup: VIR_FREE(cmd); - return VIR_DOMAIN_NOSTATE; + VIR_FREE(ret); + return state; }
int @@ -827,7 +854,8 @@ phypGetVIOSPartitionID(virConnectPtr conn) ConnectionData *connection_data = conn->networkPrivateData; phyp_driverPtr phyp_driver = conn->privateData; LIBSSH2_SESSION *session = connection_data->session; - char *cmd; + char *cmd = NULL; + char *ret = NULL; int exit_status = 0; int id = -1; char *char_ptr; @@ -840,7 +868,7 @@ phypGetVIOSPartitionID(virConnectPtr conn) goto err; }
- char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn);
if (exit_status < 0 || ret == NULL) goto err; @@ -848,10 +876,13 @@ phypGetVIOSPartitionID(virConnectPtr conn) if (virStrToLong_i(ret, &char_ptr, 10, &id) == -1) goto err;
+ VIR_FREE(cmd); + VIR_FREE(ret); return id;
err: VIR_FREE(cmd); + VIR_FREE(ret); return -1; }
@@ -861,44 +892,41 @@ phypDiskType(virConnectPtr conn, char *backing_device) phyp_driverPtr phyp_driver = conn->privateData; ConnectionData *connection_data = conn->networkPrivateData; LIBSSH2_SESSION *session = connection_data->session; - char *cmd; + char *cmd = NULL; + char *ret = NULL; int exit_status = 0; char *char_ptr; char *managed_system = phyp_driver->managed_system; int vios_id = phyp_driver->vios_id; + int disk_type = -1;
if (virAsprintf(&cmd, "viosvrcmd -m %s -p %d -c \"lssp -field name type " "-fmt , -all|grep %s|sed -e 's/^.*,//g'\"", managed_system, vios_id, backing_device) < 0) { virReportOOMError(conn); - goto err; + goto cleanup; }
- char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn);
- if (ret == NULL) - goto err; + if (exit_status < 0 || ret == NULL) + goto cleanup;
char_ptr = strchr(ret, '\n');
if (char_ptr) *char_ptr = '\0';
- if (exit_status < 0 || ret == NULL) - goto err; - - VIR_FREE(cmd); if (STREQ(ret, "LVPOOL")) - return VIR_DOMAIN_DISK_TYPE_BLOCK; + disk_type = VIR_DOMAIN_DISK_TYPE_BLOCK; else if (STREQ(ret, "FBPOOL")) - return VIR_DOMAIN_DISK_TYPE_FILE; - else - goto err; + disk_type = VIR_DOMAIN_DISK_TYPE_FILE;
- err: + cleanup: VIR_FREE(cmd); - return -1; + VIR_FREE(ret); + return disk_type; }
/* This is a generic function that won't be used directly by @@ -918,7 +946,8 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type) int exit_status = 0; int ndom = 0; char *char_ptr; - char *cmd; + char *cmd = NULL; + char *ret = NULL; char *managed_system = phyp_driver->managed_system; const char *state;
@@ -936,7 +965,7 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type) goto err; }
- char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn);
if (exit_status < 0 || ret == NULL) goto err; @@ -945,10 +974,12 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type) goto err;
VIR_FREE(cmd); + VIR_FREE(ret); return ndom;
err: VIR_FREE(cmd); + VIR_FREE(ret); return 0; }
@@ -984,7 +1015,8 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids, char *char_ptr; unsigned int i = 0, j = 0; char id_c[10]; - char *cmd; + char *cmd = NULL; + char *ret = NULL; const char *state;
if (type == 0) @@ -1001,7 +1033,7 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids, virReportOOMError(conn); goto err; } - char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn);
/* I need to parse the textual return in order to get the ret */ if (exit_status < 0 || ret == NULL) @@ -1023,10 +1055,12 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids, }
VIR_FREE(cmd); + VIR_FREE(ret); return got;
err: VIR_FREE(cmd); + VIR_FREE(ret); return 0; }
@@ -1045,8 +1079,11 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int nnames) char *managed_system = phyp_driver->managed_system; int exit_status = 0; int got = 0; - char *cmd; - char *domains; + int i; + char *cmd = NULL; + char *ret = NULL; + char *domains = NULL; + char *char_ptr2 = NULL;
if (virAsprintf (&cmd, @@ -1056,42 +1093,37 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int nnames) goto err; }
- char *ret = phypExec(session, cmd, &exit_status, conn); - - if (VIR_ALLOC(domains) < 0) - virReportOOMError(conn); - - domains = strdup(ret); - if (!domains) - goto err; + ret = phypExec(session, cmd, &exit_status, conn);
- char *char_ptr2 = NULL; /* I need to parse the textual return in order to get the domains */ - if (exit_status < 0 || domains == NULL) + if (exit_status < 0 || ret == NULL) goto err; else { + domains = ret; + while (got < nnames) { char_ptr2 = strchr(domains, '\n');
if (char_ptr2) { *char_ptr2 = '\0'; - if (!strdup(domains)) + if ((names[got++] = strdup(domains)) == NULL) { + virReportOOMError(conn); goto err; - names[got] = strdup(domains); + } char_ptr2++; domains = char_ptr2; - got++; } } }
- VIR_FREE(domains); VIR_FREE(cmd); VIR_FREE(ret); return got;
err: - VIR_FREE(domains); + for (i = 0; i < got; i++) + VIR_FREE(names[i]); + VIR_FREE(cmd); VIR_FREE(ret); return 0; } @@ -1242,7 +1274,8 @@ phypDomainResume(virDomainPtr dom) LIBSSH2_SESSION *session = connection_data->session; char *managed_system = phyp_driver->managed_system; int exit_status = 0; - char *cmd; + char *cmd = NULL; + char *ret = NULL;
if (virAsprintf (&cmd, @@ -1252,7 +1285,7 @@ phypDomainResume(virDomainPtr dom) goto err; }
- char *ret = phypExec(session, cmd, &exit_status, dom->conn); + ret = phypExec(session, cmd, &exit_status, dom->conn);
if (exit_status < 0) goto err; @@ -1275,7 +1308,8 @@ phypDomainShutdown(virDomainPtr dom) LIBSSH2_SESSION *session = connection_data->session; char *managed_system = phyp_driver->managed_system; int exit_status = 0; - char *cmd; + char *cmd = NULL; + char *ret = NULL;
if (virAsprintf (&cmd, @@ -1285,7 +1319,7 @@ phypDomainShutdown(virDomainPtr dom) goto err; }
- char *ret = phypExec(session, cmd, &exit_status, dom->conn); + ret = phypExec(session, cmd, &exit_status, dom->conn);
if (exit_status < 0) goto err; @@ -1331,7 +1365,8 @@ phypDomainDestroy(virDomainPtr dom) LIBSSH2_SESSION *session = connection_data->session; char *managed_system = phyp_driver->managed_system; int exit_status = 0; - char *cmd; + char *cmd = NULL; + char *ret = NULL;
if (virAsprintf (&cmd, @@ -1340,7 +1375,7 @@ phypDomainDestroy(virDomainPtr dom) goto err; }
- char *ret = phypExec(session, cmd, &exit_status, dom->conn); + ret = phypExec(session, cmd, &exit_status, dom->conn);
if (exit_status < 0) goto err; @@ -1406,7 +1441,8 @@ phypDomainCreateAndStart(virConnectPtr conn,
err: virDomainDefFree(def); - VIR_FREE(dom); + if (dom) + virUnrefDomain(dom); return NULL; }
@@ -1474,7 +1510,8 @@ phypDomainSetCPU(virDomainPtr dom, unsigned int nvcpus) LIBSSH2_SESSION *session = connection_data->session; char *managed_system = phyp_driver->managed_system; int exit_status = 0; - char *cmd; + char *cmd = NULL; + char *ret = NULL; char operation; unsigned long ncpus = 0; unsigned int amount = 0; @@ -1507,7 +1544,7 @@ phypDomainSetCPU(virDomainPtr dom, unsigned int nvcpus) goto err; }
- char *ret = phypExec(session, cmd, &exit_status, dom->conn); + ret = phypExec(session, cmd, &exit_status, dom->conn);
if (exit_status < 0) { VIR_ERROR("%s", @@ -1606,7 +1643,8 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) phyp_driverPtr phyp_driver = conn->privateData; LIBSSH2_SESSION *session = connection_data->session; char *managed_system = phyp_driver->managed_system; - char *cmd; + char *cmd = NULL; + char *ret = NULL; int exit_status = 0;
if (virAsprintf @@ -1620,7 +1658,7 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) goto err; }
- char *ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn);
if (exit_status < 0) { VIR_ERROR("%s\"%s\"", "Unable to create LPAR. Reason: ", ret); @@ -1728,8 +1766,10 @@ phypUUIDTable_ReadFile(virConnectPtr conn)
rc = read(fd, buffer, sizeof(int)); if (rc == sizeof(int)) { - if (VIR_ALLOC(uuid_table->lpars[i]) < 0) + if (VIR_ALLOC(uuid_table->lpars[i]) < 0) { virReportOOMError(conn); + goto err; + } uuid_table->lpars[i]->id = (*buffer); } else { VIR_WARN("%s", @@ -1851,6 +1891,21 @@ phypUUIDTable_Init(virConnectPtr conn) return -1; }
+void +phypUUIDTable_Free(uuid_tablePtr uuid_table) +{ + int i; + + if (uuid_table == NULL) + return; + + for (i = 0; i < uuid_table->nlpars; i++) + VIR_FREE(uuid_table->lpars[i]); + + VIR_FREE(uuid_table->lpars); + VIR_FREE(uuid_table); +} + int phypUUIDTable_Push(virConnectPtr conn) { diff --git a/src/phyp/phyp_driver.h b/src/phyp/phyp_driver.h index 5ec12be..d05f184 100644 --- a/src/phyp/phyp_driver.h +++ b/src/phyp/phyp_driver.h @@ -92,6 +92,8 @@ int phypUUIDTable_Push(virConnectPtr conn);
int phypUUIDTable_Init(virConnectPtr conn);
+void phypUUIDTable_Free(uuid_tablePtr uuid_table); + int escape_specialcharacters(char *src, char *dst, size_t dstlen);
int waitsocket(int socket_fd, LIBSSH2_SESSION * session);
A lot of cleanups indeed, thanks a lot for going through the code, ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

--- src/phyp/phyp_driver.c | 13 +++++++++---- 1 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 5379cd3..b94d0fa 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1040,9 +1040,13 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids, goto err; else { while (got < nids) { - if (ret[i] == '\n') { - if (virStrToLong_i(id_c, &char_ptr, 10, &ids[got]) == -1) - return 0; + if (ret[i] == '\0') + break; + else if (ret[i] == '\n') { + if (virStrToLong_i(id_c, &char_ptr, 10, &ids[got]) == -1) { + VIR_ERROR("Cannot parse number from '%s'", id_c); + goto err; + } memset(id_c, 0, 10); j = 0; got++; @@ -1112,7 +1116,8 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int nnames) } char_ptr2++; domains = char_ptr2; - } + } else + break; } } -- 1.6.0.4

On Fri, Nov 06, 2009 at 04:28:01AM +0100, Matthias Bolte wrote:
--- src/phyp/phyp_driver.c | 13 +++++++++---- 1 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 5379cd3..b94d0fa 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1040,9 +1040,13 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids, goto err; else { while (got < nids) { - if (ret[i] == '\n') { - if (virStrToLong_i(id_c, &char_ptr, 10, &ids[got]) == -1) - return 0; + if (ret[i] == '\0') + break; + else if (ret[i] == '\n') { + if (virStrToLong_i(id_c, &char_ptr, 10, &ids[got]) == -1) { + VIR_ERROR("Cannot parse number from '%s'", id_c); + goto err; + } memset(id_c, 0, 10); j = 0; got++; @@ -1112,7 +1116,8 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int nnames) } char_ptr2++; domains = char_ptr2; - } + } else + break; } }
Right, that looks way safer, thanks ! ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

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; -- 1.6.0.4

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@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

Also reset UUID to all 0 instead of all 48 (== '0') in phypUUIDTable_RemLpar() --- src/phyp/phyp_driver.c | 20 ++++---------------- 1 files changed, 4 insertions(+), 16 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 8d54ae7..ab5af17 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -237,7 +237,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, struct addrinfo hints; int ret; - memset(&hints, '\0', sizeof(hints)); + memset(&hints, 0, sizeof(hints)); hints.ai_flags = AI_ADDRCONFIG | AI_NUMERICSERV; hints.ai_socktype = SOCK_STREAM; hints.ai_protocol = 0; @@ -1214,10 +1214,6 @@ phypDomainDumpXML(virDomainPtr dom, int flags) virDomainDefPtr def = NULL; char *ret = NULL; char *managed_system = phyp_driver->managed_system; - unsigned char *lpar_uuid = NULL; - - if (VIR_ALLOC_N(lpar_uuid, VIR_UUID_BUFLEN) < 0) - virReportOOMError(dom->conn); if (VIR_ALLOC(def) < 0) virReportOOMError(dom->conn); @@ -1233,12 +1229,7 @@ phypDomainDumpXML(virDomainPtr dom, int flags) goto err; } - if (phypGetLparUUID(lpar_uuid, dom->id, dom->conn) == -1) { - VIR_ERROR("%s", "Unable to generate random uuid."); - goto err; - } - - if (!memcpy(def->uuid, lpar_uuid, VIR_UUID_BUFLEN)) { + if (phypGetLparUUID(def->uuid, dom->id, dom->conn) == -1) { VIR_ERROR("%s", "Unable to generate random uuid."); goto err; } @@ -1695,8 +1686,7 @@ phypUUIDTable_RemLpar(virConnectPtr conn, int id) for (i = 0; i <= uuid_table->nlpars; i++) { if (uuid_table->lpars[i]->id == id) { uuid_table->lpars[i]->id = -1; - if (!memset(uuid_table->lpars[i]->uuid, '0', VIR_UUID_BUFLEN)) - goto exit; + memset(uuid_table->lpars[i]->uuid, 0, VIR_UUID_BUFLEN); } } @@ -1706,7 +1696,6 @@ phypUUIDTable_RemLpar(virConnectPtr conn, int id) if (phypUUIDTable_Push(conn) == -1) goto err; - exit: return 0; err: @@ -1734,8 +1723,7 @@ phypUUIDTable_AddLpar(virConnectPtr conn, unsigned char *uuid, int id) } uuid_table->lpars[i]->id = id; - if (memmove(uuid_table->lpars[i]->uuid, uuid, VIR_UUID_BUFLEN) == NULL) - goto err; + memmove(uuid_table->lpars[i]->uuid, uuid, VIR_UUID_BUFLEN); if (phypUUIDTable_WriteFile(conn) == -1) goto err; -- 1.6.0.4

On Fri, Nov 06, 2009 at 04:28:03AM +0100, Matthias Bolte wrote:
Also reset UUID to all 0 instead of all 48 (== '0') in phypUUIDTable_RemLpar() --- src/phyp/phyp_driver.c | 20 ++++---------------- 1 files changed, 4 insertions(+), 16 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 8d54ae7..ab5af17 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -237,7 +237,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, struct addrinfo hints; int ret;
- memset(&hints, '\0', sizeof(hints)); + memset(&hints, 0, sizeof(hints)); hints.ai_flags = AI_ADDRCONFIG | AI_NUMERICSERV; hints.ai_socktype = SOCK_STREAM; hints.ai_protocol = 0; @@ -1214,10 +1214,6 @@ phypDomainDumpXML(virDomainPtr dom, int flags) virDomainDefPtr def = NULL; char *ret = NULL; char *managed_system = phyp_driver->managed_system; - unsigned char *lpar_uuid = NULL; - - if (VIR_ALLOC_N(lpar_uuid, VIR_UUID_BUFLEN) < 0) - virReportOOMError(dom->conn);
if (VIR_ALLOC(def) < 0) virReportOOMError(dom->conn); @@ -1233,12 +1229,7 @@ phypDomainDumpXML(virDomainPtr dom, int flags) goto err; }
- if (phypGetLparUUID(lpar_uuid, dom->id, dom->conn) == -1) { - VIR_ERROR("%s", "Unable to generate random uuid."); - goto err; - } - - if (!memcpy(def->uuid, lpar_uuid, VIR_UUID_BUFLEN)) { + if (phypGetLparUUID(def->uuid, dom->id, dom->conn) == -1) { VIR_ERROR("%s", "Unable to generate random uuid."); goto err; } @@ -1695,8 +1686,7 @@ phypUUIDTable_RemLpar(virConnectPtr conn, int id) for (i = 0; i <= uuid_table->nlpars; i++) { if (uuid_table->lpars[i]->id == id) { uuid_table->lpars[i]->id = -1; - if (!memset(uuid_table->lpars[i]->uuid, '0', VIR_UUID_BUFLEN)) - goto exit; + memset(uuid_table->lpars[i]->uuid, 0, VIR_UUID_BUFLEN); } }
@@ -1706,7 +1696,6 @@ phypUUIDTable_RemLpar(virConnectPtr conn, int id) if (phypUUIDTable_Push(conn) == -1) goto err;
- exit: return 0;
err: @@ -1734,8 +1723,7 @@ phypUUIDTable_AddLpar(virConnectPtr conn, unsigned char *uuid, int id) }
uuid_table->lpars[i]->id = id; - if (memmove(uuid_table->lpars[i]->uuid, uuid, VIR_UUID_BUFLEN) == NULL) - goto err; + memmove(uuid_table->lpars[i]->uuid, uuid, VIR_UUID_BUFLEN);
if (phypUUIDTable_WriteFile(conn) == -1) goto err;
Good cleanup, and good catch !!! ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

--- src/phyp/phyp_driver.c | 35 ++++++++++++----------------------- 1 files changed, 12 insertions(+), 23 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index ab5af17..1529c24 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -508,7 +508,7 @@ phypGetLparNAME(LIBSSH2_SESSION * session, const char *managed_system, ret = phypExec(session, cmd, &exit_status, conn); - if (ret == NULL) + if (exit_status < 0 || ret == NULL) goto err; char *char_ptr = strchr(ret, '\n'); @@ -516,9 +516,6 @@ phypGetLparNAME(LIBSSH2_SESSION * session, const char *managed_system, if (char_ptr) *char_ptr = '\0'; - if (exit_status < 0 || ret == NULL) - goto err; - VIR_FREE(cmd); return ret; @@ -593,16 +590,13 @@ phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id, ret = phypExec(session, cmd, &exit_status, conn); - if (ret == NULL) + if (exit_status < 0 || ret == NULL) goto err; - char *mem_char_ptr = strchr(ret, '\n'); - - if (mem_char_ptr) - *mem_char_ptr = '\0'; + char_ptr = strchr(ret, '\n'); - if (exit_status < 0) - goto err; + if (char_ptr) + *char_ptr = '\0'; if (virStrToLong_i(ret, &char_ptr, 10, &memory) == -1) goto err; @@ -641,6 +635,7 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, LIBSSH2_SESSION *session = connection_data->session; char *cmd = NULL; char *ret = NULL; + char *char_ptr; int exit_status = 0; int vcpus = 0; @@ -663,10 +658,10 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, } ret = phypExec(session, cmd, &exit_status, conn); - if (ret == NULL) + if (exit_status < 0 || ret == NULL) goto err; - char *char_ptr = strchr(ret, '\n'); + char_ptr = strchr(ret, '\n'); if (char_ptr) *char_ptr = '\0'; @@ -674,9 +669,6 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, if (virStrToLong_i(ret, &char_ptr, 10, &vcpus) == -1) goto err; - if (exit_status < 0) - goto err; - VIR_FREE(cmd); VIR_FREE(ret); return (unsigned long) vcpus; @@ -708,16 +700,13 @@ phypGetRemoteSlot(virConnectPtr conn, const char *managed_system, } ret = phypExec(session, cmd, &exit_status, conn); - if (ret == NULL) + if (exit_status < 0 || ret == NULL) goto err; - char *char_ptr2 = strchr(ret, '\n'); - - if (char_ptr2) - *char_ptr2 = '\0'; + char_ptr = strchr(ret, '\n'); - if (exit_status < 0) - goto err; + if (char_ptr) + *char_ptr = '\0'; if (virStrToLong_i(ret, &char_ptr, 10, &remote_slot) == -1) goto err; -- 1.6.0.4

On Fri, Nov 06, 2009 at 04:28:04AM +0100, Matthias Bolte wrote:
--- src/phyp/phyp_driver.c | 35 ++++++++++++----------------------- 1 files changed, 12 insertions(+), 23 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index ab5af17..1529c24 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -508,7 +508,7 @@ phypGetLparNAME(LIBSSH2_SESSION * session, const char *managed_system,
ret = phypExec(session, cmd, &exit_status, conn);
- if (ret == NULL) + if (exit_status < 0 || ret == NULL) goto err;
char *char_ptr = strchr(ret, '\n'); @@ -516,9 +516,6 @@ phypGetLparNAME(LIBSSH2_SESSION * session, const char *managed_system, if (char_ptr) *char_ptr = '\0';
- if (exit_status < 0 || ret == NULL) - goto err; - VIR_FREE(cmd); return ret;
@@ -593,16 +590,13 @@ phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id,
ret = phypExec(session, cmd, &exit_status, conn);
- if (ret == NULL) + if (exit_status < 0 || ret == NULL) goto err;
- char *mem_char_ptr = strchr(ret, '\n'); - - if (mem_char_ptr) - *mem_char_ptr = '\0'; + char_ptr = strchr(ret, '\n');
- if (exit_status < 0) - goto err; + if (char_ptr) + *char_ptr = '\0';
if (virStrToLong_i(ret, &char_ptr, 10, &memory) == -1) goto err; @@ -641,6 +635,7 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, LIBSSH2_SESSION *session = connection_data->session; char *cmd = NULL; char *ret = NULL; + char *char_ptr; int exit_status = 0; int vcpus = 0;
@@ -663,10 +658,10 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, } ret = phypExec(session, cmd, &exit_status, conn);
- if (ret == NULL) + if (exit_status < 0 || ret == NULL) goto err;
- char *char_ptr = strchr(ret, '\n'); + char_ptr = strchr(ret, '\n');
if (char_ptr) *char_ptr = '\0'; @@ -674,9 +669,6 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, if (virStrToLong_i(ret, &char_ptr, 10, &vcpus) == -1) goto err;
- if (exit_status < 0) - goto err; - VIR_FREE(cmd); VIR_FREE(ret); return (unsigned long) vcpus; @@ -708,16 +700,13 @@ phypGetRemoteSlot(virConnectPtr conn, const char *managed_system, } ret = phypExec(session, cmd, &exit_status, conn);
- if (ret == NULL) + if (exit_status < 0 || ret == NULL) goto err;
- char *char_ptr2 = strchr(ret, '\n'); - - if (char_ptr2) - *char_ptr2 = '\0'; + char_ptr = strchr(ret, '\n');
- if (exit_status < 0) - goto err; + if (char_ptr) + *char_ptr = '\0';
if (virStrToLong_i(ret, &char_ptr, 10, &remote_slot) == -1) goto err;
ACK, also includes other cleanups ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

- Make reading ID from file working for IDs > 127 - Fix inverse error check for writing ID to file - Use feof() to distinguish EOF from real error of fread() - Don't interpret libssh2 error codes as number of bytes --- src/phyp/phyp_driver.c | 19 ++++++++++++------- 1 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 1529c24..43430a9 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1735,7 +1735,7 @@ phypUUIDTable_ReadFile(virConnectPtr conn) int fd = -1; char local_file[] = "./uuid_table"; int rc = 0; - char buffer[1024]; + int id; if ((fd = open(local_file, O_RDONLY)) == -1) { VIR_WARN("%s", "Unable to write information to local file."); @@ -1746,13 +1746,13 @@ phypUUIDTable_ReadFile(virConnectPtr conn) if (VIR_ALLOC_N(uuid_table->lpars, uuid_table->nlpars) >= 0) { for (i = 0; i < uuid_table->nlpars; i++) { - rc = read(fd, buffer, sizeof(int)); + rc = read(fd, &id, sizeof(int)); if (rc == sizeof(int)) { if (VIR_ALLOC(uuid_table->lpars[i]) < 0) { virReportOOMError(conn); goto err; } - uuid_table->lpars[i]->id = (*buffer); + uuid_table->lpars[i]->id = id; } else { VIR_WARN("%s", "Unable to read from information to local file."); @@ -1790,7 +1790,7 @@ phypUUIDTable_WriteFile(virConnectPtr conn) for (i = 0; i < uuid_table->nlpars; i++) { if (safewrite(fd, &uuid_table->lpars[i]->id, - sizeof(uuid_table->lpars[i]->id)) == + sizeof(uuid_table->lpars[i]->id)) != sizeof(uuid_table->lpars[i]->id)) { VIR_ERROR("%s", "Unable to write information to local file."); goto err; @@ -1944,8 +1944,13 @@ phypUUIDTable_Push(virConnectPtr conn) do { nread = fread(buffer, 1, sizeof(buffer), fd); if (nread <= 0) { - /* end of file */ - break; + if (feof(fd)) { + /* end of file */ + break; + } else { + VIR_ERROR("Failed to read from '%s'", local_file); + goto err; + } } ptr = buffer; sent = 0; @@ -1955,7 +1960,7 @@ phypUUIDTable_Push(virConnectPtr conn) rc = libssh2_channel_write(channel, ptr, nread); if (LIBSSH2_ERROR_EAGAIN == rc) { /* must loop around */ continue; - } else { + } else if (rc > 0) { /* rc indicates how many bytes were written this time */ sent += rc; } -- 1.6.0.4

On Fri, Nov 06, 2009 at 04:28:05AM +0100, Matthias Bolte wrote:
- Make reading ID from file working for IDs > 127 - Fix inverse error check for writing ID to file - Use feof() to distinguish EOF from real error of fread() - Don't interpret libssh2 error codes as number of bytes --- src/phyp/phyp_driver.c | 19 ++++++++++++------- 1 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 1529c24..43430a9 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1735,7 +1735,7 @@ phypUUIDTable_ReadFile(virConnectPtr conn) int fd = -1; char local_file[] = "./uuid_table"; int rc = 0; - char buffer[1024]; + int id;
if ((fd = open(local_file, O_RDONLY)) == -1) { VIR_WARN("%s", "Unable to write information to local file."); @@ -1746,13 +1746,13 @@ phypUUIDTable_ReadFile(virConnectPtr conn) if (VIR_ALLOC_N(uuid_table->lpars, uuid_table->nlpars) >= 0) { for (i = 0; i < uuid_table->nlpars; i++) {
- rc = read(fd, buffer, sizeof(int)); + rc = read(fd, &id, sizeof(int)); if (rc == sizeof(int)) { if (VIR_ALLOC(uuid_table->lpars[i]) < 0) { virReportOOMError(conn); goto err; } - uuid_table->lpars[i]->id = (*buffer); + uuid_table->lpars[i]->id = id; } else { VIR_WARN("%s", "Unable to read from information to local file."); @@ -1790,7 +1790,7 @@ phypUUIDTable_WriteFile(virConnectPtr conn)
for (i = 0; i < uuid_table->nlpars; i++) { if (safewrite(fd, &uuid_table->lpars[i]->id, - sizeof(uuid_table->lpars[i]->id)) == + sizeof(uuid_table->lpars[i]->id)) != sizeof(uuid_table->lpars[i]->id)) { VIR_ERROR("%s", "Unable to write information to local file."); goto err;
Oops I missed that one :-)
@@ -1944,8 +1944,13 @@ phypUUIDTable_Push(virConnectPtr conn) do { nread = fread(buffer, 1, sizeof(buffer), fd); if (nread <= 0) { - /* end of file */ - break; + if (feof(fd)) { + /* end of file */ + break; + } else { + VIR_ERROR("Failed to read from '%s'", local_file); + goto err; + } } ptr = buffer; sent = 0; @@ -1955,7 +1960,7 @@ phypUUIDTable_Push(virConnectPtr conn) rc = libssh2_channel_write(channel, ptr, nread); if (LIBSSH2_ERROR_EAGAIN == rc) { /* must loop around */ continue; - } else { + } else if (rc > 0) { /* rc indicates how many bytes were written this time */ sent += rc; }
ACK ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

--- src/phyp/phyp_driver.c | 47 ++++++++++++++--------------------------------- 1 files changed, 14 insertions(+), 33 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 43430a9..6263fd2 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1131,29 +1131,21 @@ phypDomainLookupByName(virConnectPtr conn, const char *lpar_name) virDomainPtr dom = NULL; int lpar_id = 0; char *managed_system = phyp_driver->managed_system; - unsigned char *lpar_uuid = NULL; - - if (VIR_ALLOC_N(lpar_uuid, VIR_UUID_BUFLEN) < 0) - virReportOOMError(dom->conn); + unsigned char lpar_uuid[VIR_UUID_BUFLEN]; lpar_id = phypGetLparID(session, managed_system, lpar_name, conn); if (lpar_id == -1) - goto err; + return NULL; if (phypGetLparUUID(lpar_uuid, lpar_id, conn) == -1) - goto err; + return NULL; dom = virGetDomain(conn, lpar_name, lpar_uuid); if (dom) dom->id = lpar_id; - VIR_FREE(lpar_uuid); return dom; - - err: - VIR_FREE(lpar_uuid); - return NULL; } static virDomainPtr @@ -1165,10 +1157,7 @@ phypDomainLookupByID(virConnectPtr conn, int lpar_id) virDomainPtr dom = NULL; char *managed_system = phyp_driver->managed_system; int exit_status = 0; - unsigned char *lpar_uuid = NULL; - - if (VIR_ALLOC_N(lpar_uuid, VIR_UUID_BUFLEN) < 0) - virReportOOMError(dom->conn); + unsigned char lpar_uuid[VIR_UUID_BUFLEN]; char *lpar_name = phypGetLparNAME(session, managed_system, lpar_id, conn); @@ -1185,12 +1174,10 @@ phypDomainLookupByID(virConnectPtr conn, int lpar_id) dom->id = lpar_id; VIR_FREE(lpar_name); - VIR_FREE(lpar_uuid); return dom; err: VIR_FREE(lpar_name); - VIR_FREE(lpar_uuid); return NULL; } @@ -1200,17 +1187,15 @@ phypDomainDumpXML(virDomainPtr dom, int flags) ConnectionData *connection_data = dom->conn->networkPrivateData; phyp_driverPtr phyp_driver = dom->conn->privateData; LIBSSH2_SESSION *session = connection_data->session; - virDomainDefPtr def = NULL; - char *ret = NULL; + virDomainDef def; char *managed_system = phyp_driver->managed_system; - if (VIR_ALLOC(def) < 0) - virReportOOMError(dom->conn); + memset(&def, 0, sizeof(virDomainDef)); - def->virtType = VIR_DOMAIN_VIRT_PHYP; - def->id = dom->id; + def.virtType = VIR_DOMAIN_VIRT_PHYP; + def.id = dom->id; - char *lpar_name = phypGetLparNAME(session, managed_system, def->id, + char *lpar_name = phypGetLparNAME(session, managed_system, def.id, dom->conn); if (lpar_name == NULL) { @@ -1218,36 +1203,32 @@ phypDomainDumpXML(virDomainPtr dom, int flags) goto err; } - if (phypGetLparUUID(def->uuid, dom->id, dom->conn) == -1) { + if (phypGetLparUUID(def.uuid, dom->id, dom->conn) == -1) { VIR_ERROR("%s", "Unable to generate random uuid."); goto err; } - if ((def->maxmem = + if ((def.maxmem = phypGetLparMem(dom->conn, managed_system, dom->id, 0)) == 0) { VIR_ERROR("%s", "Unable to determine domain's max memory."); goto err; } - if ((def->memory = + if ((def.memory = phypGetLparMem(dom->conn, managed_system, dom->id, 1)) == 0) { VIR_ERROR("%s", "Unable to determine domain's memory."); goto err; } - if ((def->vcpus = + if ((def.vcpus = phypGetLparCPU(dom->conn, managed_system, dom->id)) == 0) { VIR_ERROR("%s", "Unable to determine domain's CPU."); goto err; } - ret = virDomainDefFormat(dom->conn, def, flags); - - virDomainDefFree(def); - return ret; + return virDomainDefFormat(dom->conn, &def, flags); err: - virDomainDefFree(def); return NULL; } -- 1.6.0.4

On Fri, Nov 06, 2009 at 04:28:06AM +0100, Matthias Bolte wrote:
--- src/phyp/phyp_driver.c | 47 ++++++++++++++--------------------------------- 1 files changed, 14 insertions(+), 33 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 43430a9..6263fd2 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1131,29 +1131,21 @@ phypDomainLookupByName(virConnectPtr conn, const char *lpar_name) virDomainPtr dom = NULL; int lpar_id = 0; char *managed_system = phyp_driver->managed_system; - unsigned char *lpar_uuid = NULL; - - if (VIR_ALLOC_N(lpar_uuid, VIR_UUID_BUFLEN) < 0) - virReportOOMError(dom->conn); + unsigned char lpar_uuid[VIR_UUID_BUFLEN];
lpar_id = phypGetLparID(session, managed_system, lpar_name, conn); if (lpar_id == -1) - goto err; + return NULL;
if (phypGetLparUUID(lpar_uuid, lpar_id, conn) == -1) - goto err; + return NULL;
dom = virGetDomain(conn, lpar_name, lpar_uuid);
if (dom) dom->id = lpar_id;
- VIR_FREE(lpar_uuid); return dom; - - err: - VIR_FREE(lpar_uuid); - return NULL; }
static virDomainPtr @@ -1165,10 +1157,7 @@ phypDomainLookupByID(virConnectPtr conn, int lpar_id) virDomainPtr dom = NULL; char *managed_system = phyp_driver->managed_system; int exit_status = 0; - unsigned char *lpar_uuid = NULL; - - if (VIR_ALLOC_N(lpar_uuid, VIR_UUID_BUFLEN) < 0) - virReportOOMError(dom->conn); + unsigned char lpar_uuid[VIR_UUID_BUFLEN];
char *lpar_name = phypGetLparNAME(session, managed_system, lpar_id, conn); @@ -1185,12 +1174,10 @@ phypDomainLookupByID(virConnectPtr conn, int lpar_id) dom->id = lpar_id;
VIR_FREE(lpar_name); - VIR_FREE(lpar_uuid); return dom;
err: VIR_FREE(lpar_name); - VIR_FREE(lpar_uuid); return NULL; }
@@ -1200,17 +1187,15 @@ phypDomainDumpXML(virDomainPtr dom, int flags) ConnectionData *connection_data = dom->conn->networkPrivateData; phyp_driverPtr phyp_driver = dom->conn->privateData; LIBSSH2_SESSION *session = connection_data->session; - virDomainDefPtr def = NULL; - char *ret = NULL; + virDomainDef def; char *managed_system = phyp_driver->managed_system;
- if (VIR_ALLOC(def) < 0) - virReportOOMError(dom->conn); + memset(&def, 0, sizeof(virDomainDef));
- def->virtType = VIR_DOMAIN_VIRT_PHYP; - def->id = dom->id; + def.virtType = VIR_DOMAIN_VIRT_PHYP; + def.id = dom->id;
- char *lpar_name = phypGetLparNAME(session, managed_system, def->id, + char *lpar_name = phypGetLparNAME(session, managed_system, def.id, dom->conn);
if (lpar_name == NULL) { @@ -1218,36 +1203,32 @@ phypDomainDumpXML(virDomainPtr dom, int flags) goto err; }
- if (phypGetLparUUID(def->uuid, dom->id, dom->conn) == -1) { + if (phypGetLparUUID(def.uuid, dom->id, dom->conn) == -1) { VIR_ERROR("%s", "Unable to generate random uuid."); goto err; }
- if ((def->maxmem = + if ((def.maxmem = phypGetLparMem(dom->conn, managed_system, dom->id, 0)) == 0) { VIR_ERROR("%s", "Unable to determine domain's max memory."); goto err; }
- if ((def->memory = + if ((def.memory = phypGetLparMem(dom->conn, managed_system, dom->id, 1)) == 0) { VIR_ERROR("%s", "Unable to determine domain's memory."); goto err; }
- if ((def->vcpus = + if ((def.vcpus = phypGetLparCPU(dom->conn, managed_system, dom->id)) == 0) { VIR_ERROR("%s", "Unable to determine domain's CPU."); goto err; }
- ret = virDomainDefFormat(dom->conn, def, flags); - - virDomainDefFree(def); - return ret; + return virDomainDefFormat(dom->conn, &def, flags);
err: - virDomainDefFree(def); return NULL; }
ACK, UUID strings are short enough to be allocated on stack ! Simplifies the code and avoid mistakes, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

--- src/phyp/phyp_driver.c | 71 +++++++++++++++++++----------------------------- 1 files changed, 28 insertions(+), 43 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 6263fd2..a92046a 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -61,6 +61,10 @@ #define VIR_FROM_THIS VIR_FROM_PHYP +#define PHYP_ERROR(conn, code, fmt...) \ + virReportErrorHelper(conn, VIR_FROM_PHYP, code, __FILE__, __FUNCTION__, \ + __LINE__, fmt) + /* * URI: phyp://user@[hmc|ivm]/managed_system * */ @@ -86,23 +90,14 @@ phypOpen(virConnectPtr conn, return VIR_DRV_OPEN_DECLINED; if (conn->uri->server == NULL) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Missing server name in phyp:// URI")); - return VIR_DRV_OPEN_ERROR; - } - - if (conn->uri->path == NULL) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Missing managed system name in phyp:// URI")); + PHYP_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + _("Missing server name in phyp:// URI")); return VIR_DRV_OPEN_ERROR; } if (conn->uri->path == NULL) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Missing path name in phyp:// URI")); + PHYP_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + _("Missing managed system name in phyp:// URI")); return VIR_DRV_OPEN_ERROR; } @@ -148,16 +143,14 @@ phypOpen(virConnectPtr conn, *char_ptr = '\0'; if (escape_specialcharacters(conn->uri->path, string, len) == -1) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Error parsing 'path'. Invalid characters.")); + PHYP_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + _("Error parsing 'path'. Invalid characters.")); goto failure; } if ((session = openSSHSession(conn, auth, &internal_socket)) == NULL) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Error while opening SSH session.")); + PHYP_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + _("Error while opening SSH session.")); goto failure; } //conn->uri->path = string; @@ -244,9 +237,8 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, ret = getaddrinfo(hostname, "22", &hints, &ai); if (ret != 0) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, - _("Error while getting %s address info"), hostname); + PHYP_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + _("Error while getting %s address info"), hostname); goto err; } @@ -262,9 +254,8 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, cur = cur->ai_next; } - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, - _("Failed to connect to %s"), hostname); + PHYP_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + _("Failed to connect to %s"), hostname); freeaddrinfo(ai); goto err; @@ -283,9 +274,8 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, while ((rc = libssh2_session_startup(session, sock)) == LIBSSH2_ERROR_EAGAIN) ; if (rc) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Failure establishing SSH session.")); + PHYP_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + _("Failure establishing SSH session.")); goto disconnect; } @@ -307,9 +297,8 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, }; if (!auth || !auth->cb) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("No authentication callback provided.")); + PHYP_ERROR(conn, VIR_ERR_AUTH_FAILED, + _("No authentication callback provided.")); goto disconnect; } @@ -319,9 +308,8 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, } if (!hasPassphrase) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Required credentials are not supported.")); + PHYP_ERROR(conn, VIR_ERR_AUTH_FAILED, + _("Required credentials are not supported.")); goto disconnect; } @@ -329,18 +317,16 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, (auth->cb) (creds, ARRAY_CARDINALITY(creds), auth->cbdata); if (res < 0) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Unable to fetch credentials.")); + PHYP_ERROR(conn, VIR_ERR_AUTH_FAILED, + _("Unable to fetch credentials.")); goto disconnect; } if (creds[0].result) { password = creds[0].result; } else { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Unable to get password certificates")); + PHYP_ERROR(conn, VIR_ERR_AUTH_FAILED, + _("Unable to get password certificates")); goto disconnect; } @@ -350,9 +336,8 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, LIBSSH2_ERROR_EAGAIN) ; if (rc) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Authentication failed")); + PHYP_ERROR(conn, VIR_ERR_AUTH_FAILED, + _("Authentication failed")); goto disconnect; } else goto exit; -- 1.6.0.4

On Fri, Nov 06, 2009 at 04:28:07AM +0100, Matthias Bolte wrote:
--- src/phyp/phyp_driver.c | 71 +++++++++++++++++++----------------------------- 1 files changed, 28 insertions(+), 43 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 6263fd2..a92046a 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -61,6 +61,10 @@
#define VIR_FROM_THIS VIR_FROM_PHYP
+#define PHYP_ERROR(conn, code, fmt...) \ + virReportErrorHelper(conn, VIR_FROM_PHYP, code, __FILE__, __FUNCTION__, \ + __LINE__, fmt) + /* * URI: phyp://user@[hmc|ivm]/managed_system * */ @@ -86,23 +90,14 @@ phypOpen(virConnectPtr conn, return VIR_DRV_OPEN_DECLINED;
if (conn->uri->server == NULL) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Missing server name in phyp:// URI")); - return VIR_DRV_OPEN_ERROR; - } - - if (conn->uri->path == NULL) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Missing managed system name in phyp:// URI")); + PHYP_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + _("Missing server name in phyp:// URI")); return VIR_DRV_OPEN_ERROR; }
if (conn->uri->path == NULL) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Missing path name in phyp:// URI")); + PHYP_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + _("Missing managed system name in phyp:// URI")); return VIR_DRV_OPEN_ERROR; }
Hum, okay, I had to look twice to convince myself the error messages were properly matched :-)
@@ -148,16 +143,14 @@ phypOpen(virConnectPtr conn, *char_ptr = '\0';
if (escape_specialcharacters(conn->uri->path, string, len) == -1) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Error parsing 'path'. Invalid characters.")); + PHYP_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + _("Error parsing 'path'. Invalid characters.")); goto failure; }
if ((session = openSSHSession(conn, auth, &internal_socket)) == NULL) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Error while opening SSH session.")); + PHYP_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + _("Error while opening SSH session.")); goto failure; } //conn->uri->path = string; @@ -244,9 +237,8 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth,
ret = getaddrinfo(hostname, "22", &hints, &ai); if (ret != 0) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, - _("Error while getting %s address info"), hostname); + PHYP_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + _("Error while getting %s address info"), hostname); goto err; }
@@ -262,9 +254,8 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, cur = cur->ai_next; }
- virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, - _("Failed to connect to %s"), hostname); + PHYP_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + _("Failed to connect to %s"), hostname); freeaddrinfo(ai); goto err;
@@ -283,9 +274,8 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, while ((rc = libssh2_session_startup(session, sock)) == LIBSSH2_ERROR_EAGAIN) ; if (rc) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Failure establishing SSH session.")); + PHYP_ERROR(conn, VIR_ERR_INTERNAL_ERROR, + _("Failure establishing SSH session.")); goto disconnect; }
@@ -307,9 +297,8 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, };
if (!auth || !auth->cb) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("No authentication callback provided.")); + PHYP_ERROR(conn, VIR_ERR_AUTH_FAILED, + _("No authentication callback provided.")); goto disconnect; }
@@ -319,9 +308,8 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, }
if (!hasPassphrase) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Required credentials are not supported.")); + PHYP_ERROR(conn, VIR_ERR_AUTH_FAILED, + _("Required credentials are not supported.")); goto disconnect; }
@@ -329,18 +317,16 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, (auth->cb) (creds, ARRAY_CARDINALITY(creds), auth->cbdata);
if (res < 0) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Unable to fetch credentials.")); + PHYP_ERROR(conn, VIR_ERR_AUTH_FAILED, + _("Unable to fetch credentials.")); goto disconnect; }
if (creds[0].result) { password = creds[0].result; } else { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Unable to get password certificates")); + PHYP_ERROR(conn, VIR_ERR_AUTH_FAILED, + _("Unable to get password certificates")); goto disconnect; }
@@ -350,9 +336,8 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, LIBSSH2_ERROR_EAGAIN) ;
if (rc) { - virRaiseError(conn, NULL, NULL, 0, VIR_FROM_PHYP, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, "%s", - _("Authentication failed")); + PHYP_ERROR(conn, VIR_ERR_AUTH_FAILED, + _("Authentication failed")); goto disconnect; } else goto exit;
ACK, very good ! thanks a lot ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

Matthias Bolte wrote:
After the last large patch has been applied I reviewed the whole driver code and found several bugs and issues. Most important memory leaks and two potential infinite loops.
I've no Power Hypervisor at hand, so I can't test this changes with an actual system. Eduardo could you test this patches to make sure they don't break existing functionality?
Matthias
Hello Matthias, I've tested all patches and none of them break any functionality. Thank you very much for all those fixes! []'s -- Eduardo Otubo Software Engineer Linux Technology Center IBM Systems & Technology Group Mobile: +55 19 8135 0885 eotubo@linux.vnet.ibm.com

2009/11/6 Eduardo Otubo <otubo@linux.vnet.ibm.com>:
Matthias Bolte wrote:
After the last large patch has been applied I reviewed the whole driver code and found several bugs and issues. Most important memory leaks and two potential infinite loops.
I've no Power Hypervisor at hand, so I can't test this changes with an actual system. Eduardo could you test this patches to make sure they don't break existing functionality?
Matthias
Hello Matthias,
I've tested all patches and none of them break any functionality. Thank you very much for all those fixes!
[]'s
Fine, I've pushed the series. Matthias
participants (3)
-
Daniel Veillard
-
Eduardo Otubo
-
Matthias Bolte