10:30 a.m.
This set adds {Get,Set}SchedulerParameters support to the LXC driver. A
single parameter of "cpu_shares" is added, which gets/sets the cpu.shares
key in the domain's cgroup.
I also added bits to "virsh schedinfo" to support this.
10:30 a.m.
New subject: [libvirt] [PATCH 1 of 3] Add get/set of cpu.shares to cgroup implementation
This brings get/set of U64 out of the #if 0, which looks messier than it is.
diff -r 64f19f607bc6 -r 6fb284fa200a src/cgroup.c
--- a/src/cgroup.c Fri Oct 03 17:58:02 2008 +0000
+++ b/src/cgroup.c Tue Oct 07 08:21:49 2008 -0700
@@ -224,26 +224,6 @@
return rc;
}
-#if 0
-/* This is included for completeness, but not yet used */
-
-static int virCgroupSetValueI64(virCgroupPtr group,
- const char *key,
- int64_t value)
-{
- char *strval = NULL;
- int rc;
-
- if (asprintf(&strval, "%" PRIi64, value) == -1)
- return -ENOMEM;
-
- rc = virCgroupSetValueStr(group, key, strval);
-
- VIR_FREE(strval);
-
- return rc;
-}
-
static int virCgroupGetValueStr(virCgroupPtr group,
const char *key,
char **value)
@@ -293,20 +273,21 @@
return rc;
}
-static int virCgroupGetValueU64(virCgroupPtr group,
+#if 0
+/* This is included for completeness, but not yet used */
+
+static int virCgroupSetValueI64(virCgroupPtr group,
const char *key,
- uint64_t *value)
+ int64_t value)
{
char *strval = NULL;
- int rc = 0;
+ int rc;
- rc = virCgroupGetValueStr(group, key, &strval);
- if (rc != 0)
- goto out;
+ if (asprintf(&strval, "%" PRIi64, value) == -1)
+ return -ENOMEM;
- if (sscanf(strval, "%" SCNu64, value) != 1)
- rc = -EINVAL;
-out:
+ rc = virCgroupSetValueStr(group, key, strval);
+
VIR_FREE(strval);
return rc;
@@ -331,6 +312,25 @@
return rc;
}
#endif
+
+static int virCgroupGetValueU64(virCgroupPtr group,
+ const char *key,
+ uint64_t *value)
+{
+ char *strval = NULL;
+ int rc = 0;
+
+ rc = virCgroupGetValueStr(group, key, &strval);
+ if (rc != 0)
+ goto out;
+
+ if (sscanf(strval, "%" SCNu64, value) != 1)
+ rc = -EINVAL;
+out:
+ VIR_FREE(strval);
+
+ return rc;
+}
static int _virCgroupInherit(const char *path,
const char *key)
@@ -760,3 +760,13 @@
return rc;
}
+
+int virCgroupSetCpuShares(virCgroupPtr group, unsigned long shares)
+{
+ return virCgroupSetValueU64(group, "cpu.shares", (uint64_t)shares);
+}
+
+int virCgroupGetCpuShares(virCgroupPtr group, unsigned long *shares)
+{
+ return virCgroupGetValueU64(group, "cpu.shares", (uint64_t *)shares);
+}
diff -r 64f19f607bc6 -r 6fb284fa200a src/cgroup.h
--- a/src/cgroup.h Fri Oct 03 17:58:02 2008 +0000
+++ b/src/cgroup.h Tue Oct 07 08:21:49 2008 -0700
@@ -36,6 +36,9 @@
int major,
int minor);
+int virCgroupSetCpuShares(virCgroupPtr group, unsigned long shares);
+int virCgroupGetCpuShares(virCgroupPtr group, unsigned long *shares);
+
int virCgroupRemove(virCgroupPtr group);
void virCgroupFree(virCgroupPtr *group);
10:30 a.m.
New subject: [libvirt] [PATCH 2 of 3] Add scheduling parameter support for LXC domains
diff -r 6fb284fa200a -r ebecbe5caa03 src/lxc_driver.c
--- a/src/lxc_driver.c Tue Oct 07 08:21:49 2008 -0700
+++ b/src/lxc_driver.c Tue Oct 07 08:21:50 2008 -0700
@@ -35,6 +35,7 @@
#include <unistd.h>
#include <wait.h>
+#include "internal.h"
#include "lxc_conf.h"
#include "lxc_container.h"
#include "lxc_driver.h"
@@ -1149,6 +1150,94 @@
return 0;
}
+static char *lxcGetSchedulerType(virDomainPtr domain, int *nparams)
+{
+ if (nparams)
+ *nparams = 1;
+
+ return strdup("cgroup");
+}
+
+static int lxcSetSchedulerParameters(virDomainPtr _domain,
+ virSchedParameterPtr params,
+ int nparams)
+{
+ int i;
+ int rc;
+ virCgroupPtr group;
+ virDomainObjPtr domain;
+
+ if (virCgroupHaveSupport() != 0)
+ return 0;
+
+ domain = virDomainFindByUUID(lxc_driver->domains, _domain->uuid);
+ if (domain == NULL) {
+ lxcError(NULL, _domain, VIR_ERR_INTERNAL_ERROR,
+ _("No such domain %s"), _domain->uuid);
+ return -EINVAL;
+ }
+
+ rc = virCgroupForDomain(domain->def, "lxc", &group);
+ if (rc != 0)
+ return rc;
+
+ for (i = 0; i < nparams; i++) {
+ virSchedParameterPtr param = ¶ms[i];
+
+ if (STREQ(param->field, "cpu_shares")) {
+ rc = virCgroupSetCpuShares(group, params[i].value.ui);
+ } else {
+ lxcError(NULL, _domain, VIR_ERR_INVALID_ARG,
+ _("Invalid parameter `%s'"), param->field);
+ rc = -ENOENT;
+ goto out;
+ }
+ }
+
+ rc = 0;
+out:
+ virCgroupFree(&group);
+
+ return rc;
+}
+
+static int lxcGetSchedulerParameters(virDomainPtr _domain,
+ virSchedParameterPtr params,
+ int *nparams)
+{
+ int rc = 0;
+ virCgroupPtr group;
+ virDomainObjPtr domain;
+
+ if (virCgroupHaveSupport() != 0)
+ return 0;
+
+ if ((*nparams) != 1) {
+ lxcError(NULL, _domain, VIR_ERR_INVALID_ARG,
+ _("Invalid parameter count"));
+ return -1;
+ }
+
+ domain = virDomainFindByUUID(lxc_driver->domains, _domain->uuid);
+ if (domain == NULL) {
+ lxcError(NULL, _domain, VIR_ERR_INTERNAL_ERROR,
+ _("No such domain %s"), _domain->uuid);
+ return -ENOENT;
+ }
+
+ rc = virCgroupForDomain(domain->def, "lxc", &group);
+ if (rc != 0)
+ return rc;
+
+ rc = virCgroupGetCpuShares(group, (unsigned long *)¶ms[0].value.ul);
+ strncpy(params[0].field, "cpu_shares", sizeof(params[0].field));
+ params[0].type = VIR_DOMAIN_SCHED_FIELD_ULLONG;
+
+ virCgroupFree(&group);
+
+ return rc;
+}
+
/* Function Tables */
static virDriver lxcDriver = {
VIR_DRV_LXC, /* the number virDrvNo */
@@ -1198,9 +1287,9 @@
NULL, /* domainDetachDevice */
NULL, /* domainGetAutostart */
NULL, /* domainSetAutostart */
- NULL, /* domainGetSchedulerType */
- NULL, /* domainGetSchedulerParameters */
- NULL, /* domainSetSchedulerParameters */
+ lxcGetSchedulerType, /* domainGetSchedulerType */
+ lxcGetSchedulerParameters, /* domainGetSchedulerParameters */
+ lxcSetSchedulerParameters, /* domainSetSchedulerParameters */
NULL, /* domainMigratePrepare */
NULL, /* domainMigratePerform */
NULL, /* domainMigrateFinish */
@@ -1211,7 +1300,6 @@
NULL, /* nodeGetCellsFreeMemory */
NULL, /* getFreeMemory */
};
-
static virStateDriver lxcStateDriver = {
lxcStartup,
11:15 a.m.
New subject: [libvirt] [PATCH 2 of 3] Add scheduling parameter support for LXC domains
On Tue, Oct 07, 2008 at 08:30:21AM -0700, Dan Smith wrote:
diff -r 6fb284fa200a -r ebecbe5caa03 src/lxc_driver.c
--- a/src/lxc_driver.c Tue Oct 07 08:21:49 2008 -0700
+++ b/src/lxc_driver.c Tue Oct 07 08:21:50 2008 -0700
@@ -35,6 +35,7 @@
#include <unistd.h>
#include <wait.h>
+#include "internal.h"
#include "lxc_conf.h"
#include "lxc_container.h"
#include "lxc_driver.h"
@@ -1149,6 +1150,94 @@
return 0;
}
+static char *lxcGetSchedulerType(virDomainPtr domain, int *nparams)
+{
+ if (nparams)
+ *nparams = 1;
+
+ return strdup("cgroup");
+}
cgroups isn't really the schedular here - its just the mechanism for
supplying the cpu_shares parameter to the schedular.
I sort of imagined that for Linux based VMs we'd key this off the
various schedulars available for sched_getscheduler(2), but
actually thinking about it more, its probably best to just say that
the schedular is 'posix', and make the choice of SCHED_FIFO,
SCHED_RR, SCHED_OTHER, etc be a tunable parameter.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
12:28 p.m.
New subject: [libvirt] [PATCH 2 of 3] Add scheduling parameter support for LXC domains
DB> cgroups isn't really the schedular here - its just the mechanism
DB> for supplying the cpu_shares parameter to the schedular.
That's true, although I was thinking along the lines of how kconfig
asks you for the method for grouping tasks, of which "cgroups" is an
option. But, yeah.
DB> I sort of imagined that for Linux based VMs we'd key this off the
DB> various schedulars available for sched_getscheduler(2), but
DB> actually thinking about it more, its probably best to just say
DB> that the schedular is 'posix', and make the choice of SCHED_FIFO,
DB> SCHED_RR, SCHED_OTHER, etc be a tunable parameter.
Okay, sounds good. I'll change it.
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
10:30 a.m.
New subject: [libvirt] [PATCH 3 of 3] Add cpu_shares support to virsh schedinfo
diff -r ebecbe5caa03 -r b07cd92a30e9 src/virsh.c
--- a/src/virsh.c Tue Oct 07 08:21:50 2008 -0700
+++ b/src/virsh.c Tue Oct 07 08:21:50 2008 -0700
@@ -1114,6 +1114,7 @@
{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id
or uuid")},
{"weight", VSH_OT_INT, VSH_OFLAG_NONE, gettext_noop("weight for
XEN_CREDIT")},
{"cap", VSH_OT_INT, VSH_OFLAG_NONE, gettext_noop("cap for
XEN_CREDIT")},
+ {"cpu_shares", VSH_OT_INT, VSH_OFLAG_NONE, gettext_noop("cpu shares
for LXC")},
{NULL, 0, 0, NULL}
};
@@ -1128,11 +1129,14 @@
int nr_inputparams = 0;
int inputparams = 0;
int weightfound = 0;
+ int sharesfound = 0;
int weight = 0;
int capfound = 0;
+ int shares = 0;
int cap = 0;
char str_weight[] = "weight";
char str_cap[] = "cap";
+ char str_shares[] = "cpu_shares";
int ret_val = FALSE;
if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
@@ -1152,6 +1156,7 @@
}
}
+ /* Currently supports Xen Credit only */
if(vshCommandOptBool(cmd, "cap")) {
cap = vshCommandOptInt(cmd, "cap", &capfound);
if (!capfound) {
@@ -1162,6 +1167,17 @@
}
}
+ /* Currently supports LXC cgroup only */
+ if(vshCommandOptBool(cmd, "cpu_shares")) {
+ shares = vshCommandOptInt(cmd, "cpu_shares", &sharesfound);
+ if (!sharesfound) {
+ vshError(ctl, FALSE, "%s", _("Invalid value of
cpu_shares"));
+ goto cleanup;
+ } else {
+ nr_inputparams++;
+ }
+ }
+
params = vshMalloc(ctl, sizeof (virSchedParameter) * nr_inputparams);
if (params == NULL) {
goto cleanup;
@@ -1180,7 +1196,13 @@
params[inputparams].value.ui = cap;
inputparams++;
}
- /* End Currently supports Xen Credit only */
+
+ if (sharesfound) {
+ strncpy(params[inputparams].field,str_shares,sizeof(str_shares));
+ params[inputparams].type = VIR_DOMAIN_SCHED_FIELD_UINT;
+ params[inputparams].value.ul = shares;
+ inputparams++;
+ }
assert (inputparams == nr_inputparams);
11:10 a.m.
New subject: [libvirt] [PATCH 3 of 3] Add cpu_shares support to virsh schedinfo
On Tue, Oct 07, 2008 at 08:30:22AM -0700, Dan Smith wrote:
diff -r ebecbe5caa03 -r b07cd92a30e9 src/virsh.c
--- a/src/virsh.c Tue Oct 07 08:21:50 2008 -0700
+++ b/src/virsh.c Tue Oct 07 08:21:50 2008 -0700
@@ -1114,6 +1114,7 @@
{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id
or uuid")},
{"weight", VSH_OT_INT, VSH_OFLAG_NONE, gettext_noop("weight for
XEN_CREDIT")},
{"cap", VSH_OT_INT, VSH_OFLAG_NONE, gettext_noop("cap for
XEN_CREDIT")},
+ {"cpu_shares", VSH_OT_INT, VSH_OFLAG_NONE, gettext_noop("cpu shares
for LXC")},
Not your fault, but this schedinfo command is a really dumb wrt
to option handling. Requiring that we add new options for every
possible schedular param for every possible hypervisor is just
crazy.
Instead of using a syntax
schedinfo --weight 10
it should use
schedinfo --set weight=10
so it automatically has support for every parameter supported
by the hypervisor in question. It can validate that the user
suplied param name is correct by calling 'virDomainGetSchedularParams'
to see what are available.
We need to implement a --set option, and deprecate use of the existing
--weight and --cap options in the help message/man page.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
11:16 a.m.
New subject: [libvirt] [PATCH 3 of 3] Add cpu_shares support to virsh schedinfo
DB> Not your fault, but this schedinfo command is a really dumb wrt to
DB> option handling. Requiring that we add new options for every
DB> possible schedular param for every possible hypervisor is just
DB> crazy.
Agreed. I actually just wrote this patch for my own testing, but
decided to send it out as part of the rest anyway.
DB> it should use
DB> schedinfo --set weight=10
Agreed :)
DB> so it automatically has support for every parameter supported by
DB> the hypervisor in question. It can validate that the user suplied
DB> param name is correct by calling 'virDomainGetSchedularParams' to
DB> see what are available.
I can do that, but since the CIM providers don't depend on virsh for
this, can we move forward with the first two patches before we get
this fixed up?
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
5868
days inactive
5868
days old
7 comments
2 participants
participants (2)
-
Dan Smith -
Daniel P. Berrange