On Thu, Mar 27, 2008 at 02:50:20PM -0700, Dave Leskovec wrote: > Daniel Veillard wrote: > >> + int execStringLen = strlen(vmDef->init) + 1 + 5; > >> + strcpy(execString, "exec "); > >> + strcat(execString, vmDef->init); > > > > Hum, it seems there is an off by one allocation error, you don't allocate > > the space for the terminating 0 > > A comment probably would have helped here :-) the + 1 up there in setting the > execStringLen is for the NULL. Oops, for some reason I counted 5 for 'exec', ahum :-) [...] > >> + vm->def->id = vm->pid; > >> + lxcSaveConfig(NULL, driver, vm, vm->def); > > > > We should make sure at some point taht there is some kind of locking > > mechanism when creating those config files, no ? > > Good question. Right now libvirtd should be the only process accessing the > file. Is it multi-threaded that would cause collisions? The other possibility > is if we found we needed to save the config file from within the container. > That's not currently the case and I'd stay away from that if at all possible. Okay, then that's not needed, fine by me :-) > > [...] > > > > Hum, now that config names are saved based on domain names, wouldn't > > it be better to use a name based lookup ? Less precise but more direct > > Not sure what you're referring to here. name based lookup for what? Hum, wrong part quoted, it's about lxcDomainStart() just below: :+static int lxcDomainStart(virDomainPtr dom) :+{ :+ int rc = -1; :+ virConnectPtr conn = dom->conn; :+ lxc_driver_t *driver = (lxc_driver_t *)(conn->privateData); :+ lxc_vm_t *vm = lxcFindVMByUUID(driver, dom->uuid); :+ > > Looks fine overall. I wonder if 1 fork/clone per container can't be > > reduced to a single process doing the fd processing for all the containers > > running. But that's probably harder, more fragile and for little gain. > > Yes, that's been tossed around. I'm holding off pursuing that for now until > devpts namespace and it's impacts are known. Okay, just wondering :-) thanks ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list