On 06/05/2012 02:16 AM, tangchen wrote:
> create a new cgroup and move all hypervisor 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 hypervisor threads(include vhost-net threads)
A really useful thing to add to this commit message would be an ascii
view of the cgroup hierarchy being created. If I understand correctly,
this creates the following levels:
cgroup mount point
libvirt subdirectory (all libvirt management)
driver subdirectory (all guests tied to one driver)
hypervisor subdirectory (all processes tied to one guest)
vcpu subdirectory (all processes tied to one VCPU of a guest)
I would almost prefer to call it a VM cgroup instead of a hypervisor
cgroup (and that reflects back to naming chosen in 2/13), as I tend to
think of 'hypervisor' meaning 'qemu' - the technology that drives
multiple guests, while I think of 'VM' meaning 'single guest', a
collection of possible multiple processes under a single qemu process
umbrella for running a given guest.
>
> +int qemuSetupCgroupForHypervisor(struct qemud_driver *driver,
> + virDomainObjPtr vm)
More evidence that the naming choice is confusing - you named the
parameter 'vm' instead of 'hypervisor'. That is, I think naming this
qemuSetupCgroupForVM(...) makes more sense.
> +
> + if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) {
> + /* If we does not know VCPU<->PID mapping or all vcpu runs in the same
s/vcpu runs/vcpus run/
> + * thread, we cannot control each vcpu.
> + */
> + virCgroupFree(&cgroup);
> + return 0;
It makes sense to ignore failure to set up a vcpu sub-cgroup if the user
never requested the feature, with the end result being that they lose
out on the functionality. But if the user explicitly requested per-vcpu
usage and we can't set it up, should this return a failure? In other
words, I'm worried whether we need to detect whether it is always safe
to ignore the failure (as done here) or whether there are situations
where setup failure should prevent running the VM until the cgroup
situation is resolved.
> +
> + rc = virCgroupMoveTask(cgroup, cgroup_hypervisor);
> + if (rc < 0) {
> + virReportSystemError(-rc,
> + _("Unable to move taks from domain cgroup to "
s/taks/task/, and listing the task id might be useful for diagnostic
purposes.
> +++ b/src/qemu/qemu_process.c
> @@ -3674,6 +3674,10 @@ int qemuProcessStart(virConnectPtr conn,
> if (qemuSetupCgroupForVcpu(driver, vm) < 0)
> goto cleanup;
>
> + VIR_DEBUG("Setting cgroup for hypervisor(if required)");
s/hypervisor(if/hypervisor (if/
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list