[libvirt] [PATCH 0/2] Fix timerActive logic in virNetDaemon
Nowadays the daemon can quit even if there's a client connected, so let's fix that. If ACK'd, I'll backport this to v1.2.17-maint as well as v1.2.17 is the only affected release. Martin Kletzander (2): rpc: Add virNetDaemonHasClients rpc: Rework timerActive logic in daemon src/libvirt_remote.syms | 1 + src/rpc/virnetdaemon.c | 37 ++++++++++++++++++++++--------------- src/rpc/virnetdaemon.h | 2 ++ 3 files changed, 25 insertions(+), 15 deletions(-) -- 2.4.5
So callers don't have to iterate over each server. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt_remote.syms | 1 + src/rpc/virnetdaemon.c | 13 +++++++++++++ src/rpc/virnetdaemon.h | 2 ++ 3 files changed, 16 insertions(+) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index e6ca041ea8e1..6bfdcfa819bf 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -65,6 +65,7 @@ virNetDaemonAddSignalHandler; virNetDaemonAutoShutdown; virNetDaemonClose; virNetDaemonGetServer; +virNetDaemonHasClients; virNetDaemonIsPrivileged; virNetDaemonNew; virNetDaemonNewPostExecRestart; diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 67dff147be60..6b132823274c 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -747,3 +747,16 @@ virNetDaemonClose(virNetDaemonPtr dmn) virObjectUnlock(dmn); } + +bool +virNetDaemonHasClients(virNetDaemonPtr dmn) +{ + size_t i = 0; + + for (i = 0; i < dmn->nservers; i++) { + if (virNetServerHasClients(dmn->servers[i])) + return true; + } + + return false; +} diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h index 9e176d65ca6a..bb320539a887 100644 --- a/src/rpc/virnetdaemon.h +++ b/src/rpc/virnetdaemon.h @@ -76,6 +76,8 @@ void virNetDaemonQuit(virNetDaemonPtr dmn); void virNetDaemonClose(virNetDaemonPtr dmn); +bool virNetDaemonHasClients(virNetDaemonPtr dmn); + virNetServerPtr virNetDaemonGetServer(virNetDaemonPtr dmn, int subServerID); -- 2.4.5
Daemon used false logic for determining whether there were any clients. When the timer was inactive, it was activated if at least one of the servers did not have clients. So the bool was being flipped there and back all the time in case there was one client, for example. Initially introduced by fa1420736882. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1240283 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/virnetdaemon.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 6b132823274c..19573c5f4c42 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -682,23 +682,17 @@ virNetDaemonRun(virNetDaemonPtr dmn) */ if (dmn->autoShutdownTimeout) { if (timerActive) { - for (i = 0; i < dmn->nservers; i++) { - if (virNetServerHasClients(dmn->servers[i])) { - VIR_DEBUG("Deactivating shutdown timer %d", timerid); - virEventUpdateTimeout(timerid, -1); - timerActive = false; - break; - } + if (virNetDaemonHasClients(dmn)) { + VIR_DEBUG("Deactivating shutdown timer %d", timerid); + virEventUpdateTimeout(timerid, -1); + timerActive = false; } } else { - for (i = 0; i < dmn->nservers; i++) { - if (!virNetServerHasClients(dmn->servers[i])) { - VIR_DEBUG("Activating shutdown timer %d", timerid); - virEventUpdateTimeout(timerid, - dmn->autoShutdownTimeout * 1000); - timerActive = true; - break; - } + if (virNetDaemonHasClients(dmn)) { + VIR_DEBUG("Activating shutdown timer %d", timerid); + virEventUpdateTimeout(timerid, + dmn->autoShutdownTimeout * 1000); + timerActive = true; } } } -- 2.4.5
On 10.07.2015 10:49, Martin Kletzander wrote:
Daemon used false logic for determining whether there were any clients. When the timer was inactive, it was activated if at least one of the servers did not have clients. So the bool was being flipped there and back all the time in case there was one client, for example.
Initially introduced by fa1420736882.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1240283
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/rpc/virnetdaemon.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 6b132823274c..19573c5f4c42 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -682,23 +682,17 @@ virNetDaemonRun(virNetDaemonPtr dmn) */ if (dmn->autoShutdownTimeout) { if (timerActive) { - for (i = 0; i < dmn->nservers; i++) { - if (virNetServerHasClients(dmn->servers[i])) { - VIR_DEBUG("Deactivating shutdown timer %d", timerid); - virEventUpdateTimeout(timerid, -1); - timerActive = false; - break; - } + if (virNetDaemonHasClients(dmn)) { + VIR_DEBUG("Deactivating shutdown timer %d", timerid); + virEventUpdateTimeout(timerid, -1); + timerActive = false; } } else { - for (i = 0; i < dmn->nservers; i++) { - if (!virNetServerHasClients(dmn->servers[i])) { - VIR_DEBUG("Activating shutdown timer %d", timerid); - virEventUpdateTimeout(timerid, - dmn->autoShutdownTimeout * 1000); - timerActive = true; - break; - } + if (virNetDaemonHasClients(dmn)) {
I believe this condition needs to be negated.
+ VIR_DEBUG("Activating shutdown timer %d", timerid); + virEventUpdateTimeout(timerid, + dmn->autoShutdownTimeout * 1000); + timerActive = true; } } }
Good catch! ACK series. Michal
participants (2)
-
Martin Kletzander -
Michal Privoznik