[libvirt] [PATCH 2/2] Cancel migration/dump if user presses Ctrl-C when migration/dump is in progress

[Daniel P. Berrange]

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 at 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