Re: [patch 8/9] Implement better signal handling

[Mark McLoughlin <markmc@xxxxxxxxxx>]

On Fri, 2007-02-16 at 19:08 +0000, Daniel P. Berrange wrote:
> On Fri, Feb 16, 2007 at 02:44:46PM +0000, Mark McLoughlin wrote:
> > Handle SIGHUP by shutting down all guests and networks
> > and re-loading configs
> 
> This violates the 'principle of least surprise'. I certainly do not expect
> a config file reload to terminate all active guests. Since QEMU is a full
> virt system, there is no graceful shutdown process, and so this is equivalent
> to ripping the power cable out of all your VMs. IMHO this makes SIGHUP
> essentially useless.
> 
> We already have the ability to store a secondary config against any active
> guest VMs, which will be automatically activated upon next boot of the 
> guest - this is how we let virDefineDomain()  override the config of an
> existing VM.  IMHO, sending SIGHUP to the daemon should just scan for
> config files and if any VMs are active, load into the secondary config.

	All good points, fixed now with the attached patch.

> > Handle SIGTERM/SIGINT by cleanly shutting down
> 
> Ack, this is desirable. We should also hook in SIGQUIT

	Yep, done.

Cheers,
Mark.

Index: libvirt/qemud/qemud.c
===================================================================
--- libvirt.orig/qemud/qemud.c	2007-02-17 13:14:04.000000000 +0000
+++ libvirt.orig/qemud/qemud.c	2007-02-17 13:14:04.000000000 +0000
@@ -81,50 +81,11 @@ static int qemudDispatchSignal(struct qe
         return -1;
     }
 
-    qemudLog(QEMUD_INFO, "Received signal %d; shuting down guests and "
-             "networks and purging config", sigc);
-
-    /* shutdown active VMs */
-    vm = server->activevms;
-    while (vm) {
-        struct qemud_vm *next = vm->next;
-        qemudShutdownVMDaemon(server, vm);
-        vm = next;
-    }
-
-    /* free inactive VMs */
-    vm = server->inactivevms;
-    while (vm) {
-        struct qemud_vm *next = vm->next;
-        qemudFreeVM(vm);
-        vm = next;
-    }
-    server->inactivevms = NULL;
-    server->ninactivevms = 0;
-
-    /* shutdown active networks */
-    network = server->activenetworks;
-    while (network) {
-        struct qemud_network *next = network->next;
-        qemudShutdownNetworkDaemon(server, network);
-        network = next;
-    }
-
-    /* free inactive networks */
-    network = server->inactivenetworks;
-    while (network) {
-        struct qemud_network *next = network->next;
-        qemudFreeNetwork(network);
-        network = next;
-    }
-    server->inactivenetworks = NULL;
-    server->ninactivenetworks = 0;
-
     ret = 0;
 
     switch (sigc) {
     case SIGHUP:
-        qemudLog(QEMUD_INFO, "Reloading configuration");
+        qemudLog(QEMUD_INFO, "Reloading configuration on SIGHUP");
         ret = qemudScanConfigs(server);
         break;
 
@@ -132,6 +93,43 @@ static int qemudDispatchSignal(struct qe
     case SIGQUIT:
     case SIGTERM:
         qemudLog(QEMUD_WARN, "Shutting down on signal %d", sigc);
+
+        /* shutdown active VMs */
+        vm = server->activevms;
+        while (vm) {
+            struct qemud_vm *next = vm->next;
+            qemudShutdownVMDaemon(server, vm);
+            vm = next;
+        }
+
+        /* free inactive VMs */
+        vm = server->inactivevms;
+        while (vm) {
+            struct qemud_vm *next = vm->next;
+            qemudFreeVM(vm);
+            vm = next;
+        }
+        server->inactivevms = NULL;
+        server->ninactivevms = 0;
+
+        /* shutdown active networks */
+        network = server->activenetworks;
+        while (network) {
+            struct qemud_network *next = network->next;
+            qemudShutdownNetworkDaemon(server, network);
+            network = next;
+        }
+
+        /* free inactive networks */
+        network = server->inactivenetworks;
+        while (network) {
+            struct qemud_network *next = network->next;
+            qemudFreeNetwork(network);
+            network = next;
+        }
+        server->inactivenetworks = NULL;
+        server->ninactivenetworks = 0;
+
         server->shutdown = 1;
         break;
 

Index: libvirt/qemud/qemud.c
===================================================================
--- libvirt.orig/qemud/qemud.c	2007-02-16 18:30:55.000000000 +0000
+++ libvirt.orig/qemud/qemud.c	2007-02-16 18:30:55.000000000 +0000
@@ -129,6 +129,7 @@ static int qemudDispatchSignal(struct qe
         break;
 
     case SIGINT:
+    case SIGQUIT:
     case SIGTERM:
         qemudLog(QEMUD_WARN, "Shutting down on signal %d", sigc);
         server->shutdown = 1;
@@ -1610,6 +1611,7 @@ int main(int argc, char **argv) {
 
     sigaction(SIGHUP, &sig_action, NULL);
     sigaction(SIGINT, &sig_action, NULL);
+    sigaction(SIGQUIT, &sig_action, NULL);
     sigaction(SIGTERM, &sig_action, NULL);
     sigaction(SIGCHLD, &sig_action, NULL);