Daniel Veillard wrote:
> Okay, that's something i promised last week, but it has some significant
> changes:
> - cleanup the XPath methods to access the XML informations, reusing the
> existing methods from xml.h
> - make string fields from __lxc_vm_def dynamic (except the UUID)
> - fix what looked like a funny leak situation when vm def structures were
> deallocated (see changes around lxcFreeVM/lxcFreeVMs/lxcFreeVMDef),
> i hope i didn't overlooked something but valgrind seems happy now leak
> wise.
> Overall it looks a bit simpler to me :-)
I agree :-)
>
> Valgrind only report left is
> ==1== Invalid write of size 4
> ==1== at 0x470C8C: lxcCheckContainerSupport (lxc_driver.c:91)
> ==1== Address 0xFFFFFFFFFFFFFFEC is not stack'd, malloc'd or (recently) free'd
> ==1==
> ==1== Process terminating with default action of signal 11 (SIGSEGV)
> ==1== Access not within mapped region at address 0xFFFFFFFFFFFFFFEC
> ==1== at 0x470C8C: lxcCheckContainerSupport (lxc_driver.c:91)
>
> which is related to the probe code of the driver
> ---------------------------
> stack = malloc(getpagesize() * 4);
> if(!stack) {
> DEBUG0("Unable to allocate stack");
> rc = -1;
> goto check_complete;
> }
>
> childStack = stack + (getpagesize() * 4);
>
> cpid = clone(lxcDummyChild, childStack, flags, NULL);
> ---------------------------
> I would assume it's valgrind being a bit pedantic because we pass the address
> just over the allocated stack limit, which should be fine since stack is
> addressed in predecrementing mode (BTW isn't that a portability bug in the
> code stack direction should probably be checked no ?).
This sounds like a pretty uncommon case - only the HP PA-RISC that I've found.
This could certainly be addressed though...
> Still maybe it's a good idea to pass a pointer in the allocated area ...
This could be done with a small loss (looks like 16 bytes) of stack space.
>
> Daniel
>
>
> void lxcFreeVMs(lxc_vm_t *vms)
> *************** void lxcFreeVMs(lxc_vm_t *vms)
> *** 859,865 ****
> while (curVm) {
> lxcFreeVM(curVm);
> nextVm = curVm->next;
> - free(curVm);
> curVm = nextVm;
> }
> }
[...]
> --- 819,825 ----
> void lxcFreeVM(lxc_vm_t *vm)
> {
> lxcFreeVMDef(vm->def);
> ! free(vm);
> }
This breaks doesn't it? After calling lxcFreeVM(), curVm is no longer valid
since it was free()'d. Need to pull out the next pointer before lxcFreeVM().
--
Best Regards,
Dave Leskovec
IBM Linux Technology Center
Open Virtualization
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list