11:23 a.m.
RFC:
https://www.redhat.com/archives/libvir-list/2018-January/msg00318.html
Adjustments since RFC...
Patches 1&2: No change, were already R-B'd
Patch 3: Removed code as noted in code review, update commit message
Patch 4: From old series removed, see below for more details
Patch 9: no change
NB: Patches 5-8 and 10 from Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
are removed as they seemed to not be necessary
Replaced the former patch 4 with series of patches to (slowly) provide
support to disable new connections, handle removing waiting jobs, causing
the waiting workers to quit, and allow any running jobs to complete.
As it turns out, waiting for running jobs to complete cannot be done
from the virNetServerClose callbacks because that means the event loop
processing done during virNetServerRun will not allow any currently
long running worker job thread a means to complete.
So when virNetDaemonQuit is called as a result of the libvirtd signal
handlers for SIG{QUIT|INT|TERM}, instead of just causing virNetServerRun
to quit immediately, alter to using a quitRequested flag and then use
that quitRequested flag to check for long running worker threads before
causing the event loop to quit resulting in libvirtd being able to run
through the virNetDaemonClose processing.
John Ferlan (9):
libvirtd: Alter refcnt processing for domain server objects
libvirtd: Alter refcnt processing for server program objects
netserver: Remove ServiceToggle during ServerDispose
util: Introduce virThreadPoolDrain
rpc: Introduce virNetServerQuitRequested
rpc: Introduce virNetServerWorkerCount
rpc: Alter virNetDaemonQuit processing
docs: Add news article for libvirtd issue
APPLY ONLY FOR TESTING PURPOSES
daemon/libvirtd.c | 43 +++++++++++++++++++++++---------
docs/news.xml | 12 +++++++++
src/libvirt_private.syms | 1 +
src/libvirt_remote.syms | 2 ++
src/qemu/qemu_driver.c | 5 ++++
src/rpc/virnetdaemon.c | 45 +++++++++++++++++++++++++++++++++-
src/rpc/virnetserver.c | 52 ++++++++++++++++++++++++++++++++++++---
src/rpc/virnetserver.h | 4 +++
src/util/virthreadpool.c | 64 ++++++++++++++++++++++++++++++++++++++++--------
src/util/virthreadpool.h | 2 ++
10 files changed, 204 insertions(+), 26 deletions(-)
--
2.13.6
11:23 a.m.
New subject: [libvirt] [PATCH 1/9] libvirtd: Alter refcnt processing for domain server objects
Once virNetDaemonAddServer returns, the @srv or @srvAdm have
either been added to the @dmn list increasing the refcnt or
there was an error. In either case, we can lower the refcnt
from virNetServerNew, but not set to NULL. Thus "using" the
hash table reference when adding programs later or allowing
the immediate free of the object on error.
The @dmn dispose function (virNetDaemonDispose) will handle
the Unref of each object during the virHashFree when the
object is removed from the hash table.
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
daemon/libvirtd.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 6d3b83355..0afd1dd82 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1065,6 +1065,7 @@ int main(int argc, char **argv) {
char *remote_config_file = NULL;
int statuswrite = -1;
int ret = 1;
+ int rc;
int pid_file_fd = -1;
char *pid_file = NULL;
char *sock_file = NULL;
@@ -1319,7 +1320,11 @@ int main(int argc, char **argv) {
goto cleanup;
}
- if (virNetDaemonAddServer(dmn, srv) < 0) {
+ /* Add @srv to @dmn servers hash table and Unref to allow removal from
+ * hash table at @dmn disposal to decide when to free @srv */
+ rc = virNetDaemonAddServer(dmn, srv);
+ virObjectUnref(srv);
+ if (rc < 0) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
@@ -1393,7 +1398,11 @@ int main(int argc, char **argv) {
goto cleanup;
}
- if (virNetDaemonAddServer(dmn, srvAdm) < 0) {
+ /* Add @srvAdm to @dmn servers hash table and Unref to allow removal from
+ * hash table at @dmn disposal to decide when to free @srvAdm */
+ rc = virNetDaemonAddServer(dmn, srvAdm);
+ virObjectUnref(srvAdm);
+ if (rc < 0) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
@@ -1516,11 +1525,9 @@ int main(int argc, char **argv) {
}
virObjectUnref(adminProgram);
- virObjectUnref(srvAdm);
virObjectUnref(qemuProgram);
virObjectUnref(lxcProgram);
virObjectUnref(remoteProgram);
- virObjectUnref(srv);
virObjectUnref(dmn);
virNetlinkShutdown();
--
2.13.6
3 a.m.
New subject: [libvirt] [PATCH 1/9] libvirtd: Alter refcnt processing for domain server objects
On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan <jferlan(a)redhat.com> wrote:
Once virNetDaemonAddServer returns, the @srv or @srvAdm have
either been added to the @dmn list increasing the refcnt or
there was an error. In either case, we can lower the refcnt
from virNetServerNew, but not set to NULL. Thus "using" the
hash table reference when adding programs later or allowing
the immediate free of the object on error.
The @dmn dispose function (virNetDaemonDispose) will handle
the Unref of each object during the virHashFree when the
object is removed from the hash table.
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
daemon/libvirtd.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 6d3b83355..0afd1dd82 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1065,6 +1065,7 @@ int main(int argc, char **argv) {
char *remote_config_file = NULL;
int statuswrite = -1;
int ret = 1;
+ int rc;
int pid_file_fd = -1;
char *pid_file = NULL;
char *sock_file = NULL;
@@ -1319,7 +1320,11 @@ int main(int argc, char **argv) {
goto cleanup;
}
- if (virNetDaemonAddServer(dmn, srv) < 0) {
+ /* Add @srv to @dmn servers hash table and Unref to allow removal from
+ * hash table at @dmn disposal to decide when to free @srv */
+ rc = virNetDaemonAddServer(dmn, srv);
Reviewed-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
Wouldn’t it be cleaner to initialize the server and then add it to the
daemon? (disadvantage: it makes the correct unreferencing a little more
complicated).
<pseudo_code>
create_and_initialize_daemon
create_and_add_programs_server
virNetDaemonAddServer()
…
</pseudo_code>
+ virObjectUnref(srv);
+ if (rc < 0) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
@@ -1393,7 +1398,11 @@ int main(int argc, char **argv) {
goto cleanup;
}
- if (virNetDaemonAddServer(dmn, srvAdm) < 0) {
+ /* Add @srvAdm to @dmn servers hash table and Unref to allow removal from
+ * hash table at @dmn disposal to decide when to free @srvAdm */
+ rc = virNetDaemonAddServer(dmn, srvAdm);
+ virObjectUnref(srvAdm);
+ if (rc < 0) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
@@ -1516,11 +1525,9 @@ int main(int argc, char **argv) {
}
virObjectUnref(adminProgram);
- virObjectUnref(srvAdm);
virObjectUnref(qemuProgram);
virObjectUnref(lxcProgram);
virObjectUnref(remoteProgram);
- virObjectUnref(srv);
virObjectUnref(dmn);
virNetlinkShutdown();
--
2.13.6
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
11:23 a.m.
New subject: [libvirt] [PATCH 2/9] libvirtd: Alter refcnt processing for server program objects
Once virNetServerProgramNew returns, the program objects have
either been added to the @srv or @srvAdm list increasing the
refcnt or there was an error. In either case, we can lower the
refcnt from virNetServerProgramNew and set the object to NULL
since it won't be used any more.
The @srv or @srvAdm dispose function (virNetServerDispose) will
handle the Unref of each object during the virHashFree when the
object is removed from the hash table.
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
daemon/libvirtd.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 0afd1dd82..b47f875d9 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1352,7 +1352,11 @@ int main(int argc, char **argv) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
- if (virNetServerAddProgram(srv, remoteProgram) < 0) {
+
+ rc = virNetServerAddProgram(srv, remoteProgram);
+ virObjectUnref(remoteProgram);
+ remoteProgram = NULL;
+ if (rc < 0) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
@@ -1364,7 +1368,11 @@ int main(int argc, char **argv) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
- if (virNetServerAddProgram(srv, lxcProgram) < 0) {
+
+ rc = virNetServerAddProgram(srv, lxcProgram);
+ virObjectUnref(lxcProgram);
+ lxcProgram = NULL;
+ if (rc < 0) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
@@ -1376,7 +1384,11 @@ int main(int argc, char **argv) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
- if (virNetServerAddProgram(srv, qemuProgram) < 0) {
+
+ rc = virNetServerAddProgram(srv, qemuProgram);
+ virObjectUnref(qemuProgram);
+ qemuProgram = NULL;
+ if (rc < 0) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
@@ -1414,7 +1426,11 @@ int main(int argc, char **argv) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
- if (virNetServerAddProgram(srvAdm, adminProgram) < 0) {
+
+ rc = virNetServerAddProgram(srvAdm, adminProgram);
+ virObjectUnref(adminProgram);
+ adminProgram = NULL;
+ if (rc < 0) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
@@ -1524,10 +1540,6 @@ int main(int argc, char **argv) {
virStateCleanup();
}
- virObjectUnref(adminProgram);
- virObjectUnref(qemuProgram);
- virObjectUnref(lxcProgram);
- virObjectUnref(remoteProgram);
virObjectUnref(dmn);
virNetlinkShutdown();
--
2.13.6
3:01 a.m.
New subject: [libvirt] [PATCH 2/9] libvirtd: Alter refcnt processing for server program objects
On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan <jferlan(a)redhat.com> wrote:
Once virNetServerProgramNew returns, the program objects have
either been added to the @srv or @srvAdm list increasing the
refcnt or there was an error. In either case, we can lower the
refcnt from virNetServerProgramNew and set the object to NULL
since it won't be used any more.
The @srv or @srvAdm dispose function (virNetServerDispose) will
handle the Unref of each object during the virHashFree when the
object is removed from the hash table.
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
daemon/libvirtd.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
Reviewed-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
4:12 a.m.
New subject: [libvirt] [PATCH 2/9] libvirtd: Alter refcnt processing for server program objects
On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan <jferlan(a)redhat.com> wrote:
Once virNetServerProgramNew returns, the program objects have
either been added to the @srv or @srvAdm list increasing the
refcnt or there was an error. In either case, we can lower the
refcnt from virNetServerProgramNew and set the object to NULL
since it won't be used any more.
The @srv or @srvAdm dispose function (virNetServerDispose) will
handle the Unref of each object during the virHashFree when the
object is removed from the hash table.
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
daemon/libvirtd.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 0afd1dd82..b47f875d9 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1352,7 +1352,11 @@ int main(int argc, char **argv) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
- if (virNetServerAddProgram(srv, remoteProgram) < 0) {
+
+ rc = virNetServerAddProgram(srv, remoteProgram);
+ virObjectUnref(remoteProgram);
+ remoteProgram = NULL;
I’ve just looked at the code and all those variables are a global
variables…
libvirtd.c:
virNetServerProgramPtr remoteProgram = NULL;
virNetServerProgramPtr adminProgram = NULL;
virNetServerProgramPtr qemuProgram = NULL;
virNetServerProgramPtr lxcProgram = NULL;
So this is not a good idea to set them to NULL… As this results in a
segmentation fault
[Current thread is 1 (Thread 0x3ff908d7910 (LWP 195236))]
(gdb) bt
#0 virNetServerProgramGetID (prog=prog@entry=0x0) at
../../src/rpc/virnetserverprogram.c:93
#1 0x000000011b726ac2 in remoteDispatchObjectEventSend (client=<optimized out>,
program=0x0, procnr=procnr@entry=319, proc=0x11b767120
<xdr_remote_domain_event_callback_reboot_msg>, data=data@entry=0x3fffee7d948) at
../../daemon/remote.c:3997
#2 0x000000011b7577d2 in remoteRelayDomainEventReboot (conn=<optimized out>,
dom=0x143663320, opaque=0x3ff700330c0) at ../../daemon/remote.c:365
#3 0x000003ff904173a8 in virDomainEventDispatchDefaultFunc (conn=0x3fef42ca860,
event=0x1436a37b0, cb=0x11b7576d0 <remoteRelayDomainEventReboot>,
cbopaque=0x3ff700330c0) at ../../src/conf/domain_event.c:1787
#4 0x000003ff90414e4e in virObjectEventStateDispatchCallbacks
(state=state@entry=0x3ff2c0facf0, event=0x1436a37b0,
callbacks=callbacks@entry=0x3ff2c106370) at ../../src/conf/object_event.c:715
#5 0x000003ff90414eb4 in virObjectEventStateQueueDispatch
(state=state@entry=0x3ff2c0facf0, queue=queue@entry=0x3fffee7dc10,
callbacks=0x3ff2c106370) at ../../src/conf/object_event.c:729
#6 0x000003ff904156d4 in virObjectEventStateFlush (state=0x3ff2c0facf0) at
../../src/conf/object_event.c:830
#7 0x000003ff9041574e in virObjectEventTimer (timer=<optimized out>,
opaque=<optimized out>) at ../../src/conf/object_event.c:560
#8 0x000003ff90343cc4 in virEventPollDispatchTimeouts () at
../../src/util/vireventpoll.c:457
#9 0x000003ff9034577c in virEventPollRunOnce () at ../../src/util/vireventpoll.c:653
#10 0x000003ff90343852 in virEventRunDefaultImpl () at ../../src/util/virevent.c:327
#11 0x000003ff904e5e78 in virNetDaemonRun (dmn=0x1436303b0) at
../../src/rpc/virnetdaemon.c:880
#12 0x000000011b7256ca in main (argc=<optimized out>, argv=<optimized out>) at
../../daemon/libvirtd.c:1523
Therefore, I’ll withdraw my Reviewed-by: Marc Hartmayer
<mhartmay(a)linux.vnet.ibm.com>.
+ if (rc < 0) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
@@ -1364,7 +1368,11 @@ int main(int argc, char **argv) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
- if (virNetServerAddProgram(srv, lxcProgram) < 0) {
+
+ rc = virNetServerAddProgram(srv, lxcProgram);
+ virObjectUnref(lxcProgram);
+ lxcProgram = NULL;
+ if (rc < 0) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
@@ -1376,7 +1384,11 @@ int main(int argc, char **argv) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
- if (virNetServerAddProgram(srv, qemuProgram) < 0) {
+
+ rc = virNetServerAddProgram(srv, qemuProgram);
+ virObjectUnref(qemuProgram);
+ qemuProgram = NULL;
+ if (rc < 0) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
@@ -1414,7 +1426,11 @@ int main(int argc, char **argv) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
- if (virNetServerAddProgram(srvAdm, adminProgram) < 0) {
+
+ rc = virNetServerAddProgram(srvAdm, adminProgram);
+ virObjectUnref(adminProgram);
+ adminProgram = NULL;
+ if (rc < 0) {
ret = VIR_DAEMON_ERR_INIT;
goto cleanup;
}
@@ -1524,10 +1540,6 @@ int main(int argc, char **argv) {
virStateCleanup();
}
- virObjectUnref(adminProgram);
- virObjectUnref(qemuProgram);
- virObjectUnref(lxcProgram);
- virObjectUnref(remoteProgram);
virObjectUnref(dmn);
virNetlinkShutdown();
--
2.13.6
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
6:03 a.m.
New subject: [libvirt] [PATCH 2/9] libvirtd: Alter refcnt processing for server program objects
On 01/24/2018 05:12 AM, Marc Hartmayer wrote:
On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan
<jferlan(a)redhat.com> wrote:
> Once virNetServerProgramNew returns, the program objects have
> either been added to the @srv or @srvAdm list increasing the
> refcnt or there was an error. In either case, we can lower the
> refcnt from virNetServerProgramNew and set the object to NULL
> since it won't be used any more.
>
> The @srv or @srvAdm dispose function (virNetServerDispose) will
> handle the Unref of each object during the virHashFree when the
> object is removed from the hash table.
>
> Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> daemon/libvirtd.c | 28 ++++++++++++++++++++--------
> 1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index 0afd1dd82..b47f875d9 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -1352,7 +1352,11 @@ int main(int argc, char **argv) {
> ret = VIR_DAEMON_ERR_INIT;
> goto cleanup;
> }
> - if (virNetServerAddProgram(srv, remoteProgram) < 0) {
> +
> + rc = virNetServerAddProgram(srv, remoteProgram);
> + virObjectUnref(remoteProgram);
> + remoteProgram = NULL;
I’ve just looked at the code and all those variables are a global
variables…
libvirtd.c:
virNetServerProgramPtr remoteProgram = NULL;
virNetServerProgramPtr adminProgram = NULL;
virNetServerProgramPtr qemuProgram = NULL;
virNetServerProgramPtr lxcProgram = NULL;
So this is not a good idea to set them to NULL… As this results in a
segmentation fault
Oh right... There really was no reason to set them to NULL since the
only time they're used is in the context they are used. If the
AddProgram was successful, then there would be 2 refs anyway. They're
not used later in the code, so I'll remove the = NULL after the Unref.
John
[Current thread is 1 (Thread 0x3ff908d7910 (LWP 195236))]
(gdb) bt
#0 virNetServerProgramGetID (prog=prog@entry=0x0) at
../../src/rpc/virnetserverprogram.c:93
#1 0x000000011b726ac2 in remoteDispatchObjectEventSend (client=<optimized out>,
program=0x0, procnr=procnr@entry=319, proc=0x11b767120
<xdr_remote_domain_event_callback_reboot_msg>, data=data@entry=0x3fffee7d948) at
../../daemon/remote.c:3997
#2 0x000000011b7577d2 in remoteRelayDomainEventReboot (conn=<optimized out>,
dom=0x143663320, opaque=0x3ff700330c0) at ../../daemon/remote.c:365
#3 0x000003ff904173a8 in virDomainEventDispatchDefaultFunc (conn=0x3fef42ca860,
event=0x1436a37b0, cb=0x11b7576d0 <remoteRelayDomainEventReboot>,
cbopaque=0x3ff700330c0) at ../../src/conf/domain_event.c:1787
#4 0x000003ff90414e4e in virObjectEventStateDispatchCallbacks
(state=state@entry=0x3ff2c0facf0, event=0x1436a37b0,
callbacks=callbacks@entry=0x3ff2c106370) at ../../src/conf/object_event.c:715
#5 0x000003ff90414eb4 in virObjectEventStateQueueDispatch
(state=state@entry=0x3ff2c0facf0, queue=queue@entry=0x3fffee7dc10,
callbacks=0x3ff2c106370) at ../../src/conf/object_event.c:729
#6 0x000003ff904156d4 in virObjectEventStateFlush (state=0x3ff2c0facf0) at
../../src/conf/object_event.c:830
#7 0x000003ff9041574e in virObjectEventTimer (timer=<optimized out>,
opaque=<optimized out>) at ../../src/conf/object_event.c:560
#8 0x000003ff90343cc4 in virEventPollDispatchTimeouts () at
../../src/util/vireventpoll.c:457
#9 0x000003ff9034577c in virEventPollRunOnce () at ../../src/util/vireventpoll.c:653
#10 0x000003ff90343852 in virEventRunDefaultImpl () at ../../src/util/virevent.c:327
#11 0x000003ff904e5e78 in virNetDaemonRun (dmn=0x1436303b0) at
../../src/rpc/virnetdaemon.c:880
#12 0x000000011b7256ca in main (argc=<optimized out>, argv=<optimized out>)
at ../../daemon/libvirtd.c:1523
Therefore, I’ll withdraw my Reviewed-by: Marc Hartmayer
<mhartmay(a)linux.vnet.ibm.com>.
> + if (rc < 0) {
> ret = VIR_DAEMON_ERR_INIT;
> goto cleanup;
> }
> @@ -1364,7 +1368,11 @@ int main(int argc, char **argv) {
> ret = VIR_DAEMON_ERR_INIT;
> goto cleanup;
> }
> - if (virNetServerAddProgram(srv, lxcProgram) < 0) {
> +
> + rc = virNetServerAddProgram(srv, lxcProgram);
> + virObjectUnref(lxcProgram);
> + lxcProgram = NULL;
> + if (rc < 0) {
> ret = VIR_DAEMON_ERR_INIT;
> goto cleanup;
> }
> @@ -1376,7 +1384,11 @@ int main(int argc, char **argv) {
> ret = VIR_DAEMON_ERR_INIT;
> goto cleanup;
> }
> - if (virNetServerAddProgram(srv, qemuProgram) < 0) {
> +
> + rc = virNetServerAddProgram(srv, qemuProgram);
> + virObjectUnref(qemuProgram);
> + qemuProgram = NULL;
> + if (rc < 0) {
> ret = VIR_DAEMON_ERR_INIT;
> goto cleanup;
> }
> @@ -1414,7 +1426,11 @@ int main(int argc, char **argv) {
> ret = VIR_DAEMON_ERR_INIT;
> goto cleanup;
> }
> - if (virNetServerAddProgram(srvAdm, adminProgram) < 0) {
> +
> + rc = virNetServerAddProgram(srvAdm, adminProgram);
> + virObjectUnref(adminProgram);
> + adminProgram = NULL;
> + if (rc < 0) {
> ret = VIR_DAEMON_ERR_INIT;
> goto cleanup;
> }
> @@ -1524,10 +1540,6 @@ int main(int argc, char **argv) {
> virStateCleanup();
> }
>
> - virObjectUnref(adminProgram);
> - virObjectUnref(qemuProgram);
> - virObjectUnref(lxcProgram);
> - virObjectUnref(remoteProgram);
> virObjectUnref(dmn);
>
> virNetlinkShutdown();
> --
> 2.13.6
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
6:48 a.m.
New subject: [libvirt] [PATCH 2/9] libvirtd: Alter refcnt processing for server program objects
On Wed, Jan 24, 2018 at 01:03 PM +0100, John Ferlan <jferlan(a)redhat.com> wrote:
On 01/24/2018 05:12 AM, Marc Hartmayer wrote:
> On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan <jferlan(a)redhat.com>
wrote:
>> Once virNetServerProgramNew returns, the program objects have
>> either been added to the @srv or @srvAdm list increasing the
>> refcnt or there was an error. In either case, we can lower the
>> refcnt from virNetServerProgramNew and set the object to NULL
>> since it won't be used any more.
>>
>> The @srv or @srvAdm dispose function (virNetServerDispose) will
>> handle the Unref of each object during the virHashFree when the
>> object is removed from the hash table.
>>
>> Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
>> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
>> ---
>> daemon/libvirtd.c | 28 ++++++++++++++++++++--------
>> 1 file changed, 20 insertions(+), 8 deletions(-)
>>
>> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
>> index 0afd1dd82..b47f875d9 100644
>> --- a/daemon/libvirtd.c
>> +++ b/daemon/libvirtd.c
>> @@ -1352,7 +1352,11 @@ int main(int argc, char **argv) {
>> ret = VIR_DAEMON_ERR_INIT;
>> goto cleanup;
>> }
>> - if (virNetServerAddProgram(srv, remoteProgram) < 0) {
>> +
>> + rc = virNetServerAddProgram(srv, remoteProgram);
>> + virObjectUnref(remoteProgram);
>> + remoteProgram = NULL;
>
>
> I’ve just looked at the code and all those variables are a global
> variables…
>
> libvirtd.c:
> virNetServerProgramPtr remoteProgram = NULL;
> virNetServerProgramPtr adminProgram = NULL;
> virNetServerProgramPtr qemuProgram = NULL;
> virNetServerProgramPtr lxcProgram = NULL;
>
> So this is not a good idea to set them to NULL… As this results in a
> segmentation fault
Oh right... There really was no reason to set them to NULL since the
only time they're used is in the context they are used. If the
AddProgram was successful, then there would be 2 refs anyway. They're
not used later in the code, so I'll remove the = NULL after the Unref.
Is it possible to get rid of these global variables? At least for
adminProgram and lxcProgram this should be easily to achieve as they
aren’t used anywhere else than in this function context.
libvirtd.h: (Only these variables are declared as extern)
extern virNetServerProgramPtr remoteProgram;
extern virNetServerProgramPtr qemuProgram;
John
>
[…snip…]
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
1:21 p.m.
New subject: [libvirt] [PATCH 2/9] libvirtd: Alter refcnt processing for server program objects
On 01/24/2018 07:48 AM, Marc Hartmayer wrote:
On Wed, Jan 24, 2018 at 01:03 PM +0100, John Ferlan
<jferlan(a)redhat.com> wrote:
> On 01/24/2018 05:12 AM, Marc Hartmayer wrote:
>> On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan <jferlan(a)redhat.com>
wrote:
>>> Once virNetServerProgramNew returns, the program objects have
>>> either been added to the @srv or @srvAdm list increasing the
>>> refcnt or there was an error. In either case, we can lower the
>>> refcnt from virNetServerProgramNew and set the object to NULL
>>> since it won't be used any more.
>>>
>>> The @srv or @srvAdm dispose function (virNetServerDispose) will
>>> handle the Unref of each object during the virHashFree when the
>>> object is removed from the hash table.
>>>
>>> Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
>>> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
>>> ---
>>> daemon/libvirtd.c | 28 ++++++++++++++++++++--------
>>> 1 file changed, 20 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
>>> index 0afd1dd82..b47f875d9 100644
>>> --- a/daemon/libvirtd.c
>>> +++ b/daemon/libvirtd.c
>>> @@ -1352,7 +1352,11 @@ int main(int argc, char **argv) {
>>> ret = VIR_DAEMON_ERR_INIT;
>>> goto cleanup;
>>> }
>>> - if (virNetServerAddProgram(srv, remoteProgram) < 0) {
>>> +
>>> + rc = virNetServerAddProgram(srv, remoteProgram);
>>> + virObjectUnref(remoteProgram);
>>> + remoteProgram = NULL;
>>
>>
>> I’ve just looked at the code and all those variables are a global
>> variables…
>>
>> libvirtd.c:
>> virNetServerProgramPtr remoteProgram = NULL;
>> virNetServerProgramPtr adminProgram = NULL;
>> virNetServerProgramPtr qemuProgram = NULL;
>> virNetServerProgramPtr lxcProgram = NULL;
>>
>> So this is not a good idea to set them to NULL… As this results in a
>> segmentation fault
>
> Oh right... There really was no reason to set them to NULL since the
> only time they're used is in the context they are used. If the
> AddProgram was successful, then there would be 2 refs anyway. They're
> not used later in the code, so I'll remove the = NULL after the Unref.
Is it possible to get rid of these global variables? At least for
adminProgram and lxcProgram this should be easily to achieve as they
aren’t used anywhere else than in this function context.
I suppose so, not sure if that was within the context of what I was
doing. It'd have to be a separate "pre-patch" - in fact doable without
this series. I also know Dan's been working through adding admin
protocol support for logd/lockd in another series...
John
libvirtd.h: (Only these variables are declared as extern)
extern virNetServerProgramPtr remoteProgram;
extern virNetServerProgramPtr qemuProgram;
>
> John
>
>>
[…snip…]
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
11:23 a.m.
New subject: [libvirt] [PATCH 3/9] netserver: Remove ServiceToggle during ServerDispose
No sense in calling ServiceToggle for all nservices during
ServiceDispose since ServerClose calls ServiceClose which
removes the IOCallback that's being toggled via ServiceToggle.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
---
src/rpc/virnetserver.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 77a4c0b8d..7bab11efb 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -805,9 +805,6 @@ void virNetServerDispose(void *obj)
VIR_FREE(srv->name);
- for (i = 0; i < srv->nservices; i++)
- virNetServerServiceToggle(srv->services[i], false);
-
virThreadPoolFree(srv->workers);
for (i = 0; i < srv->nservices; i++)
--
2.13.6
11:23 a.m.
New subject: [libvirt] [PATCH 4/9] util: Introduce virThreadPoolDrain
Create a mechanism to allow the domain/server quit code to be able
to cause any pending jobs to be be purged and request current workers
to quit.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/virthreadpool.c | 64 ++++++++++++++++++++++++++++++++++++++++--------
src/util/virthreadpool.h | 2 ++
3 files changed, 57 insertions(+), 10 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bc8cc1fba..6ffceb46b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2865,6 +2865,7 @@ virThreadJobSetWorker;
# util/virthreadpool.h
+virThreadPoolDrain;
virThreadPoolFree;
virThreadPoolGetCurrentWorkers;
virThreadPoolGetFreeWorkers;
diff --git a/src/util/virthreadpool.c b/src/util/virthreadpool.c
index 10f2bd2c3..0baa05d12 100644
--- a/src/util/virthreadpool.c
+++ b/src/util/virthreadpool.c
@@ -30,9 +30,12 @@
#include "viralloc.h"
#include "virthread.h"
#include "virerror.h"
+#include "virlog.h"
#define VIR_FROM_THIS VIR_FROM_NONE
+VIR_LOG_INIT("util.threadpool");
+
typedef struct _virThreadPoolJob virThreadPoolJob;
typedef virThreadPoolJob *virThreadPoolJobPtr;
@@ -93,6 +96,24 @@ static inline bool virThreadPoolWorkerQuitHelper(size_t count, size_t
limit)
return count > limit;
}
+
+static void
+virThreadPoolJobRemove(virThreadPoolPtr pool,
+ virThreadPoolJobPtr job)
+{
+ if (job->prev)
+ job->prev->next = job->next;
+ else
+ pool->jobList.head = job->next;
+ if (job->next)
+ job->next->prev = job->prev;
+ else
+ pool->jobList.tail = job->prev;
+
+ pool->jobQueueDepth--;
+}
+
+
static void virThreadPoolWorker(void *opaque)
{
struct virThreadPoolWorkerData *data = opaque;
@@ -152,16 +173,7 @@ static void virThreadPoolWorker(void *opaque)
pool->jobList.firstPrio = tmp;
}
- if (job->prev)
- job->prev->next = job->next;
- else
- pool->jobList.head = job->next;
- if (job->next)
- job->next->prev = job->prev;
- else
- pool->jobList.tail = job->prev;
-
- pool->jobQueueDepth--;
+ virThreadPoolJobRemove(pool, job);
virMutexUnlock(&pool->mutex);
(pool->jobFunc)(job->data, pool->jobOpaque);
@@ -307,6 +319,38 @@ void virThreadPoolFree(virThreadPoolPtr pool)
}
+/*
+ * virThreadPoolDrain:
+ * @pool: Pointer to thread pool
+ *
+ * Cause any pending job to be purged and notify the current workers
+ * of the impending quit.
+ */
+void
+virThreadPoolDrain(virThreadPoolPtr pool)
+{
+ virMutexLock(&pool->mutex);
+
+ VIR_DEBUG("nWorkers=%zd, nPrioWorkers=%zd jobQueueDepth=%zd",
+ pool->nWorkers, pool->nPrioWorkers, pool->jobQueueDepth);
+
+ while (pool->jobList.head != pool->jobList.tail) {
+ virThreadPoolJobPtr job = pool->jobList.head;
+
+ virThreadPoolJobRemove(pool, job);
+ VIR_FREE(job);
+ }
+
+ pool->quit = true;
+ if (pool->nWorkers > 0)
+ virCondBroadcast(&pool->cond);
+ if (pool->nPrioWorkers > 0)
+ virCondBroadcast(&pool->prioCond);
+
+ virMutexUnlock(&pool->mutex);
+}
+
+
size_t virThreadPoolGetMinWorkers(virThreadPoolPtr pool)
{
size_t ret;
diff --git a/src/util/virthreadpool.h b/src/util/virthreadpool.h
index e1f362f5b..c54b166b1 100644
--- a/src/util/virthreadpool.h
+++ b/src/util/virthreadpool.h
@@ -52,6 +52,8 @@ size_t virThreadPoolGetJobQueueDepth(virThreadPoolPtr pool);
void virThreadPoolFree(virThreadPoolPtr pool);
+void virThreadPoolDrain(virThreadPoolPtr pool);
+
int virThreadPoolSendJob(virThreadPoolPtr pool,
unsigned int priority,
void *jobdata) ATTRIBUTE_NONNULL(1)
--
2.13.6
11:23 a.m.
New subject: [libvirt] [PATCH 5/9] rpc: Introduce virNetServerQuitRequested
When virNetDaemonQuit is called, we need to let the NetServers
know a quit is pending and a subsequent Close will tear down
the environment.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/libvirt_remote.syms | 1 +
src/rpc/virnetdaemon.c | 13 +++++++++++++
src/rpc/virnetserver.c | 23 +++++++++++++++++++++++
src/rpc/virnetserver.h | 2 ++
4 files changed, 39 insertions(+)
diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
index a181c4cf7..3170fbd7c 100644
--- a/src/libvirt_remote.syms
+++ b/src/libvirt_remote.syms
@@ -116,6 +116,7 @@ virNetServerNewPostExecRestart;
virNetServerNextClientID;
virNetServerPreExecRestart;
virNetServerProcessClients;
+virNetServerQuitRequested;
virNetServerSetClientAuthenticated;
virNetServerStart;
virNetServerUpdateServices;
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index 8c2141489..e5b376ced 100644
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -850,6 +850,18 @@ virNetDaemonRun(virNetDaemonPtr dmn)
}
+static int
+daemonServerQuitRequested(void *payload,
+ const void *key ATTRIBUTE_UNUSED,
+ void *opaque ATTRIBUTE_UNUSED)
+{
+ virNetServerPtr srv = payload;
+
+ virNetServerQuitRequested(srv);
+ return 0;
+}
+
+
void
virNetDaemonQuit(virNetDaemonPtr dmn)
{
@@ -857,6 +869,7 @@ virNetDaemonQuit(virNetDaemonPtr dmn)
VIR_DEBUG("Quit requested %p", dmn);
dmn->quit = true;
+ virHashForEach(dmn->servers, daemonServerQuitRequested, NULL);
virObjectUnlock(dmn);
}
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 7bab11efb..7114749ab 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -823,6 +823,29 @@ void virNetServerDispose(void *obj)
virNetServerMDNSFree(srv->mdns);
}
+
+/* virNetServerQuitRequested:
+ * @srv: Netserver pointer
+ *
+ * Disable new connections and drain anything waiting to run.
+ */
+void
+virNetServerQuitRequested(virNetServerPtr srv)
+{
+ size_t i;
+
+ if (!srv)
+ return;
+
+ VIR_DEBUG("Quit server requested '%s'", srv->name);
+
+ for (i = 0; i < srv->nservices; i++)
+ virNetServerServiceToggle(srv->services[i], false);
+
+ virThreadPoolDrain(srv->workers);
+}
+
+
void virNetServerClose(virNetServerPtr srv)
{
size_t i;
diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
index 7728a67f5..a9bd3480b 100644
--- a/src/rpc/virnetserver.h
+++ b/src/rpc/virnetserver.h
@@ -57,6 +57,8 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object,
virFreeCallback clientPrivFree,
void *clientPrivOpaque);
+void virNetServerQuitRequested(virNetServerPtr srv);
+
void virNetServerClose(virNetServerPtr srv);
virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv);
--
2.13.6
11:23 a.m.
New subject: [libvirt] [PATCH 6/9] rpc: Introduce virNetServerWorkerCount
Get a count of the number of workers for all servers that are still
processing a job.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/libvirt_remote.syms | 1 +
src/rpc/virnetserver.c | 26 ++++++++++++++++++++++++++
src/rpc/virnetserver.h | 2 ++
3 files changed, 29 insertions(+)
diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
index 3170fbd7c..c6fd156ef 100644
--- a/src/libvirt_remote.syms
+++ b/src/libvirt_remote.syms
@@ -120,6 +120,7 @@ virNetServerQuitRequested;
virNetServerSetClientAuthenticated;
virNetServerStart;
virNetServerUpdateServices;
+virNetServerWorkerCount;
# rpc/virnetserverclient.h
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 7114749ab..dc92dfea0 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -846,6 +846,32 @@ virNetServerQuitRequested(virNetServerPtr srv)
}
+/* virNetServerWorkerCount:
+ * @srv: Netserver pointer
+ *
+ * Return the number of workers still waiting to be processed. This
+ * allows the quit requested code to wait for all worker jobs to be
+ * completed before actually quitting.
+ */
+size_t
+virNetServerWorkerCount(virNetServerPtr srv)
+{
+ size_t workerCount;
+
+ virObjectLock(srv);
+
+ workerCount = virThreadPoolGetCurrentWorkers(srv->workers) +
+ virThreadPoolGetPriorityWorkers(srv->workers);
+
+ if (workerCount > 0)
+ VIR_DEBUG("server '%s' still has %zd workers", srv->name,
workerCount);
+
+ virObjectUnlock(srv);
+
+ return workerCount;
+}
+
+
void virNetServerClose(virNetServerPtr srv)
{
size_t i;
diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
index a9bd3480b..1b1916814 100644
--- a/src/rpc/virnetserver.h
+++ b/src/rpc/virnetserver.h
@@ -59,6 +59,8 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object,
void virNetServerQuitRequested(virNetServerPtr srv);
+size_t virNetServerWorkerCount(virNetServerPtr srv);
+
void virNetServerClose(virNetServerPtr srv);
virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv);
--
2.13.6
11:23 a.m.
New subject: [libvirt] [PATCH 7/9] rpc: Alter virNetDaemonQuit processing
When virNetDaemonQuit is called from libvirtd's shutdown
handler (daemonShutdownHandler) we need to perform the quit
in multiple steps. The first part is to "request" the quit
and notify the NetServer's of the impending quit which causes
the NetServers to Drain their pending queue and tell workers
to quit and the second is wait for any currently running
worker jobs to complete. Once the workers are complete, then
we can cause the quit to occur.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/rpc/virnetdaemon.c | 32 +++++++++++++++++++++++++++++++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index e5b376ced..6aee712a5 100644
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -73,6 +73,7 @@ struct _virNetDaemon {
virHashTablePtr servers;
virJSONValuePtr srvObject;
+ bool quitRequested;
bool quit;
unsigned int autoShutdownTimeout;
@@ -779,6 +780,32 @@ daemonServerProcessClients(void *payload,
return 0;
}
+
+static int
+daemonServerWorkerCount(void *payload,
+ const void *key ATTRIBUTE_UNUSED,
+ void *opaque)
+{
+ size_t *workerCount = opaque;
+ virNetServerPtr srv = payload;
+
+ *workerCount += virNetServerWorkerCount(srv);
+
+ return 0;
+}
+
+
+static bool
+daemonServerWorkersDone(virNetDaemonPtr dmn)
+{
+ size_t workerCount = 0;
+
+ virHashForEach(dmn->servers, daemonServerWorkerCount, &workerCount);
+
+ return workerCount == 0;
+}
+
+
void
virNetDaemonRun(virNetDaemonPtr dmn)
{
@@ -843,6 +870,9 @@ virNetDaemonRun(virNetDaemonPtr dmn)
virObjectLock(dmn);
virHashForEach(dmn->servers, daemonServerProcessClients, NULL);
+
+ if (dmn->quitRequested && daemonServerWorkersDone(dmn))
+ dmn->quit = true;
}
cleanup:
@@ -868,7 +898,7 @@ virNetDaemonQuit(virNetDaemonPtr dmn)
virObjectLock(dmn);
VIR_DEBUG("Quit requested %p", dmn);
- dmn->quit = true;
+ dmn->quitRequested = true;
virHashForEach(dmn->servers, daemonServerQuitRequested, NULL);
virObjectUnlock(dmn);
--
2.13.6
12:19 p.m.
New subject: [libvirt] [PATCH 7/9] rpc: Alter virNetDaemonQuit processing
On 01/19/2018 12:23 PM, John Ferlan wrote:
When virNetDaemonQuit is called from libvirtd's shutdown
handler (daemonShutdownHandler) we need to perform the quit
in multiple steps. The first part is to "request" the quit
and notify the NetServer's of the impending quit which causes
the NetServers to Drain their pending queue and tell workers
to quit and the second is wait for any currently running
worker jobs to complete. Once the workers are complete, then
we can cause the quit to occur.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/rpc/virnetdaemon.c | 32 +++++++++++++++++++++++++++++++-
1 file changed, 31 insertions(+), 1 deletion(-)
sigh - too quick with the send and neglected to run the so what happens
when I have nothing pending check/test... Attached is a patch that
would be merged with this one.
John
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index e5b376ced..6aee712a5 100644
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -73,6 +73,7 @@ struct _virNetDaemon {
virHashTablePtr servers;
virJSONValuePtr srvObject;
+ bool quitRequested;
bool quit;
unsigned int autoShutdownTimeout;
@@ -779,6 +780,32 @@ daemonServerProcessClients(void *payload,
return 0;
}
+
+static int
+daemonServerWorkerCount(void *payload,
+ const void *key ATTRIBUTE_UNUSED,
+ void *opaque)
+{
+ size_t *workerCount = opaque;
+ virNetServerPtr srv = payload;
+
+ *workerCount += virNetServerWorkerCount(srv);
+
+ return 0;
+}
+
+
+static bool
+daemonServerWorkersDone(virNetDaemonPtr dmn)
+{
+ size_t workerCount = 0;
+
+ virHashForEach(dmn->servers, daemonServerWorkerCount, &workerCount);
+
+ return workerCount == 0;
+}
+
+
void
virNetDaemonRun(virNetDaemonPtr dmn)
{
@@ -843,6 +870,9 @@ virNetDaemonRun(virNetDaemonPtr dmn)
virObjectLock(dmn);
virHashForEach(dmn->servers, daemonServerProcessClients, NULL);
+
+ if (dmn->quitRequested && daemonServerWorkersDone(dmn))
+ dmn->quit = true;
}
cleanup:
@@ -868,7 +898,7 @@ virNetDaemonQuit(virNetDaemonPtr dmn)
virObjectLock(dmn);
VIR_DEBUG("Quit requested %p", dmn);
- dmn->quit = true;
+ dmn->quitRequested = true;
virHashForEach(dmn->servers, daemonServerQuitRequested, NULL);
virObjectUnlock(dmn);
11:21 a.m.
New subject: [libvirt] [PATCH 7/9] rpc: Alter virNetDaemonQuit processing
On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan <jferlan(a)redhat.com> wrote:
When virNetDaemonQuit is called from libvirtd's shutdown
handler (daemonShutdownHandler) we need to perform the quit
in multiple steps. The first part is to "request" the quit
and notify the NetServer's of the impending quit which causes
the NetServers to Drain their pending queue and tell workers
to quit and the second is wait for any currently running
worker jobs to complete. Once the workers are complete, then
we can cause the quit to occur.
Instead of “polling” for 'daemonServerWorkersDone' we could use a pipe
for signaling. This would also ensure that we do not remain in the
poll() call indefinitely and it avoids the timer hack.
[…snip]
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
11:23 a.m.
New subject: [libvirt] [PATCH 8/9] docs: Add news article for libvirtd issue
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
docs/news.xml | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml
index b4d980624..18800119d 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -39,6 +39,18 @@
<section title="Improvements">
</section>
<section title="Bug fixes">
+ <change>
+ <summary>
+ Fix issues with unexpected libvirtd termination
+ </summary>
+ <description>
+ If there were outstanding worker threads when libvirtd
+ received a quit signal via SIGTERM, SIGINT, or SIGQUIT
+ then, libvirtd may have crashed or hung waiting for a
+ worker job to send a completion notification that cannot
+ be received because the event loop was stopped too soon.
+ </description>
+ </change>
</section>
</release>
<release version="v4.0.0" date="2018-01-19">
--
2.13.6
11:23 a.m.
New subject: [libvirt] [PATCH 9/9] APPLY ONLY FOR TESTING PURPOSES
Modify GetAllStats to generate a long enough pause in order to send a
SIGTERM to libvirtd while a client connection is processing.
In order to "set things up":
1. In one terminal window from a local branch built using these
patches using a root account run libvirtd debug, e.g.:
# ./run gdb daemon/libvirtd
once running, type a 'c' (e.g. continue) and <return>
2. Start a domain (or have one running with the current libvirtd)
virsh start $domain
3. Prepare a domstats command for that domain (but don't yet
hit <return> in order run it):
virsh domstats $domain
4. Prepare a kill command for the running libvirtd, e.g.:
jferlan 4143 4107 0 09:51 pts/1 00:00:00 vim +1396 daemon/libvirtd.c
root 30054 21195 6 11:17 pts/8 00:00:01 gdb
/home/jferlan/git/libvirt.work/daemon/.libs/lt-libvirtd
root 30087 30054 7 11:17 pts/8 00:00:01
/home/jferlan/git/libvirt.work/daemon/.libs/lt-libvirtd
root 30385 19861 0 11:17 pts/17 00:00:00 grep --color=auto libvirtd
but again don't hit <return> yet.
5. Align your stars perfectly now...
a. Hit <return> on your domstats command
b. Swap to the kill command window and hit <return>
This should cause the libvirtd debug window to stop, but since you
already typed 'c' it'll continue at least briefly, for example:
...
[Thread 0x7fffc3231700 (LWP 30374) exited]
Detaching after fork from child process 30376.
Detaching after fork from child process 30377.
Detaching after fork from child process 30378.
[Thread 0x7fffc4445700 (LWP 30106) exited]
c
2018-01-10 16:18:12.962+0000: 30094: info : libvirt version: 4.0.0
2018-01-10 16:18:12.962+0000: 30094: info : hostname: unknown4ceb42c824f4.attlocal.net
2018-01-10 16:18:12.962+0000: 30094: warning : qemuConnectGetAllDomainStats:20265 : k =
-5340232226128654848
Thread 1 "lt-libvirtd" received signal SIGTERM, Terminated.
0x00007ffff3ae6d2d in poll () from /lib64/libc.so.6
...
(gdb) c
Continuing.
[Thread 0x7fffc5c48700 (LWP 30103) exited]
[Thread 0x7fffc5447700 (LWP 30104) exited]
[Thread 0x7fffc4c46700 (LWP 30105) exited]
[Thread 0x7fffc6449700 (LWP 30102) exited]
[Thread 0x7fffc6c4a700 (LWP 30101) exited]
[Thread 0x7fffe3b57700 (LWP 30097) exited]
[Thread 0x7fffe4358700 (LWP 30096) exited]
[Thread 0x7fffe2354700 (LWP 30100) exited]
[Thread 0x7fffe3356700 (LWP 30098) exited]
[Thread 0x7fffe2b55700 (LWP 30099) exited]
[Thread 0x7fffe535a700 (LWP 30094) exited]
[Thread 0x7fffe5b5b700 (LWP 30093) exited]
[Thread 0x7fffe635c700 (LWP 30092) exited]
[Thread 0x7fffe6b5d700 (LWP 30091) exited]
2018-01-10 16:18:25.451+0000: 30095: warning : qemuConnectGetAllDomainStats:20265 : k =
-5340232226128654848
[Thread 0x7fffe4b59700 (LWP 30095) exited]
[Thread 0x7fffc3a32700 (LWP 30187) exited]
[Inferior 1 (process 30087) exited normally]
(gdb) c
The program is not being run.
(gdb) quit
The virsh domstats window will "close" as follows:
error: Disconnected from qemu:///system due to end of file
error: End of file while reading data: Input/output error
If something's wrong, then the libvirtd window may not exit those
final two threads in which case you could interrupt it (^c) and
check the threads (thread apply all bt) which will probably show
some sort of hang... My testing shows that the hang no longer
occurs with all the previous patches applied.
The subsequent patch calling virHashRemoveAll from virNetDaemonClose
does not seem to be necessary, although I suppose it cannot hurt as
the same essential functionality occurs during the Dispose function
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a203c9297..d1be8048f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20184,6 +20184,7 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
bool enforce = !!(flags & VIR_CONNECT_GET_ALL_DOMAINS_STATS_ENFORCE_STATS);
int nstats = 0;
size_t i;
+ size_t j, k = 0;
int ret = -1;
unsigned int privflags = 0;
unsigned int domflags = 0;
@@ -20221,6 +20222,10 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
if (qemuDomainGetStatsNeedMonitor(stats))
privflags |= QEMU_DOMAIN_STATS_HAVE_JOB;
+ for (j = 0; j < 10000000000; j++) // Add one more zero for longer...
+ k = j + k;
+ VIR_WARN("k = %zd", k);
+
for (i = 0; i < nvms; i++) {
virDomainStatsRecordPtr tmp = NULL;
domflags = 0;
--
2.13.6
12:33 a.m.
Hi, John.
Thank you for driving this issue! Unfortunately I have little time now
to take participation in reviweing etc.
Nikolay
On 19.01.2018 20:23, John Ferlan wrote:
RFC:
https://www.redhat.com/archives/libvir-list/2018-January/msg00318.html
Adjustments since RFC...
Patches 1&2: No change, were already R-B'd
Patch 3: Removed code as noted in code review, update commit message
Patch 4: From old series removed, see below for more details
Patch 9: no change
NB: Patches 5-8 and 10 from Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
are removed as they seemed to not be necessary
Replaced the former patch 4 with series of patches to (slowly) provide
support to disable new connections, handle removing waiting jobs, causing
the waiting workers to quit, and allow any running jobs to complete.
As it turns out, waiting for running jobs to complete cannot be done
from the virNetServerClose callbacks because that means the event loop
processing done during virNetServerRun will not allow any currently
long running worker job thread a means to complete.
So when virNetDaemonQuit is called as a result of the libvirtd signal
handlers for SIG{QUIT|INT|TERM}, instead of just causing virNetServerRun
to quit immediately, alter to using a quitRequested flag and then use
that quitRequested flag to check for long running worker threads before
causing the event loop to quit resulting in libvirtd being able to run
through the virNetDaemonClose processing.
John Ferlan (9):
libvirtd: Alter refcnt processing for domain server objects
libvirtd: Alter refcnt processing for server program objects
netserver: Remove ServiceToggle during ServerDispose
util: Introduce virThreadPoolDrain
rpc: Introduce virNetServerQuitRequested
rpc: Introduce virNetServerWorkerCount
rpc: Alter virNetDaemonQuit processing
docs: Add news article for libvirtd issue
APPLY ONLY FOR TESTING PURPOSES
daemon/libvirtd.c | 43 +++++++++++++++++++++++---------
docs/news.xml | 12 +++++++++
src/libvirt_private.syms | 1 +
src/libvirt_remote.syms | 2 ++
src/qemu/qemu_driver.c | 5 ++++
src/rpc/virnetdaemon.c | 45 +++++++++++++++++++++++++++++++++-
src/rpc/virnetserver.c | 52 ++++++++++++++++++++++++++++++++++++---
src/rpc/virnetserver.h | 4 +++
src/util/virthreadpool.c | 64 ++++++++++++++++++++++++++++++++++++++++--------
src/util/virthreadpool.h | 2 ++
10 files changed, 204 insertions(+), 26 deletions(-)
3:21 a.m.
On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan <jferlan(a)redhat.com> wrote:
RFC:
https://www.redhat.com/archives/libvir-list/2018-January/msg00318.html
Adjustments since RFC...
Patches 1&2: No change, were already R-B'd
Patch 3: Removed code as noted in code review, update commit message
Patch 4: From old series removed, see below for more details
Patch 9: no change
NB: Patches 5-8 and 10 from Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
are removed as they seemed to not be necessary
Replaced the former patch 4 with series of patches to (slowly) provide
support to disable new connections, handle removing waiting jobs, causing
the waiting workers to quit, and allow any running jobs to complete.
As it turns out, waiting for running jobs to complete cannot be done
from the virNetServerClose callbacks because that means the event loop
processing done during virNetServerRun will not allow any currently
long running worker job thread a means to complete.
So when virNetDaemonQuit is called as a result of the libvirtd signal
handlers for SIG{QUIT|INT|TERM}, instead of just causing virNetServerRun
to quit immediately, alter to using a quitRequested flag and then use
that quitRequested flag to check for long running worker threads before
causing the event loop to quit resulting in libvirtd being able to run
through the virNetDaemonClose processing.
Gave a quick test:
+ Didn't get a segmentation fault at the end of libvirtd (at least for
my quick test)
- a single SIGTERM doesn’t always lead to the termination of libvirtd
now (debugged it: main thread is waiting for poll()). This behavior
can be easily reproduced: Start libvirtd on the CLI, wait some
seconds for the first initialization -> CTRL + C -> libvirtd doesn’t
terminate, but also doesn’t accept new connections.
John Ferlan (9):
libvirtd: Alter refcnt processing for domain server objects
libvirtd: Alter refcnt processing for server program objects
netserver: Remove ServiceToggle during ServerDispose
util: Introduce virThreadPoolDrain
rpc: Introduce virNetServerQuitRequested
rpc: Introduce virNetServerWorkerCount
rpc: Alter virNetDaemonQuit processing
docs: Add news article for libvirtd issue
APPLY ONLY FOR TESTING PURPOSES
daemon/libvirtd.c | 43 +++++++++++++++++++++++---------
docs/news.xml | 12 +++++++++
src/libvirt_private.syms | 1 +
src/libvirt_remote.syms | 2 ++
src/qemu/qemu_driver.c | 5 ++++
src/rpc/virnetdaemon.c | 45 +++++++++++++++++++++++++++++++++-
src/rpc/virnetserver.c | 52 ++++++++++++++++++++++++++++++++++++---
src/rpc/virnetserver.h | 4 +++
src/util/virthreadpool.c | 64 ++++++++++++++++++++++++++++++++++++++++--------
src/util/virthreadpool.h | 2 ++
10 files changed, 204 insertions(+), 26 deletions(-)
--
2.13.6
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
9:01 a.m.
On 01/23/2018 04:21 AM, Marc Hartmayer wrote:
On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan
<jferlan(a)redhat.com> wrote:
> RFC:
> https://www.redhat.com/archives/libvir-list/2018-January/msg00318.html
>
> Adjustments since RFC...
>
> Patches 1&2: No change, were already R-B'd
> Patch 3: Removed code as noted in code review, update commit message
> Patch 4: From old series removed, see below for more details
> Patch 9: no change
> NB: Patches 5-8 and 10 from Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
> are removed as they seemed to not be necessary
>
> Replaced the former patch 4 with series of patches to (slowly) provide
> support to disable new connections, handle removing waiting jobs, causing
> the waiting workers to quit, and allow any running jobs to complete.
>
> As it turns out, waiting for running jobs to complete cannot be done
> from the virNetServerClose callbacks because that means the event loop
> processing done during virNetServerRun will not allow any currently
> long running worker job thread a means to complete.
>
> So when virNetDaemonQuit is called as a result of the libvirtd signal
> handlers for SIG{QUIT|INT|TERM}, instead of just causing virNetServerRun
> to quit immediately, alter to using a quitRequested flag and then use
> that quitRequested flag to check for long running worker threads before
> causing the event loop to quit resulting in libvirtd being able to run
> through the virNetDaemonClose processing.
Gave a quick test:
+ Didn't get a segmentation fault at the end of libvirtd (at least for
my quick test)
- a single SIGTERM doesn’t always lead to the termination of libvirtd
now (debugged it: main thread is waiting for poll()). This behavior
can be easily reproduced: Start libvirtd on the CLI, wait some
seconds for the first initialization -> CTRL + C -> libvirtd doesn’t
terminate, but also doesn’t accept new connections.
Does this include the "update" in my response to patch 7? It should be
extract-able and apply-able.
John
>
> John Ferlan (9):
> libvirtd: Alter refcnt processing for domain server objects
> libvirtd: Alter refcnt processing for server program objects
> netserver: Remove ServiceToggle during ServerDispose
> util: Introduce virThreadPoolDrain
> rpc: Introduce virNetServerQuitRequested
> rpc: Introduce virNetServerWorkerCount
> rpc: Alter virNetDaemonQuit processing
> docs: Add news article for libvirtd issue
> APPLY ONLY FOR TESTING PURPOSES
>
> daemon/libvirtd.c | 43 +++++++++++++++++++++++---------
> docs/news.xml | 12 +++++++++
> src/libvirt_private.syms | 1 +
> src/libvirt_remote.syms | 2 ++
> src/qemu/qemu_driver.c | 5 ++++
> src/rpc/virnetdaemon.c | 45 +++++++++++++++++++++++++++++++++-
> src/rpc/virnetserver.c | 52 ++++++++++++++++++++++++++++++++++++---
> src/rpc/virnetserver.h | 4 +++
> src/util/virthreadpool.c | 64 ++++++++++++++++++++++++++++++++++++++++--------
> src/util/virthreadpool.h | 2 ++
> 10 files changed, 204 insertions(+), 26 deletions(-)
>
> --
> 2.13.6
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
10:41 a.m.
On Tue, Jan 23, 2018 at 04:01 PM +0100, John Ferlan <jferlan(a)redhat.com> wrote:
On 01/23/2018 04:21 AM, Marc Hartmayer wrote:
> On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan <jferlan(a)redhat.com>
wrote:
>> RFC:
>> https://www.redhat.com/archives/libvir-list/2018-January/msg00318.html
>>
>> Adjustments since RFC...
>>
>> Patches 1&2: No change, were already R-B'd
>> Patch 3: Removed code as noted in code review, update commit message
>> Patch 4: From old series removed, see below for more details
>> Patch 9: no change
>> NB: Patches 5-8 and 10 from Nikolay Shirokovskiy
<nshirokovskiy(a)virtuozzo.com>
>> are removed as they seemed to not be necessary
>>
>> Replaced the former patch 4 with series of patches to (slowly) provide
>> support to disable new connections, handle removing waiting jobs, causing
>> the waiting workers to quit, and allow any running jobs to complete.
>>
>> As it turns out, waiting for running jobs to complete cannot be done
>> from the virNetServerClose callbacks because that means the event loop
>> processing done during virNetServerRun will not allow any currently
>> long running worker job thread a means to complete.
>>
>> So when virNetDaemonQuit is called as a result of the libvirtd signal
>> handlers for SIG{QUIT|INT|TERM}, instead of just causing virNetServerRun
>> to quit immediately, alter to using a quitRequested flag and then use
>> that quitRequested flag to check for long running worker threads before
>> causing the event loop to quit resulting in libvirtd being able to run
>> through the virNetDaemonClose processing.
>
> Gave a quick test:
> + Didn't get a segmentation fault at the end of libvirtd (at least for
> my quick test)
> - a single SIGTERM doesn’t always lead to the termination of libvirtd
> now (debugged it: main thread is waiting for poll()). This behavior
> can be easily reproduced: Start libvirtd on the CLI, wait some
> seconds for the first initialization -> CTRL + C -> libvirtd doesn’t
> terminate, but also doesn’t accept new connections.
>
Does this include the "update" in my response to patch 7? It should be
extract-able and apply-able.
No, I forgot this one :( I’ll try it tomorrow.
John
>>
>> John Ferlan (9):
>> libvirtd: Alter refcnt processing for domain server objects
>> libvirtd: Alter refcnt processing for server program objects
>> netserver: Remove ServiceToggle during ServerDispose
>> util: Introduce virThreadPoolDrain
>> rpc: Introduce virNetServerQuitRequested
>> rpc: Introduce virNetServerWorkerCount
>> rpc: Alter virNetDaemonQuit processing
>> docs: Add news article for libvirtd issue
>> APPLY ONLY FOR TESTING PURPOSES
>>
>> daemon/libvirtd.c | 43 +++++++++++++++++++++++---------
>> docs/news.xml | 12 +++++++++
>> src/libvirt_private.syms | 1 +
>> src/libvirt_remote.syms | 2 ++
>> src/qemu/qemu_driver.c | 5 ++++
>> src/rpc/virnetdaemon.c | 45 +++++++++++++++++++++++++++++++++-
>> src/rpc/virnetserver.c | 52 ++++++++++++++++++++++++++++++++++++---
>> src/rpc/virnetserver.h | 4 +++
>> src/util/virthreadpool.c | 64 ++++++++++++++++++++++++++++++++++++++++--------
>> src/util/virthreadpool.h | 2 ++
>> 10 files changed, 204 insertions(+), 26 deletions(-)
>>
>> --
>> 2.13.6
>>
>> --
>> libvir-list mailing list
>> libvir-list(a)redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
>>
> --
> Beste Grüße / Kind regards
> Marc Hartmayer
>
> IBM Deutschland Research & Development GmbH
> Vorsitzende des Aufsichtsrats: Martina Koederitz
> Geschäftsführung: Dirk Wittkopp
> Sitz der Gesellschaft: Böblingen
> Registergericht: Amtsgericht Stuttgart, HRB 243294
>
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
11:02 a.m.
On Tue, Jan 23, 2018 at 05:41 PM +0100, Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
wrote:
On Tue, Jan 23, 2018 at 04:01 PM +0100, John Ferlan
<jferlan(a)redhat.com> wrote:
> On 01/23/2018 04:21 AM, Marc Hartmayer wrote:
>> On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan <jferlan(a)redhat.com>
wrote:
>>> RFC:
>>> https://www.redhat.com/archives/libvir-list/2018-January/msg00318.html
>>>
>>> Adjustments since RFC...
>>>
>>> Patches 1&2: No change, were already R-B'd
>>> Patch 3: Removed code as noted in code review, update commit message
>>> Patch 4: From old series removed, see below for more details
>>> Patch 9: no change
>>> NB: Patches 5-8 and 10 from Nikolay Shirokovskiy
<nshirokovskiy(a)virtuozzo.com>
>>> are removed as they seemed to not be necessary
>>>
>>> Replaced the former patch 4 with series of patches to (slowly) provide
>>> support to disable new connections, handle removing waiting jobs, causing
>>> the waiting workers to quit, and allow any running jobs to complete.
>>>
>>> As it turns out, waiting for running jobs to complete cannot be done
>>> from the virNetServerClose callbacks because that means the event loop
>>> processing done during virNetServerRun will not allow any currently
>>> long running worker job thread a means to complete.
>>>
>>> So when virNetDaemonQuit is called as a result of the libvirtd signal
>>> handlers for SIG{QUIT|INT|TERM}, instead of just causing virNetServerRun
>>> to quit immediately, alter to using a quitRequested flag and then use
>>> that quitRequested flag to check for long running worker threads before
>>> causing the event loop to quit resulting in libvirtd being able to run
>>> through the virNetDaemonClose processing.
>>
>> Gave a quick test:
>> + Didn't get a segmentation fault at the end of libvirtd (at least for
>> my quick test)
>> - a single SIGTERM doesn’t always lead to the termination of libvirtd
>> now (debugged it: main thread is waiting for poll()). This behavior
>> can be easily reproduced: Start libvirtd on the CLI, wait some
>> seconds for the first initialization -> CTRL + C -> libvirtd doesn’t
>> terminate, but also doesn’t accept new connections.
>>
>
> Does this include the "update" in my response to patch 7? It should be
> extract-able and apply-able.
No, I forgot this one :( I’ll try it tomorrow.
It works with the new patch 7.
[…snip]
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
2:47 a.m.
On 19.01.2018 20:23, John Ferlan wrote:
RFC:
https://www.redhat.com/archives/libvir-list/2018-January/msg00318.html
Adjustments since RFC...
Patches 1&2: No change, were already R-B'd
Patch 3: Removed code as noted in code review, update commit message
Patch 4: From old series removed, see below for more details
Patch 9: no change
NB: Patches 5-8 and 10 from Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
are removed as they seemed to not be necessary
Replaced the former patch 4 with series of patches to (slowly) provide
support to disable new connections, handle removing waiting jobs, causing
the waiting workers to quit, and allow any running jobs to complete.
As it turns out, waiting for running jobs to complete cannot be done
from the virNetServerClose callbacks because that means the event loop
processing done during virNetServerRun will not allow any currently
long running worker job thread a means to complete.
Hi, John.
So you suggest a different appoarch. Instead of introducing means to
help rpc threads to finish after event loop is finished (stateShutdown hook in my series)
you suggest to block futher new rpc processing and then waiting running
rpc calls to finish keeping event loop running for that purpose.
This could lead to libvirtd never finish its termination if one of
qemu processes do not respond for example. In case of long running
operation such as migration finishing can take much time. On the
over hand if we finish rpc threads abruptly as in case of stateShutdown hook
we will deal with all possible error paths on daemon finishing. May
be good approach is to abort long running operation, then give rpc threads
some time to finish as you suggest and only after that finish them abruptly to handle
hanging qemu etc.
Also in this approach we keep event loop running for rpc threads only
but there can be other threads that depend on the loop. For example if
qemu is rebooted we spawn a thread that executes qemuProcessFakeReboot
that sends commands to qemu (i.e. depends on event loop). We need to await
such threads finishing too while keep event loop running. In approach of
stateShutdown hook we finish all threads that uses qemu monitor by closing
the monitor.
And hack with timeout in a loop... I think of a different aproach for waiting
rpc threads to finish their work. First let's make drain function only flush
job queue and take some callback which will be called when all workers will
be free from work (let's keep worker threads as Dan suggested). In this callback we
can use same technique as in virNetDaemonSignalHandler. That is make some
IO to set some flag in the event loop thread and cause virEventRunDefaultImpl
to finish and then test this flag in virNetDaemonRun.
Nikolay
So when virNetDaemonQuit is called as a result of the libvirtd signal
handlers for SIG{QUIT|INT|TERM}, instead of just causing virNetServerRun
to quit immediately, alter to using a quitRequested flag and then use
that quitRequested flag to check for long running worker threads before
causing the event loop to quit resulting in libvirtd being able to run
through the virNetDaemonClose processing.
John Ferlan (9):
libvirtd: Alter refcnt processing for domain server objects
libvirtd: Alter refcnt processing for server program objects
netserver: Remove ServiceToggle during ServerDispose
util: Introduce virThreadPoolDrain
rpc: Introduce virNetServerQuitRequested
rpc: Introduce virNetServerWorkerCount
rpc: Alter virNetDaemonQuit processing
docs: Add news article for libvirtd issue
APPLY ONLY FOR TESTING PURPOSES
daemon/libvirtd.c | 43 +++++++++++++++++++++++---------
docs/news.xml | 12 +++++++++
src/libvirt_private.syms | 1 +
src/libvirt_remote.syms | 2 ++
src/qemu/qemu_driver.c | 5 ++++
src/rpc/virnetdaemon.c | 45 +++++++++++++++++++++++++++++++++-
src/rpc/virnetserver.c | 52 ++++++++++++++++++++++++++++++++++++---
src/rpc/virnetserver.h | 4 +++
src/util/virthreadpool.c | 64 ++++++++++++++++++++++++++++++++++++++++--------
src/util/virthreadpool.h | 2 ++
10 files changed, 204 insertions(+), 26 deletions(-)
4:29 a.m.
On Fri, Jan 26, 2018 at 11:47:04AM +0300, Nikolay Shirokovskiy wrote:
On 19.01.2018 20:23, John Ferlan wrote:
> RFC:
> https://www.redhat.com/archives/libvir-list/2018-January/msg00318.html
>
> Adjustments since RFC...
>
> Patches 1&2: No change, were already R-B'd
> Patch 3: Removed code as noted in code review, update commit message
> Patch 4: From old series removed, see below for more details
> Patch 9: no change
> NB: Patches 5-8 and 10 from Nikolay Shirokovskiy
<nshirokovskiy(a)virtuozzo.com>
> are removed as they seemed to not be necessary
>
> Replaced the former patch 4 with series of patches to (slowly) provide
> support to disable new connections, handle removing waiting jobs, causing
> the waiting workers to quit, and allow any running jobs to complete.
>
> As it turns out, waiting for running jobs to complete cannot be done
> from the virNetServerClose callbacks because that means the event loop
> processing done during virNetServerRun will not allow any currently
> long running worker job thread a means to complete.
Hi, John.
So you suggest a different appoarch. Instead of introducing means to
help rpc threads to finish after event loop is finished (stateShutdown hook in my
series)
you suggest to block futher new rpc processing and then waiting running
rpc calls to finish keeping event loop running for that purpose.
This could lead to libvirtd never finish its termination if one of
qemu processes do not respond for example. In case of long running
operation such as migration finishing can take much time. On the
over hand if we finish rpc threads abruptly as in case of stateShutdown hook
we will deal with all possible error paths on daemon finishing. May
be good approach is to abort long running operation, then give rpc threads
some time to finish as you suggest and only after that finish them abruptly
to handle hanging qemu etc.
We should keep in mind why we are bothering todo any of this "graceful"
cleanup. 99% of the time it would be fine for libvirtd to just immediately
immediately exit and not run any cleanup code, since the OS cleans everything
up regardless. Really the only benefit of running cleanup is so that developers
can check for memory leaks across the process execution.
On balance it is *way* more important that libvirtd is guranteed to exit in
a short, finite amount of time, even if that means skipping cleanup, because
that is what is relevant to production deployment.
So this means while it is nice to wait for worker threads to complete their
current RPC option, we should not wait too long. eg Give them 15 seconds to
finish their work, and if they're not done then just unconditionally exit
libvirtd, possibly skipping remaining cleanup if that's neccessary to avoid
SEGV.
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 :|
4:36 p.m.
On 01/26/2018 03:47 AM, Nikolay Shirokovskiy wrote:
On 19.01.2018 20:23, John Ferlan wrote:
> RFC:
> https://www.redhat.com/archives/libvir-list/2018-January/msg00318.html
>
> Adjustments since RFC...
>
> Patches 1&2: No change, were already R-B'd
> Patch 3: Removed code as noted in code review, update commit message
> Patch 4: From old series removed, see below for more details
> Patch 9: no change
> NB: Patches 5-8 and 10 from Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com>
> are removed as they seemed to not be necessary
>
> Replaced the former patch 4 with series of patches to (slowly) provide
> support to disable new connections, handle removing waiting jobs, causing
> the waiting workers to quit, and allow any running jobs to complete.
>
> As it turns out, waiting for running jobs to complete cannot be done
> from the virNetServerClose callbacks because that means the event loop
> processing done during virNetServerRun will not allow any currently
> long running worker job thread a means to complete.
Hi, John.
Sorry - been busy in other areas lately - trying to get back to this...
So you suggest a different appoarch. Instead of introducing means to
help rpc threads to finish after event loop is finished (stateShutdown hook in my
series)
you suggest to block futher new rpc processing and then waiting running
rpc calls to finish keeping event loop running for that purpose.
Somewhere along the way in one of the reviews it was suggested to give
the workers perhaps a few extra cycles to complete anything they might
be doing. It was also suggested a mechanism to do that - which is what I
tried to accomplish here.
Based on that and that your mechanism was more of an "immediate
separation" by IIRC closing the monitor connection and letting that
close handler do whatever magic happens there.
This not to say your mechanism is the wrong way to go about it, but it
also hasn't garnered support nor have you actively attempted to champion
it. So I presented an alternative approach.
This could lead to libvirtd never finish its termination if one of
qemu processes do not respond for example. In case of long running
operation such as migration finishing can take much time. On the
over hand if we finish rpc threads abruptly as in case of stateShutdown hook
we will deal with all possible error paths on daemon finishing. May
be good approach is to abort long running operation, then give rpc threads
some time to finish as you suggest and only after that finish them abruptly to handle
hanging qemu etc.
True, but it's also easy enough to add something to the last and updated
patch to add the QuitTimer and force an exit without going through the
rest of the "friendly" quit (IOW: more or less what Dan has suggested).
There's an incredible amount of cycles being spent for what amounts to
trying to be nice from a SIGTERM, SIGQUIT, or SIGINT - IOW: eventual death.
Also in this approach we keep event loop running for rpc threads only
but there can be other threads that depend on the loop. For example if
qemu is rebooted we spawn a thread that executes qemuProcessFakeReboot
that sends commands to qemu (i.e. depends on event loop). We need to await
such threads finishing too while keep event loop running. In approach of
stateShutdown hook we finish all threads that uses qemu monitor by closing
the monitor.
And hack with timeout in a loop... I think of a different aproach for waiting
rpc threads to finish their work. First let's make drain function only flush
job queue and take some callback which will be called when all workers will
be free from work (let's keep worker threads as Dan suggested). In this callback we
can use same technique as in virNetDaemonSignalHandler. That is make some
IO to set some flag in the event loop thread and cause virEventRunDefaultImpl
to finish and then test this flag in virNetDaemonRun.
Please feel free to spend as many cycles as you can making adjustments
to what I have. I'm working through some alterations and posting a v2 of
what I have and we'll see where it takes me/us.
John
Nikolay
>
> So when virNetDaemonQuit is called as a result of the libvirtd signal
> handlers for SIG{QUIT|INT|TERM}, instead of just causing virNetServerRun
> to quit immediately, alter to using a quitRequested flag and then use
> that quitRequested flag to check for long running worker threads before
> causing the event loop to quit resulting in libvirtd being able to run
> through the virNetDaemonClose processing.
>
> John Ferlan (9):
> libvirtd: Alter refcnt processing for domain server objects
> libvirtd: Alter refcnt processing for server program objects
> netserver: Remove ServiceToggle during ServerDispose
> util: Introduce virThreadPoolDrain
> rpc: Introduce virNetServerQuitRequested
> rpc: Introduce virNetServerWorkerCount
> rpc: Alter virNetDaemonQuit processing
> docs: Add news article for libvirtd issue
> APPLY ONLY FOR TESTING PURPOSES
>
> daemon/libvirtd.c | 43 +++++++++++++++++++++++---------
> docs/news.xml | 12 +++++++++
> src/libvirt_private.syms | 1 +
> src/libvirt_remote.syms | 2 ++
> src/qemu/qemu_driver.c | 5 ++++
> src/rpc/virnetdaemon.c | 45 +++++++++++++++++++++++++++++++++-
> src/rpc/virnetserver.c | 52 ++++++++++++++++++++++++++++++++++++---
> src/rpc/virnetserver.h | 4 +++
> src/util/virthreadpool.c | 64 ++++++++++++++++++++++++++++++++++++++++--------
> src/util/virthreadpool.h | 2 ++
> 10 files changed, 204 insertions(+), 26 deletions(-)
>
11:11 a.m.
On Wed, Feb 14, 2018 at 11:36 PM +0100, John Ferlan <jferlan(a)redhat.com> wrote:
On 01/26/2018 03:47 AM, Nikolay Shirokovskiy wrote:
>
>
> On 19.01.2018 20:23, John Ferlan wrote:
>> RFC:
>> https://www.redhat.com/archives/libvir-list/2018-January/msg00318.html
>>
>> Adjustments since RFC...
>>
>> Patches 1&2: No change, were already R-B'd
>> Patch 3: Removed code as noted in code review, update commit message
>> Patch 4: From old series removed, see below for more details
>> Patch 9: no change
>> NB: Patches 5-8 and 10 from Nikolay Shirokovskiy
<nshirokovskiy(a)virtuozzo.com>
>> are removed as they seemed to not be necessary
>>
>> Replaced the former patch 4 with series of patches to (slowly) provide
>> support to disable new connections, handle removing waiting jobs, causing
>> the waiting workers to quit, and allow any running jobs to complete.
>>
>> As it turns out, waiting for running jobs to complete cannot be done
>> from the virNetServerClose callbacks because that means the event loop
>> processing done during virNetServerRun will not allow any currently
>> long running worker job thread a means to complete.
>
> Hi, John.
Sorry - been busy in other areas lately - trying to get back to this...
>
> So you suggest a different appoarch. Instead of introducing means to
> help rpc threads to finish after event loop is finished (stateShutdown hook in my
series)
> you suggest to block futher new rpc processing and then waiting running
> rpc calls to finish keeping event loop running for that purpose.
Somewhere along the way in one of the reviews it was suggested to give
the workers perhaps a few extra cycles to complete anything they might
be doing. It was also suggested a mechanism to do that - which is what I
tried to accomplish here.
Based on that and that your mechanism was more of an "immediate
separation" by IIRC closing the monitor connection and letting that
close handler do whatever magic happens there.
This not to say your mechanism is the wrong way to go about it, but it
also hasn't garnered support nor have you actively attempted to champion
it. So I presented an alternative approach.
>
> This could lead to libvirtd never finish its termination if one of
> qemu processes do not respond for example. In case of long running
> operation such as migration finishing can take much time. On the
> over hand if we finish rpc threads abruptly as in case of stateShutdown hook
> we will deal with all possible error paths on daemon finishing. May
> be good approach is to abort long running operation, then give rpc threads
> some time to finish as you suggest and only after that finish them abruptly to
handle
> hanging qemu etc.
True, but it's also easy enough to add something to the last and updated
patch to add the QuitTimer and force an exit without going through the
rest of the "friendly" quit (IOW: more or less what Dan has suggested).
There's an incredible amount of cycles being spent for what amounts to
trying to be nice from a SIGTERM, SIGQUIT, or SIGINT - IOW: eventual death.
>
> Also in this approach we keep event loop running for rpc threads only
> but there can be other threads that depend on the loop. For example if
> qemu is rebooted we spawn a thread that executes qemuProcessFakeReboot
> that sends commands to qemu (i.e. depends on event loop). We need to await
> such threads finishing too while keep event loop running. In approach of
> stateShutdown hook we finish all threads that uses qemu monitor by closing
> the monitor.
>
> And hack with timeout in a loop... I think of a different aproach for waiting
> rpc threads to finish their work. First let's make drain function only flush
> job queue and take some callback which will be called when all workers will
> be free from work (let's keep worker threads as Dan suggested). In this callback
we
> can use same technique as in virNetDaemonSignalHandler. That is make some
> IO to set some flag in the event loop thread and cause virEventRunDefaultImpl
> to finish and then test this flag in virNetDaemonRun.
>
Please feel free to spend as many cycles as you can making adjustments
to what I have. I'm working through some alterations and posting a v2 of
what I have and we'll see where it takes me/us.
John
Is there any update so far? I’m asking because I’m still getting
segmentation faults and hang-ups on termination of libvirtd (using the
newest version of libvirt).
Example for a hang-up:
➤ bt
#0 0x000003fffca8df84 in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0
#1 0x000003fffdac29ca in virCondWait (c=<optimized out>, m=<optimized out>)
at ../../src/util/virthread.c:154
#2 0x000003fffdac381c in virThreadPoolFree (pool=<optimized out>) at
../../src/util/virthreadpool.c:290
#3 0x000003fffdbb21ae in virNetServerDispose (obj=0x1000cc640) at
../../src/rpc/virnetserver.c:803
#4 0x000003fffda97286 in virObjectUnref (anyobj=<optimized out>) at
../../src/util/virobject.c:350
#5 0x000003fffda97a5a in virObjectFreeHashData (opaque=<optimized out>,
name=<optimized out>) at ../../src/util/virobject.c:591
#6 0x000003fffda66576 in virHashFree (table=<optimized out>) at
../../src/util/virhash.c:305
#7 0x000003fffdbaff82 in virNetDaemonDispose (obj=0x1000cc3c0) at
../../src/rpc/virnetdaemon.c:105
#8 0x000003fffda97286 in virObjectUnref (anyobj=<optimized out>) at
../../src/util/virobject.c:350
#9 0x0000000100026cd6 in main (argc=<optimized out>, argv=<optimized out>) at
../../src/remote/remote_daemon.c:1487
And segmentation faults happen for RPC jobs that are still running.
[…snip]
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
2:21 p.m.
Is there any update so far? I’m asking because I’m still getting
segmentation faults and hang-ups on termination of libvirtd (using the
newest version of libvirt).
Example for a hang-up:
➤ bt
#0 0x000003fffca8df84 in pthread_cond_wait@(a)GLIBC_2.3.2 () from /lib64/libpthread.so.0
#1 0x000003fffdac29ca in virCondWait (c=<optimized out>, m=<optimized out>)
at ../../src/util/virthread.c:154
#2 0x000003fffdac381c in virThreadPoolFree (pool=<optimized out>) at
../../src/util/virthreadpool.c:290
#3 0x000003fffdbb21ae in virNetServerDispose (obj=0x1000cc640) at
../../src/rpc/virnetserver.c:803
#4 0x000003fffda97286 in virObjectUnref (anyobj=<optimized out>) at
../../src/util/virobject.c:350
#5 0x000003fffda97a5a in virObjectFreeHashData (opaque=<optimized out>,
name=<optimized out>) at ../../src/util/virobject.c:591
#6 0x000003fffda66576 in virHashFree (table=<optimized out>) at
../../src/util/virhash.c:305
#7 0x000003fffdbaff82 in virNetDaemonDispose (obj=0x1000cc3c0) at
../../src/rpc/virnetdaemon.c:105
#8 0x000003fffda97286 in virObjectUnref (anyobj=<optimized out>) at
../../src/util/virobject.c:350
#9 0x0000000100026cd6 in main (argc=<optimized out>, argv=<optimized out>)
at ../../src/remote/remote_daemon.c:1487
And segmentation faults happen for RPC jobs that are still running.
There has been zero of my cycles spent thinking about this. Partially
because I'm busy in other areas, partially because I know Daniel is
planning changes in libvirtd
(https://www.redhat.com/archives/libvir-list/2018-May/msg01307.html),
and partially because I'm not sure I have a {reliable|simple} reproducer
(at least I don't recall).
I do still have various branches in various states of disarray that are
way behind current head (easy to happen it seems lately).
John
4:15 a.m.
On Tue, Jul 03, 2018 at 09:21 PM +0200, John Ferlan <jferlan(a)redhat.com> wrote:
>
> Is there any update so far? I’m asking because I’m still getting
> segmentation faults and hang-ups on termination of libvirtd (using the
> newest version of libvirt).
>
> Example for a hang-up:
> ➤ bt
> #0 0x000003fffca8df84 in pthread_cond_wait@(a)GLIBC_2.3.2 () from
/lib64/libpthread.so.0
> #1 0x000003fffdac29ca in virCondWait (c=<optimized out>, m=<optimized
out>) at ../../src/util/virthread.c:154
> #2 0x000003fffdac381c in virThreadPoolFree (pool=<optimized out>) at
../../src/util/virthreadpool.c:290
> #3 0x000003fffdbb21ae in virNetServerDispose (obj=0x1000cc640) at
../../src/rpc/virnetserver.c:803
> #4 0x000003fffda97286 in virObjectUnref (anyobj=<optimized out>) at
../../src/util/virobject.c:350
> #5 0x000003fffda97a5a in virObjectFreeHashData (opaque=<optimized out>,
name=<optimized out>) at ../../src/util/virobject.c:591
> #6 0x000003fffda66576 in virHashFree (table=<optimized out>) at
../../src/util/virhash.c:305
> #7 0x000003fffdbaff82 in virNetDaemonDispose (obj=0x1000cc3c0) at
../../src/rpc/virnetdaemon.c:105
> #8 0x000003fffda97286 in virObjectUnref (anyobj=<optimized out>) at
../../src/util/virobject.c:350
> #9 0x0000000100026cd6 in main (argc=<optimized out>, argv=<optimized
out>) at ../../src/remote/remote_daemon.c:1487
>
> And segmentation faults happen for RPC jobs that are still running.
>
There has been zero of my cycles spent thinking about this. Partially
because I'm busy in other areas, partially because I know Daniel is
planning changes in libvirtd
(https://www.redhat.com/archives/libvir-list/2018-May/msg01307.html),
and partially because I'm not sure I have a {reliable|simple} reproducer
(at least I don't recall).
I do a simple start/destroy loop and send a SIGTERM to libvirtd. This
leads in almost every case to a segmentation fault.
I do still have various branches in various states of disarray that are
way behind current head (easy to happen it seems lately).
John
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
5:29 p.m.
On 07/06/2018 05:15 AM, Marc Hartmayer wrote:
On Tue, Jul 03, 2018 at 09:21 PM +0200, John Ferlan
<jferlan(a)redhat.com> wrote:
>>
>> Is there any update so far? I’m asking because I’m still getting
>> segmentation faults and hang-ups on termination of libvirtd (using the
>> newest version of libvirt).
>>
>> Example for a hang-up:
>> ➤ bt
>> #0 0x000003fffca8df84 in pthread_cond_wait@(a)GLIBC_2.3.2 () from
/lib64/libpthread.so.0
>> #1 0x000003fffdac29ca in virCondWait (c=<optimized out>, m=<optimized
out>) at ../../src/util/virthread.c:154
>> #2 0x000003fffdac381c in virThreadPoolFree (pool=<optimized out>) at
../../src/util/virthreadpool.c:290
>> #3 0x000003fffdbb21ae in virNetServerDispose (obj=0x1000cc640) at
../../src/rpc/virnetserver.c:803
>> #4 0x000003fffda97286 in virObjectUnref (anyobj=<optimized out>) at
../../src/util/virobject.c:350
>> #5 0x000003fffda97a5a in virObjectFreeHashData (opaque=<optimized out>,
name=<optimized out>) at ../../src/util/virobject.c:591
>> #6 0x000003fffda66576 in virHashFree (table=<optimized out>) at
../../src/util/virhash.c:305
>> #7 0x000003fffdbaff82 in virNetDaemonDispose (obj=0x1000cc3c0) at
../../src/rpc/virnetdaemon.c:105
>> #8 0x000003fffda97286 in virObjectUnref (anyobj=<optimized out>) at
../../src/util/virobject.c:350
>> #9 0x0000000100026cd6 in main (argc=<optimized out>, argv=<optimized
out>) at ../../src/remote/remote_daemon.c:1487
>>
>> And segmentation faults happen for RPC jobs that are still running.
>>
>
> There has been zero of my cycles spent thinking about this. Partially
> because I'm busy in other areas, partially because I know Daniel is
> planning changes in libvirtd
> (https://www.redhat.com/archives/libvir-list/2018-May/msg01307.html),
> and partially because I'm not sure I have a {reliable|simple} reproducer
> (at least I don't recall).
I do a simple start/destroy loop and send a SIGTERM to libvirtd. This
leads in almost every case to a segmentation fault.
>
> I do still have various branches in various states of disarray that are
> way behind current head (easy to happen it seems lately).
>
I found/updated my branches and started recalling where things were left
off. I had R-By's for patch 1 & 2 until you noted the issue with the
global variables which was fixed in my branch.
Since there's been a lot of code motion since and what I have now is
different I figure I should repost the series and go from there. I
haven't addressed the use pipe instead of polling comment you had in
patch7. I haven't put much thought into it, but in general given how
things work how would you expect pipe signaling to work in this model?
Essentially we have a client that either is not responding or is in the
middle of something large.
NB: I won't be around for the first half of next week to see any
responses or answer questions... Perhaps it's vacation mode that isn't
allowing me to think about the pipe signaling logic right now ;-)
John
2144
days inactive
2312
days old
29 comments
5 participants
participants (5)
-
Daniel P. Berrangé -
John Ferlan -
Marc Hartmayer -
Marc Hartmayer -
Nikolay Shirokovskiy