On Thu, Mar 20, 2008 at 02:43:28PM -0700, Dave Leskovec wrote:

Daniel Veillard wrote:

...

[...]

...

+static int lxcListDomains(virConnectPtr conn, int *ids, int nids) +{ + lxc_driver_t *driver = (lxc_driver_t *)conn->privateData; + lxc_vm_t *vm; + int numDoms = 0; + + for (vm = driver->vms; vm && (numDoms < nids); vm = vm->next) { + if (lxcIsActiveVM(vm)) { + ids[numDoms] = vm->def->id; + numDoms++; + } + } + + return numDoms; +}

so we can only list domains created by this libvirt instance, right ? Or I'm missing something, I assume virsh list works but I don't see how.

Well, yes and no. The list of vms is local to the process however all container configs are stored to file when they're created. So, a later instance of libvirt (later being after a container is created) will pick up the config file and know about that container. However, if 2 instances of libvirt are running and one creates a container, the other won't know about it until it's restarted or reconnected. This and a few related issues have been sticking in the back of my mind for a little while. I'm wondering if the solution isn't to have the lxc driver under libvirtd. That or load and unload the list of vms around every operation.

Yes, this is a fundamentally stateful driver, so should run in the context of the daemon as the QEMU driver does. Loading & unloading the persistent state in the open/close method is hacking around the capabilities we already provide for stateful drivers, and it is not safe to have multiple libvirt clients reading/writing the same config files without any form of locking. By using the libvirt daemon stateful driver support we avoid the locking issues, avoid the synchronization problems between multiple clients, and avoid having to have the fork()d container re-write the config files to include the PID and so on. The following patches make the driver properly integrate with the stateful driver APIs. It also changes the config files to be named by on VM name instead of UUID, since this is what the QEMU driver does & its more user friendly. It also adds the CLONE_XXX constants since they have not yet been added to the libc sched.h file. IMHO we should enable the driver by default, since it can already probe for availability at runtime. Finally it also fixes a typo where it wrote 'linuxcontainer' as the domain type in the config file instead of 'lxc'. Dan. Index: src/lxc_conf.c =================================================================== RCS file: /data/cvs/libvirt/src/lxc_conf.c,v retrieving revision 1.1 diff -u -p -r1.1 lxc_conf.c --- src/lxc_conf.c 21 Mar 2008 15:03:37 -0000 1.1 +++ src/lxc_conf.c 21 Mar 2008 16:47:26 -0000 @@ -593,7 +593,6 @@ int lxcSaveVMDef(virConnectPtr conn, lxc_vm_def_t *def) { int rc = -1; - char uuidstr[VIR_UUID_STRING_BUFLEN]; if (vm->configFile[0] == '\0') { if ((rc = virFileMakePath(driver->configDir))) { @@ -603,16 +602,15 @@ int lxcSaveVMDef(virConnectPtr conn, goto save_complete; } - virUUIDFormat(def->uuid, uuidstr); - if (virFileBuildPath(driver->configDir, uuidstr, ".xml", + if (virFileBuildPath(driver->configDir, vm->def->name, ".xml", vm->configFile, PATH_MAX) < 0) { lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, _("cannot construct config file path")); goto save_complete; } - strncpy(vm->configFileBase, uuidstr, PATH_MAX); - strncat(vm->configFileBase, ".xml", PATH_MAX - strlen(uuidstr)); + strncpy(vm->configFileBase, vm->def->name, PATH_MAX-1); + strncat(vm->configFileBase, ".xml", PATH_MAX - strlen(vm->def->name)-1); } @@ -631,7 +629,6 @@ static lxc_vm_t * lxcLoadConfig(lxc_driv { lxc_vm_def_t *containerDef; lxc_vm_t * vm; - char uuidstr[VIR_UUID_STRING_BUFLEN]; containerDef = lxcParseVMDef(NULL, xmlData, file); if (NULL == containerDef) { @@ -639,9 +636,8 @@ static lxc_vm_t * lxcLoadConfig(lxc_driv return NULL; } - virUUIDFormat(containerDef->uuid, uuidstr); - if (!virFileMatchesNameSuffix(file, uuidstr, ".xml")) { - DEBUG0("Container uuid does not match config file name"); + if (!virFileMatchesNameSuffix(file, containerDef->name, ".xml")) { + DEBUG0("Container name does not match config file name"); lxcFreeVMDef(containerDef); return NULL; } @@ -662,14 +658,12 @@ static lxc_vm_t * lxcLoadConfig(lxc_driv return vm; } -int lxcLoadDriverConfig(virConnectPtr conn) +int lxcLoadDriverConfig(lxc_driver_t *driver) { - lxc_driver_t *driverPtr = (lxc_driver_t*)conn->privateData; - /* Set the container configuration directory */ - driverPtr->configDir = strdup(SYSCONF_DIR "/libvirt/lxc"); - if (NULL == driverPtr->configDir) { - lxcError(conn, NULL, VIR_ERR_NO_MEMORY, "configDir"); + driver->configDir = strdup(SYSCONF_DIR "/libvirt/lxc"); + if (NULL == driver->configDir) { + lxcError(NULL, NULL, VIR_ERR_NO_MEMORY, "configDir"); return -1; } @@ -702,19 +696,18 @@ load_complete: return rc; } -int lxcLoadContainerInfo(virConnectPtr conn) +int lxcLoadContainerInfo(lxc_driver_t *driver) { int rc = -1; - lxc_driver_t *driverPtr = (lxc_driver_t*)conn->privateData; DIR *dir; struct dirent *dirEntry; - if (!(dir = opendir(driverPtr->configDir))) { + if (!(dir = opendir(driver->configDir))) { if (ENOENT == errno) { /* no config dir => no containers */ rc = 0; } else { - lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR, + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("failed to open config directory: %s"), strerror(errno)); } @@ -730,7 +723,7 @@ int lxcLoadContainerInfo(virConnectPtr c continue; } - lxcLoadContainerConfigFile(driverPtr, dirEntry->d_name); + lxcLoadContainerConfigFile(driver, dirEntry->d_name); } closedir(dir); @@ -758,12 +751,13 @@ char *lxcGenerateXML(virConnectPtr conn, } if (lxcIsActiveVM(vm)) { - if (virBufferVSprintf(buf, "<domain type='linuxcontainer' id='%d'>\n", - vm->def->id) < 0) { + if (virBufferVSprintf(buf, "<domain type='%s' id='%d'>\n", + LXC_DOMAIN_TYPE, vm->def->id) < 0) { goto no_memory; } } else { - if (virBufferAddLit(buf, "<domain type='linuxcontainer'>\n") < 0) { + if (virBufferVSprintf(buf, "<domain type='%s'>\n", + LXC_DOMAIN_TYPE) < 0) { goto no_memory; } } @@ -940,3 +934,12 @@ int lxcDeleteConfig(virConnectPtr conn, #endif /* WITH_LXC */ + +/* + * Local variables: + * indent-tabs-mode: nil + * c-indent-level: 4 + * c-basic-offset: 4 + * tab-width: 4 + * End: + */ Index: src/lxc_conf.h =================================================================== RCS file: /data/cvs/libvirt/src/lxc_conf.h,v retrieving revision 1.1 diff -u -p -r1.1 lxc_conf.h --- src/lxc_conf.h 21 Mar 2008 15:03:37 -0000 1.1 +++ src/lxc_conf.h 21 Mar 2008 16:47:26 -0000 @@ -102,12 +102,12 @@ int lxcSaveVMDef(virConnectPtr conn, lxc_driver_t *driver, lxc_vm_t *vm, lxc_vm_def_t *def); -int lxcLoadDriverConfig(virConnectPtr conn); +int lxcLoadDriverConfig(lxc_driver_t *driver); int lxcSaveConfig(virConnectPtr conn, lxc_driver_t *driver, lxc_vm_t *vm, lxc_vm_def_t *def); -int lxcLoadContainerInfo(virConnectPtr conn); +int lxcLoadContainerInfo(lxc_driver_t *driver); int lxcLoadContainerConfigFile(lxc_driver_t *driver, const char *file); lxc_vm_t * lxcAssignVMDef(virConnectPtr conn, Index: src/lxc_driver.c =================================================================== RCS file: /data/cvs/libvirt/src/lxc_driver.c,v retrieving revision 1.1 diff -u -p -r1.1 lxc_driver.c --- src/lxc_driver.c 21 Mar 2008 15:03:37 -0000 1.1 +++ src/lxc_driver.c 21 Mar 2008 16:47:26 -0000 @@ -41,8 +41,27 @@ #define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt, __VA_ARGS__) #define DEBUG0(msg) VIR_DEBUG(__FILE__, "%s", msg) -static int lxcStartup(virConnectPtr conn); -static int lxcShutdown(virConnectPtr conn); +/* + * GLibc headers are behind the kernel, so we define these + * constants if they're not present already. + */ + +#ifndef CLONE_NEWPID +#define CLONE_NEWPID 0x20000000 +#endif +#ifndef CLONE_NEWUTS +#define CLONE_NEWUTS 0x04000000 +#endif +#ifndef CLONE_NEWUSER +#define CLONE_NEWUSER 0x10000000 +#endif +#ifndef CLONE_NEWIPC +#define CLONE_NEWIPC 0x08000000 +#endif + +static int lxcStartup(void); +static int lxcShutdown(void); +static lxc_driver_t *lxc_driver; /* Functions */ static int lxcDummyChild( void *argv ATTRIBUTE_UNUSED ) @@ -105,6 +124,9 @@ static virDrvOpenStatus lxcOpen(virConne goto declineConnection; } + if (lxc_driver == NULL) + goto declineConnection; + /* Verify uri was specified */ if ((NULL == uri) || (NULL == uri->scheme)) { goto declineConnection; @@ -115,15 +137,7 @@ static virDrvOpenStatus lxcOpen(virConne goto declineConnection; } - /* Check that this is a container enabled kernel */ - if(0 != lxcCheckContainerSupport()) { - goto declineConnection; - } - - /* initialize driver data */ - if (0 > lxcStartup(conn)) { - goto declineConnection; - } + conn->privateData = lxc_driver; return VIR_DRV_OPEN_SUCCESS; @@ -133,7 +147,8 @@ declineConnection: static int lxcClose(virConnectPtr conn) { - return lxcShutdown(conn); + conn->privateData = NULL; + return 0; } static virDomainPtr lxcDomainLookupByID(virConnectPtr conn, @@ -360,26 +375,29 @@ static char *lxcDomainDumpXML(virDomainP return lxcGenerateXML(dom->conn, driver, vm, vm->def); } -static int lxcStartup(virConnectPtr conn) + +static int lxcStartup(void) { - lxc_driver_t *driver; + lxc_driver = calloc(1, sizeof(lxc_driver_t)); + if (NULL == lxc_driver) { + return -1; + } - driver = calloc(1, sizeof(lxc_driver_t)); - if (NULL == driver) { + /* Check that this is a container enabled kernel */ + if(0 != lxcCheckContainerSupport()) { return -1; } - conn->privateData = driver; /* Call function to load lxc driver configuration information */ - if (lxcLoadDriverConfig(conn) < 0) { - lxcShutdown(conn); + if (lxcLoadDriverConfig(lxc_driver) < 0) { + lxcShutdown(); return -1; } /* Call function to load the container configuration files */ - if (lxcLoadContainerInfo(conn) < 0) { - lxcShutdown(conn); + if (lxcLoadContainerInfo(lxc_driver) < 0) { + lxcShutdown(); return -1; } @@ -392,19 +410,37 @@ static void lxcFreeDriver(lxc_driver_t * free(driver); } -static int lxcShutdown(virConnectPtr conn) +static int lxcShutdown(void) { - lxc_driver_t *driver = (lxc_driver_t *)conn->privateData; - lxc_vm_t *vms = driver->vms; + lxc_vm_t *vms = lxc_driver->vms; lxcFreeVMs(vms); - driver->vms = NULL; - lxcFreeDriver(driver); - conn->privateData = NULL; + lxc_driver->vms = NULL; + lxcFreeDriver(lxc_driver); return 0; } +/** + * lxcActive: + * + * Checks if the LXC daemon is active, i.e. has an active domain + * + * Returns 1 if active, 0 otherwise + */ +static int +lxcActive(void) { + /* If we've any active networks or guests, then we + * mark this driver as active + */ + if (lxc_driver->nactivevms) + return 1; + + /* Otherwise we're happy to deal with a shutdown */ + return 0; +} + + /* Function Tables */ static virDriver lxcDriver = { VIR_DRV_LXC, /* the number virDrvNo */ @@ -466,9 +502,18 @@ static virDriver lxcDriver = { NULL, /* getFreeMemory */ }; + +static virStateDriver lxcStateDriver = { + lxcStartup, + lxcShutdown, + NULL, /* reload */ + lxcActive, +}; + int lxcRegister(void) { virRegisterDriver(&lxcDriver); + virRegisterStateDriver(&lxcStateDriver); return 0; } Index: src/remote_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/remote_internal.c,v retrieving revision 1.63 diff -u -p -r1.63 remote_internal.c --- src/remote_internal.c 17 Mar 2008 17:30:48 -0000 1.63 +++ src/remote_internal.c 21 Mar 2008 16:47:32 -0000 @@ -843,6 +843,12 @@ remoteOpen (virConnectPtr conn, rflags |= VIR_DRV_OPEN_REMOTE_UNIX; } #endif +#if WITH_LXC + if (uri && + uri->scheme && STREQ (uri->scheme, "lxc")) { + rflags |= VIR_DRV_OPEN_REMOTE_UNIX; + } +#endif priv->magic = DEAD; priv->sock = -1; -- |: Red Hat, Engineering, Boston -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 :|