[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] PATCH: 1/5: Removing state from lxc_vm_t



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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]