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@xxxxxxxxxx>
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list