NuKVM/libvirt
3
0
Fork 0
mirror of https://github.com/libvirt/libvirt.git synced 2025-02-25 18:55:26 -06:00
Code Issues Packages Projects Releases Wiki Activity

Revert "util: cgroup: use VIR_AUTOPTR for aggregate types"

Browse Source 
This reverts commit dd47145aaa.

Turns out, our code relies on virCgroupFree(&var) setting
var = NULL.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
This commit is contained in:
Michal Privoznik 2018-07-30 11:03:16 +02:00
parent 7f6d6b965a
commit 81acbc4cb1
1 changed files with 127 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

200
src/util/vircgroup.c
View File

@ -836,21 +836,25 @@ virCgroupGetValueForBlkDev(virCgroupPtr group,
{ {
VIR_AUTOFREE(char *) prefix = NULL; VIR_AUTOFREE(char *) prefix = NULL;
VIR_AUTOFREE(char *) str = NULL; VIR_AUTOFREE(char *) str = NULL;
VIR_AUTOPTR(virString) lines = NULL; char **lines = NULL;
int ret = -1;
if (virCgroupGetValueStr(group, controller, key, &str) < 0) if (virCgroupGetValueStr(group, controller, key, &str) < 0)
return -1; goto error;
if (!(prefix = virCgroupGetBlockDevString(path))) if (!(prefix = virCgroupGetBlockDevString(path)))
return -1; goto error;
if (!(lines = virStringSplit(str, "\n", -1))) if (!(lines = virStringSplit(str, "\n", -1)))
return -1; goto error;
if (VIR_STRDUP(*value, virStringListGetFirstWithPrefix(lines, prefix)) < 0) if (VIR_STRDUP(*value, virStringListGetFirstWithPrefix(lines, prefix)) < 0)
return -1; goto error;
return 0; ret = 0;
error:
virStringListFree(lines);
return ret;
} }
@ -1213,11 +1217,12 @@ virCgroupAddTaskController(virCgroupPtr group, pid_t pid, int controller)
static int static int
virCgroupSetPartitionSuffix(const char *path, char **res) virCgroupSetPartitionSuffix(const char *path, char **res)
{ {
VIR_AUTOPTR(virString) tokens = NULL; char **tokens;
size_t i; size_t i;
int ret = -1;
if (!(tokens = virStringSplit(path, "/", 0))) if (!(tokens = virStringSplit(path, "/", 0)))
return -1; return ret;
for (i = 0; tokens[i] != NULL; i++) { for (i = 0; tokens[i] != NULL; i++) {
/* Whitelist the 3 top level fixed dirs /* Whitelist the 3 top level fixed dirs
@ -1236,18 +1241,22 @@ virCgroupSetPartitionSuffix(const char *path, char **res)
!strchr(tokens[i], '.')) { !strchr(tokens[i], '.')) {
if (VIR_REALLOC_N(tokens[i], if (VIR_REALLOC_N(tokens[i],
strlen(tokens[i]) + strlen(".partition") + 1) < 0) strlen(tokens[i]) + strlen(".partition") + 1) < 0)
return -1; goto cleanup;
strcat(tokens[i], ".partition"); strcat(tokens[i], ".partition");
} }
if (virCgroupPartitionEscape(&(tokens[i])) < 0) if (virCgroupPartitionEscape(&(tokens[i])) < 0)
return -1; goto cleanup;
} }
if (!(*res = virStringListJoin((const char **)tokens, "/"))) if (!(*res = virStringListJoin((const char **)tokens, "/")))
return -1; goto cleanup;
return 0; ret = 0;
cleanup:
virStringListFree(tokens);
return ret;
} }
@ -1268,10 +1277,10 @@ virCgroupNewPartition(const char *path,
int controllers, int controllers,
virCgroupPtr *group) virCgroupPtr *group)
{ {
int ret = -1;
VIR_AUTOFREE(char *) parentPath = NULL; VIR_AUTOFREE(char *) parentPath = NULL;
VIR_AUTOFREE(char *) newPath = NULL; VIR_AUTOFREE(char *) newPath = NULL;
VIR_AUTOPTR(virCgroup) parent = NULL; virCgroupPtr parent = NULL;
VIR_AUTOPTR(virCgroup) tmpGroup = NULL;
VIR_DEBUG("path=%s create=%d controllers=%x", VIR_DEBUG("path=%s create=%d controllers=%x",
path, create, controllers); path, create, controllers);
@ -1283,31 +1292,35 @@ virCgroupNewPartition(const char *path,
} }
if (virCgroupSetPartitionSuffix(path, &newPath) < 0) if (virCgroupSetPartitionSuffix(path, &newPath) < 0)
return -1; goto cleanup;
if (virCgroupNew(-1, newPath, NULL, controllers, &tmpGroup) < 0) if (virCgroupNew(-1, newPath, NULL, controllers, group) < 0)
return -1; goto cleanup;
if (STRNEQ(newPath, "/")) { if (STRNEQ(newPath, "/")) {
char *tmp; char *tmp;
if (VIR_STRDUP(parentPath, newPath) < 0) if (VIR_STRDUP(parentPath, newPath) < 0)
return -1; goto cleanup;
tmp = strrchr(parentPath, '/'); tmp = strrchr(parentPath, '/');
tmp++; tmp++;
*tmp = '\0'; *tmp = '\0';
if (virCgroupNew(-1, parentPath, NULL, controllers, &parent) < 0) if (virCgroupNew(-1, parentPath, NULL, controllers, &parent) < 0)
return -1; goto cleanup;
if (virCgroupMakeGroup(parent, tmpGroup, create, VIR_CGROUP_NONE) < 0) { if (virCgroupMakeGroup(parent, *group, create, VIR_CGROUP_NONE) < 0) {
virCgroupRemove(tmpGroup); virCgroupRemove(*group);
return -1; goto cleanup;
} }
} }
VIR_STEAL_PTR(*group, tmpGroup); ret = 0;
return 0; cleanup:
if (ret != 0)
virCgroupFree(*group);
virCgroupFree(parent);
return ret;
} }
@ -1489,9 +1502,9 @@ virCgroupNewMachineSystemd(const char *name,
int controllers, int controllers,
virCgroupPtr *group) virCgroupPtr *group)
{ {
int ret = -1;
int rv; int rv;
VIR_AUTOPTR(virCgroup) init = NULL; virCgroupPtr init, parent = NULL;
VIR_AUTOPTR(virCgroup) parent = NULL;
VIR_AUTOFREE(char *) path = NULL; VIR_AUTOFREE(char *) path = NULL;
char *offset; char *offset;
@ -1518,10 +1531,12 @@ virCgroupNewMachineSystemd(const char *name,
path = init->controllers[VIR_CGROUP_CONTROLLER_SYSTEMD].placement; path = init->controllers[VIR_CGROUP_CONTROLLER_SYSTEMD].placement;
init->controllers[VIR_CGROUP_CONTROLLER_SYSTEMD].placement = NULL; init->controllers[VIR_CGROUP_CONTROLLER_SYSTEMD].placement = NULL;
virCgroupFree(init);
if (!path || STREQ(path, "/") || path[0] != '/') { if (!path || STREQ(path, "/") || path[0] != '/') {
VIR_DEBUG("Systemd didn't setup its controller"); VIR_DEBUG("Systemd didn't setup its controller");
return -2; ret = -2;
goto cleanup;
} }
offset = path; offset = path;
@ -1531,7 +1546,7 @@ virCgroupNewMachineSystemd(const char *name,
NULL, NULL,
controllers, controllers,
&parent) < 0) &parent) < 0)
return -1; goto cleanup;
for (;;) { for (;;) {
@ -1545,11 +1560,11 @@ virCgroupNewMachineSystemd(const char *name,
parent, parent,
controllers, controllers,
&tmp) < 0) &tmp) < 0)
return -1; goto cleanup;
if (virCgroupMakeGroup(parent, tmp, true, VIR_CGROUP_NONE) < 0) { if (virCgroupMakeGroup(parent, tmp, true, VIR_CGROUP_NONE) < 0) {
virCgroupFree(tmp); virCgroupFree(tmp);
return -1; goto cleanup;
} }
if (t) { if (t) {
*t = '/'; *t = '/';
@ -1572,7 +1587,10 @@ virCgroupNewMachineSystemd(const char *name,
} }
} }
return 0; ret = 0;
cleanup:
virCgroupFree(parent);
return ret;
} }
@ -1593,7 +1611,8 @@ virCgroupNewMachineManual(const char *name,
int controllers, int controllers,
virCgroupPtr *group) virCgroupPtr *group)
{ {
VIR_AUTOPTR(virCgroup) parent = NULL; virCgroupPtr parent = NULL;
int ret = -1;
VIR_DEBUG("Fallback to non-systemd setup"); VIR_DEBUG("Fallback to non-systemd setup");
if (virCgroupNewPartition(partition, if (virCgroupNewPartition(partition,
@ -1601,9 +1620,9 @@ virCgroupNewMachineManual(const char *name,
controllers, controllers,
&parent) < 0) { &parent) < 0) {
if (virCgroupNewIgnoreError()) if (virCgroupNewIgnoreError())
return 0; goto done;
return -1; goto cleanup;
} }
if (virCgroupNewDomainPartition(parent, if (virCgroupNewDomainPartition(parent,
@ -1611,7 +1630,7 @@ virCgroupNewMachineManual(const char *name,
name, name,
true, true,
group) < 0) group) < 0)
return -1; goto cleanup;
if (virCgroupAddTask(*group, pidleader) < 0) { if (virCgroupAddTask(*group, pidleader) < 0) {
virErrorPtr saved = virSaveLastError(); virErrorPtr saved = virSaveLastError();
@ -1623,7 +1642,12 @@ virCgroupNewMachineManual(const char *name,
} }
} }
return 0; done:
ret = 0;
cleanup:
virCgroupFree(parent);
return ret;
} }
@ -2352,7 +2376,7 @@ static virOnceControl virCgroupMemoryOnce = VIR_ONCE_CONTROL_INITIALIZER;
static void static void
virCgroupMemoryOnceInit(void) virCgroupMemoryOnceInit(void)
{ {
VIR_AUTOPTR(virCgroup) group = NULL; virCgroupPtr group;
unsigned long long int mem_unlimited = 0ULL; unsigned long long int mem_unlimited = 0ULL;
if (virCgroupNew(-1, "/", NULL, -1, &group) < 0) if (virCgroupNew(-1, "/", NULL, -1, &group) < 0)
@ -2366,6 +2390,7 @@ virCgroupMemoryOnceInit(void)
"memory.limit_in_bytes", "memory.limit_in_bytes",
&mem_unlimited)); &mem_unlimited));
cleanup: cleanup:
virCgroupFree(group);
virCgroupMemoryUnlimitedKB = mem_unlimited >> 10; virCgroupMemoryUnlimitedKB = mem_unlimited >> 10;
} }
@ -2966,21 +2991,22 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group,
size_t nsum, size_t nsum,
virBitmapPtr cpumap) virBitmapPtr cpumap)
{ {
int ret = -1;
ssize_t i = -1; ssize_t i = -1;
virCgroupPtr group_vcpu = NULL;
while ((i = virBitmapNextSetBit(guestvcpus, i)) >= 0) { while ((i = virBitmapNextSetBit(guestvcpus, i)) >= 0) {
VIR_AUTOFREE(char *) buf = NULL; VIR_AUTOFREE(char *) buf = NULL;
VIR_AUTOPTR(virCgroup) group_vcpu = NULL;
char *pos; char *pos;
unsigned long long tmp; unsigned long long tmp;
ssize_t j; ssize_t j;
if (virCgroupNewThread(group, VIR_CGROUP_THREAD_VCPU, i, if (virCgroupNewThread(group, VIR_CGROUP_THREAD_VCPU, i,
false, &group_vcpu) < 0) false, &group_vcpu) < 0)
return -1; goto cleanup;
if (virCgroupGetCpuacctPercpuUsage(group_vcpu, &buf) < 0) if (virCgroupGetCpuacctPercpuUsage(group_vcpu, &buf) < 0)
return -1; goto cleanup;
pos = buf; pos = buf;
for (j = virBitmapNextSetBit(cpumap, -1); for (j = virBitmapNextSetBit(cpumap, -1);
@ -2989,13 +3015,18 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group,
if (virStrToLong_ull(pos, &pos, 10, &tmp) < 0) { if (virStrToLong_ull(pos, &pos, 10, &tmp) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("cpuacct parse error")); _("cpuacct parse error"));
return -1; goto cleanup;
} }
sum_cpu_time[j] += tmp; sum_cpu_time[j] += tmp;
} }
virCgroupFree(group_vcpu);
} }
return 0; ret = 0;
cleanup:
virCgroupFree(group_vcpu);
return ret;
} }
@ -3027,6 +3058,7 @@ virCgroupGetPercpuStats(virCgroupPtr group,
unsigned int ncpus, unsigned int ncpus,
virBitmapPtr guestvcpus) virBitmapPtr guestvcpus)
{ {
int ret = -1;
size_t i; size_t i;
int need_cpus, total_cpus; int need_cpus, total_cpus;
char *pos; char *pos;
@ -3035,7 +3067,7 @@ virCgroupGetPercpuStats(virCgroupPtr group,
virTypedParameterPtr ent; virTypedParameterPtr ent;
int param_idx; int param_idx;
unsigned long long cpu_time; unsigned long long cpu_time;
VIR_AUTOPTR(virBitmap) cpumap = NULL; virBitmapPtr cpumap = NULL;
/* return the number of supported params */ /* return the number of supported params */
if (nparams == 0 && ncpus != 0) { if (nparams == 0 && ncpus != 0) {
@ -3052,19 +3084,21 @@ virCgroupGetPercpuStats(virCgroupPtr group,
total_cpus = virBitmapSize(cpumap); total_cpus = virBitmapSize(cpumap);
/* return total number of cpus */ /* return total number of cpus */
if (ncpus == 0) if (ncpus == 0) {
return total_cpus; ret = total_cpus;
goto cleanup;
}
if (start_cpu >= total_cpus) { if (start_cpu >= total_cpus) {
virReportError(VIR_ERR_INVALID_ARG, virReportError(VIR_ERR_INVALID_ARG,
_("start_cpu %d larger than maximum of %d"), _("start_cpu %d larger than maximum of %d"),
start_cpu, total_cpus - 1); start_cpu, total_cpus - 1);
return -1; goto cleanup;
} }
/* we get percpu cputime accounting info. */ /* we get percpu cputime accounting info. */
if (virCgroupGetCpuacctPercpuUsage(group, &buf)) if (virCgroupGetCpuacctPercpuUsage(group, &buf))
return -1; goto cleanup;
pos = buf; pos = buf;
/* return percpu cputime in index 0 */ /* return percpu cputime in index 0 */
@ -3079,14 +3113,14 @@ virCgroupGetPercpuStats(virCgroupPtr group,
} else if (virStrToLong_ull(pos, &pos, 10, &cpu_time) < 0) { } else if (virStrToLong_ull(pos, &pos, 10, &cpu_time) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("cpuacct parse error")); _("cpuacct parse error"));
return -1; goto cleanup;
} }
if (i < start_cpu) if (i < start_cpu)
continue; continue;
ent = &params[(i - start_cpu) * nparams + param_idx]; ent = &params[(i - start_cpu) * nparams + param_idx];
if (virTypedParameterAssign(ent, VIR_DOMAIN_CPU_STATS_CPUTIME, if (virTypedParameterAssign(ent, VIR_DOMAIN_CPU_STATS_CPUTIME,
VIR_TYPED_PARAM_ULLONG, cpu_time) < 0) VIR_TYPED_PARAM_ULLONG, cpu_time) < 0)
return -1; goto cleanup;
} }
/* return percpu vcputime in index 1 */ /* return percpu vcputime in index 1 */
@ -3094,10 +3128,10 @@ virCgroupGetPercpuStats(virCgroupPtr group,
if (guestvcpus && param_idx < nparams) { if (guestvcpus && param_idx < nparams) {
if (VIR_ALLOC_N(sum_cpu_time, need_cpus) < 0) if (VIR_ALLOC_N(sum_cpu_time, need_cpus) < 0)
return -1; goto cleanup;
if (virCgroupGetPercpuVcpuSum(group, guestvcpus, sum_cpu_time, if (virCgroupGetPercpuVcpuSum(group, guestvcpus, sum_cpu_time,
need_cpus, cpumap) < 0) need_cpus, cpumap) < 0)
return -1; goto cleanup;
for (i = start_cpu; i < need_cpus; i++) { for (i = start_cpu; i < need_cpus; i++) {
if (virTypedParameterAssign(&params[(i - start_cpu) * nparams + if (virTypedParameterAssign(&params[(i - start_cpu) * nparams +
@ -3105,13 +3139,17 @@ virCgroupGetPercpuStats(virCgroupPtr group,
VIR_DOMAIN_CPU_STATS_VCPUTIME, VIR_DOMAIN_CPU_STATS_VCPUTIME,
VIR_TYPED_PARAM_ULLONG, VIR_TYPED_PARAM_ULLONG,
sum_cpu_time[i]) < 0) sum_cpu_time[i]) < 0)
return -1; goto cleanup;
} }
param_idx++; param_idx++;
} }
return param_idx; ret = param_idx;
cleanup:
virBitmapFree(cpumap);
return ret;
} }
@ -3467,18 +3505,23 @@ int
virCgroupKill(virCgroupPtr group, int signum) virCgroupKill(virCgroupPtr group, int signum)
{ {
VIR_DEBUG("group=%p path=%s signum=%d", group, group->path, signum); VIR_DEBUG("group=%p path=%s signum=%d", group, group->path, signum);
int ret;
/* The 'tasks' file in cgroups can contain duplicated /* The 'tasks' file in cgroups can contain duplicated
* pids, so we use a hash to track which we've already * pids, so we use a hash to track which we've already
* killed. * killed.
*/ */
VIR_AUTOPTR(virHashTable) pids = virHashCreateFull(100, virHashTablePtr pids = virHashCreateFull(100,
NULL, NULL,
virCgroupPidCode, virCgroupPidCode,
virCgroupPidEqual, virCgroupPidEqual,
virCgroupPidCopy, virCgroupPidCopy,
NULL); NULL);
return virCgroupKillInternal(group, signum, pids); ret = virCgroupKillInternal(group, signum, pids);
virHashFree(pids);
return ret;
} }
@ -3493,6 +3536,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group,
bool killedAny = false; bool killedAny = false;
VIR_AUTOFREE(char *) keypath = NULL; VIR_AUTOFREE(char *) keypath = NULL;
DIR *dp = NULL; DIR *dp = NULL;
virCgroupPtr subgroup = NULL;
struct dirent *ent; struct dirent *ent;
int direrr; int direrr;
VIR_DEBUG("group=%p path=%s signum=%d pids=%p", VIR_DEBUG("group=%p path=%s signum=%d pids=%p",
@ -3517,8 +3561,6 @@ virCgroupKillRecursiveInternal(virCgroupPtr group,
} }
while ((direrr = virDirRead(dp, &ent, keypath)) > 0) { while ((direrr = virDirRead(dp, &ent, keypath)) > 0) {
VIR_AUTOPTR(virCgroup) subgroup = NULL;
if (ent->d_type != DT_DIR) if (ent->d_type != DT_DIR)
continue; continue;
@ -3535,6 +3577,8 @@ virCgroupKillRecursiveInternal(virCgroupPtr group,
if (dormdir) if (dormdir)
virCgroupRemove(subgroup); virCgroupRemove(subgroup);
virCgroupFree(subgroup);
} }
if (direrr < 0) if (direrr < 0)
goto cleanup; goto cleanup;
@ -3543,6 +3587,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group,
ret = killedAny ? 1 : 0; ret = killedAny ? 1 : 0;
cleanup: cleanup:
virCgroupFree(subgroup);
VIR_DIR_CLOSE(dp); VIR_DIR_CLOSE(dp);
return ret; return ret;
} }
@ -3551,15 +3596,20 @@ virCgroupKillRecursiveInternal(virCgroupPtr group,
int int
virCgroupKillRecursive(virCgroupPtr group, int signum) virCgroupKillRecursive(virCgroupPtr group, int signum)
{ {
int ret;
VIR_DEBUG("group=%p path=%s signum=%d", group, group->path, signum); VIR_DEBUG("group=%p path=%s signum=%d", group, group->path, signum);
VIR_AUTOPTR(virHashTable) pids = virHashCreateFull(100, virHashTablePtr pids = virHashCreateFull(100,
NULL, NULL,
virCgroupPidCode, virCgroupPidCode,
virCgroupPidEqual, virCgroupPidEqual,
virCgroupPidCopy, virCgroupPidCopy,
NULL); NULL);
return virCgroupKillRecursiveInternal(group, signum, pids, false); ret = virCgroupKillRecursiveInternal(group, signum, pids, false);
virHashFree(pids);
return ret;
} }
@ -3894,12 +3944,15 @@ virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller)
bool bool
virCgroupControllerAvailable(int controller) virCgroupControllerAvailable(int controller)
{ {
VIR_AUTOPTR(virCgroup) cgroup = NULL; virCgroupPtr cgroup;
bool ret = false;
if (virCgroupNewSelf(&cgroup) < 0) if (virCgroupNewSelf(&cgroup) < 0)
return false; return ret;
return virCgroupHasController(cgroup, controller); ret = virCgroupHasController(cgroup, controller);
virCgroupFree(cgroup);
return ret;
} }
#else /* !VIR_CGROUP_SUPPORTED */ #else /* !VIR_CGROUP_SUPPORTED */
@ -4687,7 +4740,7 @@ virCgroupDelThread(virCgroupPtr cgroup,
virCgroupThreadName nameval, virCgroupThreadName nameval,
int idx) int idx)
{ {
VIR_AUTOPTR(virCgroup) new_cgroup = NULL; virCgroupPtr new_cgroup = NULL;
if (cgroup) { if (cgroup) {
if (virCgroupNewThread(cgroup, nameval, idx, false, &new_cgroup) < 0) if (virCgroupNewThread(cgroup, nameval, idx, false, &new_cgroup) < 0)
@ -4695,6 +4748,7 @@ virCgroupDelThread(virCgroupPtr cgroup,
/* Remove the offlined cgroup */ /* Remove the offlined cgroup */
virCgroupRemove(new_cgroup); virCgroupRemove(new_cgroup);
virCgroupFree(new_cgroup);
} }
return 0; return 0;