External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-10-02 04:15:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

util: Alter return value of virReadFCHost and fix mem leak

Browse Source 
https://bugzilla.redhat.com/show_bug.cgi?id=1357416

Rather than return a 0 or -1 and the *result string, return just the result
string to the caller.  Alter all the callers to handle the different return.

As a side effect or result of this, it's much clearer that we cannot just
assign the returned string into the scsi_host wwnn, wwpn, and fabric_wwn
fields - rather we should fetch a temporary string, then as long as our
fetch was good, VIR_FREE what may have been there, and STEAL what we just got.
This fixes a memory leak in the virNodeDeviceCreateXML code path through
find_new_device and nodeDeviceLookupSCSIHostByWWN which will continually
call nodeDeviceSysfsGetSCSIHostCaps until the expected wwnn/wwpn is found
in the device object capabilities.

Signed-off-by: John Ferlan <jferlan@redhat.com>
This commit is contained in:
John Ferlan 2016-10-11 11:21:19 -04:00
parent 041dfc2b7d
commit f29b13f830
4 changed files with 50 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
src/node_device/node_device_linux_sysfs.c
View File

@ -44,8 +44,7 @@ VIR_LOG_INIT("node_device.node_device_linux_sysfs");
int
nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapDataPtr d)
{
char *max_vports = NULL;
char *vports = NULL;
char *tmp = NULL;
int ret = -1;
if (virReadSCSIUniqueId(NULL, d->scsi_host.host,
@ -59,64 +58,53 @@ nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapDataPtr d)
if (virIsCapableFCHost(NULL, d->scsi_host.host)) {
d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST;
if (virReadFCHost(NULL,
d->scsi_host.host,
"port_name",
&d->scsi_host.wwpn) < 0) {
if (!(tmp = virReadFCHost(NULL, d->scsi_host.host, "port_name"))) {
VIR_WARN("Failed to read WWPN for host%d", d->scsi_host.host);
goto cleanup;
}
VIR_FREE(d->scsi_host.wwpn);
VIR_STEAL_PTR(d->scsi_host.wwpn, tmp);
if (virReadFCHost(NULL,
d->scsi_host.host,
"node_name",
&d->scsi_host.wwnn) < 0) {
if (!(tmp = virReadFCHost(NULL, d->scsi_host.host, "node_name"))) {
VIR_WARN("Failed to read WWNN for host%d", d->scsi_host.host);
goto cleanup;
}
VIR_FREE(d->scsi_host.wwnn);
VIR_STEAL_PTR(d->scsi_host.wwnn, tmp);
if (virReadFCHost(NULL,
d->scsi_host.host,
"fabric_name",
&d->scsi_host.fabric_wwn) < 0) {
if (!(tmp = virReadFCHost(NULL, d->scsi_host.host, "fabric_name"))) {
VIR_WARN("Failed to read fabric WWN for host%d",
d->scsi_host.host);
goto cleanup;
}
VIR_FREE(d->scsi_host.fabric_wwn);
VIR_STEAL_PTR(d->scsi_host.fabric_wwn, tmp);
}
if (virIsCapableVport(NULL, d->scsi_host.host)) {
d->scsi_host.flags |= VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS;
if (virReadFCHost(NULL,
d->scsi_host.host,
"max_npiv_vports",
&max_vports) < 0) {
if (!(tmp = virReadFCHost(NULL, d->scsi_host.host,
"max_npiv_vports"))) {
VIR_WARN("Failed to read max_npiv_vports for host%d",
d->scsi_host.host);
goto cleanup;
}
if (virReadFCHost(NULL,
d->scsi_host.host,
"npiv_vports_inuse",
&vports) < 0) {
if (virStrToLong_i(tmp, NULL, 10, &d->scsi_host.max_vports) < 0) {
VIR_WARN("Failed to parse value of max_npiv_vports '%s'", tmp);
goto cleanup;
}
if (!(tmp = virReadFCHost(NULL, d->scsi_host.host,
"npiv_vports_inuse"))) {
VIR_WARN("Failed to read npiv_vports_inuse for host%d",
d->scsi_host.host);
goto cleanup;
}
if (virStrToLong_i(max_vports, NULL, 10,
&d->scsi_host.max_vports) < 0) {
VIR_WARN("Failed to parse value of max_npiv_vports '%s'",
max_vports);
goto cleanup;
}
if (virStrToLong_i(vports, NULL, 10,
&d->scsi_host.vports) < 0) {
VIR_WARN("Failed to parse value of npiv_vports_inuse '%s'",
vports);
if (virStrToLong_i(tmp, NULL, 10, &d->scsi_host.vports) < 0) {
VIR_WARN("Failed to parse value of npiv_vports_inuse '%s'", tmp);
goto cleanup;
}
}
@ -132,8 +120,7 @@ nodeDeviceSysfsGetSCSIHostCaps(virNodeDevCapDataPtr d)
VIR_FREE(d->scsi_host.wwpn);
VIR_FREE(d->scsi_host.fabric_wwn);
}
VIR_FREE(max_vports);
VIR_FREE(vports);
VIR_FREE(tmp);
return ret;
}

34
src/util/virutil.c
View File

@ -2008,24 +2008,21 @@ virGetSCSIHostNameByParentaddr(unsigned int domain,
* @sysfs_prefix: "fc_host" sysfs path, defaults to SYSFS_FC_HOST_PATH
* @host: Host number, E.g. 5 of "fc_host/host5"
* @entry: Name of the sysfs entry to read
* @result: Return the entry value as string
*
* Read the value of sysfs "fc_host" entry.
*
* Returns 0 on success, and @result is filled with the entry value.
* as string, Otherwise returns -1. Caller must free @result after
* use.
* Returns result as a stringon success, caller must free @result after
* Otherwise returns NULL.
*/
int
char *
virReadFCHost(const char *sysfs_prefix,
int host,
const char *entry,
char **result)
const char *entry)
{
char *sysfs_path = NULL;
char *p = NULL;
int ret = -1;
char *buf = NULL;
char *result = NULL;
if (virAsprintf(&sysfs_path, "%s/host%d/%s",
sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH,
@ -2043,14 +2040,12 @@ virReadFCHost(const char *sysfs_prefix,
else
p = buf;
if (VIR_STRDUP(*result, p) < 0)
goto cleanup;
ignore_value(VIR_STRDUP(result, p));
ret = 0;
cleanup:
VIR_FREE(sysfs_path);
VIR_FREE(buf);
return ret;
return result;
}
bool
@ -2171,7 +2166,7 @@ virManageVport(const int parent_host,
return ret;
}
/* virGetHostNameByWWN:
/* virGetFCHostNameByWWN:
*
* Iterate over the sysfs tree to get FC host name (e.g. host5)
* by the provided "wwnn,wwpn" pair.
@ -2298,7 +2293,7 @@ virFindFCHostCapableVport(const char *sysfs_prefix)
if (!virIsCapableVport(prefix, host))
continue;
if (virReadFCHost(prefix, host, "port_state", &state) < 0) {
if (!(state = virReadFCHost(prefix, host, "port_state"))) {
VIR_DEBUG("Failed to read port_state for host%d", host);
continue;
}
@ -2310,12 +2305,12 @@ virFindFCHostCapableVport(const char *sysfs_prefix)
}
VIR_FREE(state);
if (virReadFCHost(prefix, host, "max_npiv_vports", &max_vports) < 0) {
if (!(max_vports = virReadFCHost(prefix, host, "max_npiv_vports"))) {
VIR_DEBUG("Failed to read max_npiv_vports for host%d", host);
continue;
}
if (virReadFCHost(prefix, host, "npiv_vports_inuse", &vports) < 0) {
if (!(vports = virReadFCHost(prefix, host, "npiv_vports_inuse"))) {
VIR_DEBUG("Failed to read npiv_vports_inuse for host%d", host);
VIR_FREE(max_vports);
continue;
@ -2379,14 +2374,13 @@ virGetSCSIHostNameByParentaddr(unsigned int domain ATTRIBUTE_UNUSED,
return NULL;
}
int
char *
virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED,
int host ATTRIBUTE_UNUSED,
const char *entry ATTRIBUTE_UNUSED,
char **result ATTRIBUTE_UNUSED)
const char *entry ATTRIBUTE_UNUSED)
{
virReportSystemError(ENOSYS, "%s", _("Not supported on this platform"));
return -1;
return NULL;
}
bool

7
src/util/virutil.h
View File

@ -182,11 +182,10 @@ virGetSCSIHostNameByParentaddr(unsigned int domain,
unsigned int slot,
unsigned int function,
unsigned int unique_id);
int virReadFCHost(const char *sysfs_prefix,
char *virReadFCHost(const char *sysfs_prefix,
int host,
const char *entry,
char **result)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
const char *entry)
ATTRIBUTE_NONNULL(3);
bool virIsCapableFCHost(const char *sysfs_prefix, int host);
bool virIsCapableVport(const char *sysfs_prefix, int host);

30
tests/fchosttest.c
View File

@ -68,35 +68,25 @@ test3(const void *data ATTRIBUTE_UNUSED)
char *vports = NULL;
int ret = -1;
if (virReadFCHost(TEST_FC_HOST_PREFIX,
TEST_FC_HOST_NUM,
"node_name",
&wwnn) < 0)
if (!(wwnn = virReadFCHost(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM,
"node_name")))
return -1;
if (virReadFCHost(TEST_FC_HOST_PREFIX,
TEST_FC_HOST_NUM,
"port_name",
&wwpn) < 0)
if (!(wwpn = virReadFCHost(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM,
"port_name")))
goto cleanup;
if (virReadFCHost(TEST_FC_HOST_PREFIX,
TEST_FC_HOST_NUM,
"fabric_name",
&fabric_wwn) < 0)
if (!(fabric_wwn = virReadFCHost(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM,
"fabric_name")))
goto cleanup;
if (virReadFCHost(TEST_FC_HOST_PREFIX,
TEST_FC_HOST_NUM,
"max_npiv_vports",
&max_vports) < 0)
if (!(max_vports = virReadFCHost(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM,
"max_npiv_vports")))
goto cleanup;
if (virReadFCHost(TEST_FC_HOST_PREFIX,
TEST_FC_HOST_NUM,
"npiv_vports_inuse",
&vports) < 0)
if (!(vports = virReadFCHost(TEST_FC_HOST_PREFIX, TEST_FC_HOST_NUM,
"npiv_vports_inuse")))
goto cleanup;
if (STRNEQ(expect_wwnn, wwnn) ||