[libvirt] [PATCH 0/4] more phyp refactoring

Phyp still had several memory leaks and in general some bad copy-and-paste design. This series consolidates some common patterns into simpler helpers, avoiding leaks in the process. Eric Blake (4): phyp: prefer memcpy over memmove when legal phyp: avoid memory leaks in return values phyp: avoid memory leaks in command values phyp: another simplification src/phyp/phyp_driver.c | 1128 +++++++----------------------------------------- 1 files changed, 163 insertions(+), 965 deletions(-) -- 1.7.4.2

* src/phyp/phyp_driver.c (phypUUIDTable_AddLpar) (phypGetLparUUID, phypGetStoragePoolUUID, phypVolumeGetXMLDesc) (phypGetStoragePoolXMLDesc): Use faster method. --- memmove is only required for overlapping regions; none of these instances could overlap and memcpy is theoretically faster src/phyp/phyp_driver.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index a1b6819..ec2ac09 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -633,7 +633,7 @@ phypUUIDTable_AddLpar(virConnectPtr conn, unsigned char *uuid, int id) } uuid_table->lpars[i]->id = id; - memmove(uuid_table->lpars[i]->uuid, uuid, VIR_UUID_BUFLEN); + memcpy(uuid_table->lpars[i]->uuid, uuid, VIR_UUID_BUFLEN); if (phypUUIDTable_WriteFile(conn) == -1) goto err; @@ -1398,7 +1398,7 @@ phypGetLparUUID(unsigned char *uuid, int lpar_id, virConnectPtr conn) for (i = 0; i < uuid_table->nlpars; i++) { if (lpars[i]->id == lpar_id) { - memmove(uuid, lpars[i]->uuid, VIR_UUID_BUFLEN); + memcpy(uuid, lpars[i]->uuid, VIR_UUID_BUFLEN); return 0; } } @@ -2623,7 +2623,7 @@ phypGetStoragePoolUUID(virConnectPtr conn, unsigned char *uuid, if (exit_status < 0 || ret == NULL) goto err; - if (memmove(uuid, ret, VIR_UUID_BUFLEN) == NULL) + if (memcpy(uuid, ret, VIR_UUID_BUFLEN) == NULL) goto err; VIR_FREE(cmd); @@ -2676,7 +2676,7 @@ phypVolumeGetXMLDesc(virStorageVolPtr vol, unsigned int flags) goto err; } - if (memmove(pool.uuid, sp->uuid, VIR_UUID_BUFLEN) == NULL) { + if (memcpy(pool.uuid, sp->uuid, VIR_UUID_BUFLEN) == NULL) { VIR_ERROR0(_("Unable to determine storage sp's uuid.")); goto err; } @@ -2705,7 +2705,7 @@ phypVolumeGetXMLDesc(virStorageVolPtr vol, unsigned int flags) goto err; } - if (memmove(voldef.key, vol->key, PATH_MAX) == NULL) { + if (memcpy(voldef.key, vol->key, PATH_MAX) == NULL) { VIR_ERROR0(_("Unable to determine volume's key.")); goto err; } @@ -3262,7 +3262,7 @@ phypGetStoragePoolXMLDesc(virStoragePoolPtr pool, unsigned int flags) goto err; } - if (memmove(def.uuid, pool->uuid, VIR_UUID_BUFLEN) == NULL) { + if (memcpy(def.uuid, pool->uuid, VIR_UUID_BUFLEN) == NULL) { VIR_ERROR0(_("Unable to determine storage pool's uuid.")); goto err; } -- 1.7.4.2

* src/phyp/phyp_driver.c (phypCreateServerSCSIAdapter) (phypAttachDevice, phypVolumeLookupByPath, phypVolumeGetPath): Avoid leaking phypExec result. (phypBuildStoragePool): Avoid NULL dereference. (phypInterfaceDestroy): Avoid redundant free. --- src/phyp/phyp_driver.c | 49 +++++++++++++++++++++++++++++------------------ 1 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index ec2ac09..2f00461 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1,4 +1,3 @@ - /* * Copyright (C) 2010-2011 Red Hat, Inc. * Copyright IBM Corp. 2009 @@ -1853,6 +1852,7 @@ phypCreateServerSCSIAdapter(virConnectPtr conn) } cmd = virBufferContentAndReset(&buf); + VIR_FREE(ret); ret = phypExec(session, cmd, &exit_status, conn); if (exit_status < 0 || ret == NULL) @@ -1861,6 +1861,7 @@ phypCreateServerSCSIAdapter(virConnectPtr conn) /* Finally I add the new scsi adapter to VIOS using the same slot * I used in the VIOS configuration. * */ + VIR_FREE(ret); virBufferAddLit(&buf, "chhwres -r virtualio --rsubtype scsi"); if (system_type == HMC) virBufferVSprintf(&buf, " -m %s", managed_system); @@ -2047,6 +2048,7 @@ phypAttachDevice(virDomainPtr domain, const char *xml) /* Let's get the slot number for the adapter we just created * */ + VIR_FREE(ret); virBufferAddLit(&buf, "lshwres -r virtualio --rsubtype scsi"); if (system_type == HMC) virBufferVSprintf(&buf, " -m %s", managed_system); @@ -2071,6 +2073,7 @@ phypAttachDevice(virDomainPtr domain, const char *xml) /* Listing all the virtual_scsi_adapter interfaces, the new adapter must * be appended to this list * */ + VIR_FREE(ret); virBufferAddLit(&buf, "lssyscfg"); if (system_type == HMC) virBufferVSprintf(&buf, " -m %s", managed_system); @@ -2108,6 +2111,7 @@ phypAttachDevice(virDomainPtr domain, const char *xml) } cmd = virBufferContentAndReset(&buf); + VIR_FREE(ret); ret = phypExec(session, cmd, &exit_status, conn); if (virStrToLong_i(ret, &char_ptr, 10, &slot) == -1) @@ -2116,6 +2120,7 @@ phypAttachDevice(virDomainPtr domain, const char *xml) /* Finally I add the new scsi adapter to VIOS using the same slot * I used in the VIOS configuration. * */ + VIR_FREE(ret); virBufferAddLit(&buf, "chhwres -r virtualio --rsubtype scsi"); if (system_type == HMC) virBufferVSprintf(&buf, " -m %s", managed_system); @@ -2542,9 +2547,11 @@ phypVolumeLookupByPath(virConnectPtr conn, const char *volname) int vios_id = phyp_driver->vios_id; int exit_status = 0; char *cmd = NULL; - char *spname = NULL; + char *ret = NULL; char *key = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; + char *char_ptr; + virStorageVolPtr result = NULL; if (system_type == HMC) virBufferVSprintf(&buf, "viosvrcmd -m %s --id %d -c '", @@ -2564,25 +2571,30 @@ phypVolumeLookupByPath(virConnectPtr conn, const char *volname) } cmd = virBufferContentAndReset(&buf); - spname = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn); - if (exit_status < 0 || spname == NULL) - return NULL; + if (exit_status < 0 || ret == NULL) + goto cleanup; - char *char_ptr = strchr(spname, '\n'); + char_ptr = strchr(ret, '\n'); if (char_ptr) *char_ptr = '\0'; if (VIR_ALLOC_N(key, MAX_KEY_SIZE) < 0) { virReportOOMError(); - return NULL; + goto cleanup; } if (phypVolumeGetKey(conn, key, volname) == -1) - return NULL; + goto cleanup; + + result = virGetStorageVol(conn, ret, volname, key); - return virGetStorageVol(conn, spname, volname, key); +cleanup: + VIR_FREE(ret); + VIR_FREE(key); + return result; } static int @@ -2738,7 +2750,7 @@ phypVolumeGetPath(virStorageVolPtr vol) int vios_id = phyp_driver->vios_id; int exit_status = 0; char *cmd = NULL; - char *sp = NULL; + char *ret = NULL; char *path = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; char *char_ptr; @@ -2763,20 +2775,20 @@ phypVolumeGetPath(virStorageVolPtr vol) } cmd = virBufferContentAndReset(&buf); - sp = phypExec(session, cmd, &exit_status, conn); + ret = phypExec(session, cmd, &exit_status, conn); - if (exit_status < 0 || sp == NULL) + if (exit_status < 0 || ret == NULL) goto err; - char_ptr = strchr(sp, '\n'); + char_ptr = strchr(ret, '\n'); if (char_ptr) *char_ptr = '\0'; - pv = phypVolumeGetPhysicalVolumeByStoragePool(vol, sp); + pv = phypVolumeGetPhysicalVolumeByStoragePool(vol, ret); if (pv) { - if (virAsprintf(&path, "/%s/%s/%s", pv, sp, vol->name) < 0) { + if (virAsprintf(&path, "/%s/%s/%s", pv, ret, vol->name) < 0) { virReportOOMError(); goto err; } @@ -2785,11 +2797,12 @@ phypVolumeGetPath(virStorageVolPtr vol) } VIR_FREE(cmd); + VIR_FREE(ret); return path; err: VIR_FREE(cmd); - VIR_FREE(sp); + VIR_FREE(ret); VIR_FREE(path); return NULL; @@ -3013,7 +3026,7 @@ phypBuildStoragePool(virConnectPtr conn, virStoragePoolDefPtr def) ret = phypExec(session, cmd, &exit_status, conn); if (exit_status < 0) { - VIR_ERROR(_("Unable to create Storage Pool: %s"), ret); + VIR_ERROR(_("Unable to create Storage Pool: %s"), NULLSTR(ret)); goto err; } @@ -3357,8 +3370,6 @@ phypInterfaceDestroy(virInterfacePtr iface, } cmd = virBufferContentAndReset(&buf); - VIR_FREE(ret); - ret = phypExec(session, cmd, &exit_status, iface->conn); if (exit_status < 0 || ret == NULL) -- 1.7.4.2

* src/phyp/phyp_driver.c (phypExecBuffer): New function. Use it throughout file for less code, and for plugging a few leaks. (phypExec): Simplify control flow. (phypGetSystemType): Avoid passing NULL. (phypDestroyStoragePool): Avoid clobbering cmd. --- Lots of cleanup! src/phyp/phyp_driver.c | 788 ++++++------------------------------------------ 1 files changed, 88 insertions(+), 700 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 2f00461..cd0e5f9 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -111,6 +111,7 @@ waitsocket(int socket_fd, LIBSSH2_SESSION * session) /* this function is the layer that manipulates the ssh channel itself * and executes the commands on the remote machine */ static char * +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) phypExec(LIBSSH2_SESSION * session, char *cmd, int *exit_status, virConnectPtr conn) { @@ -182,23 +183,44 @@ phypExec(LIBSSH2_SESSION * session, char *cmd, int *exit_status, (*exit_status) = exitcode; libssh2_channel_free(channel); channel = NULL; - goto exit; + VIR_FREE(buffer); - err: + if (virBufferError(&tex_ret)) { + virBufferFreeAndReset(&tex_ret); + virReportOOMError(); + return NULL; + } + return virBufferContentAndReset(&tex_ret); + +err: (*exit_status) = SSH_CMD_ERR; virBufferFreeAndReset(&tex_ret); VIR_FREE(buffer); return NULL; +} - exit: - VIR_FREE(buffer); +/* Convenience wrapper function */ +static char * ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) +phypExecBuffer(LIBSSH2_SESSION *session, virBufferPtr buf, int *exit_status, + virConnectPtr conn, bool strip_newline) +{ + char *cmd; + char *ret; - if (virBufferError(&tex_ret)) { - virBufferFreeAndReset(&tex_ret); + if (virBufferError(buf)) { + virBufferFreeAndReset(buf); virReportOOMError(); return NULL; } - return virBufferContentAndReset(&tex_ret); + cmd = virBufferContentAndReset(buf); + ret = phypExec(session, cmd, exit_status, conn); + VIR_FREE(cmd); + if (ret && *exit_status == 0 && strip_newline) { + char *nl = strchr(ret, '\n'); + if (nl) + *nl = '\0'; + } + return ret; } static int @@ -212,7 +234,7 @@ phypGetSystemType(virConnectPtr conn) if (virAsprintf(&cmd, "lshmc -V") < 0) { virReportOOMError(); - exit_status = -1; + return -1; } ret = phypExec(session, cmd, &exit_status, conn); @@ -228,7 +250,6 @@ phypGetVIOSPartitionID(virConnectPtr conn) phyp_driverPtr phyp_driver = conn->privateData; LIBSSH2_SESSION *session = connection_data->session; int system_type = phyp_driver->system_type; - char *cmd = NULL; char *ret = NULL; int exit_status = 0; int id = -1; @@ -241,14 +262,7 @@ phypGetVIOSPartitionID(virConnectPtr conn) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferAddLit(&buf, " -r lpar -F lpar_id,lpar_env" "|sed -n '/vioserver/ {\n s/,.*$//\n p\n}'"); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -256,12 +270,10 @@ 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; } @@ -328,7 +340,6 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type) int exit_status = 0; int ndom = 0; char *char_ptr; - char *cmd = NULL; char *ret = NULL; char *managed_system = phyp_driver->managed_system; const char *state; @@ -350,14 +361,7 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -F lpar_id,state %s |grep -c '^[0-9]*'", state); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -365,12 +369,10 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type) if (virStrToLong_i(ret, &char_ptr, 10, &ndom) == -1) goto err; - VIR_FREE(cmd); VIR_FREE(ret); return ndom; err: - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -393,7 +395,6 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids, char *managed_system = phyp_driver->managed_system; int exit_status = 0; int got = -1; - char *cmd = NULL; char *ret = NULL; char *line, *next_line; const char *state; @@ -409,14 +410,7 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids, virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -F lpar_id,state %s | sed -e 's/,.*$//'", state); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -437,7 +431,6 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids, } err: - VIR_FREE(cmd); VIR_FREE(ret); return got; } @@ -1304,7 +1297,6 @@ phypGetLparID(LIBSSH2_SESSION * session, const char *managed_system, int exit_status = 0; int lpar_id = 0; char *char_ptr; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -1312,14 +1304,7 @@ phypGetLparID(LIBSSH2_SESSION * session, const char *managed_system, if (system_type == HMC) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " --filter lpar_names=%s -F lpar_id", name); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -1327,12 +1312,10 @@ phypGetLparID(LIBSSH2_SESSION * session, const char *managed_system, if (virStrToLong_i(ret, &char_ptr, 10, &lpar_id) == -1) goto err; - VIR_FREE(cmd); VIR_FREE(ret); return lpar_id; err: - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -1344,38 +1327,22 @@ phypGetLparNAME(LIBSSH2_SESSION * session, const char *managed_system, { phyp_driverPtr phyp_driver = conn->privateData; int system_type = phyp_driver->system_type; - char *cmd = NULL; char *ret = NULL; int exit_status = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *char_ptr; virBufferAddLit(&buf, "lssyscfg -r lpar"); if (system_type == HMC) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " --filter lpar_ids=%d -F name", lpar_id); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); if (exit_status < 0 || ret == NULL) goto err; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - - VIR_FREE(cmd); return ret; err: - VIR_FREE(cmd); VIR_FREE(ret); return NULL; } @@ -1418,7 +1385,6 @@ phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id, LIBSSH2_SESSION *session = connection_data->session; phyp_driverPtr phyp_driver = conn->privateData; int system_type = phyp_driver->system_type; - char *cmd = NULL; char *ret = NULL; char *char_ptr; int memory = 0; @@ -1434,32 +1400,18 @@ phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id, virBufferVSprintf(&buf, " -r mem --level lpar -F %s --filter lpar_ids=%d", type ? "curr_mem" : "curr_max_mem", lpar_id); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return 0; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - if (virStrToLong_i(ret, &char_ptr, 10, &memory) == -1) goto err; - VIR_FREE(cmd); VIR_FREE(ret); return memory; err: - VIR_FREE(cmd); VIR_FREE(ret); return 0; @@ -1473,7 +1425,6 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, LIBSSH2_SESSION *session = connection_data->session; phyp_driverPtr phyp_driver = conn->privateData; int system_type = phyp_driver->system_type; - char *cmd = NULL; char *ret = NULL; char *char_ptr; int exit_status = 0; @@ -1486,32 +1437,18 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, virBufferVSprintf(&buf, " -r proc --level lpar -F %s --filter lpar_ids=%d", type ? "curr_max_procs" : "curr_procs", lpar_id); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return 0; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - if (virStrToLong_i(ret, &char_ptr, 10, &vcpus) == -1) goto err; - VIR_FREE(cmd); VIR_FREE(ret); return (unsigned long) vcpus; err: - VIR_FREE(cmd); VIR_FREE(ret); return 0; } @@ -1551,7 +1488,6 @@ phypGetRemoteSlot(virConnectPtr conn, const char *managed_system, LIBSSH2_SESSION *session = connection_data->session; phyp_driverPtr phyp_driver = conn->privateData; int system_type = phyp_driver->system_type; - char *cmd = NULL; char *ret = NULL; char *char_ptr; int remote_slot = 0; @@ -1563,32 +1499,18 @@ phypGetRemoteSlot(virConnectPtr conn, const char *managed_system, virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -r virtualio --rsubtype scsi -F " "remote_slot_num --filter lpar_names=%s", lpar_name); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - if (virStrToLong_i(ret, &char_ptr, 10, &remote_slot) == -1) goto err; - VIR_FREE(cmd); VIR_FREE(ret); return remote_slot; err: - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -1604,7 +1526,6 @@ phypGetBackingDevice(virConnectPtr conn, const char *managed_system, LIBSSH2_SESSION *session = connection_data->session; phyp_driverPtr phyp_driver = conn->privateData; int system_type = phyp_driver->system_type; - char *cmd = NULL; char *ret = NULL; int remote_slot = 0; int exit_status = 0; @@ -1621,14 +1542,7 @@ phypGetBackingDevice(virConnectPtr conn, const char *managed_system, virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -r virtualio --rsubtype scsi -F " "backing_devices --filter slots=%d", remote_slot); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -1665,12 +1579,10 @@ phypGetBackingDevice(virConnectPtr conn, const char *managed_system, if (char_ptr) *char_ptr = '\0'; - VIR_FREE(cmd); VIR_FREE(ret); return backing_device; err: - VIR_FREE(cmd); VIR_FREE(ret); return NULL; } @@ -1684,10 +1596,8 @@ phypGetLparProfile(virConnectPtr conn, int lpar_id) char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *char_ptr; virBufferAddLit(&buf, "lssyscfg"); if (system_type == HMC) @@ -1695,28 +1605,14 @@ phypGetLparProfile(virConnectPtr conn, int lpar_id) virBufferVSprintf(&buf, " -r prof --filter lpar_ids=%d -F name|head -n 1", lpar_id); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); if (exit_status < 0 || ret == NULL) goto err; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - - VIR_FREE(cmd); return ret; err: - VIR_FREE(cmd); VIR_FREE(ret); return NULL; } @@ -1732,7 +1628,6 @@ phypGetVIOSNextSlotNumber(virConnectPtr conn) int vios_id = phyp_driver->vios_id; int exit_status = 0; char *char_ptr; - char *cmd = NULL; char *ret = NULL; char *profile = NULL; int slot = 0; @@ -1754,16 +1649,7 @@ phypGetVIOSNextSlotNumber(virConnectPtr conn) "virtual_serial_adapters|sed -e 's/\"//g' -e " "'s/,/\\n/g'|sed -e 's/\\(^[0-9][0-9]\\*\\).*$/\\1/'" "|sort|tail -n 1", profile); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -1771,12 +1657,10 @@ phypGetVIOSNextSlotNumber(virConnectPtr conn) if (virStrToLong_i(ret, &char_ptr, 10, &slot) == -1) goto err; - VIR_FREE(cmd); VIR_FREE(ret); return slot + 1; err: - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -1791,7 +1675,6 @@ phypCreateServerSCSIAdapter(virConnectPtr conn) int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; char *profile = NULL; int slot = 0; @@ -1824,14 +1707,7 @@ phypCreateServerSCSIAdapter(virConnectPtr conn) virBufferVSprintf(&buf, " -r prof --filter lpar_ids=%d,profile_names=%s" " -F virtual_scsi_adapters|sed -e s/\\\"//g", vios_id, profile); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -1845,15 +1721,8 @@ phypCreateServerSCSIAdapter(virConnectPtr conn) virBufferVSprintf(&buf, " -r prof -i 'name=%s,lpar_id=%d," "\"virtual_scsi_adapters=%s,%d/server/any/any/1\"'", vios_name, vios_id, ret, slot); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - VIR_FREE(ret); - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -1868,28 +1737,19 @@ phypCreateServerSCSIAdapter(virConnectPtr conn) virBufferVSprintf(&buf, " -p %s -o a -s %d -d 0 -a \"adapter_type=server\"", vios_name, slot); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; VIR_FREE(profile); VIR_FREE(vios_name); - VIR_FREE(cmd); VIR_FREE(ret); return 0; err: VIR_FREE(profile); VIR_FREE(vios_name); - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -1904,10 +1764,8 @@ phypGetVIOSFreeSCSIAdapter(virConnectPtr conn) int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *char_ptr; if (system_type == HMC) virBufferVSprintf(&buf, "viosvrcmd -m %s --id %d -c '", @@ -1919,29 +1777,14 @@ phypGetVIOSFreeSCSIAdapter(virConnectPtr conn) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|sed '/,[^.*]/d; s/,//g; q'"); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); if (exit_status < 0 || ret == NULL) goto err; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - - VIR_FREE(cmd); return ret; err: - VIR_FREE(cmd); VIR_FREE(ret); return NULL; } @@ -1960,7 +1803,6 @@ phypAttachDevice(virDomainPtr domain, const char *xml) int vios_id = phyp_driver->vios_id; int exit_status = 0; char *char_ptr = NULL; - char *cmd = NULL; char *ret = NULL; char *scsi_adapter = NULL; int slot = 0; @@ -2028,15 +1870,7 @@ phypAttachDevice(virDomainPtr domain, const char *xml) if (system_type == HMC) virBufferAddChar(&buf, '\''); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -2055,14 +1889,7 @@ phypAttachDevice(virDomainPtr domain, const char *xml) virBufferVSprintf(&buf, " slot_num,backing_device|grep %s|cut -d, -f1", dev->data.disk->src); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -2081,14 +1908,7 @@ phypAttachDevice(virDomainPtr domain, const char *xml) " -r prof --filter lpar_ids=%d,profile_names=%s" " -F virtual_scsi_adapters|sed -e 's/\"//g'", vios_id, profile); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -2104,15 +1924,8 @@ phypAttachDevice(virDomainPtr domain, const char *xml) "\"virtual_scsi_adapters=%s,%d/client/%d/%s/0\"'", domain_name, domain->id, ret, slot, vios_id, vios_name); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - VIR_FREE(ret); - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (virStrToLong_i(ret, &char_ptr, 10, &slot) == -1) goto err; @@ -2127,14 +1940,7 @@ phypAttachDevice(virDomainPtr domain, const char *xml) virBufferVSprintf(&buf, " -p %s -o a -s %d -d 0 -a \"adapter_type=server\"", domain_name, slot); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) { VIR_ERROR0(_ @@ -2143,7 +1949,6 @@ phypAttachDevice(virDomainPtr domain, const char *xml) goto err; } - VIR_FREE(cmd); VIR_FREE(ret); VIR_FREE(def); VIR_FREE(dev); @@ -2152,7 +1957,6 @@ phypAttachDevice(virDomainPtr domain, const char *xml) return 0; err: - VIR_FREE(cmd); VIR_FREE(ret); VIR_FREE(def); VIR_FREE(dev); @@ -2171,10 +1975,8 @@ phypVolumeGetKey(virConnectPtr conn, char *key, const char *name) int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *char_ptr; if (system_type == HMC) virBufferVSprintf(&buf, "viosvrcmd -m %s --id %d -c '", @@ -2186,33 +1988,18 @@ phypVolumeGetKey(virConnectPtr conn, char *key, const char *name) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|sed -e 's/^LV IDENTIFIER://' -e 's/ //g'"); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); if (exit_status < 0 || ret == NULL) goto err; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - if (memcpy(key, ret, MAX_KEY_SIZE) == NULL) goto err; - VIR_FREE(cmd); VIR_FREE(ret); return 0; err: - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -2227,10 +2014,8 @@ phypGetStoragePoolDevice(virConnectPtr conn, char *name) int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *char_ptr; if (system_type == HMC) virBufferVSprintf(&buf, "viosvrcmd -m %s --id %d -c '", @@ -2242,29 +2027,14 @@ phypGetStoragePoolDevice(virConnectPtr conn, char *name) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|sed '1d; s/ //g'"); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); if (exit_status < 0 || ret == NULL) goto err; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - - VIR_FREE(cmd); return ret; err: - VIR_FREE(cmd); VIR_FREE(ret); return NULL; } @@ -2279,7 +2049,6 @@ phypGetStoragePoolSize(virConnectPtr conn, char *name) int system_type = phyp_driver->system_type; int exit_status = 0; int vios_id = phyp_driver->vios_id; - char *cmd = NULL; char *ret = NULL; int sp_size = 0; char *char_ptr; @@ -2295,15 +2064,7 @@ phypGetStoragePoolSize(virConnectPtr conn, char *name) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|sed '1d; s/ //g'"); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -2311,12 +2072,10 @@ phypGetStoragePoolSize(virConnectPtr conn, char *name) if (virStrToLong_i(ret, &char_ptr, 10, &sp_size) == -1) goto err; - VIR_FREE(cmd); VIR_FREE(ret); return sp_size; err: - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -2331,7 +2090,6 @@ phypBuildVolume(virConnectPtr conn, const char *lvname, const char *spname, int vios_id = phyp_driver->vios_id; int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; - char *cmd = NULL; char *ret = NULL; int exit_status = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -2344,15 +2102,7 @@ phypBuildVolume(virConnectPtr conn, const char *lvname, const char *spname, if (system_type == HMC) virBufferAddChar(&buf, '\''); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0) { VIR_ERROR(_("Unable to create Volume: %s"), ret); @@ -2362,12 +2112,10 @@ phypBuildVolume(virConnectPtr conn, const char *lvname, const char *spname, if (phypVolumeGetKey(conn, key, lvname) == -1) goto err;; - VIR_FREE(cmd); VIR_FREE(ret); return 0; err: - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -2493,10 +2241,8 @@ phypVolumeGetPhysicalVolumeByStoragePool(virStorageVolPtr vol, char *sp) int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *char_ptr; if (system_type == HMC) virBufferVSprintf(&buf, "viosvrcmd -m %s --id %d -c '", @@ -2508,29 +2254,14 @@ phypVolumeGetPhysicalVolumeByStoragePool(virStorageVolPtr vol, char *sp) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|sed 1d"); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); if (exit_status < 0 || ret == NULL) goto err; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - - VIR_FREE(cmd); return ret; err: - VIR_FREE(cmd); VIR_FREE(ret); return NULL; @@ -2546,11 +2277,9 @@ phypVolumeLookupByPath(virConnectPtr conn, const char *volname) int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; char *key = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *char_ptr; virStorageVolPtr result = NULL; if (system_type == HMC) @@ -2563,24 +2292,11 @@ phypVolumeLookupByPath(virConnectPtr conn, const char *volname) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|sed -e 's/^VOLUME GROUP://g' -e 's/ //g'"); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); if (exit_status < 0 || ret == NULL) goto cleanup; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - if (VIR_ALLOC_N(key, MAX_KEY_SIZE) < 0) { virReportOOMError(); goto cleanup; @@ -2608,7 +2324,6 @@ phypGetStoragePoolUUID(virConnectPtr conn, unsigned char *uuid, int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -2622,15 +2337,7 @@ phypGetStoragePoolUUID(virConnectPtr conn, unsigned char *uuid, virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|sed '1,2d'"); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -2638,12 +2345,10 @@ phypGetStoragePoolUUID(virConnectPtr conn, unsigned char *uuid, if (memcpy(uuid, ret, VIR_UUID_BUFLEN) == NULL) goto err; - VIR_FREE(cmd); VIR_FREE(ret); return 0; err: - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -2749,11 +2454,9 @@ phypVolumeGetPath(virStorageVolPtr vol) int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; char *path = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - char *char_ptr; char *pv; if (system_type == HMC) @@ -2767,24 +2470,11 @@ phypVolumeGetPath(virStorageVolPtr vol) virBufferVSprintf(&buf, "|sed -e 's/^VOLUME GROUP://g' -e 's/ //g'"); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); if (exit_status < 0 || ret == NULL) goto err; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - pv = phypVolumeGetPhysicalVolumeByStoragePool(vol, ret); if (pv) { @@ -2796,12 +2486,10 @@ phypVolumeGetPath(virStorageVolPtr vol) goto err; } - VIR_FREE(cmd); VIR_FREE(ret); return path; err: - VIR_FREE(cmd); VIR_FREE(ret); VIR_FREE(path); return NULL; @@ -2822,7 +2510,6 @@ phypStoragePoolListVolumes(virStoragePoolPtr pool, char **const volumes, int exit_status = 0; int got = 0; int i; - char *cmd = NULL; char *ret = NULL; char *volumes_list = NULL; char *char_ptr2 = NULL; @@ -2838,15 +2525,7 @@ phypStoragePoolListVolumes(virStoragePoolPtr pool, char **const volumes, virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|sed '1,2d'"); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); /* I need to parse the textual return in order to get the volumes */ if (exit_status < 0 || ret == NULL) @@ -2870,14 +2549,12 @@ phypStoragePoolListVolumes(virStoragePoolPtr pool, char **const volumes, } } - VIR_FREE(cmd); VIR_FREE(ret); return got; err: for (i = 0; i < got; i++) VIR_FREE(volumes[i]); - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -2892,7 +2569,6 @@ phypStoragePoolNumOfVolumes(virStoragePoolPtr pool) int system_type = phyp_driver->system_type; int exit_status = 0; int nvolumes = 0; - char *cmd = NULL; char *ret = NULL; char *managed_system = phyp_driver->managed_system; int vios_id = phyp_driver->vios_id; @@ -2906,15 +2582,7 @@ phypStoragePoolNumOfVolumes(virStoragePoolPtr pool) if (system_type == HMC) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|grep -c '^.*$'"); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -2925,12 +2593,10 @@ phypStoragePoolNumOfVolumes(virStoragePoolPtr pool) /* We need to remove 2 line from the header text output */ nvolumes -= 2; - VIR_FREE(cmd); VIR_FREE(ret); return nvolumes; err: - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -2945,7 +2611,6 @@ phypDestroyStoragePool(virStoragePoolPtr pool) int vios_id = phyp_driver->vios_id; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; - char *cmd = NULL; char *ret = NULL; int exit_status = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -2958,35 +2623,17 @@ phypDestroyStoragePool(virStoragePoolPtr pool) if (system_type == HMC) virBufferAddChar(&buf, '\''); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - if (virAsprintf(&cmd, - "viosvrcmd -m %s --id %d -c " - "'rmsp %s'", managed_system, vios_id, - pool->name) < 0) { - virReportOOMError(); - goto err; - } - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0) { VIR_ERROR(_("Unable to create Storage Pool: %s"), ret); goto err; } - VIR_FREE(cmd); VIR_FREE(ret); return 0; err: - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -3001,7 +2648,6 @@ phypBuildStoragePool(virConnectPtr conn, virStoragePoolDefPtr def) int vios_id = phyp_driver->vios_id; int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; - char *cmd = NULL; char *ret = NULL; int exit_status = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -3015,27 +2661,17 @@ phypBuildStoragePool(virConnectPtr conn, virStoragePoolDefPtr def) if (system_type == HMC) virBufferAddChar(&buf, '\''); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0) { VIR_ERROR(_("Unable to create Storage Pool: %s"), NULLSTR(ret)); goto err; } - VIR_FREE(cmd); VIR_FREE(ret); return 0; err: - VIR_FREE(cmd); VIR_FREE(ret); return -1; @@ -3050,7 +2686,6 @@ phypNumOfStoragePools(virConnectPtr conn) int system_type = phyp_driver->system_type; int exit_status = 0; int nsp = 0; - char *cmd = NULL; char *ret = NULL; char *managed_system = phyp_driver->managed_system; int vios_id = phyp_driver->vios_id; @@ -3067,15 +2702,7 @@ phypNumOfStoragePools(virConnectPtr conn) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|grep -c '^.*$'"); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -3083,12 +2710,10 @@ phypNumOfStoragePools(virConnectPtr conn) if (virStrToLong_i(ret, &char_ptr, 10, &nsp) == -1) goto err; - VIR_FREE(cmd); VIR_FREE(ret); return nsp; err: - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -3105,7 +2730,6 @@ phypListStoragePools(virConnectPtr conn, char **const pools, int npools) int exit_status = 0; int got = 0; int i; - char *cmd = NULL; char *ret = NULL; char *storage_pools = NULL; char *char_ptr2 = NULL; @@ -3119,15 +2743,7 @@ phypListStoragePools(virConnectPtr conn, char **const pools, int npools) if (system_type == HMC) virBufferAddChar(&buf, '\''); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); /* I need to parse the textual return in order to get the storage pools */ if (exit_status < 0 || ret == NULL) @@ -3151,14 +2767,12 @@ phypListStoragePools(virConnectPtr conn, char **const pools, int npools) } } - VIR_FREE(cmd); VIR_FREE(ret); return got; err: for (i = 0; i < got; i++) VIR_FREE(pools[i]); - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -3321,7 +2935,6 @@ phypInterfaceDestroy(virInterfacePtr iface, int slot_num = 0; int lpar_id = 0; char *char_ptr; - char *cmd = NULL; char *ret = NULL; /* Getting the remote slot number */ @@ -3334,15 +2947,7 @@ phypInterfaceDestroy(virInterfacePtr iface, " -r virtualio --rsubtype eth --level lpar " " -F mac_addr,slot_num|" " sed -n '/%s/ s/^.*,//p'", iface->mac); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto err; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, iface->conn); + ret = phypExecBuffer(session, &buf, &exit_status, iface->conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -3351,7 +2956,6 @@ phypInterfaceDestroy(virInterfacePtr iface, goto err; /* Getting the remote slot number */ - VIR_FREE(cmd); VIR_FREE(ret); virBufferAddLit(&buf, "lshwres "); @@ -3362,15 +2966,7 @@ phypInterfaceDestroy(virInterfacePtr iface, " -r virtualio --rsubtype eth --level lpar " " -F mac_addr,lpar_id|" " sed -n '/%s/ s/^.*,//p'", iface->mac); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto err; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, iface->conn); + ret = phypExecBuffer(session, &buf, &exit_status, iface->conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -3379,7 +2975,6 @@ phypInterfaceDestroy(virInterfacePtr iface, goto err; /* excluding interface */ - VIR_FREE(cmd); VIR_FREE(ret); virBufferAddLit(&buf, "chhwres "); @@ -3389,25 +2984,15 @@ phypInterfaceDestroy(virInterfacePtr iface, virBufferVSprintf(&buf, " -r virtualio --rsubtype eth" " --id %d -o r -s %d", lpar_id, slot_num); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto err; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, iface->conn); + ret = phypExecBuffer(session, &buf, &exit_status, iface->conn, false); if (exit_status < 0 || ret != NULL) goto err; - VIR_FREE(cmd); VIR_FREE(ret); return 0; err: - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -3426,7 +3011,6 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, int system_type = phyp_driver->system_type; int exit_status = 0; char *char_ptr; - char *cmd = NULL; int slot = 0; char *ret = NULL; char name[PHYP_IFACENAME_SIZE]; @@ -3445,15 +3029,7 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, " -r virtualio --rsubtype slot --level slot" " -Fslot_num --filter lpar_names=%s" " |sort|tail -n 1", def->name); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto err; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -3465,7 +3041,6 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, slot++; /* Now adding the new network interface */ - VIR_FREE(cmd); VIR_FREE(ret); virBufferAddLit(&buf, "chhwres "); @@ -3476,15 +3051,7 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, " -r virtualio --rsubtype eth" " -p %s -o a -s %d -a port_vlan_id=1," "ieee_virtual_eth=0", def->name, slot); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto err; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret != NULL) goto err; @@ -3495,7 +3062,6 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, sleep(1); /* Getting the new interface name */ - VIR_FREE(cmd); VIR_FREE(ret); virBufferAddLit(&buf, "lshwres "); @@ -3506,19 +3072,10 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, " -r virtualio --rsubtype slot --level slot" " |sed '/lpar_name=%s/!d; /slot_num=%d/!d; " "s/^.*drc_name=//'", def->name, slot); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto err; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) { /* roll back and excluding interface if error*/ - VIR_FREE(cmd); VIR_FREE(ret); virBufferAddLit(&buf, "chhwres "); @@ -3528,23 +3085,13 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, virBufferVSprintf(&buf, " -r virtualio --rsubtype eth" " -p %s -o r -s %d", def->name, slot); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto err; - } - - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); goto err; } memcpy(name, ret, PHYP_IFACENAME_SIZE-1); /* Getting the new interface mac addr */ - VIR_FREE(cmd); VIR_FREE(ret); virBufferAddLit(&buf, "lshwres "); @@ -3555,28 +3102,18 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, "-r virtualio --rsubtype eth --level lpar " " |sed '/lpar_name=%s/!d; /slot_num=%d/!d; " "s/^.*mac_addr=//'", def->name, slot); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto err; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; memcpy(mac, ret, PHYP_MAC_SIZE-1); - VIR_FREE(cmd); VIR_FREE(ret); virInterfaceDefFree(def); return virGetInterface(conn, name, mac); err: - VIR_FREE(cmd); VIR_FREE(ret); virInterfaceDefFree(def); return NULL; @@ -3593,7 +3130,6 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) int system_type = phyp_driver->system_type; int exit_status = 0; char *char_ptr; - char *cmd = NULL; char *ret = NULL; int slot = 0; int lpar_id = 0; @@ -3609,15 +3145,7 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) " -r virtualio --rsubtype slot --level slot " " -F drc_name,slot_num |" " sed -n '/%s/ s/^.*,//p'", name); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto err; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -3626,7 +3154,6 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) goto err; /*Getting the lpar_id for the interface */ - VIR_FREE(cmd); VIR_FREE(ret); virBufferAddLit(&buf, "lshwres "); @@ -3637,15 +3164,7 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) " -r virtualio --rsubtype slot --level slot " " -F drc_name,lpar_id |" " sed -n '/%s/ s/^.*,//p'", name); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -3662,17 +3181,8 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) " -r virtualio --rsubtype eth --level lpar " " -F lpar_id,slot_num,mac_addr|" " sed -n '/%d,%d/ s/^.*,//p'", lpar_id, slot); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return NULL; - } - cmd = virBufferContentAndReset(&buf); - VIR_FREE(ret); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -3681,12 +3191,10 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) result = virGetInterface(conn, name, ret); - VIR_FREE(cmd); VIR_FREE(ret); return result; err: - VIR_FREE(cmd); VIR_FREE(ret); return NULL; @@ -3704,7 +3212,6 @@ phypInterfaceIsActive(virInterfacePtr iface) int exit_status = 0; int state = 0; char *char_ptr; - char *cmd = NULL; char *ret = NULL; virBufferAddLit(&buf, "lshwres "); @@ -3715,15 +3222,7 @@ phypInterfaceIsActive(virInterfacePtr iface) " -r virtualio --rsubtype eth --level lpar " " -F mac_addr,state |" " sed -n '/%s/ s/^.*,//p'", iface->mac); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, iface->conn); + ret = phypExecBuffer(session, &buf, &exit_status, iface->conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -3731,12 +3230,10 @@ phypInterfaceIsActive(virInterfacePtr iface) if (virStrToLong_i(ret, &char_ptr, 10, &state) == -1) goto err; - VIR_FREE(cmd); VIR_FREE(ret); return state; err: - VIR_FREE(cmd); VIR_FREE(ret); return -1; @@ -3754,7 +3251,6 @@ phypListInterfaces(virConnectPtr conn, char **const names, int nnames) int exit_status = 0; int got = 0; int i; - char *cmd = NULL; char *ret = NULL; char *networks = NULL; char *char_ptr2 = NULL; @@ -3766,16 +3262,10 @@ phypListInterfaces(virConnectPtr conn, char **const names, int nnames) virBufferVSprintf(&buf, " -r virtualio --rsubtype slot --level slot|" " sed '/eth/!d; /lpar_id=%d/d; s/^.*drc_name=//g'", vios_id); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - goto err; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); - /* I need to parse the textual return in order to get the network interfaces */ + /* I need to parse the textual return in order to get the network + * interfaces */ if (exit_status < 0 || ret == NULL) goto err; @@ -3797,14 +3287,12 @@ phypListInterfaces(virConnectPtr conn, char **const names, int nnames) } } - VIR_FREE(cmd); VIR_FREE(ret); return got; err: for (i = 0; i < got; i++) VIR_FREE(names[i]); - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -3821,7 +3309,6 @@ phypNumOfInterfaces(virConnectPtr conn) int exit_status = 0; int nnets = 0; char *char_ptr; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -3832,15 +3319,7 @@ phypNumOfInterfaces(virConnectPtr conn) virBufferVSprintf(&buf, "-r virtualio --rsubtype eth --level lpar|" "grep -v lpar_id=%d|grep -c lpar_name", vios_id); - - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) goto err; @@ -3848,12 +3327,10 @@ phypNumOfInterfaces(virConnectPtr conn) if (virStrToLong_i(ret, &char_ptr, 10, &nnets) == -1) goto err; - VIR_FREE(cmd); VIR_FREE(ret); return nnets; err: - VIR_FREE(cmd); VIR_FREE(ret); return -1; @@ -3866,10 +3343,8 @@ phypGetLparState(virConnectPtr conn, unsigned int lpar_id) phyp_driverPtr phyp_driver = conn->privateData; LIBSSH2_SESSION *session = connection_data->session; int system_type = phyp_driver->system_type; - 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; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -3878,23 +3353,11 @@ phypGetLparState(virConnectPtr conn, unsigned int lpar_id) if (system_type == HMC) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -F state --filter lpar_ids=%d", lpar_id); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return state; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); if (exit_status < 0 || ret == NULL) goto cleanup; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - if (STREQ(ret, "Running")) state = VIR_DOMAIN_RUNNING; else if (STREQ(ret, "Not Activated")) @@ -3903,7 +3366,6 @@ phypGetLparState(virConnectPtr conn, unsigned int lpar_id) state = VIR_DOMAIN_SHUTDOWN; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return state; } @@ -3917,10 +3379,8 @@ phypDiskType(virConnectPtr conn, char *backing_device) ConnectionData *connection_data = conn->networkPrivateData; LIBSSH2_SESSION *session = connection_data->session; int system_type = phyp_driver->system_type; - 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; @@ -3932,30 +3392,17 @@ phypDiskType(virConnectPtr conn, char *backing_device) virBufferVSprintf(&buf, " -p %d -c \"lssp -field name type " "-fmt , -all|sed -n '/%s/ {\n s/^.*,//\n p\n}'\"", vios_id, backing_device); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return disk_type; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, true); if (exit_status < 0 || ret == NULL) goto cleanup; - char_ptr = strchr(ret, '\n'); - - if (char_ptr) - *char_ptr = '\0'; - if (STREQ(ret, "LVPOOL")) disk_type = VIR_DOMAIN_DISK_TYPE_BLOCK; else if (STREQ(ret, "FBPOOL")) disk_type = VIR_DOMAIN_DISK_TYPE_FILE; cleanup: - VIR_FREE(cmd); VIR_FREE(ret); return disk_type; } @@ -3989,7 +3436,6 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int nnames) int exit_status = 0; int got = 0; int i; - char *cmd = NULL; char *ret = NULL; char *domains = NULL; char *char_ptr2 = NULL; @@ -4000,14 +3446,7 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int nnames) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -F name,state" "|sed -n '/Not Activated/ {\n s/,.*$//\n p\n}'"); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); /* I need to parse the textual return in order to get the domains */ if (exit_status < 0 || ret == NULL) @@ -4031,14 +3470,12 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int nnames) } } - VIR_FREE(cmd); VIR_FREE(ret); return got; err: for (i = 0; i < got; i++) VIR_FREE(names[i]); - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -4162,7 +3599,6 @@ phypDomainResume(virDomainPtr dom) int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -4171,24 +3607,15 @@ phypDomainResume(virDomainPtr dom) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -r lpar -o on --id %d -f %s", dom->id, dom->name); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, dom->conn); + ret = phypExecBuffer(session, &buf, &exit_status, dom->conn, false); if (exit_status < 0) goto err; - VIR_FREE(cmd); VIR_FREE(ret); return 0; err: - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -4203,7 +3630,6 @@ phypDomainShutdown(virDomainPtr dom) int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -4211,24 +3637,15 @@ phypDomainShutdown(virDomainPtr dom) if (system_type == HMC) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -r lpar -o shutdown --id %d", dom->id); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return 0; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, dom->conn); + ret = phypExecBuffer(session, &buf, &exit_status, dom->conn, false); if (exit_status < 0) goto err; - VIR_FREE(cmd); VIR_FREE(ret); return 0; err: - VIR_FREE(cmd); VIR_FREE(ret); return 0; } @@ -4265,7 +3682,6 @@ phypDomainDestroy(virDomainPtr dom) int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -4273,14 +3689,7 @@ phypDomainDestroy(virDomainPtr dom) if (system_type == HMC) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -r lpar --id %d", dom->id); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, dom->conn); + ret = phypExecBuffer(session, &buf, &exit_status, dom->conn, false); if (exit_status < 0) goto err; @@ -4290,12 +3699,10 @@ phypDomainDestroy(virDomainPtr dom) dom->id = -1; - VIR_FREE(cmd); VIR_FREE(ret); return 0; err: - VIR_FREE(cmd); VIR_FREE(ret); return -1; @@ -4309,7 +3716,6 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) LIBSSH2_SESSION *session = connection_data->session; int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; - char *cmd = NULL; char *ret = NULL; int exit_status = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -4349,14 +3755,7 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) def->name, (int) def->mem.cur_balloon, (int) def->mem.cur_balloon, (int) def->mem.max_balloon, (int) def->vcpus, def->disks[0]->src); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return -1; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, conn); + ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0) { VIR_ERROR(_("Unable to create LPAR. Reason: '%s'"), ret); @@ -4368,12 +3767,10 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr def) goto err; } - VIR_FREE(cmd); VIR_FREE(ret); return 0; err: - VIR_FREE(cmd); VIR_FREE(ret); return -1; } @@ -4454,7 +3851,6 @@ phypDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, int system_type = phyp_driver->system_type; char *managed_system = phyp_driver->managed_system; int exit_status = 0; - char *cmd = NULL; char *ret = NULL; char operation; unsigned long ncpus = 0; @@ -4490,14 +3886,7 @@ phypDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, virBufferVSprintf(&buf, " --id %d -o %c --procunits %d 2>&1 |sed " "-e 's/^.*\\([0-9][0-9]*.[0-9][0-9]*\\).*$/\\1/'", dom->id, operation, amount); - if (virBufferError(&buf)) { - virBufferFreeAndReset(&buf); - virReportOOMError(); - return 0; - } - cmd = virBufferContentAndReset(&buf); - - ret = phypExec(session, cmd, &exit_status, dom->conn); + ret = phypExecBuffer(session, &buf, &exit_status, dom->conn, false); if (exit_status < 0) { VIR_ERROR0(_ @@ -4505,7 +3894,6 @@ phypDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, " Contact your support to enable this feature.")); } - VIR_FREE(cmd); VIR_FREE(ret); return 0; -- 1.7.4.2

Rather than copying and pasting lots of code, factor it into a single helper function. * src/phyp/phyp_driver.c (phypExecInt): New function. (phypGetVIOSPartitionID, phypNumDomainsGeneric, phypGetLparID) (phypGetLparMem, phypGetLparCPUGeneric, phypGetRemoteSlot) (phypGetVIOSNextSlotNumber, phypAttachDevice) (phypGetStoragePoolSize, phypStoragePoolNumOfVolumes) (phypNumOfStoragePools, phypInterfaceDestroy) (phypInterfaceDefineXML, phypInterfaceLookupByName) (phypInterfaceIsActive, phypNumOfInterfaces): Use it. --- src/phyp/phyp_driver.c | 353 +++++++++++------------------------------------- 1 files changed, 76 insertions(+), 277 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index cd0e5f9..d7043f3 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -223,2 +223,2 @@ phypExecBuffer(LIBSSH2_SESSION *session, virBufferPtr buf, int *exit_status, return ret; } +/* Convenience wrapper function */ +static int ATTRIBUTE_NONNULL(1) +phypExecInt(LIBSSH2_SESSION *session, virBufferPtr buf, virConnectPtr conn, + int *result) +{ + char *str; + int ret; + + str = phypExecBuffer(session, buf, &ret, conn, true); + if (!str || ret) { + VIR_FREE(str); + return -1; + } + ret = virStrToLong_i(str, NULL, 10, result); + VIR_FREE(str); + return ret; +} + static int phypGetSystemType(virConnectPtr conn) { @@ -250,10 +268,7 @@ phypGetVIOSPartitionID(virConnectPtr conn) phyp_driverPtr phyp_driver = conn->privateData; LIBSSH2_SESSION *session = connection_data->session; int system_type = phyp_driver->system_type; - char *ret = NULL; - int exit_status = 0; int id = -1; - char *char_ptr; char *managed_system = phyp_driver->managed_system; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -262,20 +277,9 @@ phypGetVIOSPartitionID(virConnectPtr conn) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferAddLit(&buf, " -r lpar -F lpar_id,lpar_env" "|sed -n '/vioserver/ {\n s/,.*$//\n p\n}'"); - ret = phypExecBuffer(session, &buf, &exit_status, conn, false); - - if (exit_status < 0 || ret == NULL) - goto err; - - if (virStrToLong_i(ret, &char_ptr, 10, &id) == -1) - goto err; - - VIR_FREE(ret); + if (phypExecInt(session, &buf, conn, &id) < 0) + return -1; return id; - - err: - VIR_FREE(ret); - return -1; } static virCapsPtr @@ -337,10 +341,7 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type) phyp_driverPtr phyp_driver = conn->privateData; LIBSSH2_SESSION *session = connection_data->session; int system_type = phyp_driver->system_type; - int exit_status = 0; int ndom = 0; - char *char_ptr; - char *ret = NULL; char *managed_system = phyp_driver->managed_system; const char *state; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -361,20 +362,9 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -F lpar_id,state %s |grep -c '^[0-9]*'", state); - ret = phypExecBuffer(session, &buf, &exit_status, conn, false); - - if (exit_status < 0 || ret == NULL) - goto err; - - if (virStrToLong_i(ret, &char_ptr, 10, &ndom) == -1) - goto err; - - VIR_FREE(ret); + if (phypExecInt(session, &buf, conn, &ndom) < 0) + return -1; return ndom; - - err: - VIR_FREE(ret); - return -1; } /* This is a generic function that won't be used directly by @@ -1294,30 +1284,16 @@ phypGetLparID(LIBSSH2_SESSION * session, const char *managed_system, { phyp_driverPtr phyp_driver = conn->privateData; int system_type = phyp_driver->system_type; - int exit_status = 0; int lpar_id = 0; - char *char_ptr; - char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; virBufferAddLit(&buf, "lssyscfg -r lpar"); if (system_type == HMC) virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " --filter lpar_names=%s -F lpar_id", name); - ret = phypExecBuffer(session, &buf, &exit_status, conn, false); - - if (exit_status < 0 || ret == NULL) - goto err; - - if (virStrToLong_i(ret, &char_ptr, 10, &lpar_id) == -1) - goto err; - - VIR_FREE(ret); + if (phypExecInt(session, &buf, conn, &lpar_id) < 0) + lpar_id = -1; return lpar_id; - - err: - VIR_FREE(ret); - return -1; } /* return the lpar name given a lpar_id and a managed system name */ @@ -1385,10 +1361,7 @@ phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id, LIBSSH2_SESSION *session = connection_data->session; phyp_driverPtr phyp_driver = conn->privateData; int system_type = phyp_driver->system_type; - char *ret = NULL; - char *char_ptr; int memory = 0; - int exit_status = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; if (type != 1 && type != 0) @@ -1400,21 +1373,9 @@ phypGetLparMem(virConnectPtr conn, const char *managed_system, int lpar_id, virBufferVSprintf(&buf, " -r mem --level lpar -F %s --filter lpar_ids=%d", type ? "curr_mem" : "curr_max_mem", lpar_id); - ret = phypExecBuffer(session, &buf, &exit_status, conn, false); - - if (exit_status < 0 || ret == NULL) - goto err; - - if (virStrToLong_i(ret, &char_ptr, 10, &memory) == -1) - goto err; - - VIR_FREE(ret); + if (phypExecInt(session, &buf, conn, &memory) < 0) + return 0; return memory; - - err: - VIR_FREE(ret); - return 0; - } static unsigned long @@ -1425,9 +1386,6 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, LIBSSH2_SESSION *session = connection_data->session; phyp_driverPtr phyp_driver = conn->privateData; int system_type = phyp_driver->system_type; - char *ret = NULL; - char *char_ptr; - int exit_status = 0; int vcpus = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -1437,20 +1395,9 @@ phypGetLparCPUGeneric(virConnectPtr conn, const char *managed_system, virBufferVSprintf(&buf, " -r proc --level lpar -F %s --filter lpar_ids=%d", type ? "curr_max_procs" : "curr_procs", lpar_id); - ret = phypExecBuffer(session, &buf, &exit_status, conn, false); - - if (exit_status < 0 || ret == NULL) - goto err; - - if (virStrToLong_i(ret, &char_ptr, 10, &vcpus) == -1) - goto err; - - VIR_FREE(ret); - return (unsigned long) vcpus; - - err: - VIR_FREE(ret); - return 0; + if (phypExecInt(session, &buf, conn, &vcpus) < 0) + return 0; + return vcpus; } static unsigned long @@ -1488,10 +1435,7 @@ phypGetRemoteSlot(virConnectPtr conn, const char *managed_system, LIBSSH2_SESSION *session = connection_data->session; phyp_driverPtr phyp_driver = conn->privateData; int system_type = phyp_driver->system_type; - char *ret = NULL; - char *char_ptr; int remote_slot = 0; - int exit_status = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; virBufferAddLit(&buf, "lshwres"); @@ -1499,20 +1443,9 @@ phypGetRemoteSlot(virConnectPtr conn, const char *managed_system, virBufferVSprintf(&buf, " -m %s", managed_system); virBufferVSprintf(&buf, " -r virtualio --rsubtype scsi -F " "remote_slot_num --filter lpar_names=%s", lpar_name); - ret = phypExecBuffer(session, &buf, &exit_status, conn, false); - - if (exit_status < 0 || ret == NULL) - goto err; - - if (virStrToLong_i(ret, &char_ptr, 10, &remote_slot) == -1) - goto err; - - VIR_FREE(ret); + if (phypExecInt(session, &buf, conn, &remote_slot) < 0) + return -1; return remote_slot; - - err: - VIR_FREE(ret); - return -1; } /* XXX - is this needed? */ @@ -1626,16 +1559,13 @@ phypGetVIOSNextSlotNumber(virConnectPtr conn) char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; - int exit_status = 0; - char *char_ptr; - char *ret = NULL; char *profile = NULL; int slot = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; if (!(profile = phypGetLparProfile(conn, vios_id))) { VIR_ERROR0(_("Unable to get VIOS profile name.")); - goto err; + return -1; } virBufferAddLit(&buf, "lssyscfg"); @@ -1649,20 +1579,9 @@ phypGetVIOSNextSlotNumber(virConnectPtr conn) "virtual_serial_adapters|sed -e 's/\"//g' -e " "'s/,/\\n/g'|sed -e 's/\\(^[0-9][0-9]\\*\\).*$/\\1/'" "|sort|tail -n 1", profile); - ret = phypExecBuffer(session, &buf, &exit_status, conn, false); - - if (exit_status < 0 || ret == NULL) - goto err; - - if (virStrToLong_i(ret, &char_ptr, 10, &slot) == -1) - goto err; - - VIR_FREE(ret); + if (phypExecInt(session, &buf, conn, &slot) < 0) + return -1; return slot + 1; - - err: - VIR_FREE(ret); - return -1; } static int @@ -1802,7 +1721,6 @@ phypAttachDevice(virDomainPtr domain, const char *xml) int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; int exit_status = 0; - char *char_ptr = NULL; char *ret = NULL; char *scsi_adapter = NULL; int slot = 0; @@ -1889,18 +1807,12 @@ phypAttachDevice(virDomainPtr domain, const char *xml) virBufferVSprintf(&buf, " slot_num,backing_device|grep %s|cut -d, -f1", dev->data.disk->src); - ret = phypExecBuffer(session, &buf, &exit_status, conn, false); - - if (exit_status < 0 || ret == NULL) - goto err; - - if (virStrToLong_i(ret, &char_ptr, 10, &slot) == -1) + if (phypExecInt(session, &buf, conn, &slot) < 0) goto err; /* Listing all the virtual_scsi_adapter interfaces, the new adapter must * be appended to this list * */ - VIR_FREE(ret); virBufferAddLit(&buf, "lssyscfg"); if (system_type == HMC) virBufferVSprintf(&buf, " -m %s", managed_system); @@ -1924,10 +1836,7 @@ phypAttachDevice(virDomainPtr domain, const char *xml) "\"virtual_scsi_adapters=%s,%d/client/%d/%s/0\"'", domain_name, domain->id, ret, slot, vios_id, vios_name); - VIR_FREE(ret); - ret = phypExecBuffer(session, &buf, &exit_status, conn, false); - - if (virStrToLong_i(ret, &char_ptr, 10, &slot) == -1) + if (phypExecInt(session, &buf, conn, &slot) < 0) goto err; /* Finally I add the new scsi adapter to VIOS using the same slot @@ -2047,11 +1956,8 @@ phypGetStoragePoolSize(virConnectPtr conn, char *name) LIBSSH2_SESSION *session = connection_data->session; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; - int exit_status = 0; int vios_id = phyp_driver->vios_id; - char *ret = NULL; int sp_size = 0; - char *char_ptr; virBuffer buf = VIR_BUFFER_INITIALIZER; if (system_type == HMC) @@ -2064,20 +1970,9 @@ phypGetStoragePoolSize(virConnectPtr conn, char *name) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|sed '1d; s/ //g'"); - ret = phypExecBuffer(session, &buf, &exit_status, conn, false); - - if (exit_status < 0 || ret == NULL) - goto err; - - if (virStrToLong_i(ret, &char_ptr, 10, &sp_size) == -1) - goto err; - - VIR_FREE(ret); + if (phypExecInt(session, &buf, conn, &sp_size) < 0) + return -1; return sp_size; - - err: - VIR_FREE(ret); - return -1; } static int @@ -2512,7 +2407,7 @@ phypStoragePoolListVolumes(virStoragePoolPtr pool, char **const volumes, int i; char *ret = NULL; char *volumes_list = NULL; - char *char_ptr2 = NULL; + char *char_ptr = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; if (system_type == HMC) @@ -2534,16 +2429,16 @@ phypStoragePoolListVolumes(virStoragePoolPtr pool, char **const volumes, volumes_list = ret; while (got < nvolumes) { - char_ptr2 = strchr(volumes_list, '\n'); + char_ptr = strchr(volumes_list, '\n'); - if (char_ptr2) { - *char_ptr2 = '\0'; + if (char_ptr) { + *char_ptr = '\0'; if ((volumes[got++] = strdup(volumes_list)) == NULL) { virReportOOMError(); goto err; } - char_ptr2++; - volumes_list = char_ptr2; + char_ptr++; + volumes_list = char_ptr; } else break; } @@ -2567,12 +2462,9 @@ phypStoragePoolNumOfVolumes(virStoragePoolPtr pool) phyp_driverPtr phyp_driver = conn->privateData; LIBSSH2_SESSION *session = connection_data->session; int system_type = phyp_driver->system_type; - int exit_status = 0; int nvolumes = 0; - char *ret = NULL; char *managed_system = phyp_driver->managed_system; int vios_id = phyp_driver->vios_id; - char *char_ptr; virBuffer buf = VIR_BUFFER_INITIALIZER; if (system_type == HMC) @@ -2582,23 +2474,11 @@ phypStoragePoolNumOfVolumes(virStoragePoolPtr pool) if (system_type == HMC) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|grep -c '^.*$'"); - ret = phypExecBuffer(session, &buf, &exit_status, conn, false); - - if (exit_status < 0 || ret == NULL) - goto err; - - if (virStrToLong_i(ret, &char_ptr, 10, &nvolumes) == -1) - goto err; + if (phypExecInt(session, &buf, conn, &nvolumes) < 0) + return -1; /* We need to remove 2 line from the header text output */ - nvolumes -= 2; - - VIR_FREE(ret); - return nvolumes; - - err: - VIR_FREE(ret); - return -1; + return nvolumes - 2; } static int @@ -2684,12 +2564,9 @@ phypNumOfStoragePools(virConnectPtr conn) phyp_driverPtr phyp_driver = conn->privateData; LIBSSH2_SESSION *session = connection_data->session; int system_type = phyp_driver->system_type; - int exit_status = 0; int nsp = 0; - char *ret = NULL; char *managed_system = phyp_driver->managed_system; int vios_id = phyp_driver->vios_id; - char *char_ptr; virBuffer buf = VIR_BUFFER_INITIALIZER; if (system_type == HMC) @@ -2702,20 +2579,9 @@ phypNumOfStoragePools(virConnectPtr conn) virBufferAddChar(&buf, '\''); virBufferVSprintf(&buf, "|grep -c '^.*$'"); - ret = phypExecBuffer(session, &buf, &exit_status, conn, false); - - if (exit_status < 0 || ret == NULL) - goto err; - - if (virStrToLong_i(ret, &char_ptr, 10, &nsp) == -1) - goto err; - - VIR_FREE(ret); + if (phypExecInt(session, &buf, conn, &nsp) < 0) + return -1; return nsp; - - err: - VIR_FREE(ret); - return -1; } static int @@ -2732,7 +2598,7 @@ phypListStoragePools(virConnectPtr conn, char **const pools, int npools) int i; char *ret = NULL; char *storage_pools = NULL; - char *char_ptr2 = NULL; + char *char_ptr = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; if (system_type == HMC) @@ -2752,16 +2618,16 @@ phypListStoragePools(virConnectPtr conn, char **const pools, int npools) storage_pools = ret; while (got < npools) { - char_ptr2 = strchr(storage_pools, '\n'); + char_ptr = strchr(storage_pools, '\n'); - if (char_ptr2) { - *char_ptr2 = '\0'; + if (char_ptr) { + *char_ptr = '\0'; if ((pools[got++] = strdup(storage_pools)) == NULL) { virReportOOMError(); goto err; } - char_ptr2++; - storage_pools = char_ptr2; + char_ptr++; + storage_pools = char_ptr; } else break; } @@ -2934,7 +2800,6 @@ phypInterfaceDestroy(virInterfacePtr iface, int exit_status = 0; int slot_num = 0; int lpar_id = 0; - char *char_ptr; char *ret = NULL; /* Getting the remote slot number */ @@ -2947,17 +2812,10 @@ phypInterfaceDestroy(virInterfacePtr iface, " -r virtualio --rsubtype eth --level lpar " " -F mac_addr,slot_num|" " sed -n '/%s/ s/^.*,//p'", iface->mac); - ret = phypExecBuffer(session, &buf, &exit_status, iface->conn, false); - - if (exit_status < 0 || ret == NULL) - goto err; - - if (virStrToLong_i(ret, &char_ptr, 10, &slot_num) == -1) + if (phypExecInt(session, &buf, iface->conn, &slot_num) < 0) goto err; /* Getting the remote slot number */ - VIR_FREE(ret); - virBufferAddLit(&buf, "lshwres "); if (system_type == HMC) virBufferVSprintf(&buf, "-m %s ", managed_system); @@ -2966,17 +2824,10 @@ phypInterfaceDestroy(virInterfacePtr iface, " -r virtualio --rsubtype eth --level lpar " " -F mac_addr,lpar_id|" " sed -n '/%s/ s/^.*,//p'", iface->mac); - ret = phypExecBuffer(session, &buf, &exit_status, iface->conn, false); - - if (exit_status < 0 || ret == NULL) - goto err; - - if (virStrToLong_i(ret, &char_ptr, 10, &lpar_id) == -1) + if (phypExecInt(session, &buf, iface->conn, &lpar_id) < 0) goto err; /* excluding interface */ - VIR_FREE(ret); - virBufferAddLit(&buf, "chhwres "); if (system_type == HMC) virBufferVSprintf(&buf, "-m %s ", managed_system); @@ -3010,7 +2861,6 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int exit_status = 0; - char *char_ptr; int slot = 0; char *ret = NULL; char name[PHYP_IFACENAME_SIZE]; @@ -3029,20 +2879,13 @@ phypInterfaceDefineXML(virConnectPtr conn, const char *xml, " -r virtualio --rsubtype slot --level slot" " -Fslot_num --filter lpar_names=%s" " |sort|tail -n 1", def->name); - ret = phypExecBuffer(session, &buf, &exit_status, conn, false); - - if (exit_status < 0 || ret == NULL) - goto err; - - if (virStrToLong_i(ret, &char_ptr, 10, &slot) == -1) + if (phypExecInt(session, &buf, conn, &slot) < 0) goto err; /* The next free slot itself: */ slot++; /* Now adding the new network interface */ - VIR_FREE(ret); - virBufferAddLit(&buf, "chhwres "); if (system_type == HMC) virBufferVSprintf(&buf, "-m %s ", managed_system); @@ -3129,7 +2972,6 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int exit_status = 0; - char *char_ptr; char *ret = NULL; int slot = 0; int lpar_id = 0; @@ -3145,17 +2987,10 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) " -r virtualio --rsubtype slot --level slot " " -F drc_name,slot_num |" " sed -n '/%s/ s/^.*,//p'", name); - ret = phypExecBuffer(session, &buf, &exit_status, conn, false); - - if (exit_status < 0 || ret == NULL) - goto err; - - if (virStrToLong_i(ret, &char_ptr, 10, &slot) == -1) + if (phypExecInt(session, &buf, conn, &slot) < 0) goto err; /*Getting the lpar_id for the interface */ - VIR_FREE(ret); - virBufferAddLit(&buf, "lshwres "); if (system_type == HMC) virBufferVSprintf(&buf, "-m %s ", managed_system); @@ -3164,12 +2999,7 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) " -r virtualio --rsubtype slot --level slot " " -F drc_name,lpar_id |" " sed -n '/%s/ s/^.*,//p'", name); - ret = phypExecBuffer(session, &buf, &exit_status, conn, false); - - if (exit_status < 0 || ret == NULL) - goto err; - - if (virStrToLong_i(ret, &char_ptr, 10, &lpar_id) == -1) + if (phypExecInt(session, &buf, conn, &lpar_id) < 0) goto err; /*Getting the interface mac */ @@ -3181,7 +3011,6 @@ phypInterfaceLookupByName(virConnectPtr conn, const char *name) " -r virtualio --rsubtype eth --level lpar " " -F lpar_id,slot_num,mac_addr|" " sed -n '/%d,%d/ s/^.*,//p'", lpar_id, slot); - VIR_FREE(ret); ret = phypExecBuffer(session, &buf, &exit_status, conn, false); if (exit_status < 0 || ret == NULL) @@ -3209,10 +3038,7 @@ phypInterfaceIsActive(virInterfacePtr iface) virBuffer buf = VIR_BUFFER_INITIALIZER; char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; - int exit_status = 0; int state = 0; - char *char_ptr; - char *ret = NULL; virBufferAddLit(&buf, "lshwres "); if (system_type == HMC) @@ -3222,21 +3048,9 @@ phypInterfaceIsActive(virInterfacePtr iface) " -r virtualio --rsubtype eth --level lpar " " -F mac_addr,state |" " sed -n '/%s/ s/^.*,//p'", iface->mac); - ret = phypExecBuffer(session, &buf, &exit_status, iface->conn, false); - - if (exit_status < 0 || ret == NULL) - goto err; - - if (virStrToLong_i(ret, &char_ptr, 10, &state) == -1) - goto err; - - VIR_FREE(ret); + if (phypExecInt(session, &buf, iface->conn, &state) < 0) + return -1; return state; - - err: - VIR_FREE(ret); - return -1; - } static int @@ -3253,7 +3067,7 @@ phypListInterfaces(virConnectPtr conn, char **const names, int nnames) int i; char *ret = NULL; char *networks = NULL; - char *char_ptr2 = NULL; + char *char_ptr = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; virBufferAddLit(&buf, "lshwres"); @@ -3272,16 +3086,16 @@ phypListInterfaces(virConnectPtr conn, char **const names, int nnames) networks = ret; while (got < nnames) { - char_ptr2 = strchr(networks, '\n'); + char_ptr = strchr(networks, '\n'); - if (char_ptr2) { - *char_ptr2 = '\0'; + if (char_ptr) { + *char_ptr = '\0'; if ((names[got++] = strdup(networks)) == NULL) { virReportOOMError(); goto err; } - char_ptr2++; - networks = char_ptr2; + char_ptr++; + networks = char_ptr; } else { break; } @@ -3306,10 +3120,7 @@ phypNumOfInterfaces(virConnectPtr conn) char *managed_system = phyp_driver->managed_system; int system_type = phyp_driver->system_type; int vios_id = phyp_driver->vios_id; - int exit_status = 0; int nnets = 0; - char *char_ptr; - char *ret = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; virBufferAddLit(&buf, "lshwres "); @@ -3319,21 +3130,9 @@ phypNumOfInterfaces(virConnectPtr conn) virBufferVSprintf(&buf, "-r virtualio --rsubtype eth --level lpar|" "grep -v lpar_id=%d|grep -c lpar_name", vios_id); - ret = phypExecBuffer(session, &buf, &exit_status, conn, false); - - if (exit_status < 0 || ret == NULL) - goto err; - - if (virStrToLong_i(ret, &char_ptr, 10, &nnets) == -1) - goto err; - - VIR_FREE(ret); + if (phypExecInt(session, &buf, conn, &nnets) < 0) + return -1; return nnets; - - err: - VIR_FREE(ret); - return -1; - } static int @@ -3438,7 +3237,7 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int nnames) int i; char *ret = NULL; char *domains = NULL; - char *char_ptr2 = NULL; + char *char_ptr = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; virBufferAddLit(&buf, "lssyscfg -r lpar"); @@ -3455,16 +3254,16 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int nnames) domains = ret; while (got < nnames) { - char_ptr2 = strchr(domains, '\n'); + char_ptr = strchr(domains, '\n'); - if (char_ptr2) { - *char_ptr2 = '\0'; + if (char_ptr) { + *char_ptr = '\0'; if ((names[got++] = strdup(domains)) == NULL) { virReportOOMError(); goto err; } - char_ptr2++; - domains = char_ptr2; + char_ptr++; + domains = char_ptr; } else break; } -- 1.7.4.2

On 04/13/2011 04:07 PM, Eric Blake wrote:
Phyp still had several memory leaks and in general some bad copy-and-paste design. This series consolidates some common patterns into simpler helpers, avoiding leaks in the process.
Eric Blake (4): phyp: prefer memcpy over memmove when legal phyp: avoid memory leaks in return values phyp: avoid memory leaks in command values phyp: another simplification
src/phyp/phyp_driver.c | 1128 +++++++----------------------------------------- 1 files changed, 163 insertions(+), 965 deletions(-)
Serves me right for thinking Matthias' cleanups had already been applied. Now I get to rebase, with lots of conflicts :) -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2011/4/14 Eric Blake <eblake@redhat.com>:
On 04/13/2011 04:07 PM, Eric Blake wrote:
Phyp still had several memory leaks and in general some bad copy-and-paste design. This series consolidates some common patterns into simpler helpers, avoiding leaks in the process.
Eric Blake (4): phyp: prefer memcpy over memmove when legal phyp: avoid memory leaks in return values phyp: avoid memory leaks in command values phyp: another simplification
src/phyp/phyp_driver.c | 1128 +++++++----------------------------------------- 1 files changed, 163 insertions(+), 965 deletions(-)
Serves me right for thinking Matthias' cleanups had already been applied. Now I get to rebase, with lots of conflicts :)
Sorry for that :( I'm currently quite busy with my studies and can only spent some time on weekends for libvirt development to get some distraction. This results in longer delays between libvirt related things I do. Today, I successfully took an exam and am relaxing with libvirt stuff right now. You probably want to post a rebased v2 before I review your phyp patches, don't you? Matthias

On 04/14/2011 09:51 AM, Matthias Bolte wrote:
src/phyp/phyp_driver.c | 1128 +++++++----------------------------------------- 1 files changed, 163 insertions(+), 965 deletions(-)
Serves me right for thinking Matthias' cleanups had already been applied. Now I get to rebase, with lots of conflicts :)
Sorry for that :(
No problem. The fault is mostly mine for seeing your series and its review, and assuming it was in without verifying it myself. :)
I'm currently quite busy with my studies and can only spent some time on weekends for libvirt development to get some distraction. This results in longer delays between libvirt related things I do. Today, I successfully took an exam and am relaxing with libvirt stuff right now.
You probably want to post a rebased v2 before I review your phyp patches, don't you?
Yep, and v2 will be larger because I've found more leaks in the meantime. Plus, your 2/5 missed cleaning up the error paths for all the new interface functions that I just pushed in between when you submitted your patches for review vs. actually pushing them, so I'm fixing those to be consistent, as well. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Matthias Bolte