This commit tries to fix a 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 automatic 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@gmail.com>
---
src/lxc/lxc_cgroup.c | 31 ++++++++++++++++
src/lxc/lxc_container.c | 15 ++++++++
src/lxc/lxc_fuse.c | 78 ++++++++++++++++++++++++++++++++++++++---
3 files changed, 120 insertions(+), 4 deletions(-)
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
index 470337e675..a6c73d9d55 100644
--- a/src/lxc/lxc_cgroup.c
+++ b/src/lxc/lxc_cgroup.c
@@ -59,6 +59,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 (virCgroupSetCpusetCpus(cgroup,
+ virBufferCurrentContent(cpuset)) < 0) {
+ virBufferFreeAndReset(cpuset);
+ return -1;
+ }
+
+ virBufferFreeAndReset(cpuset);
+ return 0;
+}
+
+
static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def,
virCgroupPtr cgroup,
virBitmapPtr nodemask)
@@ -76,6 +104,9 @@ static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def,
goto cleanup;
/* free mask to make sure we won't use it in a wrong way later */
VIR_FREE(mask);
+ } else {
+ /* auto mode for VCPU limits */
+ virLXCCgroupSetupVcpuAuto(def, cgroup);
}
if (virDomainNumatuneGetMode(def->numa, -1, &mode) < 0 ||
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 41efe43a14..1a2c97c9f4 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -999,6 +999,7 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def,
{
int ret;
char *meminfo_path = NULL;
+ char *cpuinfo_path = NULL;
VIR_DEBUG("Mount /proc/meminfo stateDir=%s", stateDir);
@@ -1013,7 +1014,21 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def,
meminfo_path);
}
+ VIR_DEBUG("Mount /proc/cpuinfo stateDir=%s", stateDir);
+
+ cpuinfo_path = g_strdup_printf("/.oldroot/%s/%s.fuse/cpuinfo",
+ stateDir,
+ def->name);
+
+ if ((ret = mount(cpuinfo_path, "/proc/cpuinfo",
+ NULL, MS_BIND, NULL)) < 0) {
+ virReportSystemError(errno,
+ _("Failed to mount %s on /proc/cpuinfo"),
+ cpuinfo_path);
+ }
+
VIR_FREE(meminfo_path);
+ VIR_FREE(cpuinfo_path);
return ret;
}
#else
diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c
index 44f240a0b5..12fa69d494 100644
--- a/src/lxc/lxc_fuse.c
+++ b/src/lxc/lxc_fuse.c
@@ -37,6 +37,7 @@
#if WITH_FUSE
static const char *fuse_meminfo_path = "/meminfo";
+static const char *fuse_cpuinfo_path = "/cpuinfo";
static int lxcProcGetattr(const char *path, struct stat *stbuf)
{
@@ -54,7 +55,8 @@ static int lxcProcGetattr(const char *path, struct stat *stbuf)
if (STREQ(path, "/")) {
stbuf->st_mode = S_IFDIR | 0755;
stbuf->st_nlink = 2;
- } else if (STREQ(path, fuse_meminfo_path)) {
+ } else if (STREQ(path, fuse_meminfo_path) ||
+ STREQ(path, fuse_cpuinfo_path)) {
if (stat(mempath, &sb) < 0) {
res = -errno;
goto cleanup;
@@ -90,6 +92,7 @@ static int lxcProcReaddir(const char *path, void *buf,
filler(buf, ".", NULL, 0);
filler(buf, "..", NULL, 0);
filler(buf, fuse_meminfo_path + 1, NULL, 0);
+ filler(buf, fuse_cpuinfo_path + 1, NULL, 0);
return 0;
}
@@ -97,7 +100,8 @@ static int lxcProcReaddir(const char *path, void *buf,
static int lxcProcOpen(const char *path G_GNUC_UNUSED,
struct fuse_file_info *fi G_GNUC_UNUSED)
{
- if (STRNEQ(path, fuse_meminfo_path))
+ if (STRNEQ(path, fuse_meminfo_path) &&
+ STRNEQ(path, fuse_cpuinfo_path))
return -ENOENT;
if ((fi->flags & 3) != O_RDONLY)
@@ -125,7 +129,7 @@ static int lxcProcHostRead(char *path, char *buf, size_t size, off_t offset)
static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
char *buf, size_t size, off_t offset)
{
- int res;
+ int res = -1;
FILE *fd = NULL;
char *line = NULL;
size_t n;
@@ -151,7 +155,6 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
goto cleanup;
}
- res = -1;
while (getline(&line, &n, fd) > 0) {
char *ptr = strchr(line, ':');
if (!ptr)
@@ -235,6 +238,70 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
return res;
}
+
+static int lxcProcReadCpuinfo(char *hostpath, virDomainDefPtr def,
+ char *buf, size_t size, off_t offset)
+{
+ int res = -1;
+ FILE *fd = NULL;
+ char *line = NULL;
+ size_t n;
+ virBuffer buffer = VIR_BUFFER_INITIALIZER;
+ virBufferPtr new_cpuinfo = &buffer;
+ size_t cpu;
+ size_t nvcpu;
+ size_t curcpu = 0;
+ bool get_proc = false;
+
+ fd = fopen(hostpath, "r");
+ if (fd == NULL) {
+ virReportSystemError(errno, _("Cannot open %s"), hostpath);
+ res = -errno;
+ goto cleanup;
+ }
+
+ /* /proc/cpuinfo does not support fseek */
+ if (offset > 0) {
+ res = 0;
+ goto cleanup;
+ }
+
+ nvcpu = virDomainDefGetVcpus(def);
+ while (getline(&line, &n, fd) > 0) {
+ if (sscanf(line, "processor\t: %zu", &cpu) == 1) {
+ virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(def, cpu);
+ /* VCPU is mapped */
+ if (vcpu) {
+ if (curcpu == nvcpu)
+ break;
+
+ 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)
+ virBufferAdd(new_cpuinfo, line, -1);
+ }
+ }
+
+ res = strlen(virBufferCurrentContent(new_cpuinfo));
+ if (res > size)
+ res = size;
+ memcpy(buf, virBufferCurrentContent(new_cpuinfo), res);
+
+ cleanup:
+ VIR_FREE(line);
+ virBufferFreeAndReset(new_cpuinfo);
+ VIR_FORCE_FCLOSE(fd);
+ return res;
+}
+
+
static int lxcProcRead(const char *path G_GNUC_UNUSED,
char *buf G_GNUC_UNUSED,
size_t size G_GNUC_UNUSED,
@@ -254,6 +321,9 @@ static int lxcProcRead(const char *path G_GNUC_UNUSED,
if (STREQ(path, fuse_meminfo_path)) {
if ((res = lxcProcReadMeminfo(hostpath, def, buf, size, offset)) < 0)
res = lxcProcHostRead(hostpath, buf, size, offset);
+ } else if (STREQ(path, fuse_cpuinfo_path)) {
+ if ((res = lxcProcReadCpuinfo(hostpath, def, buf, size, offset)) < 0)
+ res = lxcProcHostRead(hostpath, buf, size, offset);
}
VIR_FREE(hostpath);
--
2.20.1
On 2/16/20 2:11 PM, Julio Faracco wrote:
> This commit tries to fix a lots of issues related to LXC VCPUs. One of
Extra 'a' there. "tries to fix lots of issues ..."
> 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`
This parentesis is never closed.
> attribute is declared. So, this commit adds a virtual cpuinfo based on
> VCPU mapping and it automatic limits the CPU usage according VCPU count.
"and it automatically limits ..."
>
> 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@gmail.com>
> ---
> src/lxc/lxc_cgroup.c | 31 ++++++++++++++++
> src/lxc/lxc_container.c | 15 ++++++++
> src/lxc/lxc_fuse.c | 78 ++++++++++++++++++++++++++++++++++++++---
> 3 files changed, 120 insertions(+), 4 deletions(-)
>
> diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
> index 470337e675..a6c73d9d55 100644
> --- a/src/lxc/lxc_cgroup.c
> +++ b/src/lxc/lxc_cgroup.c
> @@ -59,6 +59,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 (virCgroupSetCpusetCpus(cgroup,
> + virBufferCurrentContent(cpuset)) < 0) {
> + virBufferFreeAndReset(cpuset);
> + return -1;
> + }
> +
> + virBufferFreeAndReset(cpuset);
> + return 0;
> +}
> +
> +
> static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def,
> virCgroupPtr cgroup,
> virBitmapPtr nodemask)
> @@ -76,6 +104,9 @@ static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def,
> goto cleanup;
> /* free mask to make sure we won't use it in a wrong way later */
> VIR_FREE(mask);
> + } else {
> + /* auto mode for VCPU limits */
> + virLXCCgroupSetupVcpuAuto(def, cgroup);
> }
>
> if (virDomainNumatuneGetMode(def->numa, -1, &mode) < 0 ||
> diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
> index 41efe43a14..1a2c97c9f4 100644
> --- a/src/lxc/lxc_container.c
> +++ b/src/lxc/lxc_container.c
> @@ -999,6 +999,7 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def,
> {
> int ret;
> char *meminfo_path = NULL;
> + char *cpuinfo_path = NULL;
You can use g_autofree on cpuinfo_path to avoid the need for VIR_FREE() in the
end.
>
> VIR_DEBUG("Mount /proc/meminfo stateDir=%s", stateDir);
>
> @@ -1013,7 +1014,21 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def,
> meminfo_path);
> }
>
> + VIR_DEBUG("Mount /proc/cpuinfo stateDir=%s", stateDir);
> +
> + cpuinfo_path = g_strdup_printf("/.oldroot/%s/%s.fuse/cpuinfo",
> + stateDir,
> + def->name);
> +
> + if ((ret = mount(cpuinfo_path, "/proc/cpuinfo",
> + NULL, MS_BIND, NULL)) < 0) {
> + virReportSystemError(errno,
> + _("Failed to mount %s on /proc/cpuinfo"),
> + cpuinfo_path);
> + }
> +
> VIR_FREE(meminfo_path);
> + VIR_FREE(cpuinfo_path);
> return ret;
> }
> #else
> diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c
> index 44f240a0b5..12fa69d494 100644
> --- a/src/lxc/lxc_fuse.c
> +++ b/src/lxc/lxc_fuse.c
> @@ -37,6 +37,7 @@
> #if WITH_FUSE
>
> static const char *fuse_meminfo_path = "/meminfo";
> +static const char *fuse_cpuinfo_path = "/cpuinfo";
>
> static int lxcProcGetattr(const char *path, struct stat *stbuf)
> {
> @@ -54,7 +55,8 @@ static int lxcProcGetattr(const char *path, struct stat *stbuf)
> if (STREQ(path, "/")) {
> stbuf->st_mode = S_IFDIR | 0755;
> stbuf->st_nlink = 2;
> - } else if (STREQ(path, fuse_meminfo_path)) {
> + } else if (STREQ(path, fuse_meminfo_path) ||
> + STREQ(path, fuse_cpuinfo_path)) {
> if (stat(mempath, &sb) < 0) {
> res = -errno;
> goto cleanup;
> @@ -90,6 +92,7 @@ static int lxcProcReaddir(const char *path, void *buf,
> filler(buf, ".", NULL, 0);
> filler(buf, "..", NULL, 0);
> filler(buf, fuse_meminfo_path + 1, NULL, 0);
> + filler(buf, fuse_cpuinfo_path + 1, NULL, 0);
>
> return 0;
> }
> @@ -97,7 +100,8 @@ static int lxcProcReaddir(const char *path, void *buf,
> static int lxcProcOpen(const char *path G_GNUC_UNUSED,
> struct fuse_file_info *fi G_GNUC_UNUSED)
> {
> - if (STRNEQ(path, fuse_meminfo_path))
> + if (STRNEQ(path, fuse_meminfo_path) &&
> + STRNEQ(path, fuse_cpuinfo_path))
> return -ENOENT;
>
> if ((fi->flags & 3) != O_RDONLY)
> @@ -125,7 +129,7 @@ static int lxcProcHostRead(char *path, char *buf, size_t size, off_t offset)
> static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
> char *buf, size_t size, off_t offset)
> {
> - int res;
> + int res = -1;
> FILE *fd = NULL;
> char *line = NULL;
> size_t n;
> @@ -151,7 +155,6 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
> goto cleanup;
> }
>
> - res = -1;
> while (getline(&line, &n, fd) > 0) {
> char *ptr = strchr(line, ':');
> if (!ptr)
> @@ -235,6 +238,70 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
> return res;
> }
>
> +
> +static int lxcProcReadCpuinfo(char *hostpath, virDomainDefPtr def,
> + char *buf, size_t size, off_t offset)
> +{
> + int res = -1;
> + FILE *fd = NULL;
I believe that instead of creating the fd as FILE you can use
VIR_AUTOCLOSE fd = -1;
And it will auto close the fd for you when context is gone. Then you won't
need VIR_FORCE_FCLOSE().
> + char *line = NULL;
'line' can use g_autofree as well.
> + size_t n;
> + virBuffer buffer = VIR_BUFFER_INITIALIZER;
> + virBufferPtr new_cpuinfo = &buffer;
> + size_t cpu;
> + size_t nvcpu;
> + size_t curcpu = 0;
> + bool get_proc = false;
> +
> + fd = fopen(hostpath, "r");
Suggestion: fopen() here is fine, but I believe that you can use utilities like
virFileReadAll() as well. It will give you a buffer with the file contents so
you don't need to rely on sscanf() and getline() in the loop below.
> + if (fd == NULL) {
> + virReportSystemError(errno, _("Cannot open %s"), hostpath);
> + res = -errno;
> + goto cleanup;
> + }
> +
> + /* /proc/cpuinfo does not support fseek */
> + if (offset > 0) {
> + res = 0;
> + goto cleanup;
> + }
> +
> + nvcpu = virDomainDefGetVcpus(def);
> + while (getline(&line, &n, fd) > 0) {
> + if (sscanf(line, "processor\t: %zu", &cpu) == 1) {
> + virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(def, cpu);
> + /* VCPU is mapped */
> + if (vcpu) {
> + if (curcpu == nvcpu)
> + break;
> +
> + 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)
> + virBufferAdd(new_cpuinfo, line, -1);
> + }
> + }
> +
> + res = strlen(virBufferCurrentContent(new_cpuinfo));
> + if (res > size)
> + res = size;
> + memcpy(buf, virBufferCurrentContent(new_cpuinfo), res);
> +
> + cleanup:
> + VIR_FREE(line);
> + virBufferFreeAndReset(new_cpuinfo);
> + VIR_FORCE_FCLOSE(fd);
> + return res;
> +}
> +
> +
> static int lxcProcRead(const char *path G_GNUC_UNUSED,
> char *buf G_GNUC_UNUSED,
> size_t size G_GNUC_UNUSED,
> @@ -254,6 +321,9 @@ static int lxcProcRead(const char *path G_GNUC_UNUSED,
> if (STREQ(path, fuse_meminfo_path)) {
> if ((res = lxcProcReadMeminfo(hostpath, def, buf, size, offset)) < 0)
> res = lxcProcHostRead(hostpath, buf, size, offset);
> + } else if (STREQ(path, fuse_cpuinfo_path)) {
> + if ((res = lxcProcReadCpuinfo(hostpath, def, buf, size, offset)) < 0)
> + res = lxcProcHostRead(hostpath, buf, size, offset);
> }
>
> VIR_FREE(hostpath);
>
Em qua., 19 de fev. de 2020 às 09:27, Daniel Henrique Barboza
<danielhb413@gmail.com> escreveu:
>
>
>
> On 2/16/20 2:11 PM, Julio Faracco wrote:
> > This commit tries to fix a lots of issues related to LXC VCPUs. One of
>
> Extra 'a' there. "tries to fix lots of issues ..."
>
>
> > 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`
>
> This parentesis is never closed.
>
> > attribute is declared. So, this commit adds a virtual cpuinfo based on
> > VCPU mapping and it automatic limits the CPU usage according VCPU count.
>
> "and it automatically limits ..."
>
> >
> > 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@gmail.com>
> > ---
> > src/lxc/lxc_cgroup.c | 31 ++++++++++++++++
> > src/lxc/lxc_container.c | 15 ++++++++
> > src/lxc/lxc_fuse.c | 78 ++++++++++++++++++++++++++++++++++++++---
> > 3 files changed, 120 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c
> > index 470337e675..a6c73d9d55 100644
> > --- a/src/lxc/lxc_cgroup.c
> > +++ b/src/lxc/lxc_cgroup.c
> > @@ -59,6 +59,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 (virCgroupSetCpusetCpus(cgroup,
> > + virBufferCurrentContent(cpuset)) < 0) {
> > + virBufferFreeAndReset(cpuset);
> > + return -1;
> > + }
> > +
> > + virBufferFreeAndReset(cpuset);
> > + return 0;
> > +}
> > +
> > +
> > static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def,
> > virCgroupPtr cgroup,
> > virBitmapPtr nodemask)
> > @@ -76,6 +104,9 @@ static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def,
> > goto cleanup;
> > /* free mask to make sure we won't use it in a wrong way later */
> > VIR_FREE(mask);
> > + } else {
> > + /* auto mode for VCPU limits */
> > + virLXCCgroupSetupVcpuAuto(def, cgroup);
> > }
> >
> > if (virDomainNumatuneGetMode(def->numa, -1, &mode) < 0 ||
> > diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
> > index 41efe43a14..1a2c97c9f4 100644
> > --- a/src/lxc/lxc_container.c
> > +++ b/src/lxc/lxc_container.c
> > @@ -999,6 +999,7 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def,
> > {
> > int ret;
> > char *meminfo_path = NULL;
> > + char *cpuinfo_path = NULL;
>
>
> You can use g_autofree on cpuinfo_path to avoid the need for VIR_FREE() in the
> end.
>
> >
> > VIR_DEBUG("Mount /proc/meminfo stateDir=%s", stateDir);
> >
> > @@ -1013,7 +1014,21 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def,
> > meminfo_path);
> > }
> >
> > + VIR_DEBUG("Mount /proc/cpuinfo stateDir=%s", stateDir);
> > +
> > + cpuinfo_path = g_strdup_printf("/.oldroot/%s/%s.fuse/cpuinfo",
> > + stateDir,
> > + def->name);
> > +
> > + if ((ret = mount(cpuinfo_path, "/proc/cpuinfo",
> > + NULL, MS_BIND, NULL)) < 0) {
> > + virReportSystemError(errno,
> > + _("Failed to mount %s on /proc/cpuinfo"),
> > + cpuinfo_path);
> > + }
> > +
> > VIR_FREE(meminfo_path);
> > + VIR_FREE(cpuinfo_path);
> > return ret;
> > }
> > #else
> > diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c
> > index 44f240a0b5..12fa69d494 100644
> > --- a/src/lxc/lxc_fuse.c
> > +++ b/src/lxc/lxc_fuse.c
> > @@ -37,6 +37,7 @@
> > #if WITH_FUSE
> >
> > static const char *fuse_meminfo_path = "/meminfo";
> > +static const char *fuse_cpuinfo_path = "/cpuinfo";
> >
> > static int lxcProcGetattr(const char *path, struct stat *stbuf)
> > {
> > @@ -54,7 +55,8 @@ static int lxcProcGetattr(const char *path, struct stat *stbuf)
> > if (STREQ(path, "/")) {
> > stbuf->st_mode = S_IFDIR | 0755;
> > stbuf->st_nlink = 2;
> > - } else if (STREQ(path, fuse_meminfo_path)) {
> > + } else if (STREQ(path, fuse_meminfo_path) ||
> > + STREQ(path, fuse_cpuinfo_path)) {
> > if (stat(mempath, &sb) < 0) {
> > res = -errno;
> > goto cleanup;
> > @@ -90,6 +92,7 @@ static int lxcProcReaddir(const char *path, void *buf,
> > filler(buf, ".", NULL, 0);
> > filler(buf, "..", NULL, 0);
> > filler(buf, fuse_meminfo_path + 1, NULL, 0);
> > + filler(buf, fuse_cpuinfo_path + 1, NULL, 0);
> >
> > return 0;
> > }
> > @@ -97,7 +100,8 @@ static int lxcProcReaddir(const char *path, void *buf,
> > static int lxcProcOpen(const char *path G_GNUC_UNUSED,
> > struct fuse_file_info *fi G_GNUC_UNUSED)
> > {
> > - if (STRNEQ(path, fuse_meminfo_path))
> > + if (STRNEQ(path, fuse_meminfo_path) &&
> > + STRNEQ(path, fuse_cpuinfo_path))
> > return -ENOENT;
> >
> > if ((fi->flags & 3) != O_RDONLY)
> > @@ -125,7 +129,7 @@ static int lxcProcHostRead(char *path, char *buf, size_t size, off_t offset)
> > static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
> > char *buf, size_t size, off_t offset)
> > {
> > - int res;
> > + int res = -1;
> > FILE *fd = NULL;
> > char *line = NULL;
> > size_t n;
> > @@ -151,7 +155,6 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
> > goto cleanup;
> > }
> >
> > - res = -1;
> > while (getline(&line, &n, fd) > 0) {
> > char *ptr = strchr(line, ':');
> > if (!ptr)
> > @@ -235,6 +238,70 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
> > return res;
> > }
> >
> > +
> > +static int lxcProcReadCpuinfo(char *hostpath, virDomainDefPtr def,
> > + char *buf, size_t size, off_t offset)
> > +{
> > + int res = -1;
> > + FILE *fd = NULL;
>
>
> I believe that instead of creating the fd as FILE you can use
>
> VIR_AUTOCLOSE fd = -1;
>
>
> And it will auto close the fd for you when context is gone. Then you won't
> need VIR_FORCE_FCLOSE().
>
>
> > + char *line = NULL;
>
>
> 'line' can use g_autofree as well.
>
>
> > + size_t n;
> > + virBuffer buffer = VIR_BUFFER_INITIALIZER;
> > + virBufferPtr new_cpuinfo = &buffer;
> > + size_t cpu;
> > + size_t nvcpu;
> > + size_t curcpu = 0;
> > + bool get_proc = false;
> > +
> > + fd = fopen(hostpath, "r");
>
>
> Suggestion: fopen() here is fine, but I believe that you can use utilities like
> virFileReadAll() as well. It will give you a buffer with the file contents so
> you don't need to rely on sscanf() and getline() in the loop below.
Hi Daniel,
Thanks for your review. I totally agree with you.
I just kept this fopen() (and other functions) to keep the same style
of that driver.
I will resubmit this series with your suggestions and I will include
some cleanups also.
I think it is better.
This comment is related to other reviewed patches from this series too.
>
>
> > + if (fd == NULL) {
> > + virReportSystemError(errno, _("Cannot open %s"), hostpath);
> > + res = -errno;
> > + goto cleanup;
> > + }
> > +
> > + /* /proc/cpuinfo does not support fseek */
> > + if (offset > 0) {
> > + res = 0;
> > + goto cleanup;
> > + }
> > +
> > + nvcpu = virDomainDefGetVcpus(def);
> > + while (getline(&line, &n, fd) > 0) {
> > + if (sscanf(line, "processor\t: %zu", &cpu) == 1) {
> > + virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(def, cpu);
> > + /* VCPU is mapped */
> > + if (vcpu) {
> > + if (curcpu == nvcpu)
> > + break;
> > +
> > + 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)
> > + virBufferAdd(new_cpuinfo, line, -1);
> > + }
> > + }
> > +
> > + res = strlen(virBufferCurrentContent(new_cpuinfo));
> > + if (res > size)
> > + res = size;
> > + memcpy(buf, virBufferCurrentContent(new_cpuinfo), res);
> > +
> > + cleanup:
> > + VIR_FREE(line);
> > + virBufferFreeAndReset(new_cpuinfo);
> > + VIR_FORCE_FCLOSE(fd);
> > + return res;
> > +}
> > +
> > +
> > static int lxcProcRead(const char *path G_GNUC_UNUSED,
> > char *buf G_GNUC_UNUSED,
> > size_t size G_GNUC_UNUSED,
> > @@ -254,6 +321,9 @@ static int lxcProcRead(const char *path G_GNUC_UNUSED,
> > if (STREQ(path, fuse_meminfo_path)) {
> > if ((res = lxcProcReadMeminfo(hostpath, def, buf, size, offset)) < 0)
> > res = lxcProcHostRead(hostpath, buf, size, offset);
> > + } else if (STREQ(path, fuse_cpuinfo_path)) {
> > + if ((res = lxcProcReadCpuinfo(hostpath, def, buf, size, offset)) < 0)
> > + res = lxcProcHostRead(hostpath, buf, size, offset);
> > }
> >
> > VIR_FREE(hostpath);
> >
© 2016 - 2026 Red Hat, Inc.