Fix lxc /proc/meminfo virtualization (bz #1300781)

Fix 'permission denied' errors trying to unlink disk images (bz #1289327)
Fix qemu:///session connect race failures (bz #1271183)
driver: log missing modules as INFO, not WARN (bz #1274849)
This commit is contained in:
Cole Robinson 2016-03-17 17:34:43 -04:00
parent a29c57e351
commit ef2d4d8159
10 changed files with 513 additions and 1 deletions

View File

@ -0,0 +1,42 @@
From: John Ferlan <jferlan@redhat.com>
Date: Wed, 30 Sep 2015 17:37:27 -0400
Subject: [PATCH] virfile: Fix error path for forked virFileRemove
As it turns out the caller in this case expects a return < 0 for failure
and to get/use "errno" rather than using the negative of returned status.
Again different than the create path.
If someone "deleted" a file from the pool without using virsh vol-delete,
then the unlink/rmdir would return an error (-1) and set errno to ENOENT.
The caller checks errno for ENOENT when determining whether to throw an
error message indicating the failure. Without the change, the error
message is:
error: Failed to delete vol $vol
error: cannot unlink file '/$pathto/$vol': Success
This patch thus allows the fork path to follow the non-fork path
where unlink/rmdir return -1 and errno.
(cherry picked from commit cb19cff468432e55366014658f405066ce06c2f2)
---
src/util/virfile.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 7b14ee8..d742645 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2381,9 +2381,10 @@ virFileUnlink(const char *path,
path, msg);
VIR_FREE(msg);
if (WIFEXITED(status))
- ret = -WEXITSTATUS(status);
+ errno = WEXITSTATUS(status);
else
- ret = -EACCES;
+ errno = EACCES;
+ ret = -errno;
}
parenterror:

View File

@ -0,0 +1,146 @@
From: Cole Robinson <crobinso@redhat.com>
Date: Thu, 21 Jan 2016 13:06:03 -0500
Subject: [PATCH] lxc: fuse: Unindent meminfo logic
Reverse the conditional at the start so we aren't stuffing all the logic
in an 'if' block
(cherry picked from commit f65dcfcd140d0a627aeab3fa0e9dc5f74da98e6d)
---
src/lxc/lxc_fuse.c | 122 ++++++++++++++++++++++++++---------------------------
1 file changed, 61 insertions(+), 61 deletions(-)
diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c
index 34a69cc..f9c02c0 100644
--- a/src/lxc/lxc_fuse.c
+++ b/src/lxc/lxc_fuse.c
@@ -161,68 +161,68 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
res = -1;
while (copied < size && getline(&line, &n, fd) > 0) {
char *ptr = strchr(line, ':');
- if (ptr) {
- *ptr = '\0';
-
- if (STREQ(line, "MemTotal") &&
- (virMemoryLimitIsSet(def->mem.hard_limit) ||
- virDomainDefGetMemoryActual(def))) {
- virBufferAsprintf(new_meminfo, "MemTotal: %8llu kB\n",
- meminfo.memtotal);
- } else if (STREQ(line, "MemFree") &&
- (virMemoryLimitIsSet(def->mem.hard_limit) ||
- virDomainDefGetMemoryActual(def))) {
- virBufferAsprintf(new_meminfo, "MemFree: %8llu kB\n",
- (meminfo.memtotal - meminfo.memusage));
- } else if (STREQ(line, "Buffers")) {
- virBufferAsprintf(new_meminfo, "Buffers: %8d kB\n", 0);
- } else if (STREQ(line, "Cached")) {
- virBufferAsprintf(new_meminfo, "Cached: %8llu kB\n",
- meminfo.cached);
- } else if (STREQ(line, "Active")) {
- virBufferAsprintf(new_meminfo, "Active: %8llu kB\n",
- (meminfo.active_anon + meminfo.active_file));
- } else if (STREQ(line, "Inactive")) {
- virBufferAsprintf(new_meminfo, "Inactive: %8llu kB\n",
- (meminfo.inactive_anon + meminfo.inactive_file));
- } else if (STREQ(line, "Active(anon)")) {
- virBufferAsprintf(new_meminfo, "Active(anon): %8llu kB\n",
- meminfo.active_anon);
- } else if (STREQ(line, "Inactive(anon)")) {
- virBufferAsprintf(new_meminfo, "Inactive(anon): %8llu kB\n",
- meminfo.inactive_anon);
- } else if (STREQ(line, "Active(file)")) {
- virBufferAsprintf(new_meminfo, "Active(file): %8llu kB\n",
- meminfo.active_file);
- } else if (STREQ(line, "Inactive(file)")) {
- virBufferAsprintf(new_meminfo, "Inactive(file): %8llu kB\n",
- meminfo.inactive_file);
- } else if (STREQ(line, "Unevictable")) {
- virBufferAsprintf(new_meminfo, "Unevictable: %8llu kB\n",
- meminfo.unevictable);
- } else if (STREQ(line, "SwapTotal") &&
- virMemoryLimitIsSet(def->mem.swap_hard_limit)) {
- virBufferAsprintf(new_meminfo, "SwapTotal: %8llu kB\n",
- (meminfo.swaptotal - meminfo.memtotal));
- } else if (STREQ(line, "SwapFree") &&
- virMemoryLimitIsSet(def->mem.swap_hard_limit)) {
- virBufferAsprintf(new_meminfo, "SwapFree: %8llu kB\n",
- (meminfo.swaptotal - meminfo.memtotal -
- meminfo.swapusage + meminfo.memusage));
- } else {
- *ptr = ':';
- virBufferAdd(new_meminfo, line, -1);
- }
-
- if (virBufferCheckError(new_meminfo) < 0) {
- res = -errno;
- goto cleanup;
- }
-
- copied += strlen(line);
- if (copied > size)
- copied = size;
+ if (!ptr)
+ continue;
+ *ptr = '\0';
+
+ if (STREQ(line, "MemTotal") &&
+ (virMemoryLimitIsSet(def->mem.hard_limit) ||
+ virDomainDefGetMemoryActual(def))) {
+ virBufferAsprintf(new_meminfo, "MemTotal: %8llu kB\n",
+ meminfo.memtotal);
+ } else if (STREQ(line, "MemFree") &&
+ (virMemoryLimitIsSet(def->mem.hard_limit) ||
+ virDomainDefGetMemoryActual(def))) {
+ virBufferAsprintf(new_meminfo, "MemFree: %8llu kB\n",
+ (meminfo.memtotal - meminfo.memusage));
+ } else if (STREQ(line, "Buffers")) {
+ virBufferAsprintf(new_meminfo, "Buffers: %8d kB\n", 0);
+ } else if (STREQ(line, "Cached")) {
+ virBufferAsprintf(new_meminfo, "Cached: %8llu kB\n",
+ meminfo.cached);
+ } else if (STREQ(line, "Active")) {
+ virBufferAsprintf(new_meminfo, "Active: %8llu kB\n",
+ (meminfo.active_anon + meminfo.active_file));
+ } else if (STREQ(line, "Inactive")) {
+ virBufferAsprintf(new_meminfo, "Inactive: %8llu kB\n",
+ (meminfo.inactive_anon + meminfo.inactive_file));
+ } else if (STREQ(line, "Active(anon)")) {
+ virBufferAsprintf(new_meminfo, "Active(anon): %8llu kB\n",
+ meminfo.active_anon);
+ } else if (STREQ(line, "Inactive(anon)")) {
+ virBufferAsprintf(new_meminfo, "Inactive(anon): %8llu kB\n",
+ meminfo.inactive_anon);
+ } else if (STREQ(line, "Active(file)")) {
+ virBufferAsprintf(new_meminfo, "Active(file): %8llu kB\n",
+ meminfo.active_file);
+ } else if (STREQ(line, "Inactive(file)")) {
+ virBufferAsprintf(new_meminfo, "Inactive(file): %8llu kB\n",
+ meminfo.inactive_file);
+ } else if (STREQ(line, "Unevictable")) {
+ virBufferAsprintf(new_meminfo, "Unevictable: %8llu kB\n",
+ meminfo.unevictable);
+ } else if (STREQ(line, "SwapTotal") &&
+ virMemoryLimitIsSet(def->mem.swap_hard_limit)) {
+ virBufferAsprintf(new_meminfo, "SwapTotal: %8llu kB\n",
+ (meminfo.swaptotal - meminfo.memtotal));
+ } else if (STREQ(line, "SwapFree") &&
+ virMemoryLimitIsSet(def->mem.swap_hard_limit)) {
+ virBufferAsprintf(new_meminfo, "SwapFree: %8llu kB\n",
+ (meminfo.swaptotal - meminfo.memtotal -
+ meminfo.swapusage + meminfo.memusage));
+ } else {
+ *ptr = ':';
+ virBufferAdd(new_meminfo, line, -1);
}
+
+ if (virBufferCheckError(new_meminfo) < 0) {
+ res = -errno;
+ goto cleanup;
+ }
+
+ copied += strlen(line);
+ if (copied > size)
+ copied = size;
}
res = copied;
memcpy(buf, virBufferCurrentContent(new_meminfo), copied);

View File

@ -0,0 +1,55 @@
From: Cole Robinson <crobinso@redhat.com>
Date: Thu, 21 Jan 2016 13:14:54 -0500
Subject: [PATCH] lxc: fuse: Fix /proc/meminfo size calculation
We virtualize bits of /proc/meminfo by replacing host values with
values specific to the container.
However for calculating the final size of the returned data, we are
using the size of the original file and not the altered copy, which
could give garbelled output.
(cherry picked from commit 8418245a7e00f873594f1000c9606d08265088e0)
---
src/lxc/lxc_fuse.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c
index f9c02c0..e6369f8 100644
--- a/src/lxc/lxc_fuse.c
+++ b/src/lxc/lxc_fuse.c
@@ -131,7 +131,6 @@ 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 copied = 0;
int res;
FILE *fd = NULL;
char *line = NULL;
@@ -159,7 +158,7 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
}
res = -1;
- while (copied < size && getline(&line, &n, fd) > 0) {
+ while (getline(&line, &n, fd) > 0) {
char *ptr = strchr(line, ':');
if (!ptr)
continue;
@@ -219,13 +218,11 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
res = -errno;
goto cleanup;
}
-
- copied += strlen(line);
- if (copied > size)
- copied = size;
}
- res = copied;
- memcpy(buf, virBufferCurrentContent(new_meminfo), copied);
+ res = strlen(virBufferCurrentContent(new_meminfo));
+ if (res > size)
+ res = size;
+ memcpy(buf, virBufferCurrentContent(new_meminfo), res);
cleanup:
VIR_FREE(line);

View File

@ -0,0 +1,34 @@
From: Cole Robinson <crobinso@redhat.com>
Date: Thu, 21 Jan 2016 13:18:04 -0500
Subject: [PATCH] lxc: fuse: Fill in MemAvailable for /proc/meminfo
'free' on Fedora 23 will use MemAvailable to calculate its 'available'
field, but we are passing through the host's value. Set it to match
MemFree, which is what 'free' will do for older linux that don't have
MemAvailable
https://bugzilla.redhat.com/show_bug.cgi?id=1300781
(cherry picked from commit c7be484d1136834614089c9a74a3818594852f24)
---
src/lxc/lxc_fuse.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c
index e6369f8..691ddee 100644
--- a/src/lxc/lxc_fuse.c
+++ b/src/lxc/lxc_fuse.c
@@ -174,6 +174,14 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
virDomainDefGetMemoryActual(def))) {
virBufferAsprintf(new_meminfo, "MemFree: %8llu kB\n",
(meminfo.memtotal - meminfo.memusage));
+ } else if (STREQ(line, "MemAvailable") &&
+ (virMemoryLimitIsSet(def->mem.hard_limit) ||
+ virDomainDefGetMemoryActual(def))) {
+ /* MemAvailable is actually MemFree + SRReclaimable +
+ some other bits, but MemFree is the closest approximation
+ we have */
+ virBufferAsprintf(new_meminfo, "MemAvailable: %8llu kB\n",
+ (meminfo.memtotal - meminfo.memusage));
} else if (STREQ(line, "Buffers")) {
virBufferAsprintf(new_meminfo, "Buffers: %8d kB\n", 0);
} else if (STREQ(line, "Cached")) {

View File

@ -0,0 +1,35 @@
From: Cole Robinson <crobinso@redhat.com>
Date: Thu, 21 Jan 2016 13:33:50 -0500
Subject: [PATCH] lxc: fuse: Stub out Slab bits in /proc/meminfo
'free' on fedora23 wants to use the Slab field for calculated used
memory. The equation is:
used = MemTotal - MemFree - (Cached + Slab) - Buffers
We already set Cached and Buffers to 0, do the same for Slab and its
related values
https://bugzilla.redhat.com/show_bug.cgi?id=1300781
(cherry picked from commit 81da8bc73b6bc6777632b65a0df45335f7caebe4)
---
src/lxc/lxc_fuse.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c
index 691ddee..8c20a7d 100644
--- a/src/lxc/lxc_fuse.c
+++ b/src/lxc/lxc_fuse.c
@@ -217,6 +217,12 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
virBufferAsprintf(new_meminfo, "SwapFree: %8llu kB\n",
(meminfo.swaptotal - meminfo.memtotal -
meminfo.swapusage + meminfo.memusage));
+ } else if (STREQ(line, "Slab")) {
+ virBufferAsprintf(new_meminfo, "Slab: %8d kB\n", 0);
+ } else if (STREQ(line, "SReclaimable")) {
+ virBufferAsprintf(new_meminfo, "SReclaimable: %8d kB\n", 0);
+ } else if (STREQ(line, "SUnreclaim")) {
+ virBufferAsprintf(new_meminfo, "SUnreclaim: %8d kB\n", 0);
} else {
*ptr = ':';
virBufferAdd(new_meminfo, line, -1);

View File

@ -0,0 +1,62 @@
From: Cole Robinson <crobinso@redhat.com>
Date: Wed, 9 Mar 2016 10:53:54 -0500
Subject: [PATCH] util: virfile: Clarify setuid usage for virFileRemove
Break these checks out into their own function, and clearly document
each one. This shouldn't change behavior
(cherry picked from commit 7cf5343709935694b76af7b134447a2c555400b6)
---
src/util/virfile.c | 32 +++++++++++++++++++++++++++-----
1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index d742645..45bb249 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2307,6 +2307,32 @@ virFileOpenAs(const char *path, int openflags, mode_t mode,
}
+/* virFileRemoveNeedsSetuid:
+ * @uid: file uid to check
+ * @gid: file gid to check
+ *
+ * Return true if we should use setuid/setgid before deleting a file
+ * owned by the passed uid/gid pair. Needed for NFS with root-squash
+ */
+static bool
+virFileRemoveNeedsSetuid(uid_t uid, gid_t gid)
+{
+ /* If running unprivileged, setuid isn't going to work */
+ if (geteuid() != 0)
+ return false;
+
+ /* uid/gid weren't specified */
+ if ((uid == (uid_t) -1) && (gid == (gid_t) -1))
+ return false;
+
+ /* already running as proper uid/gid */
+ if (uid == geteuid() && gid == getegid())
+ return false;
+
+ return true;
+}
+
+
/* virFileUnlink:
* @path: file to unlink
* @uid: uid that was used to create the file (not required)
@@ -2329,11 +2355,7 @@ virFileUnlink(const char *path,
gid_t *groups;
int ngroups;
- /* If not running as root or if a non explicit uid/gid was being used for
- * the file/volume, then use unlink directly
- */
- if ((geteuid() != 0) ||
- ((uid == (uid_t) -1) && (gid == (gid_t) -1)))
+ if (!virFileRemoveNeedsSetuid(uid, gid))
return unlink(path);
/* Otherwise, we have to deal with the NFS root-squash craziness

View File

@ -0,0 +1,55 @@
From: Cole Robinson <crobinso@redhat.com>
Date: Wed, 9 Mar 2016 12:20:37 -0500
Subject: [PATCH] util: virfile: Only setuid for virFileRemove if on NFS
NFS with root-squash is the only reason we need to do setuid/setgid
crazyness in virFileRemove, so limit that behavior to the NFS case.
(cherry picked from commit adefc561cc4c6a007529769c3df286f2ed461684)
---
src/util/virfile.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 45bb249..1dc6601 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2308,6 +2308,7 @@ virFileOpenAs(const char *path, int openflags, mode_t mode,
/* virFileRemoveNeedsSetuid:
+ * @path: file we plan to remove
* @uid: file uid to check
* @gid: file gid to check
*
@@ -2315,7 +2316,7 @@ virFileOpenAs(const char *path, int openflags, mode_t mode,
* owned by the passed uid/gid pair. Needed for NFS with root-squash
*/
static bool
-virFileRemoveNeedsSetuid(uid_t uid, gid_t gid)
+virFileRemoveNeedsSetuid(const char *path, uid_t uid, gid_t gid)
{
/* If running unprivileged, setuid isn't going to work */
if (geteuid() != 0)
@@ -2329,6 +2330,12 @@ virFileRemoveNeedsSetuid(uid_t uid, gid_t gid)
if (uid == geteuid() && gid == getegid())
return false;
+ /* Only perform the setuid stuff for NFS, which is the only case
+ that may actually need it. This can error, but just be safe and
+ only check for a clear negative result. */
+ if (virFileIsSharedFSType(path, VIR_FILE_SHFS_NFS) == 0)
+ return false;
+
return true;
}
@@ -2355,7 +2362,7 @@ virFileUnlink(const char *path,
gid_t *groups;
int ngroups;
- if (!virFileRemoveNeedsSetuid(uid, gid))
+ if (!virFileRemoveNeedsSetuid(path, uid, gid))
return unlink(path);
/* Otherwise, we have to deal with the NFS root-squash craziness

View File

@ -0,0 +1,37 @@
From: Cole Robinson <crobinso@redhat.com>
Date: Tue, 15 Mar 2016 17:04:32 -0400
Subject: [PATCH] rpc: wait longer for session daemon to start up
https://bugzilla.redhat.com/show_bug.cgi?id=1271183
We only wait 0.5 seconds for the session daemon to start up and present
its socket, which isn't sufficient for many users. Bump up the sleep
interval and retry amount so we wait for a total of 5.0 seconds.
(cherry picked from commit ca0c06f4008154de55e0b3109885facd0bf02d32)
---
src/rpc/virnetsocket.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index b132532..ecedcf5 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -614,7 +614,7 @@ int virNetSocketNewConnectUNIX(const char *path,
char *lockpath = NULL;
int lockfd = -1;
int fd = -1;
- int retries = 100;
+ int retries = 500;
virSocketAddr localAddr;
virSocketAddr remoteAddr;
char *rundir = NULL;
@@ -707,7 +707,7 @@ int virNetSocketNewConnectUNIX(const char *path,
daemonLaunched = true;
}
- usleep(5000);
+ usleep(10000);
}
localAddr.len = sizeof(localAddr.data);

View File

@ -0,0 +1,27 @@
From: Jovanka Gulicoska <jovanka.gulicoska@gmail.com>
Date: Thu, 17 Mar 2016 20:02:20 +0100
Subject: [PATCH] driver: log missing modules as INFO, not WARN
Missing modules is a common expected scenario for most libvirt usage on
RPM distributions like Fedora, so it doesn't really warrant logging at
WARN level. Use INFO instead
https://bugzilla.redhat.com/show_bug.cgi?id=1274849
(cherry picked from commit 9a0c7f5f834185db9017c34aabc03ad99cf37bed)
---
src/driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/driver.c b/src/driver.c
index 2985538..1514a3b 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -62,7 +62,7 @@ virDriverLoadModule(const char *name)
return NULL;
if (access(modfile, R_OK) < 0) {
- VIR_WARN("Module %s not accessible", modfile);
+ VIR_INFO("Module %s not accessible", modfile);
goto cleanup;
}

View File

@ -378,7 +378,7 @@
Summary: Library providing a simple virtualization API
Name: libvirt
Version: 1.2.18.2
Release: 2%{?dist}%{?extra_release}
Release: 3%{?dist}%{?extra_release}
License: LGPLv2+
Group: Development/Libraries
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root
@ -403,6 +403,19 @@ Patch0005: 0005-rpc-socket-Explicitly-error-if-we-exceed-retry-count.patch
Patch0006: 0006-rpc-socket-Don-t-repeatedly-attempt-to-launch-daemon.patch
# Fix parallel VM start/top svirt errors on kernel/initrd (bz #1269975)
Patch0007: 0007-security-Do-not-restore-kernel-and-initrd-labels.patch
# Fix lxc /proc/meminfo virtualization (bz #1300781)
Patch0008: 0008-virfile-Fix-error-path-for-forked-virFileRemove.patch
Patch0009: 0009-lxc-fuse-Unindent-meminfo-logic.patch
Patch0010: 0010-lxc-fuse-Fix-proc-meminfo-size-calculation.patch
Patch0011: 0011-lxc-fuse-Fill-in-MemAvailable-for-proc-meminfo.patch
Patch0012: 0012-lxc-fuse-Stub-out-Slab-bits-in-proc-meminfo.patch
# Fix 'permission denied' errors trying to unlink disk images (bz #1289327)
Patch0013: 0013-util-virfile-Clarify-setuid-usage-for-virFileRemove.patch
Patch0014: 0014-util-virfile-Only-setuid-for-virFileRemove-if-on-NFS.patch
# Fix qemu:///session connect race failures (bz #1271183)
Patch0015: 0015-rpc-wait-longer-for-session-daemon-to-start-up.patch
# driver: log missing modules as INFO, not WARN (bz #1274849)
Patch0016: 0016-driver-log-missing-modules-as-INFO-not-WARN.patch
%if %{with_libvirtd}
Requires: libvirt-daemon = %{version}-%{release}
@ -2346,6 +2359,12 @@ exit 0
%doc examples/systemtap
%changelog
* Thu Mar 17 2016 Cole Robinson <crobinso@redhat.com> - 1.2.18.2-3
- Fix lxc /proc/meminfo virtualization (bz #1300781)
- Fix 'permission denied' errors trying to unlink disk images (bz #1289327)
- Fix qemu:///session connect race failures (bz #1271183)
- driver: log missing modules as INFO, not WARN (bz #1274849)
* Wed Jan 20 2016 Cole Robinson <crobinso@redhat.com> - 1.2.18.2-2
- Fix XML validation with qemu commandline passthrough (bz #1292131)
- Fix crash in libvirt_leasehelper (bz #1202350)