Re: [libvirt] PATCH: 2/3: Implement schedular params for QEMU

2009-02-27 Thread Daniel Veillard
On Thu, Feb 26, 2009 at 04:40:18PM +, Daniel P. Berrange wrote:
 This patch implements the schedular parameter APIs. This adds a
 single tunable 'cpu_shares' that is provided by cgroups. This is
 a slightly more fancy way of doing nice priorities, giving a way
 to tune relative priority of  VMs

  The scheduler API is one of the parts I'm most afraid of from an
API perspective...

[...]
 +static int qemuSetSchedulerParameters(virDomainPtr domain,
 + virSchedParameterPtr params,
 + int nparams)
[...]
 +for (i = 0; i  nparams; i++) {
 +virSchedParameterPtr param = params[i];
 +
 +if (STREQ(param-field, cpu_shares)) {

  I think we should also check param-type to be of type 
VIR_DOMAIN_SCHED_FIELD_ULLONG before passing  value.ui
 +if (virCgroupSetCpuShares(group, params[i].value.ui) != 0)
  maybe use param-value.ui since we have a pointer handy, avoid
index trouble if the code evolves.

 +goto cleanup;
 +} else {
 +qemudReportError(domain-conn, domain, NULL, VIR_ERR_INVALID_ARG,
 + _(Invalid parameter `%s'), param-field);
 +goto cleanup;
 +}
 +}
 +ret = 0;

  Patch looks fine otherwise,

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

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


Re: [libvirt] PATCH: 2/3: Implement schedular params for QEMU

2009-02-26 Thread Daniel P. Berrange
This patch implements the schedular parameter APIs. This adds a
single tunable 'cpu_shares' that is provided by cgroups. This is
a slightly more fancy way of doing nice priorities, giving a way
to tune relative priority of  VMs

Daniel

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -3844,6 +3844,111 @@ cleanup:
 return ret;
 }
 
+
+static char *qemuGetSchedulerType(virDomainPtr domain ATTRIBUTE_UNUSED,
+ int *nparams)
+{
+if (nparams)
+*nparams = 1;
+
+return strdup(posix);
+}
+
+static int qemuSetSchedulerParameters(virDomainPtr domain,
+ virSchedParameterPtr params,
+ int nparams)
+{
+struct qemud_driver *driver = domain-conn-privateData;
+int i;
+virCgroupPtr group = NULL;
+virDomainObjPtr vm = NULL;
+int ret = -1;
+
+if (virCgroupHaveSupport() != 0)
+return -1;
+
+qemuDriverLock(driver);
+vm = virDomainFindByUUID(driver-domains, domain-uuid);
+qemuDriverUnlock(driver);
+
+if (vm == NULL) {
+qemudReportError(domain-conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
+ _(No such domain %s), domain-uuid);
+goto cleanup;
+}
+
+if (virCgroupForDomain(vm-def, qemu, group) != 0)
+goto cleanup;
+
+for (i = 0; i  nparams; i++) {
+virSchedParameterPtr param = params[i];
+
+if (STREQ(param-field, cpu_shares)) {
+if (virCgroupSetCpuShares(group, params[i].value.ui) != 0)
+goto cleanup;
+} else {
+qemudReportError(domain-conn, domain, NULL, VIR_ERR_INVALID_ARG,
+ _(Invalid parameter `%s'), param-field);
+goto cleanup;
+}
+}
+ret = 0;
+
+cleanup:
+virCgroupFree(group);
+if (vm)
+virDomainObjUnlock(vm);
+return ret;
+}
+
+static int qemuGetSchedulerParameters(virDomainPtr domain,
+ virSchedParameterPtr params,
+ int *nparams)
+{
+struct qemud_driver *driver = domain-conn-privateData;
+virCgroupPtr group = NULL;
+virDomainObjPtr vm = NULL;
+unsigned long val;
+int ret = -1;
+
+if (virCgroupHaveSupport() != 0)
+return -1;
+
+if ((*nparams) != 1) {
+qemudReportError(domain-conn, domain, NULL, VIR_ERR_INVALID_ARG,
+ %s, _(Invalid parameter count));
+return -1;
+}
+
+qemuDriverLock(driver);
+vm = virDomainFindByUUID(driver-domains, domain-uuid);
+qemuDriverUnlock(driver);
+
+if (vm == NULL) {
+qemudReportError(domain-conn, domain, NULL, VIR_ERR_INTERNAL_ERROR,
+ _(No such domain %s), domain-uuid);
+goto cleanup;
+}
+
+if (virCgroupForDomain(vm-def, qemu, group) != 0)
+goto cleanup;
+
+if (virCgroupGetCpuShares(group, val) != 0)
+goto cleanup;
+params[0].value.ul = val;
+strncpy(params[0].field, cpu_shares, sizeof(params[0].field));
+params[0].type = VIR_DOMAIN_SCHED_FIELD_ULLONG;
+
+ret = 0;
+
+cleanup:
+virCgroupFree(group);
+if (vm)
+virDomainObjUnlock(vm);
+return ret;
+}
+
+
 /* This uses the 'info blockstats' monitor command which was
  * integrated into both qemu  kvm in late 2007.  If the command is
  * not supported we detect this and return the appropriate error.
@@ -4650,9 +4755,9 @@ static virDriver qemuDriver = {
 qemudDomainDetachDevice, /* domainDetachDevice */
 qemudDomainGetAutostart, /* domainGetAutostart */
 qemudDomainSetAutostart, /* domainSetAutostart */
-NULL, /* domainGetSchedulerType */
-NULL, /* domainGetSchedulerParameters */
-NULL, /* domainSetSchedulerParameters */
+qemuGetSchedulerType, /* domainGetSchedulerType */
+qemuGetSchedulerParameters, /* domainGetSchedulerParameters */
+qemuSetSchedulerParameters, /* domainSetSchedulerParameters */
 NULL, /* domainMigratePrepare (v1) */
 qemudDomainMigratePerform, /* domainMigratePerform */
 NULL, /* domainMigrateFinish */

-- 
|: 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 :|

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