On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

We will need to extract current cgroup v1 implementation into separate backend because there will be new cgroup v2 implementation and both will have to co-exit.

typo: co-exit -> co-exist

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/Makefile.am | 1 + src/libvirt_private.syms | 3 ++ src/util/Makefile.inc.am | 2 ++ src/util/vircgroupbackend.c | 64 +++++++++++++++++++++++++++++++++++++ src/util/vircgroupbackend.h | 46 ++++++++++++++++++++++++++ 5 files changed, 116 insertions(+) create mode 100644 src/util/vircgroupbackend.c create mode 100644 src/util/vircgroupbackend.h

diff --git a/src/Makefile.am b/src/Makefile.am index 2a3ed0d42d..3465629005 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -673,6 +673,7 @@ libvirt_setuid_rpc_client_la_SOURCES = \ util/virbitmap.c \ util/virbuffer.c \ util/vircgroup.c \ + util/vircgroupbackend.c \ util/vircommand.c \ util/virconf.c \ util/virdbus.c \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 75c59fbf89..eb302b3559 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1576,6 +1576,9 @@ virCgroupSetOwner; virCgroupSupportsCpuBW; virCgroupTerminateMachine;

+# util/vircgroupbackend.h +virCgroupBackendGetAll; +virCgroupBackendRegister;

# util/virclosecallbacks.h virCloseCallbacksGet; diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am index a22265606c..1b236ca62b 100644 --- a/src/util/Makefile.inc.am +++ b/src/util/Makefile.inc.am @@ -23,6 +23,8 @@ UTIL_SOURCES = \ util/virperf.h \ util/vircgroup.c \ util/vircgroup.h util/vircgrouppriv.h \ + util/vircgroupbackend.c \ + util/vircgroupbackend.h \ util/virclosecallbacks.c \ util/virclosecallbacks.h \ util/vircommand.c \ diff --git a/src/util/vircgroupbackend.c b/src/util/vircgroupbackend.c new file mode 100644 index 0000000000..e014bfc0e6 --- /dev/null +++ b/src/util/vircgroupbackend.c @@ -0,0 +1,64 @@ +/* + * vircgroupbackend.c: methods for cgroups backend + * + * Copyright (C) 2018 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ +#include <config.h> + +#include "vircgroupbackend.h" +#include "virerror.h" +#include "virthread.h" + +#define VIR_FROM_THIS VIR_FROM_CGROUP + +VIR_ENUM_DECL(virCgroupBackend); +VIR_ENUM_IMPL(virCgroupBackend, VIR_CGROUP_BACKEND_TYPE_LAST, + "cgroup V1"); + +static virOnceControl virCgroupBackendOnce = VIR_ONCE_CONTROL_INITIALIZER; +static virCgroupBackendPtr virCgroupBackends[VIR_CGROUP_BACKEND_TYPE_LAST] = { 0 }; + +void +virCgroupBackendRegister(virCgroupBackendPtr backend) +{ + if (virCgroupBackends[backend->type]) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cgroup backend '%s' already registered."), + virCgroupBackendTypeToString(backend->type)); + return; + } + + virCgroupBackends[backend->type] = backend; +} + + +static void +virCgroupBackendOnceInit(void) +{ +} + + +virCgroupBackendPtr * +virCgroupBackendGetAll(void) +{ + if (virOnce(&virCgroupBackendOnce, virCgroupBackendOnceInit) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to initialize cgroup backend.")); + return NULL; + } + return virCgroupBackends; +} diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h new file mode 100644 index 0000000000..db052485a8 --- /dev/null +++ b/src/util/vircgroupbackend.h @@ -0,0 +1,46 @@ +/* + * vircgroupbackend.h: methods for cgroups backend + * + * Copyright (C) 2018 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#ifndef __VIR_CGROUP_BACKEND_H__ +# define __VIR_CGROUP_BACKEND_H__ + +# include "internal.h" + +# include "vircgroup.h" + + +typedef enum { + VIR_CGROUP_BACKEND_TYPE_V1 = 0, + VIR_CGROUP_BACKEND_TYPE_LAST, +} virCgroupBackendType; + +struct _virCgroupBackend { + virCgroupBackendType type; +}; +typedef struct _virCgroupBackend virCgroupBackend; +typedef virCgroupBackend *virCgroupBackendPtr; + +void +virCgroupBackendRegister(virCgroupBackendPtr backend); + +virCgroupBackendPtr * +virCgroupBackendGetAll(void); + +#endif /* __VIR_CGROUP_BACKEND_H__ */ -- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 28 +++++++++------------------- src/util/vircgroupbackend.h | 7 +++++++ src/util/vircgroupv1.c | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index e85e0bde24..a5478a3fa4 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -51,6 +51,7 @@

#include "virutil.h" #include "viralloc.h" +#include "vircgroupbackend.h" #include "virerror.h" #include "virlog.h" #include "virfile.h" @@ -64,8 +65,6 @@

VIR_LOG_INIT("util.cgroup");

-#define CGROUP_MAX_VAL 512 - #define VIR_FROM_THIS VIR_FROM_CGROUP

#define CGROUP_NB_TOTAL_CPU_STAT_PARAM 3 @@ -132,29 +131,20 @@ virCgroupGetDevicePermsString(int perms) bool virCgroupAvailable(void) { - bool ret = false; - FILE *mounts = NULL; - struct mntent entry; - char buf[CGROUP_MAX_VAL]; + size_t i; + virCgroupBackendPtr *backends = virCgroupBackendGetAll();

- if (!virFileExists("/proc/cgroups")) + if (!backends) return false;

- if (!(mounts = fopen("/proc/mounts", "r"))) - return false; - - while (getmntent_r(mounts, &entry, buf, sizeof(buf)) != NULL) { - /* We're looking for at least one 'cgroup' fs mount, - * which is *not* a named mount. */ - if (STREQ(entry.mnt_type, "cgroup") && - !strstr(entry.mnt_opts, "name=")) { - ret = true; - break; + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (backends[i] && backends[i]->available && + backends[i]->available()) {

A little bit down in this patch I see the following comment about the "available" callback: /* Mandatory callbacks that need to be implemented for every backend. */ Considering this is part of the mandatory ones, do we really need to do the `backends[i]->available` check?

+ return true; } }

- VIR_FORCE_FCLOSE(mounts); - return ret; + return false; }

diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index db052485a8..88f51416b0 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -25,14 +25,21 @@

# include "vircgroup.h"

+# define CGROUP_MAX_VAL 512

typedef enum { VIR_CGROUP_BACKEND_TYPE_V1 = 0, VIR_CGROUP_BACKEND_TYPE_LAST, } virCgroupBackendType;

