[libvirt] Start of freeze for libvirt-0.9.5 and availability of rc1
With a bit of delay, we are entering the freeze for libvirt-0.9.5 . We may make an exception for the last couple of patches from Hu to add string to typed parameters and the extra associated patch, as well as bug fixes too of course ! I have made a release candidate 1 tarball (and associated rpms) at ftp://libvirt.org/libvirt/libvirt-0.9.5-rc1.tar.gz This seems to pass my minimal tests without problems, but please give it a try too and report problems, I think I will make an rc2 next Monday (or earlier) and then try to make the release around Wednesday next week thanks in advance, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 08/09/2011, at 5:29 PM, Daniel Veillard wrote:
With a bit of delay, we are entering the freeze for libvirt-0.9.5 . We may make an exception for the last couple of patches from Hu to add string to typed parameters and the extra associated patch, as well as bug fixes too of course !
I have made a release candidate 1 tarball (and associated rpms) at ftp://libvirt.org/libvirt/libvirt-0.9.5-rc1.tar.gz
This seems to pass my minimal tests without problems, but please give it a try too and report problems,
Just tried it on OSX 10.7.1 64-bit. It barfs with the following: CC libvirt_driver_storage_la-storage_backend_scsi.lo CC libvirt_net_rpc_server_la-virnetserverprogram.lo storage/storage_backend_fs.c:616: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'virStorageBackendFileSystemProbe' storage/storage_backend_fs.c: In function 'virStorageBackendExecuteMKFS': storage/storage_backend_fs.c:635: error: 'MKFS' undeclared (first use in this function) storage/storage_backend_fs.c:635: error: (Each undeclared identifier is reported only once storage/storage_backend_fs.c:635: error: for each function it appears in.) storage/storage_backend_fs.c: In function 'virStorageBackendMakeFileSystem': storage/storage_backend_fs.c:681: error: 'FILESYSTEM_PROBE_NOT_FOUND' undeclared (first use in this function) make[3]: *** [libvirt_driver_storage_la-storage_backend_fs.lo] Error 1 make[3]: *** Waiting for unfinished jobs.... make[2]: *** [all] Error 2 make[1]: *** [all-recursive] Error 1 make: *** [all] Error 2 Does anyone have time to look into it? If it's helpful, the Mac Mini in the Westford RH lab was recently upgraded to OSX 10.7, so could be used for testing/debugging if someone wants. :) NOTE - I'm not subscribed to libvir-list, so please CC me on responses. :) Regards and best wishes, Justin Clift -- Aeolus Community Manager http://www.aeolusproject.org
On Thu, Sep 08, 2011 at 06:54:08PM +1000, Justin Clift thus spake:
On 08/09/2011, at 5:29 PM, Daniel Veillard wrote:
With a bit of delay, we are entering the freeze for libvirt-0.9.5 . We may make an exception for the last couple of patches from Hu to add string to typed parameters and the extra associated patch, as well as bug fixes too of course !
I have made a release candidate 1 tarball (and associated rpms) at ftp://libvirt.org/libvirt/libvirt-0.9.5-rc1.tar.gz
This seems to pass my minimal tests without problems, but please give it a try too and report problems,
Just tried it on OSX 10.7.1 64-bit. It barfs with the following:
CC libvirt_driver_storage_la-storage_backend_scsi.lo CC libvirt_net_rpc_server_la-virnetserverprogram.lo storage/storage_backend_fs.c:616: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'virStorageBackendFileSystemProbe' storage/storage_backend_fs.c: In function 'virStorageBackendExecuteMKFS': storage/storage_backend_fs.c:635: error: 'MKFS' undeclared (first use in this function) storage/storage_backend_fs.c:635: error: (Each undeclared identifier is reported only once storage/storage_backend_fs.c:635: error: for each function it appears in.) storage/storage_backend_fs.c: In function 'virStorageBackendMakeFileSystem': storage/storage_backend_fs.c:681: error: 'FILESYSTEM_PROBE_NOT_FOUND' undeclared (first use in this function) make[3]: *** [libvirt_driver_storage_la-storage_backend_fs.lo] Error 1 make[3]: *** Waiting for unfinished jobs.... make[2]: *** [all] Error 2 make[1]: *** [all-recursive] Error 1 make: *** [all] Error 2
Does anyone have time to look into it?
If it's helpful, the Mac Mini in the Westford RH lab was recently upgraded to OSX 10.7, so could be used for testing/debugging if someone wants. :)
NOTE - I'm not subscribed to libvir-list, so please CC me on responses. :)
Regards and best wishes,
Justin Clift
I can confirm this build issue on FreeBSD. -jgh -- Jason Helfman System Administrator experts-exchange.com http://www.experts-exchange.com/M_4830110.html E4AD 7CF1 1396 27F6 79DD 4342 5E92 AD66 8C8C FBA5
Daniel Veillard wrote:
With a bit of delay, we are entering the freeze for libvirt-0.9.5 . We may make an exception for the last couple of patches from Hu to add string to typed parameters and the extra associated patch, as well as bug fixes too of course !
Would it be possible to get a review of the remaining virDomainMigrateGetMaxSpeed V2 patches? Along with the new API, it would be nice to have a qemu impl in the release. Thanks! Jim https://www.redhat.com/archives/libvir-list/2011-September/msg00087.html https://www.redhat.com/archives/libvir-list/2011-September/msg00091.html https://www.redhat.com/archives/libvir-list/2011-September/msg00088.html https://www.redhat.com/archives/libvir-list/2011-September/msg00089.html https://www.redhat.com/archives/libvir-list/2011-September/msg00090.html
I have made a second release candidate tarball (and associated rpms) at ftp://libvirt.org/libvirt/libvirt-0.9.5-rc2.tar.gz and tagged in git for it. I'm afraid we still didn't fix the MacOS-X / BSD problem , there is also some new code compared to rc1, so it's likely I will do an rc3 on Friday, and push the final release only beginning of last week. Please give it a try, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
I have made a third release candidate tarball (and associated rpms) at ftp://libvirt.org/libvirt/libvirt-0.9.5-rc2.tar.gz and tagged in git for it. This one should fis the MacOS-X/BSD portability problem thanks to Peter and Eric, and if everything goes well I will probably release 0.9.5 on Monday or more likely on Tuesday to give people a bit more time to test that latest candidate So give it a try, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Sun, Sep 18, 2011 at 06:55:06PM +0800, Daniel Veillard thus spake:
I have made a third release candidate tarball (and associated rpms) at ftp://libvirt.org/libvirt/libvirt-0.9.5-rc2.tar.gz and tagged in git for it. This one should fis the MacOS-X/BSD portability problem thanks to Peter and Eric, and if everything goes well I will probably release 0.9.5 on Monday or more likely on Tuesday to give people a bit more time to test that latest candidate
So give it a try, thanks !
Daniel
Failed build on FreeBSD: gmake[3]: Entering directory `/home/jhelfman/ports/devel/libvirt/work/libvirt-0.9.5/src' CCLD libvirt_iohelper ./.libs/libvirt_util.a(libvirt_util_la-threads.o)(.text+0x263): In function `virThreadCreate': : undefined reference to `pthread_create' -jgh -- Jason Helfman System Administrator experts-exchange.com http://www.experts-exchange.com/M_4830110.html E4AD 7CF1 1396 27F6 79DD 4342 5E92 AD66 8C8C FBA5
On Sun, Sep 18, 2011 at 08:48:23PM -0700, Jason Helfman wrote:
On Sun, Sep 18, 2011 at 06:55:06PM +0800, Daniel Veillard thus spake:
I have made a third release candidate tarball (and associated rpms) at ftp://libvirt.org/libvirt/libvirt-0.9.5-rc2.tar.gz and tagged in git for it. This one should fis the MacOS-X/BSD portability problem thanks to Peter and Eric, and if everything goes well I will probably release 0.9.5 on Monday or more likely on Tuesday to give people a bit more time to test that latest candidate
So give it a try, thanks !
Daniel
Failed build on FreeBSD:
gmake[3]: Entering directory `/home/jhelfman/ports/devel/libvirt/work/libvirt-0.9.5/src' CCLD libvirt_iohelper ./.libs/libvirt_util.a(libvirt_util_la-threads.o)(.text+0x263): In function `virThreadCreate': : undefined reference to `pthread_create'
Hum, it compiles so you have pthreads on the system, but maybe it needs to be linked through a special -lpthread linker option Looking at configure.ac it seems that gnulib is setting $LIB_PTHREAD can you look in your config.log for pthread lookup result and in the resulting src/Makefile to see how LIB_PTHREAD is set, thanks, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Mon, Sep 19, 2011 at 11:56:52AM +0800, Daniel Veillard thus spake:
On Sun, Sep 18, 2011 at 08:48:23PM -0700, Jason Helfman wrote:
On Sun, Sep 18, 2011 at 06:55:06PM +0800, Daniel Veillard thus spake:
I have made a third release candidate tarball (and associated rpms) at ftp://libvirt.org/libvirt/libvirt-0.9.5-rc2.tar.gz and tagged in git for it. This one should fis the MacOS-X/BSD portability problem thanks to Peter and Eric, and if everything goes well I will probably release 0.9.5 on Monday or more likely on Tuesday to give people a bit more time to test that latest candidate
So give it a try, thanks !
Daniel
Failed build on FreeBSD:
gmake[3]: Entering directory `/home/jhelfman/ports/devel/libvirt/work/libvirt-0.9.5/src' CCLD libvirt_iohelper ./.libs/libvirt_util.a(libvirt_util_la-threads.o)(.text+0x263): In function `virThreadCreate': : undefined reference to `pthread_create'
Hum, it compiles so you have pthreads on the system, but maybe it needs to be linked through a special -lpthread linker option Looking at configure.ac it seems that gnulib is setting $LIB_PTHREAD can you look in your config.log for pthread lookup result and in the resulting src/Makefile to see how LIB_PTHREAD is set,
thanks,
Daniel
That value is empty. LIB_PTHREAD = -jgh -- Jason Helfman System Administrator experts-exchange.com http://www.experts-exchange.com/M_4830110.html E4AD 7CF1 1396 27F6 79DD 4342 5E92 AD66 8C8C FBA5
On Mon, Sep 19, 2011 at 09:23:02AM -0700, Jason Helfman thus spake:
On Mon, Sep 19, 2011 at 11:56:52AM +0800, Daniel Veillard thus spake:
On Sun, Sep 18, 2011 at 08:48:23PM -0700, Jason Helfman wrote:
On Sun, Sep 18, 2011 at 06:55:06PM +0800, Daniel Veillard thus spake:
I have made a third release candidate tarball (and associated rpms) at ftp://libvirt.org/libvirt/libvirt-0.9.5-rc2.tar.gz and tagged in git for it. This one should fis the MacOS-X/BSD portability problem thanks to Peter and Eric, and if everything goes well I will probably release 0.9.5 on Monday or more likely on Tuesday to give people a bit more time to test that latest candidate
So give it a try, thanks !
Daniel
Failed build on FreeBSD:
gmake[3]: Entering directory `/home/jhelfman/ports/devel/libvirt/work/libvirt-0.9.5/src' CCLD libvirt_iohelper ./.libs/libvirt_util.a(libvirt_util_la-threads.o)(.text+0x263): In function `virThreadCreate': : undefined reference to `pthread_create'
Hum, it compiles so you have pthreads on the system, but maybe it needs to be linked through a special -lpthread linker option Looking at configure.ac it seems that gnulib is setting $LIB_PTHREAD can you look in your config.log for pthread lookup result and in the resulting src/Makefile to see how LIB_PTHREAD is set,
thanks,
Daniel
That value is empty. LIB_PTHREAD =
-jgh
Any ideas on how this is empty? Thanks, Jason -- Jason Helfman System Administrator experts-exchange.com http://www.experts-exchange.com/M_4830110.html E4AD 7CF1 1396 27F6 79DD 4342 5E92 AD66 8C8C FBA5
于 2011年09月20日 12:43, Jason Helfman 写道:
On Mon, Sep 19, 2011 at 09:23:02AM -0700, Jason Helfman thus spake:
On Mon, Sep 19, 2011 at 11:56:52AM +0800, Daniel Veillard thus spake:
On Sun, Sep 18, 2011 at 08:48:23PM -0700, Jason Helfman wrote:
On Sun, Sep 18, 2011 at 06:55:06PM +0800, Daniel Veillard thus spake:
I have made a third release candidate tarball (and associated rpms) at ftp://libvirt.org/libvirt/libvirt-0.9.5-rc2.tar.gz and tagged in git for it. This one should fis the MacOS-X/BSD portability problem thanks to Peter and Eric, and if everything goes well I will probably release 0.9.5 on Monday or more likely on Tuesday to give people a bit more time to test that latest candidate
So give it a try, thanks !
Daniel
Failed build on FreeBSD:
gmake[3]: Entering directory `/home/jhelfman/ports/devel/libvirt/work/libvirt-0.9.5/src' CCLD libvirt_iohelper ./.libs/libvirt_util.a(libvirt_util_la-threads.o)(.text+0x263): In function `virThreadCreate': : undefined reference to `pthread_create'
Hum, it compiles so you have pthreads on the system, but maybe it needs to be linked through a special -lpthread linker option Looking at configure.ac it seems that gnulib is setting $LIB_PTHREAD can you look in your config.log for pthread lookup result and in the resulting src/Makefile to see how LIB_PTHREAD is set,
thanks,
Daniel
That value is empty. LIB_PTHREAD =
-jgh
Any ideas on how this is empty?
Thanks, Jason
Should be caused by gl_PTHREAD_CHECK of gnulib doesn't work well on FreeBSD. But don't have a FreeBSD box in hand. Osier
On Tue, Sep 20, 2011 at 01:08:18PM +0800, Osier Yang thus spake:
于 2011年09月20日 12:43, Jason Helfman 写道:
On Mon, Sep 19, 2011 at 09:23:02AM -0700, Jason Helfman thus spake:
On Mon, Sep 19, 2011 at 11:56:52AM +0800, Daniel Veillard thus spake:
On Sun, Sep 18, 2011 at 08:48:23PM -0700, Jason Helfman wrote:
On Sun, Sep 18, 2011 at 06:55:06PM +0800, Daniel Veillard thus spake:
I have made a third release candidate tarball (and associated rpms) at ftp://libvirt.org/libvirt/libvirt-0.9.5-rc2.tar.gz and tagged in git for it. This one should fis the MacOS-X/BSD portability problem thanks to Peter and Eric, and if everything goes well I will probably release 0.9.5 on Monday or more likely on Tuesday to give people a bit more time to test that latest candidate
So give it a try, thanks !
Daniel
Failed build on FreeBSD:
gmake[3]: Entering directory `/home/jhelfman/ports/devel/libvirt/work/libvirt-0.9.5/src' CCLD libvirt_iohelper ./.libs/libvirt_util.a(libvirt_util_la-threads.o)(.text+0x263): In function `virThreadCreate': : undefined reference to `pthread_create'
Hum, it compiles so you have pthreads on the system, but maybe it needs to be linked through a special -lpthread linker option Looking at configure.ac it seems that gnulib is setting $LIB_PTHREAD can you look in your config.log for pthread lookup result and in the resulting src/Makefile to see how LIB_PTHREAD is set,
thanks,
Daniel
That value is empty. LIB_PTHREAD =
-jgh
Any ideas on how this is empty?
Thanks, Jason
Should be caused by gl_PTHREAD_CHECK of gnulib doesn't work well on FreeBSD. But don't have a FreeBSD box in hand.
Osier
I was able to get this to compile by adding the following to the port: --enable-pthreads And the following to the configure enviroment "-pthread" for ldflags. -jgh -- Jason Helfman System Administrator experts-exchange.com http://www.experts-exchange.com/M_4830110.html E4AD 7CF1 1396 27F6 79DD 4342 5E92 AD66 8C8C FBA5
On 18/09/2011, at 8:55 PM, Daniel Veillard wrote:
I have made a third release candidate tarball (and associated rpms) at ftp://libvirt.org/libvirt/libvirt-0.9.5-rc2.tar.gz and tagged in git for it. This one should fis the MacOS-X/BSD portability problem thanks to Peter and Eric, and if everything goes well I will probably release 0.9.5 on Monday or more likely on Tuesday to give people a bit more time to test that latest candidate
So give it a try, thanks !
Yep, seems ok on OSX 10.7.1 64-bit now. Regards and best wishes, Justin Clift -- Aeolus Community Manager http://www.aeolusproject.org
I am getting SIGABRT and SIGSEGV in libvirtd when trying to catch blockJob events. When running under valgrind I get the following: ==19819== Thread 1: ==19819== Invalid free() / delete / delete[] ==19819== at 0x4C282ED: free (vg_replace_malloc.c:366) ==19819== by 0x4E7B48: virFree (memory.c:310) ==19819== by 0x7669C32: virDomainEventFree (domain_event.c:510) ==19819== by 0x766AFE2: virDomainEventQueueDispatch (domain_event.c:1154) ==19819== by 0x766B19D: virDomainEventStateFlush (domain_event.c:1195) ==19819== by 0x483E15: qemuDomainEventFlush (qemu_domain.c:134) ==19819== by 0x507535: virEventPollRunOnce (event_poll.c:421) ==19819== by 0x4E6D44: virEventRunDefaultImpl (event.c:247) ==19819== by 0x44813C: virNetServerRun (virnetserver.c:701) ==19819== by 0x41FECE: main (libvirtd.c:1564) ==19819== Address 0x131b0a30 is 0 bytes inside a block of size 15 free'd ==19819== at 0x4C282ED: free (vg_replace_malloc.c:366) ==19819== by 0x7FB006C: xdr_string (xdr.c:722) ==19819== by 0x43A5FD: xdr_remote_nonnull_string (remote_protocol.c:30) ==19819== by 0x442E2B: xdr_remote_domain_event_block_job_msg (remote_protocol.c:4000) ==19819== by 0x7FAF6C4: xdr_free (xdr.c:72) ==19819== by 0x431BDA: remoteRelayDomainEventBlockJob (remote.c:363) ==19819== by 0x766ADBA: virDomainEventDispatchDefaultFunc (domain_event.c:1079) ==19819== by 0x482C67: qemuDomainEventDispatchFunc (qemu_domain.c:125) ==19819== by 0x766AF3D: virDomainEventDispatch (domain_event.c:1136) ==19819== by 0x766AFD1: virDomainEventQueueDispatch (domain_event.c:1153) ==19819== by 0x766B19D: virDomainEventStateFlush (domain_event.c:1195) ==19819== by 0x483E15: qemuDomainEventFlush (qemu_domain.c:134) ==19819== On a different recreate under gdb I get: Program received signal SIGSEGV, Segmentation fault. malloc_consolidate (av=0x7f4220000020) at malloc.c:5155 5155 malloc.c: No such file or directory. in malloc.c (gdb) bt #0 malloc_consolidate (av=0x7f4220000020) at malloc.c:5155 #1 0x00007f422ef09528 in _int_free (av=0x7f4220000020, p=0x7f4220080f50) at malloc.c:5034 #2 0x00007f422ef0d8e3 in __libc_free (mem=<value optimized out>) at malloc.c:3738 #3 0x00000000004e7b29 in virFree (ptrptr=0x7fff5f07a458) at util/memory.c:310 #4 0x000000000044a1cf in virNetMessageFree (msg=0x7f4220080f60) at rpc/virnetmessage.c:69 #5 0x0000000000445d4a in virNetServerClientDispatchWrite ( sock=<value optimized out>, events=2, opaque=0x7f4220000b90) at rpc/virnetserverclient.c:902 #6 virNetServerClientDispatchEvent (sock=<value optimized out>, events=2, opaque=0x7f4220000b90) at rpc/virnetserverclient.c:956 #7 0x0000000000507787 in virEventPollDispatchHandles () at util/event_poll.c:470 #8 virEventPollRunOnce () at util/event_poll.c:611 #9 0x00000000004e6d25 in virEventRunDefaultImpl () at util/event.c:247 #10 0x000000000044811d in virNetServerRun (srv=0x1dfec10) at rpc/virnetserver.c:701 #11 0x000000000041feaf in main (argc=<value optimized out>, argv=<value optimized out>) at libvirtd.c:1564 Looks like a double free somewhere. Commit: a91d3115b5c460af8a6f70d2092d0bc5ef9b723e seems to have surfaced the problem. On Thu, Sep 15, 2011 at 12:11:13AM +0800, Daniel Veillard wrote:
I have made a second release candidate tarball (and associated rpms) at ftp://libvirt.org/libvirt/libvirt-0.9.5-rc2.tar.gz and tagged in git for it. I'm afraid we still didn't fix the MacOS-X / BSD problem , there is also some new code compared to rc1, so it's likely I will do an rc3 on Friday, and push the final release only beginning of last week.
Please give it a try, thanks !
Daniel
-- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Adam Litke <agl@us.ibm.com> IBM Linux Technology Center
On Sun, Sep 18, 2011 at 09:37:22AM -0500, Adam Litke wrote:
I am getting SIGABRT and SIGSEGV in libvirtd when trying to catch blockJob events.
When running under valgrind I get the following: ==19819== Thread 1: ==19819== Invalid free() / delete / delete[] ==19819== at 0x4C282ED: free (vg_replace_malloc.c:366) ==19819== by 0x4E7B48: virFree (memory.c:310) ==19819== by 0x7669C32: virDomainEventFree (domain_event.c:510) ==19819== by 0x766AFE2: virDomainEventQueueDispatch (domain_event.c:1154) ==19819== by 0x766B19D: virDomainEventStateFlush (domain_event.c:1195) ==19819== by 0x483E15: qemuDomainEventFlush (qemu_domain.c:134) ==19819== by 0x507535: virEventPollRunOnce (event_poll.c:421) ==19819== by 0x4E6D44: virEventRunDefaultImpl (event.c:247) ==19819== by 0x44813C: virNetServerRun (virnetserver.c:701) ==19819== by 0x41FECE: main (libvirtd.c:1564) ==19819== Address 0x131b0a30 is 0 bytes inside a block of size 15 free'd ==19819== at 0x4C282ED: free (vg_replace_malloc.c:366) ==19819== by 0x7FB006C: xdr_string (xdr.c:722) ==19819== by 0x43A5FD: xdr_remote_nonnull_string (remote_protocol.c:30) ==19819== by 0x442E2B: xdr_remote_domain_event_block_job_msg (remote_protocol.c:4000) ==19819== by 0x7FAF6C4: xdr_free (xdr.c:72) ==19819== by 0x431BDA: remoteRelayDomainEventBlockJob (remote.c:363)
Hum, I wonder if remoteRelayDomainEventBlockJob shouldn't strdup the path string instead of using it directly in the remote_domain_event_block_job_msg block. As a result since we now free the datapointed by the xdr message within remoteDispatchDomainEventSend() , this errors wasn't shown before but leads to a double free now. BTW it seems we don't check all allocations in the xdr code (on purpose ?) for example make_nonnull_domain() doesn't check a strdup. Could you check the following patch ? Daniel diff --git a/daemon/remote.c b/daemon/remote.c index 38bbb10..1d9156c 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -356,7 +356,11 @@ static int remoteRelayDomainEventBlockJob(virConnectPtr conn ATTRIBUTE_UNUSED, /* build return data */ memset(&data, 0, sizeof data); make_nonnull_domain(&data.dom, dom); - data.path = (char*)path; + data.path = strdup(path); + if (data.path == NULL) { + virReportOOMError(); + return -1; + } data.type = type; data.status = status; -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Mon, Sep 19, 2011 at 04:04:04PM +0800, Daniel Veillard wrote:
On Sun, Sep 18, 2011 at 09:37:22AM -0500, Adam Litke wrote: Hum, I wonder if remoteRelayDomainEventBlockJob shouldn't strdup the path string instead of using it directly in the remote_domain_event_block_job_msg block. As a result since we now free the datapointed by the xdr message within remoteDispatchDomainEventSend() , this errors wasn't shown before but leads to a double free now.
BTW it seems we don't check all allocations in the xdr code (on purpose ?) for example make_nonnull_domain() doesn't check a strdup.
Could you check the following patch ?
Yep, this seems to fix the problem (and an extra check with valgrind shows no memory leaks. Although I haven't verified it, the functions: remoteRelayDomainEventIOError remoteRelayDomainEventIOErrorReason remoteRelayDomainEventGraphics appear to have the same problem as well.
diff --git a/daemon/remote.c b/daemon/remote.c index 38bbb10..1d9156c 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -356,7 +356,11 @@ static int remoteRelayDomainEventBlockJob(virConnectPtr conn ATTRIBUTE_UNUSED, /* build return data */ memset(&data, 0, sizeof data); make_nonnull_domain(&data.dom, dom); - data.path = (char*)path; + data.path = strdup(path); + if (data.path == NULL) { + virReportOOMError(); + return -1; + } data.type = type; data.status = status;
Tested-by: Adam Litke <agl@us.ibm.com> -- Adam Litke <agl@us.ibm.com> IBM Linux Technology Center
On Mon, Sep 19, 2011 at 08:15:18AM -0500, Adam Litke wrote:
On Mon, Sep 19, 2011 at 04:04:04PM +0800, Daniel Veillard wrote:
On Sun, Sep 18, 2011 at 09:37:22AM -0500, Adam Litke wrote: Hum, I wonder if remoteRelayDomainEventBlockJob shouldn't strdup the path string instead of using it directly in the remote_domain_event_block_job_msg block. As a result since we now free the datapointed by the xdr message within remoteDispatchDomainEventSend() , this errors wasn't shown before but leads to a double free now.
BTW it seems we don't check all allocations in the xdr code (on purpose ?) for example make_nonnull_domain() doesn't check a strdup.
Could you check the following patch ?
Yep, this seems to fix the problem (and an extra check with valgrind shows no memory leaks. Although I haven't verified it, the functions:
remoteRelayDomainEventIOError remoteRelayDomainEventIOErrorReason remoteRelayDomainEventGraphics
appear to have the same problem as well.
Right though they do far more allocations. I ended up pushing the following patch which tries to be a bit better on handling memory allocation error, but it's not fully complete yet: Fix crash on events due to allocation errors remoteRelayDomainEventBlockJob, remoteRelayDomainEventIOError, remoteRelayDomainEventIOErrorReason and remoteRelayDomainEventGraphics were using const string directly in rpc structure, before calling remoteDispatchDomainEventSend(). But that routine now frees up all the pointed allocated memory from the rpc structure and we end up with a double free. This now strdup() all the strings passed and provide mem_error goto labels to be used when an allocation error occurs. Note that the cleanup isn't completely finished because all relaying function also call make_nonnull_domain() which also allocate a string and never handle the error case. This patches doesn't try to address this as this is only error correctness a priori and touches far more functions in this module: * daemon/remote.c: fix string allocations and memory error handling for remoteRelayDomainEventBlockJob, remoteRelayDomainEventIOError, remoteRelayDomainEventIOErrorReason and remoteRelayDomainEventGraphics diff --git a/daemon/remote.c b/daemon/remote.c index 45244f8..74e759a 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -234,9 +234,13 @@ static int remoteRelayDomainEventIOError(virConnectPtr conn ATTRIBUTE_UNUSED, /* build return data */ memset(&data, 0, sizeof data); + data.srcPath = strdup(srcPath); + if (data.srcPath == NULL) + goto mem_error; + data.devAlias = strdup(devAlias); + if (data.devAlias == NULL) + goto mem_error; make_nonnull_domain(&data.dom, dom); - data.srcPath = (char*)srcPath; - data.devAlias = (char*)devAlias; data.action = action; remoteDispatchDomainEventSend(client, remoteProgram, @@ -244,6 +248,11 @@ static int remoteRelayDomainEventIOError(virConnectPtr conn ATTRIBUTE_UNUSED, (xdrproc_t)xdr_remote_domain_event_io_error_msg, &data); return 0; +mem_error: + virReportOOMError(); + virFree(data.srcPath); + virFree(data.devAlias); + return -1; } @@ -266,17 +275,31 @@ static int remoteRelayDomainEventIOErrorReason(virConnectPtr conn ATTRIBUTE_UNUS /* build return data */ memset(&data, 0, sizeof data); - make_nonnull_domain(&data.dom, dom); - data.srcPath = (char*)srcPath; - data.devAlias = (char*)devAlias; + data.srcPath = strdup(srcPath); + if (data.srcPath == NULL) + goto mem_error; + data.devAlias = strdup(devAlias); + if (data.devAlias == NULL) + goto mem_error; data.action = action; - data.reason = (char*)reason; + data.reason = strdup(reason); + if (data.reason == NULL) + goto mem_error; + + make_nonnull_domain(&data.dom, dom); remoteDispatchDomainEventSend(client, remoteProgram, REMOTE_PROC_DOMAIN_EVENT_IO_ERROR_REASON, (xdrproc_t)xdr_remote_domain_event_io_error_reason_msg, &data); return 0; + +mem_error: + virReportOOMError(); + virFree(data.srcPath); + virFree(data.devAlias); + virFree(data.reason); + return -1; } @@ -308,33 +331,62 @@ static int remoteRelayDomainEventGraphics(virConnectPtr conn ATTRIBUTE_UNUSED, /* build return data */ memset(&data, 0, sizeof data); - make_nonnull_domain(&data.dom, dom); data.phase = phase; - data.authScheme = (char*)authScheme; - data.local.family = local->family; - data.local.node = (char *)local->node; - data.local.service = (char *)local->service; - data.remote.family = remote->family; - data.remote.node = (char*)remote->node; - data.remote.service = (char*)remote->service; + data.authScheme = strdup(authScheme); + if (data.authScheme == NULL) + goto mem_error; + + data.local.node = strdup(local->node); + if (data.local.node == NULL) + goto mem_error; + data.local.service = strdup(local->service); + if (data.local.service == NULL) + goto mem_error; + + data.remote.node = strdup(remote->node); + if (data.remote.node == NULL) + goto mem_error; + data.remote.service = strdup(remote->service); + if (data.remote.service == NULL) + goto mem_error; data.subject.subject_len = subject->nidentity; - if (VIR_ALLOC_N(data.subject.subject_val, data.subject.subject_len) < 0) { - virReportOOMError(); - return -1; - } + if (VIR_ALLOC_N(data.subject.subject_val, data.subject.subject_len) < 0) + goto mem_error; + for (i = 0 ; i < data.subject.subject_len ; i++) { - data.subject.subject_val[i].type = (char*)subject->identities[i].type; - data.subject.subject_val[i].name = (char*)subject->identities[i].name; + data.subject.subject_val[i].type = strdup(subject->identities[i].type); + if (data.subject.subject_val[i].type == NULL) + goto mem_error; + data.subject.subject_val[i].name = strdup(subject->identities[i].name); + if (data.subject.subject_val[i].name == NULL) + goto mem_error; } + make_nonnull_domain(&data.dom, dom); remoteDispatchDomainEventSend(client, remoteProgram, REMOTE_PROC_DOMAIN_EVENT_GRAPHICS, (xdrproc_t)xdr_remote_domain_event_graphics_msg, &data); return 0; + +mem_error: + virReportOOMError(); + virFree(data.authScheme); + virFree(data.local.node); + virFree(data.local.service); + virFree(data.remote.node); + virFree(data.remote.service); + if (data.subject.subject_val != NULL) { + for (i = 0 ; i < data.subject.subject_len ; i++) { + virFree(data.subject.subject_val[i].type); + virFree(data.subject.subject_val[i].name); + } + virFree(data.subject.subject_val); + } + return -1; } static int remoteRelayDomainEventBlockJob(virConnectPtr conn ATTRIBUTE_UNUSED, @@ -355,16 +407,23 @@ static int remoteRelayDomainEventBlockJob(virConnectPtr conn ATTRIBUTE_UNUSED, /* build return data */ memset(&data, 0, sizeof data); - make_nonnull_domain(&data.dom, dom); - data.path = (char*)path; + data.path = strdup(path); + if (data.path == NULL) + goto mem_error; data.type = type; data.status = status; + make_nonnull_domain(&data.dom, dom); remoteDispatchDomainEventSend(client, remoteProgram, REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB, (xdrproc_t)xdr_remote_domain_event_block_job_msg, &data); return 0; + +mem_error: + virReportOOMError(); + virFree(data.path); + return -1; } -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 09/19/2011 10:03 PM, Daniel Veillard wrote:
* daemon/remote.c: fix string allocations and memory error handling for remoteRelayDomainEventBlockJob, remoteRelayDomainEventIOError, remoteRelayDomainEventIOErrorReason and remoteRelayDomainEventGraphics
return 0; +mem_error: + virReportOOMError(); + virFree(data.srcPath); + virFree(data.devAlias);
This is broken. It will make us crash on OOM. See: https://www.redhat.com/archives/libvir-list/2011-September/msg00779.html -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Tue, Sep 20, 2011 at 12:03:00PM +0800, Daniel Veillard wrote:
On Mon, Sep 19, 2011 at 08:15:18AM -0500, Adam Litke wrote:
On Mon, Sep 19, 2011 at 04:04:04PM +0800, Daniel Veillard wrote:
On Sun, Sep 18, 2011 at 09:37:22AM -0500, Adam Litke wrote: Hum, I wonder if remoteRelayDomainEventBlockJob shouldn't strdup the path string instead of using it directly in the remote_domain_event_block_job_msg block. As a result since we now free the datapointed by the xdr message within remoteDispatchDomainEventSend() , this errors wasn't shown before but leads to a double free now.
BTW it seems we don't check all allocations in the xdr code (on purpose ?) for example make_nonnull_domain() doesn't check a strdup.
Could you check the following patch ?
Yep, this seems to fix the problem (and an extra check with valgrind shows no memory leaks. Although I haven't verified it, the functions:
remoteRelayDomainEventIOError remoteRelayDomainEventIOErrorReason remoteRelayDomainEventGraphics
appear to have the same problem as well.
Right though they do far more allocations. I ended up pushing the following patch which tries to be a bit better on handling memory allocation error, but it's not fully complete yet:
Fix crash on events due to allocation errors
remoteRelayDomainEventBlockJob, remoteRelayDomainEventIOError, remoteRelayDomainEventIOErrorReason and remoteRelayDomainEventGraphics were using const string directly in rpc structure, before calling remoteDispatchDomainEventSend(). But that routine now frees up all the pointed allocated memory from the rpc structure and we end up with a double free. This now strdup() all the strings passed and provide mem_error goto labels to be used when an allocation error occurs. Note that the cleanup isn't completely finished because all relaying function also call make_nonnull_domain() which also allocate a string and never handle the error case. This patches doesn't try to address this as this is only error correctness a priori and touches far more functions in this module:
* daemon/remote.c: fix string allocations and memory error handling for remoteRelayDomainEventBlockJob, remoteRelayDomainEventIOError, remoteRelayDomainEventIOErrorReason and remoteRelayDomainEventGraphics
diff --git a/daemon/remote.c b/daemon/remote.c index 45244f8..74e759a 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -234,9 +234,13 @@ static int remoteRelayDomainEventIOError(virConnectPtr conn ATTRIBUTE_UNUSED,
/* build return data */ memset(&data, 0, sizeof data); + data.srcPath = strdup(srcPath); + if (data.srcPath == NULL) + goto mem_error; + data.devAlias = strdup(devAlias); + if (data.devAlias == NULL) + goto mem_error; make_nonnull_domain(&data.dom, dom); - data.srcPath = (char*)srcPath; - data.devAlias = (char*)devAlias; data.action = action;
remoteDispatchDomainEventSend(client, remoteProgram, @@ -244,6 +248,11 @@ static int remoteRelayDomainEventIOError(virConnectPtr conn ATTRIBUTE_UNUSED, (xdrproc_t)xdr_remote_domain_event_io_error_msg, &data);
return 0; +mem_error: + virReportOOMError(); + virFree(data.srcPath); + virFree(data.devAlias); + return -1;
s/virFree/VIR_FREE/ throughout all this code. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Wed, Sep 21, 2011 at 10:55:37AM +0100, Daniel P. Berrange wrote:
On Tue, Sep 20, 2011 at 12:03:00PM +0800, Daniel Veillard wrote:
On Mon, Sep 19, 2011 at 08:15:18AM -0500, Adam Litke wrote:
On Mon, Sep 19, 2011 at 04:04:04PM +0800, Daniel Veillard wrote:
On Sun, Sep 18, 2011 at 09:37:22AM -0500, Adam Litke wrote: Hum, I wonder if remoteRelayDomainEventBlockJob shouldn't strdup the path string instead of using it directly in the remote_domain_event_block_job_msg block. As a result since we now free the datapointed by the xdr message within remoteDispatchDomainEventSend() , this errors wasn't shown before but leads to a double free now.
BTW it seems we don't check all allocations in the xdr code (on purpose ?) for example make_nonnull_domain() doesn't check a strdup.
Could you check the following patch ?
Yep, this seems to fix the problem (and an extra check with valgrind shows no memory leaks. Although I haven't verified it, the functions:
remoteRelayDomainEventIOError remoteRelayDomainEventIOErrorReason remoteRelayDomainEventGraphics
appear to have the same problem as well.
Right though they do far more allocations. I ended up pushing the following patch which tries to be a bit better on handling memory allocation error, but it's not fully complete yet:
Fix crash on events due to allocation errors
remoteRelayDomainEventBlockJob, remoteRelayDomainEventIOError, remoteRelayDomainEventIOErrorReason and remoteRelayDomainEventGraphics were using const string directly in rpc structure, before calling remoteDispatchDomainEventSend(). But that routine now frees up all the pointed allocated memory from the rpc structure and we end up with a double free. This now strdup() all the strings passed and provide mem_error goto labels to be used when an allocation error occurs. Note that the cleanup isn't completely finished because all relaying function also call make_nonnull_domain() which also allocate a string and never handle the error case. This patches doesn't try to address this as this is only error correctness a priori and touches far more functions in this module:
* daemon/remote.c: fix string allocations and memory error handling for remoteRelayDomainEventBlockJob, remoteRelayDomainEventIOError, remoteRelayDomainEventIOErrorReason and remoteRelayDomainEventGraphics
diff --git a/daemon/remote.c b/daemon/remote.c index 45244f8..74e759a 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -234,9 +234,13 @@ static int remoteRelayDomainEventIOError(virConnectPtr conn ATTRIBUTE_UNUSED,
/* build return data */ memset(&data, 0, sizeof data); + data.srcPath = strdup(srcPath); + if (data.srcPath == NULL) + goto mem_error; + data.devAlias = strdup(devAlias); + if (data.devAlias == NULL) + goto mem_error; make_nonnull_domain(&data.dom, dom); - data.srcPath = (char*)srcPath; - data.devAlias = (char*)devAlias; data.action = action;
remoteDispatchDomainEventSend(client, remoteProgram, @@ -244,6 +248,11 @@ static int remoteRelayDomainEventIOError(virConnectPtr conn ATTRIBUTE_UNUSED, (xdrproc_t)xdr_remote_domain_event_io_error_msg, &data);
return 0; +mem_error: + virReportOOMError(); + virFree(data.srcPath); + virFree(data.devAlias); + return -1;
s/virFree/VIR_FREE/ throughout all this code.
Yeah Eric pointed that out, I already commited his patch and the associated make syntax-check test :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (8)
-
Adam Litke -
Daniel P. Berrange -
Daniel Veillard -
Eric Blake -
Jason Helfman -
Jim Fehlig -
Justin Clift -
Osier Yang