On Mon, Feb 24, 2020 at 11:24:28AM -0300, Julio Faracco wrote:
> This commit tries to fix lots of issues related to LXC VCPUs. One of
> them is related to /proc/cpuinfo content. If only 1 VCPU is set, LXC
> containers will show all CPUs available for host. The second one is
> related to CPU share, if an user set only 1 VCPU, the container/process
> will use all available CPUs. (This is not the case when `cpuset`
> attribute is declared). So, this commit adds a virtual cpuinfo based on
> VCPU mapping and it automatically limits the CPU usage according VCPU
> count.
>
> Example (now):
> LXC container - 8 CPUS with 2 VCPU:
> lxc-root# stress --cpu 8
>
> On host machine, only CPU 0 and 1 have 100% usage.
>
> Signed-off-by: Julio Faracco <jcfaracco@xxxxxxxxx>
> ---
> src/lxc/lxc_cgroup.c | 31 ++++++++++++++
> src/lxc/lxc_container.c | 39 ++++++++++-------
> src/lxc/lxc_fuse.c | 95 ++++++++++++++++++++++++++++++++++++++---
> 3 files changed, 145 insertions(+), 20 deletions(-)
>
> diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
> index d29b65092a..912a252473 100644
> --- a/src/lxc/lxc_cgroup.c
> +++ b/src/lxc/lxc_cgroup.c
> @@ -50,6 +50,34 @@ static int virLXCCgroupSetupCpuTune(virDomainDefPtr def,
> }
>
>
> +static int virLXCCgroupSetupVcpuAuto(virDomainDefPtr def,
> + virCgroupPtr cgroup)
> +{
> + size_t i;
> + int vcpumax;
> + virBuffer buffer = VIR_BUFFER_INITIALIZER;
> + virBufferPtr cpuset = &buffer;
> +
> + vcpumax = virDomainDefGetVcpusMax(def);
> + for (i = 0; i < vcpumax; i++) {
> + virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(def, i);
> + /* Cgroup is smart enough to convert numbers separated
> + * by comma into ranges. Example: "0,1,2,5," -> "0-2,5".
> + * Libvirt does not need to process it here. */
> + if (vcpu)
> + virBufferAsprintf(cpuset, "%zu,", i);
> + }
If I have 4 containers, each with vcpu==2, then all 4 containers
are going to be taskset onto host CPUs 0 & 1, and host CPUs 2,3,4,5,6,7
are going to be unused. This is not viable as a strategy.
> + if (virCgroupSetCpusetCpus(cgroup,
> + virBufferCurrentContent(cpuset)) < 0) {
> + virBufferFreeAndReset(cpuset);
> + return -1;
> + }
> +
> + virBufferFreeAndReset(cpuset);
> + return 0;
> +}
> +static int
> +lxcProcReadCpuinfoParse(virDomainDefPtr def, char *base,
> + virBufferPtr new_cpuinfo)
> +{
> + char *procline = NULL;
> + char *saveptr = base;
> + size_t cpu;
> + size_t nvcpu;
> + size_t curcpu = 0;
> + bool get_proc = false;
> +
> + nvcpu = virDomainDefGetVcpus(def);
> + while ((procline = strtok_r(NULL, "\n", &saveptr))) {
> + if (sscanf(procline, "processor\t: %zu", &cpu) == 1) {
This doesn't work because it is assuming the /proc/cpuinfo data format
for x86 architecture. Every architecture puts different info in this
file, and only some of them using "processor: NN" as a delimiter for
new entries. There's many examples in tests/sysinfodata/ and
in tests/virhostcpudata/ showing this problem
We had considered taking /proc/cpuinfo in the past, but decided against
it. Since this file is non-standard data format varying across arches,
well written apps won't actually look at /proc/cpuinfo at all. Instead
they'll use /sys/devices/system/cpu instead. I don't think we want to
make /proc/cpuinfo be inconsistent with data in sysfs.
> + virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(def, cpu);
> + /* VCPU is mapped */
> + if (vcpu) {
> + if (curcpu == nvcpu)
> + break;
> +
> + if (curcpu > 0)
> + virBufferAddLit(new_cpuinfo, "\n");
> +
> + virBufferAsprintf(new_cpuinfo, "processor\t: %zu\n",
> + curcpu);
> + curcpu++;
> + get_proc = true;
> + } else {
> + get_proc = false;
> + }
> + } else {
> + /* It is not a processor index */
> + if (get_proc)
> + virBufferAsprintf(new_cpuinfo, "%s\n", procline);
> + }
> + }
> +
> + virBufferAddLit(new_cpuinfo, "\n");
> +
> + return strlen(virBufferCurrentContent(new_cpuinfo));
> +}
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
