Re: [Libvir] [PATCH] lxc: start domain
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/