On Thu, Dec 23, 2010 at 05:07:34PM +0800, Hu Tao wrote:
--- src/remote/remote_driver.c | 2 +- tools/virsh.c | 126 ++++++++++++++++++++++++++++++++++++++------ 2 files changed, 110 insertions(+), 18 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index ee2de4a..d24d54f 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -10122,7 +10122,7 @@ remoteIOEventLoop(virConnectPtr conn,
repoll: ret = poll(fds, ARRAY_CARDINALITY(fds), -1); - if (ret < 0 && errno == EAGAIN) + if (ret < 0 && (errno == EAGAIN || errno == EINTR)) goto repoll;
I'm not convinced this is a good change. It makes it impossible to interrupt libvirt clients when they're doing remote I/O. Yes, you're adding special SIGINT handling to virsh to provide an alternative way to stop things, but this only applies to migration/dump. So a user won't be able to Ctrl-C virsh on other commands, nor for any non-virsh applications. In essence, this reverts the previous fix: commit 47fec8eac2bb38246addb0a0cc368fdbadad4738 Author: Daniel Veillard <veillard@redhat.com> Date: Fri Oct 30 12:08:26 2009 +0100 Remote code caught EINTR making it ininterruptable John Levon raised the issue that remoteIOEventLoop() poll call was reissued after EINTR was caught making it uninterruptible.
#ifdef HAVE_PTHREAD_SIGMASK diff --git a/tools/virsh.c b/tools/virsh.c index 8c123bb..efc7d1a 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -54,6 +54,9 @@ #include "files.h" #include "../daemon/event.h" #include "configmake.h" +#include "threads.h" +#include "threadpool.h" +#include "datatypes.h"
static char *progname;
@@ -208,6 +211,7 @@ typedef struct __vshCmd { typedef struct __vshControl { char *name; /* connection name */ virConnectPtr conn; /* connection to hypervisor (MAY BE NULL) */ + virDomainPtr dom; vshCmd *cmd; /* the current command */ char *cmdstr; /* string with command */ int imode; /* interactive mode? */ @@ -221,6 +225,9 @@ typedef struct __vshControl { int log_fd; /* log file descriptor */ char *historydir; /* readline history directory name */ char *historyfile; /* readline history file name */ + + virMutex mutex; + virThreadPoolPtr threadPool; } __vshControl;
I'm not really understanding why we need to introduce all this thread pool complexity just to be able to catch Ctrl-C and run virJobAbort ? If we already have a thread running to handle migration, monitoring progress info, then that existing thread could just check a global variable to see whether a sigint has occurred. Regards, Daniel