Jim Meyering wrote:
> Dave Leskovec <dlesko@xxxxxxxxxxxxxxxxxx> wrote:
> ...
>>>> +#ifndef _SIGNAL_H
>>>> +#include <signal.h>
>>>> +#endif
>>> In practice it's fine to include <signal.h> unconditionally,
>>> and even multiple times. Have you encountered a version of <signal.h>
>>> that may not be included twice? If so, it probably deserves a comment
>>> with the details.
>> No, I don't have any special condition here. This is probably some past
>> conditioning resurfacing briefly. If I remember correctly, it had more to do
>> with compile efficiency rather than avoiding compile failures from multiple
>> inclusions.
>
> Then don't bother.
> gcc performs a handy optimization whereby it doesn't even open
> the header file the second (and subsequent) time it's included, as
> long as it's entire contents is wrapped in the usual sort of guard:
>
> #ifndef SYM
> #define SYM
> ...
> #endif
Thanks Jim. I've attached an updated patch with those two changes. While
making these changes, I noticed that I missed updating the storage drivers state
driver table. I've fixed that as well.
--
Best Regards,
Dave Leskovec
IBM Linux Technology Center
Open Virtualization
---
qemud/qemud.c | 26 ++++++----
src/driver.h | 4 +
src/internal.h | 2
src/libvirt.c | 11 ++++
src/libvirt_sym.version | 1
src/lxc_driver.c | 114 +++++++++++++++++++++++++++++++++++-------------
src/qemu_driver.c | 1
src/remote_internal.c | 1
src/storage_driver.c | 1
9 files changed, 120 insertions(+), 41 deletions(-)
Index: b/qemud/qemud.c
===================================================================
--- a/qemud/qemud.c 2008-04-28 02:09:52.000000000 -0700
+++ b/qemud/qemud.c 2008-04-30 15:34:29.000000000 -0700
@@ -109,16 +109,16 @@
static sig_atomic_t sig_errors = 0;
static int sig_lasterrno = 0;
-static void sig_handler(int sig) {
- unsigned char sigc = sig;
+static void sig_handler(int sig, siginfo_t * siginfo,
+ void* context ATTRIBUTE_UNUSED) {
int origerrno;
int r;
- if (sig == SIGCHLD) /* We explicitly waitpid the child later */
- return;
+ /* set the sig num in the struct */
+ siginfo->si_signo = sig;
origerrno = errno;
- r = safewrite(sigwrite, &sigc, 1);
+ r = safewrite(sigwrite, siginfo, sizeof(*siginfo));
if (r == -1) {
sig_errors++;
sig_lasterrno = errno;
@@ -232,10 +232,10 @@
int events ATTRIBUTE_UNUSED,
void *opaque) {
struct qemud_server *server = (struct qemud_server *)opaque;
- unsigned char sigc;
+ siginfo_t siginfo;
int ret;
- if (read(server->sigread, &sigc, 1) != 1) {
+ if (read(server->sigread, &siginfo, sizeof(siginfo)) != sizeof(siginfo)) {
qemudLog(QEMUD_ERR, _("Failed to read from signal pipe: %s"),
strerror(errno));
return;
@@ -243,7 +243,7 @@
ret = 0;
- switch (sigc) {
+ switch (siginfo.si_signo) {
case SIGHUP:
qemudLog(QEMUD_INFO, "%s", _("Reloading configuration on SIGHUP"));
if (virStateReload() < 0)
@@ -253,11 +253,15 @@
case SIGINT:
case SIGQUIT:
case SIGTERM:
- qemudLog(QEMUD_WARN, _("Shutting down on signal %d"), sigc);
+ qemudLog(QEMUD_WARN, _("Shutting down on signal %d"),
+ siginfo.si_signo);
server->shutdown = 1;
break;
default:
+ qemudLog(QEMUD_INFO, _("Received signal %d, dispatching to drivers"),
+ siginfo.si_signo);
+ virStateSigDispatcher(&siginfo);
break;
}
@@ -2186,8 +2190,8 @@
goto error1;
}
- sig_action.sa_handler = sig_handler;
- sig_action.sa_flags = 0;
+ sig_action.sa_sigaction = sig_handler;
+ sig_action.sa_flags = SA_SIGINFO;
sigemptyset(&sig_action.sa_mask);
sigaction(SIGHUP, &sig_action, NULL);
Index: b/src/driver.h
===================================================================
--- a/src/driver.h 2008-04-10 09:54:54.000000000 -0700
+++ b/src/driver.h 2008-05-05 23:28:40.000000000 -0700
@@ -11,6 +11,8 @@
#include <libxml/uri.h>
+#include <signal.h>
+
#ifdef __cplusplus
extern "C" {
#endif
@@ -565,6 +567,7 @@
typedef int (*virDrvStateCleanup) (void);
typedef int (*virDrvStateReload) (void);
typedef int (*virDrvStateActive) (void);
+typedef int (*virDrvSigHandler) (siginfo_t *siginfo);
typedef struct _virStateDriver virStateDriver;
typedef virStateDriver *virStateDriverPtr;
@@ -574,6 +577,7 @@
virDrvStateCleanup cleanup;
virDrvStateReload reload;
virDrvStateActive active;
+ virDrvSigHandler sigHandler;
};
/*
Index: b/src/internal.h
===================================================================
--- a/src/internal.h 2008-04-28 14:44:54.000000000 -0700
+++ b/src/internal.h 2008-04-30 15:01:24.000000000 -0700
@@ -344,10 +344,12 @@
int __virStateCleanup(void);
int __virStateReload(void);
int __virStateActive(void);
+int __virStateSigDispatcher(siginfo_t *siginfo);
#define virStateInitialize() __virStateInitialize()
#define virStateCleanup() __virStateCleanup()
#define virStateReload() __virStateReload()
#define virStateActive() __virStateActive()
+#define virStateSigDispatcher(s) __virStateSigDispatcher(s)
int __virDrvSupportsFeature (virConnectPtr conn, int feature);
Index: b/src/libvirt.c
===================================================================
--- a/src/libvirt.c 2008-04-10 09:54:54.000000000 -0700
+++ b/src/libvirt.c 2008-04-30 15:01:24.000000000 -0700
@@ -607,6 +607,17 @@
return ret;
}
+int __virStateSigDispatcher(siginfo_t *siginfo) {
+ int i, ret = 0;
+
+ for (i = 0 ; i < virStateDriverTabCount ; i++) {
+ if (virStateDriverTab[i]->sigHandler &&
+ virStateDriverTab[i]->sigHandler(siginfo))
+ ret = 1;
+ }
+ return ret;
+}
+
/**
Index: b/src/libvirt_sym.version
===================================================================
--- a/src/libvirt_sym.version 2008-04-28 08:14:59.000000000 -0700
+++ b/src/libvirt_sym.version 2008-04-30 15:01:24.000000000 -0700
@@ -170,6 +170,7 @@
__virStateCleanup;
__virStateReload;
__virStateActive;
+ __virStateSigDispatcher;
__virDrvSupportsFeature;
Index: b/src/lxc_driver.c
===================================================================
--- a/src/lxc_driver.c 2008-04-10 09:53:29.000000000 -0700
+++ b/src/lxc_driver.c 2008-05-05 23:21:56.000000000 -0700
@@ -841,55 +841,42 @@
}
/**
- * lxcDomainDestroy:
- * @dom: Ptr to domain to destroy
+ * lxcVmCleanup:
+ * @vm: Ptr to VM to clean up
*
- * Sends SIGKILL to container root process to terminate the container
+ * waitpid() on the container process. kill and wait the tty process
+ * This is called by boh lxcDomainDestroy and lxcSigHandler when a
+ * container exits.
*
* Returns 0 on success or -1 in case of error
*/
-static int lxcDomainDestroy(virDomainPtr dom)
+static int lxcVMCleanup(lxc_driver_t *driver, lxc_vm_t * vm)
{
int rc = -1;
int waitRc;
- lxc_driver_t *driver = (lxc_driver_t*)dom->conn->privateData;
- lxc_vm_t *vm = lxcFindVMByID(driver, dom->id);
- int childStatus;
-
- if (!vm) {
- lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN,
- _("no domain with id %d"), dom->id);
- goto error_out;
- }
-
- if (0 > (kill(vm->def->id, SIGKILL))) {
- if (ESRCH != errno) {
- lxcError(dom->conn, dom, VIR_ERR_INTERNAL_ERROR,
- _("sending SIGKILL failed: %s"), strerror(errno));
-
- goto error_out;
- }
- }
-
- vm->state = VIR_DOMAIN_SHUTDOWN;
+ int childStatus = -1;
while (((waitRc = waitpid(vm->def->id, &childStatus, 0)) == -1) &&
errno == EINTR);
if (waitRc != vm->def->id) {
- lxcError(dom->conn, dom, VIR_ERR_INTERNAL_ERROR,
+ lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("waitpid failed to wait for container %d: %d %s"),
vm->def->id, waitRc, strerror(errno));
goto kill_tty;
}
- rc = WEXITSTATUS(childStatus);
- DEBUG("container exited with rc: %d", rc);
+ rc = 0;
+
+ if (WIFEXITED(childStatus)) {
+ rc = WEXITSTATUS(childStatus);
+ DEBUG("container exited with rc: %d", rc);
+ }
kill_tty:
if (0 > (kill(vm->pid, SIGKILL))) {
if (ESRCH != errno) {
- lxcError(dom->conn, dom, VIR_ERR_INTERNAL_ERROR,
+ lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("sending SIGKILL to tty process failed: %s"),
strerror(errno));
@@ -901,7 +888,7 @@
errno == EINTR);
if (waitRc != vm->pid) {
- lxcError(dom->conn, dom, VIR_ERR_INTERNAL_ERROR,
+ lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("waitpid failed to wait for tty %d: %d %s"),
vm->pid, waitRc, strerror(errno));
}
@@ -912,8 +899,43 @@
vm->def->id = -1;
driver->nactivevms--;
driver->ninactivevms++;
+ lxcSaveConfig(NULL, driver, vm, vm->def);
- rc = 0;
+ return rc;
+ }
+
+/**
+ * lxcDomainDestroy:
+ * @dom: Ptr to domain to destroy
+ *
+ * Sends SIGKILL to container root process to terminate the container
+ *
+ * Returns 0 on success or -1 in case of error
+ */
+static int lxcDomainDestroy(virDomainPtr dom)
+{
+ int rc = -1;
+ lxc_driver_t *driver = (lxc_driver_t*)dom->conn->privateData;
+ lxc_vm_t *vm = lxcFindVMByID(driver, dom->id);
+
+ if (!vm) {
+ lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN,
+ _("no domain with id %d"), dom->id);
+ goto error_out;
+ }
+
+ if (0 > (kill(vm->def->id, SIGKILL))) {
+ if (ESRCH != errno) {
+ lxcError(dom->conn, dom, VIR_ERR_INTERNAL_ERROR,
+ _("sending SIGKILL failed: %s"), strerror(errno));
+
+ goto error_out;
+ }
+ }
+
+ vm->state = VIR_DOMAIN_SHUTDOWN;
+
+ rc = lxcVMCleanup(driver, vm);
error_out:
return rc;
@@ -993,6 +1015,37 @@
return 0;
}
+/**
+ * lxcSigHandler:
+ * @siginfo: Pointer to siginfo_t structure
+ *
+ * Handles signals received by libvirtd. Currently this is used to
+ * catch SIGCHLD from an exiting container.
+ *
+ * Returns 0 on success or -1 in case of error
+ */
+static int lxcSigHandler(siginfo_t *siginfo)
+{
+ int rc = -1;
+ lxc_vm_t *vm;
+
+ if (siginfo->si_signo == SIGCHLD) {
+ vm = lxcFindVMByID(lxc_driver, siginfo->si_pid);
+
+ if (NULL == vm) {
+ DEBUG("Ignoring SIGCHLD from non-container process %d\n",
+ siginfo->si_pid);
+ goto cleanup;
+ }
+
+ rc = lxcVMCleanup(lxc_driver, vm);
+
+ }
+
+cleanup:
+ return rc;
+}
+
/* Function Tables */
static virDriver lxcDriver = {
@@ -1061,6 +1114,7 @@
lxcShutdown,
NULL, /* reload */
lxcActive,
+ lxcSigHandler
};
int lxcRegister(void)
Index: b/src/qemu_driver.c
===================================================================
--- a/src/qemu_driver.c 2008-04-25 13:46:13.000000000 -0700
+++ b/src/qemu_driver.c 2008-04-30 15:01:24.000000000 -0700
@@ -3133,6 +3133,7 @@
qemudShutdown,
qemudReload,
qemudActive,
+ NULL
};
int qemudRegister(void) {
Index: b/src/remote_internal.c
===================================================================
--- a/src/remote_internal.c 2008-04-29 12:43:57.000000000 -0700
+++ b/src/remote_internal.c 2008-05-05 23:17:43.000000000 -0700
@@ -4794,6 +4794,7 @@
NULL,
NULL,
NULL,
+ NULL
};
Index: b/src/storage_driver.c
===================================================================
--- a/src/storage_driver.c 2008-04-18 01:33:23.000000000 -0700
+++ b/src/storage_driver.c 2008-05-05 23:35:34.000000000 -0700
@@ -1245,6 +1245,7 @@
storageDriverShutdown,
storageDriverReload,
storageDriverActive,
+ NULL
};
int storageRegister(void) {
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list