[libvirt] [PATCH v2.1 00/21] Supports for emulator-pin and emulator-bandwidth

This series adds support of emulator-pin to pin emulator threads on specified physical CPUs, and emulator-bandwidth to control physical CPU bandwidth for emulator threads. changes: v2.1: - rebase - include emulator-bandwidth patches - minor fix of virCgroupAddTaskStrController v2: - rename hypervisor to emulator all through the series - no Flags-suffix for new APIs - reduce code duplication - rollback to old vcpupin def if error Hu Tao (12): Introduce the function virCgroupMoveTask add function bitmapFromBytemap() to convert bytemap to bitmap refactor virDomainVcpuPinAdd() updates of some vcpupin related functions Enable cpuset cgroup and synchronous vcpupin info to cgroup. Change virDomainVcpuPinDefParseXML to support parsing emulatorpin qemu: support emulator pinning Add a new function vshPrintPinInfo. new command emulatorpin limit cpu bandwidth only for vcpus qemu: introduce period/quota tuning for emulator new interface to control emulator_period/emulator_quota Tang Chen (6): Support emulatorpin xml parse. qemu: synchronize emulatorpin info to cgroup Add qemuProcessSetEmulatorAffinites and set emulator threads affinities Introduce virDomainPinEmulator and virDomainGetEmulatorPinInfo functions. Introduce virDomainEmulatorPinAdd and virDomainEmulatorPinDel functions remote: introduce emulator pinning RPCs Wen Congyang (3): Introduce the function virCgroupForEmulator create a new cgroup and move all emulator threads to the new cgroup qemu: support of emulator_period and emulator_quota's modification daemon/remote.c | 91 ++++ docs/formatdomain.html.in | 33 ++ docs/schemas/domaincommon.rng | 17 + include/libvirt/libvirt.h.in | 34 +- src/conf/domain_conf.c | 300 ++++++++++--- src/conf/domain_conf.h | 17 +- src/driver.h | 12 + src/libvirt.c | 147 +++++++ src/libvirt_private.syms | 9 + src/libvirt_public.syms | 2 + src/libxl/libxl_driver.c | 13 +- src/qemu/qemu_cgroup.c | 173 ++++++-- src/qemu/qemu_cgroup.h | 7 + src/qemu/qemu_driver.c | 523 +++++++++++++++++++---- src/qemu/qemu_process.c | 60 ++- src/remote/remote_driver.c | 99 +++++ src/remote/remote_protocol.x | 21 +- src/remote_protocol-structs | 22 + src/util/cgroup.c | 187 +++++++- src/util/cgroup.h | 15 + src/xen/xend_internal.c | 13 +- tests/qemuxml2argvdata/qemuxml2argv-cputune.xml | 1 + tools/virsh-domain.c | 253 ++++++++++- tools/virsh.pod | 27 +- 24 files changed, 1868 insertions(+), 208 deletions(-) -- 1.7.10.2

