I've been working on getting LXC ready to use the common XML routines. One of the first tasks is finding a way to remove some of the state stored in the lxc_vm_t struct, so it can be switched over to virDomainObj struct instead. This patch does some simple re-factoring of the way the TTYs and control socket are handled to prepare for the switchover. lxc_conf.c | 1 lxc_conf.h | 10 -- lxc_container.c | 61 +++++++--------- lxc_container.h | 14 +++ lxc_driver.c | 211 ++++++++++++++++---------------------------------------- 5 files changed, 103 insertions(+), 194 deletions(-) Daniel diff -r 6273f6456bd2 src/lxc_conf.c --- a/src/lxc_conf.c Sun Jul 13 10:33:25 2008 +0100 +++ b/src/lxc_conf.c Sun Jul 13 11:45:04 2008 +0100 @@ -1041,7 +1041,6 @@ void lxcFreeVM(lxc_vm_t *vm) { lxcFreeVMDef(vm->def); - VIR_FREE(vm->containerTty); VIR_FREE(vm); } diff -r 6273f6456bd2 src/lxc_conf.h --- a/src/lxc_conf.h Sun Jul 13 10:33:25 2008 +0100 +++ b/src/lxc_conf.h Sun Jul 13 11:45:04 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 { @@ -100,10 +94,6 @@ char ttyPidFile[PATH_MAX]; int parentTty; - int containerTtyFd; - char *containerTty; - - int sockpair[2]; lxc_vm_def_t *def; diff -r 6273f6456bd2 src/lxc_container.c --- a/src/lxc_container.c Sun Jul 13 10:33:25 2008 +0100 +++ b/src/lxc_container.c Sun Jul 13 11:45:04 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,7 +82,7 @@ * * 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; @@ -94,10 +93,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; } @@ -106,8 +105,6 @@ _("ioctl(TIOCSTTY) failed: %s"), strerror(errno)); goto cleanup; } - - close(0); close(1); close(2); if (dup2(ttyfd, 0) < 0) { lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, @@ -144,12 +141,18 @@ * * 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; + int open_max, i; - if(lxcSetContainerStdio(vm->containerTty) < 0) { + /* 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++) + close(i); + + if(lxcSetContainerStdio(ttyPath) < 0) { goto exit_with_error; } @@ -161,7 +164,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 +172,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 +199,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 +213,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 +232,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 +273,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 6273f6456bd2 src/lxc_container.h --- a/src/lxc_container.h Sun Jul 13 10:33:25 2008 +0100 +++ b/src/lxc_container.h Sun Jul 13 11:45:04 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 6273f6456bd2 src/lxc_driver.c --- a/src/lxc_driver.c Sun Jul 13 10:33:25 2008 +0100 +++ b/src/lxc_driver.c Sun Jul 13 11:45:04 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; + char *containerTtyPath = NULL; /* open parent tty */ - if (lxcSetupTtyTunnel(conn, vmDef, &vm->parentTty) < 0) { + VIR_FREE(vm->def->tty); + if (lxcOpenTty(conn, &vm->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,7 +921,7 @@ if (vm->pid == 0) { /* child process calls forward routine */ - lxcTtyForward(vm->parentTty, vm->containerTtyFd); + lxcTtyForward(vm->parentTty, containerTty); } if (lxcStoreTtyPid(driver, vm)) { @@ -1019,7 +929,7 @@ } close(vm->parentTty); - close(vm->containerTtyFd); + 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 :|