This is same as the patch I sent yesterday...
This patch does some simple re-factoring of the way the TTYs and
control socket are handled to reduce the amount of state stored
in the lxc_vm_t structure, in preparation for the switchover to
the generic domain handling APIs.
lxc_conf.c | 1
lxc_conf.h | 12 ---
lxc_container.c | 60 +++++++--------
lxc_container.h | 14 +++
lxc_driver.c | 213 ++++++++++++++++----------------------------------------
5 files changed, 104 insertions(+), 196 deletions(-)
Daniel
diff -r 63b8398c302e src/lxc_conf.c
--- a/src/lxc_conf.c Mon Jul 14 12:18:23 2008 +0100
+++ b/src/lxc_conf.c Tue Jul 15 11:55:48 2008 +0100
@@ -1041,7 +1041,6 @@
void lxcFreeVM(lxc_vm_t *vm)
{
lxcFreeVMDef(vm->def);
- VIR_FREE(vm->containerTty);
VIR_FREE(vm);
}
diff -r 63b8398c302e src/lxc_conf.h
--- a/src/lxc_conf.h Mon Jul 14 12:18:23 2008 +0100
+++ b/src/lxc_conf.h Tue Jul 15 11:55:48 2008 +0100
@@ -35,12 +35,6 @@
#define LXC_MAX_XML_LENGTH 16384
#define LXC_MAX_ERROR_LEN 1024
#define LXC_DOMAIN_TYPE "lxc"
-#define LXC_PARENT_SOCKET 0
-#define LXC_CONTAINER_SOCKET 1
-
-/* messages between parent and container */
-typedef char lxc_message_t;
-#define LXC_CONTINUE_MSG 'c'
/* types of networks for containers */
enum lxc_net_type {
@@ -98,12 +92,6 @@
char configFileBase[PATH_MAX];
char ttyPidFile[PATH_MAX];
-
- int parentTty;
- int containerTtyFd;
- char *containerTty;
-
- int sockpair[2];
lxc_vm_def_t *def;
diff -r 63b8398c302e src/lxc_container.c
--- a/src/lxc_container.c Mon Jul 14 12:18:23 2008 +0100
+++ b/src/lxc_container.c Tue Jul 15 11:55:48 2008 +0100
@@ -33,7 +33,6 @@
#include <unistd.h>
#include "lxc_container.h"
-#include "lxc_conf.h"
#include "util.h"
#include "memory.h"
#include "veth.h"
@@ -83,10 +82,11 @@
*
* Returns 0 on success or -1 in case of error
*/
-static int lxcSetContainerStdio(const char *ttyName)
+static int lxcSetContainerStdio(const char *ttyPath)
{
int rc = -1;
int ttyfd;
+ int open_max, i;
if (setsid() < 0) {
lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
@@ -94,10 +94,10 @@
goto error_out;
}
- ttyfd = open(ttyName, O_RDWR|O_NOCTTY);
+ ttyfd = open(ttyPath, O_RDWR|O_NOCTTY);
if (ttyfd < 0) {
lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _("open(%s) failed: %s"), ttyName, strerror(errno));
+ _("open(%s) failed: %s"), ttyPath, strerror(errno));
goto error_out;
}
@@ -107,7 +107,12 @@
goto cleanup;
}
- close(0); close(1); close(2);
+ /* Just in case someone forget to set FD_CLOEXEC, explicitly
+ * close all FDs before executing the container */
+ open_max = sysconf (_SC_OPEN_MAX);
+ for (i = 0; i < open_max; i++)
+ if (i != ttyfd)
+ close(i);
if (dup2(ttyfd, 0) < 0) {
lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
@@ -144,12 +149,11 @@
*
* Returns 0 on success or -1 in case of error
*/
-static int lxcExecWithTty(lxc_vm_t *vm)
+static int lxcExecWithTty(lxc_vm_def_t *vmDef, char *ttyPath)
{
int rc = -1;
- lxc_vm_def_t *vmDef = vm->def;
- if(lxcSetContainerStdio(vm->containerTty) < 0) {
+ if(lxcSetContainerStdio(ttyPath) < 0) {
goto exit_with_error;
}
@@ -161,7 +165,7 @@
/**
* lxcWaitForContinue:
- * @vm: Pointer to vm structure
+ * @monitor: monitor FD from parent
*
* This function will wait for the container continue message from the
* parent process. It will send this message on the socket pair stored in
@@ -169,31 +173,23 @@
*
* Returns 0 on success or -1 in case of error
*/
-static int lxcWaitForContinue(lxc_vm_t *vm)
+static int lxcWaitForContinue(int monitor)
{
- int rc = -1;
lxc_message_t msg;
int readLen;
- readLen = saferead(vm->sockpair[LXC_CONTAINER_SOCKET], &msg, sizeof(msg));
- if (readLen != sizeof(msg)) {
+ readLen = saferead(monitor, &msg, sizeof(msg));
+ if (readLen != sizeof(msg) ||
+ msg != LXC_CONTINUE_MSG) {
lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("Failed to read the container continue message: %s"),
strerror(errno));
- goto error_out;
+ return -1;
}
DEBUG0("Received container continue message");
- close(vm->sockpair[LXC_PARENT_SOCKET]);
- vm->sockpair[LXC_PARENT_SOCKET] = -1;
- close(vm->sockpair[LXC_CONTAINER_SOCKET]);
- vm->sockpair[LXC_CONTAINER_SOCKET] = -1;
-
- rc = 0;
-
-error_out:
- return rc;
+ return 0;
}
/**
@@ -204,12 +200,12 @@
*
* Returns 0 on success or nonzero in case of error
*/
-static int lxcEnableInterfaces(const lxc_vm_t *vm)
+static int lxcEnableInterfaces(const lxc_vm_def_t *def)
{
int rc = 0;
const lxc_net_def_t *net;
- for (net = vm->def->nets; net; net = net->next) {
+ for (net = def->nets; net; net = net->next) {
DEBUG("Enabling %s", net->containerVeth);
rc = vethInterfaceUpOrDown(net->containerVeth, 1);
if (0 != rc) {
@@ -218,7 +214,7 @@
}
/* enable lo device only if there were other net devices */
- if (vm->def->nets)
+ if (def->nets)
rc = vethInterfaceUpOrDown("lo", 1);
error_out:
@@ -237,11 +233,11 @@
*
* Returns 0 on success or -1 in case of error
*/
-int lxcChild( void *argv )
+int lxcChild( void *data )
{
int rc = -1;
- lxc_vm_t *vm = (lxc_vm_t *)argv;
- lxc_vm_def_t *vmDef = vm->def;
+ lxc_child_argv_t *argv = data;
+ lxc_vm_def_t *vmDef = argv->config;
lxc_mount_t *curMount;
int i;
@@ -278,16 +274,16 @@
}
/* Wait for interface devices to show up */
- if (0 != (rc = lxcWaitForContinue(vm))) {
+ if (0 != (rc = lxcWaitForContinue(argv->monitor))) {
goto cleanup;
}
/* enable interfaces */
- if (0 != (rc = lxcEnableInterfaces(vm))) {
+ if (0 != (rc = lxcEnableInterfaces(vmDef))) {
goto cleanup;
}
- rc = lxcExecWithTty(vm);
+ rc = lxcExecWithTty(vmDef, argv->ttyPath);
/* this function will only return if an error occured */
cleanup:
diff -r 63b8398c302e src/lxc_container.h
--- a/src/lxc_container.h Mon Jul 14 12:18:23 2008 +0100
+++ b/src/lxc_container.h Tue Jul 15 11:55:48 2008 +0100
@@ -24,7 +24,21 @@
#ifndef LXC_CONTAINER_H
#define LXC_CONTAINER_H
+#include "lxc_conf.h"
+
#ifdef WITH_LXC
+
+typedef struct __lxc_child_argv lxc_child_argv_t;
+struct __lxc_child_argv {
+ lxc_vm_def_t *config;
+ int monitor;
+ char *ttyPath;
+};
+
+/* messages between parent and container */
+typedef char lxc_message_t;
+#define LXC_CONTINUE_MSG 'c'
+
/* Function declarations */
int lxcChild( void *argv );
diff -r 63b8398c302e src/lxc_driver.c
--- a/src/lxc_driver.c Mon Jul 14 12:18:23 2008 +0100
+++ b/src/lxc_driver.c Tue Jul 15 11:55:48 2008 +0100
@@ -561,27 +561,23 @@
/**
* lxcSendContainerContinue:
- * @vm: pointer to virtual machine structure
+ * @monitor: FD for communicating with child
*
* Sends the continue message via the socket pair stored in the vm
* structure.
*
* Returns 0 on success or -1 in case of error
*/
-static int lxcSendContainerContinue(const lxc_vm_t *vm)
+static int lxcSendContainerContinue(virConnectPtr conn,
+ int monitor)
{
int rc = -1;
lxc_message_t msg = LXC_CONTINUE_MSG;
int writeCount = 0;
- if (NULL == vm) {
- goto error_out;
- }
-
- writeCount = safewrite(vm->sockpair[LXC_PARENT_SOCKET], &msg,
- sizeof(msg));
+ writeCount = safewrite(monitor, &msg, sizeof(msg));
if (writeCount != sizeof(msg)) {
- lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
_("unable to send container continue message: %s"),
strerror(errno));
goto error_out;
@@ -605,12 +601,15 @@
*/
static int lxcStartContainer(virConnectPtr conn,
lxc_driver_t* driver,
- lxc_vm_t *vm)
+ lxc_vm_t *vm,
+ int monitor,
+ char *ttyPath)
{
int rc = -1;
int flags;
int stacksize = getpagesize() * 4;
char *stack, *stacktop;
+ lxc_child_argv_t args = { vm->def, monitor, ttyPath };
/* allocate a stack for the container */
if (VIR_ALLOC_N(stack, stacksize) < 0) {
@@ -625,7 +624,7 @@
if (vm->def->nets != NULL)
flags |= CLONE_NEWNET;
- vm->def->id = clone(lxcChild, stacktop, flags, (void *)vm);
+ vm->def->id = clone(lxcChild, stacktop, flags, &args);
DEBUG("clone() returned, %d", vm->def->id);
@@ -643,117 +642,9 @@
return rc;
}
-/**
- * lxcPutTtyInRawMode:
- * @conn: pointer to connection
- * @ttyDev: file descriptor for tty
- *
- * Sets tty attributes via cfmakeraw()
- *
- * Returns 0 on success or -1 in case of error
- */
-static int lxcPutTtyInRawMode(virConnectPtr conn, int ttyDev)
-{
- int rc = -1;
-
- struct termios ttyAttr;
-
- if (tcgetattr(ttyDev, &ttyAttr) < 0) {
- lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
- "tcgetattr() failed: %s", strerror(errno));
- goto cleanup;
- }
-
- cfmakeraw(&ttyAttr);
-
- if (tcsetattr(ttyDev, TCSADRAIN, &ttyAttr) < 0) {
- lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
- "tcsetattr failed: %s", strerror(errno));
- goto cleanup;
- }
-
- rc = 0;
-
-cleanup:
- return rc;
-}
/**
- * lxcSetupTtyTunnel:
- * @conn: pointer to connection
- * @vmDef: pointer to virtual machine definition structure
- * @ttyDev: pointer to int. On success will be set to fd for master
- * end of tty
- *
- * Opens and configures the parent side tty
- *
- * Returns 0 on success or -1 in case of error
- */
-static int lxcSetupTtyTunnel(virConnectPtr conn,
- lxc_vm_def_t *vmDef,
- int* ttyDev)
-{
- int rc = -1;
- char *ptsStr;
-
- if (0 < strlen(vmDef->tty)) {
- *ttyDev = posix_openpt(O_RDWR|O_NOCTTY|O_NONBLOCK);
- if (*ttyDev < 0) {
- lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
- "open() tty failed: %s", strerror(errno));
- goto setup_complete;
- }
-
- rc = grantpt(*ttyDev);
- if (rc < 0) {
- lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
- "grantpt() failed: %s", strerror(errno));
- goto setup_complete;
- }
-
- rc = unlockpt(*ttyDev);
- if (rc < 0) {
- lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
- "unlockpt() failed: %s", strerror(errno));
- goto setup_complete;
- }
-
- /* get the name and print it to stdout */
- ptsStr = ptsname(*ttyDev);
- if (ptsStr == NULL) {
- lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
- "ptsname() failed");
- goto setup_complete;
- }
- /* This value needs to be stored in the container configuration file */
- VIR_FREE(vmDef->tty);
- if (!(vmDef->tty = strdup(ptsStr))) {
- lxcError(conn, NULL, VIR_ERR_NO_MEMORY,
- _("unable to get storage for vm tty name"));
- goto setup_complete;
- }
-
- /* Enter raw mode, so all characters are passed directly to child */
- if (lxcPutTtyInRawMode(conn, *ttyDev) < 0) {
- goto setup_complete;
- }
-
- } else {
- *ttyDev = -1;
- }
-
- rc = 0;
-
-setup_complete:
- if((0 != rc) && (*ttyDev > 0)) {
- close(*ttyDev);
- }
-
- return rc;
-}
-
-/**
- * lxcSetupContainerTty:
+ * lxcOpenTty:
* @conn: pointer to connection
* @ttymaster: pointer to int. On success, set to fd for master end
* @ttyName: On success, will point to string slave end of tty. Caller
@@ -763,12 +654,12 @@
*
* Returns 0 on success or -1 in case of error
*/
-static int lxcSetupContainerTty(virConnectPtr conn,
- int *ttymaster,
- char **ttyName)
+static int lxcOpenTty(virConnectPtr conn,
+ int *ttymaster,
+ char **ttyName,
+ int rawmode)
{
int rc = -1;
- char tempTtyName[PATH_MAX];
*ttymaster = posix_openpt(O_RDWR|O_NOCTTY|O_NONBLOCK);
if (*ttymaster < 0) {
@@ -783,27 +674,43 @@
goto cleanup;
}
- if (0 != ptsname_r(*ttymaster, tempTtyName, sizeof(tempTtyName))) {
- lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
- _("ptsname_r failed: %s"), strerror(errno));
- goto cleanup;
+ if (rawmode) {
+ struct termios ttyAttr;
+ if (tcgetattr(*ttymaster, &ttyAttr) < 0) {
+ lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
+ "tcgetattr() failed: %s", strerror(errno));
+ goto cleanup;
+ }
+
+ cfmakeraw(&ttyAttr);
+
+ if (tcsetattr(*ttymaster, TCSADRAIN, &ttyAttr) < 0) {
+ lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
+ "tcsetattr failed: %s", strerror(errno));
+ goto cleanup;
+ }
}
- if (VIR_ALLOC_N(*ttyName, strlen(tempTtyName) + 1) < 0) {
- lxcError(conn, NULL, VIR_ERR_NO_MEMORY,
- _("unable to allocate container name string"));
- goto cleanup;
+ if (ttyName) {
+ char tempTtyName[PATH_MAX];
+ if (0 != ptsname_r(*ttymaster, tempTtyName, sizeof(tempTtyName))) {
+ lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("ptsname_r failed: %s"), strerror(errno));
+ goto cleanup;
+ }
+
+ if ((*ttyName = strdup(tempTtyName)) == NULL) {
+ lxcError(conn, NULL, VIR_ERR_NO_MEMORY, NULL);
+ goto cleanup;
+ }
}
-
- strcpy(*ttyName, tempTtyName);
rc = 0;
cleanup:
- if (0 != rc) {
- if (-1 != *ttymaster) {
- close(*ttymaster);
- }
+ if (rc != 0 &&
+ *ttymaster != -1) {
+ close(*ttymaster);
}
return rc;
@@ -989,15 +896,18 @@
lxc_vm_t * vm)
{
int rc = -1;
- lxc_vm_def_t *vmDef = vm->def;
+ int sockpair[2];
+ int containerTty, parentTty;
+ char *containerTtyPath = NULL;
/* open parent tty */
- if (lxcSetupTtyTunnel(conn, vmDef, &vm->parentTty) < 0) {
+ VIR_FREE(vm->def->tty);
+ if (lxcOpenTty(conn, &parentTty, &vm->def->tty, 1) < 0) {
goto cleanup;
}
/* open container tty */
- if (lxcSetupContainerTty(conn, &(vm->containerTtyFd),
&(vm->containerTty)) < 0) {
+ if (lxcOpenTty(conn, &containerTty, &containerTtyPath, 0) < 0) {
goto cleanup;
}
@@ -1011,15 +921,15 @@
if (vm->pid == 0) {
/* child process calls forward routine */
- lxcTtyForward(vm->parentTty, vm->containerTtyFd);
+ lxcTtyForward(parentTty, containerTty);
}
if (lxcStoreTtyPid(driver, vm)) {
DEBUG0("unable to store tty pid");
}
- close(vm->parentTty);
- close(vm->containerTtyFd);
+ close(parentTty);
+ close(containerTty);
if (0 != (rc = lxcSetupInterfaces(conn, vm))) {
goto cleanup;
@@ -1027,7 +937,7 @@
/* create a socket pair to send continue message to the container once */
/* we've completed the post clone configuration */
- if (0 != socketpair(PF_UNIX, SOCK_STREAM, 0, vm->sockpair)) {
+ if (0 != socketpair(PF_UNIX, SOCK_STREAM, 0, sockpair)) {
lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
_("sockpair failed: %s"), strerror(errno));
goto cleanup;
@@ -1035,7 +945,9 @@
/* check this rc */
- rc = lxcStartContainer(conn, driver, vm);
+ rc = lxcStartContainer(conn, driver, vm,
+ sockpair[1],
+ containerTtyPath);
if (rc != 0)
goto cleanup;
@@ -1043,7 +955,7 @@
if (rc != 0)
goto cleanup;
- rc = lxcSendContainerContinue(vm);
+ rc = lxcSendContainerContinue(conn, sockpair[0]);
if (rc != 0)
goto cleanup;
@@ -1052,10 +964,9 @@
driver->nactivevms++;
cleanup:
- close(vm->sockpair[LXC_PARENT_SOCKET]);
- vm->sockpair[LXC_PARENT_SOCKET] = -1;
- close(vm->sockpair[LXC_CONTAINER_SOCKET]);
- vm->sockpair[LXC_CONTAINER_SOCKET] = -1;
+ close(sockpair[0]);
+ close(sockpair[1]);
+ VIR_FREE(containerTtyPath);
return rc;
}
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://ovirt.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|