From: Wen Congyang <wency@cn.fujitsu.com> Introduce the function virCgroupForEmulator() to create sub directory for simulator thread(include I/O thread, vhost-net thread) Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/libvirt_private.syms | 1 + src/util/cgroup.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/util/cgroup.h | 4 ++++ 3 files changed, 47 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a55fb73..6e834fe 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -71,6 +71,7 @@ virCgroupDenyDeviceMajor; virCgroupDenyDevicePath; virCgroupForDomain; virCgroupForDriver; +virCgroupForEmulator; virCgroupForVcpu; virCgroupFree; virCgroupGetBlkioWeight; diff --git a/src/util/cgroup.c b/src/util/cgroup.c index 2256c23..169b56a 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -957,6 +957,48 @@ int virCgroupForVcpu(virCgroupPtr driver ATTRIBUTE_UNUSED, #endif /** + * virCgroupForEmulator: + * + * @driver: group for the domain + * @group: Pointer to returned virCgroupPtr + * + * Returns: 0 on success or -errno on failure + */ +#if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R +int virCgroupForEmulator(virCgroupPtr driver, + virCgroupPtr *group, + int create) +{ + int rc; + char *path; + + if (driver == NULL) + return -EINVAL; + + if (virAsprintf(&path, "%s/emulator", driver->path) < 0) + return -ENOMEM; + + rc = virCgroupNew(path, group); + VIR_FREE(path); + + if (rc == 0) { + rc = virCgroupMakeGroup(driver, *group, create, VIR_CGROUP_VCPU); + if (rc != 0) + virCgroupFree(group); + } + + return rc; +} +#else +int virCgroupForEmulator(virCgroupPtr driver ATTRIBUTE_UNUSED, + virCgroupPtr *group ATTRIBUTE_UNUSED, + int create ATTRIBUTE_UNUSED) +{ + return -ENXIO; +} + +#endif +/** * virCgroupSetBlkioWeight: * * @group: The cgroup to change io weight for diff --git a/src/util/cgroup.h b/src/util/cgroup.h index 265f7c9..9f803a5 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -59,6 +59,10 @@ int virCgroupForVcpu(virCgroupPtr driver, virCgroupPtr *group, int create); +int virCgroupForEmulator(virCgroupPtr driver, + virCgroupPtr *group, + int create); + int virCgroupPathOfController(virCgroupPtr group, int controller, const char *key, -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:24PM +0800, Hu Tao wrote:
From: Wen Congyang <wency@cn.fujitsu.com>
Introduce the function virCgroupForEmulator() to create sub directory for simulator thread(include I/O thread, vhost-net thread)
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/libvirt_private.syms | 1 + src/util/cgroup.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/util/cgroup.h | 4 ++++ 3 files changed, 47 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a55fb73..6e834fe 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -71,6 +71,7 @@ virCgroupDenyDeviceMajor; virCgroupDenyDevicePath; virCgroupForDomain; virCgroupForDriver; +virCgroupForEmulator; virCgroupForVcpu; virCgroupFree; virCgroupGetBlkioWeight; diff --git a/src/util/cgroup.c b/src/util/cgroup.c index 2256c23..169b56a 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -957,6 +957,48 @@ int virCgroupForVcpu(virCgroupPtr driver ATTRIBUTE_UNUSED, #endif
/** + * virCgroupForEmulator: + * + * @driver: group for the domain + * @group: Pointer to returned virCgroupPtr + * + * Returns: 0 on success or -errno on failure + */ +#if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R +int virCgroupForEmulator(virCgroupPtr driver, + virCgroupPtr *group, + int create)
We should s/int create/bool create/ throughout the cgroups.[ch] files, but I'll ACK your patch anyway, since you're just following existing practice in this file. If you want to send a followup patch later to switch cgroups.c to use 'bool create' everywhere that'd be useful.
+{ + int rc; + char *path; + + if (driver == NULL) + return -EINVAL; + + if (virAsprintf(&path, "%s/emulator", driver->path) < 0) + return -ENOMEM; + + rc = virCgroupNew(path, group); + VIR_FREE(path); + + if (rc == 0) { + rc = virCgroupMakeGroup(driver, *group, create, VIR_CGROUP_VCPU); + if (rc != 0) + virCgroupFree(group); + } + + return rc; +} +#else +int virCgroupForEmulator(virCgroupPtr driver ATTRIBUTE_UNUSED, + virCgroupPtr *group ATTRIBUTE_UNUSED, + int create ATTRIBUTE_UNUSED) +{ + return -ENXIO; +} + +#endif +/** * virCgroupSetBlkioWeight: * * @group: The cgroup to change io weight for diff --git a/src/util/cgroup.h b/src/util/cgroup.h index 265f7c9..9f803a5 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -59,6 +59,10 @@ int virCgroupForVcpu(virCgroupPtr driver, virCgroupPtr *group, int create);
+int virCgroupForEmulator(virCgroupPtr driver, + virCgroupPtr *group, + int create); + int virCgroupPathOfController(virCgroupPtr group, int controller, const char *key,
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Introduce a new API to move tasks of one controller from a cgroup to another cgroup Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/libvirt_private.syms | 2 + src/util/cgroup.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/cgroup.h | 8 ++++ 3 files changed, 120 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6e834fe..0b0a70d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -60,6 +60,7 @@ virCapabilitiesSetMacPrefix; # cgroup.h virCgroupAddTask; +virCgroupAddTaskController; virCgroupAllowDevice; virCgroupAllowDeviceMajor; virCgroupAllowDevicePath; @@ -91,6 +92,7 @@ virCgroupKill; virCgroupKillPainfully; virCgroupKillRecursive; virCgroupMounted; +virCgroupMoveTask; virCgroupPathOfController; virCgroupRemove; virCgroupSetBlkioDeviceWeight; diff --git a/src/util/cgroup.c b/src/util/cgroup.c index 169b56a..24e08b4 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -802,6 +802,116 @@ int virCgroupAddTask(virCgroupPtr group, pid_t pid) return rc; } +/** + * virCgroupAddTaskController: + * + * @group: The cgroup to add a task to + * @pid: The pid of the task to add + * @controller: The cgroup controller to be operated on + * + * Returns: 0 on success or -errno on failure + */ +int virCgroupAddTaskController(virCgroupPtr group, pid_t pid, int controller) +{ + if (controller < VIR_CGROUP_CONTROLLER_CPU || + controller > VIR_CGROUP_CONTROLLER_BLKIO) + return -EINVAL; + + if (!group->controllers[controller].mountPoint) + return -EINVAL; + + return virCgroupSetValueU64(group, controller, "tasks", + (unsigned long long)pid); +} + + +static int virCgroupAddTaskStrController(virCgroupPtr group, + const char *pidstr, + int controller) +{ + char *str = NULL, *cur = NULL, *next = NULL; + unsigned long long p = 0; + int rc = 0; + char *endp; + + if (virAsprintf(&str, "%s", pidstr) < 0) + return -1; + + cur = str; + while (*cur != '\0') { + rc = virStrToLong_ull(cur, &endp, 10, &p); + if (rc != 0) + goto cleanup; + + rc = virCgroupAddTaskController(group, p, controller); + if (rc != 0) + goto cleanup; + + next = strchr(cur, '\n'); + if (next) { + cur = next + 1; + *next = '\0'; + } else { + break; + } + } + +cleanup: + VIR_FREE(str); + return rc; +} + +/** + * virCgroupMoveTask: + * + * @src_group: The source cgroup where all tasks are removed from + * @dest_group: The destination where all tasks are added to + * @controller: The cgroup controller to be operated on + * + * Returns: 0 on success or -errno on failure + */ +int virCgroupMoveTask(virCgroupPtr src_group, virCgroupPtr dest_group, + int controller) +{ + int rc = 0, err = 0; + char *content = NULL; + + if (controller < VIR_CGROUP_CONTROLLER_CPU || + controller > VIR_CGROUP_CONTROLLER_BLKIO) + return -EINVAL; + + if (!src_group->controllers[controller].mountPoint || + !dest_group->controllers[controller].mountPoint) { + VIR_WARN("no vm cgroup in controller %d", controller); + return 0; + } + + rc = virCgroupGetValueStr(src_group, controller, "tasks", &content); + if (rc != 0) + return rc; + + rc = virCgroupAddTaskStrController(dest_group, content, controller); + if (rc != 0) + goto cleanup; + + VIR_FREE(content); + + return 0; + +cleanup: + /* + * We don't need to recover dest_cgroup because cgroup will make sure + * that one task only resides in one cgroup of the same controller. + */ + err = virCgroupAddTaskStrController(src_group, content, controller); + if (err != 0) + VIR_ERROR(_("Cannot recover cgroup %s from %s"), + src_group->controllers[controller].mountPoint, + dest_group->controllers[controller].mountPoint); + VIR_FREE(content); + + return rc; +} /** * virCgroupForDriver: diff --git a/src/util/cgroup.h b/src/util/cgroup.h index 9f803a5..727e536 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -70,6 +70,14 @@ int virCgroupPathOfController(virCgroupPtr group, int virCgroupAddTask(virCgroupPtr group, pid_t pid); +int virCgroupAddTaskController(virCgroupPtr group, + pid_t pid, + int controller); + +int virCgroupMoveTask(virCgroupPtr src_group, + virCgroupPtr dest_group, + int controller); + int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight); int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight); -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:25PM +0800, Hu Tao wrote:
Introduce a new API to move tasks of one controller from a cgroup to another cgroup
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/libvirt_private.syms | 2 + src/util/cgroup.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/cgroup.h | 8 ++++ 3 files changed, 120 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6e834fe..0b0a70d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -60,6 +60,7 @@ virCapabilitiesSetMacPrefix;
# cgroup.h virCgroupAddTask; +virCgroupAddTaskController; virCgroupAllowDevice; virCgroupAllowDeviceMajor; virCgroupAllowDevicePath; @@ -91,6 +92,7 @@ virCgroupKill; virCgroupKillPainfully; virCgroupKillRecursive; virCgroupMounted; +virCgroupMoveTask; virCgroupPathOfController; virCgroupRemove; virCgroupSetBlkioDeviceWeight; diff --git a/src/util/cgroup.c b/src/util/cgroup.c index 169b56a..24e08b4 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -802,6 +802,116 @@ int virCgroupAddTask(virCgroupPtr group, pid_t pid) return rc; }
+/** + * virCgroupAddTaskController: + * + * @group: The cgroup to add a task to + * @pid: The pid of the task to add + * @controller: The cgroup controller to be operated on + * + * Returns: 0 on success or -errno on failure + */ +int virCgroupAddTaskController(virCgroupPtr group, pid_t pid, int controller) +{ + if (controller < VIR_CGROUP_CONTROLLER_CPU || + controller > VIR_CGROUP_CONTROLLER_BLKIO) + return -EINVAL;
This conditional should really be if (controller < 0 || controller >= VIR_CGROUP_CONTROLLER_LAST)
+ + if (!group->controllers[controller].mountPoint) + return -EINVAL; + + return virCgroupSetValueU64(group, controller, "tasks", + (unsigned long long)pid); +} + + +static int virCgroupAddTaskStrController(virCgroupPtr group, + const char *pidstr, + int controller) +{ + char *str = NULL, *cur = NULL, *next = NULL; + unsigned long long p = 0; + int rc = 0; + char *endp; + + if (virAsprintf(&str, "%s", pidstr) < 0) + return -1; + + cur = str; + while (*cur != '\0') { + rc = virStrToLong_ull(cur, &endp, 10, &p); + if (rc != 0) + goto cleanup; + + rc = virCgroupAddTaskController(group, p, controller); + if (rc != 0) + goto cleanup; + + next = strchr(cur, '\n'); + if (next) { + cur = next + 1; + *next = '\0'; + } else { + break; + } + } + +cleanup: + VIR_FREE(str); + return rc; +} + +/** + * virCgroupMoveTask: + * + * @src_group: The source cgroup where all tasks are removed from + * @dest_group: The destination where all tasks are added to + * @controller: The cgroup controller to be operated on + * + * Returns: 0 on success or -errno on failure + */ +int virCgroupMoveTask(virCgroupPtr src_group, virCgroupPtr dest_group, + int controller) +{ + int rc = 0, err = 0; + char *content = NULL; + + if (controller < VIR_CGROUP_CONTROLLER_CPU || + controller > VIR_CGROUP_CONTROLLER_BLKIO) + return -EINVAL; + + if (!src_group->controllers[controller].mountPoint || + !dest_group->controllers[controller].mountPoint) { + VIR_WARN("no vm cgroup in controller %d", controller); + return 0; + } + + rc = virCgroupGetValueStr(src_group, controller, "tasks", &content); + if (rc != 0) + return rc; + + rc = virCgroupAddTaskStrController(dest_group, content, controller); + if (rc != 0) + goto cleanup; + + VIR_FREE(content); + + return 0; + +cleanup: + /* + * We don't need to recover dest_cgroup because cgroup will make sure + * that one task only resides in one cgroup of the same controller. + */ + err = virCgroupAddTaskStrController(src_group, content, controller); + if (err != 0) + VIR_ERROR(_("Cannot recover cgroup %s from %s"), + src_group->controllers[controller].mountPoint, + dest_group->controllers[controller].mountPoint); + VIR_FREE(content); + + return rc; +}
/** * virCgroupForDriver: diff --git a/src/util/cgroup.h b/src/util/cgroup.h index 9f803a5..727e536 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -70,6 +70,14 @@ int virCgroupPathOfController(virCgroupPtr group,
int virCgroupAddTask(virCgroupPtr group, pid_t pid);
+int virCgroupAddTaskController(virCgroupPtr group, + pid_t pid, + int controller); + +int virCgroupMoveTask(virCgroupPtr src_group, + virCgroupPtr dest_group, + int controller); + int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight); int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight);
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

From: Wen Congyang <wency@cn.fujitsu.com> Create a new cgroup and move all emulator threads to the new cgroup. And then we can do the other things: 1. limit only vcpu usage rather than the whole qemu 2. limit for emulator threads(include vhost-net threads) Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/qemu/qemu_cgroup.c | 71 ++++++++++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_cgroup.h | 2 ++ src/qemu/qemu_process.c | 6 +++- 3 files changed, 74 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index b11d28b..8a5a536 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -533,11 +533,12 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) } if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) { - /* If we does not know VCPU<->PID mapping or all vcpu runs in the same + /* If we does not know VCPU<->PID mapping or all vcpus run in the same * thread, we cannot control each vcpu. */ - virCgroupFree(&cgroup); - return 0; + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to get vcpus' pids.")); + goto cleanup; } for (i = 0; i < priv->nvcpupids; i++) { @@ -574,7 +575,11 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) return 0; cleanup: - virCgroupFree(&cgroup_vcpu); + if (cgroup_vcpu) { + virCgroupRemove(cgroup_vcpu); + virCgroupFree(&cgroup_vcpu); + } + if (cgroup) { virCgroupRemove(cgroup); virCgroupFree(&cgroup); @@ -583,6 +588,64 @@ cleanup: return -1; } +int qemuSetupCgroupForEmulator(struct qemud_driver *driver, + virDomainObjPtr vm) +{ + virCgroupPtr cgroup = NULL; + virCgroupPtr cgroup_emulator = NULL; + int rc, i; + + if (driver->cgroup == NULL) + return 0; /* Not supported, so claim success */ + + rc = virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0); + if (rc != 0) { + virReportSystemError(-rc, + _("Unable to find cgroup for %s"), + vm->def->name); + goto cleanup; + } + + rc = virCgroupForEmulator(cgroup, &cgroup_emulator, 1); + if (rc < 0) { + virReportSystemError(-rc, + _("Unable to create emulator cgroup for %s"), + vm->def->name); + goto cleanup; + } + + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + if (!qemuCgroupControllerActive(driver, i)) { + VIR_WARN("cgroup %d is not active", i); + continue; + } + rc = virCgroupMoveTask(cgroup, cgroup_emulator, i); + if (rc < 0) { + virReportSystemError(-rc, + _("Unable to move tasks from domain cgroup to " + "emulator cgroup in controller %d for %s"), + i, vm->def->name); + goto cleanup; + } + } + + virCgroupFree(&cgroup_emulator); + virCgroupFree(&cgroup); + return 0; + +cleanup: + if (cgroup_emulator) { + virCgroupRemove(cgroup_emulator); + virCgroupFree(&cgroup_emulator); + } + + if (cgroup) { + virCgroupRemove(cgroup); + virCgroupFree(&cgroup); + } + + return rc; +} int qemuRemoveCgroup(struct qemud_driver *driver, virDomainObjPtr vm, diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index 5973430..34a9312 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -54,6 +54,8 @@ int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, unsigned long long period, long long quota); int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm); +int qemuSetupCgroupForEmulator(struct qemud_driver *driver, + virDomainObjPtr vm); int qemuRemoveCgroup(struct qemud_driver *driver, virDomainObjPtr vm, int quiet); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 8a9f995..762f298 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3752,10 +3752,14 @@ int qemuProcessStart(virConnectPtr conn, if (qemuProcessDetectVcpuPIDs(driver, vm) < 0) goto cleanup; - VIR_DEBUG("Setting cgroup for each VCPU(if required)"); + VIR_DEBUG("Setting cgroup for each VCPU (if required)"); if (qemuSetupCgroupForVcpu(driver, vm) < 0) goto cleanup; + VIR_DEBUG("Setting cgroup for emulator (if required)"); + if (qemuSetupCgroupForEmulator(driver, vm) < 0) + goto cleanup; + VIR_DEBUG("Setting VCPU affinities"); if (qemuProcessSetVcpuAffinites(conn, vm) < 0) goto cleanup; -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:26PM +0800, Hu Tao wrote:
From: Wen Congyang <wency@cn.fujitsu.com>
Create a new cgroup and move all emulator threads to the new cgroup. And then we can do the other things: 1. limit only vcpu usage rather than the whole qemu 2. limit for emulator threads(include vhost-net threads)
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/qemu/qemu_cgroup.c | 71 ++++++++++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_cgroup.h | 2 ++ src/qemu/qemu_process.c | 6 +++- 3 files changed, 74 insertions(+), 5 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

--- src/conf/domain_conf.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c9f5a3c..4e52177 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10893,36 +10893,47 @@ virDomainVcpuPinFindByVcpu(virDomainVcpuPinDefPtr *def, return NULL; } -int -virDomainVcpuPinAdd(virDomainDefPtr def, - unsigned char *cpumap, - int maplen, - int vcpu) +char *bitmapFromBytemap(unsigned char *bytemap, int maplen) { - virDomainVcpuPinDefPtr *vcpupin_list = NULL; - virDomainVcpuPinDefPtr vcpupin = NULL; - char *cpumask = NULL; + char *bitmap = NULL; int i; - if (VIR_ALLOC_N(cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) { + if (VIR_ALLOC_N(bitmap, VIR_DOMAIN_CPUMASK_LEN) < 0) { virReportOOMError(); goto cleanup; } - /* Reset cpumask to all 0s. */ + /* Reset bitmap to all 0s. */ for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; i++) - cpumask[i] = 0; + bitmap[i] = 0; - /* Convert bitmap (cpumap) to cpumask, which is byte map? */ + /* Convert bitmap (bytemap) to bitmap, which is byte map? */ for (i = 0; i < maplen; i++) { int cur; for (cur = 0; cur < 8; cur++) { - if (cpumap[i] & (1 << cur)) - cpumask[i * 8 + cur] = 1; + if (bytemap[i] & (1 << cur)) + bitmap[i * 8 + cur] = 1; } } +cleanup: + return bitmap; +} + +int +virDomainVcpuPinAdd(virDomainDefPtr def, + unsigned char *cpumap, + int maplen, + int vcpu) +{ + virDomainVcpuPinDefPtr *vcpupin_list = NULL; + virDomainVcpuPinDefPtr vcpupin = NULL; + char *cpumask = NULL; + + if ((cpumask = bitmapFromBytemap(cpumap, maplen)) == NULL) + goto cleanup; + /* No vcpupin exists yet. */ if (!def->cputune.nvcpupin) { if (VIR_ALLOC(vcpupin) < 0) { -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:27PM +0800, Hu Tao wrote:
--- src/conf/domain_conf.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Tue, Aug 21, 2012 at 05:18:27PM +0800, Hu Tao wrote:
--- src/conf/domain_conf.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c9f5a3c..4e52177 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10893,36 +10893,47 @@ virDomainVcpuPinFindByVcpu(virDomainVcpuPinDefPtr *def, return NULL; }
-int -virDomainVcpuPinAdd(virDomainDefPtr def, - unsigned char *cpumap, - int maplen, - int vcpu) +char *bitmapFromBytemap(unsigned char *bytemap, int maplen)
I had to make that function static as it is not used anywhere else and not exported .
{ - virDomainVcpuPinDefPtr *vcpupin_list = NULL; - virDomainVcpuPinDefPtr vcpupin = NULL; - char *cpumask = NULL; + char *bitmap = NULL; int i;
- if (VIR_ALLOC_N(cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) { + if (VIR_ALLOC_N(bitmap, VIR_DOMAIN_CPUMASK_LEN) < 0) { virReportOOMError(); goto cleanup; }
- /* Reset cpumask to all 0s. */ + /* Reset bitmap to all 0s. */ for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; i++) - cpumask[i] = 0; + bitmap[i] = 0;
- /* Convert bitmap (cpumap) to cpumask, which is byte map? */ + /* Convert bitmap (bytemap) to bitmap, which is byte map? */ for (i = 0; i < maplen; i++) { int cur;
for (cur = 0; cur < 8; cur++) { - if (cpumap[i] & (1 << cur)) - cpumask[i * 8 + cur] = 1; + if (bytemap[i] & (1 << cur)) + bitmap[i * 8 + cur] = 1; } }
+cleanup: + return bitmap; +} + +int +virDomainVcpuPinAdd(virDomainDefPtr def, + unsigned char *cpumap, + int maplen, + int vcpu) +{ + virDomainVcpuPinDefPtr *vcpupin_list = NULL; + virDomainVcpuPinDefPtr vcpupin = NULL; + char *cpumask = NULL; + + if ((cpumask = bitmapFromBytemap(cpumap, maplen)) == NULL) + goto cleanup; + /* No vcpupin exists yet. */ if (!def->cputune.nvcpupin) { if (VIR_ALLOC(vcpupin) < 0) {
Fine other wise, ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

--- src/conf/domain_conf.c | 81 ++++++++++++++++++---------------------------- src/conf/domain_conf.h | 3 +- src/libxl/libxl_driver.c | 13 +++++++- src/qemu/qemu_driver.c | 26 +++++++++++++-- src/xen/xend_internal.c | 13 +++++++- 5 files changed, 82 insertions(+), 54 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4e52177..56ee4c9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10921,69 +10921,52 @@ cleanup: return bitmap; } -int -virDomainVcpuPinAdd(virDomainDefPtr def, - unsigned char *cpumap, - int maplen, - int vcpu) +int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr *vcpupin_list, + int *nvcpupin, + unsigned char *cpumap, + int maplen, + int vcpu) { - virDomainVcpuPinDefPtr *vcpupin_list = NULL; virDomainVcpuPinDefPtr vcpupin = NULL; char *cpumask = NULL; - if ((cpumask = bitmapFromBytemap(cpumap, maplen)) == NULL) - goto cleanup; - - /* No vcpupin exists yet. */ - if (!def->cputune.nvcpupin) { - if (VIR_ALLOC(vcpupin) < 0) { - virReportOOMError(); - goto cleanup; - } + if (!vcpupin_list) + return -1; - if (VIR_ALLOC(vcpupin_list) < 0) { - virReportOOMError(); - VIR_FREE(vcpupin); - goto cleanup; - } + if ((cpumask = bitmapFromBytemap(cpumap, maplen)) == NULL) + return -1; + vcpupin = virDomainVcpuPinFindByVcpu(vcpupin_list, + *nvcpupin, + vcpu); + if (vcpupin) { vcpupin->vcpuid = vcpu; vcpupin->cpumask = cpumask; - vcpupin_list[def->cputune.nvcpupin++] = vcpupin; - def->cputune.vcpupin = vcpupin_list; - } else { - if (virDomainVcpuPinIsDuplicate(def->cputune.vcpupin, - def->cputune.nvcpupin, - vcpu)) { - vcpupin = virDomainVcpuPinFindByVcpu(def->cputune.vcpupin, - def->cputune.nvcpupin, - vcpu); - vcpupin->vcpuid = vcpu; - vcpupin->cpumask = cpumask; - } else { - if (VIR_ALLOC(vcpupin) < 0) { - virReportOOMError(); - goto cleanup; - } + return 0; + } + + /* No existing vcpupin matches vcpu, adding a new one */ + + if (VIR_ALLOC(vcpupin) < 0) { + virReportOOMError(); + VIR_FREE(cpumask); + return -1; + } + vcpupin->vcpuid = vcpu; + vcpupin->cpumask = cpumask; - if (VIR_REALLOC_N(def->cputune.vcpupin, def->cputune.nvcpupin + 1) < 0) { - virReportOOMError(); - VIR_FREE(vcpupin); - goto cleanup; - } - vcpupin->vcpuid = vcpu; - vcpupin->cpumask = cpumask; - def->cputune.vcpupin[def->cputune.nvcpupin++] = vcpupin; - } + if (VIR_REALLOC_N(vcpupin_list, *nvcpupin + 1) < 0) { + virReportOOMError(); + VIR_FREE(cpumask); + VIR_FREE(vcpupin); + return -1; } - return 0; + vcpupin_list[(*nvcpupin)++] = vcpupin; -cleanup: - VIR_FREE(cpumask); - return -1; + return 0; } int diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 74abe6c..30aef6b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1968,7 +1968,8 @@ int virDomainCpuSetParse(const char *str, char *virDomainCpuSetFormat(char *cpuset, int maxcpu); -int virDomainVcpuPinAdd(virDomainDefPtr def, +int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr *vcpupin_list, + int *nvcpupin, unsigned char *cpumap, int maplen, int vcpu); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 398a9a2..7881cd1 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2454,7 +2454,18 @@ libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned char *cpumap, goto cleanup; } - if (virDomainVcpuPinAdd(vm->def, cpumap, maplen, vcpu) < 0) { + if (!vm->def->cputune->vcpupin) { + if (VIR_ALLOC(vm->def->cputune->vcpupin) < 0) { + virReportOOMError(); + goto cleanup; + } + vm->def->cputune->nvcpupin = 0; + } + if (virDomainVcpuPinAdd(vm->def->cputune->vcpupin + &vm->def->cputune->nvcpupin, + cpumap, + maplen, + vcpu) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to update or add vcpupin xml")); goto cleanup; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 109d18d..17c66d7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3785,7 +3785,18 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, goto cleanup; } } else { - if (virDomainVcpuPinAdd(vm->def, cpumap, maplen, vcpu) < 0) { + if (!vm->def->cputune.vcpupin) { + if (VIR_ALLOC(vm->def->cputune.vcpupin) < 0) { + virReportOOMError(); + goto cleanup; + } + vm->def->cputune.nvcpupin = 0; + } + if (virDomainVcpuPinAdd(vm->def->cputune.vcpupin, + &vm->def->cputune.nvcpupin, + cpumap, + maplen, + vcpu) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to update or add vcpupin xml of " "a running domain")); @@ -3807,7 +3818,18 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, goto cleanup; } } else { - if (virDomainVcpuPinAdd(persistentDef, cpumap, maplen, vcpu) < 0) { + if (!persistentDef->cputune.vcpupin) { + if (VIR_ALLOC(persistentDef->cputune.vcpupin) < 0) { + virReportOOMError(); + goto cleanup; + } + persistentDef->cputune.nvcpupin = 0; + } + if (virDomainVcpuPinAdd(persistentDef->cputune.vcpupin, + &persistentDef->cputune.nvcpupin, + cpumap, + maplen, + vcpu) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to update or add vcpupin xml of " "a persistent domain")); diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index f93b249..f29f533 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -2296,7 +2296,18 @@ xenDaemonDomainPinVcpu(virDomainPtr domain, unsigned int vcpu, goto cleanup; if (ret == 0) { - if (virDomainVcpuPinAdd(def, cpumap, maplen, vcpu) < 0) { + if (!def->cputune.vcpupin) { + if (VIR_ALLOC(def->cputune.vcpupin) < 0) { + virReportOOMError(); + goto cleanup; + } + def->cputune.nvcpupin = 0; + } + if (virDomainVcpuPinAdd(def->cputune->vcpupin, + &def->cputune->nvcpupin, + cpumap, + maplen, + vcpu) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to add vcpupin xml entry")); return -1; -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:28PM +0800, Hu Tao wrote:
--- src/conf/domain_conf.c | 81 ++++++++++++++++++---------------------------- src/conf/domain_conf.h | 3 +- src/libxl/libxl_driver.c | 13 +++++++- src/qemu/qemu_driver.c | 26 +++++++++++++-- src/xen/xend_internal.c | 13 +++++++- 5 files changed, 82 insertions(+), 54 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4e52177..56ee4c9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10921,69 +10921,52 @@ cleanup: return bitmap; }
-int -virDomainVcpuPinAdd(virDomainDefPtr def, - unsigned char *cpumap, - int maplen, - int vcpu) +int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr *vcpupin_list, + int *nvcpupin, + unsigned char *cpumap, + int maplen, + int vcpu) { - virDomainVcpuPinDefPtr *vcpupin_list = NULL; virDomainVcpuPinDefPtr vcpupin = NULL; char *cpumask = NULL;
- if ((cpumask = bitmapFromBytemap(cpumap, maplen)) == NULL) - goto cleanup; - - /* No vcpupin exists yet. */ - if (!def->cputune.nvcpupin) { - if (VIR_ALLOC(vcpupin) < 0) { - virReportOOMError(); - goto cleanup; - } + if (!vcpupin_list) + return -1;
- if (VIR_ALLOC(vcpupin_list) < 0) { - virReportOOMError(); - VIR_FREE(vcpupin); - goto cleanup; - } + if ((cpumask = bitmapFromBytemap(cpumap, maplen)) == NULL) + return -1;
+ vcpupin = virDomainVcpuPinFindByVcpu(vcpupin_list, + *nvcpupin, + vcpu); + if (vcpupin) { vcpupin->vcpuid = vcpu; vcpupin->cpumask = cpumask; - vcpupin_list[def->cputune.nvcpupin++] = vcpupin;
- def->cputune.vcpupin = vcpupin_list; - } else { - if (virDomainVcpuPinIsDuplicate(def->cputune.vcpupin, - def->cputune.nvcpupin, - vcpu)) { - vcpupin = virDomainVcpuPinFindByVcpu(def->cputune.vcpupin, - def->cputune.nvcpupin, - vcpu); - vcpupin->vcpuid = vcpu; - vcpupin->cpumask = cpumask; - } else { - if (VIR_ALLOC(vcpupin) < 0) { - virReportOOMError(); - goto cleanup; - } + return 0; + } + + /* No existing vcpupin matches vcpu, adding a new one */ + + if (VIR_ALLOC(vcpupin) < 0) { + virReportOOMError(); + VIR_FREE(cpumask); + return -1; + } + vcpupin->vcpuid = vcpu; + vcpupin->cpumask = cpumask;
- if (VIR_REALLOC_N(def->cputune.vcpupin, def->cputune.nvcpupin + 1) < 0) { - virReportOOMError(); - VIR_FREE(vcpupin); - goto cleanup; - }
- vcpupin->vcpuid = vcpu; - vcpupin->cpumask = cpumask; - def->cputune.vcpupin[def->cputune.nvcpupin++] = vcpupin; - } + if (VIR_REALLOC_N(vcpupin_list, *nvcpupin + 1) < 0) { + virReportOOMError(); + VIR_FREE(cpumask); + VIR_FREE(vcpupin); + return -1; }
- return 0; + vcpupin_list[(*nvcpupin)++] = vcpupin;
-cleanup: - VIR_FREE(cpumask); - return -1; + return 0; }
int diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 74abe6c..30aef6b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1968,7 +1968,8 @@ int virDomainCpuSetParse(const char *str, char *virDomainCpuSetFormat(char *cpuset, int maxcpu);
-int virDomainVcpuPinAdd(virDomainDefPtr def, +int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr *vcpupin_list, + int *nvcpupin, unsigned char *cpumap, int maplen, int vcpu); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 398a9a2..7881cd1 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2454,7 +2454,18 @@ libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned char *cpumap, goto cleanup; }
- if (virDomainVcpuPinAdd(vm->def, cpumap, maplen, vcpu) < 0) { + if (!vm->def->cputune->vcpupin) { + if (VIR_ALLOC(vm->def->cputune->vcpupin) < 0) { + virReportOOMError(); + goto cleanup; + } + vm->def->cputune->nvcpupin = 0; + } + if (virDomainVcpuPinAdd(vm->def->cputune->vcpupin + &vm->def->cputune->nvcpupin, + cpumap, + maplen, + vcpu) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to update or add vcpupin xml")); goto cleanup; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 109d18d..17c66d7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3785,7 +3785,18 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, goto cleanup; } } else { - if (virDomainVcpuPinAdd(vm->def, cpumap, maplen, vcpu) < 0) { + if (!vm->def->cputune.vcpupin) { + if (VIR_ALLOC(vm->def->cputune.vcpupin) < 0) { + virReportOOMError(); + goto cleanup; + } + vm->def->cputune.nvcpupin = 0; + } + if (virDomainVcpuPinAdd(vm->def->cputune.vcpupin, + &vm->def->cputune.nvcpupin, + cpumap, + maplen, + vcpu) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to update or add vcpupin xml of " "a running domain")); @@ -3807,7 +3818,18 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, goto cleanup; } } else { - if (virDomainVcpuPinAdd(persistentDef, cpumap, maplen, vcpu) < 0) { + if (!persistentDef->cputune.vcpupin) { + if (VIR_ALLOC(persistentDef->cputune.vcpupin) < 0) { + virReportOOMError(); + goto cleanup; + } + persistentDef->cputune.nvcpupin = 0; + } + if (virDomainVcpuPinAdd(persistentDef->cputune.vcpupin, + &persistentDef->cputune.nvcpupin, + cpumap, + maplen, + vcpu) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to update or add vcpupin xml of " "a persistent domain")); diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index f93b249..f29f533 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -2296,7 +2296,18 @@ xenDaemonDomainPinVcpu(virDomainPtr domain, unsigned int vcpu, goto cleanup;
if (ret == 0) { - if (virDomainVcpuPinAdd(def, cpumap, maplen, vcpu) < 0) { + if (!def->cputune.vcpupin) { + if (VIR_ALLOC(def->cputune.vcpupin) < 0) { + virReportOOMError(); + goto cleanup; + } + def->cputune.nvcpupin = 0; + } + if (virDomainVcpuPinAdd(def->cputune->vcpupin, + &def->cputune->nvcpupin, + cpumap, + maplen, + vcpu) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to add vcpupin xml entry")); return -1;
Clearly you didn't tried to compile the xen drivers ! Please get a recent fedora and install xen on it to be able to catch those. i had to squash in the following. With that ACK, Daniel diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 7881cd1..d8ecf13 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2454,15 +2454,15 @@ libxlDomainPinVcpu(virDomainPtr dom, unsigned int vcpu, unsigned char *cpumap, goto cleanup; } - if (!vm->def->cputune->vcpupin) { - if (VIR_ALLOC(vm->def->cputune->vcpupin) < 0) { + if (!vm->def->cputune.vcpupin) { + if (VIR_ALLOC(vm->def->cputune.vcpupin) < 0) { virReportOOMError(); goto cleanup; } - vm->def->cputune->nvcpupin = 0; + vm->def->cputune.nvcpupin = 0; } - if (virDomainVcpuPinAdd(vm->def->cputune->vcpupin - &vm->def->cputune->nvcpupin, + if (virDomainVcpuPinAdd(vm->def->cputune.vcpupin, + &vm->def->cputune.nvcpupin, cpumap, maplen, vcpu) < 0) { diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index f29f533..99def42 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -2303,8 +2303,8 @@ xenDaemonDomainPinVcpu(virDomainPtr domain, unsigned int vcpu, } def->cputune.nvcpupin = 0; } - if (virDomainVcpuPinAdd(def->cputune->vcpupin, - &def->cputune->nvcpupin, + if (virDomainVcpuPinAdd(def->cputune.vcpupin, + &def->cputune.nvcpupin, cpumap, maplen, vcpu) < 0) { -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

1. add a new function virDomainVcpuPinDefCopy 2. make virDomainVcpuPinDefFree non-static --- src/conf/domain_conf.c | 34 +++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 5 +++++ src/libvirt_private.syms | 2 ++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 56ee4c9..ff27bc7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1505,7 +1505,39 @@ virDomainClockDefClear(virDomainClockDefPtr def) VIR_FREE(def->timers); } -static void +virDomainVcpuPinDefPtr * +virDomainVcpuPinDefCopy(virDomainVcpuPinDefPtr *src, int nvcpupin) +{ + int i = 0; + virDomainVcpuPinDefPtr *ret; + + if (VIR_ALLOC_N(ret, nvcpupin) < 0) { + goto nomem; + } + + for (i = 0; i < nvcpupin; i++) { + if (VIR_ALLOC(ret[i]) < 0) + goto nomem; + if (VIR_ALLOC_N(ret[i]->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) + goto nomem; + ret[i]->vcpuid = src[i]->vcpuid; + memcpy(ret[i]->cpumask, src[i]->cpumask, VIR_DOMAIN_CPUMASK_LEN); + } + + return ret; + +nomem: + while (i >= 0) { + VIR_FREE(ret[i]->cpumask); + VIR_FREE(ret[i]); + } + VIR_FREE(ret); + virReportOOMError(); + + return NULL; +} + +void virDomainVcpuPinDefFree(virDomainVcpuPinDefPtr *def, int nvcpupin) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 30aef6b..025c7fe 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1520,6 +1520,11 @@ struct _virDomainVcpuPinDef { char *cpumask; }; +void virDomainVcpuPinDefFree(virDomainVcpuPinDefPtr *def, int nvcpupin); + +virDomainVcpuPinDefPtr *virDomainVcpuPinDefCopy(virDomainVcpuPinDefPtr *src, + int nvcpupin); + int virDomainVcpuPinIsDuplicate(virDomainVcpuPinDefPtr *def, int nvcpupin, int vcpu); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0b0a70d..b513faf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -504,6 +504,8 @@ virDomainTimerTickpolicyTypeToString; virDomainTimerTrackTypeFromString; virDomainTimerTrackTypeToString; virDomainVcpuPinAdd; +virDomainVcpuPinDefCopy; +virDomainVcpuPinDefFree; virDomainVcpuPinDel; virDomainVcpuPinFindByVcpu; virDomainVcpuPinIsDuplicate; -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:29PM +0800, Hu Tao wrote:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 56ee4c9..ff27bc7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1505,7 +1505,39 @@ virDomainClockDefClear(virDomainClockDefPtr def) VIR_FREE(def->timers); }
-static void +virDomainVcpuPinDefPtr * +virDomainVcpuPinDefCopy(virDomainVcpuPinDefPtr *src, int nvcpupin) +{ + int i = 0; + virDomainVcpuPinDefPtr *ret; + + if (VIR_ALLOC_N(ret, nvcpupin) < 0) { + goto nomem; + }
Libvirt standard practice is to use 'no_memory' for the label label in this scenario. Also you don't need {} around a single line if()
+ + for (i = 0; i < nvcpupin; i++) { + if (VIR_ALLOC(ret[i]) < 0) + goto nomem; + if (VIR_ALLOC_N(ret[i]->cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) + goto nomem; + ret[i]->vcpuid = src[i]->vcpuid; + memcpy(ret[i]->cpumask, src[i]->cpumask, VIR_DOMAIN_CPUMASK_LEN); + } + + return ret; + +nomem: + while (i >= 0) { + VIR_FREE(ret[i]->cpumask); + VIR_FREE(ret[i]); + } + VIR_FREE(ret); + virReportOOMError(); + + return NULL; +} + +void virDomainVcpuPinDefFree(virDomainVcpuPinDefPtr *def, int nvcpupin) {
ACK with the label rename Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

vcpu threads pin are implemented using sched_setaffinity(), but not controlled by cgroup. This patch does the following things: 1) enable cpuset cgroup 2) reflect all the vcpu threads pin info to cgroup Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/libvirt_private.syms | 2 ++ src/qemu/qemu_cgroup.c | 43 ++++++++++++++++++++++++ src/qemu/qemu_cgroup.h | 4 +++ src/qemu/qemu_driver.c | 82 +++++++++++++++++++++++++++++++++------------- src/util/cgroup.c | 35 +++++++++++++++++++- src/util/cgroup.h | 3 ++ 6 files changed, 146 insertions(+), 23 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b513faf..80ea39a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -82,6 +82,7 @@ virCgroupGetCpuShares; virCgroupGetCpuacctPercpuUsage; virCgroupGetCpuacctStat; virCgroupGetCpuacctUsage; +virCgroupGetCpusetCpus; virCgroupGetCpusetMems; virCgroupGetFreezerState; virCgroupGetMemSwapHardLimit; @@ -100,6 +101,7 @@ virCgroupSetBlkioWeight; virCgroupSetCpuCfsPeriod; virCgroupSetCpuCfsQuota; virCgroupSetCpuShares; +virCgroupSetCpusetCpus; virCgroupSetCpusetMems; virCgroupSetFreezerState; virCgroupSetMemSwapHardLimit; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 8a5a536..37874d3 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -491,11 +491,45 @@ cleanup: return -1; } +int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, + virDomainVcpuPinDefPtr *vcpupin, + int nvcpupin, + int vcpuid) +{ + int i, rc = 0; + char *new_cpus = NULL; + + for (i = 0; i < nvcpupin; i++) { + if (vcpuid == vcpupin[i]->vcpuid) { + new_cpus = virDomainCpuSetFormat(vcpupin[i]->cpumask, + VIR_DOMAIN_CPUMASK_LEN); + if (!new_cpus) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to convert cpu mask")); + rc = -1; + goto cleanup; + } + rc = virCgroupSetCpusetCpus(cgroup, new_cpus); + if (rc != 0) { + virReportSystemError(-rc, + "%s", + _("Unable to set cpuset.cpus")); + goto cleanup; + } + } + } + +cleanup: + VIR_FREE(new_cpus); + return rc; +} + int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) { virCgroupPtr cgroup = NULL; virCgroupPtr cgroup_vcpu = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDefPtr def = vm->def; int rc; unsigned int i; unsigned long long period = vm->def->cputune.period; @@ -567,6 +601,15 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) } } + /* Set vcpupin in cgroup if vcpupin xml is provided */ + if (def->cputune.nvcpupin && + qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET) && + qemuSetupCgroupVcpuPin(cgroup_vcpu, + def->cputune.vcpupin, + def->cputune.nvcpupin, + i) < 0) + goto cleanup; + virCgroupFree(&cgroup_vcpu); } diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index 34a9312..fa93cdb 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -53,6 +53,10 @@ int qemuSetupCgroup(struct qemud_driver *driver, int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, unsigned long long period, long long quota); +int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, + virDomainVcpuPinDefPtr *vcpupin, + int nvcpupin, + int vcpuid); int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm); int qemuSetupCgroupForEmulator(struct qemud_driver *driver, virDomainObjPtr vm); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 17c66d7..4552172 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3714,11 +3714,15 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; virDomainDefPtr persistentDef = NULL; + virCgroupPtr cgroup_dom = NULL; + virCgroupPtr cgroup_vcpu = NULL; int maxcpu, hostcpus; virNodeInfo nodeinfo; int ret = -1; qemuDomainObjPrivatePtr priv; bool canResetting = true; + int newVcpuPinNum = 0; + virDomainVcpuPinDefPtr *newVcpuPin = NULL; int pcpu; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | @@ -3767,43 +3771,73 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_LIVE) { - if (priv->vcpupids != NULL) { - if (virProcessInfoSetAffinity(priv->vcpupids[vcpu], - cpumap, maplen, maxcpu) < 0) - goto cleanup; - } else { + if (priv->vcpupids == NULL) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cpu affinity is not supported")); goto cleanup; } - if (canResetting) { - if (virDomainVcpuPinDel(vm->def, vcpu) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to delete vcpupin xml of " - "a running domain")); + if (vm->def->cputune.vcpupin) { + newVcpuPin = virDomainVcpuPinDefCopy(vm->def->cputune.vcpupin, + vm->def->cputune.nvcpupin); + if (!newVcpuPin) + goto cleanup; + + newVcpuPinNum = vm->def->cputune.nvcpupin; + } else { + if (VIR_ALLOC(newVcpuPin) < 0) { + virReportOOMError(); + goto cleanup; + } + newVcpuPinNum = 0; + } + + if (virDomainVcpuPinAdd(newVcpuPin, &newVcpuPinNum, cpumap, maplen, vcpu) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to update vcpupin")); + virDomainVcpuPinDefFree(newVcpuPin, newVcpuPinNum); + goto cleanup; + } + + /* Configure the corresponding cpuset cgroup before set affinity. */ + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET)) { + if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup_dom, 0) == 0 && + virCgroupForVcpu(cgroup_dom, vcpu, &cgroup_vcpu, 0) == 0 && + qemuSetupCgroupVcpuPin(cgroup_vcpu, newVcpuPin, newVcpuPinNum, vcpu) < 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("failed to set cpuset.cpus in cgroup" + " for vcpu %d"), vcpu); goto cleanup; } } else { - if (!vm->def->cputune.vcpupin) { - if (VIR_ALLOC(vm->def->cputune.vcpupin) < 0) { - virReportOOMError(); - goto cleanup; - } - vm->def->cputune.nvcpupin = 0; + if (virProcessInfoSetAffinity(priv->vcpupids[vcpu], + cpumap, maplen, maxcpu) < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("failed to set cpu affinity for vcpu %d"), + vcpu); + goto cleanup; } - if (virDomainVcpuPinAdd(vm->def->cputune.vcpupin, - &vm->def->cputune.nvcpupin, - cpumap, - maplen, - vcpu) < 0) { + } + + if (canResetting) { + if (virDomainVcpuPinDel(vm->def, vcpu) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to update or add vcpupin xml of " + _("failed to delete vcpupin xml of " "a running domain")); goto cleanup; } + } else { + if (vm->def->cputune.vcpupin) + virDomainVcpuPinDefFree(vm->def->cputune.vcpupin, vm->def->cputune.nvcpupin); + + vm->def->cputune.vcpupin = newVcpuPin; + vm->def->cputune.nvcpupin = newVcpuPinNum; + newVcpuPin = NULL; } + if (newVcpuPin) + virDomainVcpuPinDefFree(newVcpuPin, newVcpuPinNum); + if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) goto cleanup; } @@ -3844,6 +3878,10 @@ qemudDomainPinVcpuFlags(virDomainPtr dom, ret = 0; cleanup: + if (cgroup_vcpu) + virCgroupFree(&cgroup_vcpu); + if (cgroup_dom) + virCgroupFree(&cgroup_dom); if (vm) virDomainObjUnlock(vm); return ret; diff --git a/src/util/cgroup.c b/src/util/cgroup.c index 24e08b4..1808911 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -543,7 +543,8 @@ static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, /* We need to control cpu bandwidth for each vcpu now */ if ((flags & VIR_CGROUP_VCPU) && (i != VIR_CGROUP_CONTROLLER_CPU && - i != VIR_CGROUP_CONTROLLER_CPUACCT)) { + i != VIR_CGROUP_CONTROLLER_CPUACCT && + i != VIR_CGROUP_CONTROLLER_CPUSET)) { /* treat it as unmounted and we can use virCgroupAddTask */ VIR_FREE(group->controllers[i].mountPoint); continue; @@ -1403,6 +1404,38 @@ int virCgroupGetCpusetMems(virCgroupPtr group, char **mems) } /** + * virCgroupSetCpusetCpus: + * + * @group: The cgroup to set cpuset.cpus for + * @cpus: the cpus to set + * + * Retuens: 0 on success + */ +int virCgroupSetCpusetCpus(virCgroupPtr group, const char *cpus) +{ + return virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_CPUSET, + "cpuset.cpus", + cpus); +} + +/** + * virCgroupGetCpusetCpus: + * + * @group: The cgroup to get cpuset.cpus for + * @cpus: the cpus to get + * + * Retuens: 0 on success + */ +int virCgroupGetCpusetCpus(virCgroupPtr group, char **cpus) +{ + return virCgroupGetValueStr(group, + VIR_CGROUP_CONTROLLER_CPUSET, + "cpuset.cpus", + cpus); +} + +/** * virCgroupDenyAllDevices: * * @group: The cgroup to deny all permissions, for all devices diff --git a/src/util/cgroup.h b/src/util/cgroup.h index 727e536..68ac232 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -151,6 +151,9 @@ int virCgroupGetFreezerState(virCgroupPtr group, char **state); int virCgroupSetCpusetMems(virCgroupPtr group, const char *mems); int virCgroupGetCpusetMems(virCgroupPtr group, char **mems); +int virCgroupSetCpusetCpus(virCgroupPtr group, const char *cpus); +int virCgroupGetCpusetCpus(virCgroupPtr group, char **cpus); + int virCgroupRemove(virCgroupPtr group); void virCgroupFree(virCgroupPtr *group); -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:30PM +0800, Hu Tao wrote:
vcpu threads pin are implemented using sched_setaffinity(), but not controlled by cgroup. This patch does the following things:
1) enable cpuset cgroup 2) reflect all the vcpu threads pin info to cgroup
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/libvirt_private.syms | 2 ++ src/qemu/qemu_cgroup.c | 43 ++++++++++++++++++++++++ src/qemu/qemu_cgroup.h | 4 +++ src/qemu/qemu_driver.c | 82 +++++++++++++++++++++++++++++++++------------- src/util/cgroup.c | 35 +++++++++++++++++++- src/util/cgroup.h | 3 ++ 6 files changed, 146 insertions(+), 23 deletions(-)
ACK Danielx -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

--- src/conf/domain_conf.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ff27bc7..62ba9de 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7860,7 +7860,19 @@ cleanup: return ret; } -/* Parse the XML definition for a vcpupin */ +/* Parse the XML definition for a vcpupin or emulatorpin. + * + * vcpupin has the form of + * + * <vcpupin vcpu='0' cpuset='0'/> + * + * and emulatorpin has the form of + * + * <emulatorpin cpuset='0'/> + * + * A vcpuid of -1 is valid and only valid for emulatorpin. So callers + * have to check the returned cpuid for validity. + */ static virDomainVcpuPinDefPtr virDomainVcpuPinDefParseXML(const xmlNodePtr node, xmlXPathContextPtr ctxt, @@ -7868,7 +7880,7 @@ virDomainVcpuPinDefParseXML(const xmlNodePtr node, { virDomainVcpuPinDefPtr def; xmlNodePtr oldnode = ctxt->node; - unsigned int vcpuid; + unsigned int vcpuid = -1; char *tmp = NULL; int ret; @@ -7884,13 +7896,9 @@ virDomainVcpuPinDefParseXML(const xmlNodePtr node, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("vcpu id must be an unsigned integer")); goto error; - } else if (ret == -1) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("can't parse vcpupin node")); - goto error; } - if (vcpuid >= maxvcpus) { + if (vcpuid != -1 && vcpuid >= maxvcpus) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("vcpu id must be less than maxvcpus")); goto error; -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:31PM +0800, Hu Tao wrote:
--- src/conf/domain_conf.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ff27bc7..62ba9de 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7860,7 +7860,19 @@ cleanup: return ret; }
-/* Parse the XML definition for a vcpupin */ +/* Parse the XML definition for a vcpupin or emulatorpin. + * + * vcpupin has the form of + * + * <vcpupin vcpu='0' cpuset='0'/> + * + * and emulatorpin has the form of + * + * <emulatorpin cpuset='0'/> + * + * A vcpuid of -1 is valid and only valid for emulatorpin. So callers + * have to check the returned cpuid for validity.
You didn't modify the existing caller to check this, nor does the new caller you added in the next patch check this. I'm not really a fan of this style of API. IMHO, you should pass in a parameter indicating whether 'vcpu' is allowed in the XML or not and then keep validation in this method.
static virDomainVcpuPinDefPtr virDomainVcpuPinDefParseXML(const xmlNodePtr node, xmlXPathContextPtr ctxt, @@ -7868,7 +7880,7 @@ virDomainVcpuPinDefParseXML(const xmlNodePtr node, { virDomainVcpuPinDefPtr def; xmlNodePtr oldnode = ctxt->node; - unsigned int vcpuid; + unsigned int vcpuid = -1; char *tmp = NULL; int ret;
@@ -7884,13 +7896,9 @@ virDomainVcpuPinDefParseXML(const xmlNodePtr node, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("vcpu id must be an unsigned integer")); goto error; - } else if (ret == -1) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("can't parse vcpupin node")); - goto error; }
- if (vcpuid >= maxvcpus) { + if (vcpuid != -1 && vcpuid >= maxvcpus) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("vcpu id must be less than maxvcpus")); goto error;
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Tue, Aug 21, 2012 at 05:44:10PM +0100, Daniel P. Berrange wrote:
On Tue, Aug 21, 2012 at 05:18:31PM +0800, Hu Tao wrote:
--- src/conf/domain_conf.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ff27bc7..62ba9de 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7860,7 +7860,19 @@ cleanup: return ret; }
-/* Parse the XML definition for a vcpupin */ +/* Parse the XML definition for a vcpupin or emulatorpin. + * + * vcpupin has the form of + * + * <vcpupin vcpu='0' cpuset='0'/> + * + * and emulatorpin has the form of + * + * <emulatorpin cpuset='0'/> + * + * A vcpuid of -1 is valid and only valid for emulatorpin. So callers + * have to check the returned cpuid for validity.
You didn't modify the existing caller to check this, nor does the new caller you added in the next patch check this. I'm not really a fan of this style of API. IMHO, you should pass in a parameter indicating whether 'vcpu' is allowed in the XML or not and then keep validation in this method.
Done, i reworked that the following way Daniel Change virDomainVcpuPinDefParseXML to support parsing emulatorpin diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5533355..ee247f6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7902,15 +7902,28 @@ cleanup: return ret; } -/* Parse the XML definition for a vcpupin */ +/* Parse the XML definition for a vcpupin or emulatorpin. + * + * vcpupin has the form of + * + * <vcpupin vcpu='0' cpuset='0'/> + * + * and emulatorpin has the form of + * + * <emulatorpin cpuset='0'/> + * + * A vcpuid of -1 is valid and only valid for emulatorpin. So callers + * have to check the returned cpuid for validity. + */ static virDomainVcpuPinDefPtr virDomainVcpuPinDefParseXML(const xmlNodePtr node, xmlXPathContextPtr ctxt, - int maxvcpus) + int maxvcpus, + int emulator) { virDomainVcpuPinDefPtr def; xmlNodePtr oldnode = ctxt->node; - unsigned int vcpuid; + int vcpuid; char *tmp = NULL; int ret; @@ -7921,14 +7934,14 @@ virDomainVcpuPinDefParseXML(const xmlNodePtr node, ctxt->node = node; - ret = virXPathUInt("string(./@vcpu)", ctxt, &vcpuid); - if (ret == -2) { + ret = virXPathInt("string(./@vcpu)", ctxt, &vcpuid); + if ((ret == -2) || (vcpuid < -1)) { virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("vcpu id must be an unsigned integer")); + "%s", _("vcpu id must be an unsigned integer or -1")); goto error; - } else if (ret == -1) { + } else if ((vcpuid == -1) && (emulator == 0)) { virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("can't parse vcpupin node")); + "%s", _("vcpu id value -1 is not allowed for vcpupin")); goto error; } @@ -8346,7 +8359,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, for (i = 0 ; i < n ; i++) { virDomainVcpuPinDefPtr vcpupin = NULL; - vcpupin = virDomainVcpuPinDefParseXML(nodes[i], ctxt, def->maxvcpus); + vcpupin = virDomainVcpuPinDefParseXML(nodes[i], ctxt, def->maxvcpus, 0); if (!vcpupin) goto error; -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

From: Tang Chen <tangchen@cn.fujitsu.com> This patch adds a new xml element <emulatorpin>, which is a sibling to the existing <vcpupin> element under the <cputune>, to pin emulator threads to specified physical CPUs. Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- docs/formatdomain.html.in | 9 ++++ docs/schemas/domaincommon.rng | 7 ++++ src/conf/domain_conf.c | 50 ++++++++++++++++++++++- src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-cputune.xml | 1 + 5 files changed, 66 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 8e07489..81ec2cd 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -384,6 +384,7 @@ <vcpupin vcpu="1" cpuset="0,1"/> <vcpupin vcpu="2" cpuset="2,3"/> <vcpupin vcpu="3" cpuset="0,4"/> + <emulatorpin cpuset="1-3"/%gt; <shares>2048</shares> <period>1000000</period> <quota>-1</quota> @@ -410,6 +411,14 @@ of element <code>vcpu</code>. (NB: Only qemu driver support) <span class="since">Since 0.9.0</span> </dd> + <dt><code>emulatorpin</code></dt> + <dd> + The optional <code>emulatorpin</code> element specifies which of host + physical CPUs the "emulator", a subset of a domain not including vcpu, + will be pinned to. If this is ommitted, "emulator" is pinned to all + the physical CPUs by default. It contains one required attribute + <code>cpuset</code> specifying which physical CPUs to pin to. + </dd> <dt><code>shares</code></dt> <dd> The optional <code>shares</code> element specifies the proportional diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 401b76b..b02ad96 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -591,6 +591,13 @@ </attribute> </element> </zeroOrMore> + <optional> + <element name="emulatorpin"> + <attribute name="cpuset"> + <ref name="cpuset"/> + </attribute> + </element> + </optional> </element> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 62ba9de..94ec095 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8330,6 +8330,34 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, } VIR_FREE(nodes); + if ((n = virXPathNodeSet("./cputune/emulatorpin", ctxt, &nodes)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot extract emulatorpin nodes")); + goto error; + } + + if (n) { + if (n > 1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only one emulatorpin is supported")); + VIR_FREE(nodes); + goto error; + } + + if (VIR_ALLOC(def->cputune.emulatorpin) < 0) { + goto no_memory; + } + + virDomainVcpuPinDefPtr emulatorpin = NULL; + emulatorpin = virDomainVcpuPinDefParseXML(nodes[0], ctxt, 0); + + if (!emulatorpin) + goto error; + + def->cputune.emulatorpin = emulatorpin; + } + VIR_FREE(nodes); + /* Extract numatune if exists. */ if ((n = virXPathNodeSet("./numatune", ctxt, &nodes)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -12930,7 +12958,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAsprintf(buf, ">%u</vcpu>\n", def->maxvcpus); if (def->cputune.shares || def->cputune.vcpupin || - def->cputune.period || def->cputune.quota) + def->cputune.period || def->cputune.quota || + def->cputune.emulatorpin) virBufferAddLit(buf, " <cputune>\n"); if (def->cputune.shares) @@ -12962,8 +12991,25 @@ virDomainDefFormatInternal(virDomainDefPtr def, } } + if (def->cputune.emulatorpin) { + virBufferAsprintf(buf, " <emulatorpin "); + + char *cpumask = NULL; + cpumask = virDomainCpuSetFormat(def->cputune.emulatorpin->cpumask, + VIR_DOMAIN_CPUMASK_LEN); + if (cpumask == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("failed to format cpuset for emulator")); + goto cleanup; + } + + virBufferAsprintf(buf, "cpuset='%s'/>\n", cpumask); + VIR_FREE(cpumask); + } + if (def->cputune.shares || def->cputune.vcpupin || - def->cputune.period || def->cputune.quota) + def->cputune.period || def->cputune.quota || + def->cputune.emulatorpin) virBufferAddLit(buf, " </cputune>\n"); if (def->numatune.memory.nodemask || diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 025c7fe..a7b2ff6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1600,6 +1600,7 @@ struct _virDomainDef { long long quota; int nvcpupin; virDomainVcpuPinDefPtr *vcpupin; + virDomainVcpuPinDefPtr emulatorpin; } cputune; virDomainNumatuneDef numatune; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cputune.xml b/tests/qemuxml2argvdata/qemuxml2argv-cputune.xml index df3101d..593e650 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cputune.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cputune.xml @@ -10,6 +10,7 @@ <quota>-1</quota> <vcpupin vcpu='0' cpuset='0'/> <vcpupin vcpu='1' cpuset='1'/> + <emulatorpin cpuset='1'/> </cputune> <os> <type arch='i686' machine='pc'>hvm</type> -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:32PM +0800, Hu Tao wrote:
From: Tang Chen <tangchen@cn.fujitsu.com>
This patch adds a new xml element <emulatorpin>, which is a sibling to the existing <vcpupin> element under the <cputune>, to pin emulator threads to specified physical CPUs.
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- docs/formatdomain.html.in | 9 ++++ docs/schemas/domaincommon.rng | 7 ++++ src/conf/domain_conf.c | 50 ++++++++++++++++++++++- src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-cputune.xml | 1 + 5 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 8e07489..81ec2cd 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -384,6 +384,7 @@ <vcpupin vcpu="1" cpuset="0,1"/> <vcpupin vcpu="2" cpuset="2,3"/> <vcpupin vcpu="3" cpuset="0,4"/> + <emulatorpin cpuset="1-3"/%gt; <shares>2048</shares> <period>1000000</period> <quota>-1</quota> @@ -410,6 +411,14 @@ of element <code>vcpu</code>. (NB: Only qemu driver support) <span class="since">Since 0.9.0</span> </dd> + <dt><code>emulatorpin</code></dt> + <dd> + The optional <code>emulatorpin</code> element specifies which of host + physical CPUs the "emulator", a subset of a domain not including vcpu, + will be pinned to. If this is ommitted, "emulator" is pinned to all + the physical CPUs by default. It contains one required attribute + <code>cpuset</code> specifying which physical CPUs to pin to. + </dd> <dt><code>shares</code></dt> <dd> The optional <code>shares</code> element specifies the proportional diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 401b76b..b02ad96 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -591,6 +591,13 @@ </attribute> </element> </zeroOrMore> + <optional> + <element name="emulatorpin"> + <attribute name="cpuset"> + <ref name="cpuset"/> + </attribute> + </element> + </optional> </element> </optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 62ba9de..94ec095 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8330,6 +8330,34 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, } VIR_FREE(nodes);
+ if ((n = virXPathNodeSet("./cputune/emulatorpin", ctxt, &nodes)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot extract emulatorpin nodes")); + goto error; + } + + if (n) { + if (n > 1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("only one emulatorpin is supported")); + VIR_FREE(nodes); + goto error; + } + + if (VIR_ALLOC(def->cputune.emulatorpin) < 0) { + goto no_memory; + } + + virDomainVcpuPinDefPtr emulatorpin = NULL; + emulatorpin = virDomainVcpuPinDefParseXML(nodes[0], ctxt, 0); + + if (!emulatorpin) + goto error; + + def->cputune.emulatorpin = emulatorpin; + } + VIR_FREE(nodes); + /* Extract numatune if exists. */ if ((n = virXPathNodeSet("./numatune", ctxt, &nodes)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -12930,7 +12958,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAsprintf(buf, ">%u</vcpu>\n", def->maxvcpus);
if (def->cputune.shares || def->cputune.vcpupin || - def->cputune.period || def->cputune.quota) + def->cputune.period || def->cputune.quota || + def->cputune.emulatorpin) virBufferAddLit(buf, " <cputune>\n");
if (def->cputune.shares) @@ -12962,8 +12991,25 @@ virDomainDefFormatInternal(virDomainDefPtr def, } }
+ if (def->cputune.emulatorpin) { + virBufferAsprintf(buf, " <emulatorpin "); + + char *cpumask = NULL; + cpumask = virDomainCpuSetFormat(def->cputune.emulatorpin->cpumask, + VIR_DOMAIN_CPUMASK_LEN); + if (cpumask == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("failed to format cpuset for emulator")); + goto cleanup; + } + + virBufferAsprintf(buf, "cpuset='%s'/>\n", cpumask); + VIR_FREE(cpumask); + } + if (def->cputune.shares || def->cputune.vcpupin || - def->cputune.period || def->cputune.quota) + def->cputune.period || def->cputune.quota || + def->cputune.emulatorpin) virBufferAddLit(buf, " </cputune>\n");
if (def->numatune.memory.nodemask || diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 025c7fe..a7b2ff6 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1600,6 +1600,7 @@ struct _virDomainDef { long long quota; int nvcpupin; virDomainVcpuPinDefPtr *vcpupin; + virDomainVcpuPinDefPtr emulatorpin; } cputune;
virDomainNumatuneDef numatune; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cputune.xml b/tests/qemuxml2argvdata/qemuxml2argv-cputune.xml index df3101d..593e650 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cputune.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cputune.xml @@ -10,6 +10,7 @@ <quota>-1</quota> <vcpupin vcpu='0' cpuset='0'/> <vcpupin vcpu='1' cpuset='1'/> + <emulatorpin cpuset='1'/> </cputune> <os> <type arch='i686' machine='pc'>hvm</type>
ACK, but it needed the following fixup to cope with the fix for patch 8 Daniel diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 59f22ac..304ab88 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8396,7 +8396,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, } virDomainVcpuPinDefPtr emulatorpin = NULL; - emulatorpin = virDomainVcpuPinDefParseXML(nodes[0], ctxt, 0); + emulatorpin = virDomainVcpuPinDefParseXML(nodes[0], ctxt, + def->maxvcpus, 1); if (!emulatorpin) goto error; -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

