From 88f6c007c3fb4324396ec397de57c8a80ba7b31d Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Thu, 16 Jul 2015 15:35:05 +0200 Subject: [PATCH] cgroup: Drop resource partition from virSystemdMakeScopeName The scope name, even according to our docs is "machine-$DRIVER\x2d$VMNAME.scope" virSystemdMakeScopeName would use the resource partition name instead of "machine-" if it was specified thus creating invalid scope paths. This makes libvirt drop cgroups for a VM that uses custom resource partition upon reconnecting since the detected scope name would not match the expected name generated by virSystemdMakeScopeName. The error is exposed by the following log entry: debug : virCgroupValidateMachineGroup:302 : Name 'machine-qemu\x2dtestvm.scope' for controller 'cpu' does not match 'testvm', 'testvm.libvirt-qemu' or 'machine-test-qemu\x2dtestvm.scope' for a "/machine/test" resource and "testvm" vm. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1238570 --- src/lxc/lxc_process.c | 6 ------ src/qemu/qemu_cgroup.c | 3 --- src/util/vircgroup.c | 11 ++--------- src/util/vircgroup.h | 1 - src/util/virsystemd.c | 9 ++------- src/util/virsystemd.h | 3 +-- tests/virsystemdtest.c | 20 +++++++------------- 7 files changed, 12 insertions(+), 41 deletions(-) diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 2bdce3b0bc..87ee484364 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1319,9 +1319,6 @@ int virLXCProcessStart(virConnectPtr conn, * more reliable way to kill everything off if something * goes wrong from here onwards ... */ if (virCgroupNewDetectMachine(vm->def->name, "lxc", vm->pid, - vm->def->resource ? - vm->def->resource->partition : - NULL, -1, &priv->cgroup) < 0) goto cleanup; @@ -1505,9 +1502,6 @@ virLXCProcessReconnectDomain(virDomainObjPtr vm, goto error; if (virCgroupNewDetectMachine(vm->def->name, "lxc", vm->pid, - vm->def->resource ? - vm->def->resource->partition : - NULL, -1, &priv->cgroup) < 0) goto error; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 8ed74eeef0..ab21e12709 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -855,9 +855,6 @@ qemuConnectCgroup(virQEMUDriverPtr driver, if (virCgroupNewDetectMachine(vm->def->name, "qemu", vm->pid, - vm->def->resource ? - vm->def->resource->partition : - NULL, cfg->cgroupControllers, &priv->cgroup) < 0) goto cleanup; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 0ef2d29626..0599ba5878 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -243,7 +243,6 @@ static bool virCgroupValidateMachineGroup(virCgroupPtr group, const char *name, const char *drivername, - const char *partition, bool stripEmulatorSuffix) { size_t i; @@ -258,10 +257,7 @@ virCgroupValidateMachineGroup(virCgroupPtr group, if (virCgroupPartitionEscape(&partname) < 0) goto cleanup; - if (!partition) - partition = "/machine"; - - if (!(scopename = virSystemdMakeScopeName(name, drivername, partition))) + if (!(scopename = virSystemdMakeScopeName(name, drivername))) goto cleanup; if (virCgroupPartitionEscape(&scopename) < 0) @@ -1498,7 +1494,6 @@ int virCgroupNewDetectMachine(const char *name, const char *drivername, pid_t pid, - const char *partition, int controllers, virCgroupPtr *group) { @@ -1508,8 +1503,7 @@ virCgroupNewDetectMachine(const char *name, return -1; } - if (!virCgroupValidateMachineGroup(*group, name, drivername, partition, - true)) { + if (!virCgroupValidateMachineGroup(*group, name, drivername, true)) { VIR_DEBUG("Failed to validate machine name for '%s' driver '%s'", name, drivername); virCgroupFree(group); @@ -4047,7 +4041,6 @@ int virCgroupNewDetectMachine(const char *name ATTRIBUTE_UNUSED, const char *drivername ATTRIBUTE_UNUSED, pid_t pid ATTRIBUTE_UNUSED, - const char *partition ATTRIBUTE_UNUSED, int controllers ATTRIBUTE_UNUSED, virCgroupPtr *group ATTRIBUTE_UNUSED) { diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index e75c5228e9..675a1851a1 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -92,7 +92,6 @@ int virCgroupNewDetect(pid_t pid, int virCgroupNewDetectMachine(const char *name, const char *drivername, pid_t pid, - const char *partition, int controllers, virCgroupPtr *group); diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 8cedf8db75..54c409d7f6 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -80,16 +80,11 @@ static void virSystemdEscapeName(virBufferPtr buf, char *virSystemdMakeScopeName(const char *name, - const char *drivername, - const char *partition) + const char *drivername) { virBuffer buf = VIR_BUFFER_INITIALIZER; - if (*partition == '/') - partition++; - - virSystemdEscapeName(&buf, partition); - virBufferAddChar(&buf, '-'); + virBufferAddLit(&buf, "machine-"); virSystemdEscapeName(&buf, drivername); virBufferAddLit(&buf, "\\x2d"); virSystemdEscapeName(&buf, name); diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h index 7a29dbafff..8af2169101 100644 --- a/src/util/virsystemd.h +++ b/src/util/virsystemd.h @@ -25,8 +25,7 @@ # include "internal.h" char *virSystemdMakeScopeName(const char *name, - const char *drivername, - const char *slicename); + const char *drivername); char *virSystemdMakeSliceName(const char *partition); char *virSystemdMakeMachineName(const char *name, diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c index 261c4cc345..d0b9335b24 100644 --- a/tests/virsystemdtest.c +++ b/tests/virsystemdtest.c @@ -340,7 +340,6 @@ static int testCreateNetwork(const void *opaque ATTRIBUTE_UNUSED) struct testScopeData { const char *name; - const char *partition; const char *expected; }; @@ -351,9 +350,7 @@ testScopeName(const void *opaque) int ret = -1; char *actual = NULL; - if (!(actual = virSystemdMakeScopeName(data->name, - "lxc", - data->partition))) + if (!(actual = virSystemdMakeScopeName(data->name, "lxc"))) goto cleanup; if (STRNEQ(actual, data->expected)) { @@ -472,22 +469,19 @@ mymain(void) if (virtTestRun("Test create with network ", testCreateNetwork, NULL) < 0) ret = -1; -# define TEST_SCOPE(name, partition, unitname) \ +# define TEST_SCOPE(name, unitname) \ do { \ struct testScopeData data = { \ - name, partition, unitname \ + name, unitname \ }; \ if (virtTestRun("Test scopename", testScopeName, &data) < 0) \ ret = -1; \ } while (0) - TEST_SCOPE("demo", "/machine", "machine-lxc\\x2ddemo.scope"); - TEST_SCOPE("demo-name", "/machine", "machine-lxc\\x2ddemo\\x2dname.scope"); - TEST_SCOPE("demo!name", "/machine", "machine-lxc\\x2ddemo\\x21name.scope"); - TEST_SCOPE(".demo", "/machine", "machine-lxc\\x2d\\x2edemo.scope"); - TEST_SCOPE("demo", "/machine/eng-dept", "machine-eng\\x2ddept-lxc\\x2ddemo.scope"); - TEST_SCOPE("demo", "/machine/eng-dept/testing!stuff", - "machine-eng\\x2ddept-testing\\x21stuff-lxc\\x2ddemo.scope"); + TEST_SCOPE("demo", "machine-lxc\\x2ddemo.scope"); + TEST_SCOPE("demo-name", "machine-lxc\\x2ddemo\\x2dname.scope"); + TEST_SCOPE("demo!name", "machine-lxc\\x2ddemo\\x21name.scope"); + TEST_SCOPE(".demo", "machine-lxc\\x2d\\x2edemo.scope"); # define TESTS_PM_SUPPORT_HELPER(name, function) \ do { \