8:03 a.m.
Patch against CVS version, it tries to avoid 2 issues:
- problem with global data settings when loading of the driver got
interrupted. Check more NULLs and reset to NULL, i was getting crashes
when running the regression tests and the /etc/libvirtd/lxc was not
accessible.
- avoid trying to open the driver if not running as root, that's not
a problem in normal case because the daemon runs as root, but
when running 'make check' as a normal user libvirtd is run as an
user and compiling the lxc driver in would break the regression tests
Dave could you double-check this, hopefully it has no side effect ?
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/
8:46 a.m.
New subject: [Libvir] Lxc conf patch
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 :-)
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 ?). Still maybe it's a
good idea to pass a pointer in the allocated area ...
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/
7:09 a.m.
New subject: [Libvir] Lxc conf patch
On Fri, Mar 28, 2008 at 09:46:38AM -0400, 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 :-)
okay, patches applied in CVS, tell me if there is any problem
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/
5:06 p.m.
New subject: [Libvir] Re: Lxc conf patch
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
5:24 p.m.
New subject: [Libvir] Re: Lxc conf patch
Meant to include this in the last reply - Here's a patch for the lxcFreeVMs() issue:
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
8:01 a.m.
New subject: [Libvir] Re: Lxc conf patch
On Mon, Mar 31, 2008 at 03:24:22PM -0700, Dave Leskovec wrote:
Meant to include this in the last reply - Here's a patch for the
lxcFreeVMs() issue:
>
> 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().
yup, obviously access to freed memory, my bad ...
while (curVm) {
- lxcFreeVM(curVm);
nextVm = curVm->next;
+ lxcFreeVM(curVm);
curVm = nextVm;
Applied and commited,
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/
7:10 a.m.
On Fri, Mar 28, 2008 at 09:03:26AM -0400, Daniel Veillard wrote:
Patch against CVS version, it tries to avoid 2 issues:
- problem with global data settings when loading of the driver got
interrupted. Check more NULLs and reset to NULL, i was getting crashes
when running the regression tests and the /etc/libvirtd/lxc was not
accessible.
- avoid trying to open the driver if not running as root, that's not
a problem in normal case because the daemon runs as root, but
when running 'make check' as a normal user libvirtd is run as an
user and compiling the lxc driver in would break the regression tests
Patch applied to CVS too,
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/
1:51 p.m.
Daniel Veillard wrote:
On Fri, Mar 28, 2008 at 09:03:26AM -0400, Daniel Veillard wrote:
> Patch against CVS version, it tries to avoid 2 issues:
> - problem with global data settings when loading of the driver got
> interrupted. Check more NULLs and reset to NULL, i was getting crashes
> when running the regression tests and the /etc/libvirtd/lxc was not
> accessible.
> - avoid trying to open the driver if not running as root, that's not
> a problem in normal case because the daemon runs as root, but
> when running 'make check' as a normal user libvirtd is run as an
> user and compiling the lxc driver in would break the regression tests
Patch applied to CVS too,
Daniel
Great, looks good. Thanks!
--
Best Regards,
Dave Leskovec
IBM Linux Technology Center
Open Virtualization
5892
days inactive
5896
days old
7 comments
2 participants
participants (2)
-
Daniel Veillard -
Dave Leskovec