From: Tang Chen <tangchen@cn.fujitsu.com> Introduce qemuSetupCgroupEmulatorPin() function to add emulator threads pin info to cpuset cgroup, the same as vcpupin. Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/qemu/qemu_cgroup.c | 51 ++++++++++++++++++++++++++++++++---------------- src/qemu/qemu_cgroup.h | 1 + 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 37874d3..9bebfd5 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -496,29 +496,40 @@ int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, int nvcpupin, int vcpuid) { - int i, rc = 0; - char *new_cpus = NULL; + int i; for (i = 0; i < nvcpupin; i++) { if (vcpuid == vcpupin[i]->vcpuid) { - new_cpus = virDomainCpuSetFormat(vcpupin[i]->cpumask, - VIR_DOMAIN_CPUMASK_LEN); - if (!new_cpus) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to convert cpu mask")); - rc = -1; - goto cleanup; - } - rc = virCgroupSetCpusetCpus(cgroup, new_cpus); - if (rc != 0) { - virReportSystemError(-rc, - "%s", - _("Unable to set cpuset.cpus")); - goto cleanup; - } + return qemuSetupCgroupEmulatorPin(cgroup, vcpupin[i]); } } + return -1; +} + +int qemuSetupCgroupEmulatorPin(virCgroupPtr cgroup, + virDomainVcpuPinDefPtr vcpupin) +{ + int rc = 0; + char *new_cpus = NULL; + + new_cpus = virDomainCpuSetFormat(vcpupin->cpumask, + VIR_DOMAIN_CPUMASK_LEN); + if (!new_cpus) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to convert cpu mask")); + rc = -1; + goto cleanup; + } + + rc = virCgroupSetCpusetCpus(cgroup, new_cpus); + if (rc < 0) { + virReportSystemError(-rc, + "%s", + _("Unable to set cpuset.cpus")); + goto cleanup; + } + cleanup: VIR_FREE(new_cpus); return rc; @@ -636,6 +647,7 @@ int qemuSetupCgroupForEmulator(struct qemud_driver *driver, { virCgroupPtr cgroup = NULL; virCgroupPtr cgroup_emulator = NULL; + virDomainDefPtr def = vm->def; int rc, i; if (driver->cgroup == NULL) @@ -672,6 +684,11 @@ int qemuSetupCgroupForEmulator(struct qemud_driver *driver, } } + if (def->cputune.emulatorpin && + qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET) && + qemuSetupCgroupEmulatorPin(cgroup_emulator, def->cputune.emulatorpin) < 0) + goto cleanup; + virCgroupFree(&cgroup_emulator); virCgroupFree(&cgroup); return 0; diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index fa93cdb..04f70a1 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -57,6 +57,7 @@ int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, virDomainVcpuPinDefPtr *vcpupin, int nvcpupin, int vcpuid); +int qemuSetupCgroupEmulatorPin(virCgroupPtr cgroup, virDomainVcpuPinDefPtr vcpupin); int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm); int qemuSetupCgroupForEmulator(struct qemud_driver *driver, virDomainObjPtr vm); -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:33PM +0800, Hu Tao wrote:
From: Tang Chen <tangchen@cn.fujitsu.com>
Introduce qemuSetupCgroupEmulatorPin() function to add emulator threads pin info to cpuset cgroup, the same as vcpupin.
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/qemu/qemu_cgroup.c | 51 ++++++++++++++++++++++++++++++++---------------- src/qemu/qemu_cgroup.h | 1 + 2 files changed, 35 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 37874d3..9bebfd5 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -496,29 +496,40 @@ int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, int nvcpupin, int vcpuid) { - int i, rc = 0; - char *new_cpus = NULL; + int i;
for (i = 0; i < nvcpupin; i++) { if (vcpuid == vcpupin[i]->vcpuid) { - new_cpus = virDomainCpuSetFormat(vcpupin[i]->cpumask, - VIR_DOMAIN_CPUMASK_LEN); - if (!new_cpus) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to convert cpu mask")); - rc = -1; - goto cleanup; - } - rc = virCgroupSetCpusetCpus(cgroup, new_cpus); - if (rc != 0) { - virReportSystemError(-rc, - "%s", - _("Unable to set cpuset.cpus")); - goto cleanup; - } + return qemuSetupCgroupEmulatorPin(cgroup, vcpupin[i]); } }
+ return -1; +} + +int qemuSetupCgroupEmulatorPin(virCgroupPtr cgroup, + virDomainVcpuPinDefPtr vcpupin) +{ + int rc = 0; + char *new_cpus = NULL; + + new_cpus = virDomainCpuSetFormat(vcpupin->cpumask, + VIR_DOMAIN_CPUMASK_LEN); + if (!new_cpus) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to convert cpu mask")); + rc = -1; + goto cleanup; + } + + rc = virCgroupSetCpusetCpus(cgroup, new_cpus); + if (rc < 0) { + virReportSystemError(-rc, + "%s", + _("Unable to set cpuset.cpus")); + goto cleanup; + } + cleanup: VIR_FREE(new_cpus); return rc; @@ -636,6 +647,7 @@ int qemuSetupCgroupForEmulator(struct qemud_driver *driver, { virCgroupPtr cgroup = NULL; virCgroupPtr cgroup_emulator = NULL; + virDomainDefPtr def = vm->def; int rc, i;
if (driver->cgroup == NULL) @@ -672,6 +684,11 @@ int qemuSetupCgroupForEmulator(struct qemud_driver *driver, } }
+ if (def->cputune.emulatorpin && + qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPUSET) && + qemuSetupCgroupEmulatorPin(cgroup_emulator, def->cputune.emulatorpin) < 0) + goto cleanup; + virCgroupFree(&cgroup_emulator); virCgroupFree(&cgroup); return 0; diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index fa93cdb..04f70a1 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -57,6 +57,7 @@ int qemuSetupCgroupVcpuPin(virCgroupPtr cgroup, virDomainVcpuPinDefPtr *vcpupin, int nvcpupin, int vcpuid); +int qemuSetupCgroupEmulatorPin(virCgroupPtr cgroup, virDomainVcpuPinDefPtr vcpupin); int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm); int qemuSetupCgroupForEmulator(struct qemud_driver *driver, virDomainObjPtr vm);
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

