Re: [libvirt] [PATCH 2/2] Cancel migration/dump if user presses Ctrl-C when migration/dump is in progress
On 12/23/2010 02:07 AM, Hu Tao wrote:
---
No commit message beyond the title?
@@ -208,6 +211,7 @@ typedef struct __vshCmd {
typedef struct __vshControl {
char *name; /* connection name */
virConnectPtr conn; /* connection to hypervisor (MAY BE NULL) */
+ virDomainPtr dom;
No comment as to it's purpose?
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;
Likewise.
@@ -509,6 +518,44 @@ static void vshCatchDisconnect(int sig,
siginfo_t * siginfo,
if ((sig == SIGPIPE) || (siginfo->si_signo == SIGPIPE))
disconnected++;
}
+#endif
+
+#ifdef SIGINT
Unnecessary ifdef - SIGINT is available on all platforms (it is required
by C).
+static void vshCatchInt(int sig, siginfo_t *siginfo,
+ void *context ATTRIBUTE_UNUSED) {
+ vshControl *ctl = &_ctl;
+ vshCmd *cmds, *c;
+ char *cmdStr = NULL;
+
+ virMutexLock(&ctl->mutex);
+ if (!ctl->dom)
+ goto error;
+
+ if (virAsprintf(&cmdStr, "domjobabort %s", ctl->dom->name) <
0)
_BIG_ no-no. virAsprintf calls malloc(), which is NOT signal-safe (that
is, if the signal arrived while some other thread was in the middle of a
malloc(), you've just caused deadlock). The only safe thing to do in a
signal handler is to set some state variables that are then checked in
the main processing loop, when you are back in a safe state to act on a
(minimally-delayed) basis on the signal.
See how daemon/libvirtd.c uses qemudDispatchSignalEvent (although that's
being rewritten by danpb's factorization into a cleaner rpc
server/client model).
+ goto error;
+
+ if ((sig == SIGINT) || (siginfo->si_signo == SIGINT)) {
+ if (!vshConnectionUsability (ctl, ctl->conn))
+ goto error;
+
+ vshCommandStringParse(ctl, cmdStr);
+ cmds = ctl->cmd;
+ ctl->cmd = NULL;
+ virMutexUnlock(&ctl->mutex);
+ do {
+ c = cmds;
+ cmds = cmds->next;
+ c->next = NULL;
+ ignore_value(virThreadPoolSendJob(ctl->threadPool, c, true));
Continuing in the vein of bad practice - blocking on another thread to
complete inside a signal handler is crazy. But setting up a threadpool
to handle signals in a separate thread means that your signal-handling
thread is already independently available to cancel the job that is
underway in the primary thread.
+ } while (cmds);
+ }
+ VIR_FREE(cmdStr);
+ return;
+error:
+ virMutexUnlock(&ctl->mutex);
+ VIR_FREE(cmdStr);
+}
+#endif
/*
* vshSetupSignals:
@@ -520,17 +567,20 @@ static void
vshSetupSignals(void) {
struct sigaction sig_action;
- sig_action.sa_sigaction = vshCatchDisconnect;
sig_action.sa_flags = SA_SIGINFO;
sigemptyset(&sig_action.sa_mask);
+#ifdef SIGPIPE
Might be unnecessary - gnulib guarantees that a working replacement for
SIGPIPE is available on mingw, which is the only platform that lacks it
natively. Oh, but that module is currently LGPLv3+, I'll have to ask on
the gnulib list if people are willing to relax it to LGPLv2+ so we can
use it.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)