10:48 a.m.
From: "Daniel P. Berrange" <berrange(a)redhat.com>
This is a much changed / expanded version of my previous work to
create cgroups via systemd. The difference is that this time it
actually works :-)
I'm not proposing this for merge until after the 1.1.1 release.
Daniel P. Berrange (4):
Add APIs for formatting systemd slice/scope names
Add support for systemd cgroup mount
Cope with races while killing processes
Enable support for systemd-machined in cgroups creation
src/libvirt_private.syms | 2 +
src/lxc/lxc_process.c | 10 +-
src/qemu/qemu_cgroup.c | 1 +
src/util/vircgroup.c | 270 +++++++++++++++++++++++++++++++++++++++++------
src/util/vircgroup.h | 2 +
src/util/virsystemd.c | 96 ++++++++++++++++-
src/util/virsystemd.h | 5 +
tests/vircgrouptest.c | 9 ++
tests/virsystemdtest.c | 48 +++++++++
9 files changed, 403 insertions(+), 40 deletions(-)
--
1.8.1.4
10:48 a.m.
New subject: [libvirt] [PATCH 1/4] Add APIs for formatting systemd slice/scope names
From: "Daniel P. Berrange" <berrange(a)redhat.com>
There are some interesting escaping rules to consider when dealing
with systemd slice/scope names. Thus it is helpful to have APIs
for formatting names
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/libvirt_private.syms | 2 ++
src/util/virsystemd.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++--
src/util/virsystemd.h | 5 +++
tests/virsystemdtest.c | 48 +++++++++++++++++++++++++
4 files changed, 144 insertions(+), 2 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d9615ea..0247a46 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1935,6 +1935,8 @@ virSysinfoSetup;
# util/virsystemd.h
virSystemdCreateMachine;
+virSystemdMakeScopeName;
+virSystemdMakeSliceName;
# util/virthread.h
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 11d1153..251b846 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -27,9 +27,96 @@
#include "viralloc.h"
#include "virutil.h"
#include "virlog.h"
+#include "virerror.h"
#define VIR_FROM_THIS VIR_FROM_SYSTEMD
+
+static void virSystemdEscapeName(virBufferPtr buf,
+ const char *name)
+{
+ static const char hextable[16] = "0123456789abcdef";
+
+#define ESCAPE(c) \
+ do { \
+ virBufferAddChar(buf, '\\'); \
+ virBufferAddChar(buf, 'x'); \
+ virBufferAddChar(buf, hextable[(c >> 4) & 15]); \
+ virBufferAddChar(buf, hextable[c & 15]); \
+ } while (0)
+
+#define VALID_CHARS \
+ "0123456789" \
+ "abcdefghijklmnopqrstuvwxyz" \
+ "ABCDEFGHIJKLMNOPQRSTUVWXYZ" \
+ ":-_.\\"
+
+ if (*name == '.') {
+ ESCAPE(*name);
+ name++;
+ }
+
+ while (*name) {
+ if (*name == '/')
+ virBufferAddChar(buf, '-');
+ else if (*name == '-' ||
+ *name == '\\' ||
+ !strchr(VALID_CHARS, *name))
+ ESCAPE(*name);
+ else
+ virBufferAddChar(buf, *name);
+ name++;
+ }
+
+#undef ESCAPE
+#undef VALID_CHARS
+}
+
+
+char *virSystemdMakeScopeName(const char *name,
+ const char *drivername,
+ const char *partition)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+ if (*partition == '/')
+ partition++;
+
+ virSystemdEscapeName(&buf, partition);
+ virBufferAddChar(&buf, '-');
+ virSystemdEscapeName(&buf, drivername);
+ virBufferAddLit(&buf, "\\x2d");
+ virSystemdEscapeName(&buf, name);
+ virBufferAddLit(&buf, ".scope");
+
+ if (virBufferError(&buf)) {
+ virReportOOMError();
+ return NULL;
+ }
+
+ return virBufferContentAndReset(&buf);
+}
+
+
+char *virSystemdMakeSliceName(const char *partition)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+ if (*partition == '/')
+ partition++;
+
+ virSystemdEscapeName(&buf, partition);
+ virBufferAddLit(&buf, ".slice");
+
+ if (virBufferError(&buf)) {
+ virReportOOMError();
+ return NULL;
+ }
+
+ return virBufferContentAndReset(&buf);
+}
+
+
/**
* virSystemdCreateMachine:
* @name: driver unique name of the machine
@@ -75,8 +162,8 @@ int virSystemdCreateMachine(const char *name,
goto cleanup;
if (partition) {
- if (virAsprintf(&slicename, "%s.slice", partition) < 0)
- goto cleanup;
+ if (!(slicename = virSystemdMakeSliceName(partition)))
+ goto cleanup;
} else {
if (VIR_STRDUP(slicename, "") < 0)
goto cleanup;
diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h
index 9ca4e0b..414ae5a 100644
--- a/src/util/virsystemd.h
+++ b/src/util/virsystemd.h
@@ -24,6 +24,11 @@
# include "internal.h"
+char *virSystemdMakeScopeName(const char *name,
+ const char *drivername,
+ const char *slicename);
+char *virSystemdMakeSliceName(const char *partition);
+
int virSystemdCreateMachine(const char *name,
const char *drivername,
bool privileged,
diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index bcf3ad3..784c45c 100644
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -138,6 +138,38 @@ static int testCreateBadSystemd(const void *opaque ATTRIBUTE_UNUSED)
return 0;
}
+
+struct testScopeData {
+ const char *name;
+ const char *partition;
+ const char *expected;
+};
+
+static int
+testScopeName(const void *opaque)
+{
+ const struct testScopeData *data = opaque;
+ int ret = -1;
+ char *actual = NULL;
+
+ if (!(actual = virSystemdMakeScopeName(data->name,
+ "lxc",
+ data->partition)))
+ goto cleanup;
+
+ if (STRNEQ(actual, data->expected)) {
+ fprintf(stderr, "Expected '%s' but got '%s'\n",
+ data->expected, actual);
+ goto cleanup;
+ }
+
+ ret = 0;
+
+ cleanup:
+ VIR_FREE(actual);
+ return ret;
+}
+
static int
mymain(void)
{
@@ -152,6 +184,22 @@ mymain(void)
if (virtTestRun("Test create bad systemd ", 1, testCreateBadSystemd, NULL)
< 0)
ret = -1;
+#define TEST_SCOPE(name, partition, unitname) \
+ do { \
+ struct testScopeData data = { \
+ name, partition, unitname \
+ }; \
+ if (virtTestRun("Test scopename", 1, 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");
+
return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
1.8.1.4
1:05 p.m.
New subject: [libvirt] [PATCH 1/4] Add APIs for formatting systemd slice/scope names
On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
There are some interesting escaping rules to consider when dealing
with systemd slice/scope names. Thus it is helpful to have APIs
for formatting names
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/libvirt_private.syms | 2 ++
src/util/virsystemd.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++--
src/util/virsystemd.h | 5 +++
tests/virsystemdtest.c | 48 +++++++++++++++++++++++++
4 files changed, 144 insertions(+), 2 deletions(-)
+
+#define VALID_CHARS \
+ "0123456789" \
+ "abcdefghijklmnopqrstuvwxyz" \
+ "ABCDEFGHIJKLMNOPQRSTUVWXYZ" \
+ ":-_.\\"
If you would remove - and \\ from this list...
+
+ if (*name == '.') {
+ ESCAPE(*name);
+ name++;
+ }
+
+ while (*name) {
+ if (*name == '/')
+ virBufferAddChar(buf, '-');
+ else if (*name == '-' ||
+ *name == '\\' ||
+ !strchr(VALID_CHARS, *name))
...then this could be simplified to just !strchr().
+
+
+char *virSystemdMakeScopeName(const char *name,
Is it worth using the style:
char *
virSystemdMakeScopeName(...
+ TEST_SCOPE("demo",
"/machine/eng-dept/testing!stuff",
"machine-eng\\x2ddept-testing\\x21stuff-lxc\\x2ddemo.scope");
Worth wrapping the long line.
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
1:47 p.m.
New subject: [libvirt] [PATCH 1/4] Add APIs for formatting systemd slice/scope names
On Tue, Jul 30, 2013 at 12:05:51PM -0600, Eric Blake wrote:
On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange(a)redhat.com>
>
> There are some interesting escaping rules to consider when dealing
> with systemd slice/scope names. Thus it is helpful to have APIs
> for formatting names
>
> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
> ---
> src/libvirt_private.syms | 2 ++
> src/util/virsystemd.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++--
> src/util/virsystemd.h | 5 +++
> tests/virsystemdtest.c | 48 +++++++++++++++++++++++++
> 4 files changed, 144 insertions(+), 2 deletions(-)
>
> +
> +#define VALID_CHARS \
> + "0123456789" \
> + "abcdefghijklmnopqrstuvwxyz" \
> + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" \
> + ":-_.\\"
If you would remove - and \\ from this list...
This is the set that systemd uses, and we need to match it
exactly, otherwise we won't detect the cgroups correctly.
> +
> + if (*name == '.') {
> + ESCAPE(*name);
> + name++;
> + }
> +
> + while (*name) {
> + if (*name == '/')
> + virBufferAddChar(buf, '-');
> + else if (*name == '-' ||
> + *name == '\\' ||
> + !strchr(VALID_CHARS, *name))
...then this could be simplified to just !strchr().
> +
> +
> +char *virSystemdMakeScopeName(const char *name,
Is it worth using the style:
char *
virSystemdMakeScopeName(...
> + TEST_SCOPE("demo", "/machine/eng-dept/testing!stuff",
"machine-eng\\x2ddept-testing\\x21stuff-lxc\\x2ddemo.scope");
Worth wrapping the long line.
ACK.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
1:56 p.m.
New subject: [libvirt] [PATCH 1/4] Add APIs for formatting systemd slice/scope names
On 07/30/2013 12:47 PM, Daniel P. Berrange wrote:
On Tue, Jul 30, 2013 at 12:05:51PM -0600, Eric Blake wrote:
> On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
>> From: "Daniel P. Berrange" <berrange(a)redhat.com>
>>
>> There are some interesting escaping rules to consider when dealing
>> with systemd slice/scope names. Thus it is helpful to have APIs
>> for formatting names
>>
>> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
>> ---
>> src/libvirt_private.syms | 2 ++
>> src/util/virsystemd.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++--
>> src/util/virsystemd.h | 5 +++
>> tests/virsystemdtest.c | 48 +++++++++++++++++++++++++
>> 4 files changed, 144 insertions(+), 2 deletions(-)
>>
>
>> +
>> +#define VALID_CHARS \
>> + "0123456789" \
>> + "abcdefghijklmnopqrstuvwxyz" \
>> + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" \
>> + ":-_.\\"
>
> If you would remove - and \\ from this list...
This is the set that systemd uses, and we need to match it
exactly, otherwise we won't detect the cgroups correctly.
My point is that your only use of VALID_CHARS was in the strchr() to see
which characters need escaping. systemd is using the full set for
characters present _after_ escaping, whereas you are only using it
_before_ escaping, and have to special-case your escaping as a result.
But I'm not too concerned about the efficiency to insist on that
micro-optimization.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:09 p.m.
New subject: [libvirt] [PATCH 1/4] Add APIs for formatting systemd slice/scope names
On Fri, Jul 26, 2013 at 5:48 PM, Daniel P. Berrange <berrange(a)redhat.com> wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
There are some interesting escaping rules to consider when dealing
with systemd slice/scope names. Thus it is helpful to have APIs
for formatting names
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/libvirt_private.syms | 2 ++
src/util/virsystemd.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++--
src/util/virsystemd.h | 5 +++
tests/virsystemdtest.c | 48 +++++++++++++++++++++++++
4 files changed, 144 insertions(+), 2 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d9615ea..0247a46 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1935,6 +1935,8 @@ virSysinfoSetup;
# util/virsystemd.h
virSystemdCreateMachine;
+virSystemdMakeScopeName;
+virSystemdMakeSliceName;
# util/virthread.h
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 11d1153..251b846 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -27,9 +27,96 @@
#include "viralloc.h"
#include "virutil.h"
#include "virlog.h"
+#include "virerror.h"
#define VIR_FROM_THIS VIR_FROM_SYSTEMD
+
+static void virSystemdEscapeName(virBufferPtr buf,
+ const char *name)
+{
+ static const char hextable[16] = "0123456789abcdef";
+
+#define ESCAPE(c) \
+ do { \
+ virBufferAddChar(buf, '\\'); \
+ virBufferAddChar(buf, 'x'); \
+ virBufferAddChar(buf, hextable[(c >> 4) & 15]); \
+ virBufferAddChar(buf, hextable[c & 15]); \
+ } while (0)
+
+#define VALID_CHARS \
+ "0123456789" \
+ "abcdefghijklmnopqrstuvwxyz" \
+ "ABCDEFGHIJKLMNOPQRSTUVWXYZ" \
+ ":-_.\\"
+
+ if (*name == '.') {
+ ESCAPE(*name);
+ name++;
+ }
+
+ while (*name) {
+ if (*name == '/')
+ virBufferAddChar(buf, '-');
+ else if (*name == '-' ||
+ *name == '\\' ||
+ !strchr(VALID_CHARS, *name))
+ ESCAPE(*name);
+ else
+ virBufferAddChar(buf, *name);
+ name++;
+ }
+
+#undef ESCAPE
+#undef VALID_CHARS
+}
+
+
+char *virSystemdMakeScopeName(const char *name,
+ const char *drivername,
+ const char *partition)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+ if (*partition == '/')
+ partition++;
+
+ virSystemdEscapeName(&buf, partition);
+ virBufferAddChar(&buf, '-');
+ virSystemdEscapeName(&buf, drivername);
+ virBufferAddLit(&buf, "\\x2d");
What is the idea behind this?
Now we end up with paths like:
/sys/fs/cgroup/memory/machine.slice/machine-lxc\x2dmyfunnycontainer.scope
--
Thanks,
//richard
4:51 a.m.
New subject: [libvirt] [PATCH 1/4] Add APIs for formatting systemd slice/scope names
On Sun, Dec 15, 2013 at 07:09:19PM +0100, Richard Weinberger wrote:
On Fri, Jul 26, 2013 at 5:48 PM, Daniel P. Berrange
<berrange(a)redhat.com> wrote:
> +char *virSystemdMakeScopeName(const char *name,
> + const char *drivername,
> + const char *partition)
> +{
> + virBuffer buf = VIR_BUFFER_INITIALIZER;
> +
> + if (*partition == '/')
> + partition++;
> +
> + virSystemdEscapeName(&buf, partition);
> + virBufferAddChar(&buf, '-');
> + virSystemdEscapeName(&buf, drivername);
> + virBufferAddLit(&buf, "\\x2d");
What is the idea behind this?
Now we end up with paths like:
/sys/fs/cgroup/memory/machine.slice/machine-lxc\x2dmyfunnycontainer.scope
The string prefix 'machine' is a special systemd string. The second part
is the libvirt name 'lxc-$containername'. Systemd requires that the '-'
be
escaped hence we use '\x2d' here. You'll see this in a number of other
systemd unit names too.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:56 a.m.
New subject: [libvirt] [PATCH 1/4] Add APIs for formatting systemd slice/scope names
Am Montag, 16. Dezember 2013, 10:51:01 schrieb Daniel P. Berrange:
On Sun, Dec 15, 2013 at 07:09:19PM +0100, Richard Weinberger wrote:
> On Fri, Jul 26, 2013 at 5:48 PM, Daniel P. Berrange <berrange(a)redhat.com>
wrote:
> > +char *virSystemdMakeScopeName(const char *name,
> > + const char *drivername,
> > + const char *partition)
> > +{
> > + virBuffer buf = VIR_BUFFER_INITIALIZER;
> > +
> > + if (*partition == '/')
> > + partition++;
> > +
> > + virSystemdEscapeName(&buf, partition);
> > + virBufferAddChar(&buf, '-');
> > + virSystemdEscapeName(&buf, drivername);
> > + virBufferAddLit(&buf, "\\x2d");
>
> What is the idea behind this?
> Now we end up with paths like:
> /sys/fs/cgroup/memory/machine.slice/machine-lxc\x2dmyfunnycontainer.scope
The string prefix 'machine' is a special systemd string. The second part
is the libvirt name 'lxc-$containername'. Systemd requires that the '-'
be
escaped hence we use '\x2d' here. You'll see this in a number of other
systemd unit names too.
Srsly?
I really hoped that this is a libvirt bug and not something by
(systemd-)design. :-\
Thanks,
//richard
10:48 a.m.
New subject: [libvirt] [PATCH 2/4] Add support for systemd cgroup mount
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Systemd uses a named cgroup mount for tracking processes. Add
it as another type of controller, albeit one which we have to
special case in a number of places. In particular we must
never create/delete directories there, nor add tasks. Essentially
the systemd mount is to be considered read-only for libvirt.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/util/vircgroup.c | 68 +++++++++++++++++++++++++++++++++++++++------------
src/util/vircgroup.h | 1 +
tests/vircgrouptest.c | 9 +++++++
3 files changed, 63 insertions(+), 15 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 68ec953..5ff8850 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -57,7 +57,8 @@
VIR_ENUM_IMPL(virCgroupController, VIR_CGROUP_CONTROLLER_LAST,
"cpu", "cpuacct", "cpuset",
"memory", "devices",
- "freezer", "blkio", "net_cls",
"perf_event");
+ "freezer", "blkio", "net_cls",
"perf_event",
+ "name=systemd");
typedef enum {
VIR_CGROUP_NONE = 0, /* create subdir under each cgroup if possible. */
@@ -115,6 +116,9 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
char *tmp;
+ if (i == VIR_CGROUP_CONTROLLER_SYSTEMD)
+ continue;
+
if (!group->controllers[i].placement)
continue;
@@ -320,6 +324,9 @@ static int virCgroupCopyPlacement(virCgroupPtr group,
if (!group->controllers[i].mountPoint)
continue;
+ if (i == VIR_CGROUP_CONTROLLER_SYSTEMD)
+ continue;
+
if (path[0] == '/') {
if (VIR_STRDUP(group->controllers[i].placement, path) < 0)
return -1;
@@ -375,6 +382,8 @@ static int virCgroupDetectPlacement(virCgroupPtr group,
int ret = -1;
char *procfile;
+ VIR_DEBUG("Detecting placement for pid %lld path %s",
+ (unsigned long long)pid, path);
if (pid == -1) {
if (VIR_STRDUP(procfile, "/proc/self/cgroup") < 0)
goto cleanup;
@@ -411,6 +420,7 @@ static int virCgroupDetectPlacement(virCgroupPtr group,
const char *typestr = virCgroupControllerTypeToString(i);
int typelen = strlen(typestr);
char *tmp = controllers;
+
while (tmp) {
char *next = strchr(tmp, ',');
int len;
@@ -427,13 +437,20 @@ static int virCgroupDetectPlacement(virCgroupPtr group,
* selfpath=="/libvirt.service" + path="foo" ->
"/libvirt.service/foo"
*/
if (typelen == len && STREQLEN(typestr, tmp, len) &&
- group->controllers[i].mountPoint != NULL) {
- if (virAsprintf(&group->controllers[i].placement,
- "%s%s%s", selfpath,
- (STREQ(selfpath, "/") ||
- STREQ(path, "") ? "" :
"/"),
- path) < 0)
- goto cleanup;
+ group->controllers[i].mountPoint != NULL &&
+ group->controllers[i].placement == NULL) {
+ if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) {
+ if (VIR_STRDUP(group->controllers[i].placement,
+ selfpath) < 0)
+ goto cleanup;
+ } else {
+ if (virAsprintf(&group->controllers[i].placement,
+ "%s%s%s", selfpath,
+ (STREQ(selfpath, "/") ||
+ STREQ(path, "") ? "" :
"/"),
+ path) < 0)
+ goto cleanup;
+ }
}
tmp = next;
@@ -524,13 +541,16 @@ static int virCgroupDetect(virCgroupPtr group,
return -1;
}
- if (parent || path[0] == '/') {
- if (virCgroupCopyPlacement(group, path, parent) < 0)
- return -1;
- } else {
- if (virCgroupDetectPlacement(group, pid, path) < 0)
- return -1;
- }
+ /* In some cases we can copy part of the placement info
+ * based on the parent cgroup...
+ */
+ if ((parent || path[0] == '/') &&
+ virCgroupCopyPlacement(group, path, parent) < 0)
+ return -1;
+
+ /* ... but use /proc/cgroups to fill in the rest */
+ if (virCgroupDetectPlacement(group, pid, path) < 0)
+ return -1;
/* Check that for every mounted controller, we found our placement */
for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
@@ -822,6 +842,12 @@ static int virCgroupMakeGroup(virCgroupPtr parent,
for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
char *path = NULL;
+ /* We must never mkdir() in systemd's hierachy */
+ if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) {
+ VIR_DEBUG("Not creating systemd controller group");
+ continue;
+ }
+
/* Skip over controllers that aren't mounted */
if (!group->controllers[i].mountPoint) {
VIR_DEBUG("Skipping unmounted controller %s",
@@ -1026,6 +1052,10 @@ int virCgroupRemove(virCgroupPtr group)
if (!group->controllers[i].mountPoint)
continue;
+ /* We must never rmdir() in systemd's hiearchy */
+ if (i == VIR_CGROUP_CONTROLLER_SYSTEMD)
+ continue;
+
/* Don't delete the root group, if we accidentally
ended up in it for some reason */
if (STREQ(group->controllers[i].placement, "/"))
@@ -1065,6 +1095,10 @@ int virCgroupAddTask(virCgroupPtr group, pid_t pid)
if (!group->controllers[i].mountPoint)
continue;
+ /* We must never add tasks in systemd's hiearchy */
+ if (i == VIR_CGROUP_CONTROLLER_SYSTEMD)
+ continue;
+
if (virCgroupSetValueU64(group, i, "tasks", (unsigned long long)pid)
< 0)
goto cleanup;
}
@@ -1166,6 +1200,10 @@ int virCgroupMoveTask(virCgroupPtr src_group, virCgroupPtr
dest_group)
!dest_group->controllers[i].mountPoint)
continue;
+ /* We must never move tasks in systemd's hiearchy */
+ if (i == VIR_CGROUP_CONTROLLER_SYSTEMD)
+ continue;
+
/* New threads are created in the same group as their parent;
* but if a thread is created after we first read we aren't
* aware that it needs to move. Therefore, we must iterate
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index 3aaf081..e579f41 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -40,6 +40,7 @@ enum {
VIR_CGROUP_CONTROLLER_BLKIO,
VIR_CGROUP_CONTROLLER_NET_CLS,
VIR_CGROUP_CONTROLLER_PERF_EVENT,
+ VIR_CGROUP_CONTROLLER_SYSTEMD,
VIR_CGROUP_CONTROLLER_LAST
};
diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c
index 20ac494..4bdd4c9 100644
--- a/tests/vircgrouptest.c
+++ b/tests/vircgrouptest.c
@@ -87,6 +87,7 @@ const char *mountsSmall[VIR_CGROUP_CONTROLLER_LAST] = {
[VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
[VIR_CGROUP_CONTROLLER_FREEZER] = NULL,
[VIR_CGROUP_CONTROLLER_BLKIO] = NULL,
+ [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL,
};
const char *mountsFull[VIR_CGROUP_CONTROLLER_LAST] = {
[VIR_CGROUP_CONTROLLER_CPU] = "/not/really/sys/fs/cgroup/cpu,cpuacct",
@@ -96,6 +97,7 @@ const char *mountsFull[VIR_CGROUP_CONTROLLER_LAST] = {
[VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
[VIR_CGROUP_CONTROLLER_FREEZER] = "/not/really/sys/fs/cgroup/freezer",
[VIR_CGROUP_CONTROLLER_BLKIO] = "/not/really/sys/fs/cgroup/blkio",
+ [VIR_CGROUP_CONTROLLER_SYSTEMD] = "/not/really/sys/fs/cgroup/systemd",
};
const char *links[VIR_CGROUP_CONTROLLER_LAST] = {
@@ -121,6 +123,7 @@ static int testCgroupNewForSelf(const void *args ATTRIBUTE_UNUSED)
[VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
[VIR_CGROUP_CONTROLLER_FREEZER] = "/",
[VIR_CGROUP_CONTROLLER_BLKIO] = "/",
+ [VIR_CGROUP_CONTROLLER_SYSTEMD] = "/user/berrange/123",
};
if (virCgroupNewSelf(&cgroup) < 0) {
@@ -161,6 +164,7 @@ static int testCgroupNewForPartition(const void *args
ATTRIBUTE_UNUSED)
[VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
[VIR_CGROUP_CONTROLLER_FREEZER] = NULL,
[VIR_CGROUP_CONTROLLER_BLKIO] = NULL,
+ [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL,
};
const char *placementFull[VIR_CGROUP_CONTROLLER_LAST] = {
[VIR_CGROUP_CONTROLLER_CPU] = "/virtualmachines.partition",
@@ -170,6 +174,7 @@ static int testCgroupNewForPartition(const void *args
ATTRIBUTE_UNUSED)
[VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
[VIR_CGROUP_CONTROLLER_FREEZER] = "/virtualmachines.partition",
[VIR_CGROUP_CONTROLLER_BLKIO] = "/virtualmachines.partition",
+ [VIR_CGROUP_CONTROLLER_SYSTEMD] = "/user/berrange/123",
};
if ((rv = virCgroupNewPartition("/virtualmachines", false, -1,
&cgroup)) != -1) {
@@ -233,6 +238,7 @@ static int testCgroupNewForPartitionNested(const void *args
ATTRIBUTE_UNUSED)
[VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
[VIR_CGROUP_CONTROLLER_FREEZER] =
"/deployment.partition/production.partition",
[VIR_CGROUP_CONTROLLER_BLKIO] =
"/deployment.partition/production.partition",
+ [VIR_CGROUP_CONTROLLER_SYSTEMD] = "/user/berrange/123",
};
if ((rv = virCgroupNewPartition("/deployment/production", false, -1,
&cgroup)) != -1) {
@@ -281,6 +287,7 @@ static int testCgroupNewForPartitionNestedDeep(const void *args
ATTRIBUTE_UNUSED
[VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
[VIR_CGROUP_CONTROLLER_FREEZER] =
"/user/berrange.user/production.partition",
[VIR_CGROUP_CONTROLLER_BLKIO] =
"/user/berrange.user/production.partition",
+ [VIR_CGROUP_CONTROLLER_SYSTEMD] = "/user/berrange/123",
};
if ((rv = virCgroupNewPartition("/user/berrange.user/production", false,
-1, &cgroup)) != -1) {
@@ -336,6 +343,7 @@ static int testCgroupNewForPartitionDomain(const void *args
ATTRIBUTE_UNUSED)
[VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
[VIR_CGROUP_CONTROLLER_FREEZER] =
"/production.partition/foo.libvirt-lxc",
[VIR_CGROUP_CONTROLLER_BLKIO] =
"/production.partition/foo.libvirt-lxc",
+ [VIR_CGROUP_CONTROLLER_SYSTEMD] = "/user/berrange/123",
};
if ((rv = virCgroupNewPartition("/production", true, -1,
&partitioncgroup)) != 0) {
@@ -372,6 +380,7 @@ static int testCgroupNewForPartitionDomainEscaped(const void *args
ATTRIBUTE_UNU
[VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
[VIR_CGROUP_CONTROLLER_FREEZER] =
"/_cgroup.evil/net_cls.evil/__evil.evil/_cpu.foo.libvirt-lxc",
[VIR_CGROUP_CONTROLLER_BLKIO] =
"/_cgroup.evil/net_cls.evil/__evil.evil/_cpu.foo.libvirt-lxc",
+ [VIR_CGROUP_CONTROLLER_SYSTEMD] = "/user/berrange/123",
};
if ((rv = virCgroupNewPartition("/cgroup.evil", true, -1,
&partitioncgroup1)) != 0) {
--
1.8.1.4
1:13 p.m.
New subject: [libvirt] [PATCH 2/4] Add support for systemd cgroup mount
On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Systemd uses a named cgroup mount for tracking processes. Add
it as another type of controller, albeit one which we have to
special case in a number of places. In particular we must
never create/delete directories there, nor add tasks. Essentially
the systemd mount is to be considered read-only for libvirt.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/util/vircgroup.c | 68 +++++++++++++++++++++++++++++++++++++++------------
src/util/vircgroup.h | 1 +
tests/vircgrouptest.c | 9 +++++++
3 files changed, 63 insertions(+), 15 deletions(-)
@@ -524,13 +541,16 @@ static int virCgroupDetect(virCgroupPtr group,
return -1;
}
- if (parent || path[0] == '/') {
- if (virCgroupCopyPlacement(group, path, parent) < 0)
- return -1;
- } else {
- if (virCgroupDetectPlacement(group, pid, path) < 0)
- return -1;
This previously called only one of the two functions...
- }
+ /* In some cases we can copy part of the placement info
+ * based on the parent cgroup...
+ */
+ if ((parent || path[0] == '/') &&
+ virCgroupCopyPlacement(group, path, parent) < 0)
+ return -1;
+
+ /* ... but use /proc/cgroups to fill in the rest */
+ if (virCgroupDetectPlacement(group, pid, path) < 0)
...now, if virCgroupCopyPlacement returns 0, it calls both functions.
Is that intentional?
@@ -822,6 +842,12 @@ static int virCgroupMakeGroup(virCgroupPtr
parent,
for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
char *path = NULL;
+ /* We must never mkdir() in systemd's hierachy */
s/hierachy/hierarchy/
+ if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) {
+ VIR_DEBUG("Not creating systemd controller group");
+ continue;
+ }
+
/* Skip over controllers that aren't mounted */
if (!group->controllers[i].mountPoint) {
VIR_DEBUG("Skipping unmounted controller %s",
@@ -1026,6 +1052,10 @@ int virCgroupRemove(virCgroupPtr group)
if (!group->controllers[i].mountPoint)
continue;
+ /* We must never rmdir() in systemd's hiearchy */
s/hiearchy/hierarchy/
(hmm, you copied-and-pasted, but ended up with a different typo between
the two comments?)
+ if (i == VIR_CGROUP_CONTROLLER_SYSTEMD)
+ continue;
+
/* Don't delete the root group, if we accidentally
ended up in it for some reason */
if (STREQ(group->controllers[i].placement, "/"))
@@ -1065,6 +1095,10 @@ int virCgroupAddTask(virCgroupPtr group, pid_t pid)
if (!group->controllers[i].mountPoint)
continue;
+ /* We must never add tasks in systemd's hiearchy */
s/hiearchy/hierarchy/
+ if (i == VIR_CGROUP_CONTROLLER_SYSTEMD)
+ continue;
+
if (virCgroupSetValueU64(group, i, "tasks", (unsigned long long)pid)
< 0)
goto cleanup;
}
@@ -1166,6 +1200,10 @@ int virCgroupMoveTask(virCgroupPtr src_group, virCgroupPtr
dest_group)
!dest_group->controllers[i].mountPoint)
continue;
+ /* We must never move tasks in systemd's hiearchy */
s/hiearchy/hierarchy/
Conditional ACK if you can address my question and fix the typos.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
1:49 p.m.
New subject: [libvirt] [PATCH 2/4] Add support for systemd cgroup mount
On Tue, Jul 30, 2013 at 12:13:58PM -0600, Eric Blake wrote:
On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange(a)redhat.com>
>
> Systemd uses a named cgroup mount for tracking processes. Add
> it as another type of controller, albeit one which we have to
> special case in a number of places. In particular we must
> never create/delete directories there, nor add tasks. Essentially
> the systemd mount is to be considered read-only for libvirt.
>
> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
> ---
> src/util/vircgroup.c | 68 +++++++++++++++++++++++++++++++++++++++------------
> src/util/vircgroup.h | 1 +
> tests/vircgrouptest.c | 9 +++++++
> 3 files changed, 63 insertions(+), 15 deletions(-)
>
> @@ -524,13 +541,16 @@ static int virCgroupDetect(virCgroupPtr group,
> return -1;
> }
>
> - if (parent || path[0] == '/') {
> - if (virCgroupCopyPlacement(group, path, parent) < 0)
> - return -1;
> - } else {
> - if (virCgroupDetectPlacement(group, pid, path) < 0)
> - return -1;
This previously called only one of the two functions...
> - }
> + /* In some cases we can copy part of the placement info
> + * based on the parent cgroup...
> + */
> + if ((parent || path[0] == '/') &&
> + virCgroupCopyPlacement(group, path, parent) < 0)
> + return -1;
> +
> + /* ... but use /proc/cgroups to fill in the rest */
> + if (virCgroupDetectPlacement(group, pid, path) < 0)
...now, if virCgroupCopyPlacement returns 0, it calls both functions.
Is that intentional?
Yes, when we use CopyPlacement it sets up all mounts, except for
the systemd mount, which we must always probe for. The DetectPlacement
method was changed so that it only fills in data, not already filled
in by CopyPlacement.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
1:57 p.m.
New subject: [libvirt] [PATCH 2/4] Add support for systemd cgroup mount
On 07/30/2013 12:49 PM, Daniel P. Berrange wrote:
On Tue, Jul 30, 2013 at 12:13:58PM -0600, Eric Blake wrote:
> On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
>> From: "Daniel P. Berrange" <berrange(a)redhat.com>
>>
>> Systemd uses a named cgroup mount for tracking processes. Add
>> it as another type of controller, albeit one which we have to
>> special case in a number of places. In particular we must
>> never create/delete directories there, nor add tasks. Essentially
>> the systemd mount is to be considered read-only for libvirt.
>>
>> Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
>> - }
>> + /* In some cases we can copy part of the placement info
>> + * based on the parent cgroup...
>> + */
>> + if ((parent || path[0] == '/') &&
>> + virCgroupCopyPlacement(group, path, parent) < 0)
>> + return -1;
>> +
>> + /* ... but use /proc/cgroups to fill in the rest */
>> + if (virCgroupDetectPlacement(group, pid, path) < 0)
>
> ...now, if virCgroupCopyPlacement returns 0, it calls both functions.
> Is that intentional?
Yes, when we use CopyPlacement it sets up all mounts, except for
the systemd mount, which we must always probe for. The DetectPlacement
method was changed so that it only fills in data, not already filled
in by CopyPlacement.
That explains it, then. Probably worth mentioning in the commit
message, so the next reader won't miss it like I did. Condition has
been met, ACK is now granted with the typo fixes.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:48 a.m.
New subject: [libvirt] [PATCH 3/4] Cope with races while killing processes
From: "Daniel P. Berrange" <berrange(a)redhat.com>
When systemd is involved in managing processes, it may start
killing off & tearing down croups associated with the process
while we're still doing virCgroupKillPainfully. We must
explicitly check for ENOENT and treat it as if we had finished
killing processes
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/util/vircgroup.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 5ff8850..94f6692 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -2539,6 +2539,12 @@ static int virCgroupKillInternal(virCgroupPtr group, int signum,
virHashTablePtr
while (!done) {
done = true;
if (!(fp = fopen(keypath, "r"))) {
+ if (errno == ENOENT) {
+ VIR_DEBUG("No file %s, assuming done", keypath);
+ killedAny = false;
+ goto done;
+ }
+
virReportSystemError(errno,
_("Failed to read %s"),
keypath);
@@ -2578,6 +2584,7 @@ static int virCgroupKillInternal(virCgroupPtr group, int signum,
virHashTablePtr
}
}
+ done:
ret = killedAny ? 1 : 0;
cleanup:
@@ -2647,8 +2654,13 @@ static int virCgroupKillRecursiveInternal(virCgroupPtr group, int
signum, virHas
if (rc == 1)
killedAny = true;
- VIR_DEBUG("Iterate over children of %s", keypath);
+ VIR_DEBUG("Iterate over children of %s (killedAny=%d)", keypath,
killedAny);
if (!(dp = opendir(keypath))) {
+ if (errno == ENOENT) {
+ VIR_DEBUG("Path %s does not exist, assuming done", keypath);
+ killedAny = false;
+ goto done;
+ }
virReportSystemError(errno,
_("Cannot open %s"), keypath);
return -1;
@@ -2678,6 +2690,7 @@ static int virCgroupKillRecursiveInternal(virCgroupPtr group, int
signum, virHas
virCgroupFree(&subgroup);
}
+ done:
ret = killedAny ? 1 : 0;
cleanup:
--
1.8.1.4
2:07 p.m.
New subject: [libvirt] [PATCH 3/4] Cope with races while killing processes
On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
When systemd is involved in managing processes, it may start
killing off & tearing down croups associated with the process
while we're still doing virCgroupKillPainfully. We must
explicitly check for ENOENT and treat it as if we had finished
killing processes
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/util/vircgroup.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:48 a.m.
New subject: [libvirt] [PATCH 4/4] Enable support for systemd-machined in cgroups creation
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Make the virCgroupNewMachine method try to use systemd-machined
first. If that fails, then fallback to using the traditional
cgroup setup code path.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/lxc/lxc_process.c | 10 +--
src/qemu/qemu_cgroup.c | 1 +
src/util/vircgroup.c | 187 ++++++++++++++++++++++++++++++++++++++++++++-----
src/util/vircgroup.h | 1 +
src/util/virsystemd.c | 5 +-
5 files changed, 182 insertions(+), 22 deletions(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 247e516..0a28305 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1203,8 +1203,9 @@ int virLXCProcessStart(virConnectPtr conn,
goto cleanup;
}
- if (virCgroupNewDetectMachine(vm->def->name, "lxc",
- vm->pid, -1, &priv->cgroup) < 0)
+ if (virCgroupNewDetectMachine(vm->def->name, "lxc", vm->pid,
+ vm->def->resource->partition,
+ -1, &priv->cgroup) < 0)
goto error;
if (!priv->cgroup) {
@@ -1411,8 +1412,9 @@ virLXCProcessReconnectDomain(virDomainObjPtr vm,
if (!(priv->monitor = virLXCProcessConnectMonitor(driver, vm)))
goto error;
- if (virCgroupNewDetectMachine(vm->def->name, "lxc",
- vm->pid, -1, &priv->cgroup) < 0)
+ if (virCgroupNewDetectMachine(vm->def->name, "lxc", vm->pid,
+ vm->def->resource->partition,
+ -1, &priv->cgroup) < 0)
goto error;
if (!priv->cgroup) {
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 9f6b251..787ddeb 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -707,6 +707,7 @@ qemuConnectCgroup(virQEMUDriverPtr driver,
if (virCgroupNewDetectMachine(vm->def->name,
"qemu",
vm->pid,
+ vm->def->resource->partition,
cfg->cgroupControllers,
&priv->cgroup) < 0)
goto cleanup;
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 94f6692..e857362 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -50,6 +50,7 @@
#include "virhash.h"
#include "virhashcode.h"
#include "virstring.h"
+#include "virsystemd.h"
#define CGROUP_MAX_VAL 512
@@ -100,11 +101,13 @@ static bool
virCgroupValidateMachineGroup(virCgroupPtr group,
const char *name,
const char *drivername,
+ const char *partition,
bool stripEmulatorSuffix)
{
size_t i;
bool valid = false;
char *partname;
+ char *scopename;
if (virAsprintf(&partname, "%s.libvirt-%s",
name, drivername) < 0)
@@ -113,6 +116,15 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
if (virCgroupPartitionEscape(&partname) < 0)
goto cleanup;
+ if (!partition)
+ partition = "/machine";
+
+ if (!(scopename = virSystemdMakeScopeName(name, drivername, partition)))
+ goto cleanup;
+
+ if (virCgroupPartitionEscape(&scopename) < 0)
+ goto cleanup;
+
for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
char *tmp;
@@ -140,9 +152,10 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
tmp++;
if (STRNEQ(tmp, name) &&
- STRNEQ(tmp, partname)) {
- VIR_DEBUG("Name '%s' for controller '%s' does not match
'%s' or '%s'",
- tmp, virCgroupControllerTypeToString(i), name, partname);
+ STRNEQ(tmp, partname) &&
+ STRNEQ(tmp, scopename)) {
+ VIR_DEBUG("Name '%s' for controller '%s' does not match
'%s', '%s' or '%s'",
+ tmp, virCgroupControllerTypeToString(i), name, partname,
scopename);
goto cleanup;
}
}
@@ -151,6 +164,7 @@ virCgroupValidateMachineGroup(virCgroupPtr group,
cleanup:
VIR_FREE(partname);
+ VIR_FREE(scopename);
return valid;
}
@@ -1638,6 +1652,7 @@ int virCgroupNewDetect(pid_t pid ATTRIBUTE_UNUSED,
int virCgroupNewDetectMachine(const char *name,
const char *drivername,
pid_t pid,
+ const char *partition,
int controllers,
virCgroupPtr *group)
{
@@ -1647,7 +1662,7 @@ int virCgroupNewDetectMachine(const char *name,
return -1;
}
- if (!virCgroupValidateMachineGroup(*group, name, drivername, true)) {
+ if (!virCgroupValidateMachineGroup(*group, name, drivername, partition, true)) {
VIR_DEBUG("Failed to validate machine name for '%s' driver
'%s'",
name, drivername);
virCgroupFree(group);
@@ -1657,22 +1672,124 @@ int virCgroupNewDetectMachine(const char *name,
return 0;
}
-int virCgroupNewMachine(const char *name,
- const char *drivername,
- bool privileged ATTRIBUTE_UNUSED,
- const unsigned char *uuid ATTRIBUTE_UNUSED,
- const char *rootdir ATTRIBUTE_UNUSED,
- pid_t pidleader ATTRIBUTE_UNUSED,
- bool isContainer ATTRIBUTE_UNUSED,
- const char *partition,
- int controllers,
- virCgroupPtr *group)
+/*
+ * Retujrns 0 on success, -1 on fatal error, -2 on systemd not available
+ */
+static int
+virCgroupNewMachineSystemd(const char *name,
+ const char *drivername,
+ bool privileged,
+ const unsigned char *uuid,
+ const char *rootdir,
+ pid_t pidleader,
+ bool isContainer,
+ const char *partition,
+ int controllers,
+ virCgroupPtr *group)
{
- virCgroupPtr parent = NULL;
int ret = -1;
+ int rv;
+ virCgroupPtr init, parent = NULL;
+ char *path = NULL;
+ char *offset;
+
+ VIR_DEBUG("Trying to setup machine '%s' via systemd", name);
+ if ((rv = virSystemdCreateMachine(name,
+ drivername,
+ privileged,
+ uuid,
+ rootdir,
+ pidleader,
+ isContainer,
+ partition)) < 0)
+ return rv;
+
+ if (controllers != -1)
+ controllers |= (1 << VIR_CGROUP_CONTROLLER_SYSTEMD);
+
+ VIR_DEBUG("Detecting systemd placement");
+ if (virCgroupNewDetect(pidleader,
+ controllers,
+ &init) < 0)
+ return -1;
- *group = NULL;
+ path = (init)->controllers[VIR_CGROUP_CONTROLLER_SYSTEMD].placement;
+ (init)->controllers[VIR_CGROUP_CONTROLLER_SYSTEMD].placement = NULL;
+ virCgroupFree(&init);
+
+ if (!path || STREQ(path, "/") || path[0] != '/') {
+ VIR_DEBUG("Systemd didn't setup its controller");
+ ret = -2;
+ goto cleanup;
+ }
+
+ offset = path;
+
+ if (virCgroupNew(pidleader,
+ "",
+ NULL,
+ controllers,
+ &parent) < 0)
+ goto cleanup;
+
+
+ for (;;) {
+ virCgroupPtr tmp;
+ char *t = strchr(offset + 1, '/');
+ if (t)
+ *t = '\0';
+
+ if (virCgroupNew(pidleader,
+ path,
+ parent,
+ controllers,
+ &tmp) < 0)
+ goto cleanup;
+
+ if (virCgroupMakeGroup(parent, tmp, true, VIR_CGROUP_NONE) < 0) {
+ virCgroupFree(&tmp);
+ goto cleanup;
+ }
+ if (t) {
+ *t = '/';
+ offset = t;
+ virCgroupFree(&parent);
+ parent = tmp;
+ } else {
+ *group = tmp;
+ break;
+ }
+ }
+
+ if (virCgroupAddTask(*group, pidleader) < 0) {
+ virErrorPtr saved = virSaveLastError();
+ virCgroupRemove(*group);
+ virCgroupFree(group);
+ if (saved) {
+ virSetError(saved);
+ virFreeError(saved);
+ }
+ }
+
+ ret = 0;
+ cleanup:
+ virCgroupFree(&parent);
+ VIR_FREE(path);
+ return ret;
+}
+static int
+virCgroupNewMachineManual(const char *name,
+ const char *drivername,
+ pid_t pidleader,
+ const char *partition,
+ int controllers,
+ virCgroupPtr *group)
+{
+ virCgroupPtr parent = NULL;
+ int ret = -1;
+
+ VIR_DEBUG("Fallback to non-systemd setup");
if (virCgroupNewPartition(partition,
STREQ(partition, "/machine"),
controllers,
@@ -1708,6 +1825,44 @@ cleanup:
return ret;
}
+int virCgroupNewMachine(const char *name,
+ const char *drivername,
+ bool privileged,
+ const unsigned char *uuid,
+ const char *rootdir,
+ pid_t pidleader,
+ bool isContainer,
+ const char *partition,
+ int controllers,
+ virCgroupPtr *group)
+{
+ int rv;
+
+ *group = NULL;
+
+ if ((rv = virCgroupNewMachineSystemd(name,
+ drivername,
+ privileged,
+ uuid,
+ rootdir,
+ pidleader,
+ isContainer,
+ partition,
+ controllers,
+ group)) == 0)
+ return 0;
+
+ if (rv == -1)
+ return -1;
+
+ return virCgroupNewMachineManual(name,
+ drivername,
+ pidleader,
+ partition,
+ controllers,
+ group);
+}
+
bool virCgroupNewIgnoreError(void)
{
if (virLastErrorIsSystemErrno(ENXIO) ||
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index e579f41..d7ce892 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -83,6 +83,7 @@ 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 251b846..3fd653d 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -225,8 +225,9 @@ int virSystemdCreateMachine(const char *name,
iscontainer ? "container" : "vm",
(unsigned int)pidleader,
rootdir ? rootdir : "",
- 1, "Slice", "s",
- slicename) < 0) {
+ 1,
+ "Slice", "s", slicename
+ ) < 0) {
virErrorPtr err = virGetLastError();
if (err->code == VIR_ERR_DBUS_SERVICE &&
STREQ(err->str2, "org.freedesktop.DBus.Error.ServiceUnknown"))
{
--
1.8.1.4
2:22 p.m.
New subject: [libvirt] [PATCH 4/4] Enable support for systemd-machined in cgroups creation
On 07/26/2013 09:48 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Make the virCgroupNewMachine method try to use systemd-machined
first. If that fails, then fallback to using the traditional
cgroup setup code path.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/lxc/lxc_process.c | 10 +--
src/qemu/qemu_cgroup.c | 1 +
src/util/vircgroup.c | 187 ++++++++++++++++++++++++++++++++++++++++++++-----
src/util/vircgroup.h | 1 +
src/util/virsystemd.c | 5 +-
5 files changed, 182 insertions(+), 22 deletions(-)
+/*
+ * Retujrns 0 on success, -1 on fatal error, -2 on systemd not available
+ */
s/Retujrns/Returns/
- *group = NULL;
+ path = (init)->controllers[VIR_CGROUP_CONTROLLER_SYSTEMD].placement;
+ (init)->controllers[VIR_CGROUP_CONTROLLER_SYSTEMD].placement = NULL;
s/(init)/init/ (looks like you refactored, and had (*init) at one point)
+++ b/src/util/virsystemd.c
@@ -225,8 +225,9 @@ int virSystemdCreateMachine(const char *name,
iscontainer ? "container" : "vm",
(unsigned int)pidleader,
rootdir ? rootdir : "",
- 1, "Slice", "s",
- slicename) < 0) {
+ 1,
+ "Slice", "s", slicename
+ ) < 0) {
Spurious reformatting?
ACK with nits fixed.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:53 a.m.
On Fri, Jul 26, 2013 at 04:48:20PM +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
This is a much changed / expanded version of my previous work to
create cgroups via systemd. The difference is that this time it
actually works :-)
I'm not proposing this for merge until after the 1.1.1 release.
This is now ready for review...
Daniel P. Berrange (4):
Add APIs for formatting systemd slice/scope names
Add support for systemd cgroup mount
Cope with races while killing processes
Enable support for systemd-machined in cgroups creation
src/libvirt_private.syms | 2 +
src/lxc/lxc_process.c | 10 +-
src/qemu/qemu_cgroup.c | 1 +
src/util/vircgroup.c | 270 +++++++++++++++++++++++++++++++++++++++++------
src/util/vircgroup.h | 2 +
src/util/virsystemd.c | 96 ++++++++++++++++-
src/util/virsystemd.h | 5 +
tests/vircgrouptest.c | 9 ++
tests/virsystemdtest.c | 48 +++++++++
9 files changed, 403 insertions(+), 40 deletions(-)
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
3993
days inactive
4136
days old
16 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Richard Weinberger -
Richard Weinberger