Re: [libvirt PATCH v3 1/7] tools: rewrite interactive job monitoring logic

For long running jobs (save, managed save, dump & live migrate) virsh runs a background thread for executing the job and then has the main thread catch Ctrl-C for graceful shutdown, as well as displaying progress info. The monitoring code is written using poll, with a pipe used to get the completion status from the thread. Using a pipe and poll is problematic for Windows portability. This rewrites the code to use a GMainLoop instance for monitoring stdin and doing progress updates. The use of a pipe is entirely eliminated, instead there is just a shared variable between both threads containing the job completion status. No mutex locking is used because the background thread writes to the variable only when the main loop is still running, while the foreground thread only reads it after the main loop has exited. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- tools/virsh-domain.c | 388 +++++++++++++++++++++++++------------------ tools/virsh.h | 3 +- 2 files changed, 232 insertions(+), 159 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 781463f0e2..bf828c90c4 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -23,7 +23,6 @@ #include "virsh-util.h" #include <fcntl.h> -#include <poll.h> #include <signal.h> #include <sys/time.h> @@ -4224,7 +4223,6 @@ doSave(void *opaque) virshCtrlData *data = opaque; vshControl *ctl = data->ctl; const vshCmd *cmd = data->cmd; - char ret = '1'; virDomainPtr dom = NULL; const char *name = NULL; const char *to = NULL; @@ -4269,7 +4267,7 @@ doSave(void *opaque) goto out; } - ret = '0'; + data->ret = 0; out: #ifndef WIN32 @@ -4278,18 +4276,126 @@ doSave(void *opaque) #endif /* !WIN32 */ virshDomainFree(dom); VIR_FREE(xml); - ignore_value(safewrite(data->writefd, &ret, sizeof(ret))); + g_main_loop_quit(data->eventLoop); } typedef void (*jobWatchTimeoutFunc)(vshControl *ctl, virDomainPtr dom, void *opaque); -static bool +struct virshWatchData { + vshControl *ctl; + virDomainPtr dom; + jobWatchTimeoutFunc timeout_func; + void *opaque; + const char *label; + GIOChannel *stdin_ioc; + bool jobStarted; + bool verbose; +}; + +static gboolean +virshWatchTimeout(gpointer opaque) +{ + struct virshWatchData *data = opaque; + + /* suspend the domain when migration timeouts. */ + vshDebug(data->ctl, VSH_ERR_DEBUG, "watchJob: timeout\n"); + if (data->timeout_func) + (data->timeout_func)(data->ctl, data->dom, data->opaque); + + return G_SOURCE_REMOVE; +} + + +static gboolean +virshWatchProgress(gpointer opaque) +{ + struct virshWatchData *data = opaque; + virDomainJobInfo jobinfo; +#ifndef WIN32 + sigset_t sigmask, oldsigmask; + int ret;

+ + sigemptyset(&sigmask); + sigaddset(&sigmask, SIGINT); + + vshDebug(data->ctl, VSH_ERR_DEBUG, "%s", + "watchJob: progress update\n");

+ pthread_sigmask(SIG_BLOCK, &sigmask, &oldsigmask); +#endif /* !WIN32 */ + ret = virDomainGetJobInfo(data->dom, &jobinfo);