From: Tang Chen <tangchen@cn.fujitsu.com> Emulator threads should also be pinned by sched_setaffinity(), just the same as vcpu threads. Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/qemu/qemu_process.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 762f298..90d44c4 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2002,6 +2002,56 @@ cleanup: return ret; } +/* Set CPU affinities for emulator threads if emulatorpin xml provided. */ +static int +qemuProcessSetEmulatorAffinites(virConnectPtr conn, + virDomainObjPtr vm) +{ + virDomainDefPtr def = vm->def; + pid_t pid = vm->pid; + unsigned char *cpumask = NULL; + unsigned char *cpumap = NULL; + virNodeInfo nodeinfo; + int cpumaplen, hostcpus, maxcpu, i; + int ret = -1; + + if (virNodeGetInfo(conn, &nodeinfo) != 0) + return -1; + + if (!def->cputune.emulatorpin) + return 0; + + hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); + cpumaplen = VIR_CPU_MAPLEN(hostcpus); + maxcpu = cpumaplen * CHAR_BIT; + + if (maxcpu > hostcpus) + maxcpu = hostcpus; + + if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) { + virReportOOMError(); + return -1; + } + + cpumask = (unsigned char *)def->cputune.emulatorpin->cpumask; + for(i = 0; i < VIR_DOMAIN_CPUMASK_LEN; i++) { + if (cpumask[i]) + VIR_USE_CPU(cpumap, i); + } + + if (virProcessInfoSetAffinity(pid, + cpumap, + cpumaplen, + maxcpu) < 0) { + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FREE(cpumap); + return ret; +} + static int qemuProcessInitPasswords(virConnectPtr conn, struct qemud_driver *driver, @@ -3764,6 +3814,10 @@ int qemuProcessStart(virConnectPtr conn, if (qemuProcessSetVcpuAffinites(conn, vm) < 0) goto cleanup; + VIR_DEBUG("Setting affinity of emulator threads"); + if (qemuProcessSetEmulatorAffinites(conn, vm) < 0) + goto cleanup; + VIR_DEBUG("Setting any required VM passwords"); if (qemuProcessInitPasswords(conn, driver, vm) < 0) goto cleanup; -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:34PM +0800, Hu Tao wrote:
From: Tang Chen <tangchen@cn.fujitsu.com>
Emulator threads should also be pinned by sched_setaffinity(), just the same as vcpu threads.
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/qemu/qemu_process.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 762f298..90d44c4 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2002,6 +2002,56 @@ cleanup: return ret; }
+/* Set CPU affinities for emulator threads if emulatorpin xml provided. */ +static int +qemuProcessSetEmulatorAffinites(virConnectPtr conn, + virDomainObjPtr vm) +{ + virDomainDefPtr def = vm->def; + pid_t pid = vm->pid; + unsigned char *cpumask = NULL; + unsigned char *cpumap = NULL; + virNodeInfo nodeinfo; + int cpumaplen, hostcpus, maxcpu, i; + int ret = -1; + + if (virNodeGetInfo(conn, &nodeinfo) != 0) + return -1; + + if (!def->cputune.emulatorpin) + return 0; + + hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); + cpumaplen = VIR_CPU_MAPLEN(hostcpus); + maxcpu = cpumaplen * CHAR_BIT; + + if (maxcpu > hostcpus) + maxcpu = hostcpus; + + if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) { + virReportOOMError(); + return -1; + } + + cpumask = (unsigned char *)def->cputune.emulatorpin->cpumask; + for(i = 0; i < VIR_DOMAIN_CPUMASK_LEN; i++) {
missing space between for and (
+ if (cpumask[i]) + VIR_USE_CPU(cpumap, i); + } + + if (virProcessInfoSetAffinity(pid, + cpumap, + cpumaplen, + maxcpu) < 0) { + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FREE(cpumap); + return ret; +} + static int qemuProcessInitPasswords(virConnectPtr conn, struct qemud_driver *driver, @@ -3764,6 +3814,10 @@ int qemuProcessStart(virConnectPtr conn, if (qemuProcessSetVcpuAffinites(conn, vm) < 0) goto cleanup;
+ VIR_DEBUG("Setting affinity of emulator threads"); + if (qemuProcessSetEmulatorAffinites(conn, vm) < 0) + goto cleanup; + VIR_DEBUG("Setting any required VM passwords"); if (qemuProcessInitPasswords(conn, driver, vm) < 0) goto cleanup;
Looks okay, ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

From: Tang Chen <tangchen@cn.fujitsu.com> Introduce 2 APIs to set/get physical cpu pinning info of emulator threads. Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- include/libvirt/libvirt.h.in | 10 +++ src/driver.h | 12 ++++ src/libvirt.c | 147 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 2 + 4 files changed, 171 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 77a061e..43774eb 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1916,6 +1916,16 @@ int virDomainGetVcpuPinInfo (virDomainPtr domain, int maplen, unsigned int flags); +int virDomainPinEmulator (virDomainPtr domain, + unsigned char *cpumap, + int maplen, + unsigned int flags); + +int virDomainGetEmulatorPinInfo (virDomainPtr domain, + unsigned char *cpumaps, + int maplen, + unsigned int flags); + /** * VIR_USE_CPU: * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN/OUT) diff --git a/src/driver.h b/src/driver.h index 203497d..5fa2d36 100644 --- a/src/driver.h +++ b/src/driver.h @@ -306,6 +306,16 @@ typedef int unsigned char *cpumaps, int maplen, unsigned int flags); + typedef int + (*virDrvDomainPinEmulator) (virDomainPtr domain, + unsigned char *cpumap, + int maplen, + unsigned int flags); +typedef int + (*virDrvDomainGetEmulatorPinInfo) (virDomainPtr domain, + unsigned char *cpumaps, + int maplen, + unsigned int flags); typedef int (*virDrvDomainGetVcpus) (virDomainPtr domain, @@ -941,6 +951,8 @@ struct _virDriver { virDrvDomainPinVcpu domainPinVcpu; virDrvDomainPinVcpuFlags domainPinVcpuFlags; virDrvDomainGetVcpuPinInfo domainGetVcpuPinInfo; + virDrvDomainPinEmulator domainPinEmulator; + virDrvDomainGetEmulatorPinInfo domainGetEmulatorPinInfo; virDrvDomainGetVcpus domainGetVcpus; virDrvDomainGetMaxVcpus domainGetMaxVcpus; virDrvDomainGetSecurityLabel domainGetSecurityLabel; diff --git a/src/libvirt.c b/src/libvirt.c index b3fc8a8..60ce6d1 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8858,6 +8858,153 @@ error: } /** + * virDomainPinEmulator: + * @domain: pointer to domain object, or NULL for Domain0 + * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN) + * Each bit set to 1 means that corresponding CPU is usable. + * Bytes are stored in little-endian order: CPU0-7, 8-15... + * In each byte, lowest CPU number is least significant bit. + * @maplen: number of bytes in cpumap, from 1 up to size of CPU map in + * underlying virtualization system (Xen...). + * If maplen < size, missing bytes are set to zero. + * If maplen > size, failure code is returned. + * @flags: bitwise-OR of virDomainModificationImpact + * + * Dynamically change the real CPUs which can be allocated to all emulator + * threads. This function may require privileged access to the hypervisor. + * + * @flags may include VIR_DOMAIN_AFFECT_LIVE or VIR_DOMAIN_AFFECT_CONFIG. + * Both flags may be set. + * If VIR_DOMAIN_AFFECT_LIVE is set, the change affects a running domain + * and may fail if domain is not alive. + * If VIR_DOMAIN_AFFECT_CONFIG is set, the change affects persistent state, + * and will fail for transient domains. If neither flag is specified (that is, + * @flags is VIR_DOMAIN_AFFECT_CURRENT), then an inactive domain modifies + * persistent setup, while an active domain is hypervisor-dependent on whether + * just live or both live and persistent state is changed. + * Not all hypervisors can support all flag combinations. + * + * See also virDomainGetEmulatorPinInfo for querying this information. + * + * Returns 0 in case of success, -1 in case of failure. + * + */ +int +virDomainPinEmulator(virDomainPtr domain, unsigned char *cpumap, + int maplen, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "cpumap=%p, maplen=%d, flags=%x", + cpumap, maplen, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (domain->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if ((cpumap == NULL) || (maplen < 1)) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + conn = domain->conn; + + if (conn->driver->domainPinEmulator) { + int ret; + ret = conn->driver->domainPinEmulator (domain, cpumap, maplen, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + +/** + * virDomainGetEmulatorPinInfo: + * @domain: pointer to domain object, or NULL for Domain0 + * @cpumap: pointer to a bit map of real CPUs for all emulator threads of + * this domain (in 8-bit bytes) (OUT) + * There is only one cpumap for all emulator threads. + * Must not be NULL. + * @maplen: the number of bytes in one cpumap, from 1 up to size of CPU map. + * Must be positive. + * @flags: bitwise-OR of virDomainModificationImpact + * Must not be VIR_DOMAIN_AFFECT_LIVE and + * VIR_DOMAIN_AFFECT_CONFIG concurrently. + * + * Query the CPU affinity setting of all emulator threads of domain, store + * it in cpumap. + * + * Returns 1 in case of success, + * 0 in case of no emulator threads are pined to pcpus, + * -1 in case of failure. + */ +int +virDomainGetEmulatorPinInfo(virDomainPtr domain, unsigned char *cpumap, + int maplen, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "cpumap=%p, maplen=%d, flags=%x", + cpumap, maplen, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (!cpumap || maplen <= 0) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + if (INT_MULTIPLY_OVERFLOW(1, maplen)) { + virLibDomainError(VIR_ERR_OVERFLOW, _("input too large: 1 * %d"), + maplen); + goto error; + } + + /* At most one of these two flags should be set. */ + if ((flags & VIR_DOMAIN_AFFECT_LIVE) && + (flags & VIR_DOMAIN_AFFECT_CONFIG)) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + conn = domain->conn; + + if (conn->driver->domainGetEmulatorPinInfo) { + int ret; + ret = conn->driver->domainGetEmulatorPinInfo(domain, cpumap, + maplen, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + +/** * virDomainGetVcpus: * @domain: pointer to domain object, or NULL for Domain0 * @info: pointer to an array of virVcpuInfo structures (OUT) diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 43f2cf2..92ae95a 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -550,6 +550,8 @@ LIBVIRT_0.10.0 { virConnectRegisterCloseCallback; virConnectUnregisterCloseCallback; virDomainGetSecurityLabelList; + virDomainPinEmulator; + virDomainGetEmulatorPinInfo; } LIBVIRT_0.9.13; # .... define new API here using predicted next version number .... -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:35PM +0800, Hu Tao wrote:
From: Tang Chen <tangchen@cn.fujitsu.com>
Introduce 2 APIs to set/get physical cpu pinning info of emulator threads.
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- include/libvirt/libvirt.h.in | 10 +++ src/driver.h | 12 ++++ src/libvirt.c | 147 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 2 + 4 files changed, 171 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 77a061e..43774eb 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1916,6 +1916,16 @@ int virDomainGetVcpuPinInfo (virDomainPtr domain, int maplen, unsigned int flags);
+int virDomainPinEmulator (virDomainPtr domain, + unsigned char *cpumap, + int maplen, + unsigned int flags); + +int virDomainGetEmulatorPinInfo (virDomainPtr domain, + unsigned char *cpumaps, + int maplen, + unsigned int flags); +
okay, based on existing APIs like virDomainPinVcpuFlags that is consistent,
/** * VIR_USE_CPU: * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN/OUT) diff --git a/src/driver.h b/src/driver.h index 203497d..5fa2d36 100644 --- a/src/driver.h +++ b/src/driver.h @@ -306,6 +306,16 @@ typedef int unsigned char *cpumaps, int maplen, unsigned int flags); + typedef int + (*virDrvDomainPinEmulator) (virDomainPtr domain, + unsigned char *cpumap, + int maplen, + unsigned int flags); +typedef int + (*virDrvDomainGetEmulatorPinInfo) (virDomainPtr domain, + unsigned char *cpumaps, + int maplen, + unsigned int flags);
typedef int (*virDrvDomainGetVcpus) (virDomainPtr domain, @@ -941,6 +951,8 @@ struct _virDriver { virDrvDomainPinVcpu domainPinVcpu; virDrvDomainPinVcpuFlags domainPinVcpuFlags; virDrvDomainGetVcpuPinInfo domainGetVcpuPinInfo; + virDrvDomainPinEmulator domainPinEmulator; + virDrvDomainGetEmulatorPinInfo domainGetEmulatorPinInfo; virDrvDomainGetVcpus domainGetVcpus; virDrvDomainGetMaxVcpus domainGetMaxVcpus; virDrvDomainGetSecurityLabel domainGetSecurityLabel; diff --git a/src/libvirt.c b/src/libvirt.c index b3fc8a8..60ce6d1 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8858,6 +8858,153 @@ error: }
/** + * virDomainPinEmulator: + * @domain: pointer to domain object, or NULL for Domain0 + * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN) + * Each bit set to 1 means that corresponding CPU is usable. + * Bytes are stored in little-endian order: CPU0-7, 8-15... + * In each byte, lowest CPU number is least significant bit. + * @maplen: number of bytes in cpumap, from 1 up to size of CPU map in + * underlying virtualization system (Xen...). + * If maplen < size, missing bytes are set to zero. + * If maplen > size, failure code is returned. + * @flags: bitwise-OR of virDomainModificationImpact + * + * Dynamically change the real CPUs which can be allocated to all emulator + * threads. This function may require privileged access to the hypervisor. + * + * @flags may include VIR_DOMAIN_AFFECT_LIVE or VIR_DOMAIN_AFFECT_CONFIG. + * Both flags may be set. + * If VIR_DOMAIN_AFFECT_LIVE is set, the change affects a running domain + * and may fail if domain is not alive. + * If VIR_DOMAIN_AFFECT_CONFIG is set, the change affects persistent state, + * and will fail for transient domains. If neither flag is specified (that is, + * @flags is VIR_DOMAIN_AFFECT_CURRENT), then an inactive domain modifies + * persistent setup, while an active domain is hypervisor-dependent on whether + * just live or both live and persistent state is changed. + * Not all hypervisors can support all flag combinations. + * + * See also virDomainGetEmulatorPinInfo for querying this information. + * + * Returns 0 in case of success, -1 in case of failure. + * + */ +int +virDomainPinEmulator(virDomainPtr domain, unsigned char *cpumap, + int maplen, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "cpumap=%p, maplen=%d, flags=%x", + cpumap, maplen, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (domain->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if ((cpumap == NULL) || (maplen < 1)) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + conn = domain->conn; + + if (conn->driver->domainPinEmulator) { + int ret; + ret = conn->driver->domainPinEmulator (domain, cpumap, maplen, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + +/** + * virDomainGetEmulatorPinInfo: + * @domain: pointer to domain object, or NULL for Domain0 + * @cpumap: pointer to a bit map of real CPUs for all emulator threads of + * this domain (in 8-bit bytes) (OUT) + * There is only one cpumap for all emulator threads. + * Must not be NULL. + * @maplen: the number of bytes in one cpumap, from 1 up to size of CPU map. + * Must be positive. + * @flags: bitwise-OR of virDomainModificationImpact + * Must not be VIR_DOMAIN_AFFECT_LIVE and + * VIR_DOMAIN_AFFECT_CONFIG concurrently. + * + * Query the CPU affinity setting of all emulator threads of domain, store + * it in cpumap. + * + * Returns 1 in case of success, + * 0 in case of no emulator threads are pined to pcpus, + * -1 in case of failure. + */ +int +virDomainGetEmulatorPinInfo(virDomainPtr domain, unsigned char *cpumap, + int maplen, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "cpumap=%p, maplen=%d, flags=%x", + cpumap, maplen, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (!cpumap || maplen <= 0) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + if (INT_MULTIPLY_OVERFLOW(1, maplen)) { + virLibDomainError(VIR_ERR_OVERFLOW, _("input too large: 1 * %d"), + maplen); + goto error; + } + + /* At most one of these two flags should be set. */ + if ((flags & VIR_DOMAIN_AFFECT_LIVE) && + (flags & VIR_DOMAIN_AFFECT_CONFIG)) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + conn = domain->conn; + + if (conn->driver->domainGetEmulatorPinInfo) { + int ret; + ret = conn->driver->domainGetEmulatorPinInfo(domain, cpumap, + maplen, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + +/** * virDomainGetVcpus: * @domain: pointer to domain object, or NULL for Domain0 * @info: pointer to an array of virVcpuInfo structures (OUT) diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 43f2cf2..92ae95a 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -550,6 +550,8 @@ LIBVIRT_0.10.0 { virConnectRegisterCloseCallback; virConnectUnregisterCloseCallback; virDomainGetSecurityLabelList; + virDomainPinEmulator; + virDomainGetEmulatorPinInfo; } LIBVIRT_0.9.13;
# .... define new API here using predicted next version number ....
Okay, ACK ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

From: Tang Chen <tangchen@cn.fujitsu.com> Introduce 2 APIs to support emulator threads pin. 1) virDomainEmulatorPinAdd: setup emulator threads pin with a given cpumap string. 2) virDomainEmulatorPinDel: remove all emulator threads pin. Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/conf/domain_conf.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 6 ++++ src/libvirt_private.syms | 2 ++ 3 files changed, 79 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 94ec095..dab9c5d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11076,6 +11076,77 @@ virDomainVcpuPinDel(virDomainDefPtr def, int vcpu) return 0; } +int +virDomainEmulatorPinAdd(virDomainDefPtr def, + unsigned char *cpumap, + int maplen) +{ + virDomainVcpuPinDefPtr emulatorpin = NULL; + char *cpumask = NULL; + int i; + + if (VIR_ALLOC_N(cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) { + virReportOOMError(); + goto cleanup; + } + + /* Convert bitmap (cpumap) to cpumask, which is byte map. */ + for (i = 0; i < maplen; i++) { + int cur; + + for (cur = 0; cur < 8; cur++) { + if (cpumap[i] & (1 << cur)) + cpumask[i * 8 + cur] = 1; + } + } + + if (!def->cputune.emulatorpin) { + /* No emulatorpin exists yet. */ + if (VIR_ALLOC(emulatorpin) < 0) { + virReportOOMError(); + goto cleanup; + } + + emulatorpin->vcpuid = -1; + emulatorpin->cpumask = cpumask; + def->cputune.emulatorpin = emulatorpin; + } else { + /* Since there is only 1 emulatorpin for each vm, + * juest replace the old one. + */ + VIR_FREE(def->cputune.emulatorpin->cpumask); + def->cputune.emulatorpin->cpumask = cpumask; + } + + return 0; + +cleanup: + VIR_FREE(cpumask); + return -1; +} + +int +virDomainEmulatorPinDel(virDomainDefPtr def) +{ + virDomainVcpuPinDefPtr emulatorpin = NULL; + + /* No emulatorpin exists yet */ + if (!def->cputune.emulatorpin) { + return 0; + } + + emulatorpin = def->cputune.emulatorpin; + + VIR_FREE(emulatorpin->cpumask); + VIR_FREE(emulatorpin); + def->cputune.emulatorpin = NULL; + + if (def->cputune.emulatorpin) + return -1; + + return 0; +} + static int virDomainLifecycleDefFormat(virBufferPtr buf, int type, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a7b2ff6..b6bf5a8 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1982,6 +1982,12 @@ int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr *vcpupin_list, int virDomainVcpuPinDel(virDomainDefPtr def, int vcpu); +int virDomainEmulatorPinAdd(virDomainDefPtr def, + unsigned char *cpumap, + int maplen); + +int virDomainEmulatorPinDel(virDomainDefPtr def); + int virDomainDiskIndexByName(virDomainDefPtr def, const char *name, bool allow_ambiguous); const char *virDomainDiskPathByName(virDomainDefPtr, const char *name); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 80ea39a..d51a387 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -509,6 +509,8 @@ virDomainVcpuPinAdd; virDomainVcpuPinDefCopy; virDomainVcpuPinDefFree; virDomainVcpuPinDel; +virDomainEmulatorPinAdd; +virDomainEmulatorPinDel; virDomainVcpuPinFindByVcpu; virDomainVcpuPinIsDuplicate; virDomainVideoDefFree; -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:36PM +0800, Hu Tao wrote:
From: Tang Chen <tangchen@cn.fujitsu.com>
Introduce 2 APIs to support emulator threads pin. 1) virDomainEmulatorPinAdd: setup emulator threads pin with a given cpumap string. 2) virDomainEmulatorPinDel: remove all emulator threads pin.
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/conf/domain_conf.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 6 ++++ src/libvirt_private.syms | 2 ++ 3 files changed, 79 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 94ec095..dab9c5d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11076,6 +11076,77 @@ virDomainVcpuPinDel(virDomainDefPtr def, int vcpu) return 0; }
+int +virDomainEmulatorPinAdd(virDomainDefPtr def, + unsigned char *cpumap, + int maplen) +{ + virDomainVcpuPinDefPtr emulatorpin = NULL; + char *cpumask = NULL; + int i; + + if (VIR_ALLOC_N(cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0) { + virReportOOMError(); + goto cleanup; + } + + /* Convert bitmap (cpumap) to cpumask, which is byte map. */ + for (i = 0; i < maplen; i++) { + int cur; + + for (cur = 0; cur < 8; cur++) { + if (cpumap[i] & (1 << cur)) + cpumask[i * 8 + cur] = 1; + } + } + + if (!def->cputune.emulatorpin) { + /* No emulatorpin exists yet. */ + if (VIR_ALLOC(emulatorpin) < 0) { + virReportOOMError(); + goto cleanup; + } + + emulatorpin->vcpuid = -1; + emulatorpin->cpumask = cpumask; + def->cputune.emulatorpin = emulatorpin; + } else { + /* Since there is only 1 emulatorpin for each vm, + * juest replace the old one. + */ + VIR_FREE(def->cputune.emulatorpin->cpumask); + def->cputune.emulatorpin->cpumask = cpumask; + } + + return 0; + +cleanup: + VIR_FREE(cpumask); + return -1; +} + +int +virDomainEmulatorPinDel(virDomainDefPtr def) +{ + virDomainVcpuPinDefPtr emulatorpin = NULL; + + /* No emulatorpin exists yet */ + if (!def->cputune.emulatorpin) { + return 0; + } + + emulatorpin = def->cputune.emulatorpin; + + VIR_FREE(emulatorpin->cpumask); + VIR_FREE(emulatorpin); + def->cputune.emulatorpin = NULL; + + if (def->cputune.emulatorpin) + return -1; + + return 0; +} + static int virDomainLifecycleDefFormat(virBufferPtr buf, int type, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a7b2ff6..b6bf5a8 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1982,6 +1982,12 @@ int virDomainVcpuPinAdd(virDomainVcpuPinDefPtr *vcpupin_list,
int virDomainVcpuPinDel(virDomainDefPtr def, int vcpu);
+int virDomainEmulatorPinAdd(virDomainDefPtr def, + unsigned char *cpumap, + int maplen); + +int virDomainEmulatorPinDel(virDomainDefPtr def); + int virDomainDiskIndexByName(virDomainDefPtr def, const char *name, bool allow_ambiguous); const char *virDomainDiskPathByName(virDomainDefPtr, const char *name); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 80ea39a..d51a387 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -509,6 +509,8 @@ virDomainVcpuPinAdd; virDomainVcpuPinDefCopy; virDomainVcpuPinDefFree; virDomainVcpuPinDel; +virDomainEmulatorPinAdd; +virDomainEmulatorPinDel; virDomainVcpuPinFindByVcpu; virDomainVcpuPinIsDuplicate; virDomainVideoDefFree;
Okay, very similar to the Vcpu counterparts. However the symbols file should be kept sorted so added that fix Daniel diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6af099b..8962de2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -345,6 +345,8 @@ virDomainDiskSnapshotTypeFromString; virDomainDiskSnapshotTypeToString; virDomainDiskTypeFromString; virDomainDiskTypeToString; +virDomainEmulatorPinAdd; +virDomainEmulatorPinDel; virDomainFSDefFree; virDomainFSIndexByName; virDomainFSTypeFromString; @@ -511,8 +513,6 @@ virDomainVcpuPinAdd; virDomainVcpuPinDefCopy; virDomainVcpuPinDefFree; virDomainVcpuPinDel; -virDomainEmulatorPinAdd; -virDomainEmulatorPinDel; virDomainVcpuPinFindByVcpu; virDomainVcpuPinIsDuplicate; virDomainVideoDefFree; -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

Introduce 2 APIs to support emulator threads pin in qemu driver. 1) qemudDomainPinEmulator: setup emulator threads pin info. 2) qemudDomainGetEmulatorPinInfo: get all emulator threads pin info. They are similar to qemudDomainPinVcpuFlags and qemudDomainGetVcpuPinInfo. And also, remoteDispatchDomainPinEmulatorFlags and remoteDispatchDomainGetEmulatorPinInfo functions are introduced. Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/qemu/qemu_driver.c | 241 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 241 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4552172..3c8bbb7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3984,6 +3984,245 @@ cleanup: } static int +qemudDomainPinEmulator(virDomainPtr dom, + unsigned char *cpumap, + int maplen, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + virCgroupPtr cgroup_dom = NULL; + virCgroupPtr cgroup_emulator = NULL; + pid_t pid; + virDomainDefPtr persistentDef = NULL; + int maxcpu, hostcpus; + virNodeInfo nodeinfo; + int ret = -1; + qemuDomainObjPrivatePtr priv; + bool canResetting = true; + int pcpu; + int newVcpuPinNum = 0; + virDomainVcpuPinDefPtr *newVcpuPin = NULL; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, + &persistentDef) < 0) + goto cleanup; + + priv = vm->privateData; + + if (nodeGetInfo(dom->conn, &nodeinfo) < 0) + goto cleanup; + hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); + maxcpu = maplen * 8; + if (maxcpu > hostcpus) + maxcpu = hostcpus; + /* pinning to all physical cpus means resetting, + * so check if we can reset setting. + */ + for (pcpu = 0; pcpu < hostcpus; pcpu++) { + if ((cpumap[pcpu/8] & (1 << (pcpu % 8))) == 0) { + canResetting = false; + break; + } + } + + pid = vm->pid; + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + + if (priv->vcpupids != NULL) { + if (VIR_ALLOC(newVcpuPin) < 0) { + virReportOOMError(); + goto cleanup; + newVcpuPinNum = 0; + } + + if (virDomainVcpuPinAdd(newVcpuPin, &newVcpuPinNum, cpumap, maplen, -1) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to update vcpupin")); + virDomainVcpuPinDefFree(newVcpuPin, newVcpuPinNum); + goto cleanup; + } + + if (qemuCgroupControllerActive(driver, + VIR_CGROUP_CONTROLLER_CPUSET)) { + /* + * Configure the corresponding cpuset cgroup. + * If no cgroup for domain or hypervisor exists, do nothing. + */ + if (virCgroupForDomain(driver->cgroup, vm->def->name, + &cgroup_dom, 0) == 0) { + if (virCgroupForEmulator(cgroup_dom, &cgroup_emulator, 0) == 0) { + if (qemuSetupCgroupEmulatorPin(cgroup_emulator, newVcpuPin[0]) < 0) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("failed to set cpuset.cpus in cgroup" + " for emulator threads")); + goto cleanup; + } + } + } + } else { + if (virProcessInfoSetAffinity(pid, cpumap, maplen, maxcpu) < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, "%s", + _("failed to set cpu affinity for " + "emulator threads")); + goto cleanup; + } + } + + if (canResetting) { + if (virDomainEmulatorPinDel(vm->def) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to delete emulatorpin xml of " + "a running domain")); + goto cleanup; + } + } else { + if (vm->def->cputune.emulatorpin) { + VIR_FREE(vm->def->cputune.emulatorpin->cpumask); + VIR_FREE(vm->def->cputune.emulatorpin); + } + vm->def->cputune.emulatorpin = newVcpuPin[0]; + VIR_FREE(newVcpuPin); + } + + if (newVcpuPin) + virDomainVcpuPinDefFree(newVcpuPin, newVcpuPinNum); + } else { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cpu affinity is not supported")); + goto cleanup; + } + + if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) + goto cleanup; + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + + if (canResetting) { + if (virDomainEmulatorPinDel(persistentDef) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to delete emulatorpin xml of " + "a persistent domain")); + goto cleanup; + } + } else { + if (virDomainEmulatorPinAdd(persistentDef, cpumap, maplen) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to update or add emulatorpin xml " + "of a persistent domain")); + goto cleanup; + } + } + + ret = virDomainSaveConfig(driver->configDir, persistentDef); + goto cleanup; + } + + ret = 0; + +cleanup: + if (cgroup_emulator) + virCgroupFree(&cgroup_emulator); + if (cgroup_dom) + virCgroupFree(&cgroup_dom); + + if (vm) + virDomainObjUnlock(vm); + return ret; +} + +static int +qemudDomainGetEmulatorPinInfo(virDomainPtr dom, + unsigned char *cpumaps, + int maplen, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + virNodeInfo nodeinfo; + virDomainDefPtr targetDef = NULL; + int ret = -1; + int maxcpu, hostcpus, pcpu; + virDomainVcpuPinDefPtr emulatorpin = NULL; + char *cpumask = NULL; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, + &targetDef) < 0) + goto cleanup; + + if (flags & VIR_DOMAIN_AFFECT_LIVE) + targetDef = vm->def; + + /* Coverity didn't realize that targetDef must be set if we got here. */ + sa_assert(targetDef); + + if (nodeGetInfo(dom->conn, &nodeinfo) < 0) + goto cleanup; + hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); + maxcpu = maplen * 8; + if (maxcpu > hostcpus) + maxcpu = hostcpus; + + /* initialize cpumaps */ + memset(cpumaps, 0xff, maplen); + if (maxcpu % 8) { + cpumaps[maplen - 1] &= (1 << maxcpu % 8) - 1; + } + + /* If no emulatorpin, all cpus should be used */ + emulatorpin = targetDef->cputune.emulatorpin; + if (!emulatorpin) { + ret = 0; + goto cleanup; + } + + cpumask = emulatorpin->cpumask; + for (pcpu = 0; pcpu < maxcpu; pcpu++) { + if (cpumask[pcpu] == 0) + VIR_UNUSE_CPU(cpumaps, pcpu); + } + + ret = 1; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + +static int qemudDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int maxinfo, @@ -13551,6 +13790,8 @@ static virDriver qemuDriver = { .domainPinVcpu = qemudDomainPinVcpu, /* 0.4.4 */ .domainPinVcpuFlags = qemudDomainPinVcpuFlags, /* 0.9.3 */ .domainGetVcpuPinInfo = qemudDomainGetVcpuPinInfo, /* 0.9.3 */ + .domainPinEmulator = qemudDomainPinEmulator, /* 0.10.0 */ + .domainGetEmulatorPinInfo = qemudDomainGetEmulatorPinInfo, /* 0.10.0 */ .domainGetVcpus = qemudDomainGetVcpus, /* 0.4.4 */ .domainGetMaxVcpus = qemudDomainGetMaxVcpus, /* 0.4.4 */ .domainGetSecurityLabel = qemudDomainGetSecurityLabel, /* 0.6.1 */ -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:37PM +0800, Hu Tao wrote:
Introduce 2 APIs to support emulator threads pin in qemu driver.
1) qemudDomainPinEmulator: setup emulator threads pin info. 2) qemudDomainGetEmulatorPinInfo: get all emulator threads pin info.
They are similar to qemudDomainPinVcpuFlags and qemudDomainGetVcpuPinInfo. And also, remoteDispatchDomainPinEmulatorFlags and remoteDispatchDomainGetEmulatorPinInfo functions are introduced.
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- src/qemu/qemu_driver.c | 241 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 241 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4552172..3c8bbb7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3984,6 +3984,245 @@ cleanup: }
static int +qemudDomainPinEmulator(virDomainPtr dom, + unsigned char *cpumap, + int maplen, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + virCgroupPtr cgroup_dom = NULL; + virCgroupPtr cgroup_emulator = NULL; + pid_t pid; + virDomainDefPtr persistentDef = NULL; + int maxcpu, hostcpus; + virNodeInfo nodeinfo; + int ret = -1; + qemuDomainObjPrivatePtr priv; + bool canResetting = true; + int pcpu; + int newVcpuPinNum = 0; + virDomainVcpuPinDefPtr *newVcpuPin = NULL; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, + &persistentDef) < 0) + goto cleanup; + + priv = vm->privateData; + + if (nodeGetInfo(dom->conn, &nodeinfo) < 0) + goto cleanup; + hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); + maxcpu = maplen * 8; + if (maxcpu > hostcpus) + maxcpu = hostcpus; + /* pinning to all physical cpus means resetting, + * so check if we can reset setting. + */ + for (pcpu = 0; pcpu < hostcpus; pcpu++) { + if ((cpumap[pcpu/8] & (1 << (pcpu % 8))) == 0) { + canResetting = false; + break; + } + } + + pid = vm->pid; + + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + + if (priv->vcpupids != NULL) { + if (VIR_ALLOC(newVcpuPin) < 0) { + virReportOOMError(); + goto cleanup; + newVcpuPinNum = 0; + } + + if (virDomainVcpuPinAdd(newVcpuPin, &newVcpuPinNum, cpumap, maplen, -1) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to update vcpupin")); + virDomainVcpuPinDefFree(newVcpuPin, newVcpuPinNum); + goto cleanup; + } + + if (qemuCgroupControllerActive(driver, + VIR_CGROUP_CONTROLLER_CPUSET)) { + /* + * Configure the corresponding cpuset cgroup. + * If no cgroup for domain or hypervisor exists, do nothing. + */ + if (virCgroupForDomain(driver->cgroup, vm->def->name, + &cgroup_dom, 0) == 0) { + if (virCgroupForEmulator(cgroup_dom, &cgroup_emulator, 0) == 0) { + if (qemuSetupCgroupEmulatorPin(cgroup_emulator, newVcpuPin[0]) < 0) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("failed to set cpuset.cpus in cgroup" + " for emulator threads")); + goto cleanup; + } + } + } + } else { + if (virProcessInfoSetAffinity(pid, cpumap, maplen, maxcpu) < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, "%s", + _("failed to set cpu affinity for " + "emulator threads")); + goto cleanup; + } + } + + if (canResetting) { + if (virDomainEmulatorPinDel(vm->def) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to delete emulatorpin xml of " + "a running domain")); + goto cleanup; + } + } else { + if (vm->def->cputune.emulatorpin) { + VIR_FREE(vm->def->cputune.emulatorpin->cpumask); + VIR_FREE(vm->def->cputune.emulatorpin); + } + vm->def->cputune.emulatorpin = newVcpuPin[0]; + VIR_FREE(newVcpuPin); + } + + if (newVcpuPin) + virDomainVcpuPinDefFree(newVcpuPin, newVcpuPinNum); + } else { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cpu affinity is not supported")); + goto cleanup; + } + + if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) + goto cleanup; + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + + if (canResetting) { + if (virDomainEmulatorPinDel(persistentDef) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to delete emulatorpin xml of " + "a persistent domain")); + goto cleanup; + } + } else { + if (virDomainEmulatorPinAdd(persistentDef, cpumap, maplen) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to update or add emulatorpin xml " + "of a persistent domain")); + goto cleanup; + } + } + + ret = virDomainSaveConfig(driver->configDir, persistentDef); + goto cleanup; + } + + ret = 0; + +cleanup: + if (cgroup_emulator) + virCgroupFree(&cgroup_emulator); + if (cgroup_dom) + virCgroupFree(&cgroup_dom); + + if (vm) + virDomainObjUnlock(vm); + return ret; +} + +static int +qemudDomainGetEmulatorPinInfo(virDomainPtr dom, + unsigned char *cpumaps, + int maplen, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + virNodeInfo nodeinfo; + virDomainDefPtr targetDef = NULL; + int ret = -1; + int maxcpu, hostcpus, pcpu; + virDomainVcpuPinDefPtr emulatorpin = NULL; + char *cpumask = NULL; + + virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | + VIR_DOMAIN_AFFECT_CONFIG, -1); + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (virDomainLiveConfigHelperMethod(driver->caps, vm, &flags, + &targetDef) < 0) + goto cleanup; + + if (flags & VIR_DOMAIN_AFFECT_LIVE) + targetDef = vm->def; + + /* Coverity didn't realize that targetDef must be set if we got here. */ + sa_assert(targetDef); + + if (nodeGetInfo(dom->conn, &nodeinfo) < 0) + goto cleanup; + hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo); + maxcpu = maplen * 8; + if (maxcpu > hostcpus) + maxcpu = hostcpus; + + /* initialize cpumaps */ + memset(cpumaps, 0xff, maplen); + if (maxcpu % 8) { + cpumaps[maplen - 1] &= (1 << maxcpu % 8) - 1; + } + + /* If no emulatorpin, all cpus should be used */ + emulatorpin = targetDef->cputune.emulatorpin; + if (!emulatorpin) { + ret = 0; + goto cleanup; + } + + cpumask = emulatorpin->cpumask; + for (pcpu = 0; pcpu < maxcpu; pcpu++) { + if (cpumask[pcpu] == 0) + VIR_UNUSE_CPU(cpumaps, pcpu); + } + + ret = 1; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} + +static int qemudDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int maxinfo, @@ -13551,6 +13790,8 @@ static virDriver qemuDriver = { .domainPinVcpu = qemudDomainPinVcpu, /* 0.4.4 */ .domainPinVcpuFlags = qemudDomainPinVcpuFlags, /* 0.9.3 */ .domainGetVcpuPinInfo = qemudDomainGetVcpuPinInfo, /* 0.9.3 */ + .domainPinEmulator = qemudDomainPinEmulator, /* 0.10.0 */ + .domainGetEmulatorPinInfo = qemudDomainGetEmulatorPinInfo, /* 0.10.0 */ .domainGetVcpus = qemudDomainGetVcpus, /* 0.4.4 */ .domainGetMaxVcpus = qemudDomainGetMaxVcpus, /* 0.4.4 */ .domainGetSecurityLabel = qemudDomainGetSecurityLabel, /* 0.6.1 */
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

From: Tang Chen <tangchen@cn.fujitsu.com> Introduce 2 APIs to support emulator threads in remote driver. 1) remoteDomainPinEmulator: call driver api, such as qemudDomainPinEmulator. 2) remoteDomainGetEmulatorPinInfo: call driver api, such as qemudDomainGetEmulatorPinInfo. They are similar to remoteDomainPinVcpuFlags and remoteDomainGetVcpuPinInfo. Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- daemon/remote.c | 91 ++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 99 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 21 ++++++++- src/remote_protocol-structs | 22 ++++++++++ 4 files changed, 232 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index f82af86..24928f4 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1593,6 +1593,97 @@ no_memory: } static int +remoteDispatchDomainPinEmulator(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_pin_emulator_args *args) +{ + int rv = -1; + virDomainPtr dom = NULL; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + if (virDomainPinEmulator(dom, + (unsigned char *) args->cpumap.cpumap_val, + args->cpumap.cpumap_len, + args->flags) < 0) + goto cleanup; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (dom) + virDomainFree(dom); + return rv; +} + + +static int +remoteDispatchDomainGetEmulatorPinInfo(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_emulator_pin_info_args *args, + remote_domain_get_emulator_pin_info_ret *ret) +{ + virDomainPtr dom = NULL; + unsigned char *cpumaps = NULL; + int r; + int rv = -1; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + /* Allocate buffers to take the results */ + if (args->maplen > 0 && + VIR_ALLOC_N(cpumaps, args->maplen) < 0) + goto no_memory; + + if ((r = virDomainGetEmulatorPinInfo(dom, + cpumaps, + args->maplen, + args->flags)) < 0) + goto cleanup; + + ret->ret = r; + ret->cpumaps.cpumaps_len = args->maplen; + ret->cpumaps.cpumaps_val = (char *) cpumaps; + cpumaps = NULL; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + VIR_FREE(cpumaps); + if (dom) + virDomainFree(dom); + return rv; + +no_memory: + virReportOOMError(); + goto cleanup; +} + +static int remoteDispatchDomainGetVcpus(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, virNetMessagePtr msg ATTRIBUTE_UNUSED, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 977d139..841f9a3 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1843,6 +1843,103 @@ done: } static int +remoteDomainPinEmulator (virDomainPtr dom, + unsigned char *cpumap, + int cpumaplen, + unsigned int flags) +{ + int rv = -1; + struct private_data *priv = dom->conn->privateData; + remote_domain_pin_emulator_args args; + + remoteDriverLock(priv); + + if (cpumaplen > REMOTE_CPUMAP_MAX) { + virReportError(VIR_ERR_RPC, + _("%s length greater than maximum: %d > %d"), + "cpumap", cpumaplen, REMOTE_CPUMAP_MAX); + goto done; + } + + make_nonnull_domain(&args.dom, dom); + args.cpumap.cpumap_val = (char *)cpumap; + args.cpumap.cpumap_len = cpumaplen; + args.flags = flags; + + if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_PIN_EMULATOR, + (xdrproc_t) xdr_remote_domain_pin_emulator_args, + (char *) &args, + (xdrproc_t) xdr_void, (char *) NULL) == -1) { + goto done; + } + + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +} + + +static int +remoteDomainGetEmulatorPinInfo (virDomainPtr domain, + unsigned char *cpumaps, + int maplen, + unsigned int flags) +{ + int rv = -1; + int i; + remote_domain_get_emulator_pin_info_args args; + remote_domain_get_emulator_pin_info_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + /* There is only one cpumap for all emulator threads */ + if (maplen > REMOTE_CPUMAPS_MAX) { + virReportError(VIR_ERR_RPC, + _("vCPU map buffer length exceeds maximum: %d > %d"), + maplen, REMOTE_CPUMAPS_MAX); + goto done; + } + + make_nonnull_domain(&args.dom, domain); + args.maplen = maplen; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_EMULATOR_PIN_INFO, + (xdrproc_t) xdr_remote_domain_get_emulator_pin_info_args, + (char *) &args, + (xdrproc_t) xdr_remote_domain_get_emulator_pin_info_ret, + (char *) &ret) == -1) + goto done; + + if (ret.cpumaps.cpumaps_len > maplen) { + virReportError(VIR_ERR_RPC, + _("host reports map buffer length exceeds maximum: %d > %d"), + ret.cpumaps.cpumaps_len, maplen); + goto cleanup; + } + + memset(cpumaps, 0, maplen); + + for (i = 0; i < ret.cpumaps.cpumaps_len; ++i) + cpumaps[i] = ret.cpumaps.cpumaps_val[i]; + + rv = ret.ret; + +cleanup: + xdr_free ((xdrproc_t) xdr_remote_domain_get_emulator_pin_info_ret, + (char *) &ret); + +done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteDomainGetVcpus (virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, @@ -5302,6 +5399,8 @@ static virDriver remote_driver = { .domainPinVcpu = remoteDomainPinVcpu, /* 0.3.0 */ .domainPinVcpuFlags = remoteDomainPinVcpuFlags, /* 0.9.3 */ .domainGetVcpuPinInfo = remoteDomainGetVcpuPinInfo, /* 0.9.3 */ + .domainPinEmulator = remoteDomainPinEmulator, /* 0.10.0 */ + .domainGetEmulatorPinInfo = remoteDomainGetEmulatorPinInfo, /* 0.10.0 */ .domainGetVcpus = remoteDomainGetVcpus, /* 0.3.0 */ .domainGetMaxVcpus = remoteDomainGetMaxVcpus, /* 0.3.0 */ .domainGetSecurityLabel = remoteDomainGetSecurityLabel, /* 0.6.1 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 3be15f5..085d5d9 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1059,6 +1059,23 @@ struct remote_domain_get_vcpu_pin_info_ret { int num; }; +struct remote_domain_pin_emulator_args { + remote_nonnull_domain dom; + opaque cpumap<REMOTE_CPUMAP_MAX>; /* (unsigned char *) */ + unsigned int flags; +}; + +struct remote_domain_get_emulator_pin_info_args { + remote_nonnull_domain dom; + int maplen; + unsigned int flags; +}; + +struct remote_domain_get_emulator_pin_info_ret { + opaque cpumaps<REMOTE_CPUMAPS_MAX>; + int ret; +}; + struct remote_domain_get_vcpus_args { remote_nonnull_domain dom; int maxinfo; @@ -2869,7 +2886,9 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_ALL_CHILDREN = 275, /* skipgen skipgen priority:high */ REMOTE_PROC_DOMAIN_EVENT_BALLOON_CHANGE = 276, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_HOSTNAME = 277, /* autogen autogen */ - REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL_LIST = 278 /* skipgen skipgen priority:high */ + REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL_LIST = 278, /* skipgen skipgen priority:high */ + REMOTE_PROC_DOMAIN_PIN_EMULATOR = 279, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_EMULATOR_PIN_INFO = 280 /* skipgen skipgen */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index c180e6e..ed40ccb 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -718,6 +718,26 @@ struct remote_domain_get_vcpu_pin_info_ret { } cpumaps; int num; }; +struct remote_domain_pin_emulator_args { + remote_nonnull_domain dom; + struct { + u_int cpumap_len; + char * cpumap_val; + } cpumap; + u_int flags; +}; +struct remote_domain_get_emulator_pin_info_args { + remote_nonnull_domain dom; + int maplen; + u_int flags; +}; +struct remote_domain_get_emulator_pin_info_ret { + struct { + u_int cpumaps_len; + char * cpumaps_val; + } cpumaps; + int ret; +}; struct remote_domain_get_vcpus_args { remote_nonnull_domain dom; int maxinfo; @@ -2270,4 +2290,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_EVENT_BALLOON_CHANGE = 276, REMOTE_PROC_DOMAIN_GET_HOSTNAME = 277, REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL_LIST = 278, + REMOTE_PROC_DOMAIN_PIN_EMULATOR = 279, + REMOTE_PROC_DOMAIN_GET_EMULATOR_PIN_INFO = 280, }; -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:38PM +0800, Hu Tao wrote:
From: Tang Chen <tangchen@cn.fujitsu.com>
Introduce 2 APIs to support emulator threads in remote driver. 1) remoteDomainPinEmulator: call driver api, such as qemudDomainPinEmulator. 2) remoteDomainGetEmulatorPinInfo: call driver api, such as qemudDomainGetEmulatorPinInfo. They are similar to remoteDomainPinVcpuFlags and remoteDomainGetVcpuPinInfo.
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com> --- daemon/remote.c | 91 ++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 99 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 21 ++++++++- src/remote_protocol-structs | 22 ++++++++++ 4 files changed, 232 insertions(+), 1 deletion(-)
diff --git a/daemon/remote.c b/daemon/remote.c index f82af86..24928f4 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1593,6 +1593,97 @@ no_memory: }
static int +remoteDispatchDomainPinEmulator(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_pin_emulator_args *args) +{ + int rv = -1; + virDomainPtr dom = NULL; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + if (virDomainPinEmulator(dom, + (unsigned char *) args->cpumap.cpumap_val, + args->cpumap.cpumap_len, + args->flags) < 0) + goto cleanup; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (dom) + virDomainFree(dom); + return rv; +} + + +static int +remoteDispatchDomainGetEmulatorPinInfo(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client ATTRIBUTE_UNUSED, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + remote_domain_get_emulator_pin_info_args *args, + remote_domain_get_emulator_pin_info_ret *ret) +{ + virDomainPtr dom = NULL; + unsigned char *cpumaps = NULL; + int r; + int rv = -1; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(priv->conn, args->dom))) + goto cleanup; + + /* Allocate buffers to take the results */ + if (args->maplen > 0 && + VIR_ALLOC_N(cpumaps, args->maplen) < 0) + goto no_memory; + + if ((r = virDomainGetEmulatorPinInfo(dom, + cpumaps, + args->maplen, + args->flags)) < 0) + goto cleanup; + + ret->ret = r; + ret->cpumaps.cpumaps_len = args->maplen; + ret->cpumaps.cpumaps_val = (char *) cpumaps; + cpumaps = NULL; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + VIR_FREE(cpumaps); + if (dom) + virDomainFree(dom); + return rv; + +no_memory: + virReportOOMError(); + goto cleanup; +} + +static int remoteDispatchDomainGetVcpus(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, virNetMessagePtr msg ATTRIBUTE_UNUSED, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 977d139..841f9a3 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1843,6 +1843,103 @@ done: }
static int +remoteDomainPinEmulator (virDomainPtr dom, + unsigned char *cpumap, + int cpumaplen, + unsigned int flags) +{ + int rv = -1; + struct private_data *priv = dom->conn->privateData; + remote_domain_pin_emulator_args args; + + remoteDriverLock(priv); + + if (cpumaplen > REMOTE_CPUMAP_MAX) { + virReportError(VIR_ERR_RPC, + _("%s length greater than maximum: %d > %d"), + "cpumap", cpumaplen, REMOTE_CPUMAP_MAX); + goto done; + } + + make_nonnull_domain(&args.dom, dom); + args.cpumap.cpumap_val = (char *)cpumap; + args.cpumap.cpumap_len = cpumaplen; + args.flags = flags; + + if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_PIN_EMULATOR, + (xdrproc_t) xdr_remote_domain_pin_emulator_args, + (char *) &args, + (xdrproc_t) xdr_void, (char *) NULL) == -1) { + goto done; + } + + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +} + + +static int +remoteDomainGetEmulatorPinInfo (virDomainPtr domain, + unsigned char *cpumaps, + int maplen, + unsigned int flags) +{ + int rv = -1; + int i; + remote_domain_get_emulator_pin_info_args args; + remote_domain_get_emulator_pin_info_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + /* There is only one cpumap for all emulator threads */ + if (maplen > REMOTE_CPUMAPS_MAX) { + virReportError(VIR_ERR_RPC, + _("vCPU map buffer length exceeds maximum: %d > %d"), + maplen, REMOTE_CPUMAPS_MAX); + goto done; + } + + make_nonnull_domain(&args.dom, domain); + args.maplen = maplen; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_EMULATOR_PIN_INFO, + (xdrproc_t) xdr_remote_domain_get_emulator_pin_info_args, + (char *) &args, + (xdrproc_t) xdr_remote_domain_get_emulator_pin_info_ret, + (char *) &ret) == -1) + goto done; + + if (ret.cpumaps.cpumaps_len > maplen) { + virReportError(VIR_ERR_RPC, + _("host reports map buffer length exceeds maximum: %d > %d"), + ret.cpumaps.cpumaps_len, maplen); + goto cleanup; + } + + memset(cpumaps, 0, maplen); + + for (i = 0; i < ret.cpumaps.cpumaps_len; ++i) + cpumaps[i] = ret.cpumaps.cpumaps_val[i]; + + rv = ret.ret; + +cleanup: + xdr_free ((xdrproc_t) xdr_remote_domain_get_emulator_pin_info_ret, + (char *) &ret); + +done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteDomainGetVcpus (virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, @@ -5302,6 +5399,8 @@ static virDriver remote_driver = { .domainPinVcpu = remoteDomainPinVcpu, /* 0.3.0 */ .domainPinVcpuFlags = remoteDomainPinVcpuFlags, /* 0.9.3 */ .domainGetVcpuPinInfo = remoteDomainGetVcpuPinInfo, /* 0.9.3 */ + .domainPinEmulator = remoteDomainPinEmulator, /* 0.10.0 */ + .domainGetEmulatorPinInfo = remoteDomainGetEmulatorPinInfo, /* 0.10.0 */ .domainGetVcpus = remoteDomainGetVcpus, /* 0.3.0 */ .domainGetMaxVcpus = remoteDomainGetMaxVcpus, /* 0.3.0 */ .domainGetSecurityLabel = remoteDomainGetSecurityLabel, /* 0.6.1 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 3be15f5..085d5d9 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1059,6 +1059,23 @@ struct remote_domain_get_vcpu_pin_info_ret { int num; };
+struct remote_domain_pin_emulator_args { + remote_nonnull_domain dom; + opaque cpumap<REMOTE_CPUMAP_MAX>; /* (unsigned char *) */ + unsigned int flags; +}; + +struct remote_domain_get_emulator_pin_info_args { + remote_nonnull_domain dom; + int maplen; + unsigned int flags; +}; + +struct remote_domain_get_emulator_pin_info_ret { + opaque cpumaps<REMOTE_CPUMAPS_MAX>; + int ret; +}; + struct remote_domain_get_vcpus_args { remote_nonnull_domain dom; int maxinfo; @@ -2869,7 +2886,9 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_ALL_CHILDREN = 275, /* skipgen skipgen priority:high */ REMOTE_PROC_DOMAIN_EVENT_BALLOON_CHANGE = 276, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_HOSTNAME = 277, /* autogen autogen */ - REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL_LIST = 278 /* skipgen skipgen priority:high */ + REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL_LIST = 278, /* skipgen skipgen priority:high */ + REMOTE_PROC_DOMAIN_PIN_EMULATOR = 279, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_EMULATOR_PIN_INFO = 280 /* skipgen skipgen */
/* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index c180e6e..ed40ccb 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -718,6 +718,26 @@ struct remote_domain_get_vcpu_pin_info_ret { } cpumaps; int num; }; +struct remote_domain_pin_emulator_args { + remote_nonnull_domain dom; + struct { + u_int cpumap_len; + char * cpumap_val; + } cpumap; + u_int flags; +}; +struct remote_domain_get_emulator_pin_info_args { + remote_nonnull_domain dom; + int maplen; + u_int flags; +}; +struct remote_domain_get_emulator_pin_info_ret { + struct { + u_int cpumaps_len; + char * cpumaps_val; + } cpumaps; + int ret; +}; struct remote_domain_get_vcpus_args { remote_nonnull_domain dom; int maxinfo; @@ -2270,4 +2290,6 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_EVENT_BALLOON_CHANGE = 276, REMOTE_PROC_DOMAIN_GET_HOSTNAME = 277, REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL_LIST = 278, + REMOTE_PROC_DOMAIN_PIN_EMULATOR = 279, + REMOTE_PROC_DOMAIN_GET_EMULATOR_PIN_INFO = 280, };
Okay, looks fine, ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

This is a helper function to print vcpu pin info. --- tools/virsh-domain.c | 65 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 94ac1aa..047d374 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4525,6 +4525,45 @@ static const vshCmdOptDef opts_vcpupin[] = { {NULL, 0, 0, NULL} }; +/* + * Helper function to print vcpupin info. + */ +static bool +vshPrintPinInfo(unsigned char *cpumaps, size_t cpumaplen, + int maxcpu, int vcpuindex) +{ + int cpu, lastcpu; + bool bit, lastbit, isInvert; + + if (!cpumaps || cpumaplen <= 0 || maxcpu <= 0 || vcpuindex < 0) { + return false; + } + + bit = lastbit = isInvert = false; + lastcpu = -1; + + for (cpu = 0; cpu < maxcpu; cpu++) { + bit = VIR_CPU_USABLE(cpumaps, cpumaplen, vcpuindex, cpu); + + isInvert = (bit ^ lastbit); + if (bit && isInvert) { + if (lastcpu == -1) + vshPrint(ctl, "%d", cpu); + else + vshPrint(ctl, ",%d", cpu); + lastcpu = cpu; + } + if (!bit && isInvert && lastcpu != cpu - 1) + vshPrint(ctl, "-%d", cpu - 1); + lastbit = bit; + } + if (bit && !isInvert) { + vshPrint(ctl, "-%d", maxcpu - 1); + } + + return true; +} + static bool cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) { @@ -4537,7 +4576,6 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) unsigned char *cpumap = NULL; unsigned char *cpumaps = NULL; size_t cpumaplen; - bool bit, lastbit, isInvert; int i, cpu, lastcpu, maxcpu, ncpus; bool unuse = false; const char *cur; @@ -4622,30 +4660,11 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) if (vcpu != -1 && i != vcpu) continue; - bit = lastbit = isInvert = false; - lastcpu = -1; - vshPrint(ctl, "%4d: ", i); - for (cpu = 0; cpu < maxcpu; cpu++) { - - bit = VIR_CPU_USABLE(cpumaps, cpumaplen, i, cpu); - - isInvert = (bit ^ lastbit); - if (bit && isInvert) { - if (lastcpu == -1) - vshPrint(ctl, "%d", cpu); - else - vshPrint(ctl, ",%d", cpu); - lastcpu = cpu; - } - if (!bit && isInvert && lastcpu != cpu - 1) - vshPrint(ctl, "-%d", cpu - 1); - lastbit = bit; - } - if (bit && !isInvert) { - vshPrint(ctl, "-%d", maxcpu - 1); - } + ret = vshPrintPinInfo(cpumaps, cpumaplen, maxcpu, i); vshPrint(ctl, "\n"); + if (!ret) + break; } } else { -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:39PM +0800, Hu Tao wrote:
This is a helper function to print vcpu pin info. --- tools/virsh-domain.c | 65 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 23 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 94ac1aa..047d374 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4525,6 +4525,45 @@ static const vshCmdOptDef opts_vcpupin[] = { {NULL, 0, 0, NULL} };
+/* + * Helper function to print vcpupin info. + */ +static bool +vshPrintPinInfo(unsigned char *cpumaps, size_t cpumaplen, + int maxcpu, int vcpuindex) +{ + int cpu, lastcpu; + bool bit, lastbit, isInvert; + + if (!cpumaps || cpumaplen <= 0 || maxcpu <= 0 || vcpuindex < 0) { + return false; + } + + bit = lastbit = isInvert = false; + lastcpu = -1; + + for (cpu = 0; cpu < maxcpu; cpu++) { + bit = VIR_CPU_USABLE(cpumaps, cpumaplen, vcpuindex, cpu); + + isInvert = (bit ^ lastbit); + if (bit && isInvert) { + if (lastcpu == -1) + vshPrint(ctl, "%d", cpu); + else + vshPrint(ctl, ",%d", cpu); + lastcpu = cpu; + } + if (!bit && isInvert && lastcpu != cpu - 1) + vshPrint(ctl, "-%d", cpu - 1); + lastbit = bit; + } + if (bit && !isInvert) { + vshPrint(ctl, "-%d", maxcpu - 1); + } + + return true; +} + static bool cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) { @@ -4537,7 +4576,6 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) unsigned char *cpumap = NULL; unsigned char *cpumaps = NULL; size_t cpumaplen; - bool bit, lastbit, isInvert; int i, cpu, lastcpu, maxcpu, ncpus; bool unuse = false; const char *cur; @@ -4622,30 +4660,11 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) if (vcpu != -1 && i != vcpu) continue;
- bit = lastbit = isInvert = false; - lastcpu = -1; - vshPrint(ctl, "%4d: ", i); - for (cpu = 0; cpu < maxcpu; cpu++) { - - bit = VIR_CPU_USABLE(cpumaps, cpumaplen, i, cpu); - - isInvert = (bit ^ lastbit); - if (bit && isInvert) { - if (lastcpu == -1) - vshPrint(ctl, "%d", cpu); - else - vshPrint(ctl, ",%d", cpu); - lastcpu = cpu; - } - if (!bit && isInvert && lastcpu != cpu - 1) - vshPrint(ctl, "-%d", cpu - 1); - lastbit = bit; - } - if (bit && !isInvert) { - vshPrint(ctl, "-%d", maxcpu - 1); - } + ret = vshPrintPinInfo(cpumaps, cpumaplen, maxcpu, i); vshPrint(ctl, "\n"); + if (!ret) + break; }
} else {
Okay, just a bit of refactoring, ACK Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

--- tools/virsh-domain.c | 188 ++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 16 +++++ 2 files changed, 204 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 047d374..95015ad 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4769,6 +4769,193 @@ parse_error: } /* + * "emulatorpin" command + */ +static const vshCmdInfo info_emulatorpin[] = { + {"help", N_("control or query domain emulator affinity")}, + {"desc", N_("Pin domain emulator threads to host physical CPUs.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_emulatorpin[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"cpulist", VSH_OT_DATA, VSH_OFLAG_EMPTY_OK, + N_("host cpu number(s) to set, or omit option to query")}, + {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, + {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, + {"current", VSH_OT_BOOL, 0, N_("affect current domain")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdEmulatorPin(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + virNodeInfo nodeinfo; + const char *cpulist = NULL; + bool ret = true; + unsigned char *cpumap = NULL; + unsigned char *cpumaps = NULL; + size_t cpumaplen; + int i, cpu, lastcpu, maxcpu; + bool unuse = false; + const char *cur; + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); + bool query = false; /* Query mode if no cpulist */ + unsigned int flags = 0; + + if (current) { + if (live || config) { + vshError(ctl, "%s", _("--current must be specified exclusively")); + return false; + } + flags = VIR_DOMAIN_AFFECT_CURRENT; + } else { + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + /* neither option is specified */ + if (!live && !config) + flags = -1; + } + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptString(cmd, "cpulist", &cpulist) < 0) { + vshError(ctl, "%s", _("emulatorpin: Missing cpulist.")); + virDomainFree(dom); + return false; + } + query = !cpulist; + + if (virNodeGetInfo(ctl->conn, &nodeinfo) != 0) { + virDomainFree(dom); + return false; + } + + maxcpu = VIR_NODEINFO_MAXCPUS(nodeinfo); + cpumaplen = VIR_CPU_MAPLEN(maxcpu); + + /* Query mode: show CPU affinity information then exit.*/ + if (query) { + /* When query mode and neither "live", "config" nor "current" + * is specified, set VIR_DOMAIN_AFFECT_CURRENT as flags */ + if (flags == -1) + flags = VIR_DOMAIN_AFFECT_CURRENT; + + cpumaps = vshMalloc(ctl, cpumaplen); + if (virDomainGetEmulatorPinInfo(dom, cpumaps, + cpumaplen, flags) >= 0) { + vshPrint(ctl, "%s %s\n", _("emulator:"), _("CPU Affinity")); + vshPrint(ctl, "----------------------------------\n"); + vshPrint(ctl, " *: "); + ret = vshPrintPinInfo(cpumaps, cpumaplen, maxcpu, 0); + vshPrint(ctl, "\n"); + } else { + ret = false; + } + VIR_FREE(cpumaps); + goto cleanup; + } + + /* Pin mode: pinning emulator threads to specified physical cpus*/ + + cpumap = vshCalloc(ctl, cpumaplen, sizeof(cpumap)); + /* Parse cpulist */ + cur = cpulist; + if (*cur == 0) { + goto parse_error; + } else if (*cur == 'r') { + for (cpu = 0; cpu < maxcpu; cpu++) + VIR_USE_CPU(cpumap, cpu); + cur = ""; + } + + while (*cur != 0) { + + /* the char '^' denotes exclusive */ + if (*cur == '^') { + cur++; + unuse = true; + } + + /* parse physical CPU number */ + if (!c_isdigit(*cur)) + goto parse_error; + cpu = virParseNumber(&cur); + if (cpu < 0) { + goto parse_error; + } + if (cpu >= maxcpu) { + vshError(ctl, _("Physical CPU %d doesn't exist."), cpu); + goto parse_error; + } + virSkipSpaces(&cur); + + if (*cur == ',' || *cur == 0) { + if (unuse) { + VIR_UNUSE_CPU(cpumap, cpu); + } else { + VIR_USE_CPU(cpumap, cpu); + } + } else if (*cur == '-') { + /* the char '-' denotes range */ + if (unuse) { + goto parse_error; + } + cur++; + virSkipSpaces(&cur); + /* parse the end of range */ + lastcpu = virParseNumber(&cur); + if (lastcpu < cpu) { + goto parse_error; + } + if (lastcpu >= maxcpu) { + vshError(ctl, _("Physical CPU %d doesn't exist."), maxcpu); + goto parse_error; + } + for (i = cpu; i <= lastcpu; i++) { + VIR_USE_CPU(cpumap, i); + } + virSkipSpaces(&cur); + } + + if (*cur == ',') { + cur++; + virSkipSpaces(&cur); + unuse = false; + } else if (*cur == 0) { + break; + } else { + goto parse_error; + } + } + + if (flags == -1) + flags = VIR_DOMAIN_AFFECT_LIVE; + + if (virDomainPinEmulator(dom, cpumap, cpumaplen, flags) != 0) + ret = false; + +cleanup: + VIR_FREE(cpumap); + virDomainFree(dom); + return ret; + +parse_error: + vshError(ctl, "%s", _("cpulist: Invalid format.")); + ret = false; + goto cleanup; +} + +/* * "setvcpus" command */ static const vshCmdInfo info_setvcpus[] = { @@ -8200,6 +8387,7 @@ const vshCmdDef domManagementCmds[] = { {"vcpucount", cmdVcpucount, opts_vcpucount, info_vcpucount, 0}, {"vcpuinfo", cmdVcpuinfo, opts_vcpuinfo, info_vcpuinfo, 0}, {"vcpupin", cmdVcpuPin, opts_vcpupin, info_vcpupin, 0}, + {"emulatorpin", cmdEmulatorPin, opts_emulatorpin, info_emulatorpin, 0}, {"vncdisplay", cmdVNCDisplay, opts_vncdisplay, info_vncdisplay, 0}, {NULL, NULL, NULL, NULL, 0} }; diff --git a/tools/virsh.pod b/tools/virsh.pod index 35613c4..ec3c331 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1614,6 +1614,22 @@ If no flag is specified, behavior is different depending on hypervisor. B<Note>: The expression is sequentially evaluated, so "0-15,^8" is identical to "9-14,0-7,15" but not identical to "^8,0-15". +=item B<emulatorpin> I<domain> [I<cpulist>] [[I<--live>] [I<--config>] + | [I<--current>]] + +Query or change the pinning of domain's emulator threads to host physical +CPUs. + +See B<vcpupin> for I<cpulist>. + +If I<--live> is specified, affect a running guest. +If I<--config> is specified, affect the next boot of a persistent guest. +If I<--current> is specified, affect the current guest state. +Both I<--live> and I<--config> flags may be given if I<cpulist> is present, +but I<--current> is exclusive. +If no flag is specified, behavior is different depending on hypervisor. + + =item B<vncdisplay> I<domain> Output the IP address and port number for the VNC display. If the information -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:40PM +0800, Hu Tao wrote:
--- tools/virsh-domain.c | 188 ++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 16 +++++ 2 files changed, 204 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 047d374..95015ad 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4769,6 +4769,193 @@ parse_error: }
/* + * "emulatorpin" command + */ +static const vshCmdInfo info_emulatorpin[] = { + {"help", N_("control or query domain emulator affinity")}, + {"desc", N_("Pin domain emulator threads to host physical CPUs.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_emulatorpin[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"cpulist", VSH_OT_DATA, VSH_OFLAG_EMPTY_OK, + N_("host cpu number(s) to set, or omit option to query")}, + {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, + {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, + {"current", VSH_OT_BOOL, 0, N_("affect current domain")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdEmulatorPin(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + virNodeInfo nodeinfo; + const char *cpulist = NULL; + bool ret = true; + unsigned char *cpumap = NULL; + unsigned char *cpumaps = NULL; + size_t cpumaplen; + int i, cpu, lastcpu, maxcpu; + bool unuse = false; + const char *cur; + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); + bool query = false; /* Query mode if no cpulist */ + unsigned int flags = 0; + + if (current) { + if (live || config) { + vshError(ctl, "%s", _("--current must be specified exclusively")); + return false; + } + flags = VIR_DOMAIN_AFFECT_CURRENT; + } else { + if (config) + flags |= VIR_DOMAIN_AFFECT_CONFIG; + if (live) + flags |= VIR_DOMAIN_AFFECT_LIVE; + /* neither option is specified */ + if (!live && !config) + flags = -1; + } + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptString(cmd, "cpulist", &cpulist) < 0) { + vshError(ctl, "%s", _("emulatorpin: Missing cpulist.")); + virDomainFree(dom); + return false; + } + query = !cpulist; + + if (virNodeGetInfo(ctl->conn, &nodeinfo) != 0) { + virDomainFree(dom); + return false; + } + + maxcpu = VIR_NODEINFO_MAXCPUS(nodeinfo); + cpumaplen = VIR_CPU_MAPLEN(maxcpu); + + /* Query mode: show CPU affinity information then exit.*/ + if (query) { + /* When query mode and neither "live", "config" nor "current" + * is specified, set VIR_DOMAIN_AFFECT_CURRENT as flags */ + if (flags == -1) + flags = VIR_DOMAIN_AFFECT_CURRENT; + + cpumaps = vshMalloc(ctl, cpumaplen); + if (virDomainGetEmulatorPinInfo(dom, cpumaps, + cpumaplen, flags) >= 0) { + vshPrint(ctl, "%s %s\n", _("emulator:"), _("CPU Affinity")); + vshPrint(ctl, "----------------------------------\n"); + vshPrint(ctl, " *: "); + ret = vshPrintPinInfo(cpumaps, cpumaplen, maxcpu, 0); + vshPrint(ctl, "\n"); + } else { + ret = false; + } + VIR_FREE(cpumaps); + goto cleanup; + } + + /* Pin mode: pinning emulator threads to specified physical cpus*/ + + cpumap = vshCalloc(ctl, cpumaplen, sizeof(cpumap)); + /* Parse cpulist */ + cur = cpulist; + if (*cur == 0) { + goto parse_error; + } else if (*cur == 'r') { + for (cpu = 0; cpu < maxcpu; cpu++) + VIR_USE_CPU(cpumap, cpu); + cur = ""; + } + + while (*cur != 0) { + + /* the char '^' denotes exclusive */ + if (*cur == '^') { + cur++; + unuse = true; + } + + /* parse physical CPU number */ + if (!c_isdigit(*cur)) + goto parse_error;
That test looks redundant to me as virParseNumber will do that check and return -1 if it fails. I think we can remove those 2 lines and just trust virParseNumber() , but since that check is done in cmdVcpuPin() I will let that as a later cleanup for both
+ cpu = virParseNumber(&cur); + if (cpu < 0) { + goto parse_error; + } + if (cpu >= maxcpu) { + vshError(ctl, _("Physical CPU %d doesn't exist."), cpu); + goto parse_error; + } + virSkipSpaces(&cur); + + if (*cur == ',' || *cur == 0) { + if (unuse) { + VIR_UNUSE_CPU(cpumap, cpu); + } else { + VIR_USE_CPU(cpumap, cpu); + } + } else if (*cur == '-') { + /* the char '-' denotes range */ + if (unuse) { + goto parse_error; + } + cur++; + virSkipSpaces(&cur); + /* parse the end of range */ + lastcpu = virParseNumber(&cur); + if (lastcpu < cpu) { + goto parse_error; + } + if (lastcpu >= maxcpu) { + vshError(ctl, _("Physical CPU %d doesn't exist."), maxcpu); + goto parse_error; + } + for (i = cpu; i <= lastcpu; i++) { + VIR_USE_CPU(cpumap, i); + } + virSkipSpaces(&cur); + } + + if (*cur == ',') { + cur++; + virSkipSpaces(&cur); + unuse = false; + } else if (*cur == 0) { + break; + } else { + goto parse_error; + } + } + + if (flags == -1) + flags = VIR_DOMAIN_AFFECT_LIVE; + + if (virDomainPinEmulator(dom, cpumap, cpumaplen, flags) != 0) + ret = false; + +cleanup: + VIR_FREE(cpumap); + virDomainFree(dom); + return ret; + +parse_error: + vshError(ctl, "%s", _("cpulist: Invalid format.")); + ret = false; + goto cleanup; +} + +/* * "setvcpus" command */ static const vshCmdInfo info_setvcpus[] = { @@ -8200,6 +8387,7 @@ const vshCmdDef domManagementCmds[] = { {"vcpucount", cmdVcpucount, opts_vcpucount, info_vcpucount, 0}, {"vcpuinfo", cmdVcpuinfo, opts_vcpuinfo, info_vcpuinfo, 0}, {"vcpupin", cmdVcpuPin, opts_vcpupin, info_vcpupin, 0}, + {"emulatorpin", cmdEmulatorPin, opts_emulatorpin, info_emulatorpin, 0}, {"vncdisplay", cmdVNCDisplay, opts_vncdisplay, info_vncdisplay, 0}, {NULL, NULL, NULL, NULL, 0} }; diff --git a/tools/virsh.pod b/tools/virsh.pod index 35613c4..ec3c331 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1614,6 +1614,22 @@ If no flag is specified, behavior is different depending on hypervisor. B<Note>: The expression is sequentially evaluated, so "0-15,^8" is identical to "9-14,0-7,15" but not identical to "^8,0-15".
+=item B<emulatorpin> I<domain> [I<cpulist>] [[I<--live>] [I<--config>] + | [I<--current>]] + +Query or change the pinning of domain's emulator threads to host physical +CPUs. + +See B<vcpupin> for I<cpulist>. + +If I<--live> is specified, affect a running guest. +If I<--config> is specified, affect the next boot of a persistent guest. +If I<--current> is specified, affect the current guest state. +Both I<--live> and I<--config> flags may be given if I<cpulist> is present, +but I<--current> is exclusive. +If no flag is specified, behavior is different depending on hypervisor. + + =item B<vncdisplay> I<domain>
Output the IP address and port number for the VNC display. If the information
ACK, looks fine Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

This patch changes the behaviour of xml element cputune.period and cputune.quota to limit cpu bandwidth only for vcpus, and no longer limit cpu bandwidth for the whole guest. The reasons to do this are: - This matches docs of cputune.period and cputune.quota. - The other parts excepting vcpus are treated as "emulator", and there are separate period/quota settings for emulator in the subsequent patches --- src/qemu/qemu_cgroup.c | 33 ++++++------------------ src/qemu/qemu_driver.c | 65 ------------------------------------------------ 2 files changed, 8 insertions(+), 90 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 9bebfd5..2fb29b5 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -545,11 +545,16 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) unsigned int i; unsigned long long period = vm->def->cputune.period; long long quota = vm->def->cputune.quota; - long long vm_quota = 0; if (driver->cgroup == NULL) return 0; /* Not supported, so claim success */ + if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { + virReportError(VIR_ERR_SYSTEM_ERROR, "%s", + _("cgroup cpu is not active")); + return -1; + } + rc = virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0); if (rc != 0) { virReportSystemError(-rc, @@ -558,25 +563,6 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) goto cleanup; } - /* Set cpu bandwidth for the vm */ - if (period || quota) { - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { - /* Ensure that we can multiply by vcpus without overflowing. */ - if (quota > LLONG_MAX / vm->def->vcpus) { - virReportSystemError(EINVAL, "%s", - _("Unable to set cpu bandwidth quota")); - goto cleanup; - } - - if (quota > 0) - vm_quota = quota * vm->def->vcpus; - else - vm_quota = quota; - if (qemuSetupCgroupVcpuBW(cgroup, period, vm_quota) < 0) - goto cleanup; - } - } - if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) { /* If we does not know VCPU<->PID mapping or all vcpus run in the same * thread, we cannot control each vcpu. @@ -606,10 +592,8 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) } if (period || quota) { - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { - if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0) - goto cleanup; - } + if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0) + goto cleanup; } /* Set vcpupin in cgroup if vcpupin xml is provided */ @@ -624,7 +608,6 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) virCgroupFree(&cgroup_vcpu); } - virCgroupFree(&cgroup_vcpu); virCgroupFree(&cgroup); return 0; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3c8bbb7..8314375 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7700,69 +7700,10 @@ qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, qemuDomainObjPrivatePtr priv = vm->privateData; virCgroupPtr cgroup_vcpu = NULL; int rc; - long long vm_quota = 0; - long long old_quota = 0; - unsigned long long old_period = 0; if (period == 0 && quota == 0) return 0; - /* Ensure that we can multiply by vcpus without overflowing. */ - if (quota > LLONG_MAX / vm->def->vcpus) { - virReportSystemError(EINVAL, "%s", - _("Unable to set cpu bandwidth quota")); - goto cleanup; - } - - if (quota > 0) - vm_quota = quota * vm->def->vcpus; - else - vm_quota = quota; - - rc = virCgroupGetCpuCfsQuota(cgroup, &old_quota); - if (rc < 0) { - virReportSystemError(-rc, "%s", - _("unable to get cpu bandwidth tunable")); - goto cleanup; - } - - rc = virCgroupGetCpuCfsPeriod(cgroup, &old_period); - if (rc < 0) { - virReportSystemError(-rc, "%s", - _("unable to get cpu bandwidth period tunable")); - goto cleanup; - } - - /* - * If quota will be changed to a small value, we should modify vcpu's quota - * first. Otherwise, we should modify vm's quota first. - * - * If period will be changed to a small value, we should modify vm's period - * first. Otherwise, we should modify vcpu's period first. - * - * If both quota and period will be changed to a big/small value, we cannot - * modify period and quota together. - */ - if ((quota != 0) && (period != 0)) { - if (((quota > old_quota) && (period > old_period)) || - ((quota < old_quota) && (period < old_period))) { - /* modify period */ - if (qemuSetVcpusBWLive(vm, cgroup, period, 0) < 0) - goto cleanup; - - /* modify quota */ - if (qemuSetVcpusBWLive(vm, cgroup, 0, quota) < 0) - goto cleanup; - return 0; - } - } - - if (((vm_quota != 0) && (vm_quota > old_quota)) || - ((period != 0) && (period < old_period))) - /* Set cpu bandwidth for the vm */ - if (qemuSetupCgroupVcpuBW(cgroup, period, vm_quota) < 0) - goto cleanup; - /* If we does not know VCPU<->PID mapping or all vcpu runs in the same * thread, we cannot control each vcpu. So we only modify cpu bandwidth * when each vcpu has a separated thread. @@ -7785,12 +7726,6 @@ qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, } } - if (((vm_quota != 0) && (vm_quota <= old_quota)) || - ((period != 0) && (period >= old_period))) - /* Set cpu bandwidth for the vm */ - if (qemuSetupCgroupVcpuBW(cgroup, period, vm_quota) < 0) - goto cleanup; - return 0; cleanup: -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:41PM +0800, Hu Tao wrote:
This patch changes the behaviour of xml element cputune.period and cputune.quota to limit cpu bandwidth only for vcpus, and no longer limit cpu bandwidth for the whole guest.
The reasons to do this are:
- This matches docs of cputune.period and cputune.quota. - The other parts excepting vcpus are treated as "emulator", and there are separate period/quota settings for emulator in the subsequent patches
The real question is if this is actualy a change of behaviour. I think it is, but without doing this this gets quite hard to effectively control separately the usage for the hypervisor threads versus those of the guests. This is also the point of that large patch set ... so ACK based on the fact that people saw that coming and won't disagree with that possible small change in the control. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