+typedef bool +(*virCgroupAvailableCB)(void); + struct _virCgroupBackend { virCgroupBackendType type; + + /* Mandatory callbacks that need to be implemented for every backend. */ + virCgroupAvailableCB available; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 711ebf1128..c5bd2f6a3b 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -20,6 +20,10 @@ */ #include <config.h>

+#if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R +# include <mntent.h> +#endif + #include "internal.h"

#define __VIR_CGROUP_ALLOW_INCLUDE_PRIV_H__ @@ -29,6 +33,7 @@ #include "vircgroup.h" #include "vircgroupbackend.h" #include "vircgroupv1.h" +#include "virfile.h" #include "virlog.h"

VIR_LOG_INIT("util.cgroup"); @@ -43,8 +48,38 @@ VIR_ENUM_IMPL(virCgroupV1Controller, VIR_CGROUP_CONTROLLER_LAST, "name=systemd");

+/* We're looking for at least one 'cgroup' fs mount, + * which is *not* a named mount. */ +static bool +virCgroupV1Available(void) +{ + bool ret = false; + FILE *mounts = NULL; + struct mntent entry; + char buf[CGROUP_MAX_VAL]; + + if (!virFileExists("/proc/cgroups")) + return false; + + if (!(mounts = fopen("/proc/mounts", "r"))) + return false; + + while (getmntent_r(mounts, &entry, buf, sizeof(buf)) != NULL) { + if (STREQ(entry.mnt_type, "cgroup") && !strstr(entry.mnt_opts, "name=")) { + ret = true; + break; + } + } + + VIR_FORCE_FCLOSE(mounts); + return ret; +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1, + + .available = virCgroupV1Available, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

We need to update one test-case because now new cgroup object will be created only if there is any cgroup backend available.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 18 ++++++++++++++++++ src/util/vircgrouppriv.h | 3 +++ tests/vircgrouptest.c | 38 +++----------------------------------- 3 files changed, 24 insertions(+), 35 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index a5478a3fa4..0ffb5db93c 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -713,10 +713,28 @@ virCgroupDetect(virCgroupPtr group, virCgroupPtr parent) { int rc; + size_t i; + virCgroupBackendPtr *backends = virCgroupBackendGetAll();

VIR_DEBUG("group=%p controllers=%d path=%s parent=%p", group, controllers, path, parent);

+ if (!backends) + return -1; + + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (backends[i] && backends[i]->available()) { + group->backend = backends[i]; + break; + } + } + + if (!group->backend) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("no cgroup backend available")); + return -1; + } + if (parent) { if (virCgroupCopyMounts(group, parent) < 0) return -1; diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h index 046c96c52c..2caa966fee 100644 --- a/src/util/vircgrouppriv.h +++ b/src/util/vircgrouppriv.h @@ -30,6 +30,7 @@ # define __VIR_CGROUP_PRIV_H__

# include "vircgroup.h" +# include "vircgroupbackend.h"

struct _virCgroupController { int type; @@ -47,6 +48,8 @@ typedef virCgroupController *virCgroupControllerPtr; struct _virCgroup { char *path;

+ virCgroupBackendPtr backend; + virCgroupController controllers[VIR_CGROUP_CONTROLLER_LAST]; };

diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index bbbe6ffbe5..be3143ea52 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -114,16 +114,6 @@ const char *mountsAllInOne[VIR_CGROUP_CONTROLLER_LAST] = { [VIR_CGROUP_CONTROLLER_BLKIO] = "/not/really/sys/fs/cgroup", [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL, }; -const char *mountsLogind[VIR_CGROUP_CONTROLLER_LAST] = { - [VIR_CGROUP_CONTROLLER_CPU] = NULL, - [VIR_CGROUP_CONTROLLER_CPUACCT] = NULL, - [VIR_CGROUP_CONTROLLER_CPUSET] = NULL, - [VIR_CGROUP_CONTROLLER_MEMORY] = NULL, - [VIR_CGROUP_CONTROLLER_DEVICES] = NULL, - [VIR_CGROUP_CONTROLLER_FREEZER] = NULL, - [VIR_CGROUP_CONTROLLER_BLKIO] = NULL, - [VIR_CGROUP_CONTROLLER_SYSTEMD] = "/not/really/sys/fs/cgroup/sys temd", -};

const char *links[VIR_CGROUP_CONTROLLER_LAST] = { [VIR_CGROUP_CONTROLLER_CPU] = "/not/really/sys/fs/cgroup/cpu", @@ -147,17 +137,6 @@ const char *linksAllInOne[VIR_CGROUP_CONTROLLER_LAST] = { [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL, };

-const char *linksLogind[VIR_CGROUP_CONTROLLER_LAST] = { - [VIR_CGROUP_CONTROLLER_CPU] = NULL, - [VIR_CGROUP_CONTROLLER_CPUACCT] = NULL, - [VIR_CGROUP_CONTROLLER_CPUSET] = NULL, - [VIR_CGROUP_CONTROLLER_MEMORY] = NULL, - [VIR_CGROUP_CONTROLLER_DEVICES] = NULL, - [VIR_CGROUP_CONTROLLER_FREEZER] = NULL, - [VIR_CGROUP_CONTROLLER_BLKIO] = NULL, - [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL, -}; -

static int testCgroupDetectMounts(const void *args) @@ -541,24 +520,13 @@ static int testCgroupNewForSelfLogind(const void *args ATTRIBUTE_UNUSED) { virCgroupPtr cgroup = NULL; int ret = -1; - const char *placement[VIR_CGROUP_CONTROLLER_LAST] = { - [VIR_CGROUP_CONTROLLER_CPU] = NULL, - [VIR_CGROUP_CONTROLLER_CPUACCT] = NULL, - [VIR_CGROUP_CONTROLLER_CPUSET] = NULL, - [VIR_CGROUP_CONTROLLER_MEMORY] = NULL, - [VIR_CGROUP_CONTROLLER_DEVICES] = NULL, - [VIR_CGROUP_CONTROLLER_FREEZER] = NULL, - [VIR_CGROUP_CONTROLLER_BLKIO] = NULL, - [VIR_CGROUP_CONTROLLER_SYSTEMD] = "/", - };

- if (virCgroupNewSelf(&cgroup) < 0) { - fprintf(stderr, "Cannot create cgroup for self\n"); + if (virCgroupNewSelf(&cgroup) == 0) { + fprintf(stderr, "Expected cgroup creation to fail.\n");

Seems that code here would fit quite well in the last patch of the previous series in order to avoid the test failure introduced there.

goto cleanup; }

- ret = validateCgroup(cgroup, "", mountsLogind, linksLogind, placement); - + ret = 0; cleanup: virCgroupFree(&cgroup); return ret; -- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, Sep 20, 2018 at 10:03 AM, Pavel Hrdina <phrdina@redhat.com> wrote:

...

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

...

We need to update one test-case because now new cgroup object will be created only if there is any cgroup backend available.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 18 ++++++++++++++++++ src/util/vircgrouppriv.h | 3 +++ tests/vircgrouptest.c | 38 +++----------------------------------- 3 files changed, 24 insertions(+), 35 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index a5478a3fa4..0ffb5db93c 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -713,10 +713,28 @@ virCgroupDetect(virCgroupPtr group, virCgroupPtr parent) { int rc; + size_t i; + virCgroupBackendPtr *backends = virCgroupBackendGetAll();

VIR_DEBUG("group=%p controllers=%d path=%s parent=%p", group, controllers, path, parent);

+ if (!backends) + return -1; + + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (backends[i] && backends[i]->available()) { + group->backend = backends[i]; + break; + } + } + + if (!group->backend) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("no cgroup backend available")); + return -1; + } + if (parent) { if (virCgroupCopyMounts(group, parent) < 0) return -1; diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h index 046c96c52c..2caa966fee 100644 --- a/src/util/vircgrouppriv.h +++ b/src/util/vircgrouppriv.h @@ -30,6 +30,7 @@ # define __VIR_CGROUP_PRIV_H__

# include "vircgroup.h" +# include "vircgroupbackend.h"

struct _virCgroupController { int type; @@ -47,6 +48,8 @@ typedef virCgroupController *virCgroupControllerPtr; struct _virCgroup { char *path;

+ virCgroupBackendPtr backend; + virCgroupController controllers[VIR_CGROUP_CONTROLLER_LAST]; };

diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index bbbe6ffbe5..be3143ea52 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -114,16 +114,6 @@ const char *mountsAllInOne[VIR_CGROUP_ CONTROLLER_LAST] = { [VIR_CGROUP_CONTROLLER_BLKIO] = "/not/really/sys/fs/cgroup", [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL, }; -const char *mountsLogind[VIR_CGROUP_CONTROLLER_LAST] = { - [VIR_CGROUP_CONTROLLER_CPU] = NULL, - [VIR_CGROUP_CONTROLLER_CPUACCT] = NULL, - [VIR_CGROUP_CONTROLLER_CPUSET] = NULL, - [VIR_CGROUP_CONTROLLER_MEMORY] = NULL, - [VIR_CGROUP_CONTROLLER_DEVICES] = NULL, - [VIR_CGROUP_CONTROLLER_FREEZER] = NULL, - [VIR_CGROUP_CONTROLLER_BLKIO] = NULL, - [VIR_CGROUP_CONTROLLER_SYSTEMD] = "/not/really/sys/fs/cgroup/sys temd", -};

const char *links[VIR_CGROUP_CONTROLLER_LAST] = { [VIR_CGROUP_CONTROLLER_CPU] = "/not/really/sys/fs/cgroup/cpu", @@ -147,17 +137,6 @@ const char *linksAllInOne[VIR_CGROUP_ CONTROLLER_LAST] = { [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL, };

-const char *linksLogind[VIR_CGROUP_CONTROLLER_LAST] = { - [VIR_CGROUP_CONTROLLER_CPU] = NULL, - [VIR_CGROUP_CONTROLLER_CPUACCT] = NULL, - [VIR_CGROUP_CONTROLLER_CPUSET] = NULL, - [VIR_CGROUP_CONTROLLER_MEMORY] = NULL, - [VIR_CGROUP_CONTROLLER_DEVICES] = NULL, - [VIR_CGROUP_CONTROLLER_FREEZER] = NULL, - [VIR_CGROUP_CONTROLLER_BLKIO] = NULL, - [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL, -}; -

static int testCgroupDetectMounts(const void *args) @@ -541,24 +520,13 @@ static int testCgroupNewForSelfLogind(const void *args ATTRIBUTE_UNUSED) { virCgroupPtr cgroup = NULL; int ret = -1; - const char *placement[VIR_CGROUP_CONTROLLER_LAST] = { - [VIR_CGROUP_CONTROLLER_CPU] = NULL, - [VIR_CGROUP_CONTROLLER_CPUACCT] = NULL, - [VIR_CGROUP_CONTROLLER_CPUSET] = NULL, - [VIR_CGROUP_CONTROLLER_MEMORY] = NULL, - [VIR_CGROUP_CONTROLLER_DEVICES] = NULL, - [VIR_CGROUP_CONTROLLER_FREEZER] = NULL, - [VIR_CGROUP_CONTROLLER_BLKIO] = NULL, - [VIR_CGROUP_CONTROLLER_SYSTEMD] = "/", - };

- if (virCgroupNewSelf(&cgroup) < 0) { - fprintf(stderr, "Cannot create cgroup for self\n"); + if (virCgroupNewSelf(&cgroup) == 0) { + fprintf(stderr, "Expected cgroup creation to fail.\n");

Seems that code here would fit quite well in the last patch of the

On Thu, Sep 20, 2018 at 08:28:57AM +0200, Fabiano Fidêncio wrote: previous

...

series in order to avoid the test failure introduced there.

I'm not sure if I follow. This patch introduces a new restriction in the code that if no backend is available the creation of new cgroup will fail. In the previous series there is no such thing as backend.

I cannot move only the test code into different patch because the test suit would start failing.

The only thing I would be able to do is add the virCgroupAvailable() check into virCgroupDetect() without any backend code which would require the same test code change but still it would be separate patch, it would not be simple enough to fit into the last patch of the previous series.

Aha, I see! I'll leave it for you to decide what's the best way to handle the failure in the previous series and just wait for v2.

Pavel

...

...

goto cleanup; }

- ret = validateCgroup(cgroup, "", mountsLogind, linksLogind, placement); - + ret = 0; cleanup: virCgroupFree(&cgroup); return ret; -- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

...

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 23 +---------------------- src/util/vircgroupbackend.h | 5 +++++ src/util/vircgroupv1.c | 22 ++++++++++++++++++++++ 3 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 9d6eebc2b9..f5b2a14df9 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -237,27 +237,6 @@ virCgroupPartitionEscape(char **path) }

-static int -virCgroupCopyMounts(virCgroupPtr group, - virCgroupPtr parent) -{ - size_t i; - for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - if (!parent->controllers[i].mountPoint) - continue; - - if (VIR_STRDUP(group->controllers[i].mountPoint, - parent->controllers[i].mountPoint) < 0) - return -1; - - if (VIR_STRDUP(group->controllers[i].linkPoint, - parent->controllers[i].linkPoint) < 0) - return -1; - } - return 0; -} - - static int virCgroupResolveMountLink(const char *mntDir, const char *typeStr, @@ -655,7 +634,7 @@ virCgroupDetect(virCgroupPtr group, }

if (parent) { - if (virCgroupCopyMounts(group, parent) < 0) + if (group->backend->copyMounts(group, parent) < 0) return -1; } else { if (virCgroupDetectMounts(group) < 0) diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index daf47bac09..81ee597fc8 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -41,12 +41,17 @@ typedef bool const char *drivername, const char *machinename);

+typedef int +(*virCgroupCopyMountsCB)(virCgroupPtr group, + virCgroupPtr parent); + struct _virCgroupBackend { virCgroupBackendType type;

/* Mandatory callbacks that need to be implemented for every backend. */ virCgroupAvailableCB available; virCgroupValidateMachineGroupCB validateMachineGroup; + virCgroupCopyMountsCB copyMounts; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 48e3d412ae..00c3349aee 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -159,11 +159,33 @@ virCgroupV1ValidateMachineGroup(virCgroupPtr group, }

+static int +virCgroupV1CopyMounts(virCgroupPtr group, + virCgroupPtr parent) +{ + size_t i; + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + if (!parent->controllers[i].mountPoint) + continue; + + if (VIR_STRDUP(group->controllers[i].mountPoint, + parent->controllers[i].mountPoint) < 0) + return -1; + + if (VIR_STRDUP(group->controllers[i].linkPoint, + parent->controllers[i].linkPoint) < 0) + return -1; + } + return 0; +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

.available = virCgroupV1Available, .validateMachineGroup = virCgroupV1ValidateMachineGroup, + .copyMounts = virCgroupV1CopyMounts, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 138 ++----------------------------- src/util/vircgroupbackend.h | 14 ++++ src/util/vircgroupv1.c | 158 ++++++++++++++++++++++++++++++++++++ 3 files changed, 180 insertions(+), 130 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index f5b2a14df9..7557fc5576 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -237,82 +237,6 @@ virCgroupPartitionEscape(char **path) }

-static int -virCgroupResolveMountLink(const char *mntDir, - const char *typeStr, - virCgroupControllerPtr controller) -{ - VIR_AUTOFREE(char *) linkSrc = NULL; - VIR_AUTOFREE(char *) tmp = NULL; - char *dirName; - struct stat sb; - - if (VIR_STRDUP(tmp, mntDir) < 0) - return -1; - - dirName = strrchr(tmp, '/'); - if (!dirName) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Missing '/' separator in cgroup mount '%s'"), tmp); - return -1; - } - - if (!strchr(dirName + 1, ',')) - return 0; - - *dirName = '\0'; - if (virAsprintf(&linkSrc, "%s/%s", tmp, typeStr) < 0) - return -1; - *dirName = '/'; - - if (lstat(linkSrc, &sb) < 0) { - if (errno == ENOENT) { - VIR_WARN("Controller %s co-mounted at %s is missing symlink at %s", - typeStr, tmp, linkSrc); - } else { - virReportSystemError(errno, _("Cannot stat %s"), linkSrc); - return -1; - } - } else { - if (!S_ISLNK(sb.st_mode)) { - VIR_WARN("Expecting a symlink at %s for controller %s", - linkSrc, typeStr); - } else { - VIR_STEAL_PTR(controller->linkPoint, linkSrc); - } - } - - return 0; -} - - -static bool -virCgroupMountOptsMatchController(const char *mntOpts, - const char *typeStr) -{ - const char *tmp = mntOpts; - int typeLen = strlen(typeStr); - - while (tmp) { - const char *next = strchr(tmp, ','); - int len; - if (next) { - len = next - tmp; - next++; - } else { - len = strlen(tmp); - } - - if (typeLen == len && STREQLEN(typeStr, tmp, len)) - return true; - - tmp = next; - } - - return false; -} - - /* * Process /proc/mounts figuring out what controllers are * mounted and where @@ -320,7 +244,6 @@ virCgroupMountOptsMatchController(const char *mntOpts, static int virCgroupDetectMounts(virCgroupPtr group) { - size_t i; FILE *mounts = NULL; struct mntent entry; char buf[CGROUP_MAX_VAL]; @@ -333,34 +256,11 @@ virCgroupDetectMounts(virCgroupPtr group) }

while (getmntent_r(mounts, &entry, buf, sizeof(buf)) != NULL) { - if (STRNEQ(entry.mnt_type, "cgroup")) - continue; - - for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - const char *typestr = virCgroupControllerTypeToString(i); - - if (virCgroupMountOptsMatchController(entry.mnt_opts, typestr)) { - /* Note that the lines in /proc/mounts have the same - * order than the mount operations, and that there may - * be duplicates due to bind mounts. This means - * that the same mount point may be processed more than - * once. We need to save the results of the last one, - * and we need to be careful to release the memory used - * by previous processing. */ - virCgroupControllerPtr controller = &group->controllers[i]; - - VIR_FREE(controller->mountPoint); - VIR_FREE(controller->linkPoint); - if (VIR_STRDUP(controller->mountPoint, entry.mnt_dir) < 0) - goto cleanup; - - /* If it is a co-mount it has a filename like "cpu,cpuacct" - * and we must identify the symlink path */ - if (virCgroupResolveMountLink(entry.mnt_dir, typestr, - controller) < 0) { - goto cleanup; - } - } + if (group->backend->detectMounts(group, + entry.mnt_type, + entry.mnt_opts, + entry.mnt_dir) < 0) { + goto cleanup; } }

@@ -434,7 +334,6 @@ virCgroupDetectPlacement(virCgroupPtr group, pid_t pid, const char *path) { - size_t i; FILE *mapping = NULL; char line[1024]; int ret = -1; @@ -474,30 +373,9 @@ virCgroupDetectPlacement(virCgroupPtr group, controllers++; selfpath++;

- for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - const char *typestr = virCgroupControllerTypeToString(i); - - if (virCgroupMountOptsMatchController(controllers, typestr) && - group->controllers[i].mountPoint != NULL && - group->controllers[i].placement == NULL) { - /* - * selfpath == "/" + path="" -> "/" - * selfpath == "/libvirt.service" + path == "" -> "/libvirt.service" - * selfpath == "/libvirt.service" + path == "foo" -> "/libvirt.service/foo" - */ - 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; - } - } + if (group->backend->detectPlacement(group, path, controllers, + selfpath) < 0) { + goto cleanup; } }

diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 81ee597fc8..fadc7efdcf 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -45,6 +45,18 @@ typedef int (*virCgroupCopyMountsCB)(virCgroupPtr group, virCgroupPtr parent);

+typedef int +(*virCgroupDetectMountsCB)(virCgroupPtr group, + const char *mntType, + const char *mntOpts, + const char *mntDir); + +typedef int +(*virCgroupDetectPlacementCB)(virCgroupPtr group, + const char *path, + const char *controllers, + const char *selfpath); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -52,6 +64,8 @@ struct _virCgroupBackend { virCgroupAvailableCB available; virCgroupValidateMachineGroupCB validateMachineGroup; virCgroupCopyMountsCB copyMounts; + virCgroupDetectMountsCB detectMounts; + virCgroupDetectPlacementCB detectPlacement; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 00c3349aee..2882a19be2 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -23,6 +23,7 @@ #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R # include <mntent.h> #endif +#include <sys/stat.h>

#include "internal.h"

@@ -37,6 +38,7 @@ #include "virlog.h" #include "virstring.h" #include "virsystemd.h" +#include "virerror.h"

VIR_LOG_INIT("util.cgroup");

@@ -180,12 +182,168 @@ virCgroupV1CopyMounts(virCgroupPtr group, }

+static int +virCgroupV1ResolveMountLink(const char *mntDir, + const char *typeStr, + virCgroupControllerPtr controller) +{ + VIR_AUTOFREE(char *) linkSrc = NULL; + VIR_AUTOFREE(char *) tmp = NULL; + char *dirName; + struct stat sb; + + if (VIR_STRDUP(tmp, mntDir) < 0) + return -1; + + dirName = strrchr(tmp, '/'); + if (!dirName) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing '/' separator in cgroup mount '%s'"), tmp); + return -1; + } + + if (!strchr(dirName + 1, ',')) + return 0; + + *dirName = '\0'; + if (virAsprintf(&linkSrc, "%s/%s", tmp, typeStr) < 0) + return -1; + *dirName = '/'; + + if (lstat(linkSrc, &sb) < 0) { + if (errno == ENOENT) { + VIR_WARN("Controller %s co-mounted at %s is missing symlink at %s", + typeStr, tmp, linkSrc); + } else { + virReportSystemError(errno, _("Cannot stat %s"), linkSrc); + return -1; + } + } else { + if (!S_ISLNK(sb.st_mode)) { + VIR_WARN("Expecting a symlink at %s for controller %s", + linkSrc, typeStr); + } else { + VIR_STEAL_PTR(controller->linkPoint, linkSrc); + } + } + + return 0; +} + + +static bool +virCgroupV1MountOptsMatchController(const char *mntOpts, + const char *typeStr) +{ + const char *tmp = mntOpts; + int typeLen = strlen(typeStr); + + while (tmp) { + const char *next = strchr(tmp, ','); + int len; + if (next) { + len = next - tmp; + next++; + } else { + len = strlen(tmp); + } + + if (typeLen == len && STREQLEN(typeStr, tmp, len)) + return true; + + tmp = next; + } + + return false; +} + + +static int +virCgroupV1DetectMounts(virCgroupPtr group, + const char *mntType, + const char *mntOpts, + const char *mntDir) +{ + size_t i; + + if (STRNEQ(mntType, "cgroup")) + return 0; + + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + const char *typestr = virCgroupV1ControllerTypeToString(i); + + if (virCgroupV1MountOptsMatchController(mntOpts, typestr)) { + /* Note that the lines in /proc/mounts have the same + * order than the mount operations, and that there may + * be duplicates due to bind mounts. This means + * that the same mount point may be processed more than + * once. We need to save the results of the last one, + * and we need to be careful to release the memory used + * by previous processing. */ + virCgroupControllerPtr controller = &group->controllers[i]; + + VIR_FREE(controller->mountPoint); + VIR_FREE(controller->linkPoint); + if (VIR_STRDUP(controller->mountPoint, mntDir) < 0) + return -1; + + /* If it is a co-mount it has a filename like "cpu,cpuacct" + * and we must identify the symlink path */ + if (virCgroupV1ResolveMountLink(mntDir, typestr, controller) < 0) + return -1; + } + } + + return 0; +} + + +static int +virCgroupV1DetectPlacement(virCgroupPtr group, + const char *path, + const char *controllers, + const char *selfpath) +{ + size_t i; + + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + const char *typestr = virCgroupV1ControllerTypeToString(i); + + if (virCgroupV1MountOptsMatchController(controllers, typestr) && + group->controllers[i].mountPoint != NULL && + group->controllers[i].placement == NULL) { + /* + * selfpath == "/" + path="" -> "/" + * selfpath == "/libvirt.service" + path == "" -> "/libvirt.service" + * selfpath == "/libvirt.service" + path == "foo" -> "/libvirt.service/foo" + */ + if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) { + if (VIR_STRDUP(group->controllers[i].placement, + selfpath) < 0) + return -1; + } else { + if (virAsprintf(&group->controllers[i].placement, + "%s%s%s", selfpath, + (STREQ(selfpath, "/") || + STREQ(path, "") ? "" : "/"), + path) < 0) + return -1; + } + } + } + + return 0; +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

.available = virCgroupV1Available, .validateMachineGroup = virCgroupV1ValidateMachineGroup, .copyMounts = virCgroupV1CopyMounts, + .detectMounts = virCgroupV1DetectMounts, + .detectPlacement = virCgroupV1DetectPlacement, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 32 +------------------------------- src/util/vircgroupbackend.h | 5 +++++ src/util/vircgroupv1.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 0fcb047a0c..864dd876b0 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -351,36 +351,6 @@ virCgroupDetectPlacement(virCgroupPtr group, }

-static int -virCgroupValidatePlacement(virCgroupPtr group, - pid_t pid) -{ - size_t i; - - for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - if (!group->controllers[i].mountPoint) - continue; - - if (!group->controllers[i].placement) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not find placement for controller %s at %s"), - virCgroupControllerTypeToString(i), - group->controllers[i].placement); - return -1; - } - - VIR_DEBUG("Detected mount/mapping %zu:%s at %s in %s for pid %lld", - i, - virCgroupControllerTypeToString(i), - group->controllers[i].mountPoint, - group->controllers[i].placement, - (long long) pid); - } - - return 0; -} - - static int virCgroupDetectControllers(virCgroupPtr group, int controllers) @@ -506,7 +476,7 @@ virCgroupDetect(virCgroupPtr group, return -1;

/* Check that for every mounted controller, we found our placement */ - if (virCgroupValidatePlacement(group, pid) < 0) + if (group->backend->validatePlacement(group, pid) < 0) return -1;

return 0; diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 85906e7191..086cfb8090 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -62,6 +62,10 @@ typedef int const char *controllers, const char *selfpath);

+typedef int +(*virCgroupValidatePlacementCB)(virCgroupPtr group, + pid_t pid); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -72,6 +76,7 @@ struct _virCgroupBackend { virCgroupCopyPlacementCB copyPlacement; virCgroupDetectMountsCB detectMounts; virCgroupDetectPlacementCB detectPlacement; + virCgroupValidatePlacementCB validatePlacement; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index f73f40db0e..68df323d72 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -372,6 +372,36 @@ virCgroupV1DetectPlacement(virCgroupPtr group, }

+static int +virCgroupV1ValidatePlacement(virCgroupPtr group, + pid_t pid) +{ + size_t i; + + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + if (!group->controllers[i].mountPoint) + continue; + + if (!group->controllers[i].placement) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not find placement for v1 controller %s at %s"), + virCgroupV1ControllerTypeToString(i), + group->controllers[i].placement); + return -1; + } + + VIR_DEBUG("Detected mount/mapping %zu:%s at %s in %s for pid %lld", + i, + virCgroupV1ControllerTypeToString(i), + group->controllers[i].mountPoint, + group->controllers[i].placement, + (long long) pid); + } + + return 0; +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -381,6 +411,7 @@ virCgroupBackend virCgroupV1Backend = { .copyPlacement = virCgroupV1CopyPlacement, .detectMounts = virCgroupV1DetectMounts, .detectPlacement = virCgroupV1DetectPlacement, + .validatePlacement = virCgroupV1ValidatePlacement, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 05:45:31PM +0200, Pavel Hrdina wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 3 +-- src/util/vircgroupbackend.h | 4 ++++ src/util/vircgroupv1.c | 12 ++++++++++++ 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 864dd876b0..ff102e2cb0 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1317,8 +1317,7 @@ virCgroupNewMachineSystemd(const char *name, &init) < 0) return -1;

- path = init->controllers[VIR_CGROUP_CONTROLLER_SYSTEMD].placement; - init->controllers[VIR_CGROUP_CONTROLLER_SYSTEMD].placement = NULL; + path = init->backend->getPlacement(init); virCgroupFree(&init);

if (!path || STREQ(path, "/") || path[0] != '/') { diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 086cfb8090..1856164c6b 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -66,6 +66,9 @@ typedef int (*virCgroupValidatePlacementCB)(virCgroupPtr group, pid_t pid);

+typedef char * +(*virCgroupGetPlacementCB)(virCgroupPtr group); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -77,6 +80,7 @@ struct _virCgroupBackend { virCgroupDetectMountsCB detectMounts; virCgroupDetectPlacementCB detectPlacement; virCgroupValidatePlacementCB validatePlacement; + virCgroupGetPlacementCB getPlacement; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 68df323d72..ab8988cd59 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -402,6 +402,17 @@ virCgroupV1ValidatePlacement(virCgroupPtr group, }

+static char * +virCgroupV1GetPlacement(virCgroupPtr group) +{ + char *ret = NULL; + + ignore_value(VIR_STRDUP(ret, group->controllers[VIR_CGROUP_CONTROLLER_SYSTEMD].placement));

This ignores a potential OOM error which could not have happened before. I'd suggest calling this virCgroupV1StealPlacement and using VIR_STEAL_PTR here. Alternatively, propagate the return value of VIR_STRDUP. Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 66 +------------------------------------ src/util/vircgroupbackend.h | 5 +++ src/util/vircgroupv1.c | 65 ++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 65 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index ff102e2cb0..7822eec683 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -351,70 +351,6 @@ virCgroupDetectPlacement(virCgroupPtr group, }

-static int -virCgroupDetectControllers(virCgroupPtr group, - int controllers) -{ - size_t i; - size_t j; - - if (controllers >= 0) { - VIR_DEBUG("Filtering controllers %d", controllers); - /* First mark requested but non-existing controllers to be ignored */ - for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - if (((1 << i) & controllers)) { - /* Remove non-existent controllers */ - if (!group->controllers[i].mountPoint) { - VIR_DEBUG("Requested controller '%s' not mounted, ignoring", - virCgroupControllerTypeToString(i)); - controllers &= ~(1 << i); - } - } - } - for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - VIR_DEBUG("Controller '%s' wanted=%s, mount='%s'", - virCgroupControllerTypeToString(i), - (1 << i) & controllers ? "yes" : "no", - NULLSTR(group->controllers[i].mountPoint)); - if (!((1 << i) & controllers) && - group->controllers[i].mountPoint) { - /* Check whether a request to disable a controller - * clashes with co-mounting of controllers */ - for (j = 0; j < VIR_CGROUP_CONTROLLER_LAST; j++) { - if (j == i) - continue; - if (!((1 << j) & controllers)) - continue; - - if (STREQ_NULLABLE(group->controllers[i].mountPoint, - group->controllers[j].mountPoint)) { - virReportSystemError(EINVAL, - _("Controller '%s' is not wanted, but '%s' is co-mounted"), - virCgroupControllerTypeToString(i), - virCgroupControllerTypeToString(j)); - return -1; - } - } - VIR_FREE(group->controllers[i].mountPoint); - } - } - } else { - VIR_DEBUG("Auto-detecting controllers"); - controllers = 0; - for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - VIR_DEBUG("Controller '%s' present=%s", - virCgroupControllerTypeToString(i), - group->controllers[i].mountPoint ? "yes" : "no"); - if (group->controllers[i].mountPoint == NULL) - continue; - controllers |= (1 << i); - } - } - - return controllers; -} - - static int virCgroupDetect(virCgroupPtr group, pid_t pid, @@ -453,7 +389,7 @@ virCgroupDetect(virCgroupPtr group, return -1; }

- rc = virCgroupDetectControllers(group, controllers); + rc = group->backend->detectControllers(group, controllers); if (rc < 0) return -1;

diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 1856164c6b..31be70688c 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -69,6 +69,10 @@ typedef int typedef char * (*virCgroupGetPlacementCB)(virCgroupPtr group);

+typedef int +(*virCgroupDetectControllersCB)(virCgroupPtr group, + int controllers); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -81,6 +85,7 @@ struct _virCgroupBackend { virCgroupDetectPlacementCB detectPlacement; virCgroupValidatePlacementCB validatePlacement; virCgroupGetPlacementCB getPlacement; + virCgroupDetectControllersCB detectControllers; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index ab8988cd59..4dbb3020f5 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -413,6 +413,70 @@ virCgroupV1GetPlacement(virCgroupPtr group) }

+static int +virCgroupV1DetectControllers(virCgroupPtr group, + int controllers) +{ + size_t i; + size_t j; + + if (controllers >= 0) { + VIR_DEBUG("Filtering controllers %d", controllers); + /* First mark requested but non-existing controllers to be ignored */ + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + if (((1 << i) & controllers)) { + /* Remove non-existent controllers */ + if (!group->controllers[i].mountPoint) { + VIR_DEBUG("Requested controller '%s' not mounted, ignoring", + virCgroupV1ControllerTypeToString(i)); + controllers &= ~(1 << i); + } + } + } + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + VIR_DEBUG("Controller '%s' wanted=%s, mount='%s'", + virCgroupV1ControllerTypeToString(i), + (1 << i) & controllers ? "yes" : "no", + NULLSTR(group->controllers[i].mountPoint)); + if (!((1 << i) & controllers) && + group->controllers[i].mountPoint) { + /* Check whether a request to disable a controller + * clashes with co-mounting of controllers */ + for (j = 0; j < VIR_CGROUP_CONTROLLER_LAST; j++) { + if (j == i) + continue; + if (!((1 << j) & controllers)) + continue; + + if (STREQ_NULLABLE(group->controllers[i].mountPoint, + group->controllers[j].mountPoint)) { + virReportSystemError(EINVAL, + _("V1 controller '%s' is not wanted, but '%s' is co-mounted"), + virCgroupV1ControllerTypeToString(i), + virCgroupV1ControllerTypeToString(j)); + return -1; + } + } + VIR_FREE(group->controllers[i].mountPoint); + } + } + } else { + VIR_DEBUG("Auto-detecting controllers"); + controllers = 0; + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + VIR_DEBUG("Controller '%s' present=%s", + virCgroupV1ControllerTypeToString(i), + group->controllers[i].mountPoint ? "yes" : "no"); + if (group->controllers[i].mountPoint == NULL) + continue; + controllers |= (1 << i); + } + } + + return controllers; +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -424,6 +488,7 @@ virCgroupBackend virCgroupV1Backend = { .detectPlacement = virCgroupV1DetectPlacement, .validatePlacement = virCgroupV1ValidatePlacement, .getPlacement = virCgroupV1GetPlacement, + .detectControllers = virCgroupV1DetectControllers, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 22 +--------------------- src/util/vircgroupbackend.h | 7 +++++++ src/util/vircgroupv1.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 817ee8f6ca..55122a5ab9 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1454,27 +1454,7 @@ virCgroupPathOfController(virCgroupPtr group, return -1; }

- if (group->controllers[controller].mountPoint == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Controller '%s' is not mounted"), - virCgroupControllerTypeToString(controller)); - return -1; - } - - if (group->controllers[controller].placement == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Controller '%s' is not enabled for group"), - virCgroupControllerTypeToString(controller)); - return -1; - } - - if (virAsprintf(path, "%s%s/%s", - group->controllers[controller].mountPoint, - group->controllers[controller].placement, - key ? key : "") < 0) - return -1; - - return 0; + return group->backend->pathOfController(group, controller, key, path); }

diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 933f59151a..cf000529c4 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -80,6 +80,12 @@ typedef bool typedef int (*virCgroupGetAnyControllerCB)(virCgroupPtr group);

+typedef int +(*virCgroupPathOfControllerCB)(virCgroupPtr group, + int controller, + const char *key, + char **path); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -95,6 +101,7 @@ struct _virCgroupBackend { virCgroupDetectControllersCB detectControllers; virCgroupHasControllerCB hasController; virCgroupGetAnyControllerCB getAnyController; + virCgroupPathOfControllerCB pathOfController; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 105024ddde..7d92150dc3 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -506,6 +506,36 @@ virCgroupV1GetAnyController(virCgroupPtr group) }

+static int +virCgroupV1PathOfController(virCgroupPtr group, + int controller, + const char *key, + char **path) +{ + if (group->controllers[controller].mountPoint == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("v1 controller '%s' is not mounted"), + virCgroupV1ControllerTypeToString(controller)); + return -1; + } + + if (group->controllers[controller].placement == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("v1 controller '%s' is not enabled for group"), + virCgroupV1ControllerTypeToString(controller)); + return -1; + } + + if (virAsprintf(path, "%s%s/%s", + group->controllers[controller].mountPoint, + group->controllers[controller].placement, + key ? key : "") < 0) + return -1; + + return 0; +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -520,6 +550,7 @@ virCgroupBackend virCgroupV1Backend = { .detectControllers = virCgroupV1DetectControllers, .hasController = virCgroupV1HasController, .getAnyController = virCgroupV1GetAnyController, + .pathOfController = virCgroupV1PathOfController, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 35 ++------------------------------- src/util/vircgroupbackend.h | 4 ++++ src/util/vircgrouppriv.h | 2 ++ src/util/vircgroupv1.c | 39 +++++++++++++++++++++++++++++++++++++ 4 files changed, 47 insertions(+), 33 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 8083e4596d..37bfd7893c 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2917,7 +2917,7 @@ virCgroupGetCpuacctPercpuUsage(virCgroupPtr group, char **usage) }

-static int +int virCgroupRemoveRecursively(char *grppath) { DIR *grpdir; @@ -2980,38 +2980,7 @@ virCgroupRemoveRecursively(char *grppath) int virCgroupRemove(virCgroupPtr group) { - int rc = 0; - size_t i; - - VIR_DEBUG("Removing cgroup %s", group->path); - for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - VIR_AUTOFREE(char *) grppath = NULL; - - /* Skip over controllers not mounted */ - if (!group->controllers[i].mountPoint) - continue; - - /* We must never rmdir() in systemd's hierarchy */ - 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, "/")) - continue; - - if (virCgroupPathOfController(group, - i, - NULL, - &grppath) != 0) - continue; - - VIR_DEBUG("Removing cgroup %s and all child cgroups", grppath); - rc = virCgroupRemoveRecursively(grppath); - } - VIR_DEBUG("Done removing cgroup %s", group->path); - - return rc; + return group->backend->remove(group); }

diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 214c9f0726..11ed8431cf 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -100,6 +100,9 @@ typedef int bool create, unsigned int flags);

+typedef int +(*virCgroupRemoveCB)(virCgroupPtr group); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -117,6 +120,7 @@ struct _virCgroupBackend { virCgroupGetAnyControllerCB getAnyController; virCgroupPathOfControllerCB pathOfController; virCgroupMakeGroupCB makeGroup; + virCgroupRemoveCB remove; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h index 2e731458d5..a760b9bcfd 100644 --- a/src/util/vircgrouppriv.h +++ b/src/util/vircgrouppriv.h @@ -88,4 +88,6 @@ int virCgroupNewDomainPartition(virCgroupPtr partition, virCgroupPtr *group) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5);

+int virCgroupRemoveRecursively(char *grppath); + #endif /* __VIR_CGROUP_PRIV_H__ */ diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index cf484216cc..25afd39235 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -667,6 +667,44 @@ virCgroupV1MakeGroup(virCgroupPtr parent, }

+static int +virCgroupV1Remove(virCgroupPtr group) +{ + int rc = 0; + size_t i; + + VIR_DEBUG("Removing cgroup %s", group->path); + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + VIR_AUTOFREE(char *) grppath = NULL; + + /* Skip over controllers not mounted */ + if (!group->controllers[i].mountPoint) + continue; + + /* We must never rmdir() in systemd's hierarchy */ + 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, "/")) + continue; + + if (virCgroupV1PathOfController(group, + i, + NULL, + &grppath) != 0) + continue; + + VIR_DEBUG("Removing cgroup %s and all child cgroups", grppath); + rc = virCgroupRemoveRecursively(grppath); + } + VIR_DEBUG("Done removing cgroup %s", group->path); + + return rc; +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -683,6 +721,7 @@ virCgroupBackend virCgroupV1Backend = { .getAnyController = virCgroupV1GetAnyController, .pathOfController = virCgroupV1PathOfController, .makeGroup = virCgroupV1MakeGroup, + .remove = virCgroupV1Remove, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 13 +------------ src/util/vircgroupbackend.h | 5 +++++ src/util/vircgroupv1.c | 20 ++++++++++++++++++++ 3 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 5dd736930b..f5ae23836f 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3467,18 +3467,7 @@ virCgroupSupportsCpuBW(virCgroupPtr cgroup) int virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller) { - int ret = -1; - VIR_AUTOFREE(char *) content = NULL; - - if (!cgroup) - return -1; - - ret = virCgroupGetValueStr(cgroup, controller, "tasks", &content); - - if (ret == 0 && content[0] == '\0') - ret = 1; - - return ret; + return cgroup->backend->hasEmptyTasks(cgroup, controller); }

bool diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 4661d82132..1964a48ff0 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -114,6 +114,10 @@ typedef int pid_t pid, unsigned int flags);

+typedef int +(*virCgroupHasEmptyTasksCB)(virCgroupPtr cgroup, + int controller); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -133,6 +137,7 @@ struct _virCgroupBackend { virCgroupMakeGroupCB makeGroup; virCgroupRemoveCB remove; virCgroupAddTaskCB addTask; + virCgroupHasEmptyTasksCB hasEmptyTasks; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index c9f6fd925a..57bcb6a685 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -735,6 +735,25 @@ virCgroupV1AddTask(virCgroupPtr group, }

+static int +virCgroupV1HasEmptyTasks(virCgroupPtr cgroup, + int controller) +{ + int ret = -1; + VIR_AUTOFREE(char *) content = NULL; + + if (!cgroup) + return -1; + + ret = virCgroupGetValueStr(cgroup, controller, "tasks", &content); + + if (ret == 0 && content[0] == '\0') + ret = 1; + + return ret; +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -753,6 +772,7 @@ virCgroupBackend virCgroupV1Backend = { .makeGroup = virCgroupV1MakeGroup, .remove = virCgroupV1Remove, .addTask = virCgroupV1AddTask, + .hasEmptyTasks = virCgroupV1HasEmptyTasks, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 54 +----------------------------- src/util/vircgroupbackend.h | 7 ++++ src/util/vircgroupv1.c | 65 +++++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 53 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 29faf5fb2c..479a2bf664 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3284,59 +3284,7 @@ int virCgroupSetOwner(virCgroupPtr cgroup, gid_t gid, int controllers) { - int ret = -1; - size_t i; - DIR *dh = NULL; - int direrr; - - for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - VIR_AUTOFREE(char *) base = NULL; - struct dirent *de; - - if (!((1 << i) & controllers)) - continue; - - if (!cgroup->controllers[i].mountPoint) - continue; - - if (virAsprintf(&base, "%s%s", cgroup->controllers[i].mountPoint, - cgroup->controllers[i].placement) < 0) - goto cleanup; - - if (virDirOpen(&dh, base) < 0) - goto cleanup; - - while ((direrr = virDirRead(dh, &de, base)) > 0) { - VIR_AUTOFREE(char *) entry = NULL; - - if (virAsprintf(&entry, "%s/%s", base, de->d_name) < 0) - goto cleanup; - - if (chown(entry, uid, gid) < 0) { - virReportSystemError(errno, - _("cannot chown '%s' to (%u, %u)"), - entry, uid, gid); - goto cleanup; - } - } - if (direrr < 0) - goto cleanup; - - if (chown(base, uid, gid) < 0) { - virReportSystemError(errno, - _("cannot chown '%s' to (%u, %u)"), - base, uid, gid); - goto cleanup; - } - - VIR_DIR_CLOSE(dh); - } - - ret = 0; - - cleanup: - VIR_DIR_CLOSE(dh); - return ret; + return cgroup->backend->setOwner(cgroup, uid, gid, controllers); }

diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 70deb47461..4538654068 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -123,6 +123,12 @@ typedef int const char *oldroot, const char *mountopts);

+typedef int +(*virCgroupSetOwnerCB)(virCgroupPtr cgroup, + uid_t uid, + gid_t gid, + int controllers); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -144,6 +150,7 @@ struct _virCgroupBackend { virCgroupAddTaskCB addTask; virCgroupHasEmptyTasksCB hasEmptyTasks; virCgroupBindMountCB bindMount; + virCgroupSetOwnerCB setOwner; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 0514ba392d..d74fc8ff96 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -866,6 +866,70 @@ virCgroupV1BindMount(virCgroupPtr group, }

+static int +virCgroupV1SetOwner(virCgroupPtr cgroup, + uid_t uid, + gid_t gid, + int controllers) +{ + int ret = -1; + size_t i; + VIR_AUTOFREE(char *) base = NULL; + VIR_AUTOFREE(char *) entry = NULL;

I would avoid these changes here in order to keep the diff as minimal as possible.

+ DIR *dh = NULL; + int direrr; + + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + struct dirent *de; + + if (!((1 << i) & controllers)) + continue; + + if (!cgroup->controllers[i].mountPoint) + continue; + + if (virAsprintf(&base, "%s%s", cgroup->controllers[i].mountPoint, + cgroup->controllers[i].placement) < 0) + goto cleanup; + + if (virDirOpen(&dh, base) < 0) + goto cleanup; + + while ((direrr = virDirRead(dh, &de, base)) > 0) { + if (virAsprintf(&entry, "%s/%s", base, de->d_name) < 0) + goto cleanup; + + if (chown(entry, uid, gid) < 0) { + virReportSystemError(errno, + _("cannot chown '%s' to (%u, %u)"), + entry, uid, gid); + goto cleanup; + } + + VIR_FREE(entry); + } + if (direrr < 0) + goto cleanup; + + if (chown(base, uid, gid) < 0) { + virReportSystemError(errno, + _("cannot chown '%s' to (%u, %u)"), + base, uid, gid); + goto cleanup; + } + + VIR_FREE(base); + VIR_DIR_CLOSE(dh); + } + + ret = 0; + + cleanup: + VIR_DIR_CLOSE(dh); + return ret; +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -886,6 +950,7 @@ virCgroupBackend virCgroupV1Backend = { .addTask = virCgroupV1AddTask, .hasEmptyTasks = virCgroupV1HasEmptyTasks, .bindMount = virCgroupV1BindMount, + .setOwner = virCgroupV1SetOwner, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 14 ++------------ src/util/vircgroupbackend.h | 20 ++++++++++++++++++++ src/util/vircgroupv1.c | 29 +++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 479a2bf664..cb9f03d488 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1512,10 +1512,7 @@ virCgroupGetBlkioIoDeviceServiced(virCgroupPtr group, int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight) { - return virCgroupSetValueU64(group, - VIR_CGROUP_CONTROLLER_BLKIO, - "blkio.weight", - weight); + VIR_CGROUP_BACKEND_CALL(group, setBlkioWeight, -1, weight); }

@@ -1530,14 +1527,7 @@ virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight) int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight) { - unsigned long long tmp; - int ret; - ret = virCgroupGetValueU64(group, - VIR_CGROUP_CONTROLLER_BLKIO, - "blkio.weight", &tmp); - if (ret == 0) - *weight = tmp; - return ret; + VIR_CGROUP_BACKEND_CALL(group, getBlkioWeight, -1, weight); }

/** diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 4538654068..2c42227297 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -129,6 +129,14 @@ typedef int gid_t gid, int controllers);

+typedef int +(*virCgroupSetBlkioWeightCB)(virCgroupPtr group, + unsigned int weight); + +typedef int +(*virCgroupGetBlkioWeightCB)(virCgroupPtr group, + unsigned int *weight); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -151,6 +159,10 @@ struct _virCgroupBackend { virCgroupHasEmptyTasksCB hasEmptyTasks; virCgroupBindMountCB bindMount; virCgroupSetOwnerCB setOwner; + + /* Optional cgroup controller specific callbacks. */ + virCgroupSetBlkioWeightCB setBlkioWeight; + virCgroupGetBlkioWeightCB getBlkioWeight; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; @@ -161,4 +173,12 @@ virCgroupBackendRegister(virCgroupBackendPtr backend); virCgroupBackendPtr * virCgroupBackendGetAll(void);

+# define VIR_CGROUP_BACKEND_CALL(group, func, ret, ...) \ + if (!group->backend->func) { \ + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", \ + _("operation '" #func "' not supported")); \ + return ret; \ + } \ + return group->backend->func(group, ##__VA_ARGS__); + #endif /* __VIR_CGROUP_BACKEND_H__ */ diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index d74fc8ff96..380a310589 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -930,6 +930,32 @@ virCgroupV1SetOwner(virCgroupPtr cgroup, }

+static int +virCgroupV1SetBlkioWeight(virCgroupPtr group, + unsigned int weight) +{ + return virCgroupSetValueU64(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "blkio.weight", + weight); +} + + +static int +virCgroupV1GetBlkioWeight(virCgroupPtr group, + unsigned int *weight) +{ + unsigned long long tmp; + int ret; + ret = virCgroupGetValueU64(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "blkio.weight", &tmp); + if (ret == 0) + *weight = tmp; + return ret; +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -951,6 +977,9 @@ virCgroupBackend virCgroupV1Backend = { .hasEmptyTasks = virCgroupV1HasEmptyTasks, .bindMount = virCgroupV1BindMount, .setOwner = virCgroupV1SetOwner, + + .setBlkioWeight = virCgroupV1SetBlkioWeight, + .getBlkioWeight = virCgroupV1GetBlkioWeight, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 05:45:43PM +0200, Pavel Hrdina wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 87 ++-------------------------------- src/util/vircgroupbackend.h | 8 ++++ src/util/vircgroupv1.c | 94 +++++++++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 84 deletions(-)

diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 380a310589..4b9c03830c 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -956,6 +956,99 @@ virCgroupV1GetBlkioWeight(virCgroupPtr group, }

+static int +virCgroupV1GetBlkioIoServiced(virCgroupPtr group, + long long *bytes_read, + long long *bytes_write, + long long *requests_read, + long long *requests_write) +{ + long long stats_val; + VIR_AUTOFREE(char *) str1 = NULL; + VIR_AUTOFREE(char *) str2 = NULL;

+ char *p1, *p2;

These were written as:

- char *p1 = NULL; - char *p2 = NULL; in the original function. Perhaps incorrect conflict resolution with commit 94f1855f util: cgroup: use VIR_AUTOFREE instead of VIR_FREE for scalar types

+ size_t i; + + const char *value_names[] = { + "Read ", + "Write " + };

With that addressed: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 83 ++-------------------------------- src/util/vircgroupbackend.h | 9 ++++ src/util/vircgrouppriv.h | 2 + src/util/vircgroupv1.c | 89 +++++++++++++++++++++++++++++++++++++ 4 files changed, 104 insertions(+), 79 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index a9b67f7d53..0d93860df6 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -411,7 +411,7 @@ virCgroupDetect(virCgroupPtr group, }

-static char * +char * virCgroupGetBlockDevString(const char *path) { char *ret = NULL; @@ -1339,84 +1339,9 @@ virCgroupGetBlkioIoDeviceServiced(virCgroupPtr group, long long *requests_read, long long *requests_write) { - VIR_AUTOFREE(char *) str1 = NULL; - VIR_AUTOFREE(char *) str2 = NULL; - VIR_AUTOFREE(char *) str3 = NULL; - char *p1 = NULL; - char *p2 = NULL; - size_t i; - - const char *value_names[] = { - "Read ", - "Write " - }; - long long *bytes_ptrs[] = { - bytes_read, - bytes_write - }; - long long *requests_ptrs[] = { - requests_read, - requests_write - }; - - if (virCgroupGetValueStr(group, - VIR_CGROUP_CONTROLLER_BLKIO, - "blkio.throttle.io_service_bytes", &str1) < 0) - return -1; - - if (virCgroupGetValueStr(group, - VIR_CGROUP_CONTROLLER_BLKIO, - "blkio.throttle.io_serviced", &str2) < 0) - return -1; - - if (!(str3 = virCgroupGetBlockDevString(path))) - return -1; - - if (!(p1 = strstr(str1, str3))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot find byte stats for block device '%s'"), - str3); - return -1; - } - - if (!(p2 = strstr(str2, str3))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot find request stats for block device '%s'"), - str3); - return -1; - } - - for (i = 0; i < ARRAY_CARDINALITY(value_names); i++) { - if (!(p1 = strstr(p1, value_names[i]))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot find byte %sstats for block device '%s'"), - value_names[i], str3); - return -1; - } - - if (virStrToLong_ll(p1 + strlen(value_names[i]), &p1, 10, bytes_ptrs[i]) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot parse %sstat '%s'"), - value_names[i], p1 + strlen(value_names[i])); - return -1; - } - - if (!(p2 = strstr(p2, value_names[i]))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot find request %sstats for block device '%s'"), - value_names[i], str3); - return -1; - } - - if (virStrToLong_ll(p2 + strlen(value_names[i]), &p2, 10, requests_ptrs[i]) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot parse %sstat '%s'"), - value_names[i], p2 + strlen(value_names[i])); - return -1; - } - } - - return 0; + VIR_CGROUP_BACKEND_CALL(group, getBlkioIoDeviceServiced, -1, + path, bytes_read, bytes_write, + requests_read, requests_write); }

diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 90584947fd..bd78c5b937 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -144,6 +144,14 @@ typedef int long long *requests_read, long long *requests_write);

+typedef int +(*virCgroupGetBlkioIoDeviceServicedCB)(virCgroupPtr group, + const char *path, + long long *bytes_read, + long long *bytes_write, + long long *requests_read, + long long *requests_write); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -171,6 +179,7 @@ struct _virCgroupBackend { virCgroupSetBlkioWeightCB setBlkioWeight; virCgroupGetBlkioWeightCB getBlkioWeight; virCgroupGetBlkioIoServicedCB getBlkioIoServiced; + virCgroupGetBlkioIoDeviceServicedCB getBlkioIoDeviceServiced; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h index 01714370be..525c288a9a 100644 --- a/src/util/vircgrouppriv.h +++ b/src/util/vircgrouppriv.h @@ -80,6 +80,8 @@ int virCgroupSetValueI64(virCgroupPtr group,

int virCgroupPartitionEscape(char **path);

+char *virCgroupGetBlockDevString(const char *path); + int virCgroupNewPartition(const char *path, bool create, int controllers, diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 4b9c03830c..206318d436 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1049,6 +1049,94 @@ virCgroupV1GetBlkioIoServiced(virCgroupPtr group, }

+static int +virCgroupV1GetBlkioIoDeviceServiced(virCgroupPtr group, + const char *path, + long long *bytes_read, + long long *bytes_write, + long long *requests_read, + long long *requests_write) +{ + VIR_AUTOFREE(char *) str1 = NULL; + VIR_AUTOFREE(char *) str2 = NULL; + VIR_AUTOFREE(char *) str3 = NULL; + char *p1, *p2; + size_t i; + + const char *value_names[] = { + "Read ", + "Write " + }; + long long *bytes_ptrs[] = { + bytes_read, + bytes_write + }; + long long *requests_ptrs[] = { + requests_read, + requests_write + }; + + if (virCgroupGetValueStr(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "blkio.throttle.io_service_bytes", &str1) < 0) + return -1; + + if (virCgroupGetValueStr(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "blkio.throttle.io_serviced", &str2) < 0) + return -1; + + if (!(str3 = virCgroupGetBlockDevString(path))) + return -1; + + if (!(p1 = strstr(str1, str3))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot find byte stats for block device '%s'"), + str3); + return -1; + } + + if (!(p2 = strstr(str2, str3))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot find request stats for block device '%s'"), + str3); + return -1; + } + + for (i = 0; i < ARRAY_CARDINALITY(value_names); i++) { + if (!(p1 = strstr(p1, value_names[i]))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot find byte %sstats for block device '%s'"), + value_names[i], str3); + return -1; + } + + if (virStrToLong_ll(p1 + strlen(value_names[i]), &p1, 10, bytes_ptrs[i]) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot parse %sstat '%s'"), + value_names[i], p1 + strlen(value_names[i])); + return -1; + } + + if (!(p2 = strstr(p2, value_names[i]))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot find request %sstats for block device '%s'"), + value_names[i], str3); + return -1; + } + + if (virStrToLong_ll(p2 + strlen(value_names[i]), &p2, 10, requests_ptrs[i]) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot parse %sstat '%s'"), + value_names[i], p2 + strlen(value_names[i])); + return -1; + } + } + + return 0; +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -1074,6 +1162,7 @@ virCgroupBackend virCgroupV1Backend = { .setBlkioWeight = virCgroupV1SetBlkioWeight, .getBlkioWeight = virCgroupV1GetBlkioWeight, .getBlkioIoServiced = virCgroupV1GetBlkioIoServiced, + .getBlkioIoDeviceServiced = virCgroupV1GetBlkioIoDeviceServiced, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 34 ++----------------------- src/util/vircgroupbackend.h | 12 +++++++++ src/util/vircgroupv1.c | 50 +++++++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 32 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 465d540bb2..5fbad25d5c 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1387,19 +1387,7 @@ virCgroupSetBlkioDeviceReadIops(virCgroupPtr group, const char *path, unsigned int riops) { - VIR_AUTOFREE(char *) str = NULL; - VIR_AUTOFREE(char *) blkstr = NULL; - - if (!(blkstr = virCgroupGetBlockDevString(path))) - return -1; - - if (virAsprintf(&str, "%s%u", blkstr, riops) < 0) - return -1; - - return virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_BLKIO, - "blkio.throttle.read_iops_device", - str); + VIR_CGROUP_BACKEND_CALL(group, setBlkioDeviceReadIops, -1, path, riops); }

@@ -1519,25 +1507,7 @@ virCgroupGetBlkioDeviceReadIops(virCgroupPtr group, const char *path, unsigned int *riops) { - VIR_AUTOFREE(char *) str = NULL; - - if (virCgroupGetValueForBlkDev(group, - VIR_CGROUP_CONTROLLER_BLKIO, - "blkio.throttle.read_iops_device", - path, - &str) < 0) - return -1; - - if (!str) { - *riops = 0; - } else if (virStrToLong_ui(str, NULL, 10, riops) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to parse '%s' as an integer"), - str); - return -1; - } - - return 0; + VIR_CGROUP_BACKEND_CALL(group, getBlkioDeviceReadIops, -1, path, riops); }

/** diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index a84417ddb7..813ed61b6a 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -162,6 +162,16 @@ typedef int const char *path, unsigned int *weight);

+typedef int +(*virCgroupSetBlkioDeviceReadIopsCB)(virCgroupPtr group, + const char *path, + unsigned int riops); + +typedef int +(*virCgroupGetBlkioDeviceReadIopsCB)(virCgroupPtr group, + const char *path, + unsigned int *riops); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -192,6 +202,8 @@ struct _virCgroupBackend { virCgroupGetBlkioIoDeviceServicedCB getBlkioIoDeviceServiced; virCgroupSetBlkioDeviceWeightCB setBlkioDeviceWeight; virCgroupGetBlkioDeviceWeightCB getBlkioDeviceWeight; + virCgroupSetBlkioDeviceReadIopsCB setBlkioDeviceReadIops; + virCgroupGetBlkioDeviceReadIopsCB getBlkioDeviceReadIops; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index b627a9d516..d418ef41df 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1185,6 +1185,54 @@ virCgroupV1GetBlkioDeviceWeight(virCgroupPtr group, }

+static int +virCgroupV1SetBlkioDeviceReadIops(virCgroupPtr group, + const char *path, + unsigned int riops) +{ + VIR_AUTOFREE(char *) str = NULL; + VIR_AUTOFREE(char *) blkstr = NULL; + + if (!(blkstr = virCgroupGetBlockDevString(path))) + return -1; + + if (virAsprintf(&str, "%s%u", blkstr, riops) < 0) + return -1; + + return virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "blkio.throttle.read_iops_device", + str); +} + + +static int +virCgroupV1GetBlkioDeviceReadIops(virCgroupPtr group, + const char *path, + unsigned int *riops) +{ + VIR_AUTOFREE(char *) str = NULL; + + if (virCgroupGetValueForBlkDev(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "blkio.throttle.read_iops_device", + path, + &str) < 0) + return -1; + + if (!str) { + *riops = 0; + } else if (virStrToLong_ui(str, NULL, 10, riops) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse '%s' as an integer"), + str); + return -1; + } + + return 0; +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -1213,6 +1261,8 @@ virCgroupBackend virCgroupV1Backend = { .getBlkioIoDeviceServiced = virCgroupV1GetBlkioIoDeviceServiced, .setBlkioDeviceWeight = virCgroupV1SetBlkioDeviceWeight, .getBlkioDeviceWeight = virCgroupV1GetBlkioDeviceWeight, + .setBlkioDeviceReadIops = virCgroupV1SetBlkioDeviceReadIops, + .getBlkioDeviceReadIops = virCgroupV1GetBlkioDeviceReadIops, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 34 ++----------------------- src/util/vircgroupbackend.h | 12 +++++++++ src/util/vircgroupv1.c | 50 +++++++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 32 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 3fa8aebe04..08b8387fba 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1421,19 +1421,7 @@ virCgroupSetBlkioDeviceReadBps(virCgroupPtr group, const char *path, unsigned long long rbps) { - VIR_AUTOFREE(char *) str = NULL; - VIR_AUTOFREE(char *) blkstr = NULL; - - if (!(blkstr = virCgroupGetBlockDevString(path))) - return -1; - - if (virAsprintf(&str, "%s%llu", blkstr, rbps) < 0) - return -1; - - return virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_BLKIO, - "blkio.throttle.read_bps_device", - str); + VIR_CGROUP_BACKEND_CALL(group, setBlkioDeviceReadBps, -1, path, rbps); }

/** @@ -1527,25 +1515,7 @@ virCgroupGetBlkioDeviceReadBps(virCgroupPtr group, const char *path, unsigned long long *rbps) { - VIR_AUTOFREE(char *) str = NULL; - - if (virCgroupGetValueForBlkDev(group, - VIR_CGROUP_CONTROLLER_BLKIO, - "blkio.throttle.read_bps_device", - path, - &str) < 0) - return -1; - - if (!str) { - *rbps = 0; - } else if (virStrToLong_ull(str, NULL, 10, rbps) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to parse '%s' as an integer"), - str); - return -1; - } - - return 0; + VIR_CGROUP_BACKEND_CALL(group, getBlkioDeviceReadBps, -1, path, rbps); }

/** diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index e19a3820bf..3ec9a95327 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -182,6 +182,16 @@ typedef int const char *path, unsigned int *wiops);

+typedef int +(*virCgroupSetBlkioDeviceReadBpsCB)(virCgroupPtr group, + const char *path, + unsigned long long rbps); + +typedef int +(*virCgroupGetBlkioDeviceReadBpsCB)(virCgroupPtr group, + const char *path, + unsigned long long *rbps); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -216,6 +226,8 @@ struct _virCgroupBackend { virCgroupGetBlkioDeviceReadIopsCB getBlkioDeviceReadIops; virCgroupSetBlkioDeviceWriteIopsCB setBlkioDeviceWriteIops; virCgroupGetBlkioDeviceWriteIopsCB getBlkioDeviceWriteIops; + virCgroupSetBlkioDeviceReadBpsCB setBlkioDeviceReadBps; + virCgroupGetBlkioDeviceReadBpsCB getBlkioDeviceReadBps; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index feeefc287d..2c19e6147a 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1281,6 +1281,54 @@ virCgroupV1GetBlkioDeviceWriteIops(virCgroupPtr group, }

+static int +virCgroupV1SetBlkioDeviceReadBps(virCgroupPtr group, + const char *path, + unsigned long long rbps) +{ + VIR_AUTOFREE(char *) str = NULL; + VIR_AUTOFREE(char *) blkstr = NULL; + + if (!(blkstr = virCgroupGetBlockDevString(path))) + return -1; + + if (virAsprintf(&str, "%s%llu", blkstr, rbps) < 0) + return -1; + + return virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "blkio.throttle.read_bps_device", + str); +} + + +static int +virCgroupV1GetBlkioDeviceReadBps(virCgroupPtr group, + const char *path, + unsigned long long *rbps) +{ + VIR_AUTOFREE(char *) str = NULL; + + if (virCgroupGetValueForBlkDev(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "blkio.throttle.read_bps_device", + path, + &str) < 0) + return -1; + + if (!str) { + *rbps = 0; + } else if (virStrToLong_ull(str, NULL, 10, rbps) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse '%s' as an integer"), + str); + return -1; + } + + return 0; +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -1313,6 +1361,8 @@ virCgroupBackend virCgroupV1Backend = { .getBlkioDeviceReadIops = virCgroupV1GetBlkioDeviceReadIops, .setBlkioDeviceWriteIops = virCgroupV1SetBlkioDeviceWriteIops, .getBlkioDeviceWriteIops = virCgroupV1GetBlkioDeviceWriteIops, + .setBlkioDeviceReadBps = virCgroupV1SetBlkioDeviceReadBps, + .getBlkioDeviceReadBps = virCgroupV1GetBlkioDeviceReadBps, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 20 +------------------- src/util/vircgroupbackend.h | 6 ++++++ src/util/vircgroupv1.c | 28 ++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index ab50342dbb..b1c1bc9cf0 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1595,25 +1595,7 @@ virCgroupGetMemoryUnlimitedKB(void) int virCgroupSetMemory(virCgroupPtr group, unsigned long long kb) { - unsigned long long maxkb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; - - if (kb > maxkb) { - virReportError(VIR_ERR_INVALID_ARG, - _("Memory '%llu' must be less than %llu"), - kb, maxkb); - return -1; - } - - if (kb == maxkb) - return virCgroupSetValueI64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - "memory.limit_in_bytes", - -1); - else - return virCgroupSetValueU64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - "memory.limit_in_bytes", - kb << 10); + VIR_CGROUP_BACKEND_CALL(group, setMemory, -1, kb); }

diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index b59462d6ab..2d59e4af20 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -202,6 +202,10 @@ typedef int const char *path, unsigned long long *wbps);

+typedef int +(*virCgroupSetMemoryCB)(virCgroupPtr group, + unsigned long long kb); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -240,6 +244,8 @@ struct _virCgroupBackend { virCgroupGetBlkioDeviceReadBpsCB getBlkioDeviceReadBps; virCgroupSetBlkioDeviceWriteBpsCB setBlkioDeviceWriteBps; virCgroupGetBlkioDeviceWriteBpsCB getBlkioDeviceWriteBps; + + virCgroupSetMemoryCB setMemory; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 4f6d20e31f..3235ef3d2d 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1377,6 +1377,32 @@ virCgroupV1GetBlkioDeviceWriteBps(virCgroupPtr group, }

+static int +virCgroupV1SetMemory(virCgroupPtr group, + unsigned long long kb) +{ + unsigned long long maxkb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + + if (kb > maxkb) { + virReportError(VIR_ERR_INVALID_ARG, + _("Memory '%llu' must be less than %llu"), + kb, maxkb); + return -1; + } + + if (kb == maxkb) + return virCgroupSetValueI64(group, + VIR_CGROUP_CONTROLLER_MEMORY, + "memory.limit_in_bytes", + -1); + else + return virCgroupSetValueU64(group, + VIR_CGROUP_CONTROLLER_MEMORY, + "memory.limit_in_bytes", + kb << 10); +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -1413,6 +1439,8 @@ virCgroupBackend virCgroupV1Backend = { .getBlkioDeviceReadBps = virCgroupV1GetBlkioDeviceReadBps, .setBlkioDeviceWriteBps = virCgroupV1SetBlkioDeviceWriteBps, .getBlkioDeviceWriteBps = virCgroupV1GetBlkioDeviceWriteBps, + + .setMemory = virCgroupV1SetMemory, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 05:45:51PM +0200, Pavel Hrdina wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 67 +++------------------------------- src/util/vircgroupbackend.h | 10 ++++++ src/util/vircgroupv1.c | 71 +++++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 63 deletions(-)

@@ -246,6 +255,7 @@ struct _virCgroupBackend { virCgroupGetBlkioDeviceWriteBpsCB getBlkioDeviceWriteBps;

virCgroupSetMemoryCB setMemory; + virCgroupGetMemoryStatCB getMemoryStat; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 3235ef3d2d..cfe29649fb 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1403,6 +1403,76 @@ virCgroupV1SetMemory(virCgroupPtr group, }

+static int +virCgroupV1GetMemoryStat(virCgroupPtr group, + unsigned long long *cache, + unsigned long long *activeAnon, + unsigned long long *inactiveAnon, + unsigned long long *activeFile, + unsigned long long *inactiveFile, + unsigned long long *unevictable) +{

Original:

- int ret = -1; - char *stat = NULL; New: + VIR_AUTOFREE(char *) stat = NULL;

+ char *line = NULL; + unsigned long long cacheVal = 0; + unsigned long long activeAnonVal = 0; + unsigned long long inactiveAnonVal = 0; + unsigned long long activeFileVal = 0; + unsigned long long inactiveFileVal = 0; + unsigned long long unevictableVal = 0; +

With the VIR_AUTOFREE conversion separated: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 9 +-------- src/util/vircgroupbackend.h | 5 +++++ src/util/vircgroupv1.c | 16 ++++++++++++++++ 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 22db0a4bf1..1229959300 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1639,14 +1639,7 @@ virCgroupGetMemoryStat(virCgroupPtr group, int virCgroupGetMemoryUsage(virCgroupPtr group, unsigned long *kb) { - long long unsigned int usage_in_bytes; - int ret; - ret = virCgroupGetValueU64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - "memory.usage_in_bytes", &usage_in_bytes); - if (ret == 0) - *kb = (unsigned long) usage_in_bytes >> 10; - return ret; + VIR_CGROUP_BACKEND_CALL(group, getMemoryUsage, -1, kb); }

diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 5ccda8e044..672fb082c1 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -215,6 +215,10 @@ typedef int unsigned long long *inactiveFile, unsigned long long *unevictable);

+typedef int +(*virCgroupGetMemoryUsageCB)(virCgroupPtr group, + unsigned long *kb); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -256,6 +260,7 @@ struct _virCgroupBackend {

virCgroupSetMemoryCB setMemory; virCgroupGetMemoryStatCB getMemoryStat; + virCgroupGetMemoryUsageCB getMemoryUsage; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index cfe29649fb..7abb137b81 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1473,6 +1473,21 @@ virCgroupV1GetMemoryStat(virCgroupPtr group, }

+static int +virCgroupV1GetMemoryUsage(virCgroupPtr group, + unsigned long *kb) +{ + long long unsigned int usage_in_bytes; + int ret; + ret = virCgroupGetValueU64(group, + VIR_CGROUP_CONTROLLER_MEMORY, + "memory.usage_in_bytes", &usage_in_bytes); + if (ret == 0) + *kb = (unsigned long) usage_in_bytes >> 10; + return ret; +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -1512,6 +1527,7 @@ virCgroupBackend virCgroupV1Backend = {

.setMemory = virCgroupV1SetMemory, .getMemoryStat = virCgroupV1GetMemoryStat, + .getMemoryUsage = virCgroupV1GetMemoryUsage, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 05:45:53PM +0200, Pavel Hrdina wrote:

They all need virCgroupV1GetMemoryUnlimitedKB() so it's easier to move them in one commit.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 128 ++------------------------- src/util/vircgroupbackend.h | 30 +++++++ src/util/vircgrouppriv.h | 6 ++ src/util/vircgroupv1.c | 168 ++++++++++++++++++++++++++++++++++++ 4 files changed, 211 insertions(+), 121 deletions(-)

diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 672fb082c1..5e75e495c3 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -219,6 +219,30 @@ typedef int (*virCgroupGetMemoryUsageCB)(virCgroupPtr group, unsigned long *kb);

+typedef int +(*virCgroupSetMemoryHardLimitCB)(virCgroupPtr group, + unsigned long long kb); + +typedef int +(*virCgroupGetMemoryHardLimitCB)(virCgroupPtr group, + unsigned long long *kb); + +typedef int +(*virCgroupSetMemorySoftLimitCB)(virCgroupPtr group, + unsigned long long kb); + +typedef int +(*virCgroupGetMemorySoftLimitCB)(virCgroupPtr group, + unsigned long long *kb); + +typedef int +(*virCgroupSetMemSwapHardLimitCB)(virCgroupPtr group, + unsigned long long kb); + +typedef int +(*virCgroupGetMemSwapHardLimitCB)(virCgroupPtr group, + unsigned long long *kb); + struct _virCgroupBackend { virCgroupBackendType type;

There are just two different types here. Regardless: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 9 +-------- src/util/vircgroupbackend.h | 5 +++++ src/util/vircgroupv1.c | 16 ++++++++++++++++ 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index cd6b087a69..38a30b759f 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1699,14 +1699,7 @@ virCgroupGetMemSwapHardLimit(virCgroupPtr group, unsigned long long *kb) int virCgroupGetMemSwapUsage(virCgroupPtr group, unsigned long long *kb) { - long long unsigned int usage_in_bytes; - int ret; - ret = virCgroupGetValueU64(group, - VIR_CGROUP_CONTROLLER_MEMORY, - "memory.memsw.usage_in_bytes", &usage_in_bytes); - if (ret == 0) - *kb = usage_in_bytes >> 10; - return ret; + VIR_CGROUP_BACKEND_CALL(group, getMemSwapUsage, -1, kb); }

diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 5e75e495c3..f5454e41f7 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -243,6 +243,10 @@ typedef int (*virCgroupGetMemSwapHardLimitCB)(virCgroupPtr group, unsigned long long *kb);

+typedef int +(*virCgroupGetMemSwapUsageCB)(virCgroupPtr group, + unsigned long long *kb); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -291,6 +295,7 @@ struct _virCgroupBackend { virCgroupGetMemorySoftLimitCB getMemorySoftLimit; virCgroupSetMemSwapHardLimitCB setMemSwapHardLimit; virCgroupGetMemSwapHardLimitCB getMemSwapHardLimit; + virCgroupGetMemSwapUsageCB getMemSwapUsage; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 5fc046afad..936cf1b1f5 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1650,6 +1650,21 @@ virCgroupV1GetMemSwapHardLimit(virCgroupPtr group, }

+static int +virCgroupV1GetMemSwapUsage(virCgroupPtr group, + unsigned long long *kb) +{ + long long unsigned int usage_in_bytes; + int ret; + ret = virCgroupGetValueU64(group, + VIR_CGROUP_CONTROLLER_MEMORY, + "memory.memsw.usage_in_bytes", &usage_in_bytes); + if (ret == 0) + *kb = usage_in_bytes >> 10; + return ret; +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -1696,6 +1711,7 @@ virCgroupBackend virCgroupV1Backend = { .getMemorySoftLimit = virCgroupV1GetMemorySoftLimit, .setMemSwapHardLimit = virCgroupV1SetMemSwapHardLimit, .getMemSwapHardLimit = virCgroupV1GetMemSwapHardLimit, + .getMemSwapUsage = virCgroupV1GetMemSwapUsage, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 18 ++---------------- src/util/vircgroupbackend.h | 9 +++++++++ src/util/vircgroupv1.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 8a54437dfa..ca36984472 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1824,10 +1824,7 @@ virCgroupGetCpusetCpus(virCgroupPtr group, char **cpus) int virCgroupDenyAllDevices(virCgroupPtr group) { - return virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_DEVICES, - "devices.deny", - "a"); + VIR_CGROUP_BACKEND_CALL(group, denyAllDevices, -1); }

/** @@ -1847,18 +1844,7 @@ virCgroupDenyAllDevices(virCgroupPtr group) int virCgroupAllowAllDevices(virCgroupPtr group, int perms) { - int ret = -1; - - if (virCgroupAllowDevice(group, 'b', -1, -1, perms) < 0) - goto cleanup; - - if (virCgroupAllowDevice(group, 'c', -1, -1, perms) < 0) - goto cleanup; - - ret = 0; - - cleanup: - return ret; + VIR_CGROUP_BACKEND_CALL(group, allowAllDevices, -1, perms); }

diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index d7250cffdb..ce165fead4 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -261,6 +261,13 @@ typedef int int minor, int perms);

+typedef int +(*virCgroupAllowAllDevicesCB)(virCgroupPtr group, + int perms); + +typedef int +(*virCgroupDenyAllDevicesCB)(virCgroupPtr group); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -313,6 +320,8 @@ struct _virCgroupBackend {

virCgroupAllowDeviceCB allowDevice; virCgroupDenyDeviceCB denyDevice; + virCgroupAllowAllDevicesCB allowAllDevices; + virCgroupDenyAllDevicesCB denyAllDevices; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 9ac0ef555c..bd6914f878 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1731,6 +1731,35 @@ virCgroupV1DenyDevice(virCgroupPtr group, }

+static int +virCgroupV1AllowAllDevices(virCgroupPtr group, + int perms) +{ + int ret = -1; + + if (virCgroupV1AllowDevice(group, 'b', -1, -1, perms) < 0) + goto cleanup; + + if (virCgroupV1AllowDevice(group, 'c', -1, -1, perms) < 0) + goto cleanup; + + ret = 0; + + cleanup: + return ret; +} + + +static int +virCgroupV1DenyAllDevices(virCgroupPtr group) +{ + return virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_DEVICES, + "devices.deny", + "a"); +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -1781,6 +1810,8 @@ virCgroupBackend virCgroupV1Backend = {

.allowDevice = virCgroupV1AllowDevice, .denyDevice = virCgroupV1DenyDevice, + .allowAllDevices = virCgroupV1AllowAllDevices, + .denyAllDevices = virCgroupV1DenyAllDevices, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 18 ++---------------- src/util/vircgroupbackend.h | 10 ++++++++++ src/util/vircgroupv1.c | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index d7d11062e5..beb661df4b 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2229,19 +2229,7 @@ virCgroupGetCpuShares(virCgroupPtr group, unsigned long long *shares) int virCgroupSetCpuCfsPeriod(virCgroupPtr group, unsigned long long cfs_period) { - /* The cfs_period should be greater or equal than 1ms, and less or equal - * than 1s. - */ - if (cfs_period < 1000 || cfs_period > 1000000) { - virReportError(VIR_ERR_INVALID_ARG, - _("cfs_period '%llu' must be in range (1000, 1000000)"), - cfs_period); - return -1; - } - - return virCgroupSetValueU64(group, - VIR_CGROUP_CONTROLLER_CPU, - "cpu.cfs_period_us", cfs_period); + VIR_CGROUP_BACKEND_CALL(group, setCpuCfsPeriod, -1, cfs_period); }

@@ -2256,9 +2244,7 @@ virCgroupSetCpuCfsPeriod(virCgroupPtr group, unsigned long long cfs_period) int virCgroupGetCpuCfsPeriod(virCgroupPtr group, unsigned long long *cfs_period) { - return virCgroupGetValueU64(group, - VIR_CGROUP_CONTROLLER_CPU, - "cpu.cfs_period_us", cfs_period); + VIR_CGROUP_BACKEND_CALL(group, getCpuCfsPeriod, -1, cfs_period); }

diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 7dbe005319..59f68169ba 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -276,6 +276,14 @@ typedef int (*virCgroupGetCpuSharesCB)(virCgroupPtr group, unsigned long long *shares);

+typedef int +(*virCgroupSetCpuCfsPeriodCB)(virCgroupPtr group, + unsigned long long cfs_period); + +typedef int +(*virCgroupGetCpuCfsPeriodCB)(virCgroupPtr group, + unsigned long long *cfs_period); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -333,6 +341,8 @@ struct _virCgroupBackend {

virCgroupSetCpuSharesCB setCpuShares; virCgroupGetCpuSharesCB getCpuShares; + virCgroupSetCpuCfsPeriodCB setCpuCfsPeriod; + virCgroupGetCpuCfsPeriodCB getCpuCfsPeriod; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index d2989b0a63..3641371512 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1780,6 +1780,36 @@ virCgroupV1GetCpuShares(virCgroupPtr group, }

+static int +virCgroupV1SetCpuCfsPeriod(virCgroupPtr group, + unsigned long long cfs_period) +{ + /* The cfs_period should be greater or equal than 1ms, and less or equal + * than 1s. + */ + if (cfs_period < 1000 || cfs_period > 1000000) { + virReportError(VIR_ERR_INVALID_ARG, + _("cfs_period '%llu' must be in range (1000, 1000000)"), + cfs_period); + return -1; + } + + return virCgroupSetValueU64(group, + VIR_CGROUP_CONTROLLER_CPU, + "cpu.cfs_period_us", cfs_period); +} + + +static int +virCgroupV1GetCpuCfsPeriod(virCgroupPtr group, + unsigned long long *cfs_period) +{ + return virCgroupGetValueU64(group, + VIR_CGROUP_CONTROLLER_CPU, + "cpu.cfs_period_us", cfs_period); +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -1835,6 +1865,8 @@ virCgroupBackend virCgroupV1Backend = {

.setCpuShares = virCgroupV1SetCpuShares, .getCpuShares = virCgroupV1GetCpuShares, + .setCpuCfsPeriod = virCgroupV1SetCpuCfsPeriod, + .getCpuCfsPeriod = virCgroupV1GetCpuCfsPeriod, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:46 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 13 +------------ src/util/vircgroupbackend.h | 4 ++++ src/util/vircgroupv1.c | 19 +++++++++++++++++++ 3 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 17b8c13cf9..bdbed65be2 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2687,18 +2687,7 @@ int virCgroupSetOwner(virCgroupPtr cgroup, bool virCgroupSupportsCpuBW(virCgroupPtr cgroup) { - VIR_AUTOFREE(char *) path = NULL; - - if (!cgroup) - return false;

-

- if (virCgroupPathOfController(cgroup, VIR_CGROUP_CONTROLLER_CPU, - "cpu.cfs_period_us", &path) < 0) { - virResetLastError(); - return false; - } - - return virFileExists(path); + VIR_CGROUP_BACKEND_CALL(cgroup, supportsCpuBW, false); }

int diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index b94a89069a..049f9040bd 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -292,6 +292,9 @@ typedef int (*virCgroupGetCpuCfsQuotaCB)(virCgroupPtr group, long long *cfs_quota);

+typedef bool +(*virCgroupSupportsCpuBWCB)(virCgroupPtr cgroup); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -353,6 +356,7 @@ struct _virCgroupBackend { virCgroupGetCpuCfsPeriodCB getCpuCfsPeriod; virCgroupSetCpuCfsQuotaCB setCpuCfsQuota; virCgroupGetCpuCfsQuotaCB getCpuCfsQuota; + virCgroupSupportsCpuBWCB supportsCpuBW; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 5de6d27f48..88912778b5 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1840,6 +1840,24 @@ virCgroupV1GetCpuCfsQuota(virCgroupPtr group, }

+static bool +virCgroupV1SupportsCpuBW(virCgroupPtr cgroup) +{ + VIR_AUTOFREE(char *) path = NULL; + + if (!cgroup) + return false;

Not related to this patch, but this check should be removed as cgroup is never NULL here.

+ + if (virCgroupV1PathOfController(cgroup, VIR_CGROUP_CONTROLLER_CPU, + "cpu.cfs_period_us", &path) < 0) { + virResetLastError(); + return false; + } + + return virFileExists(path); +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -1899,6 +1917,7 @@ virCgroupBackend virCgroupV1Backend = { .getCpuCfsPeriod = virCgroupV1GetCpuCfsPeriod, .setCpuCfsQuota = virCgroupV1SetCpuCfsQuota, .getCpuCfsQuota = virCgroupV1GetCpuCfsQuota, + .supportsCpuBW = virCgroupV1SupportsCpuBW, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:46 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 39 +----------------------------- src/util/vircgroupbackend.h | 6 +++++ src/util/vircgroupv1.c | 47 +++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 38 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 598147acf7..9ba2318c0b 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2598,44 +2598,7 @@ int virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long *user, unsigned long long *sys) { - VIR_AUTOFREE(char *) str = NULL; - char *p; - static double scale = -1.0; - - if (virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_CPUACCT, - "cpuacct.stat", &str) < 0) - return -1; - - if (!(p = STRSKIP(str, "user ")) || - virStrToLong_ull(p, &p, 10, user) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot parse user stat '%s'"), - p); - return -1; - } - if (!(p = STRSKIP(p, "\nsystem ")) || - virStrToLong_ull(p, NULL, 10, sys) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot parse sys stat '%s'"), - p); - return -1; - } - /* times reported are in system ticks (generally 100 Hz), but that - * rate can theoretically vary between machines. Scale things - * into approximate nanoseconds. */ - if (scale < 0) { - long ticks_per_sec = sysconf(_SC_CLK_TCK); - if (ticks_per_sec == -1) { - virReportSystemError(errno, "%s", - _("Cannot determine system clock HZ")); - return -1; - } - scale = 1000000000.0 / ticks_per_sec; - } - *user *= scale; - *sys *= scale; - - return 0; + VIR_CGROUP_BACKEND_CALL(group, getCpuacctStat, -1, user, sys); }

diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 380ab9850d..a80ad0b5d3 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -303,6 +303,11 @@ typedef int (*virCgroupGetCpuacctPercpuUsageCB)(virCgroupPtr group, char **usage);

+typedef int +(*virCgroupGetCpuacctStatCB)(virCgroupPtr group, + unsigned long long *user, + unsigned long long *sys); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -368,6 +373,7 @@ struct _virCgroupBackend {

virCgroupGetCpuacctUsageCB getCpuacctUsage; virCgroupGetCpuacctPercpuUsageCB getCpuacctPercpuUsage; + virCgroupGetCpuacctStatCB getCpuacctStat; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index a827917db1..f99cf5c6d6 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1877,6 +1877,52 @@ virCgroupV1GetCpuacctPercpuUsage(virCgroupPtr group, }

+static int +virCgroupV1GetCpuacctStat(virCgroupPtr group, + unsigned long long *user, + unsigned long long *sys) +{ + VIR_AUTOFREE(char *) str = NULL; + char *p; + static double scale = -1.0; + + if (virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_CPUACCT, + "cpuacct.stat", &str) < 0) + return -1; + + if (!(p = STRSKIP(str, "user ")) || + virStrToLong_ull(p, &p, 10, user) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot parse user stat '%s'"), + p); + return -1; + } + if (!(p = STRSKIP(p, "\nsystem ")) || + virStrToLong_ull(p, NULL, 10, sys) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot parse sys stat '%s'"), + p); + return -1; + } + /* times reported are in system ticks (generally 100 Hz), but that + * rate can theoretically vary between machines. Scale things + * into approximate nanoseconds. */ + if (scale < 0) { + long ticks_per_sec = sysconf(_SC_CLK_TCK); + if (ticks_per_sec == -1) { + virReportSystemError(errno, "%s", + _("Cannot determine system clock HZ")); + return -1; + } + scale = 1000000000.0 / ticks_per_sec; + } + *user *= scale; + *sys *= scale; + + return 0; +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -1940,6 +1986,7 @@ virCgroupBackend virCgroupV1Backend = {

.getCpuacctUsage = virCgroupV1GetCpuacctUsage, .getCpuacctPercpuUsage = virCgroupV1GetCpuacctPercpuUsage, + .getCpuacctStat = virCgroupV1GetCpuacctStat, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:46 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 10 ++-------- src/util/vircgroupbackend.h | 11 +++++++++++ src/util/vircgroupv1.c | 25 +++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 8744cd067d..1e19813cb3 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1714,10 +1714,7 @@ virCgroupGetMemSwapUsage(virCgroupPtr group, unsigned long long *kb) int virCgroupSetCpusetMems(virCgroupPtr group, const char *mems) { - return virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_CPUSET, - "cpuset.mems", - mems); + VIR_CGROUP_BACKEND_CALL(group, setCpusetMems, -1, mems); }

@@ -1732,10 +1729,7 @@ virCgroupSetCpusetMems(virCgroupPtr group, const char *mems) int virCgroupGetCpusetMems(virCgroupPtr group, char **mems) { - return virCgroupGetValueStr(group, - VIR_CGROUP_CONTROLLER_CPUSET, - "cpuset.mems", - mems); + VIR_CGROUP_BACKEND_CALL(group, getCpusetMems, -1, mems); }

diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 0369b48f2b..86fe8ce2db 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -316,6 +316,14 @@ typedef int (*virCgroupGetFreezerStateCB)(virCgroupPtr group, char **state);

+typedef int +(*virCgroupSetCpusetMemsCB)(virCgroupPtr group, + const char *mems); + +typedef int +(*virCgroupGetCpusetMemsCB)(virCgroupPtr group, + char **mems); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -385,6 +393,9 @@ struct _virCgroupBackend {

virCgroupSetFreezerStateCB setFreezerState; virCgroupGetFreezerStateCB getFreezerState; + + virCgroupSetCpusetMemsCB setCpusetMems; + virCgroupGetCpusetMemsCB getCpusetMems; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index a1b2f3eb23..aff1706c5b 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1943,6 +1943,28 @@ virCgroupV1GetFreezerState(virCgroupPtr group, }

+static int +virCgroupV1SetCpusetMems(virCgroupPtr group, + const char *mems) +{ + return virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_CPUSET, + "cpuset.mems", + mems); +} + + +static int +virCgroupV1GetCpusetMems(virCgroupPtr group, + char **mems) +{ + return virCgroupGetValueStr(group, + VIR_CGROUP_CONTROLLER_CPUSET, + "cpuset.mems", + mems); +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -2010,6 +2032,9 @@ virCgroupBackend virCgroupV1Backend = {

.setFreezerState = virCgroupV1SetFreezerState, .getFreezerState = virCgroupV1GetFreezerState, + + .setCpusetMems = virCgroupV1SetCpusetMems, + .getCpusetMems = virCgroupV1GetCpusetMems, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:46 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

--- src/util/vircgroup.c | 10 ++-------- src/util/vircgroupbackend.h | 10 ++++++++++ src/util/vircgroupv1.c | 24 ++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index c37ada7c26..b61c6bf2f3 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1774,10 +1774,7 @@ virCgroupGetCpusetMemoryMigrate(virCgroupPtr group, bool *migrate) int virCgroupSetCpusetCpus(virCgroupPtr group, const char *cpus) { - return virCgroupSetValueStr(group, - VIR_CGROUP_CONTROLLER_CPUSET, - "cpuset.cpus", - cpus); + VIR_CGROUP_BACKEND_CALL(group, setCpusetCpus, -1, cpus); }

@@ -1792,10 +1789,7 @@ virCgroupSetCpusetCpus(virCgroupPtr group, const char *cpus) int virCgroupGetCpusetCpus(virCgroupPtr group, char **cpus) { - return virCgroupGetValueStr(group, - VIR_CGROUP_CONTROLLER_CPUSET, - "cpuset.cpus", - cpus); + VIR_CGROUP_BACKEND_CALL(group, getCpusetCpus, -1, cpus); }

diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 887a508a15..fc7745d714 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -332,6 +332,14 @@ typedef int (*virCgroupGetCpusetMemoryMigrateCB)(virCgroupPtr group, bool *migrate);

+typedef int +(*virCgroupSetCpusetCpusCB)(virCgroupPtr group, + const char *cpus); + +typedef int +(*virCgroupGetCpusetCpusCB)(virCgroupPtr group, + char **cpus); + struct _virCgroupBackend { virCgroupBackendType type;

@@ -406,6 +414,8 @@ struct _virCgroupBackend { virCgroupGetCpusetMemsCB getCpusetMems; virCgroupSetCpusetMemoryMigrateCB setCpusetMemoryMigrate; virCgroupGetCpusetMemoryMigrateCB getCpusetMemoryMigrate; + virCgroupSetCpusetCpusCB setCpusetCpus; + virCgroupGetCpusetCpusCB getCpusetCpus; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 8070aede3e..be0be0f582 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1990,6 +1990,28 @@ virCgroupV1GetCpusetMemoryMigrate(virCgroupPtr group, }

+static int +virCgroupV1SetCpusetCpus(virCgroupPtr group, + const char *cpus) +{ + return virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_CPUSET, + "cpuset.cpus", + cpus); +} + + +static int +virCgroupV1GetCpusetCpus(virCgroupPtr group, + char **cpus) +{ + return virCgroupGetValueStr(group, + VIR_CGROUP_CONTROLLER_CPUSET, + "cpuset.cpus", + cpus); +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -2062,6 +2084,8 @@ virCgroupBackend virCgroupV1Backend = { .getCpusetMems = virCgroupV1GetCpusetMems, .setCpusetMemoryMigrate = virCgroupV1SetCpusetMemoryMigrate, .getCpusetMemoryMigrate = virCgroupV1GetCpusetMemoryMigrate, + .setCpusetCpus = virCgroupV1SetCpusetCpus, + .getCpusetCpus = virCgroupV1GetCpusetCpus, };

-- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Sep 18, 2018 at 5:46 PM, Pavel Hrdina <phrdina@redhat.com> wrote:

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Pavel, I'm not sure what your next series is doing but why renaming the controllers to legacy?

--- src/util/vircgroup.c | 6 +- src/util/vircgrouppriv.h | 2 +- src/util/vircgroupv1.c | 140 +++++++++++++++++++-------------------- tests/vircgrouptest.c | 14 ++-- 4 files changed, 81 insertions(+), 81 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index b61c6bf2f3..e031e82914 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1248,9 +1248,9 @@ virCgroupFree(virCgroupPtr *group) return;

for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - VIR_FREE((*group)->controllers[i].mountPoint); - VIR_FREE((*group)->controllers[i].linkPoint); - VIR_FREE((*group)->controllers[i].placement); + VIR_FREE((*group)->legacy[i].mountPoint); + VIR_FREE((*group)->legacy[i].linkPoint); + VIR_FREE((*group)->legacy[i].placement); }

VIR_FREE((*group)->path); diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h index ee6aeedbb5..c50a25f195 100644 --- a/src/util/vircgrouppriv.h +++ b/src/util/vircgrouppriv.h @@ -50,7 +50,7 @@ struct _virCgroup {

virCgroupBackendPtr backend;

- virCgroupV1Controller controllers[VIR_CGROUP_CONTROLLER_LAST]; + virCgroupV1Controller legacy[VIR_CGROUP_CONTROLLER_LAST]; };

int virCgroupSetValueStr(virCgroupPtr group, diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index f0f703f811..e4364009e5 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -126,10 +126,10 @@ virCgroupV1ValidateMachineGroup(virCgroupPtr group, if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) continue;

- if (!group->controllers[i].placement) + if (!group->legacy[i].placement) continue;

- tmp = strrchr(group->controllers[i].placement, '/'); + tmp = strrchr(group->legacy[i].placement, '/'); if (!tmp) return false;

@@ -138,7 +138,7 @@ virCgroupV1ValidateMachineGroup(virCgroupPtr group, i == VIR_CGROUP_CONTROLLER_CPUSET) { if (STREQ(tmp, "/emulator")) *tmp = '\0'; - tmp = strrchr(group->controllers[i].placement, '/'); + tmp = strrchr(group->legacy[i].placement, '/'); if (!tmp) return false; } @@ -170,15 +170,15 @@ virCgroupV1CopyMounts(virCgroupPtr group, { size_t i; for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - if (!parent->controllers[i].mountPoint) + if (!parent->legacy[i].mountPoint) continue;

- if (VIR_STRDUP(group->controllers[i].mountPoint, - parent->controllers[i].mountPoint) < 0) + if (VIR_STRDUP(group->legacy[i].mountPoint, + parent->legacy[i].mountPoint) < 0) return -1;

- if (VIR_STRDUP(group->controllers[i].linkPoint, - parent->controllers[i].linkPoint) < 0) + if (VIR_STRDUP(group->legacy[i].linkPoint, + parent->legacy[i].linkPoint) < 0) return -1; } return 0; @@ -192,14 +192,14 @@ virCgroupV1CopyPlacement(virCgroupPtr group, { size_t i; for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - if (!group->controllers[i].mountPoint) + if (!group->legacy[i].mountPoint) continue;

if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) continue;

if (path[0] == '/') { - if (VIR_STRDUP(group->controllers[i].placement, path) < 0) + if (VIR_STRDUP(group->legacy[i].placement, path) < 0) return -1; } else { /* @@ -207,10 +207,10 @@ virCgroupV1CopyPlacement(virCgroupPtr group, * parent == "/libvirt.service" + path == "" => "/libvirt.service" * parent == "/libvirt.service" + path == "foo" => "/libvirt.service/foo" */ - if (virAsprintf(&group->controllers[i].placement, + if (virAsprintf(&group->legacy[i].placement, "%s%s%s", - parent->controllers[i].placement, - (STREQ(parent->controllers[i].placement, "/") || + parent->legacy[i].placement, + (STREQ(parent->legacy[i].placement, "/") || STREQ(path, "") ? "" : "/"), path) < 0) return -1; @@ -319,7 +319,7 @@ virCgroupV1DetectMounts(virCgroupPtr group, * once. We need to save the results of the last one, * and we need to be careful to release the memory used * by previous processing. */ - virCgroupV1ControllerPtr controller = &group->controllers[i]; + virCgroupV1ControllerPtr controller = &group->legacy[i];

VIR_FREE(controller->mountPoint); VIR_FREE(controller->linkPoint); @@ -349,19 +349,19 @@ virCgroupV1DetectPlacement(virCgroupPtr group, const char *typestr = virCgroupV1ControllerTypeToString(i);

if (virCgroupV1MountOptsMatchController(controllers, typestr) && - group->controllers[i].mountPoint != NULL && - group->controllers[i].placement == NULL) { + group->legacy[i].mountPoint != NULL && + group->legacy[i].placement == NULL) { /* * selfpath == "/" + path="" -> "/" * selfpath == "/libvirt.service" + path == "" -> "/libvirt.service" * selfpath == "/libvirt.service" + path == "foo" -> "/libvirt.service/foo" */ if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) { - if (VIR_STRDUP(group->controllers[i].placement, + if (VIR_STRDUP(group->legacy[i].placement, selfpath) < 0) return -1; } else { - if (virAsprintf(&group->controllers[i].placement, + if (virAsprintf(&group->legacy[i].placement, "%s%s%s", selfpath, (STREQ(selfpath, "/") || STREQ(path, "") ? "" : "/"), @@ -382,22 +382,22 @@ virCgroupV1ValidatePlacement(virCgroupPtr group, size_t i;

for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - if (!group->controllers[i].mountPoint) + if (!group->legacy[i].mountPoint) continue;

- if (!group->controllers[i].placement) { + if (!group->legacy[i].placement) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not find placement for v1 controller %s at %s"), virCgroupV1ControllerTypeToString(i), - group->controllers[i].placement); + group->legacy[i].placement); return -1; }

VIR_DEBUG("Detected mount/mapping %zu:%s at %s in %s for pid %lld", i, virCgroupV1ControllerTypeToString(i), - group->controllers[i].mountPoint, - group->controllers[i].placement, + group->legacy[i].mountPoint, + group->legacy[i].placement, (long long) pid); }

@@ -410,7 +410,7 @@ virCgroupV1GetPlacement(virCgroupPtr group) { char *ret = NULL;

- ignore_value(VIR_STRDUP(ret, group->controllers[VIR_CGROUP_ CONTROLLER_SYSTEMD].placement)); + ignore_value(VIR_STRDUP(ret, group->legacy[VIR_CGROUP_CONTR OLLER_SYSTEMD].placement));

return ret; } @@ -429,7 +429,7 @@ virCgroupV1DetectControllers(virCgroupPtr group, for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { if (((1 << i) & controllers)) { /* Remove non-existent controllers */ - if (!group->controllers[i].mountPoint) { + if (!group->legacy[i].mountPoint) { VIR_DEBUG("Requested controller '%s' not mounted, ignoring", virCgroupV1ControllerTypeToString(i)); controllers &= ~(1 << i); @@ -440,9 +440,9 @@ virCgroupV1DetectControllers(virCgroupPtr group, VIR_DEBUG("Controller '%s' wanted=%s, mount='%s'", virCgroupV1ControllerTypeToString(i), (1 << i) & controllers ? "yes" : "no", - NULLSTR(group->controllers[i].mountPoint)); + NULLSTR(group->legacy[i].mountPoint)); if (!((1 << i) & controllers) && - group->controllers[i].mountPoint) { + group->legacy[i].mountPoint) { /* Check whether a request to disable a controller * clashes with co-mounting of controllers */ for (j = 0; j < VIR_CGROUP_CONTROLLER_LAST; j++) { @@ -451,8 +451,8 @@ virCgroupV1DetectControllers(virCgroupPtr group, if (!((1 << j) & controllers)) continue;

- if (STREQ_NULLABLE(group->controllers[i].mountPoint, - group->controllers[j].mountPoint)) { + if (STREQ_NULLABLE(group->legacy[i].mountPoint, + group->legacy[j].mountPoint)) { virReportSystemError(EINVAL, _("V1 controller '%s' is not wanted, but '%s' is co-mounted"),

virCgroupV1ControllerTypeToString(i), @@ -460,7 +460,7 @@ virCgroupV1DetectControllers(virCgroupPtr group, return -1; } } - VIR_FREE(group->controllers[i].mountPoint); + VIR_FREE(group->legacy[i].mountPoint); } } } else { @@ -469,8 +469,8 @@ virCgroupV1DetectControllers(virCgroupPtr group, for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { VIR_DEBUG("Controller '%s' present=%s", virCgroupV1ControllerTypeToString(i), - group->controllers[i].mountPoint ? "yes" : "no"); - if (group->controllers[i].mountPoint == NULL) + group->legacy[i].mountPoint ? "yes" : "no"); + if (group->legacy[i].mountPoint == NULL) continue; controllers |= (1 << i); } @@ -484,7 +484,7 @@ static bool virCgroupV1HasController(virCgroupPtr group, int controller) { - return group->controllers[controller].mountPoint != NULL; + return group->legacy[controller].mountPoint != NULL; }

@@ -498,9 +498,9 @@ virCgroupV1GetAnyController(virCgroupPtr group) * of '/' to avoid doing bad stuff to the root * cgroup */ - if (group->controllers[i].mountPoint && - group->controllers[i].placement && - STRNEQ(group->controllers[i].placement, "/")) { + if (group->legacy[i].mountPoint && + group->legacy[i].placement && + STRNEQ(group->legacy[i].placement, "/")) { return i; } } @@ -515,14 +515,14 @@ virCgroupV1PathOfController(virCgroupPtr group, const char *key, char **path) { - if (group->controllers[controller].mountPoint == NULL) { + if (group->legacy[controller].mountPoint == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("v1 controller '%s' is not mounted"), virCgroupV1ControllerTypeToString(controller)); return -1; }

- if (group->controllers[controller].placement == NULL) { + if (group->legacy[controller].placement == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("v1 controller '%s' is not enabled for group"), virCgroupV1ControllerTypeToString(controller)); @@ -530,8 +530,8 @@ virCgroupV1PathOfController(virCgroupPtr group, }

if (virAsprintf(path, "%s%s/%s", - group->controllers[controller].mountPoint, - group->controllers[controller].placement, + group->legacy[controller].mountPoint, + group->legacy[controller].placement, key ? key : "") < 0) return -1;

@@ -617,7 +617,7 @@ virCgroupV1MakeGroup(virCgroupPtr parent, }

/* Skip over controllers that aren't mounted */ - if (!group->controllers[i].mountPoint) { + if (!group->legacy[i].mountPoint) { VIR_DEBUG("Skipping unmounted controller %s", virCgroupV1ControllerTypeToString(i)); continue; @@ -638,7 +638,7 @@ virCgroupV1MakeGroup(virCgroupPtr parent, * treat blkio as unmounted if mkdir fails. */ if (i == VIR_CGROUP_CONTROLLER_BLKIO) { VIR_DEBUG("Ignoring mkdir failure with blkio controller. Kernel probably too old"); - VIR_FREE(group->controllers[i].mountPoint); + VIR_FREE(group->legacy[i].mountPoint); continue; } else { virReportSystemError(errno, @@ -648,7 +648,7 @@ virCgroupV1MakeGroup(virCgroupPtr parent, } } if (i == VIR_CGROUP_CONTROLLER_CPUSET && - group->controllers[i].mountPoint != NULL && + group->legacy[i].mountPoint != NULL && virCgroupV1CpuSetInherit(parent, group) < 0) { return -1; } @@ -658,7 +658,7 @@ virCgroupV1MakeGroup(virCgroupPtr parent, */ if ((flags & VIR_CGROUP_MEM_HIERACHY) && i == VIR_CGROUP_CONTROLLER_MEMORY && - group->controllers[i].mountPoint != NULL && + group->legacy[i].mountPoint != NULL && virCgroupV1SetMemoryUseHierarchy(group) < 0) { return -1; } @@ -681,7 +681,7 @@ virCgroupV1Remove(virCgroupPtr group) VIR_AUTOFREE(char *) grppath = NULL;

/* Skip over controllers not mounted */ - if (!group->controllers[i].mountPoint) + if (!group->legacy[i].mountPoint) continue;

/* We must never rmdir() in systemd's hierarchy */ @@ -690,7 +690,7 @@ virCgroupV1Remove(virCgroupPtr group)

/* Don't delete the root group, if we accidentally ended up in it for some reason */ - if (STREQ(group->controllers[i].placement, "/")) + if (STREQ(group->legacy[i].placement, "/")) continue;

if (virCgroupV1PathOfController(group, @@ -718,7 +718,7 @@ virCgroupV1AddTask(virCgroupPtr group,

for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { /* Skip over controllers not mounted */ - if (!group->controllers[i].mountPoint) + if (!group->legacy[i].mountPoint) continue;

/* We must never add tasks in systemd's hierarchy @@ -765,17 +765,17 @@ virCgroupV1IdentifyRoot(virCgroupPtr group)

for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { char *tmp; - if (!group->controllers[i].mountPoint) + if (!group->legacy[i].mountPoint) continue; - if (!(tmp = strrchr(group->controllers[i].mountPoint, '/'))) { + if (!(tmp = strrchr(group->legacy[i].mountPoint, '/'))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not find directory separator in %s"), - group->controllers[i].mountPoint); + group->legacy[i].mountPoint); return NULL; }

- if (VIR_STRNDUP(ret, group->controllers[i].mountPoint, - tmp - group->controllers[i].mountPoint) < 0) + if (VIR_STRNDUP(ret, group->legacy[i].mountPoint, + tmp - group->legacy[i].mountPoint) < 0) return NULL; return ret; } @@ -819,44 +819,44 @@ virCgroupV1BindMount(virCgroupPtr group, }

for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - if (!group->controllers[i].mountPoint) + if (!group->legacy[i].mountPoint) continue;

- if (!virFileExists(group->controllers[i].mountPoint)) { + if (!virFileExists(group->legacy[i].mountPoint)) { VIR_AUTOFREE(char *) src = NULL; if (virAsprintf(&src, "%s%s", oldroot, - group->controllers[i].mountPoint) < 0) + group->legacy[i].mountPoint) < 0) return -1;

VIR_DEBUG("Create mount point '%s'", - group->controllers[i].mountPoint); - if (virFileMakePath(group->controllers[i].mountPoint) < 0) { + group->legacy[i].mountPoint); + if (virFileMakePath(group->legacy[i].mountPoint) < 0) { virReportSystemError(errno, _("Unable to create directory %s"), - group->controllers[i].mountPoint); + group->legacy[i].mountPoint); return -1; }

- if (mount(src, group->controllers[i].mountPoint, "none", MS_BIND, + if (mount(src, group->legacy[i].mountPoint, "none", MS_BIND, NULL) < 0) { virReportSystemError(errno, _("Failed to bind cgroup '%s' on '%s'"), - src, group->controllers[i].mountPoi nt); + src, group->legacy[i].mountPoint); return -1; } }

- if (group->controllers[i].linkPoint) { + if (group->legacy[i].linkPoint) { VIR_DEBUG("Link mount point '%s' to '%s'", - group->controllers[i].mountPoint, - group->controllers[i].linkPoint); - if (symlink(group->controllers[i].mountPoint, - group->controllers[i].linkPoint) < 0) { + group->legacy[i].mountPoint, + group->legacy[i].linkPoint); + if (symlink(group->legacy[i].mountPoint, + group->legacy[i].linkPoint) < 0) { virReportSystemError(errno, _("Unable to symlink directory %s to %s"), - group->controllers[i].mountPoint, - group->controllers[i].linkPoint); + group->legacy[i].mountPoint, + group->legacy[i].linkPoint); return -1; } } @@ -885,11 +885,11 @@ virCgroupV1SetOwner(virCgroupPtr cgroup, if (!((1 << i) & controllers)) continue;

- if (!cgroup->controllers[i].mountPoint) + if (!cgroup->legacy[i].mountPoint) continue;

- if (virAsprintf(&base, "%s%s", cgroup->controllers[i].mountPoint, - cgroup->controllers[i].placement) < 0) + if (virAsprintf(&base, "%s%s", cgroup->legacy[i].mountPoint, + cgroup->legacy[i].placement) < 0) goto cleanup;

if (virDirOpen(&dh, base) < 0) diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index be3143ea52..725f44c62e 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -56,25 +56,25 @@ static int validateCgroup(virCgroupPtr cgroup,

for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { if (STRNEQ_NULLABLE(expectMountPoint[i], - cgroup->controllers[i].mountPoint)) { + cgroup->legacy[i].mountPoint)) { fprintf(stderr, "Wrong mount '%s', expected '%s' for '%s'\n", - cgroup->controllers[i].mountPoint, + cgroup->legacy[i].mountPoint, expectMountPoint[i], virCgroupControllerTypeToString(i)); return -1; } if (STRNEQ_NULLABLE(expectLinkPoint[i], - cgroup->controllers[i].linkPoint)) { + cgroup->legacy[i].linkPoint)) { fprintf(stderr, "Wrong link '%s', expected '%s' for '%s'\n", - cgroup->controllers[i].linkPoint, + cgroup->legacy[i].linkPoint, expectLinkPoint[i], virCgroupControllerTypeToString(i)); return -1; } if (STRNEQ_NULLABLE(expectPlacement[i], - cgroup->controllers[i].placement)) { + cgroup->legacy[i].placement)) { fprintf(stderr, "Wrong placement '%s', expected '%s' for '%s'\n", - cgroup->controllers[i].placement, + cgroup->legacy[i].placement, expectPlacement[i], virCgroupControllerTypeToString(i)); return -1; @@ -160,7 +160,7 @@ testCgroupDetectMounts(const void *args) for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { virBufferAsprintf(&buf, "%-12s %s\n", virCgroupControllerTypeToString(i), - NULLSTR(group->controllers[i].mountPoint)); + NULLSTR(group->legacy[i].mountPoint)); } if (virBufferCheckError(&buf) < 0) goto cleanup; -- 2.17.1

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list