[libvirt] [PATCH RESEND RFC v4 5/6] qemu: Implement cfs_period and cfs_quota's modification

[Daniel P. Berrange]

On Thu, Jul 21, 2011 at 08:00:21AM -0500, Adam Litke wrote:
> 
> 
> On 07/20/2011 09:11 PM, Wen Congyang wrote:
> > +static int
> > +qemuGetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup,
> > +                   unsigned long long *period, long long *quota)
> > +{
> > +    virCgroupPtr cgroup_vcpu = NULL;
> > +    qemuDomainObjPrivatePtr priv = NULL;
> > +    int rc;
> > +    int ret = -1;
> > +
> > +    priv = vm->privateData;
> > +    if (priv->nvcpupids == 0 || priv->vcpupids[0] == vm->pid) {
> > +        /* We do not create sub dir for each vcpu */
> > +        rc = qemuGetVcpuBWLive(cgroup, period, quota);
> > +        if (rc < 0)
> > +            goto cleanup;
> > +
> > +        if (*quota > 0)
> > +            *quota /= vm->def->vcpus;
> > +        goto out;
> > +    }
> 
> Are you sure the above is correct?  Based on my earlier suggestion,
> <quota> is always specified as the amount of runtime afforded to a
> single vcpu.  Hence, if you are changing quota to cover for all of a
> vm's vcpus, wouldn't you want to:
> 
>           *quota *= vm->def->vcpus;

This is a good example of why I think the current design is flawed
and needs separate limits explicitly tracked, one for the VM as
a whole, and another for individual VCPUs. The per-VCPU limit
would then simply be unsupported for the case of a QEMU without
vCPU threads, but the per-VM limit could still be used.

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