This patch introduces support of setting emulator's period and quota to limit cpu bandwidth when the vm starts. Also updates XML Schema for new entries and docs. --- docs/formatdomain.html.in | 24 ++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 10 ++++++++++ src/conf/domain_conf.c | 25 +++++++++++++++++++++++-- src/conf/domain_conf.h | 2 ++ src/qemu/qemu_cgroup.c | 11 ++++++++++- 5 files changed, 69 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 81ec2cd..6142f4b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -388,6 +388,8 @@ <shares>2048</shares> <period>1000000</period> <quota>-1</quota> + <emulator_period>1000000</period> + <emulator_quota>-1</quota> </cputune> ... </domain> @@ -451,6 +453,28 @@ <span class="since">Only QEMU driver support since 0.9.4, LXC since 0.9.10</span> </dd> + + <dt><code>emulator_period</code></dt> + <dd> + The optional <code>emulator_period</code> element specifies the enforcement + interval(unit: microseconds). Within <code>emulator_period</code>, emulator + threads(those excluding vcpus) of the domain will not be allowed to consume + more than <code>emulator_quota</code> worth of runtime. The value should be + in range [1000, 1000000]. A period with value 0 means no value. + <span class="since">Only QEMU driver support since 0.10.0</span> + </dd> + <dt><code>emulator_quota</code></dt> + <dd> + The optional <code>emulator_quota</code> element specifies the maximum + allowed bandwidth(unit: microseconds) for domain's emulator threads(those + excluding vcpus). A domain with <code>emulator_quota</code> as any negative + value indicates that the domain has infinite bandwidth for emulator threads + (those excluding vcpus), which means that it is not bandwidth controlled. + The value should be in range [1000, 18446744073709551] or less than 0. A + quota with value 0 means no value. + <span class="since">Only QEMU driver support since 0.10.0</span> + </dd> + </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b02ad96..7aa6e47 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -581,6 +581,16 @@ <ref name="cpuquota"/> </element> </optional> + <optional> + <element name="emulator_period"> + <ref name="cpuperiod"/> + </element> + </optional> + <optional> + <element name="emulator_quota"> + <ref name="cpuquota"/> + </element> + </optional> <zeroOrMore> <element name="vcpupin"> <attribute name="vcpu"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index dab9c5d..7bb07b4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8297,6 +8297,14 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, &def->cputune.quota) < 0) def->cputune.quota = 0; + if (virXPathULongLong("string(./cputune/emulator_period[1])", ctxt, + &def->cputune.emulator_period) < 0) + def->cputune.emulator_period = 0; + + if (virXPathLongLong("string(./cputune/emulator_quota[1])", ctxt, + &def->cputune.emulator_quota) < 0) + def->cputune.emulator_quota = 0; + if ((n = virXPathNodeSet("./cputune/vcpupin", ctxt, &nodes)) < 0) { goto error; } @@ -13030,7 +13038,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->cputune.shares || def->cputune.vcpupin || def->cputune.period || def->cputune.quota || - def->cputune.emulatorpin) + def->cputune.emulatorpin || + def->cputune.emulator_period || def->cputune.emulator_quota) virBufferAddLit(buf, " <cputune>\n"); if (def->cputune.shares) @@ -13042,6 +13051,17 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->cputune.quota) virBufferAsprintf(buf, " <quota>%lld</quota>\n", def->cputune.quota); + + if (def->cputune.emulator_period) + virBufferAsprintf(buf, " <emulator_period>%llu" + "</emulator_period>\n", + def->cputune.emulator_period); + + if (def->cputune.emulator_quota) + virBufferAsprintf(buf, " <emulator_quota>%lld" + "</emulator_quota>\n", + def->cputune.emulator_quota); + if (def->cputune.vcpupin) { for (i = 0; i < def->cputune.nvcpupin; i++) { virBufferAsprintf(buf, " <vcpupin vcpu='%u' ", @@ -13080,7 +13100,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->cputune.shares || def->cputune.vcpupin || def->cputune.period || def->cputune.quota || - def->cputune.emulatorpin) + def->cputune.emulatorpin || + def->cputune.emulator_period || def->cputune.emulator_quota) virBufferAddLit(buf, " </cputune>\n"); if (def->numatune.memory.nodemask || diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b6bf5a8..089d9bc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1598,6 +1598,8 @@ struct _virDomainDef { unsigned long shares; unsigned long long period; long long quota; + unsigned long long emulator_period; + long long emulator_quota; int nvcpupin; virDomainVcpuPinDefPtr *vcpupin; virDomainVcpuPinDefPtr emulatorpin; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 2fb29b5..2237d11 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -564,7 +564,7 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) } if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) { - /* If we does not know VCPU<->PID mapping or all vcpus run in the same + /* If we don't know VCPU<->PID mapping or all vcpu runs in the same * thread, we cannot control each vcpu. */ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -631,6 +631,8 @@ int qemuSetupCgroupForEmulator(struct qemud_driver *driver, virCgroupPtr cgroup = NULL; virCgroupPtr cgroup_emulator = NULL; virDomainDefPtr def = vm->def; + unsigned long long period = vm->def->cputune.emulator_period; + long long quota = vm->def->cputune.emulator_quota; int rc, i; if (driver->cgroup == NULL) @@ -672,6 +674,13 @@ int qemuSetupCgroupForEmulator(struct qemud_driver *driver, qemuSetupCgroupEmulatorPin(cgroup_emulator, def->cputune.emulatorpin) < 0) goto cleanup; + if (period || quota) { + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { + if (qemuSetupCgroupVcpuBW(cgroup_emulator, period, quota) < 0) + goto cleanup; + } + } + virCgroupFree(&cgroup_emulator); virCgroupFree(&cgroup); return 0; -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:42PM +0800, Hu Tao wrote:
This patch introduces support of setting emulator's period and quota to limit cpu bandwidth when the vm starts. Also updates XML Schema for new entries and docs. --- docs/formatdomain.html.in | 24 ++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 10 ++++++++++ src/conf/domain_conf.c | 25 +++++++++++++++++++++++-- src/conf/domain_conf.h | 2 ++ src/qemu/qemu_cgroup.c | 11 ++++++++++- 5 files changed, 69 insertions(+), 3 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 81ec2cd..6142f4b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -388,6 +388,8 @@ <shares>2048</shares> <period>1000000</period> <quota>-1</quota> + <emulator_period>1000000</period> + <emulator_quota>-1</quota> </cputune> ... </domain> @@ -451,6 +453,28 @@ <span class="since">Only QEMU driver support since 0.9.4, LXC since 0.9.10</span> </dd> + + <dt><code>emulator_period</code></dt> + <dd> + The optional <code>emulator_period</code> element specifies the enforcement + interval(unit: microseconds). Within <code>emulator_period</code>, emulator + threads(those excluding vcpus) of the domain will not be allowed to consume + more than <code>emulator_quota</code> worth of runtime. The value should be + in range [1000, 1000000]. A period with value 0 means no value. + <span class="since">Only QEMU driver support since 0.10.0</span> + </dd> + <dt><code>emulator_quota</code></dt> + <dd> + The optional <code>emulator_quota</code> element specifies the maximum + allowed bandwidth(unit: microseconds) for domain's emulator threads(those + excluding vcpus). A domain with <code>emulator_quota</code> as any negative + value indicates that the domain has infinite bandwidth for emulator threads + (those excluding vcpus), which means that it is not bandwidth controlled. + The value should be in range [1000, 18446744073709551] or less than 0. A + quota with value 0 means no value. + <span class="since">Only QEMU driver support since 0.10.0</span> + </dd> + </dl>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b02ad96..7aa6e47 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -581,6 +581,16 @@ <ref name="cpuquota"/> </element> </optional> + <optional> + <element name="emulator_period"> + <ref name="cpuperiod"/> + </element> + </optional> + <optional> + <element name="emulator_quota"> + <ref name="cpuquota"/> + </element> + </optional> <zeroOrMore> <element name="vcpupin"> <attribute name="vcpu"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index dab9c5d..7bb07b4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8297,6 +8297,14 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, &def->cputune.quota) < 0) def->cputune.quota = 0;
+ if (virXPathULongLong("string(./cputune/emulator_period[1])", ctxt, + &def->cputune.emulator_period) < 0) + def->cputune.emulator_period = 0; + + if (virXPathLongLong("string(./cputune/emulator_quota[1])", ctxt, + &def->cputune.emulator_quota) < 0) + def->cputune.emulator_quota = 0; + if ((n = virXPathNodeSet("./cputune/vcpupin", ctxt, &nodes)) < 0) { goto error; } @@ -13030,7 +13038,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
if (def->cputune.shares || def->cputune.vcpupin || def->cputune.period || def->cputune.quota || - def->cputune.emulatorpin) + def->cputune.emulatorpin || + def->cputune.emulator_period || def->cputune.emulator_quota) virBufferAddLit(buf, " <cputune>\n");
if (def->cputune.shares) @@ -13042,6 +13051,17 @@ virDomainDefFormatInternal(virDomainDefPtr def, if (def->cputune.quota) virBufferAsprintf(buf, " <quota>%lld</quota>\n", def->cputune.quota); + + if (def->cputune.emulator_period) + virBufferAsprintf(buf, " <emulator_period>%llu" + "</emulator_period>\n", + def->cputune.emulator_period); + + if (def->cputune.emulator_quota) + virBufferAsprintf(buf, " <emulator_quota>%lld" + "</emulator_quota>\n", + def->cputune.emulator_quota); + if (def->cputune.vcpupin) { for (i = 0; i < def->cputune.nvcpupin; i++) { virBufferAsprintf(buf, " <vcpupin vcpu='%u' ", @@ -13080,7 +13100,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
if (def->cputune.shares || def->cputune.vcpupin || def->cputune.period || def->cputune.quota || - def->cputune.emulatorpin) + def->cputune.emulatorpin || + def->cputune.emulator_period || def->cputune.emulator_quota) virBufferAddLit(buf, " </cputune>\n");
if (def->numatune.memory.nodemask || diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b6bf5a8..089d9bc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1598,6 +1598,8 @@ struct _virDomainDef { unsigned long shares; unsigned long long period; long long quota; + unsigned long long emulator_period; + long long emulator_quota; int nvcpupin; virDomainVcpuPinDefPtr *vcpupin; virDomainVcpuPinDefPtr emulatorpin; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 2fb29b5..2237d11 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -564,7 +564,7 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) }
if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) { - /* If we does not know VCPU<->PID mapping or all vcpus run in the same + /* If we don't know VCPU<->PID mapping or all vcpu runs in the same * thread, we cannot control each vcpu. */ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -631,6 +631,8 @@ int qemuSetupCgroupForEmulator(struct qemud_driver *driver, virCgroupPtr cgroup = NULL; virCgroupPtr cgroup_emulator = NULL; virDomainDefPtr def = vm->def; + unsigned long long period = vm->def->cputune.emulator_period; + long long quota = vm->def->cputune.emulator_quota; int rc, i;
if (driver->cgroup == NULL) @@ -672,6 +674,13 @@ int qemuSetupCgroupForEmulator(struct qemud_driver *driver, qemuSetupCgroupEmulatorPin(cgroup_emulator, def->cputune.emulatorpin) < 0) goto cleanup;
+ if (period || quota) { + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { + if (qemuSetupCgroupVcpuBW(cgroup_emulator, period, quota) < 0) + goto cleanup; + } + } + virCgroupFree(&cgroup_emulator); virCgroupFree(&cgroup); return 0;
ACK, looks fine, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

This patch adds two macros: VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD, VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA for controlling cpu bandwidth for emulator activities not tied to vcpus --- include/libvirt/libvirt.h.in | 24 +++++++++++++++++++++--- tools/virsh.pod | 11 ++++++----- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 43774eb..cfe5047 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -679,19 +679,37 @@ typedef virTypedParameter *virTypedParameterPtr; * VIR_DOMAIN_SCHEDULER_VCPU_PERIOD: * * Macro represents the enforcement period for a quota, in microseconds, - * when using the posix scheduler, as a ullong. + * for vcpus only, when using the posix scheduler, as a ullong. */ #define VIR_DOMAIN_SCHEDULER_VCPU_PERIOD "vcpu_period" /** * VIR_DOMAIN_SCHEDULER_VCPU_QUOTA: * - * Macro represents the maximum bandwidth to be used within a period, - * when using the posix scheduler, as an llong. + * Macro represents the maximum bandwidth to be used within a period for + * vcpus only, when using the posix scheduler, as an llong. */ #define VIR_DOMAIN_SCHEDULER_VCPU_QUOTA "vcpu_quota" /** + * VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD: + * + * Macro represents the enforcement period for a quota in microseconds, + * when using the posix scheduler, for all emulator activity not tied to + * vcpus, as a ullong. + */ +#define VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD "emulator_period" + +/** + * VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA: + * + * Macro represents the maximum bandwidth to be used within a period for + * all emulator activity not tied to vcpus, when using the posix scheduler, + * as an llong. + */ +#define VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA "emulator_quota" + +/** * VIR_DOMAIN_SCHEDULER_WEIGHT: * * Macro represents the relative weight, when using the credit diff --git a/tools/virsh.pod b/tools/virsh.pod index ec3c331..e932d7c 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1202,7 +1202,8 @@ available for each hypervisor are: LXC (posix scheduler) : cpu_shares -QEMU/KVM (posix scheduler): cpu_shares, vcpu_period, vcpu_quota +QEMU/KVM (posix scheduler): cpu_shares, vcpu_period, vcpu_quota, +emulator_period, emulator_quota Xen (credit scheduler): weight, cap @@ -1220,10 +1221,10 @@ values 0 and 1 are automatically converted to a minimal value of 2. B<Note>: The weight and cap parameters are defined only for the XEN_CREDIT scheduler and are now I<DEPRECATED>. -B<Note>: The vcpu_period parameter has a valid value range of 1000-1000000 or -0, and the vcpu_quota parameter has a valid value range of -1000-18446744073709551 or less than 0. The value 0 for either parameter is -the same as not specifying that parameter. +B<Note>: The vcpu_period/emulator_period parameters have a valid value range +of 1000-1000000 or 0, and the vcpu_quota/emulator_quota parameters have a +valid value range of 1000-18446744073709551 or less than 0. The value 0 for +either parameter is the same as not specifying that parameter. =item B<screenshot> I<domain> [I<imagefilepath>] [I<--screen> B<screenID>] -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:43PM +0800, Hu Tao wrote:
This patch adds two macros: VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD, VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA for controlling cpu bandwidth for emulator activities not tied to vcpus --- include/libvirt/libvirt.h.in | 24 +++++++++++++++++++++--- tools/virsh.pod | 11 ++++++----- 2 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 43774eb..cfe5047 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -679,19 +679,37 @@ typedef virTypedParameter *virTypedParameterPtr; * VIR_DOMAIN_SCHEDULER_VCPU_PERIOD: * * Macro represents the enforcement period for a quota, in microseconds, - * when using the posix scheduler, as a ullong. + * for vcpus only, when using the posix scheduler, as a ullong. */ #define VIR_DOMAIN_SCHEDULER_VCPU_PERIOD "vcpu_period"
/** * VIR_DOMAIN_SCHEDULER_VCPU_QUOTA: * - * Macro represents the maximum bandwidth to be used within a period, - * when using the posix scheduler, as an llong. + * Macro represents the maximum bandwidth to be used within a period for + * vcpus only, when using the posix scheduler, as an llong. */ #define VIR_DOMAIN_SCHEDULER_VCPU_QUOTA "vcpu_quota"
/** + * VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD: + * + * Macro represents the enforcement period for a quota in microseconds, + * when using the posix scheduler, for all emulator activity not tied to + * vcpus, as a ullong. + */ +#define VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD "emulator_period" + +/** + * VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA: + * + * Macro represents the maximum bandwidth to be used within a period for + * all emulator activity not tied to vcpus, when using the posix scheduler, + * as an llong. + */ +#define VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA "emulator_quota" + +/** * VIR_DOMAIN_SCHEDULER_WEIGHT: * * Macro represents the relative weight, when using the credit diff --git a/tools/virsh.pod b/tools/virsh.pod index ec3c331..e932d7c 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1202,7 +1202,8 @@ available for each hypervisor are:
LXC (posix scheduler) : cpu_shares
-QEMU/KVM (posix scheduler): cpu_shares, vcpu_period, vcpu_quota +QEMU/KVM (posix scheduler): cpu_shares, vcpu_period, vcpu_quota, +emulator_period, emulator_quota
Xen (credit scheduler): weight, cap
@@ -1220,10 +1221,10 @@ values 0 and 1 are automatically converted to a minimal value of 2. B<Note>: The weight and cap parameters are defined only for the XEN_CREDIT scheduler and are now I<DEPRECATED>.
-B<Note>: The vcpu_period parameter has a valid value range of 1000-1000000 or -0, and the vcpu_quota parameter has a valid value range of -1000-18446744073709551 or less than 0. The value 0 for either parameter is -the same as not specifying that parameter. +B<Note>: The vcpu_period/emulator_period parameters have a valid value range +of 1000-1000000 or 0, and the vcpu_quota/emulator_quota parameters have a +valid value range of 1000-18446744073709551 or less than 0. The value 0 for +either parameter is the same as not specifying that parameter.
=item B<screenshot> I<domain> [I<imagefilepath>] [I<--screen> B<screenID>]
ACK, normal to expose and document those at the API level. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

From: Wen Congyang <wency@cn.fujitsu.com> allow the user change/get emulator's period and quota when the vm is running. --- src/qemu/qemu_driver.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 132 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8314375..0e7dc32 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6603,7 +6603,7 @@ static char *qemuGetSchedulerType(virDomainPtr dom, else if (rc == 0) *nparams = 1; else - *nparams = 3; + *nparams = 5; } ret = strdup("posix"); @@ -7734,6 +7734,40 @@ cleanup: } static int +qemuSetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, + unsigned long long period, long long quota) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virCgroupPtr cgroup_emulator = NULL; + int rc; + + if (period == 0 && quota == 0) + return 0; + + if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) { + return 0; + } + + rc = virCgroupForEmulator(cgroup, &cgroup_emulator, 0); + if (rc < 0) { + virReportSystemError(-rc, + _("Unable to find emulator cgroup for %s"), + vm->def->name); + goto cleanup; + } + + if (qemuSetupCgroupVcpuBW(cgroup_emulator, period, quota) < 0) + goto cleanup; + + virCgroupFree(&cgroup_emulator); + return 0; + +cleanup: + virCgroupFree(&cgroup_emulator); + return -1; +} + +static int qemuSetSchedulerParametersFlags(virDomainPtr dom, virTypedParameterPtr params, int nparams, @@ -7756,6 +7790,10 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, VIR_TYPED_PARAM_ULLONG, VIR_DOMAIN_SCHEDULER_VCPU_QUOTA, VIR_TYPED_PARAM_LLONG, + VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA, + VIR_TYPED_PARAM_LLONG, NULL) < 0) return -1; @@ -7838,6 +7876,32 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_CONFIG) { vmdef->cputune.quota = params[i].value.l; } + } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD)) { + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + rc = qemuSetEmulatorBandwidthLive(vm, group, params[i].value.ul, 0); + if (rc != 0) + goto cleanup; + + if (params[i].value.ul) + vm->def->cputune.emulator_period = params[i].value.ul; + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + vmdef->cputune.emulator_period = params[i].value.ul; + } + } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA)) { + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + rc = qemuSetEmulatorBandwidthLive(vm, group, 0, params[i].value.l); + if (rc != 0) + goto cleanup; + + if (params[i].value.l) + vm->def->cputune.emulator_quota = params[i].value.l; + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + vmdef->cputune.emulator_quota = params[i].value.l; + } } } @@ -7942,6 +8006,43 @@ cleanup: } static int +qemuGetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, + unsigned long long *period, long long *quota) +{ + virCgroupPtr cgroup_emulator = NULL; + qemuDomainObjPrivatePtr priv = NULL; + int rc; + int ret = -1; + + priv = vm->privateData; + if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) { + /* We don't create sub dir for each vcpu */ + *period = 0; + *quota = 0; + return 0; + } + + /* get period and quota for emulator */ + rc = virCgroupForEmulator(cgroup, &cgroup_emulator, 0); + if (!cgroup_emulator) { + virReportSystemError(-rc, + _("Unable to find emulator cgroup for %s"), + vm->def->name); + goto cleanup; + } + + rc = qemuGetVcpuBWLive(cgroup_emulator, period, quota); + if (rc < 0) + goto cleanup; + + ret = 0; + +cleanup: + virCgroupFree(&cgroup_emulator); + return ret; +} + +static int qemuGetSchedulerParametersFlags(virDomainPtr dom, virTypedParameterPtr params, int *nparams, @@ -7953,6 +8054,8 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, unsigned long long shares; unsigned long long period; long long quota; + unsigned long long emulator_period; + long long emulator_quota; int ret = -1; int rc; bool cpu_bw_status = false; @@ -7992,6 +8095,8 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, if (*nparams > 1 && cpu_bw_status) { period = persistentDef->cputune.period; quota = persistentDef->cputune.quota; + emulator_period = persistentDef->cputune.emulator_period; + emulator_quota = persistentDef->cputune.emulator_quota; } goto out; } @@ -8020,6 +8125,14 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, if (rc != 0) goto cleanup; } + + if (*nparams > 3 && cpu_bw_status) { + rc = qemuGetEmulatorBandwidthLive(vm, group, &emulator_period, + &emulator_quota); + if (rc != 0) + goto cleanup; + } + out: if (virTypedParameterAssign(¶ms[0], VIR_DOMAIN_SCHEDULER_CPU_SHARES, VIR_TYPED_PARAM_ULLONG, shares) < 0) @@ -8042,6 +8155,24 @@ out: goto cleanup; saved_nparams++; } + + if (*nparams > saved_nparams) { + if (virTypedParameterAssign(¶ms[3], + VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD, + VIR_TYPED_PARAM_ULLONG, + emulator_period) < 0) + goto cleanup; + saved_nparams++; + } + + if (*nparams > saved_nparams) { + if (virTypedParameterAssign(¶ms[4], + VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA, + VIR_TYPED_PARAM_LLONG, + emulator_quota) < 0) + goto cleanup; + saved_nparams++; + } } *nparams = saved_nparams; -- 1.7.10.2

