From d6cc78be6696eababb249be7ae73baff96ee5ea2 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Mar 17 2016 21:38:30 +0000 Subject: Fix qemu:///session disconnect after 30 seconds 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) --- diff --git a/0001-daemon-Properly-check-for-clients.patch b/0001-daemon-Properly-check-for-clients.patch new file mode 100644 index 0000000..a7fdd18 --- /dev/null +++ b/0001-daemon-Properly-check-for-clients.patch @@ -0,0 +1,46 @@ +From: Martin Kletzander +Date: Tue, 1 Mar 2016 15:42:32 +0100 +Subject: [PATCH] daemon: Properly check for clients + +virHashForEach() returns 0 if everything went nice, so our session +daemon was timing out even when there was a client connected. + +Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1315606 + +Signed-off-by: Martin Kletzander +(cherry picked from commit 6541a2b4acd453ebbf10a4427f9ec4e794d3ba6d) +--- + src/rpc/virnetdaemon.c | 14 +++++++++++--- + 1 file changed, 11 insertions(+), 3 deletions(-) + +diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c +index 298fbf4..b05ba99 100644 +--- a/src/rpc/virnetdaemon.c ++++ b/src/rpc/virnetdaemon.c +@@ -843,15 +843,23 @@ virNetDaemonClose(virNetDaemonPtr dmn) + static int + daemonServerHasClients(void *payload, + const void *key ATTRIBUTE_UNUSED, +- void *opaque ATTRIBUTE_UNUSED) ++ void *opaque) + { ++ bool *clients = opaque; + virNetServerPtr srv = payload; + +- return virNetServerHasClients(srv); ++ if (virNetServerHasClients(srv)) ++ *clients = true; ++ ++ return 0; + } + + bool + virNetDaemonHasClients(virNetDaemonPtr dmn) + { +- return virHashForEach(dmn->servers, daemonServerHasClients, NULL) > 0; ++ bool ret = false; ++ ++ virHashForEach(dmn->servers, daemonServerHasClients, &ret); ++ ++ return ret; + } diff --git a/0001-daemon-properly-check-for-clients.patch b/0001-daemon-properly-check-for-clients.patch deleted file mode 100644 index 1fef5db..0000000 --- a/0001-daemon-properly-check-for-clients.patch +++ /dev/null @@ -1,45 +0,0 @@ -From 2608c9a951caed5ebec7ad36571c5adb6671b855 Mon Sep 17 00:00:00 2001 -From: Martin Kletzander -Date: Tue, 1 Mar 2016 15:42:32 +0100 -Subject: [PATCH] daemon: properly check for clients - -Signed-off-by: Martin Kletzander -(cherry picked from commit 78b0ccc71e99f769068974ff56638c99b1c3b4de) ---- - src/rpc/virnetdaemon.c | 14 +++++++++++--- - 1 file changed, 11 insertions(+), 3 deletions(-) - -diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c -index 298fbf4..b05ba99 100644 ---- a/src/rpc/virnetdaemon.c -+++ b/src/rpc/virnetdaemon.c -@@ -843,15 +843,23 @@ virNetDaemonClose(virNetDaemonPtr dmn) - static int - daemonServerHasClients(void *payload, - const void *key ATTRIBUTE_UNUSED, -- void *opaque ATTRIBUTE_UNUSED) -+ void *opaque) - { -+ bool *clients = opaque; - virNetServerPtr srv = payload; - -- return virNetServerHasClients(srv); -+ if (virNetServerHasClients(srv)) -+ *clients = true; -+ -+ return 0; - } - - bool - virNetDaemonHasClients(virNetDaemonPtr dmn) - { -- return virHashForEach(dmn->servers, daemonServerHasClients, NULL) > 0; -+ bool ret = false; -+ -+ virHashForEach(dmn->servers, daemonServerHasClients, &ret); -+ -+ return ret; - } --- -2.7.2 - diff --git a/0002-util-virfile-Clarify-setuid-usage-for-virFileRemove.patch b/0002-util-virfile-Clarify-setuid-usage-for-virFileRemove.patch new file mode 100644 index 0000000..3c93877 --- /dev/null +++ b/0002-util-virfile-Clarify-setuid-usage-for-virFileRemove.patch @@ -0,0 +1,63 @@ +From: Cole Robinson +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 | 33 +++++++++++++++++++++++++++------ + 1 file changed, 27 insertions(+), 6 deletions(-) + +diff --git a/src/util/virfile.c b/src/util/virfile.c +index f45e18f..a913903 100644 +--- a/src/util/virfile.c ++++ b/src/util/virfile.c +@@ -2314,6 +2314,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; ++} ++ ++ + /* virFileRemove: + * @path: file to unlink or directory to remove + * @uid: uid that was used to create the file (not required) +@@ -2335,12 +2361,7 @@ virFileRemove(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 or the explicit uid/gid matches, then use unlink directly +- */ +- if ((geteuid() != 0) || +- ((uid == (uid_t) -1) && (gid == (gid_t) -1)) || +- (uid == geteuid() && gid == getegid())) { ++ if (!virFileRemoveNeedsSetuid(uid, gid)) { + if (virFileIsDir(path)) + return rmdir(path); + else diff --git a/0003-util-virfile-Only-setuid-for-virFileRemove-if-on-NFS.patch b/0003-util-virfile-Only-setuid-for-virFileRemove-if-on-NFS.patch new file mode 100644 index 0000000..c5c0e93 --- /dev/null +++ b/0003-util-virfile-Only-setuid-for-virFileRemove-if-on-NFS.patch @@ -0,0 +1,55 @@ +From: Cole Robinson +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 a913903..0bba850 100644 +--- a/src/util/virfile.c ++++ b/src/util/virfile.c +@@ -2315,6 +2315,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 + * +@@ -2322,7 +2323,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) +@@ -2336,6 +2337,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; + } + +@@ -2361,7 +2368,7 @@ virFileRemove(const char *path, + gid_t *groups; + int ngroups; + +- if (!virFileRemoveNeedsSetuid(uid, gid)) { ++ if (!virFileRemoveNeedsSetuid(path, uid, gid)) { + if (virFileIsDir(path)) + return rmdir(path); + else diff --git a/0004-rpc-wait-longer-for-session-daemon-to-start-up.patch b/0004-rpc-wait-longer-for-session-daemon-to-start-up.patch new file mode 100644 index 0000000..4473845 --- /dev/null +++ b/0004-rpc-wait-longer-for-session-daemon-to-start-up.patch @@ -0,0 +1,37 @@ +From: Cole Robinson +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 b0d5b1c..d909b94 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); diff --git a/0005-driver-log-missing-modules-as-INFO-not-WARN.patch b/0005-driver-log-missing-modules-as-INFO-not-WARN.patch new file mode 100644 index 0000000..8c7d53e --- /dev/null +++ b/0005-driver-log-missing-modules-as-INFO-not-WARN.patch @@ -0,0 +1,27 @@ +From: Jovanka Gulicoska +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; + } + diff --git a/libvirt.spec b/libvirt.spec index 032ffd6..d0d6fda 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -378,7 +378,7 @@ Summary: Library providing a simple virtualization API Name: libvirt Version: 1.3.2 -Release: 2%{?dist}%{?extra_release} +Release: 3%{?dist}%{?extra_release} License: LGPLv2+ Group: Development/Libraries BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root @@ -389,11 +389,15 @@ URL: http://libvirt.org/ %endif Source: http://libvirt.org/sources/%{?mainturl}libvirt-%{version}.tar.gz -# NON-UPSTREAM patch which fixes regression where libvirt -# drops connections after > 30 seconds. -# https://bugzilla.redhat.com/1315606 -# https://github.com/nertpinx/libvirt/commit/78b0ccc71e99f769068974ff56638c99b1c3b4de -Patch0001: 0001-daemon-properly-check-for-clients.patch +# Fix qemu:///session disconnect after 30 seconds +Patch0001: 0001-daemon-Properly-check-for-clients.patch +# Fix 'permission denied' errors trying to unlink disk images (bz #1289327) +Patch0002: 0002-util-virfile-Clarify-setuid-usage-for-virFileRemove.patch +Patch0003: 0003-util-virfile-Only-setuid-for-virFileRemove-if-on-NFS.patch +# Fix qemu:///session connect race failures (bz #1271183) +Patch0004: 0004-rpc-wait-longer-for-session-daemon-to-start-up.patch +# driver: log missing modules as INFO, not WARN (bz #1274849) +Patch0005: 0005-driver-log-missing-modules-as-INFO-not-WARN.patch %if %{with_libvirtd} Requires: libvirt-daemon = %{version}-%{release} @@ -2383,6 +2387,12 @@ exit 0 %doc examples/systemtap %changelog +* Thu Mar 17 2016 Cole Robinson - 1.3.2-3 +- Fix qemu:///session disconnect after 30 seconds +- 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 Mar 9 2016 Richard W.M. Jones - 1.3.2-2 - Add fix for RHBZ#1315606.