[libvirt] PATCH: 0/4: Make LXC controller a separate binary
Currently libvirt is just forking off a child process to handle the LXC I/O controller. This works OK, but is kinda evil. This series of patches makes it into a fully-fledged exec()able binary. With a tiny bit more work, you could even use this to launch an LXC container standalone without needing libvirt - which could be useful for debugging during development at the very least. The other nice thing is that it makes a 'ps' listing more meaningful - you see a 'libvirt_lxc' process and the name of the container as a command line arg. Daniel -- |: 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 :|
This is a preparatory patch for the later refactoring. The virDomainObj struct currently contains a configFIle and autostartLink member which is the path to the /etc/libvirt/{DRIVER}/{NAME}.xml files for QEMU/LXC drivers. In a subsequent patch we need to store a 'live' config file in /var/run/libvirt/lxc/{NAME}.xml too. Rather than introduce yet another struct variable for this, its better to just remove all of them and build the path at the places where its actually needed. This is an iteration of the previous patch I did, using Jim's suggestion of a helper method to format the filename, instead of repeating asprintf() throughout domain_conf.c | 135 +++++++++++++++++++++++++++------------------------------- domain_conf.h | 14 +++--- lxc_driver.c | 15 ++++-- qemu_driver.c | 62 ++++++++++++++++++-------- util.c | 17 +++++++ util.h | 3 + 6 files changed, 145 insertions(+), 101 deletions(-) Daniel diff -r 4af56c5570da src/domain_conf.c --- a/src/domain_conf.c Wed Aug 13 14:16:18 2008 +0100 +++ b/src/domain_conf.c Wed Aug 13 14:16:52 2008 +0100 @@ -421,8 +421,6 @@ virDomainDefFree(dom->newDef); VIR_FREE(dom->vcpupids); - VIR_FREE(dom->configFile); - VIR_FREE(dom->autostartLink); VIR_FREE(dom); } @@ -3220,31 +3218,19 @@ int virDomainSaveConfig(virConnectPtr conn, const char *configDir, - const char *autostartDir, - virDomainObjPtr dom) + virDomainDefPtr def) { char *xml; + char *configFile = NULL; int fd = -1, ret = -1; size_t towrite; int err; - if (!dom->configFile && - asprintf(&dom->configFile, "%s/%s.xml", - configDir, dom->def->name) < 0) { - dom->configFile = NULL; - virDomainReportError(conn, VIR_ERR_NO_MEMORY, NULL); + if ((configFile = virDomainConfigFile(conn, configDir, def->name)) == NULL) goto cleanup; - } - if (!dom->autostartLink && - asprintf(&dom->autostartLink, "%s/%s.xml", - autostartDir, dom->def->name) < 0) { - dom->autostartLink = NULL; - virDomainReportError(conn, VIR_ERR_NO_MEMORY, NULL); - goto cleanup; - } if (!(xml = virDomainDefFormat(conn, - dom->newDef ? dom->newDef : dom->def, + def, VIR_DOMAIN_XML_SECURE))) goto cleanup; @@ -3255,34 +3241,27 @@ goto cleanup; } - if ((err = virFileMakePath(autostartDir))) { - virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("cannot create autostart directory %s: %s"), - autostartDir, strerror(err)); - goto cleanup; - } - - if ((fd = open(dom->configFile, + if ((fd = open(configFile, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR )) < 0) { virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("cannot create config file %s: %s"), - dom->configFile, strerror(errno)); + _("cannot create config file %s: %s"), + configFile, strerror(errno)); goto cleanup; } towrite = strlen(xml); if (safewrite(fd, xml, towrite) < 0) { virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("cannot write config file %s: %s"), - dom->configFile, strerror(errno)); + _("cannot write config file %s: %s"), + configFile, strerror(errno)); goto cleanup; } if (close(fd) < 0) { virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("cannot save config file %s: %s"), - dom->configFile, strerror(errno)); + _("cannot save config file %s: %s"), + configFile, strerror(errno)); goto cleanup; } @@ -3302,25 +3281,18 @@ virDomainObjPtr *doms, const char *configDir, const char *autostartDir, - const char *file) + const char *name) { char *configFile = NULL, *autostartLink = NULL; virDomainDefPtr def = NULL; virDomainObjPtr dom; int autostart; - if (asprintf(&configFile, "%s/%s", - configDir, file) < 0) { - configFile = NULL; - virDomainReportError(conn, VIR_ERR_NO_MEMORY, NULL); + if ((configFile = virDomainConfigFile(conn, configDir, name)) == NULL) goto error; - } - if (asprintf(&autostartLink, "%s/%s", - autostartDir, file) < 0) { - autostartLink = NULL; - virDomainReportError(conn, VIR_ERR_NO_MEMORY, NULL); + if ((autostartLink = virDomainConfigFile(conn, autostartDir, name)) == NULL) goto error; - } + if ((autostart = virFileLinkPointsTo(autostartLink, configFile)) < 0) goto error; @@ -3328,20 +3300,10 @@ if (!(def = virDomainDefParseFile(conn, caps, configFile))) goto error; - if (!virFileMatchesNameSuffix(file, def->name, ".xml")) { - virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("Domain config filename '%s'" - " does not match domain name '%s'"), - configFile, def->name); - goto error; - } - if (!(dom = virDomainAssignDef(conn, doms, def))) goto error; dom->state = VIR_DOMAIN_SHUTOFF; - dom->configFile = configFile; - dom->autostartLink = autostartLink; dom->autostart = autostart; return dom; @@ -3372,20 +3334,24 @@ } while ((entry = readdir(dir))) { + virDomainObjPtr dom; + if (entry->d_name[0] == '.') continue; - if (!virFileHasSuffix(entry->d_name, ".xml")) + if (!virFileStripSuffix(entry->d_name, ".xml")) continue; /* NB: ignoring errors, so one malformed config doesn't kill the whole process */ - virDomainLoadConfig(conn, - caps, - doms, - configDir, - autostartDir, - entry->d_name); + dom = virDomainLoadConfig(conn, + caps, + doms, + configDir, + autostartDir, + entry->d_name); + if (dom) + dom->persistent = 1; } closedir(dir); @@ -3394,25 +3360,50 @@ } int virDomainDeleteConfig(virConnectPtr conn, - virDomainObjPtr dom) + const char *configDir, + const char *autostartDir, + virDomainObjPtr dom) { - if (!dom->configFile || !dom->autostartLink) { + char *configFile = NULL, *autostartLink = NULL; + int ret = -1; + + if ((configFile = virDomainConfigFile(conn, configDir, dom->def->name)) == NULL) + goto cleanup; + if ((autostartLink = virDomainConfigFile(conn, autostartDir, dom->def->name)) == NULL) + goto cleanup; + + /* Not fatal if this doesn't work */ + unlink(autostartLink); + + if (unlink(configFile) < 0 && + errno != ENOENT) { virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("no config file for %s"), dom->def->name); - return -1; + _("cannot remove config for %s: %s"), + dom->def->name, strerror(errno)); + goto cleanup; } - /* Not fatal if this doesn't work */ - unlink(dom->autostartLink); + ret = 0; - if (unlink(dom->configFile) < 0) { - virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("cannot remove config for %s: %s"), - dom->def->name, strerror(errno)); - return -1; +cleanup: + VIR_FREE(configFile); + VIR_FREE(autostartLink); + return ret; +} + +char *virDomainConfigFile(virConnectPtr conn, + const char *dir, + const char *name) +{ + char *ret = NULL; + + if (asprintf(&ret, "%s/%s.xml", dir, name) < 0) { + virDomainReportError(conn, VIR_ERR_NO_MEMORY, NULL); + return NULL; } - return 0; + return ret; } + #endif /* ! PROXY */ diff -r 4af56c5570da src/domain_conf.h --- a/src/domain_conf.h Wed Aug 13 14:16:18 2008 +0100 +++ b/src/domain_conf.h Wed Aug 13 14:16:52 2008 +0100 @@ -458,9 +458,6 @@ unsigned int autostart : 1; unsigned int persistent : 1; - char *configFile; - char *autostartLink; - virDomainDefPtr def; /* The current definition */ virDomainDefPtr newDef; /* New definition to activate at shutdown */ @@ -532,15 +529,14 @@ int virDomainSaveConfig(virConnectPtr conn, const char *configDir, - const char *autostartDir, - virDomainObjPtr dom); + virDomainDefPtr def); virDomainObjPtr virDomainLoadConfig(virConnectPtr conn, virCapsPtr caps, virDomainObjPtr *doms, const char *configDir, const char *autostartDir, - const char *file); + const char *name); int virDomainLoadAllConfigs(virConnectPtr conn, virCapsPtr caps, @@ -549,7 +545,13 @@ const char *autostartDir); int virDomainDeleteConfig(virConnectPtr conn, + const char *configDir, + const char *autostartDir, virDomainObjPtr dom); + +char *virDomainConfigFile(virConnectPtr conn, + const char *dir, + const char *name); virDomainNetDefPtr virDomainNetDefParseXML(virConnectPtr conn, xmlNodePtr node); diff -r 4af56c5570da src/lxc_driver.c --- a/src/lxc_driver.c Wed Aug 13 14:16:18 2008 +0100 +++ b/src/lxc_driver.c Wed Aug 13 14:16:52 2008 +0100 @@ -250,11 +250,11 @@ virDomainDefFree(def); return NULL; } + vm->persistent = 1; if (virDomainSaveConfig(conn, driver->configDir, - driver->autostartDir, - vm) < 0) { + vm->newDef ? vm->newDef : vm->def) < 0) { virDomainRemoveInactive(&driver->domains, vm); return NULL; } @@ -284,10 +284,17 @@ return -1; } - if (virDomainDeleteConfig(dom->conn, vm) <0) + if (!vm->persistent) { + lxcError(dom->conn, dom, VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot undefine transient domain")); return -1; + } - vm->configFile[0] = '\0'; + if (virDomainDeleteConfig(dom->conn, + driver->configDir, + driver->autostartDir, + vm) <0) + return -1; virDomainRemoveInactive(&driver->domains, vm); diff -r 4af56c5570da src/qemu_driver.c --- a/src/qemu_driver.c Wed Aug 13 14:16:18 2008 +0100 +++ b/src/qemu_driver.c Wed Aug 13 14:16:52 2008 +0100 @@ -351,7 +351,7 @@ virDomainObjPtr next = vm->next; if (virDomainIsActive(vm)) qemudShutdownVMDaemon(NULL, qemu_driver, vm); - if (!vm->configFile) + if (!vm->persistent) virDomainRemoveInactive(&qemu_driver->domains, vm); vm = next; @@ -1071,7 +1071,7 @@ static int qemudDispatchVMLog(struct qemud_driver *driver, virDomainObjPtr vm, int fd) { if (qemudVMData(driver, vm, fd) < 0) { qemudShutdownVMDaemon(NULL, driver, vm); - if (!vm->configFile) + if (!vm->persistent) virDomainRemoveInactive(&driver->domains, vm); } @@ -1081,7 +1081,7 @@ static int qemudDispatchVMFailure(struct qemud_driver *driver, virDomainObjPtr vm, int fd ATTRIBUTE_UNUSED) { qemudShutdownVMDaemon(NULL, driver, vm); - if (!vm->configFile) + if (!vm->persistent) virDomainRemoveInactive(&driver->domains, vm); return 0; @@ -2141,7 +2141,7 @@ } qemudShutdownVMDaemon(dom->conn, driver, vm); - if (!vm->configFile) + if (!vm->persistent) virDomainRemoveInactive(&driver->domains, vm); @@ -2472,7 +2472,7 @@ /* Shut it down */ qemudShutdownVMDaemon(dom->conn, driver, vm); - if (!vm->configFile) + if (!vm->persistent) virDomainRemoveInactive(&driver->domains, vm); @@ -2764,7 +2764,7 @@ if (ret < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, "%s", _("failed to start VM")); - if (!vm->configFile) + if (!vm->persistent) virDomainRemoveInactive(&driver->domains, vm); return -1; @@ -2870,11 +2870,11 @@ virDomainDefFree(def); return NULL; } + vm->persistent = 1; if (virDomainSaveConfig(conn, driver->configDir, - driver->autostartDir, - vm) < 0) { + vm->newDef ? vm->newDef : vm->def) < 0) { virDomainRemoveInactive(&driver->domains, vm); return NULL; @@ -2901,7 +2901,13 @@ return -1; } - if (virDomainDeleteConfig(dom->conn, vm) < 0) + if (!vm->persistent) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot undefine transient domain")); + return -1; + } + + if (virDomainDeleteConfig(dom->conn, driver->configDir, driver->autostartDir, vm) < 0) return -1; virDomainRemoveInactive(&driver->domains, @@ -3141,13 +3147,21 @@ } static int qemudDomainSetAutostart(virDomainPtr dom, - int autostart) { + int autostart) { struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData; virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); + char *configFile = NULL, *autostartLink = NULL; + int ret = -1; if (!vm) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN, "%s", _("no domain with matching uuid")); + return -1; + } + + if (!vm->persistent) { + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot set autostart for transient domain")); return -1; } @@ -3156,6 +3170,11 @@ if (vm->autostart == autostart) return 0; + if ((configFile = virDomainConfigFile(dom->conn, driver->configDir, vm->def->name)) == NULL) + goto cleanup; + if ((autostartLink = virDomainConfigFile(dom->conn, driver->autostartDir, vm->def->name)) == NULL) + goto cleanup; + if (autostart) { int err; @@ -3163,27 +3182,32 @@ qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, _("cannot create autostart directory %s: %s"), driver->autostartDir, strerror(err)); - return -1; + goto cleanup; } - if (symlink(vm->configFile, vm->autostartLink) < 0) { + if (symlink(configFile, autostartLink) < 0) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, - _("Failed to create symlink '%s' to '%s': %s"), - vm->autostartLink, vm->configFile, strerror(errno)); - return -1; + _("Failed to create symlink '%s to '%s': %s"), + autostartLink, configFile, strerror(errno)); + goto cleanup; } } else { - if (unlink(vm->autostartLink) < 0 && errno != ENOENT && errno != ENOTDIR) { + if (unlink(autostartLink) < 0 && errno != ENOENT && errno != ENOTDIR) { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, _("Failed to delete symlink '%s': %s"), - vm->autostartLink, strerror(errno)); - return -1; + autostartLink, strerror(errno)); + goto cleanup; } } vm->autostart = autostart; + ret = 0; - return 0; +cleanup: + VIR_FREE(configFile); + VIR_FREE(autostartLink); + + return ret; } /* This uses the 'info blockstats' monitor command which was diff -r 4af56c5570da src/util.c --- a/src/util.c Wed Aug 13 14:16:18 2008 +0100 +++ b/src/util.c Wed Aug 13 14:16:52 2008 +0100 @@ -82,6 +82,23 @@ } __virRaiseError(conn, NULL, NULL, VIR_FROM_NONE, code, VIR_ERR_ERROR, NULL, NULL, NULL, -1, -1, "%s", errorMessage); +} + +int virFileStripSuffix(char *str, + const char *suffix) +{ + int len = strlen(str); + int suffixlen = strlen(suffix); + + if (len < suffixlen) + return 0; + + if (!STREQ(str + len - suffixlen, suffix)) + return 0; + + str[len-suffixlen] = '\0'; + + return 1; } #ifndef __MINGW32__ diff -r 4af56c5570da src/util.h --- a/src/util.h Wed Aug 13 14:16:18 2008 +0100 +++ b/src/util.h Wed Aug 13 14:16:52 2008 +0100 @@ -55,6 +55,9 @@ int virFileHasSuffix(const char *str, const char *suffix); +int virFileStripSuffix(char *str, + const char *suffix); + int virFileLinkPointsTo(const char *checkLink, const char *checkDest); -- |: 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 :|
Over time we've added lots of general purpose source code files, as wel as making more of the existing ones conditionally compiled. We've done this by adding lots of #ifdef WITH_XXXX macros across the various source files. This is becoming rather a minefield with soo many conditionals sprinkled throughout the code. With all the recent re-factoring we now have very good separation between generic code, and driver specific code - each typically being in separate files. Thus it is now practical to remove the vast majority of the macros and just do the conditional compilation via the Makefile.am. The patch is not entirely clear - the resulting Makefile.am is much easier to review. It is structured in two section, first we declare variables for groups of source code files - eg generic helper files, generic domain XML files, generic network XML files, and then per-driver files. # These files are not related to driver APIs. Simply generic # helper APIs for various purposes GENERIC_LIB_SOURCES = \ bridge.c bridge.h \ buf.c buf.h \ conf.c conf.h \ event.c event.h \ iptables.c iptables.h \ memory.c memory.h \ qparams.c qparams.h \ stats_linux.c stats_linux.h \ uuid.c uuid.h \ util.c util.h \ virterror.c \ xml.c xml.h # Domain driver generic impl APIs DOMAIN_CONF_SOURCES = \ capabilities.c capabilities.h \ domain_conf.c domain_conf.h \ nodeinfo.h nodeinfo.c # Network driver generic impl APIs NETWORK_CONF_SOURCES = \ network_conf.c network_conf.h # Storage driver generic impl APIs STORAGE_CONF_SOURCES = \ storage_conf.h storage_conf.c # The remote RPC driver, covering domains, storage, networks, etc REMOTE_DRIVER_SOURCES = \ gnutls_1_0_compat.h \ remote_internal.c remote_internal.h \ ../qemud/remote_protocol.c \ ../qemud/remote_protocol.h \ socketcompat.h # Mock driver, covering domains, storage, networks, etc TEST_DRIVER_SOURCES = \ test.c test.h # Now the Hypervisor specific drivers XEN_DRIVER_SOURCES = \ proxy_internal.c proxy_internal.h \ sexpr.c sexpr.h \ xen_internal.c xen_internal.h \ xen_unified.c xen_unified.h \ xend_internal.c xend_internal.h \ xm_internal.c xm_internal.h \ xs_internal.c xs_internal.h In the second section we then build up the libvirt_la_SOURCES variable, conditionally adding the driver specific sources according to what the user asked 'configure' to build libvirt_la_SOURCES = \ driver.h \ hash.c hash.h \ internal.h \ libvirt.c \ $(GENERIC_LIB_SOURCES) \ $(DOMAIN_CONF_SOURCES) \ $(NETWORK_CONF_SOURCES) \ $(STORAGE_CONF_SOURCES) # Drivers usable outside daemon context if WITH_TEST libvirt_la_SOURCES += $(TEST_DRIVER_SOURCES) endif if WITH_REMOTE libvirt_la_SOURCES += $(REMOTE_DRIVER_SOURCES) endif if WITH_XEN libvirt_la_SOURCES += $(XEN_DRIVER_SOURCES) endif I've cut this down - the real makefile covers all drivers. Finally we also add all the sources to EXTRA_DIST, so that if someone does 'make dist', they include all the source files regardless of what has been configured to build. In removing the unneeded WITH_XXX macros from header/sources I noticed that a bunch of our header files have #ifdef __cplusplus extern "C" { #endif With is irrelevant since we're not using C++ anywhere - indeed some of the files even had the corresponding '}' missing, so clearly this is never actually used during compilation. Removing this fixes bogus indentation in some of the header files I've tested this all by running configure --without-XXX for each hypervisor driver in turn, and it seemed to work in all cases. The only thing I didn't test is MinGW. I think I really need to add a MinGW based build to the nightly build system, so we can sanity check that nightly configure.in | 15 ++- qemud/Makefile.am | 46 ++++++---- src/Makefile.am | 212 ++++++++++++++++++++++++++++++++++++-------------- src/bridge.c | 4 src/bridge.h | 4 src/conf.h | 7 - src/console.h | 10 -- src/driver.h | 7 - src/hash.h | 7 - src/internal.h | 7 - src/libvirt.c | 21 +++- src/lxc_conf.c | 3 src/lxc_conf.h | 3 src/lxc_container.c | 3 src/lxc_container.h | 4 src/lxc_driver.c | 4 src/lxc_driver.h | 4 src/nodeinfo.h | 10 -- src/openvz_conf.c | 3 src/openvz_driver.c | 3 src/proxy_internal.c | 4 src/proxy_internal.h | 7 - src/qemu_conf.c | 4 src/qemu_conf.h | 4 src/qemu_driver.c | 3 src/qemu_driver.h | 4 src/remote_internal.h | 8 - src/test.c | 6 - src/test.h | 7 - src/veth.c | 4 src/xen_internal.c | 3 src/xen_internal.h | 8 - src/xen_unified.c | 3 src/xen_unified.h | 9 -- src/xend_internal.c | 2 src/xend_internal.h | 4 src/xm_internal.c | 2 src/xs_internal.c | 3 src/xs_internal.h | 7 - tests/testutils.h | 44 ++++------ 40 files changed, 237 insertions(+), 276 deletions(-) Daniel diff -r b6d8ebb28723 configure.in --- a/configure.in Tue Aug 12 22:21:25 2008 +0100 +++ b/configure.in Tue Aug 12 22:21:47 2008 +0100 @@ -241,27 +241,35 @@ LIBVIRT_FEATURES= WITH_XEN=0 -if test "$with_openvz" = "yes" ; then +if test "$with_openvz" = "yes"; then LIBVIRT_FEATURES="$LIBVIRT_FEATURES -DWITH_OPENVZ" fi +AM_CONDITIONAL([WITH_OPENVZ], [test "$with_openvz" = "yes"]) + if test "$with_lxc" = "yes" ; then LIBVIRT_FEATURES="$LIBVIRT_FEATURES -DWITH_LXC" fi +AM_CONDITIONAL([WITH_LXC], [test "$with_lxc" = "yes"]) + if test "$with_qemu" = "yes" ; then LIBVIRT_FEATURES="$LIBVIRT_FEATURES -DWITH_QEMU" fi +AM_CONDITIONAL([WITH_QEMU], [test "$with_qemu" = "yes"]) if test "$with_test" = "yes" ; then LIBVIRT_FEATURES="$LIBVIRT_FEATURES -DWITH_TEST" fi +AM_CONDITIONAL([WITH_TEST], [test "$with_test" = "yes"]) if test "$with_remote" = "yes" ; then LIBVIRT_FEATURES="$LIBVIRT_FEATURES -DWITH_REMOTE" fi +AM_CONDITIONAL([WITH_REMOTE], [test "$with_remote" = "yes"]) if test "$with_libvirtd" = "yes" ; then LIBVIRT_FEATURES="$LIBVIRT_FEATURES -DWITH_LIBVIRTD" fi +AM_CONDITIONAL([WITH_LIBVIRTD], [test "$with_libvirtd" = "yes"]) if test "$with_xen" = "yes" ; then dnl search for the Xen store library @@ -294,11 +302,12 @@ #include <xen/xen.h> ]) fi +AM_CONDITIONAL([WITH_XEN], [test "$WITH_XEN" = "1"]) dnl -dnl check for kernel headers required by qemud/bridge.c +dnl check for kernel headers required by src/bridge.c dnl -if test "$with_qemu" = "yes" ; then +if test "$with_qemu" = "yes" -o "$with_lxc" = "yes" ; then AC_CHECK_HEADERS([linux/param.h linux/sockios.h linux/if_bridge.h linux/if_tun.h],, AC_MSG_ERROR([You must install kernel-headers in order to compile libvirt])) fi diff -r b6d8ebb28723 qemud/Makefile.am --- a/qemud/Makefile.am Tue Aug 12 22:21:25 2008 +0100 +++ b/qemud/Makefile.am Tue Aug 12 22:21:47 2008 +0100 @@ -2,19 +2,30 @@ INCLUDES = $(LIBVIRT_FEATURES) -# Distribute the generated files so that rpcgen isn't required on the -# target machine (although almost any Unix machine will have it). -EXTRA_DIST = libvirtd.init.in libvirtd.sysconf default-network.xml \ - remote_protocol.x \ - remote_protocol.c remote_protocol.h \ - remote_generate_stubs.pl rpcgen_fix.pl \ - remote_dispatch_prototypes.h \ - remote_dispatch_localvars.h \ - remote_dispatch_proc_switch.h \ - mdns.c mdns.h \ - libvirtd.sasl \ - libvirtd.conf \ - libvirtd.policy +DAEMON_SOURCES = \ + event.c event.h \ + qemud.c qemud.h \ + remote.c \ + remote_dispatch_prototypes.h \ + remote_dispatch_localvars.h \ + remote_dispatch_proc_switch.h \ + remote_protocol.h remote_protocol.c \ + $(srcdir)/../src/util-lib.c + +AVAHI_SOURCES = \ + mdns.c mdns.h + +EXTRA_DIST = \ + default-network.xml \ + remote_generate_stubs.pl rpcgen_fix.pl \ + remote_protocol.x \ + libvirtd.conf \ + libvirtd.init.in \ + libvirtd.policy \ + libvirtd.sasl \ + libvirtd.sysconf \ + $(AVAHI_SOURCES) \ + $(DAEMON_SOURCES) if RPCGEN SUFFIXES = .x @@ -45,12 +56,7 @@ confdir = $(sysconfdir)/libvirt/ conf_DATA = libvirtd.conf -libvirtd_SOURCES = \ - qemud.c qemud.h \ - remote_protocol.h remote_protocol.c \ - remote.c \ - $(srcdir)/../src/util-lib.c \ - event.c event.h +libvirtd_SOURCES = $(DAEMON_SOURCES) #-D_XOPEN_SOURCE=600 -D_XOPEN_SOURCE_EXTENDED=1 -D_POSIX_C_SOURCE=199506L libvirtd_CFLAGS = \ @@ -79,7 +85,7 @@ endif if HAVE_AVAHI -libvirtd_SOURCES += mdns.c mdns.h +libvirtd_SOURCES += $(AVAHI_SOURCES) libvirtd_CFLAGS += $(AVAHI_CFLAGS) libvirtd_LDADD += $(AVAHI_LIBS) endif diff -r b6d8ebb28723 src/Makefile.am --- a/src/Makefile.am Tue Aug 12 22:21:25 2008 +0100 +++ b/src/Makefile.am Tue Aug 12 22:21:47 2008 +0100 @@ -30,80 +30,176 @@ lib_LTLIBRARIES = libvirt.la -CLIENT_SOURCES = \ - libvirt.c internal.h \ +# These files are not related to driver APIs. Simply generic +# helper APIs for various purposes +GENERIC_LIB_SOURCES = \ + bridge.c bridge.h \ + buf.c buf.h \ + conf.c conf.h \ + event.c event.h \ + iptables.c iptables.h \ + memory.c memory.h \ + qparams.c qparams.h \ + stats_linux.c stats_linux.h \ + uuid.c uuid.h \ + util.c util.h \ + virterror.c \ + xml.c xml.h + +# Domain driver generic impl APIs +DOMAIN_CONF_SOURCES = \ + capabilities.c capabilities.h \ + domain_conf.c domain_conf.h \ + nodeinfo.h nodeinfo.c + +# Network driver generic impl APIs +NETWORK_CONF_SOURCES = \ + network_conf.c network_conf.h + +# Storage driver generic impl APIs +STORAGE_CONF_SOURCES = \ + storage_conf.h storage_conf.c + + +# The remote RPC driver, covering domains, storage, networks, etc +REMOTE_DRIVER_SOURCES = \ gnutls_1_0_compat.h \ - socketcompat.h \ - memory.c memory.h \ - hash.c hash.h \ - test.c test.h \ - buf.c buf.h \ - qparams.c qparams.h \ - domain_conf.c domain_conf.h \ - capabilities.c capabilities.h \ - xml.c xml.h \ - event.c event.h \ + remote_internal.c remote_internal.h \ + ../qemud/remote_protocol.c \ + ../qemud/remote_protocol.h \ + socketcompat.h + +# Mock driver, covering domains, storage, networks, etc +TEST_DRIVER_SOURCES = \ + test.c test.h + + + +# Now the Hypervisor specific drivers +XEN_DRIVER_SOURCES = \ + proxy_internal.c proxy_internal.h \ + sexpr.c sexpr.h \ + xen_internal.c xen_internal.h \ xen_unified.c xen_unified.h \ - xen_internal.c xen_internal.h \ - xs_internal.c xs_internal.h \ xend_internal.c xend_internal.h \ - stats_linux.c stats_linux.h \ - sexpr.c sexpr.h \ - virterror.c \ - driver.h \ - proxy_internal.c proxy_internal.h \ - conf.c conf.h \ - network_conf.c network_conf.h \ - xm_internal.c xm_internal.h \ - remote_internal.c remote_internal.h \ - bridge.c bridge.h \ - iptables.c iptables.h \ - uuid.c uuid.h \ - qemu_driver.c qemu_driver.h \ - qemu_conf.c qemu_conf.h \ - openvz_conf.c openvz_conf.h \ - openvz_driver.c openvz_driver.h \ - lxc_driver.c lxc_driver.h \ - lxc_controller.c lxc_controller.h \ + xm_internal.c xm_internal.h \ + xs_internal.c xs_internal.h + +LXC_DRIVER_SOURCES = \ lxc_conf.c lxc_conf.h \ lxc_container.c lxc_container.h \ - veth.c veth.h \ - nodeinfo.h nodeinfo.c \ - util.c util.h + lxc_controller.c lxc_controller.h \ + lxc_driver.c lxc_driver.h \ + veth.c veth.h -SERVER_SOURCES = \ - ../qemud/remote_protocol.c ../qemud/remote_protocol.h +OPENVZ_DRIVER_SOURCES = \ + openvz_conf.c openvz_conf.h \ + openvz_driver.c openvz_driver.h -if WITH_LIBVIRTD +# XXX network driver should be split out of this +QEMU_DRIVER_SOURCES = \ + qemu_conf.c qemu_conf.h \ + qemu_driver.c qemu_driver.h -CLIENT_SOURCES += \ - storage_conf.h storage_conf.c \ + +# And finally storage backend specific impls +STORAGE_DRIVER_SOURCES = \ storage_driver.h storage_driver.c \ - storage_backend.h storage_backend.c \ + storage_backend.h storage_backend.c + +STORAGE_DRIVER_FS_SOURCES = \ storage_backend_fs.h storage_backend_fs.c +STORAGE_DRIVER_LVM_SOURCES = \ + storage_backend_logical.h \ + storage_backend_logical.c + +STORAGE_DRIVER_ISCSI_SOURCES = \ + storage_backend_iscsi.h storage_backend_iscsi.c + +STORAGE_DRIVER_DISK_SOURCES = \ + storage_backend_disk.h storage_backend_disk.c + +STORAGE_HELPER_DISK_SOURCES = \ + parthelper.c + + + +######################### +# +# Build up list of libvirt.la source files based on configure conditions +# +# First deal with sources usable in non-daemon context + +libvirt_la_SOURCES = \ + driver.h \ + hash.c hash.h \ + internal.h \ + libvirt.c \ + $(GENERIC_LIB_SOURCES) \ + $(DOMAIN_CONF_SOURCES) \ + $(NETWORK_CONF_SOURCES) \ + $(STORAGE_CONF_SOURCES) + +# Drivers usable outside daemon context +if WITH_TEST +libvirt_la_SOURCES += $(TEST_DRIVER_SOURCES) +endif + +if WITH_REMOTE +libvirt_la_SOURCES += $(REMOTE_DRIVER_SOURCES) +endif + +if WITH_XEN +libvirt_la_SOURCES += $(XEN_DRIVER_SOURCES) +endif + + +# Drivers usable inside daemon context +if WITH_LIBVIRTD +libvirt_la_SOURCES += $(STORAGE_DRIVER_SOURCES) +libvirt_la_SOURCES += $(STORAGE_DRIVER_FS_SOURCES) + +if WITH_QEMU +libvirt_la_SOURCES += $(QEMU_DRIVER_SOURCES) +endif + +if WITH_LXC +libvirt_la_SOURCES += $(LXC_DRIVER_SOURCES) +endif + +if WITH_OPENVZ +libvirt_la_SOURCES += $(OPENVZ_DRIVER_SOURCES) +endif + if WITH_STORAGE_LVM -CLIENT_SOURCES += storage_backend_logical.h storage_backend_logical.c -else -EXTRA_DIST += storage_backend_logical.h storage_backend_logical.c +libvirt_la_SOURCES += $(STORAGE_DRIVER_LVM_SOURCES) endif if WITH_STORAGE_ISCSI -CLIENT_SOURCES += storage_backend_iscsi.h storage_backend_iscsi.c -else -EXTRA_DIST += storage_backend_iscsi.h storage_backend_iscsi.c +libvirt_la_SOURCES += $(STORAGE_DRIVER_ISCSI_SOURCES) endif if WITH_STORAGE_DISK -CLIENT_SOURCES += storage_backend_disk.h storage_backend_disk.c -else -EXTRA_DIST += storage_backend_disk.h storage_backend_disk.c +libvirt_la_SOURCES += $(STORAGE_DRIVER_DISK_SOURCES) +endif endif -endif +# Add all conditional sources just in case... +EXTRA_DIST += \ + $(TEST_DRIVER_SOURCES) \ + $(REMOTE_DRIVER_SOURCES) \ + $(XEN_DRIVER_SOURCES) \ + $(QEMU_DRIVER_SOURCES) \ + $(LXC_DRIVER_SOURCES) \ + $(OPENVZ_DRIVER_SOURCES) \ + $(STORAGE_DRIVER_SOURCES) \ + $(STORAGE_DRIVER_FS_SOURCES) \ + $(STORAGE_DRIVER_LVM_SOURCES) \ + $(STORAGE_DRIVER_ISCSI_SOURCES) \ + $(STORAGE_DRIVER_DISK_SOURCES) -libvirt_la_SOURCES = $(CLIENT_SOURCES) $(SERVER_SOURCES) libvirt_la_LIBADD = $(LIBXML_LIBS) $(GNUTLS_LIBS) $(SASL_LIBS) $(SELINUX_LIBS) \ $(NUMACTL_LIBS) \ @CYGWIN_EXTRA_LIBADD@ ../gnulib/lib/libgnu.la @@ -134,7 +230,11 @@ bin_PROGRAMS = virsh -virsh_SOURCES = virsh.c console.c console.h util-lib.c util-lib.h +virsh_SOURCES = \ + console.c console.h \ + util-lib.c util-lib.h \ + virsh.c + virsh_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDFLAGS) virsh_DEPENDENCIES = $(DEPS) virsh_LDADD = $(LDADDS) $(VIRSH_LIBS) @@ -176,14 +276,14 @@ if WITH_LIBVIRTD libexec_PROGRAMS = libvirt_parthelper -libvirt_parthelper_SOURCES = parthelper.c +libvirt_parthelper_SOURCES = $(STORAGE_HELPER_DISK_SOURCES) libvirt_parthelper_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDCFLAGS) libvirt_parthelper_LDADD = $(LIBPARTED_LIBS) libvirt_parthelper_CFLAGS = $(LIBPARTED_CFLAGS) endif -else -EXTRA_DIST += parthelper.c endif +EXTRA_DIST += $(STORAGE_HELPER_DISK_SOURCES) + # Create the /var/cache/libvirt directory when installing. install-exec-local: diff -r b6d8ebb28723 src/bridge.c --- a/src/bridge.c Tue Aug 12 22:21:25 2008 +0100 +++ b/src/bridge.c Tue Aug 12 22:21:47 2008 +0100 @@ -21,7 +21,7 @@ #include <config.h> -#ifdef WITH_QEMU +#if defined(WITH_QEMU) || defined(WITH_LXC) #include "bridge.h" @@ -721,4 +721,4 @@ return retval; } -#endif /* WITH_QEMU */ +#endif /* WITH_QEMU || WITH_LXC */ diff -r b6d8ebb28723 src/bridge.h --- a/src/bridge.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/bridge.h Tue Aug 12 22:21:47 2008 +0100 @@ -24,7 +24,7 @@ #include <config.h> -#ifdef WITH_QEMU +#if defined(WITH_QEMU) || defined(WITH_LXC) #include <net/if.h> #include <netinet/in.h> @@ -98,6 +98,6 @@ const char *bridge, int *enable); -#endif /* WITH_QEMU */ +#endif /* WITH_QEMU || WITH_LXC */ #endif /* __QEMUD_BRIDGE_H__ */ diff -r b6d8ebb28723 src/conf.h --- a/src/conf.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/conf.h Tue Aug 12 22:21:47 2008 +0100 @@ -10,10 +10,6 @@ #ifndef __VIR_CONF_H__ #define __VIR_CONF_H__ - -#ifdef __cplusplus -extern "C" { -#endif /** * virConfType: @@ -93,7 +89,4 @@ #define virConfWriteFile(f,c) __virConfWriteFile((f),(c)) #define virConfWriteMem(m,l,c) __virConfWriteMem((m),(l),(c)) -#ifdef __cplusplus -} -#endif #endif /* __VIR_CONF_H__ */ diff -r b6d8ebb28723 src/console.h --- a/src/console.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/console.h Tue Aug 12 22:21:47 2008 +0100 @@ -25,15 +25,7 @@ #ifndef __MINGW32__ -#ifdef __cplusplus -extern "C" { -#endif - - int vshRunConsole(const char *tty); - -#ifdef __cplusplus -} -#endif +int vshRunConsole(const char *tty); #endif /* !__MINGW32__ */ diff -r b6d8ebb28723 src/driver.h --- a/src/driver.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/driver.h Tue Aug 12 22:21:47 2008 +0100 @@ -12,10 +12,6 @@ #include <libxml/uri.h> #include <signal.h> - -#ifdef __cplusplus -extern "C" { -#endif /* * List of registered drivers numbers @@ -611,7 +607,4 @@ int virRegisterStateDriver(virStateDriverPtr); #endif -#ifdef __cplusplus -} -#endif /* __cplusplus */ #endif /* __VIR_DRIVER_H__ */ diff -r b6d8ebb28723 src/hash.h --- a/src/hash.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/hash.h Tue Aug 12 22:21:47 2008 +0100 @@ -11,10 +11,6 @@ #ifndef __VIR_HASH_H__ #define __VIR_HASH_H__ - -#ifdef __cplusplus -extern "C" { -#endif /* * The hash table. @@ -90,7 +86,4 @@ int virHashRemoveSet(virHashTablePtr table, virHashSearcher iter, virHashDeallocator f, const void *data); void *virHashSearch(virHashTablePtr table, virHashSearcher iter, const void *data); -#ifdef __cplusplus -} -#endif #endif /* ! __VIR_HASH_H__ */ diff -r b6d8ebb28723 src/internal.h --- a/src/internal.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/internal.h Tue Aug 12 22:21:47 2008 +0100 @@ -37,10 +37,6 @@ #include "libvirt/libvirt.h" #include "libvirt/virterror.h" #include "driver.h" - -#ifdef __cplusplus -extern "C" { -#endif /* On architectures which lack these limits, define them (ie. Cygwin). * Note that the libvirt code should be robust enough to handle the @@ -366,7 +362,4 @@ int __virDomainMigratePerform (virDomainPtr domain, const char *cookie, int cookielen, const char *uri, unsigned long flags, const char *dname, unsigned long bandwidth); virDomainPtr __virDomainMigrateFinish (virConnectPtr dconn, const char *dname, const char *cookie, int cookielen, const char *uri, unsigned long flags); -#ifdef __cplusplus -} -#endif /* __cplusplus */ #endif /* __VIR_INTERNAL_H__ */ diff -r b6d8ebb28723 src/libvirt.c --- a/src/libvirt.c Tue Aug 12 22:21:25 2008 +0100 +++ b/src/libvirt.c Tue Aug 12 22:21:47 2008 +0100 @@ -36,17 +36,26 @@ #include "uuid.h" #include "util.h" + +#ifdef WITH_TEST #include "test.h" +#endif +#ifdef WITH_XEN #include "xen_unified.h" +#endif +#ifdef WITH_REMOTE #include "remote_internal.h" +#endif +#ifdef WITH_QEMU #include "qemu_driver.h" -#include "storage_driver.h" +#endif #ifdef WITH_OPENVZ #include "openvz_driver.h" #endif #ifdef WITH_LXC #include "lxc_driver.h" #endif +#include "storage_driver.h" /* * TODO: @@ -273,23 +282,23 @@ #ifdef WITH_TEST if (testRegister() == -1) return -1; #endif +#ifdef WITH_XEN + if (xenUnifiedRegister () == -1) return -1; +#endif +#ifdef WITH_LIBVIRTD #ifdef WITH_QEMU if (qemudRegister() == -1) return -1; #endif -#ifdef WITH_XEN - if (xenUnifiedRegister () == -1) return -1; -#endif #ifdef WITH_OPENVZ if (openvzRegister() == -1) return -1; #endif #ifdef WITH_LXC if (lxcRegister() == -1) return -1; #endif -#ifdef WITH_LIBVIRTD if (storageRegister() == -1) return -1; -#endif #ifdef WITH_REMOTE if (remoteRegister () == -1) return -1; +#endif #endif return(0); diff -r b6d8ebb28723 src/lxc_conf.c --- a/src/lxc_conf.c Tue Aug 12 22:21:25 2008 +0100 +++ b/src/lxc_conf.c Tue Aug 12 22:21:47 2008 +0100 @@ -24,8 +24,6 @@ /* includes */ #include <config.h> - -#ifdef WITH_LXC #include <sys/utsname.h> @@ -111,4 +109,3 @@ } -#endif /* WITH_LXC */ diff -r b6d8ebb28723 src/lxc_conf.h --- a/src/lxc_conf.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/lxc_conf.h Tue Aug 12 22:21:47 2008 +0100 @@ -26,8 +26,6 @@ #include <config.h> -#ifdef WITH_LXC - #include "internal.h" #include "domain_conf.h" #include "capabilities.h" @@ -52,6 +50,5 @@ int code, const char *fmt, ...) ATTRIBUTE_FORMAT(printf,4,5); -#endif /* WITH_LXC */ #endif /* LXC_CONF_H */ diff -r b6d8ebb28723 src/lxc_container.c --- a/src/lxc_container.c Tue Aug 12 22:21:25 2008 +0100 +++ b/src/lxc_container.c Tue Aug 12 22:21:47 2008 +0100 @@ -22,8 +22,6 @@ */ #include <config.h> - -#ifdef WITH_LXC #include <fcntl.h> #include <limits.h> @@ -406,4 +404,3 @@ return 0; } -#endif /* WITH_LXC */ diff -r b6d8ebb28723 src/lxc_container.h --- a/src/lxc_container.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/lxc_container.h Tue Aug 12 22:21:47 2008 +0100 @@ -26,8 +26,6 @@ #include "lxc_conf.h" -#ifdef WITH_LXC - enum { LXC_CONTAINER_FEATURE_NET = (1 << 0), }; @@ -42,6 +40,4 @@ int lxcContainerAvailable(int features); -#endif /* LXC_DRIVER_H */ - #endif /* LXC_CONTAINER_H */ diff -r b6d8ebb28723 src/lxc_driver.c --- a/src/lxc_driver.c Tue Aug 12 22:21:25 2008 +0100 +++ b/src/lxc_driver.c Tue Aug 12 22:21:47 2008 +0100 @@ -22,8 +22,6 @@ */ #include <config.h> - -#ifdef WITH_LXC #include <fcntl.h> #include <sched.h> @@ -1112,5 +1110,3 @@ virRegisterStateDriver(&lxcStateDriver); return 0; } - -#endif /* WITH_LXC */ diff -r b6d8ebb28723 src/lxc_driver.h --- a/src/lxc_driver.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/lxc_driver.h Tue Aug 12 22:21:47 2008 +0100 @@ -26,11 +26,7 @@ #include <config.h> -#ifdef WITH_LXC - /* Function declarations */ int lxcRegister(void); -#endif /* WITH_LXC */ - #endif /* LXC_DRIVER_H */ diff -r b6d8ebb28723 src/nodeinfo.h --- a/src/nodeinfo.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/nodeinfo.h Tue Aug 12 22:21:47 2008 +0100 @@ -26,14 +26,6 @@ #include "libvirt/libvirt.h" -#ifdef __cplusplus -extern "C" { -#endif - - int virNodeInfoPopulate(virConnectPtr conn, virNodeInfoPtr nodeinfo); - -#ifdef __cplusplus -} -#endif +int virNodeInfoPopulate(virConnectPtr conn, virNodeInfoPtr nodeinfo); #endif /* __VIR_NODEINFO_H__*/ diff -r b6d8ebb28723 src/openvz_conf.c --- a/src/openvz_conf.c Tue Aug 12 22:21:25 2008 +0100 +++ b/src/openvz_conf.c Tue Aug 12 22:21:47 2008 +0100 @@ -24,8 +24,6 @@ * Anoop Joe Cyriac <anoop@binarykarma.com> * */ - -#ifdef WITH_OPENVZ #include <config.h> @@ -814,4 +812,3 @@ return 0; } -#endif diff -r b6d8ebb28723 src/openvz_driver.c --- a/src/openvz_driver.c Tue Aug 12 22:21:25 2008 +0100 +++ b/src/openvz_driver.c Tue Aug 12 22:21:47 2008 +0100 @@ -24,8 +24,6 @@ * Anoop Joe Cyriac <anoop@binarykarma.com> * */ - -#ifdef WITH_OPENVZ #include <config.h> @@ -948,4 +946,3 @@ return 0; } -#endif /* WITH_OPENVZ */ diff -r b6d8ebb28723 src/proxy_internal.c --- a/src/proxy_internal.c Tue Aug 12 22:21:25 2008 +0100 +++ b/src/proxy_internal.c Tue Aug 12 22:21:47 2008 +0100 @@ -7,8 +7,6 @@ * * Daniel Veillard <veillard@redhat.com> */ - -#ifdef WITH_XEN #include <config.h> @@ -1082,4 +1080,4 @@ return(ostype); } -#endif /* WITH_XEN */ + diff -r b6d8ebb28723 src/proxy_internal.h --- a/src/proxy_internal.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/proxy_internal.h Tue Aug 12 22:21:47 2008 +0100 @@ -13,10 +13,6 @@ #define __LIBVIR_PROXY_H__ #include "libvirt/libvirt.h" - -#ifdef __cplusplus -extern "C" { -#endif #define PROXY_SOCKET_PATH "/tmp/livirt_proxy_conn" #define PROXY_PROTO_VERSION 1 @@ -98,7 +94,4 @@ extern char * xenProxyDomainDumpXML(virDomainPtr domain, int flags); -#ifdef __cplusplus -} -#endif /* __cplusplus */ #endif /* __LIBVIR_PROXY_H__ */ diff -r b6d8ebb28723 src/qemu_conf.c --- a/src/qemu_conf.c Tue Aug 12 22:21:25 2008 +0100 +++ b/src/qemu_conf.c Tue Aug 12 22:21:47 2008 +0100 @@ -22,8 +22,6 @@ */ #include <config.h> - -#ifdef WITH_QEMU #include <dirent.h> #include <string.h> @@ -1221,5 +1219,3 @@ #undef ADD_ARG_SPACE } - -#endif /* WITH_QEMU */ diff -r b6d8ebb28723 src/qemu_conf.h --- a/src/qemu_conf.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/qemu_conf.h Tue Aug 12 22:21:47 2008 +0100 @@ -25,8 +25,6 @@ #define __QEMUD_CONF_H #include <config.h> - -#ifdef WITH_QEMU #include "internal.h" #include "iptables.h" @@ -102,6 +100,4 @@ const char *qemudVirtTypeToString (int type); -#endif /* WITH_QEMU */ - #endif /* __QEMUD_CONF_H */ diff -r b6d8ebb28723 src/qemu_driver.c --- a/src/qemu_driver.c Tue Aug 12 22:21:25 2008 +0100 +++ b/src/qemu_driver.c Tue Aug 12 22:21:47 2008 +0100 @@ -22,8 +22,6 @@ */ #include <config.h> - -#ifdef WITH_QEMU #include <sys/types.h> #include <sys/poll.h> @@ -3950,4 +3948,3 @@ return 0; } -#endif /* WITH_QEMU */ diff -r b6d8ebb28723 src/qemu_driver.h --- a/src/qemu_driver.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/qemu_driver.h Tue Aug 12 22:21:47 2008 +0100 @@ -27,12 +27,8 @@ #include <config.h> -#ifdef WITH_QEMU - #include "internal.h" int qemudRegister(void); -#endif /* WITH_QEMU */ - #endif /* QEMUD_DRIVER_H */ diff -r b6d8ebb28723 src/remote_internal.h --- a/src/remote_internal.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/remote_internal.h Tue Aug 12 22:21:47 2008 +0100 @@ -26,10 +26,6 @@ #include "libvirt/virterror.h" -#ifdef __cplusplus -extern "C" { -#endif - int remoteRegister (void); #define LIBVIRTD_LISTEN_ADDR NULL @@ -48,7 +44,5 @@ #define LIBVIRT_SERVERKEY LIBVIRT_PKI_DIR "/libvirt/private/serverkey.pem" #define LIBVIRT_SERVERCERT LIBVIRT_PKI_DIR "/libvirt/servercert.pem" -#ifdef __cplusplus -} -#endif + #endif /* __VIR_REMOTE_INTERNAL_H__ */ diff -r b6d8ebb28723 src/test.c --- a/src/test.c Tue Aug 12 22:21:25 2008 +0100 +++ b/src/test.c Tue Aug 12 22:21:48 2008 +0100 @@ -23,16 +23,12 @@ #include <config.h> -#ifdef WITH_TEST - #include <stdio.h> #include <string.h> #include <sys/time.h> #include <fcntl.h> #include <unistd.h> #include <sys/stat.h> - -#include "socketcompat.h" #include "test.h" #include "buf.h" @@ -1645,5 +1641,3 @@ return -1; return 0; } - -#endif /* WITH_TEST */ diff -r b6d8ebb28723 src/test.h --- a/src/test.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/test.h Tue Aug 12 22:21:48 2008 +0100 @@ -26,13 +26,6 @@ #include "internal.h" -#ifdef __cplusplus -extern "C" { -#endif - int testRegister(void); -#ifdef __cplusplus -} -#endif #endif /* __VIR_TEST_INTERNAL_H__ */ diff -r b6d8ebb28723 src/veth.c --- a/src/veth.c Tue Aug 12 22:21:25 2008 +0100 +++ b/src/veth.c Tue Aug 12 22:21:48 2008 +0100 @@ -10,8 +10,6 @@ */ #include <config.h> - -#ifdef WITH_LXC #include <string.h> @@ -218,4 +216,4 @@ VIR_FREE(pid); return rc; } -#endif + diff -r b6d8ebb28723 src/xen_internal.c --- a/src/xen_internal.c Tue Aug 12 22:21:25 2008 +0100 +++ b/src/xen_internal.c Tue Aug 12 22:21:48 2008 +0100 @@ -7,8 +7,6 @@ * * Daniel Veillard <veillard@redhat.com> */ - -#ifdef WITH_XEN #include <config.h> @@ -3293,4 +3291,3 @@ return maxcpu; } -#endif /* WITH_XEN */ diff -r b6d8ebb28723 src/xen_internal.h --- a/src/xen_internal.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/xen_internal.h Tue Aug 12 22:21:48 2008 +0100 @@ -10,10 +10,6 @@ #ifndef __VIR_XEN_INTERNAL_H__ #define __VIR_XEN_INTERNAL_H__ - -#ifdef __cplusplus -extern "C" { -#endif #include "internal.h" #include "capabilities.h" @@ -102,7 +98,5 @@ unsigned long long *freeMems, int startCell, int maxCells); -#ifdef __cplusplus -} -#endif + #endif /* __VIR_XEN_INTERNAL_H__ */ diff -r b6d8ebb28723 src/xen_unified.c --- a/src/xen_unified.c Tue Aug 12 22:21:25 2008 +0100 +++ b/src/xen_unified.c Tue Aug 12 22:21:48 2008 +0100 @@ -9,8 +9,6 @@ */ #include <config.h> - -#ifdef WITH_XEN /* Note: * @@ -1383,4 +1381,3 @@ return virRegisterDriver (&xenUnifiedDriver); } -#endif /* WITH_XEN */ diff -r b6d8ebb28723 src/xen_unified.h --- a/src/xen_unified.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/xen_unified.h Tue Aug 12 22:21:48 2008 +0100 @@ -19,10 +19,6 @@ #include <netinet/in.h> #else #include <winsock2.h> -#endif - -#ifdef __cplusplus -extern "C" { #endif extern int xenUnifiedRegister (void); @@ -93,7 +89,6 @@ */ struct _xenUnifiedPrivate { virCapsPtr caps; -#ifdef WITH_XEN int handle; /* Xen hypervisor handle */ int xendConfigVersion; /* XenD config version */ @@ -107,7 +102,6 @@ struct sockaddr_in addr_in; /* the inet address */ struct xs_handle *xshandle; /* handle to talk to the xenstore */ -#endif /* WITH_XEN */ int proxy; /* fd of proxy. */ @@ -124,8 +118,5 @@ int xenNbCells(virConnectPtr conn); int xenNbCpus(virConnectPtr conn); char *xenDomainUsedCpus(virDomainPtr dom); -#ifdef __cplusplus -} -#endif #endif /* __VIR_XEN_UNIFIED_H__ */ diff -r b6d8ebb28723 src/xend_internal.c --- a/src/xend_internal.c Tue Aug 12 22:21:25 2008 +0100 +++ b/src/xend_internal.c Tue Aug 12 22:21:48 2008 +0100 @@ -10,7 +10,6 @@ * archive for more details. */ -#ifdef WITH_XEN #include <config.h> #include <stdio.h> @@ -5549,4 +5548,3 @@ } #endif /* ! PROXY */ -#endif /* WITH_XEN */ diff -r b6d8ebb28723 src/xend_internal.h --- a/src/xend_internal.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/xend_internal.h Tue Aug 12 22:21:48 2008 +0100 @@ -24,10 +24,6 @@ #include "capabilities.h" #include "domain_conf.h" #include "buf.h" - -#ifdef __cplusplus -extern "C" { -#endif int xenDaemonOpen_unix(virConnectPtr conn, const char *path); diff -r b6d8ebb28723 src/xm_internal.c --- a/src/xm_internal.c Tue Aug 12 22:21:25 2008 +0100 +++ b/src/xm_internal.c Tue Aug 12 22:21:48 2008 +0100 @@ -22,7 +22,6 @@ * */ -#ifdef WITH_XEN #include <config.h> #include <dirent.h> @@ -2685,4 +2684,3 @@ return -1; } -#endif /* WITH_XEN */ diff -r b6d8ebb28723 src/xs_internal.c --- a/src/xs_internal.c Tue Aug 12 22:21:25 2008 +0100 +++ b/src/xs_internal.c Tue Aug 12 22:21:48 2008 +0100 @@ -8,7 +8,6 @@ * Daniel Veillard <veillard@redhat.com> */ -#ifdef WITH_XEN #include <config.h> #include <stdio.h> @@ -938,5 +937,3 @@ return xs_read(priv->xshandle, 0, prop, &len); } - -#endif /* WITH_XEN */ diff -r b6d8ebb28723 src/xs_internal.h --- a/src/xs_internal.h Tue Aug 12 22:21:25 2008 +0100 +++ b/src/xs_internal.h Tue Aug 12 22:21:48 2008 +0100 @@ -10,10 +10,6 @@ #ifndef __VIR_XS_INTERNAL_H__ #define __VIR_XS_INTERNAL_H__ - -#ifdef __cplusplus -extern "C" { -#endif #include "internal.h" @@ -57,7 +53,4 @@ char * xenStoreDomainGetName(virConnectPtr conn, int id); -#ifdef __cplusplus -} -#endif #endif /* __VIR_XS_INTERNAL_H__ */ diff -r b6d8ebb28723 tests/testutils.h --- a/tests/testutils.h Tue Aug 12 22:21:25 2008 +0100 +++ b/tests/testutils.h Tue Aug 12 22:21:48 2008 +0100 @@ -13,40 +13,32 @@ #ifndef __VIT_TEST_UTILS_H__ #define __VIT_TEST_UTILS_H__ -#ifdef __cplusplus -extern "C" { -#endif +double virtTestCountAverage(double *items, + int nitems); +int virtTestRun(const char *title, + int nloops, + int (*body)(const void *data), + const void *data); +int virtTestLoadFile(const char *name, + char **buf, + int buflen); +int virtTestCaptureProgramOutput(const char *const argv[], + char **buf, + int buflen); - double virtTestCountAverage(double *items, - int nitems); - int virtTestRun(const char *title, - int nloops, - int (*body)(const void *data), - const void *data); - int virtTestLoadFile(const char *name, - char **buf, - int buflen); - int virtTestCaptureProgramOutput(const char *const argv[], - char **buf, - int buflen); +int virtTestDifference(FILE *stream, + const char *expect, + const char *actual); - - int virtTestDifference(FILE *stream, - const char *expect, - const char *actual); - - int virtTestMain(int argc, - char **argv, - int (*func)(int, char **)); +int virtTestMain(int argc, + char **argv, + int (*func)(int, char **)); #define VIRT_TEST_MAIN(func) \ int main(int argc, char **argv) { \ return virtTestMain(argc,argv, func); \ } -#ifdef __cplusplus -} -#endif #endif /* __VIT_TEST_UTILS_H__ */ -- |: 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 :|
On Wed, Aug 13, 2008 at 03:17:09PM +0100, Daniel P. Berrange wrote:
Over time we've added lots of general purpose source code files, as wel as making more of the existing ones conditionally compiled. We've done this by adding lots of #ifdef WITH_XXXX macros across the various source files. This is becoming rather a minefield with soo many conditionals sprinkled throughout the code.
With all the recent re-factoring we now have very good separation between generic code, and driver specific code - each typically being in separate files. Thus it is now practical to remove the vast majority of the macros and just do the conditional compilation via the Makefile.am.
The patch is not entirely clear - the resulting Makefile.am is much easier to review. It is structured in two section, first we declare variables for groups of source code files - eg generic helper files, generic domain XML files, generic network XML files, and then per-driver files.
In general this sounds good, I don't like automake conditionals too much, but in that case yes, that's logical. [...]
In removing the unneeded WITH_XXX macros from header/sources I noticed that a bunch of our header files have
#ifdef __cplusplus extern "C" { #endif
With is irrelevant since we're not using C++ anywhere - indeed some of the files even had the corresponding '}' missing, so clearly this is never actually used during compilation. Removing this fixes bogus indentation in some of the header files
Well it should be kept in the public headers, but right internally it's just remains of cut and paste.
I've tested this all by running configure --without-XXX for each hypervisor driver in turn, and it seemed to work in all cases. The only thing I didn't test is MinGW. I think I really need to add a MinGW based build to the nightly build system, so we can sanity check that nightly --- a/configure.in Tue Aug 12 22:21:25 2008 +0100 +++ b/configure.in Tue Aug 12 22:21:47 2008 +0100 @@ -241,27 +241,35 @@ LIBVIRT_FEATURES= WITH_XEN=0
-if test "$with_openvz" = "yes" ; then +if test "$with_openvz" = "yes"; then
hum, unrelated :-)
-if test "$with_qemu" = "yes" ; then +if test "$with_qemu" = "yes" -o "$with_lxc" = "yes" ; then AC_CHECK_HEADERS([linux/param.h linux/sockios.h linux/if_bridge.h linux/if_tun.h],, AC_MSG_ERROR([You must install kernel-headers in order to compile libvirt])) fi oh, good point !
[...]
+if WITH_REMOTE +libvirt_la_SOURCES += $(REMOTE_DRIVER_SOURCES) +endif
I'm wondering about += portability, but I think this is widely used and really clarifies the resulting Makefile.am the other solution libvirt_la_SOURCES = .... if WITH_REMOTE $(REMOTE_DRIVER_SOURCES) endif if WITH_XEN .... might be a bit more portable but messes the structure, No other comment, everything else is small bugs, the cleanup of headers and reindent, Fine by me, +1 Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
This patch is the important one, switching from an I/O controller which is simply fork'd off libvirtd, to a properly execable libvirt_lxc binary. The libvirtd LXC driver writes the config it wants to launch with to /var/run/libvirt/lxc/NAME.log and invokves libvirt_lxc --name NAME This then loads the config and spawns the container. It also expects a '--console FD' arg to provide a file descriptor for the master PTY to use for the container's stdin/out/err. Currently this is compulsory but we can make it optional in future and have the container stdin/out/err plumbe straight to the libvirt_lxc process' existing stdin/out/err. This would allow launching containers standalone. If networking is configured, then we also pass '--veth NAME' for each configured interface specifying the name of the veth that will be moved into the container's namespace. The libvirt_lxc process writes a PID file to /var/run/libvirt/lxc/NAME.pid and opens a UNIX socket in the same dir - this is how libvirtd discovers when the container dies. a/src/lxc_controller.h | 41 ------ src/Makefile.am | 26 +++ src/domain_conf.c | 6 src/lxc_conf.c | 8 - src/lxc_conf.h | 4 src/lxc_controller.c | 320 +++++++++++++++++++++++++++++++------------------ src/lxc_driver.c | 191 +++++++++++++++++++---------- 7 files changed, 366 insertions(+), 230 deletions(-) Daniel diff -r 1cf789924625 src/Makefile.am --- a/src/Makefile.am Tue Aug 12 22:21:48 2008 +0100 +++ b/src/Makefile.am Tue Aug 12 22:22:37 2008 +0100 @@ -88,8 +88,13 @@ LXC_DRIVER_SOURCES = \ lxc_conf.c lxc_conf.h \ lxc_container.c lxc_container.h \ - lxc_controller.c lxc_controller.h \ lxc_driver.c lxc_driver.h \ + veth.c veth.h + +LXC_CONTROLLER_SOURCES = \ + lxc_conf.c lxc_conf.h \ + lxc_container.c lxc_container.h \ + lxc_controller.c \ veth.c veth.h OPENVZ_DRIVER_SOURCES = \ @@ -272,9 +277,11 @@ rm -f $@ mv $@-tmp $@ +libexec_PROGRAMS = + if WITH_STORAGE_DISK if WITH_LIBVIRTD -libexec_PROGRAMS = libvirt_parthelper +libexec_PROGRAMS += libvirt_parthelper libvirt_parthelper_SOURCES = $(STORAGE_HELPER_DISK_SOURCES) libvirt_parthelper_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDCFLAGS) @@ -284,6 +291,21 @@ endif EXTRA_DIST += $(STORAGE_HELPER_DISK_SOURCES) + +if WITH_LXC +if WITH_LIBVIRTD +libexec_PROGRAMS += libvirt_lxc + +libvirt_lxc_SOURCES = \ + $(LXC_CONTROLLER_SOURCES) \ + $(GENERIC_LIB_SOURCES) \ + $(DOMAIN_CONF_SOURCES) +libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDCFLAGS) +libvirt_lxc_LDADD = $(LIBXML_LIBS) ../gnulib/lib/libgnu.la +libvirt_lxc_CFLAGS = $(LIBPARTED_CFLAGS) +endif +endif +EXTRA_DIST += $(LXC_CONTROLLER_SOURCES) # Create the /var/cache/libvirt directory when installing. install-exec-local: diff -r 1cf789924625 src/domain_conf.c --- a/src/domain_conf.c Tue Aug 12 22:21:48 2008 +0100 +++ b/src/domain_conf.c Tue Aug 12 22:22:37 2008 +0100 @@ -1109,13 +1109,11 @@ break; case VIR_DOMAIN_CHR_TYPE_PTY: - /* @path attribute is an output only property - pty is auto-allocted */ - break; - case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: case VIR_DOMAIN_CHR_TYPE_PIPE: - if (path == NULL) { + if (path == NULL && + def->type != VIR_DOMAIN_CHR_TYPE_PTY) { virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("Missing source path attribute for char device")); goto error; diff -r 1cf789924625 src/lxc_conf.c --- a/src/lxc_conf.c Tue Aug 12 22:21:48 2008 +0100 +++ b/src/lxc_conf.c Tue Aug 12 22:22:37 2008 +0100 @@ -71,7 +71,7 @@ "exe", utsname.machine, sizeof(int) == 4 ? 32 : 8, - NULL, + BINDIR "/libvirt_lxc", NULL, 0, NULL)) == NULL) @@ -94,11 +94,11 @@ int lxcLoadDriverConfig(lxc_driver_t *driver) { /* Set the container configuration directory */ - if ((driver->configDir = strdup(SYSCONF_DIR "/libvirt/lxc")) == NULL) + if ((driver->configDir = strdup(LXC_CONFIG_DIR)) == NULL) goto no_memory; - if ((driver->stateDir = strdup(LOCAL_STATE_DIR "/run/libvirt/lxc")) == NULL) + if ((driver->stateDir = strdup(LXC_STATE_DIR)) == NULL) goto no_memory; - if ((driver->logDir = strdup(LOCAL_STATE_DIR "/log/libvirt/lxc")) == NULL) + if ((driver->logDir = strdup(LXC_LOG_DIR)) == NULL) goto no_memory; return 0; diff -r 1cf789924625 src/lxc_conf.h --- a/src/lxc_conf.h Tue Aug 12 22:21:48 2008 +0100 +++ b/src/lxc_conf.h Tue Aug 12 22:22:37 2008 +0100 @@ -30,6 +30,10 @@ #include "domain_conf.h" #include "capabilities.h" +#define LXC_CONFIG_DIR SYSCONF_DIR "/libvirt/lxc" +#define LXC_STATE_DIR LOCAL_STATE_DIR "/run/libvirt/lxc" +#define LXC_LOG_DIR LOCAL_STATE_DIR "/log/libvirt/lxc" + typedef struct __lxc_driver lxc_driver_t; struct __lxc_driver { virCapsPtr caps; diff -r 1cf789924625 src/lxc_controller.c --- a/src/lxc_controller.c Tue Aug 12 22:21:48 2008 +0100 +++ b/src/lxc_controller.c Tue Aug 12 22:22:37 2008 +0100 @@ -23,22 +23,22 @@ #include <config.h> -#ifdef WITH_LXC - #include <sys/epoll.h> #include <sys/wait.h> #include <sys/socket.h> +#include <sys/types.h> +#include <sys/un.h> #include <unistd.h> #include <paths.h> #include <fcntl.h> #include <signal.h> +#include <getopt.h> #include "internal.h" #include "util.h" #include "lxc_conf.h" #include "lxc_container.h" -#include "lxc_controller.h" #include "veth.h" #include "memory.h" #include "util.h" @@ -47,6 +47,56 @@ #define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt, __VA_ARGS__) #define DEBUG0(msg) VIR_DEBUG(__FILE__, "%s", msg) +int debugFlag = 0; + +static char*lxcMonitorPath(virDomainDefPtr def) +{ + char *sockpath; + if (asprintf(&sockpath, "%s/%s.sock", + LXC_STATE_DIR, def->name) < 0) { + lxcError(NULL, NULL, VIR_ERR_NO_MEMORY, NULL); + return NULL; + } + return sockpath; +} + +static int lxcMonitorServer(const char *sockpath) +{ + int fd; + struct sockaddr_un addr; + + if ((fd = socket(PF_UNIX, SOCK_STREAM, 0)) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to create server socket %s: %s"), + sockpath, strerror(errno)); + goto error; + } + + unlink(sockpath); + memset(&addr, 0, sizeof(addr)); + addr.sun_family = AF_UNIX; + strncpy(addr.sun_path, sockpath, sizeof(addr.sun_path)); + + if (bind(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to bind server socket %s: %s"), + sockpath, strerror(errno)); + goto error; + } + if (listen(fd, 30 /* backlog */ ) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to listen server socket %s: %s"), + sockpath, strerror(errno)); + goto error; + } + + return fd; + +error: + if (fd != -1) + close(fd); + return -1; +} /** * lxcFdForward: @@ -305,8 +355,7 @@ static int -lxcControllerRun(const char *stateDir, - virDomainDefPtr def, +lxcControllerRun(virDomainDefPtr def, unsigned int nveths, char **veths, int monitor, @@ -359,148 +408,187 @@ if (containerPty != -1) close(containerPty); - kill(container, SIGTERM); - waitpid(container, NULL, 0); - lxcControllerCleanupInterfaces(nveths, veths); - virFileDeletePid(stateDir, def->name); + if (container > 1) { + kill(container, SIGTERM); + waitpid(container, NULL, 0); + } return rc; } -int lxcControllerStart(const char *stateDir, - virDomainDefPtr def, - unsigned int nveths, - char **veths, - int monitor, - int appPty, - int logfd) +int main(int argc, char *argv[]) { pid_t pid; - int rc; - int status, null; - int open_max, i; + int rc = 1; int client; - struct sigaction sig_action; + char *name = NULL; + int nveths = 0; + char **veths = NULL; + int monitor = -1; + int appPty = -1; + int bg = 0; + virCapsPtr caps = NULL; + virDomainDefPtr def = NULL; + int nnets = 0; + virDomainNetDefPtr nets = NULL; + char *configFile = NULL; + char *sockpath = NULL; + const struct option const options[] = { + { "background", 0, NULL, 'b' }, + { "name", 1, NULL, 'n' }, + { "veth", 1, NULL, 'v' }, + { "console", 1, NULL, 'c' }, + { "help", 0, NULL, 'h' }, + { 0, 0, 0, 0 }, + }; - if ((pid = fork()) < 0) - return -1; + while (1) { + int c; - if (pid > 0) { - /* Original caller waits for first child to exit */ - while (1) { - rc = waitpid(pid, &status, 0); - if (rc < 0) { - if (errno == EINTR) - continue; - return -1; + c = getopt_long(argc, argv, "dn:v:m:c:h", + options, NULL); + + if (c == -1) + break; + + switch (c) { + case 'b': + bg = 1; + break; + + case 'n': + if ((name = strdup(optarg)) == NULL) { + fprintf(stderr, "%s", strerror(errno)); + goto cleanup; } - if (rc != pid) { - fprintf(stderr, - _("Unexpected pid %d != %d from waitpid\n"), - rc, pid); - return -1; + break; + + case 'v': + if (VIR_REALLOC_N(veths, nveths+1) < 0) { + fprintf(stderr, "cannot allocate veths %s", strerror(errno)); + goto cleanup; } - if (WIFEXITED(status) && - WEXITSTATUS(status) == 0) - return 0; - else { - fprintf(stderr, - _("Unexpected status %d from pid %d\n"), - status, pid); - return -1; + if ((veths[nveths++] = strdup(optarg)) == NULL) { + fprintf(stderr, "cannot allocate veth name %s", strerror(errno)); + goto cleanup; } + break; + + case 'c': + if (virStrToLong_i(optarg, NULL, 10, &appPty) < 0) { + fprintf(stderr, "malformed --console argument '%s'", optarg); + goto cleanup; + } + break; + + case 'h': + case '?': + fprintf(stderr, "\n"); + fprintf(stderr, "syntax: %s [OPTIONS]\n", argv[0]); + fprintf(stderr, "\n"); + fprintf(stderr, "Options\n"); + fprintf(stderr, "\n"); + fprintf(stderr, " -b, --background\n"); + fprintf(stderr, " -n NAME, --name NAME\n"); + fprintf(stderr, " -c FD, --console FD\n"); + fprintf(stderr, " -v VETH, --veth VETH\n"); + fprintf(stderr, " -h, --help\n"); + fprintf(stderr, "\n"); + goto cleanup; } } - /* First child is running here */ - /* Clobber all libvirtd's signal handlers so they - * don't affect us - */ - sig_action.sa_handler = SIG_DFL; - sig_action.sa_flags = 0; - sigemptyset(&sig_action.sa_mask); - - 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); - - sig_action.sa_handler = SIG_IGN; - sigaction(SIGPIPE, &sig_action, NULL); - - - /* Don't hold onto any cwd we inherit from libvirtd either */ - if (chdir("/") < 0) { - fprintf(stderr, _("Unable to change to root dir: %s\n"), - strerror(errno)); - _exit(-1); + if (name == NULL) { + fprintf(stderr, "%s: missing --name argument for configuration\n", argv[0]); + goto cleanup; } - if (setsid() < 0) { - fprintf(stderr, _("Unable to become session leader: %s\n"), - strerror(errno)); - _exit(-1); + if (appPty < 0) { + fprintf(stderr, "%s: missing --console argument for container PTY\n", argv[0]); + goto cleanup; } - if ((null = open(_PATH_DEVNULL, O_RDONLY)) < 0) { - fprintf(stderr, _("Unable to open %s: %s\n"), - _PATH_DEVNULL, strerror(errno)); - _exit(-1); + if (getuid() && 0) { + fprintf(stderr, "%s: must be run as the 'root' user\n", argv[0]); + goto cleanup; } - open_max = sysconf (_SC_OPEN_MAX); - for (i = 0; i < open_max; i++) - if (i != appPty && - i != monitor && - i != logfd && - i != null) - close(i); + if ((caps = lxcCapsInit()) == NULL) + goto cleanup; - if (dup2(null, STDIN_FILENO) < 0 || - dup2(logfd, STDOUT_FILENO) < 0 || - dup2(logfd, STDERR_FILENO) < 0) { - fprintf(stderr, _("Unable to redirect stdio: %s\n"), - strerror(errno)); - _exit(-1); + if ((configFile = virDomainConfigFile(NULL, + LXC_STATE_DIR, + name)) == NULL) + goto cleanup; + + if ((def = virDomainDefParseFile(NULL, caps, configFile)) == NULL) + goto cleanup; + + nets = def->nets; + while (nets) { + nnets++; + nets = nets->next; + } + if (nnets != nveths) { + fprintf(stderr, "%s: expecting %d veths, but got %d\n", + argv[0], nnets, nveths); + goto cleanup; } - close(null); - close(logfd); + if ((sockpath = lxcMonitorPath(def)) == NULL) + goto cleanup; - /* Now fork the real controller process */ - if ((pid = fork()) < 0) { - fprintf(stderr, _("Unable to fork controller: %s\n"), - strerror(errno)); - _exit(-1); + if ((monitor = lxcMonitorServer(sockpath)) < 0) + goto cleanup; + + if (bg) { + if ((pid = fork()) < 0) + goto cleanup; + + if (pid > 0) { + if ((rc = virFileWritePid(LXC_STATE_DIR, name, pid)) != 0) { + fprintf(stderr, _("Unable to write pid file: %s\n"), + strerror(rc)); + _exit(1); + } + + /* First child now exits, allowing original caller + * (ie libvirtd's LXC driver to complete their + * waitpid & continue */ + _exit(0); + } + + /* Don't hold onto any cwd we inherit from libvirtd either */ + if (chdir("/") < 0) { + fprintf(stderr, _("Unable to change to root dir: %s\n"), + strerror(errno)); + goto cleanup; + } + + if (setsid() < 0) { + fprintf(stderr, _("Unable to become session leader: %s\n"), + strerror(errno)); + goto cleanup; + } } - if (pid > 0) { - if ((rc = virFileWritePid(stateDir, def->name, pid)) != 0) { - fprintf(stderr, _("Unable to write pid file: %s\n"), - strerror(rc)); - _exit(-1); - } - /* First child now exits, allowing originall caller to - * complete their waitpid & continue */ - _exit(0); + /* Accept initial client which is the libvirtd daemon */ + if ((client = accept(monitor, NULL, 0)) < 0) { + fprintf(stderr, _("Failed connection from LXC driver: %s\n"), + strerror(errno)); + goto cleanup; } - /* This is real controller running finally... */ + rc = lxcControllerRun(def, nveths, veths, monitor, client, appPty); - /* Accept initial client which is the libvirtd daemon */ - if ((client = accept(monitor, NULL, 0))) { - fprintf(stderr, _("Failed connection from LXC driver: %s\n"), - strerror(errno)); - _exit(-1); - } - /* Controlling libvirtd LXC driver now knows - what our PID is, and is able to cleanup after - us from now on */ - _exit(lxcControllerRun(stateDir, def, nveths, veths, monitor, client, appPty)); +cleanup: + virFileDeletePid(LXC_STATE_DIR, def->name); + lxcControllerCleanupInterfaces(nveths, veths); + unlink(sockpath); + VIR_FREE(sockpath); + + return rc; } - -#endif diff -r 1cf789924625 src/lxc_controller.h --- a/src/lxc_controller.h Tue Aug 12 22:21:48 2008 +0100 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,41 +0,0 @@ -/* - * Copyright IBM Corp. 2008 - * - * lxc_controller.h: linux container process controller - * - * Authors: - * David L. Leskovec <dlesko at linux.vnet.ibm.com> - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA - */ - -#ifndef LXC_CONTROLLER_H -#define LXC_CONTROLLER_H - -#ifdef WITH_LXC - -#include "lxc_conf.h" - -int lxcControllerStart(const char *stateDir, - virDomainDefPtr def, - unsigned int nveths, - char **veths, - int monitor, - int appPty, - int logfd); - -#endif /* WITH_LXC */ - -#endif /* LXC_CONTROLLER_H */ diff -r 1cf789924625 src/lxc_driver.c --- a/src/lxc_driver.c Tue Aug 12 22:21:48 2008 +0100 +++ b/src/lxc_driver.c Tue Aug 12 22:22:37 2008 +0100 @@ -38,7 +38,6 @@ #include "lxc_conf.h" #include "lxc_container.h" #include "lxc_driver.h" -#include "lxc_controller.h" #include "memory.h" #include "util.h" #include "bridge.h" @@ -398,6 +397,7 @@ close(vm->monitor); virFileDeletePid(driver->stateDir, vm->def->name); + virDomainDeleteConfig(conn, driver->stateDir, NULL, vm); vm->state = VIR_DOMAIN_SHUTOFF; vm->pid = -1; @@ -507,55 +507,6 @@ return rc; } -static int lxcMonitorServer(virConnectPtr conn, - lxc_driver_t * driver, - virDomainObjPtr vm) -{ - char *sockpath = NULL; - int fd; - struct sockaddr_un addr; - - if (asprintf(&sockpath, "%s/%s.sock", - driver->stateDir, vm->def->name) < 0) { - lxcError(conn, NULL, VIR_ERR_NO_MEMORY, NULL); - return -1; - } - - if ((fd = socket(PF_UNIX, SOCK_STREAM, 0)) < 0) { - lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to create server socket: %s"), - strerror(errno)); - goto error; - } - - unlink(sockpath); - memset(&addr, 0, sizeof(addr)); - addr.sun_family = AF_UNIX; - strncpy(addr.sun_path, sockpath, sizeof(addr.sun_path)); - - if (bind(fd, (struct sockaddr *) &addr, sizeof(addr)) < 0) { - lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to bind server socket: %s"), - strerror(errno)); - goto error; - } - if (listen(fd, 30 /* backlog */ ) < 0) { - lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to listen server socket: %s"), - strerror(errno)); - goto error; - return (-1); - } - - VIR_FREE(sockpath); - return fd; - -error: - VIR_FREE(sockpath); - if (fd != -1) - close(fd); - return -1; -} static int lxcMonitorClient(virConnectPtr conn, lxc_driver_t * driver, @@ -608,6 +559,12 @@ if (signum == 0) signum = SIGINT; + if (vm->pid <= 0) { + lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, + _("invalid PID %d for container"), vm->pid); + return -1; + } + if (kill(vm->pid, signum) < 0) { if (errno != ESRCH) { lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, @@ -644,6 +601,102 @@ } +static int lxcControllerStart(virConnectPtr conn, + virDomainObjPtr vm, + int nveths, + char **veths, + int appPty, + int logfd) +{ + int i; + int rc; + int ret = -1; + int largc = 0, larga = 0; + const char **largv = NULL; + pid_t child; + int status; + +#define ADD_ARG_SPACE \ + do { \ + if (largc == larga) { \ + larga += 10; \ + if (VIR_REALLOC_N(largv, larga) < 0) \ + goto no_memory; \ + } \ + } while (0) + +#define ADD_ARG(thisarg) \ + do { \ + ADD_ARG_SPACE; \ + largv[largc++] = thisarg; \ + } while (0) + +#define ADD_ARG_LIT(thisarg) \ + do { \ + ADD_ARG_SPACE; \ + if ((largv[largc++] = strdup(thisarg)) == NULL) \ + goto no_memory; \ + } while (0) + + ADD_ARG_LIT(vm->def->emulator); + ADD_ARG_LIT("--name"); + ADD_ARG_LIT(vm->def->name); + ADD_ARG_LIT("--console"); + ADD_ARG_LIT("0"); /* Passing console master PTY as FD 0 */ + ADD_ARG_LIT("--background"); + + for (i = 0 ; i < nveths ; i++) { + ADD_ARG_LIT("--veth"); + ADD_ARG_LIT(veths[i]); + } + + ADD_ARG(NULL); + + vm->stdin_fd = appPty; /* Passing console master PTY as FD 0 */ + vm->stdout_fd = vm->stderr_fd = logfd; + + if (virExec(conn, largv, NULL, &child, + vm->stdin_fd, &vm->stdout_fd, &vm->stderr_fd, + VIR_EXEC_NONE) < 0) + goto cleanup; + + /* We now wait for the process to exit - the controller + * will fork() itself into the background - waiting for + * it to exit thus guarentees it has written its pidfile + */ + while ((rc = waitpid(child, &status, 0) == -1) && errno == EINTR); + if (rc == -1) { + lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, + _("cannot wait for '%s': %s"), + largv[0], strerror(errno)); + goto cleanup; + } + + if (!(WIFEXITED(status) && WEXITSTATUS(status) == 0)) { + lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, + _("container '%s' unexpectedly shutdown during startup"), + largv[0]); + goto cleanup; + } + +#undef ADD_ARG +#undef ADD_ARG_LIT +#undef ADD_ARG_SPACE + + ret = 0; + +cleanup: + for (i = 0 ; i < largc ; i++) + VIR_FREE(largv[i]); + + return ret; + +no_memory: + lxcError(conn, NULL, VIR_ERR_NO_MEMORY, NULL); + goto cleanup; +} + + /** * lxcVmStart: * @conn: pointer to connection @@ -660,7 +713,6 @@ { int rc = -1; unsigned int i; - int monitor; int parentTty; char *parentTtyPath = NULL; char *logfile = NULL; @@ -681,9 +733,6 @@ return -1; } - if ((monitor = lxcMonitorServer(conn, driver, vm)) < 0) - goto cleanup; - /* open parent tty */ if (virFileOpenTty(&parentTty, &parentTtyPath, 1) < 0) { lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, @@ -702,6 +751,12 @@ if (lxcSetupInterfaces(conn, vm->def, &nveths, &veths) != 0) goto cleanup; + /* Persist the live configuration now we have veth & tty info */ + if (virDomainSaveConfig(conn, driver->stateDir, vm->def) < 0) { + rc = -1; + goto cleanup; + } + if ((logfd = open(logfile, O_WRONLY | O_TRUNC | O_CREAT, S_IRUSR|S_IWUSR)) < 0) { lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, @@ -710,14 +765,11 @@ goto cleanup; } - if (lxcControllerStart(driver->stateDir, - vm->def, nveths, veths, - monitor, parentTty, logfd) < 0) + if (lxcControllerStart(conn, + vm, + nveths, veths, + parentTty, logfd) < 0) goto cleanup; - /* Close the server side of the monitor, now owned - * by the controller process */ - close(monitor); - monitor = -1; /* Connect to the controller as a client *first* because * this will block until the child has written their @@ -753,8 +805,6 @@ vethDelete(veths[i]); VIR_FREE(veths[i]); } - if (monitor != -1) - close(monitor); if (rc != 0 && vm->monitor != -1) { close(vm->monitor); vm->monitor = -1; @@ -951,6 +1001,8 @@ vm = lxc_driver->domains; while (vm) { + char *config = NULL; + virDomainDefPtr tmp; int rc; if ((vm->monitor = lxcMonitorClient(NULL, lxc_driver, vm)) < 0) { vm = vm->next; @@ -963,6 +1015,19 @@ vm->monitor = -1; vm = vm->next; continue; + } + + if ((config = virDomainConfigFile(NULL, + lxc_driver->stateDir, + vm->def->name)) == NULL) + continue; + + /* Try and load the live config */ + tmp = virDomainDefParseFile(NULL, lxc_driver->caps, config); + VIR_FREE(config); + if (tmp) { + vm->newDef = vm->def; + vm->def = tmp; } if (vm->pid != 0) { -- |: 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 :|
DB> This patch is the important one, switching from an I/O controller DB> which is simply fork'd off libvirtd, to a properly execable DB> libvirt_lxc binary. This works for me. The network interfaces aren't cleaned up properly on destroy, and I get an error from the kernel about a reference count on the fake 'lo' interface for the container. However, I get that some of the time in the original code as well, so I don't see any reason holding this stuff up any further, and I will see about tracking that down. Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
On Wed, Aug 13, 2008 at 10:20:03AM -0700, Dan Smith wrote:
DB> This patch is the important one, switching from an I/O controller DB> which is simply fork'd off libvirtd, to a properly execable DB> libvirt_lxc binary.
This works for me. The network interfaces aren't cleaned up properly on destroy, and I get an error from the kernel about a reference count on the fake 'lo' interface for the container. However, I get that some of the time in the original code as well, so I don't see any reason holding this stuff up any further, and I will see about tracking that down.
The deletion of veth's is the last thing the controller process does. So assuming it doesn't crash they should be cleaned up. It is probably worth making the libvirtd LXC driver do cleanup of veths too, as a safety net. Daniel -- |: 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 :|
DB> The deletion of veth's is the last thing the controller process DB> does. So assuming it doesn't crash they should be cleaned up. Well, a destroy operation kills the container and leaves the veth devices 100% of the time for me. While attempting to test that, I noticed that if I have a container with just bash in it, and that process exits, it stays in zombie state with the container running until I destroy it. The interfaces aren't cleaned up in that scenario either. I'll play around with it and see what I can figure out. DB> It is probably worth making the libvirtd LXC driver do cleanup of DB> veths too, as a safety net. Agreed. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
On Thu, Aug 14, 2008 at 07:55:02AM -0700, Dan Smith wrote:
DB> The deletion of veth's is the last thing the controller process DB> does. So assuming it doesn't crash they should be cleaned up.
Well, a destroy operation kills the container and leaves the veth devices 100% of the time for me.
Oh of course - libvirt_lxc isn't installing any signal handler for SIGTERM so its never doing its cleanup - it just terminates from the signal handler directly :-)
I'll play around with it and see what I can figure out.
DB> It is probably worth making the libvirtd LXC driver do cleanup of DB> veths too, as a safety net.
Definitely need this to make 'destroy' cleanup properly then Daniel -- |: 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 :|
This isn't really related to the others, except for the fact that is part of the LXC driver. This is a re-post of the patch I did for adding support for pivot_root() in the container. This allows the entire container FS to be separated from the parent OS. As noted previously this is not currently secure, since Linux has not yet gained device namespace support, but is a stepping stone toward the final solution we'll need. Since the last posting I've added the explicit chmod() suggested by Jim, and the various other bug fixes lxc_container.c | 265 +++++++++++++++++++++++++++++++++++++++++++++++++------- util.c | 12 +- 2 files changed, 237 insertions(+), 40 deletions(-) Daniel diff -r 17f02fec7fe8 src/lxc_container.c --- a/src/lxc_container.c Wed Aug 13 14:39:28 2008 +0100 +++ b/src/lxc_container.c Wed Aug 13 14:39:34 2008 +0100 @@ -1,10 +1,12 @@ /* * Copyright IBM Corp. 2008 + * Copyright Red Hat 2008 * * lxc_container.c: file description * * Authors: * David L. Leskovec <dlesko at linux.vnet.ibm.com> + * Daniel P. Berrange <berrange@redhat.com> * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -26,10 +28,18 @@ #include <fcntl.h> #include <limits.h> #include <stdlib.h> +#include <stdio.h> #include <sys/ioctl.h> #include <sys/mount.h> #include <sys/wait.h> #include <unistd.h> +#include <mntent.h> + +/* Yes, we want linux private one, for _syscall2() macro */ +#include <linux/unistd.h> + +/* For MS_MOVE */ +#include <linux/fs.h> #include "lxc_container.h" #include "util.h" @@ -103,23 +113,15 @@ * * Returns 0 on success or -1 in case of error */ -static int lxcContainerSetStdio(int control, const char *ttyPath) +static int lxcContainerSetStdio(int control, int ttyfd) { int rc = -1; - int ttyfd; int open_max, i; if (setsid() < 0) { lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("setsid failed: %s"), strerror(errno)); - goto error_out; - } - - ttyfd = open(ttyPath, O_RDWR|O_NOCTTY); - if (ttyfd < 0) { - lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("open(%s) failed: %s"), ttyPath, strerror(errno)); - goto error_out; + goto cleanup; } if (ioctl(ttyfd, TIOCSCTTY, NULL) < 0) { @@ -156,9 +158,6 @@ rc = 0; cleanup: - close(ttyfd); - -error_out: return rc; } @@ -221,6 +220,7 @@ return 0; } + /** * lxcEnableInterfaces: * @vm: Pointer to vm structure @@ -251,6 +251,20 @@ return rc; } + +//_syscall2(int, pivot_root, char *, newroot, const char *, oldroot) +extern int pivot_root(const char * new_root,const char * put_old); + +static int lxcContainerChildMountSort(const void *a, const void *b) +{ + const char **sa = (const char**)a; + const char **sb = (const char**)b; + + /* Delibrately reversed args - we need to unmount deepest + children first */ + return strcmp(*sb, *sa); +} + /** * lxcChild: * @argv: Pointer to container arguments @@ -268,8 +282,8 @@ int rc = -1; lxc_child_argv_t *argv = data; virDomainDefPtr vmDef = argv->config; - virDomainFSDefPtr curMount; - int i; + virDomainFSDefPtr tmp, root = NULL; + int ttyfd, i; if (NULL == vmDef) { lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, @@ -277,37 +291,222 @@ return -1; } +#if 0 + ttyfd = open(argv->ttyPath, O_RDWR|O_NOCTTY); + if (ttyfd < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("open(%s) failed: %s"), argv->ttyPath, strerror(errno)); + return -1; + } +#endif + /* handle the bind mounts first before doing anything else that may */ /* then access those mounted dirs */ - curMount = vmDef->fss; - for (i = 0; curMount; curMount = curMount->next) { - // XXX fix - if (curMount->type != VIR_DOMAIN_FS_TYPE_MOUNT) + for (tmp = vmDef->fss; tmp && !root; tmp = tmp->next) { + if (tmp->type != VIR_DOMAIN_FS_TYPE_MOUNT) continue; - rc = mount(curMount->src, - curMount->dst, - NULL, - MS_BIND, - NULL); - if (0 != rc) { + if (STREQ(tmp->dst, "/")) + root = tmp; + } + + if (root) { + char *oldroot; + struct mntent *mntent; + char **mounts = NULL; + int nmounts = 0; + FILE *procmnt; + const struct { + int maj; + int min; + mode_t mode; + const char *path; + } devs[] = { + { 1, 3, 0666, "/dev/null" }, + { 1, 5, 0666, "/dev/zero" }, + { 1, 7, 0666, "/dev/full" }, + { 5, 1, 0600, "/dev/console" }, + { 1, 8, 0666, "/dev/random" }, + { 1, 9, 0666, "/dev/urandom" }, + }; + + /* Got a FS mapped to /, we're going the pivot_root + approach to do a better-chroot-than-chroot */ + + /* this is based on this thread http://lkml.org/lkml/2008/3/5/29 */ + + /* First step is to ensure the new root itself is + a mount point */ + if (mount(root->src, root->src, NULL, MS_BIND, NULL) < 0) { lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to mount %s at %s for container: %s"), - curMount->src, curMount->dst, strerror(errno)); + _("failed to bind new root %s: %s"), + root->src, strerror(errno)); + return -1; + } + + if (asprintf(&oldroot, "%s/.oldroot", root->src) < 0) { + lxcError(NULL, NULL, VIR_ERR_NO_MEMORY, NULL); + return -1; + } + + if (virFileMakePath(oldroot) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to create %s: %s"), + oldroot, strerror(errno)); + return -1; + } + + /* The old root directory will live at /.oldroot after + * this and will soon be unmounted completely */ + if (pivot_root(root->src, oldroot) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to pivot root %s to %s: %s"), + oldroot, root->src, strerror(errno)); + return -1; + } + + /* CWD is undefined after pivot_root, so go to / */ + if (chdir("/") < 0) { + return -1; + } + + if (virFileMakePath("/proc") < 0 || + mount("none", "/proc", "proc", 0, NULL) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to mount /proc for container: %s"), + strerror(errno)); + return -1; + } + if (virFileMakePath("/dev") < 0 || + mount("none", "/dev", "tmpfs", 0, NULL) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to mount /dev tmpfs for container: %s"), + strerror(errno)); + return -1; + } + /* Move old devpts into container, since we have to + connect to the master ptmx which was opened in + the parent. + XXX This sucks, we need to figure out how to get our + own private devpts for isolation + */ + if (virFileMakePath("/dev/pts") < 0 || + mount("/.oldroot/dev/pts", "/dev/pts", NULL, + MS_MOVE, NULL) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to move /dev/pts into container: %s"), + strerror(errno)); + return -1; + } + + /* Populate /dev/ with a few important bits */ + for (i = 0 ; i < ARRAY_CARDINALITY(devs) ; i++) { + dev_t dev = makedev(devs[i].maj, devs[i].min); + if (mknod(devs[i].path, 0, dev) < 0 || + chmod(devs[i].path, devs[i].mode)) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to make device %s: %s"), + devs[i].path, strerror(errno)); + return -1; + } + } + + /* Pull in rest of container's mounts */ + for (tmp = vmDef->fss; tmp; tmp = tmp->next) { + char *src; + if (STREQ(tmp->dst, "/")) + continue; + // XXX fix + if (tmp->type != VIR_DOMAIN_FS_TYPE_MOUNT) + continue; + + if (asprintf(&src, "/.oldroot/%s", tmp->src) < 0) + return -1; + + if (virFileMakePath(tmp->dst) < 0 || + mount(src, tmp->dst, NULL, MS_BIND, NULL) < 0) { + VIR_FREE(src); + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to mount %s at %s for container: %s"), + tmp->src, tmp->dst, strerror(errno)); + return -1; + } + VIR_FREE(src); + } + + if (!(procmnt = setmntent("/proc/mounts", "r"))) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to read /proc/mounts: %s"), + strerror(errno)); + return -1; + } + while ((mntent = getmntent(procmnt)) != NULL) { + if (!STRPREFIX(mntent->mnt_dir, "/.oldroot")) + continue; + if (VIR_REALLOC_N(mounts, nmounts+1) < 0) { + endmntent(procmnt); + return -1; + } + if (!(mounts[nmounts++] = strdup(mntent->mnt_dir))) { + endmntent(procmnt); + return -1; + } + } + endmntent(procmnt); + + qsort(mounts, nmounts, sizeof(mounts[0]), + lxcContainerChildMountSort); + + for (i = 0 ; i < nmounts ; i++) { + if (umount(mounts[i]) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to unmount %s: %s"), + mounts[i], strerror(errno)); + return -1; + } + VIR_FREE(mounts[i]); + } + VIR_FREE(mounts); + } else { + /* Nothing mapped to /, we're using the main root, + but with extra stuff mapped in */ + for (tmp = vmDef->fss; tmp; tmp = tmp->next) { + // XXX fix + if (tmp->type != VIR_DOMAIN_FS_TYPE_MOUNT) + continue; + rc = mount(tmp->src, + tmp->dst, + NULL, + MS_BIND, + NULL); + if (0 != rc) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to mount %s at %s for container: %s"), + tmp->src, tmp->dst, strerror(errno)); + return -1; + } + } + + /* mount /proc */ + if (mount("lxcproc", "/proc", "proc", 0, NULL) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to mount /proc for container: %s"), + strerror(errno)); return -1; } } - /* mount /proc */ - rc = mount("lxcproc", "/proc", "proc", 0, NULL); - if (0 != rc) { + ttyfd = open(argv->ttyPath, O_RDWR|O_NOCTTY); + if (ttyfd < 0) { lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to mount /proc for container: %s"), - strerror(errno)); + _("open(%s) failed: %s"), argv->ttyPath, strerror(errno)); return -1; } - if (lxcContainerSetStdio(argv->monitor, argv->ttyPath) < 0) + if (lxcContainerSetStdio(argv->monitor, ttyfd) < 0) { + close(ttyfd); return -1; + } + close(ttyfd); /* Wait for interface devices to show up */ if (lxcContainerWaitForContinue(argv->monitor) < 0) diff -r 17f02fec7fe8 src/util.c --- a/src/util.c Wed Aug 13 14:39:28 2008 +0100 +++ b/src/util.c Wed Aug 13 14:39:34 2008 +0100 @@ -604,13 +604,11 @@ if (!(p = strrchr(parent, '/'))) return EINVAL; - if (p == parent) - return EPERM; - - *p = '\0'; - - if ((err = virFileMakePath(parent))) - return err; + if (p != parent) { + *p = '\0'; + if ((err = virFileMakePath(parent))) + return err; + } if (mkdir(path, 0777) < 0 && errno != EEXIST) return errno; -- |: 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 :|
DB> +#if 0 DB> + ttyfd = open(argv->ttyPath, O_RDWR|O_NOCTTY); DB> + if (ttyfd < 0) { DB> + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB> + _("open(%s) failed: %s"), argv->ttyPath, strerror(errno)); DB> + return -1; DB> + } DB> +#endif Should this bit be removed? DB> + if (root) { DB> + char *oldroot; DB> + struct mntent *mntent; DB> + char **mounts = NULL; DB> + int nmounts = 0; DB> + FILE *procmnt; DB> + const struct { DB> + int maj; DB> + int min; DB> + mode_t mode; DB> + const char *path; DB> + } devs[] = { DB> + { 1, 3, 0666, "/dev/null" }, DB> + { 1, 5, 0666, "/dev/zero" }, DB> + { 1, 7, 0666, "/dev/full" }, DB> + { 5, 1, 0600, "/dev/console" }, DB> + { 1, 8, 0666, "/dev/random" }, DB> + { 1, 9, 0666, "/dev/urandom" }, DB> + }; DB> + DB> + /* Got a FS mapped to /, we're going the pivot_root DB> + approach to do a better-chroot-than-chroot */ DB> + DB> + /* this is based on this thread http://lkml.org/lkml/2008/3/5/29 */ DB> + DB> + /* First step is to ensure the new root itself is DB> + a mount point */ DB> + if (mount(root->src, root->src, NULL, MS_BIND, NULL) < 0) { DB> lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB> - _("failed to mount %s at %s for container: %s"), DB> - curMount->src, curMount->dst, strerror(errno)); DB> + _("failed to bind new root %s: %s"), DB> + root->src, strerror(errno)); DB> + return -1; DB> + } DB> + DB> + if (asprintf(&oldroot, "%s/.oldroot", root->src) < 0) { DB> + lxcError(NULL, NULL, VIR_ERR_NO_MEMORY, NULL); DB> + return -1; DB> + } DB> + DB> + if (virFileMakePath(oldroot) < 0) { DB> + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB> + _("failed to create %s: %s"), DB> + oldroot, strerror(errno)); DB> + return -1; DB> + } DB> + DB> + /* The old root directory will live at /.oldroot after DB> + * this and will soon be unmounted completely */ DB> + if (pivot_root(root->src, oldroot) < 0) { DB> + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB> + _("failed to pivot root %s to %s: %s"), DB> + oldroot, root->src, strerror(errno)); DB> + return -1; DB> + } DB> + DB> + /* CWD is undefined after pivot_root, so go to / */ DB> + if (chdir("/") < 0) { DB> + return -1; DB> + } DB> + DB> + if (virFileMakePath("/proc") < 0 || DB> + mount("none", "/proc", "proc", 0, NULL) < 0) { DB> + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB> + _("failed to mount /proc for container: %s"), DB> + strerror(errno)); DB> + return -1; DB> + } DB> + if (virFileMakePath("/dev") < 0 || DB> + mount("none", "/dev", "tmpfs", 0, NULL) < 0) { DB> + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB> + _("failed to mount /dev tmpfs for container: %s"), DB> + strerror(errno)); DB> + return -1; DB> + } DB> + /* Move old devpts into container, since we have to DB> + connect to the master ptmx which was opened in DB> + the parent. DB> + XXX This sucks, we need to figure out how to get our DB> + own private devpts for isolation DB> + */ DB> + if (virFileMakePath("/dev/pts") < 0 || DB> + mount("/.oldroot/dev/pts", "/dev/pts", NULL, DB> + MS_MOVE, NULL) < 0) { DB> + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB> + _("failed to move /dev/pts into container: %s"), DB> + strerror(errno)); DB> + return -1; DB> + } DB> + DB> + /* Populate /dev/ with a few important bits */ DB> + for (i = 0 ; i < ARRAY_CARDINALITY(devs) ; i++) { DB> + dev_t dev = makedev(devs[i].maj, devs[i].min); DB> + if (mknod(devs[i].path, 0, dev) < 0 || DB> + chmod(devs[i].path, devs[i].mode)) { DB> + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB> + _("failed to make device %s: %s"), DB> + devs[i].path, strerror(errno)); DB> + return -1; DB> + } DB> + } DB> + DB> + /* Pull in rest of container's mounts */ DB> + for (tmp = vmDef->fss; tmp; tmp = tmp->next) { DB> + char *src; DB> + if (STREQ(tmp->dst, "/")) DB> + continue; DB> + // XXX fix DB> + if (tmp->type != VIR_DOMAIN_FS_TYPE_MOUNT) DB> + continue; DB> + DB> + if (asprintf(&src, "/.oldroot/%s", tmp->src) < 0) DB> + return -1; DB> + DB> + if (virFileMakePath(tmp->dst) < 0 || DB> + mount(src, tmp->dst, NULL, MS_BIND, NULL) < 0) { DB> + VIR_FREE(src); DB> + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB> + _("failed to mount %s at %s for container: %s"), DB> + tmp->src, tmp->dst, strerror(errno)); DB> + return -1; DB> + } DB> + VIR_FREE(src); DB> + } DB> + DB> + if (!(procmnt = setmntent("/proc/mounts", "r"))) { DB> + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB> + _("failed to read /proc/mounts: %s"), DB> + strerror(errno)); DB> + return -1; DB> + } DB> + while ((mntent = getmntent(procmnt)) != NULL) { DB> + if (!STRPREFIX(mntent->mnt_dir, "/.oldroot")) DB> + continue; DB> + if (VIR_REALLOC_N(mounts, nmounts+1) < 0) { DB> + endmntent(procmnt); DB> + return -1; DB> + } DB> + if (!(mounts[nmounts++] = strdup(mntent->mnt_dir))) { DB> + endmntent(procmnt); DB> + return -1; DB> + } DB> + } DB> + endmntent(procmnt); DB> + DB> + qsort(mounts, nmounts, sizeof(mounts[0]), DB> + lxcContainerChildMountSort); DB> + DB> + for (i = 0 ; i < nmounts ; i++) { DB> + if (umount(mounts[i]) < 0) { DB> + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB> + _("failed to unmount %s: %s"), DB> + mounts[i], strerror(errno)); DB> + return -1; DB> + } DB> + VIR_FREE(mounts[i]); DB> + } DB> + VIR_FREE(mounts); I'd really like to see this mondo if block broken out into at least an lxcPivotRoot() function, if not further. This is pretty long and hairy, IMHO. DB> + } else { DB> + /* Nothing mapped to /, we're using the main root, DB> + but with extra stuff mapped in */ DB> + for (tmp = vmDef->fss; tmp; tmp = tmp->next) { DB> + // XXX fix DB> + if (tmp->type != VIR_DOMAIN_FS_TYPE_MOUNT) DB> + continue; DB> + rc = mount(tmp->src, DB> + tmp->dst, DB> + NULL, DB> + MS_BIND, DB> + NULL); DB> + if (0 != rc) { DB> + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB> + _("failed to mount %s at %s for container: %s"), DB> + tmp->src, tmp->dst, strerror(errno)); DB> + return -1; DB> + } DB> + } DB> + DB> + /* mount /proc */ DB> + if (mount("lxcproc", "/proc", "proc", 0, NULL) < 0) { DB> + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB> + _("failed to mount /proc for container: %s"), DB> + strerror(errno)); DB> return -1; DB> } DB> } Can we do the same mount of /proc no matter which option we take above? It seems like after the root has been pivoted (or not), we can do the same mount of proc, no? It seems like all of the above could/should be in an lxcContainerDoMounts() function or something, just like the lxcContainerSetStdio() function that gets called a bit later. The added functionality is excellent, by the way :) Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
On Wed, Aug 13, 2008 at 10:57:11AM -0700, Dan Smith wrote: [snip huge pile of code]
I'd really like to see this mondo if block broken out into at least an lxcPivotRoot() function, if not further. This is pretty long and hairy, IMHO.
That is very true. I've re-factored it into a whole bunch of funtions now, so take a look at this.... lxc_container.c | 337 +++++++++++++++++++++++++++++++++++++++++++++++++------- util.c | 12 - 2 files changed, 301 insertions(+), 48 deletions(-) Daniel diff -r 831362089d7c src/lxc_container.c --- a/src/lxc_container.c Wed Aug 27 13:04:30 2008 +0100 +++ b/src/lxc_container.c Wed Aug 27 13:21:35 2008 +0100 @@ -1,10 +1,12 @@ /* * Copyright IBM Corp. 2008 + * Copyright Red Hat 2008 * * lxc_container.c: file description * * Authors: * David L. Leskovec <dlesko at linux.vnet.ibm.com> + * Daniel P. Berrange <berrange@redhat.com> * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -26,10 +28,18 @@ #include <fcntl.h> #include <limits.h> #include <stdlib.h> +#include <stdio.h> #include <sys/ioctl.h> #include <sys/mount.h> #include <sys/wait.h> #include <unistd.h> +#include <mntent.h> + +/* Yes, we want linux private one, for _syscall2() macro */ +#include <linux/unistd.h> + +/* For MS_MOVE */ +#include <linux/fs.h> #include "lxc_container.h" #include "util.h" @@ -103,23 +113,15 @@ * * Returns 0 on success or -1 in case of error */ -static int lxcContainerSetStdio(int control, const char *ttyPath) +static int lxcContainerSetStdio(int control, int ttyfd) { int rc = -1; - int ttyfd; int open_max, i; if (setsid() < 0) { lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("setsid failed: %s"), strerror(errno)); - goto error_out; - } - - ttyfd = open(ttyPath, O_RDWR|O_NOCTTY); - if (ttyfd < 0) { - lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("open(%s) failed: %s"), ttyPath, strerror(errno)); - goto error_out; + goto cleanup; } if (ioctl(ttyfd, TIOCSCTTY, NULL) < 0) { @@ -156,9 +158,6 @@ rc = 0; cleanup: - close(ttyfd); - -error_out: return rc; } @@ -221,6 +220,7 @@ return 0; } + /** * lxcEnableInterfaces: * @vm: Pointer to vm structure @@ -251,6 +251,279 @@ return rc; } + +//_syscall2(int, pivot_root, char *, newroot, const char *, oldroot) +extern int pivot_root(const char * new_root,const char * put_old); + +static int lxcContainerChildMountSort(const void *a, const void *b) +{ + const char **sa = (const char**)a; + const char **sb = (const char**)b; + + /* Delibrately reversed args - we need to unmount deepest + children first */ + return strcmp(*sb, *sa); +} + +static int lxcContainerPivotRoot(virDomainFSDefPtr root) +{ + char *oldroot; + + /* First step is to ensure the new root itself is + a mount point */ + if (mount(root->src, root->src, NULL, MS_BIND, NULL) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to bind new root %s: %s"), + root->src, strerror(errno)); + return -1; + } + + if (asprintf(&oldroot, "%s/.oldroot", root->src) < 0) { + oldroot = NULL; + lxcError(NULL, NULL, VIR_ERR_NO_MEMORY, NULL); + return -1; + } + + if (virFileMakePath(oldroot) < 0) { + VIR_FREE(oldroot); + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to create %s: %s"), + oldroot, strerror(errno)); + return -1; + } + + /* The old root directory will live at /.oldroot after + * this and will soon be unmounted completely */ + if (pivot_root(root->src, oldroot) < 0) { + VIR_FREE(oldroot); + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to pivot root %s to %s: %s"), + oldroot, root->src, strerror(errno)); + return -1; + } + VIR_FREE(oldroot); + + /* CWD is undefined after pivot_root, so go to / */ + if (chdir("/") < 0) { + return -1; + } + + return 0; +} + +static int lxcContainerPopulateDevices(void) +{ + int i; + const struct { + int maj; + int min; + mode_t mode; + const char *path; + } devs[] = { + { 1, 3, 0666, "/dev/null" }, + { 1, 5, 0666, "/dev/zero" }, + { 1, 7, 0666, "/dev/full" }, + { 5, 1, 0600, "/dev/console" }, + { 1, 8, 0666, "/dev/random" }, + { 1, 9, 0666, "/dev/urandom" }, + }; + + if (virFileMakePath("/dev") < 0 || + mount("none", "/dev", "tmpfs", 0, NULL) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to mount /dev tmpfs for container: %s"), + strerror(errno)); + return -1; + } + /* Move old devpts into container, since we have to + connect to the master ptmx which was opened in + the parent. + XXX This sucks, we need to figure out how to get our + own private devpts for isolation + */ + if (virFileMakePath("/dev/pts") < 0 || + mount("/.oldroot/dev/pts", "/dev/pts", NULL, + MS_MOVE, NULL) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to move /dev/pts into container: %s"), + strerror(errno)); + return -1; + } + + /* Populate /dev/ with a few important bits */ + for (i = 0 ; i < ARRAY_CARDINALITY(devs) ; i++) { + dev_t dev = makedev(devs[i].maj, devs[i].min); + if (mknod(devs[i].path, 0, dev) < 0 || + chmod(devs[i].path, devs[i].mode)) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to make device %s: %s"), + devs[i].path, strerror(errno)); + return -1; + } + } + + return 0; +} + + +static int lxcContainerMountNewFS(virDomainDefPtr vmDef) +{ + virDomainFSDefPtr tmp; + + /* Pull in rest of container's mounts */ + for (tmp = vmDef->fss; tmp; tmp = tmp->next) { + char *src; + if (STREQ(tmp->dst, "/")) + continue; + // XXX fix + if (tmp->type != VIR_DOMAIN_FS_TYPE_MOUNT) + continue; + + if (asprintf(&src, "/.oldroot/%s", tmp->src) < 0) + return -1; + + if (virFileMakePath(tmp->dst) < 0 || + mount(src, tmp->dst, NULL, MS_BIND, NULL) < 0) { + VIR_FREE(src); + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to mount %s at %s for container: %s"), + tmp->src, tmp->dst, strerror(errno)); + return -1; + } + VIR_FREE(src); + } + return -1; +} + + +static int lxcContainerUnmountOldFS(void) +{ + struct mntent *mntent; + char **mounts = NULL; + int nmounts = 0; + FILE *procmnt; + int i; + + if (!(procmnt = setmntent("/proc/mounts", "r"))) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to read /proc/mounts: %s"), + strerror(errno)); + return -1; + } + while ((mntent = getmntent(procmnt)) != NULL) { + if (!STRPREFIX(mntent->mnt_dir, "/.oldroot")) + continue; + if (VIR_REALLOC_N(mounts, nmounts+1) < 0) { + endmntent(procmnt); + return -1; + } + if (!(mounts[nmounts++] = strdup(mntent->mnt_dir))) { + endmntent(procmnt); + return -1; + } + } + endmntent(procmnt); + + qsort(mounts, nmounts, sizeof(mounts[0]), + lxcContainerChildMountSort); + + for (i = 0 ; i < nmounts ; i++) { + if (umount(mounts[i]) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to unmount %s: %s"), + mounts[i], strerror(errno)); + return -1; + } + VIR_FREE(mounts[i]); + } + VIR_FREE(mounts); + + return 0; +} + + +/* Got a FS mapped to /, we're going the pivot_root + * approach to do a better-chroot-than-chroot + * this is based on this thread http://lkml.org/lkml/2008/3/5/29 + */ +static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, + virDomainFSDefPtr root) +{ + if (lxcContainerPivotRoot(root) < 0) + return -1; + + if (virFileMakePath("/proc") < 0 || + mount("none", "/proc", "proc", 0, NULL) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to mount /proc for container: %s"), + strerror(errno)); + return -1; + } + + if (lxcContainerPopulateDevices() < 0) + return -1; + + if (lxcContainerMountNewFS(vmDef) < 0) + return -1; + + if (lxcContainerUnmountOldFS() < 0) + return -1; + + return 0; +} + +/* Nothing mapped to /, we're using the main root, + but with extra stuff mapped in */ +static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef) +{ + virDomainFSDefPtr tmp; + + for (tmp = vmDef->fss; tmp; tmp = tmp->next) { + // XXX fix to support other mount types + if (tmp->type != VIR_DOMAIN_FS_TYPE_MOUNT) + continue; + + if (mount(tmp->src, + tmp->dst, + NULL, + MS_BIND, + NULL) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to mount %s at %s for container: %s"), + tmp->src, tmp->dst, strerror(errno)); + return -1; + } + } + + /* mount /proc */ + if (mount("lxcproc", "/proc", "proc", 0, NULL) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to mount /proc for container: %s"), + strerror(errno)); + return -1; + } + + return 0; +} + +static int lxcContainerSetupMounts(virDomainDefPtr vmDef) +{ + virDomainFSDefPtr tmp; + virDomainFSDefPtr root = NULL; + + for (tmp = vmDef->fss; tmp && !root; tmp = tmp->next) { + if (tmp->type != VIR_DOMAIN_FS_TYPE_MOUNT) + continue; + if (STREQ(tmp->dst, "/")) + root = tmp; + } + + if (root) + return lxcContainerSetupPivotRoot(vmDef, root); + else + return lxcContainerSetupExtraMounts(vmDef); +} + /** * lxcChild: * @argv: Pointer to container arguments @@ -265,11 +538,9 @@ */ static int lxcContainerChild( void *data ) { - int rc = -1; lxc_child_argv_t *argv = data; virDomainDefPtr vmDef = argv->config; - virDomainFSDefPtr curMount; - int i; + int ttyfd; if (NULL == vmDef) { lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, @@ -277,37 +548,21 @@ return -1; } - /* handle the bind mounts first before doing anything else that may */ - /* then access those mounted dirs */ - curMount = vmDef->fss; - for (i = 0; curMount; curMount = curMount->next) { - // XXX fix - if (curMount->type != VIR_DOMAIN_FS_TYPE_MOUNT) - continue; - rc = mount(curMount->src, - curMount->dst, - NULL, - MS_BIND, - NULL); - if (0 != rc) { - lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to mount %s at %s for container: %s"), - curMount->src, curMount->dst, strerror(errno)); - return -1; - } - } + if (lxcContainerSetupMounts(vmDef) < 0) + return -1; - /* mount /proc */ - rc = mount("lxcproc", "/proc", "proc", 0, NULL); - if (0 != rc) { + ttyfd = open(argv->ttyPath, O_RDWR|O_NOCTTY); + if (ttyfd < 0) { lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to mount /proc for container: %s"), - strerror(errno)); + _("open(%s) failed: %s"), argv->ttyPath, strerror(errno)); return -1; } - if (lxcContainerSetStdio(argv->monitor, argv->ttyPath) < 0) + if (lxcContainerSetStdio(argv->monitor, ttyfd) < 0) { + close(ttyfd); return -1; + } + close(ttyfd); /* Wait for interface devices to show up */ if (lxcContainerWaitForContinue(argv->monitor) < 0) diff -r 831362089d7c src/util.c --- a/src/util.c Wed Aug 27 13:04:30 2008 +0100 +++ b/src/util.c Wed Aug 27 13:21:35 2008 +0100 @@ -616,13 +616,11 @@ if (!(p = strrchr(parent, '/'))) return EINVAL; - if (p == parent) - return EPERM; - - *p = '\0'; - - if ((err = virFileMakePath(parent))) - return err; + if (p != parent) { + *p = '\0'; + if ((err = virFileMakePath(parent))) + return err; + } if (mkdir(path, 0777) < 0 && errno != EEXIST) return errno; -- |: 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 :|
DB> +static int lxcContainerMountNewFS(virDomainDefPtr vmDef) DB> +{ DB> + virDomainFSDefPtr tmp; DB> + DB> + /* Pull in rest of container's mounts */ DB> + for (tmp = vmDef->fss; tmp; tmp = tmp->next) { DB> + char *src; DB> + if (STREQ(tmp->dst, "/")) DB> + continue; DB> + // XXX fix DB> + if (tmp->type != VIR_DOMAIN_FS_TYPE_MOUNT) DB> + continue; DB> + DB> + if (asprintf(&src, "/.oldroot/%s", tmp->src) < 0) DB> + return -1; DB> + DB> + if (virFileMakePath(tmp->dst) < 0 || DB> + mount(src, tmp->dst, NULL, MS_BIND, NULL) < 0) { DB> + VIR_FREE(src); DB> + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB> + _("failed to mount %s at %s for container: %s"), DB> + tmp->src, tmp->dst, strerror(errno)); DB> + return -1; DB> + } DB> + VIR_FREE(src); DB> + } DB> + return -1; Shouldn't this be "return 0"? AFAICT, this means this function will always fail and thus any domain with a root target will fail to start. If I change this to "return 0" I'm able to start such guests, with properly pivoted roots. On a more general note, it seems like there are a lot of places where failures trigger a "return -1" that rolls completely up the stack with no error information getting logged. Since you have the excellent per-container logging functionality, can we increase the verbosity a little so that there is some way to diagnose where things are failing? Thus far, I've just started sprinkling fprintf()'s into the code until I start to narrow things down. I'd be glad to help with that after this goes in. Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
On Wed, Aug 27, 2008 at 01:32:13PM -0700, Dan Smith wrote:
DB> +static int lxcContainerMountNewFS(virDomainDefPtr vmDef) DB> +{ DB> + virDomainFSDefPtr tmp; DB> + DB> + /* Pull in rest of container's mounts */ DB> + for (tmp = vmDef->fss; tmp; tmp = tmp->next) { DB> + char *src; DB> + if (STREQ(tmp->dst, "/")) DB> + continue; DB> + // XXX fix DB> + if (tmp->type != VIR_DOMAIN_FS_TYPE_MOUNT) DB> + continue; DB> + DB> + if (asprintf(&src, "/.oldroot/%s", tmp->src) < 0) DB> + return -1; DB> + DB> + if (virFileMakePath(tmp->dst) < 0 || DB> + mount(src, tmp->dst, NULL, MS_BIND, NULL) < 0) { DB> + VIR_FREE(src); DB> + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, DB> + _("failed to mount %s at %s for container: %s"), DB> + tmp->src, tmp->dst, strerror(errno)); DB> + return -1; DB> + } DB> + VIR_FREE(src); DB> + } DB> + return -1;
Shouldn't this be "return 0"? AFAICT, this means this function will always fail and thus any domain with a root target will fail to start. If I change this to "return 0" I'm able to start such guests, with properly pivoted roots.
Yes, it clearly should be return 0.
On a more general note, it seems like there are a lot of places where failures trigger a "return -1" that rolls completely up the stack with no error information getting logged. Since you have the excellent per-container logging functionality, can we increase the verbosity a little so that there is some way to diagnose where things are failing? Thus far, I've just started sprinkling fprintf()'s into the code until I start to narrow things down. I'd be glad to help with that after this goes in.
The newly improved virExec() API which LXC now uses ensures that libvirt's error callback is reset to the default in child processes. This means that any call to virRaiseError in the child is turned into a fprintf(stderr...). We also wire the container stdout/err to /var/log/libvirt/lxc/NAME.log So if everything is operating as I expect, all the lxcError() calls in this code should result in log messages being written out to /var/log. Daniel -- |: 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 :|
On Wed, Aug 27, 2008 at 09:58:58PM +0100, Daniel P. Berrange wrote:
On Wed, Aug 27, 2008 at 01:32:13PM -0700, Dan Smith wrote:
DB> +static int lxcContainerMountNewFS(virDomainDefPtr vmDef) DB> +{ DB> + virDomainFSDefPtr tmp; .... DB> + return -1;
Shouldn't this be "return 0"? AFAICT, this means this function will always fail and thus any domain with a root target will fail to start. If I change this to "return 0" I'm able to start such guests, with properly pivoted roots.
Yes, it clearly should be return 0.
On a more general note, it seems like there are a lot of places where failures trigger a "return -1" that rolls completely up the stack with no error information getting logged. Since you have the excellent per-container logging functionality, can we increase the verbosity a little so that there is some way to diagnose where things are failing? Thus far, I've just started sprinkling fprintf()'s into the code until I start to narrow things down. I'd be glad to help with that after this goes in.
The newly improved virExec() API which LXC now uses ensures that libvirt's error callback is reset to the default in child processes. This means that any call to virRaiseError in the child is turned into a fprintf(stderr...). We also wire the container stdout/err to /var/log/libvirt/lxc/NAME.log So if everything is operating as I expect, all the lxcError() calls in this code should result in log messages being written out to /var/log.
Of course in this particular scenario you would never have had a log message because this should never have been a 'return -1'. I found a couple of other places with missing lxcError() calls. So here's an updated patch diff -r 4d49719aa768 src/lxc_container.c --- a/src/lxc_container.c Wed Aug 27 22:19:33 2008 +0100 +++ b/src/lxc_container.c Wed Aug 27 22:21:15 2008 +0100 @@ -1,10 +1,12 @@ /* * Copyright IBM Corp. 2008 + * Copyright Red Hat 2008 * * lxc_container.c: file description * * Authors: * David L. Leskovec <dlesko at linux.vnet.ibm.com> + * Daniel P. Berrange <berrange@redhat.com> * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -26,10 +28,18 @@ #include <fcntl.h> #include <limits.h> #include <stdlib.h> +#include <stdio.h> #include <sys/ioctl.h> #include <sys/mount.h> #include <sys/wait.h> #include <unistd.h> +#include <mntent.h> + +/* Yes, we want linux private one, for _syscall2() macro */ +#include <linux/unistd.h> + +/* For MS_MOVE */ +#include <linux/fs.h> #include "lxc_container.h" #include "util.h" @@ -103,23 +113,15 @@ * * Returns 0 on success or -1 in case of error */ -static int lxcContainerSetStdio(int control, const char *ttyPath) +static int lxcContainerSetStdio(int control, int ttyfd) { int rc = -1; - int ttyfd; int open_max, i; if (setsid() < 0) { lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("setsid failed: %s"), strerror(errno)); - goto error_out; - } - - ttyfd = open(ttyPath, O_RDWR|O_NOCTTY); - if (ttyfd < 0) { - lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("open(%s) failed: %s"), ttyPath, strerror(errno)); - goto error_out; + goto cleanup; } if (ioctl(ttyfd, TIOCSCTTY, NULL) < 0) { @@ -156,9 +158,6 @@ rc = 0; cleanup: - close(ttyfd); - -error_out: return rc; } @@ -221,6 +220,7 @@ return 0; } + /** * lxcEnableInterfaces: * @vm: Pointer to vm structure @@ -251,6 +251,285 @@ return rc; } + +//_syscall2(int, pivot_root, char *, newroot, const char *, oldroot) +extern int pivot_root(const char * new_root,const char * put_old); + +static int lxcContainerChildMountSort(const void *a, const void *b) +{ + const char **sa = (const char**)a; + const char **sb = (const char**)b; + + /* Delibrately reversed args - we need to unmount deepest + children first */ + return strcmp(*sb, *sa); +} + +static int lxcContainerPivotRoot(virDomainFSDefPtr root) +{ + char *oldroot; + + /* First step is to ensure the new root itself is + a mount point */ + if (mount(root->src, root->src, NULL, MS_BIND, NULL) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to bind new root %s: %s"), + root->src, strerror(errno)); + return -1; + } + + if (asprintf(&oldroot, "%s/.oldroot", root->src) < 0) { + oldroot = NULL; + lxcError(NULL, NULL, VIR_ERR_NO_MEMORY, NULL); + return -1; + } + + if (virFileMakePath(oldroot) < 0) { + VIR_FREE(oldroot); + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to create %s: %s"), + oldroot, strerror(errno)); + return -1; + } + + /* The old root directory will live at /.oldroot after + * this and will soon be unmounted completely */ + if (pivot_root(root->src, oldroot) < 0) { + VIR_FREE(oldroot); + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to pivot root %s to %s: %s"), + oldroot, root->src, strerror(errno)); + return -1; + } + VIR_FREE(oldroot); + + /* CWD is undefined after pivot_root, so go to / */ + if (chdir("/") < 0) { + return -1; + } + + return 0; +} + +static int lxcContainerPopulateDevices(void) +{ + int i; + const struct { + int maj; + int min; + mode_t mode; + const char *path; + } devs[] = { + { 1, 3, 0666, "/dev/null" }, + { 1, 5, 0666, "/dev/zero" }, + { 1, 7, 0666, "/dev/full" }, + { 5, 1, 0600, "/dev/console" }, + { 1, 8, 0666, "/dev/random" }, + { 1, 9, 0666, "/dev/urandom" }, + }; + + if (virFileMakePath("/dev") < 0 || + mount("none", "/dev", "tmpfs", 0, NULL) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to mount /dev tmpfs for container: %s"), + strerror(errno)); + return -1; + } + /* Move old devpts into container, since we have to + connect to the master ptmx which was opened in + the parent. + XXX This sucks, we need to figure out how to get our + own private devpts for isolation + */ + if (virFileMakePath("/dev/pts") < 0 || + mount("/.oldroot/dev/pts", "/dev/pts", NULL, + MS_MOVE, NULL) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to move /dev/pts into container: %s"), + strerror(errno)); + return -1; + } + + /* Populate /dev/ with a few important bits */ + for (i = 0 ; i < ARRAY_CARDINALITY(devs) ; i++) { + dev_t dev = makedev(devs[i].maj, devs[i].min); + if (mknod(devs[i].path, 0, dev) < 0 || + chmod(devs[i].path, devs[i].mode)) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to make device %s: %s"), + devs[i].path, strerror(errno)); + return -1; + } + } + + return 0; +} + + +static int lxcContainerMountNewFS(virDomainDefPtr vmDef) +{ + virDomainFSDefPtr tmp; + + /* Pull in rest of container's mounts */ + for (tmp = vmDef->fss; tmp; tmp = tmp->next) { + char *src; + if (STREQ(tmp->dst, "/")) + continue; + // XXX fix + if (tmp->type != VIR_DOMAIN_FS_TYPE_MOUNT) + continue; + + if (asprintf(&src, "/.oldroot/%s", tmp->src) < 0) { + lxcError(NULL, NULL, VIR_ERR_NO_MEMORY, NULL); + return -1; + } + + if (virFileMakePath(tmp->dst) < 0 || + mount(src, tmp->dst, NULL, MS_BIND, NULL) < 0) { + VIR_FREE(src); + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to mount %s at %s for container: %s"), + tmp->src, tmp->dst, strerror(errno)); + return -1; + } + VIR_FREE(src); + } + + return 0; +} + + +static int lxcContainerUnmountOldFS(void) +{ + struct mntent *mntent; + char **mounts = NULL; + int nmounts = 0; + FILE *procmnt; + int i; + + if (!(procmnt = setmntent("/proc/mounts", "r"))) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to read /proc/mounts: %s"), + strerror(errno)); + return -1; + } + while ((mntent = getmntent(procmnt)) != NULL) { + if (!STRPREFIX(mntent->mnt_dir, "/.oldroot")) + continue; + + if (VIR_REALLOC_N(mounts, nmounts+1) < 0) { + endmntent(procmnt); + lxcError(NULL, NULL, VIR_ERR_NO_MEMORY, NULL); + return -1; + } + if (!(mounts[nmounts++] = strdup(mntent->mnt_dir))) { + endmntent(procmnt); + lxcError(NULL, NULL, VIR_ERR_NO_MEMORY, NULL); + return -1; + } + } + endmntent(procmnt); + + qsort(mounts, nmounts, sizeof(mounts[0]), + lxcContainerChildMountSort); + + for (i = 0 ; i < nmounts ; i++) { + if (umount(mounts[i]) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to unmount %s: %s"), + mounts[i], strerror(errno)); + return -1; + } + VIR_FREE(mounts[i]); + } + VIR_FREE(mounts); + + return 0; +} + + +/* Got a FS mapped to /, we're going the pivot_root + * approach to do a better-chroot-than-chroot + * this is based on this thread http://lkml.org/lkml/2008/3/5/29 + */ +static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, + virDomainFSDefPtr root) +{ + if (lxcContainerPivotRoot(root) < 0) + return -1; + + if (virFileMakePath("/proc") < 0 || + mount("none", "/proc", "proc", 0, NULL) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to mount /proc for container: %s"), + strerror(errno)); + return -1; + } + + if (lxcContainerPopulateDevices() < 0) + return -1; + + if (lxcContainerMountNewFS(vmDef) < 0) + return -1; + + if (lxcContainerUnmountOldFS() < 0) + return -1; + + return 0; +} + +/* Nothing mapped to /, we're using the main root, + but with extra stuff mapped in */ +static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef) +{ + virDomainFSDefPtr tmp; + + for (tmp = vmDef->fss; tmp; tmp = tmp->next) { + // XXX fix to support other mount types + if (tmp->type != VIR_DOMAIN_FS_TYPE_MOUNT) + continue; + + if (mount(tmp->src, + tmp->dst, + NULL, + MS_BIND, + NULL) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to mount %s at %s for container: %s"), + tmp->src, tmp->dst, strerror(errno)); + return -1; + } + } + + /* mount /proc */ + if (mount("lxcproc", "/proc", "proc", 0, NULL) < 0) { + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("failed to mount /proc for container: %s"), + strerror(errno)); + return -1; + } + + return 0; +} + +static int lxcContainerSetupMounts(virDomainDefPtr vmDef) +{ + virDomainFSDefPtr tmp; + virDomainFSDefPtr root = NULL; + + for (tmp = vmDef->fss; tmp && !root; tmp = tmp->next) { + if (tmp->type != VIR_DOMAIN_FS_TYPE_MOUNT) + continue; + if (STREQ(tmp->dst, "/")) + root = tmp; + } + + if (root) + return lxcContainerSetupPivotRoot(vmDef, root); + else + return lxcContainerSetupExtraMounts(vmDef); +} + /** * lxcChild: * @argv: Pointer to container arguments @@ -265,11 +544,9 @@ */ static int lxcContainerChild( void *data ) { - int rc = -1; lxc_child_argv_t *argv = data; virDomainDefPtr vmDef = argv->config; - virDomainFSDefPtr curMount; - int i; + int ttyfd; if (NULL == vmDef) { lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, @@ -277,37 +554,21 @@ return -1; } - /* handle the bind mounts first before doing anything else that may */ - /* then access those mounted dirs */ - curMount = vmDef->fss; - for (i = 0; curMount; curMount = curMount->next) { - // XXX fix - if (curMount->type != VIR_DOMAIN_FS_TYPE_MOUNT) - continue; - rc = mount(curMount->src, - curMount->dst, - NULL, - MS_BIND, - NULL); - if (0 != rc) { - lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to mount %s at %s for container: %s"), - curMount->src, curMount->dst, strerror(errno)); - return -1; - } - } + if (lxcContainerSetupMounts(vmDef) < 0) + return -1; - /* mount /proc */ - rc = mount("lxcproc", "/proc", "proc", 0, NULL); - if (0 != rc) { + ttyfd = open(argv->ttyPath, O_RDWR|O_NOCTTY); + if (ttyfd < 0) { lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to mount /proc for container: %s"), - strerror(errno)); + _("open(%s) failed: %s"), argv->ttyPath, strerror(errno)); return -1; } - if (lxcContainerSetStdio(argv->monitor, argv->ttyPath) < 0) + if (lxcContainerSetStdio(argv->monitor, ttyfd) < 0) { + close(ttyfd); return -1; + } + close(ttyfd); /* Wait for interface devices to show up */ if (lxcContainerWaitForContinue(argv->monitor) < 0) diff -r 4d49719aa768 src/util.c --- a/src/util.c Wed Aug 27 22:19:33 2008 +0100 +++ b/src/util.c Wed Aug 27 22:21:15 2008 +0100 @@ -616,13 +616,11 @@ if (!(p = strrchr(parent, '/'))) return EINVAL; - if (p == parent) - return EPERM; - - *p = '\0'; - - if ((err = virFileMakePath(parent))) - return err; + if (p != parent) { + *p = '\0'; + if ((err = virFileMakePath(parent))) + return err; + } if (mkdir(path, 0777) < 0 && errno != EEXIST) return errno; Daniel -- |: 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 :|
DB> I found a couple of other places with missing lxcError() calls. So DB> here's an updated patch Looks good to me. Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
On Thu, Aug 28, 2008 at 06:31:31AM -0700, Dan Smith wrote:
DB> I found a couple of other places with missing lxcError() calls. So DB> here's an updated patch
Looks good to me. Thanks!
Ok, I've finally committed this Daniel -- |: 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 :|
participants (3)
-
Dan Smith -
Daniel P. Berrange -
Daniel Veillard