On Tue, Aug 21, 2012 at 05:18:44PM +0800, Hu Tao wrote:
From: Wen Congyang <wency@cn.fujitsu.com>
allow the user change/get emulator's period and quota when the vm is running. --- src/qemu/qemu_driver.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 132 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8314375..0e7dc32 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6603,7 +6603,7 @@ static char *qemuGetSchedulerType(virDomainPtr dom, else if (rc == 0) *nparams = 1; else - *nparams = 3; + *nparams = 5; }
ret = strdup("posix"); @@ -7734,6 +7734,40 @@ cleanup: }
static int +qemuSetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, + unsigned long long period, long long quota) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virCgroupPtr cgroup_emulator = NULL; + int rc; + + if (period == 0 && quota == 0) + return 0; + + if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) { + return 0; + } + + rc = virCgroupForEmulator(cgroup, &cgroup_emulator, 0); + if (rc < 0) { + virReportSystemError(-rc, + _("Unable to find emulator cgroup for %s"), + vm->def->name); + goto cleanup; + } + + if (qemuSetupCgroupVcpuBW(cgroup_emulator, period, quota) < 0) + goto cleanup; + + virCgroupFree(&cgroup_emulator); + return 0; + +cleanup: + virCgroupFree(&cgroup_emulator); + return -1; +} + +static int qemuSetSchedulerParametersFlags(virDomainPtr dom, virTypedParameterPtr params, int nparams, @@ -7756,6 +7790,10 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, VIR_TYPED_PARAM_ULLONG, VIR_DOMAIN_SCHEDULER_VCPU_QUOTA, VIR_TYPED_PARAM_LLONG, + VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA, + VIR_TYPED_PARAM_LLONG, NULL) < 0) return -1;
@@ -7838,6 +7876,32 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_CONFIG) { vmdef->cputune.quota = params[i].value.l; } + } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD)) { + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + rc = qemuSetEmulatorBandwidthLive(vm, group, params[i].value.ul, 0); + if (rc != 0) + goto cleanup; + + if (params[i].value.ul) + vm->def->cputune.emulator_period = params[i].value.ul; + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + vmdef->cputune.emulator_period = params[i].value.ul; + } + } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA)) { + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + rc = qemuSetEmulatorBandwidthLive(vm, group, 0, params[i].value.l); + if (rc != 0) + goto cleanup; + + if (params[i].value.l) + vm->def->cputune.emulator_quota = params[i].value.l; + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + vmdef->cputune.emulator_quota = params[i].value.l; + } } }
@@ -7942,6 +8006,43 @@ cleanup: }
static int +qemuGetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, + unsigned long long *period, long long *quota) +{ + virCgroupPtr cgroup_emulator = NULL; + qemuDomainObjPrivatePtr priv = NULL; + int rc; + int ret = -1; + + priv = vm->privateData; + if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) { + /* We don't create sub dir for each vcpu */ + *period = 0; + *quota = 0; + return 0; + } + + /* get period and quota for emulator */ + rc = virCgroupForEmulator(cgroup, &cgroup_emulator, 0); + if (!cgroup_emulator) { + virReportSystemError(-rc, + _("Unable to find emulator cgroup for %s"), + vm->def->name); + goto cleanup; + } + + rc = qemuGetVcpuBWLive(cgroup_emulator, period, quota); + if (rc < 0) + goto cleanup; + + ret = 0; + +cleanup: + virCgroupFree(&cgroup_emulator); + return ret; +} + +static int qemuGetSchedulerParametersFlags(virDomainPtr dom, virTypedParameterPtr params, int *nparams, @@ -7953,6 +8054,8 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, unsigned long long shares; unsigned long long period; long long quota; + unsigned long long emulator_period; + long long emulator_quota; int ret = -1; int rc; bool cpu_bw_status = false; @@ -7992,6 +8095,8 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, if (*nparams > 1 && cpu_bw_status) { period = persistentDef->cputune.period; quota = persistentDef->cputune.quota; + emulator_period = persistentDef->cputune.emulator_period; + emulator_quota = persistentDef->cputune.emulator_quota; } goto out; } @@ -8020,6 +8125,14 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, if (rc != 0) goto cleanup; } + + if (*nparams > 3 && cpu_bw_status) { + rc = qemuGetEmulatorBandwidthLive(vm, group, &emulator_period, + &emulator_quota); + if (rc != 0) + goto cleanup; + } + out: if (virTypedParameterAssign(¶ms[0], VIR_DOMAIN_SCHEDULER_CPU_SHARES, VIR_TYPED_PARAM_ULLONG, shares) < 0) @@ -8042,6 +8155,24 @@ out: goto cleanup; saved_nparams++; } + + if (*nparams > saved_nparams) { + if (virTypedParameterAssign(¶ms[3], + VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD, + VIR_TYPED_PARAM_ULLONG, + emulator_period) < 0) + goto cleanup; + saved_nparams++; + } + + if (*nparams > saved_nparams) { + if (virTypedParameterAssign(¶ms[4], + VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA, + VIR_TYPED_PARAM_LLONG, + emulator_quota) < 0) + goto cleanup; + saved_nparams++; + } }
*nparams = saved_nparams;
Okay, we now have 5 parameters instead of 3. that looks fine, ACK, the question I have is if virsh is actually able to display the 5 parameters by default now, if yes the patch set seems complete ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Wed, Aug 22, 2012 at 05:04:57PM +0800, Daniel Veillard wrote:
On Tue, Aug 21, 2012 at 05:18:44PM +0800, Hu Tao wrote:
From: Wen Congyang <wency@cn.fujitsu.com>
allow the user change/get emulator's period and quota when the vm is running. --- src/qemu/qemu_driver.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 132 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8314375..0e7dc32 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6603,7 +6603,7 @@ static char *qemuGetSchedulerType(virDomainPtr dom, else if (rc == 0) *nparams = 1; else - *nparams = 3; + *nparams = 5; }
ret = strdup("posix"); @@ -7734,6 +7734,40 @@ cleanup: }
static int +qemuSetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, + unsigned long long period, long long quota) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virCgroupPtr cgroup_emulator = NULL; + int rc; + + if (period == 0 && quota == 0) + return 0; + + if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) { + return 0; + } + + rc = virCgroupForEmulator(cgroup, &cgroup_emulator, 0); + if (rc < 0) { + virReportSystemError(-rc, + _("Unable to find emulator cgroup for %s"), + vm->def->name); + goto cleanup; + } + + if (qemuSetupCgroupVcpuBW(cgroup_emulator, period, quota) < 0) + goto cleanup; + + virCgroupFree(&cgroup_emulator); + return 0; + +cleanup: + virCgroupFree(&cgroup_emulator); + return -1; +} + +static int qemuSetSchedulerParametersFlags(virDomainPtr dom, virTypedParameterPtr params, int nparams, @@ -7756,6 +7790,10 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, VIR_TYPED_PARAM_ULLONG, VIR_DOMAIN_SCHEDULER_VCPU_QUOTA, VIR_TYPED_PARAM_LLONG, + VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA, + VIR_TYPED_PARAM_LLONG, NULL) < 0) return -1;
@@ -7838,6 +7876,32 @@ qemuSetSchedulerParametersFlags(virDomainPtr dom, if (flags & VIR_DOMAIN_AFFECT_CONFIG) { vmdef->cputune.quota = params[i].value.l; } + } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD)) { + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + rc = qemuSetEmulatorBandwidthLive(vm, group, params[i].value.ul, 0); + if (rc != 0) + goto cleanup; + + if (params[i].value.ul) + vm->def->cputune.emulator_period = params[i].value.ul; + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + vmdef->cputune.emulator_period = params[i].value.ul; + } + } else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA)) { + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + rc = qemuSetEmulatorBandwidthLive(vm, group, 0, params[i].value.l); + if (rc != 0) + goto cleanup; + + if (params[i].value.l) + vm->def->cputune.emulator_quota = params[i].value.l; + } + + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + vmdef->cputune.emulator_quota = params[i].value.l; + } } }
@@ -7942,6 +8006,43 @@ cleanup: }
static int +qemuGetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, + unsigned long long *period, long long *quota) +{ + virCgroupPtr cgroup_emulator = NULL; + qemuDomainObjPrivatePtr priv = NULL; + int rc; + int ret = -1; + + priv = vm->privateData; + if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) { + /* We don't create sub dir for each vcpu */ + *period = 0; + *quota = 0; + return 0; + } + + /* get period and quota for emulator */ + rc = virCgroupForEmulator(cgroup, &cgroup_emulator, 0); + if (!cgroup_emulator) { + virReportSystemError(-rc, + _("Unable to find emulator cgroup for %s"), + vm->def->name); + goto cleanup; + } + + rc = qemuGetVcpuBWLive(cgroup_emulator, period, quota); + if (rc < 0) + goto cleanup; + + ret = 0; + +cleanup: + virCgroupFree(&cgroup_emulator); + return ret; +} + +static int qemuGetSchedulerParametersFlags(virDomainPtr dom, virTypedParameterPtr params, int *nparams, @@ -7953,6 +8054,8 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, unsigned long long shares; unsigned long long period; long long quota; + unsigned long long emulator_period; + long long emulator_quota; int ret = -1; int rc; bool cpu_bw_status = false; @@ -7992,6 +8095,8 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, if (*nparams > 1 && cpu_bw_status) { period = persistentDef->cputune.period; quota = persistentDef->cputune.quota; + emulator_period = persistentDef->cputune.emulator_period; + emulator_quota = persistentDef->cputune.emulator_quota; } goto out; } @@ -8020,6 +8125,14 @@ qemuGetSchedulerParametersFlags(virDomainPtr dom, if (rc != 0) goto cleanup; } + + if (*nparams > 3 && cpu_bw_status) { + rc = qemuGetEmulatorBandwidthLive(vm, group, &emulator_period, + &emulator_quota); + if (rc != 0) + goto cleanup; + } + out: if (virTypedParameterAssign(¶ms[0], VIR_DOMAIN_SCHEDULER_CPU_SHARES, VIR_TYPED_PARAM_ULLONG, shares) < 0) @@ -8042,6 +8155,24 @@ out: goto cleanup; saved_nparams++; } + + if (*nparams > saved_nparams) { + if (virTypedParameterAssign(¶ms[3], + VIR_DOMAIN_SCHEDULER_EMULATOR_PERIOD, + VIR_TYPED_PARAM_ULLONG, + emulator_period) < 0) + goto cleanup; + saved_nparams++; + } + + if (*nparams > saved_nparams) { + if (virTypedParameterAssign(¶ms[4], + VIR_DOMAIN_SCHEDULER_EMULATOR_QUOTA, + VIR_TYPED_PARAM_LLONG, + emulator_quota) < 0) + goto cleanup; + saved_nparams++; + } }
*nparams = saved_nparams;
Okay, we now have 5 parameters instead of 3. that looks fine, ACK,
the question I have is if virsh is actually able to display the 5 parameters by default now, if yes the patch set seems complete !
Indeed. The virsh output here is: virsh schedinfo example-domain Scheduler : posix cpu_shares : 1024 vcpu_period : 100000 vcpu_quota : -1 emulator_period: 400000 emulator_quota : 100000 -- Thanks, Hu Tao

On Wed, Aug 22, 2012 at 05:19:32PM +0800, Hu Tao wrote:
On Wed, Aug 22, 2012 at 05:04:57PM +0800, Daniel Veillard wrote:
On Tue, Aug 21, 2012 at 05:18:44PM +0800, Hu Tao wrote: [...] Okay, we now have 5 parameters instead of 3. that looks fine, ACK,
the question I have is if virsh is actually able to display the 5 parameters by default now, if yes the patch set seems complete !
Indeed. The virsh output here is:
virsh schedinfo example-domain Scheduler : posix cpu_shares : 1024 vcpu_period : 100000 vcpu_quota : -1 emulator_period: 400000 emulator_quota : 100000
okay, cool, all set then I think, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Tue, Aug 21, 2012 at 05:18:23PM +0800, Hu Tao wrote:
This series adds support of emulator-pin to pin emulator threads on specified physical CPUs, and emulator-bandwidth to control physical CPU bandwidth for emulator threads.
changes:
v2.1: - rebase - include emulator-bandwidth patches - minor fix of virCgroupAddTaskStrController
Okay, i have pushed patch 1-9 based on Dan reviews and mine, I'm looking at the end of that set now, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Wed, Aug 22, 2012 at 03:55:48PM +0800, Daniel Veillard wrote:
On Tue, Aug 21, 2012 at 05:18:23PM +0800, Hu Tao wrote:
This series adds support of emulator-pin to pin emulator threads on specified physical CPUs, and emulator-bandwidth to control physical CPU bandwidth for emulator threads.
changes:
v2.1: - rebase - include emulator-bandwidth patches - minor fix of virCgroupAddTaskStrController
Okay, i have pushed patch 1-9 based on Dan reviews and mine, I'm looking at the end of that set now,
okay, done the whole set is in, but please answer my last question on patch 21 review, thanks, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Wed, Aug 22, 2012 at 05:06:03PM +0800, Daniel Veillard wrote:
On Wed, Aug 22, 2012 at 03:55:48PM +0800, Daniel Veillard wrote:
On Tue, Aug 21, 2012 at 05:18:23PM +0800, Hu Tao wrote:
This series adds support of emulator-pin to pin emulator threads on specified physical CPUs, and emulator-bandwidth to control physical CPU bandwidth for emulator threads.
changes:
v2.1: - rebase - include emulator-bandwidth patches - minor fix of virCgroupAddTaskStrController
Okay, i have pushed patch 1-9 based on Dan reviews and mine, I'm looking at the end of that set now,
okay, done the whole set is in, but please answer my last question on patch 21 review,
Thank you for reviewing the series! -- Thanks, Hu Tao
participants (3)
-
Daniel P. Berrange
-
Daniel Veillard
-
Hu Tao