Re: [libvirt] 'make check' hangs

At 11/30/2011 11:17 PM, Eric Blake Write:
> On 11/30/2011 04:40 AM, Daniel P. Berrange wrote:
>>> Hmm, I suspect this may be caused by
>>>
>>> commit fd7e85ac6af833845aa0eb2526158c319800a0ae
>>> Author: Jiri Denemark <jdenemar <at> redhat.com>
>>> Date:   Tue Oct 11 15:05:52 2011 +0200
>>>
>>>     virsh: Always run event loop
>>>
>>>     Since virsh already implements event loop, it has to also run it. So far
>>>     the event loop was only running during virsh console command.
>>>
>>
>> Ahhh, I had discounted that because the date was Oct 11, and I figured
>> we would have seen it by now. But that's just your original commit date,
>> the merge date was Oct 24th. So yeah, I reckon this must be the culprit
> 
> I've been seeing this fairly often (about 25% of my 'make check' runs)
> since at least last Wednesday, on F16 when compiling at -O0; and the
> analysis of a race caused by not locking around ctl->quit seems like a
> reasonable culprit.
> 

This may be one reason. I think there is some bug in testdriver, and it
will also cause 'make check' hang. Here is the backtrace when 'make check'
hung:

(gdb) info threads 
  2 Thread 0x7f22e81da700 (LWP 25242)  0x0000003bdce0dff4 in __lll_lock_wait () from /lib64/libpthread.so.0

Re: [libvirt] [RFC v3 PATCH 5/5] PowerPC : Add address-type "spapr-vio"

On Tue, 2011-11-29 at 12:16 -0500, Stefan Berger wrote:
> On 11/29/2011 10:03 AM, Prerna Saxena wrote:
> >  <at>  <at>  -3048,6 +3087,7  <at>  <at>  virDomainControllerDefParseXML(xmlNodePtr node,
> >       }
> >
> >       if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
> > +         def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO &&
> >           def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> >           virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >                                _("Controllers must use the 'pci' address type"));
> Is this still the proper error message for this type of bus? You may 
> want to look at def->os.arch / def->os.machine to see what is expected 
> here in terms of bus and then have a more specific error message.

No it's not the right message, which is why we're adding that check to
skip the message if the controller has an spapr-vio address. So I don't
think we need to anything else with arch/machine.

cheers

On Tue, 2011-11-29 at 12:16 -0500, Stefan Berger wrote:
> On 11/29/2011 10:03 AM, Prerna Saxena wrote:
> >  <at>  <at>  -3048,6 +3087,7  <at>  <at>  virDomainControllerDefParseXML(xmlNodePtr node,
> >       }
> >
> >       if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE &&
> > +         def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO &&
> >           def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {

Re: [libvirt] 'make check' hangs

On 2011年11月30日 19:28, Daniel Veillard wrote:
> On Wed, Nov 30, 2011 at 09:01:28AM +0000, Daniel P. Berrange wrote:
>> On Wed, Nov 30, 2011 at 11:18:23AM +0800, Wen Congyang wrote:
>>> When I run 'make check', it hangs sometimes.
>>>
>>> I use gdb to attach lt-virsh, and the following is the
>>> backtrace:
>>> (gdb) thread 1
>>> [Switching to thread 1 (Thread 0x7fc6c04d5800 (LWP 24099))]#0  0x0000003bdce0804d in pthread_join
() from /lib64/libpthread.so.0
>>> (gdb) bt
>>> #0  0x0000003bdce0804d in pthread_join () from /lib64/libpthread.so.0
>>> #1  0x000000000041c0b3 in vshDeinit (ctl=0x7fff015f4570) at virsh.c:17262
>>> #2  0x00000000004248d1 in main (argc=<value optimized out>, argv=0x7fff015f4728) at virsh.c:17608
>>> (gdb) thread 2
>>> [Switching to thread 2 (Thread 0x7fc6c04d4700 (LWP 24138))]#0  0x0000003bdc2dc053 in poll () from /lib64/libc.so.6
>>> (gdb) bt
>>> #0  0x0000003bdc2dc053 in poll () from /lib64/libc.so.6
>>> #1  0x00007fc6c075359c in virEventPollRunOnce () at util/event_poll.c:619
>>> #2  0x00007fc6c07527d7 in virEventRunDefaultImpl () at util/event.c:247
>>> #3  0x000000000041bea3 in vshEventLoop (opaque=0x7fff015f4570) at virsh.c:16800
>>> #4  0x00007fc6c0764702 in virThreadHelper (data=<value optimized out>) at util/threads-pthread.c:157
>>> #5  0x0000003bdce077f1 in start_thread () from /lib64/libpthread.so.0
>>> #6  0x0000003bdc2e570d in clone () from /lib64/libc.so.6
>>> (gdb)
>>
>>
>> I've seen this once, for the first time yesterday, but I can't
>> see any obvious change in either virsh, or the event code that
>> would have caused this to start happening in the past couple

Re: [libvirt] [PATCH 3/6] rpc: Plug memory leak on virNetClientSendInternal() error path

On 12/01/2011 07:11 AM, Eric Blake wrote:
> On 11/29/2011 11:40 PM, Alex Jia wrote:
>> On 11/30/2011 02:26 PM, Wen Congyang wrote:
>>> At 11/30/2011 01:57 PM, ajia <at> redhat.com Write:
>>>> From: Alex Jia<ajia <at> redhat.com>
>>>>
>>>> Detected by Coverity. Leak introduced in commit 673adba.
>>>>
>>> So I think we should not free call if it is still on the queue.
>> Yeah, it's a inappropriate place, in addition, in 'cleanup' label, if
>> ret !=1, also will free 'all', then 'unlock' label still do this, the
>> following should be a right place:
>>
>> 1708     if (!client->sock || client->wantClose) {
>> 1709         virNetError(VIR_ERR_INTERNAL_ERROR, "%s",
>> 1710                     _("client socket is closed"));
>> *1711       VIR_FREE(call);*
>> 1712         goto unlock;
>> 1713     }
> Closer, but still not quite right either.  You solved the failure if
> !client->sock, but still left in the bug that a failed virCondInit did
> goto cleanup, which then did a call to virCondDestroy on uninitialized
> data, which is undefined by POSIX.
>
> Rearranging some code and consolidating to one label will solve both
> bugs (the leak if !client->sock, and the incorrect virCondDestroy call
> on virCondInit failure), while still avoiding to free call on a partial
> send.  Here's what I'm pushing under your name.
Yeah, I just saw codes again. Eric, thanks.
> diff --git c/src/rpc/virnetclient.c w/src/rpc/virnetclient.c

Re: [libvirt] [PATCHv6 0/7] block io throttle via per-device block IO tuning

Eric,

You're very nice, and thanks a lot in my heart. ;)

On Thu, Dec 1, 2011 at 3:03 AM, Eric Blake <eblake <at> redhat.com> wrote:
> On 11/24/2011 01:32 AM, Daniel Veillard wrote:
>> On Wed, Nov 23, 2011 at 02:44:42PM -0700, Eric Blake wrote:
>>> Here's the latest state of Lei's patch series with all my comments
>>> folded in.  I may have a few more tweaks to make now that I'm at
>>> the point of testing things with both old qemu (graceful rejection)
>>> and new qemu (sensical return values), so it may be a day or two (or
>>> even a weekend, since this is a holiday weekend for me) before I
>>> actually push this, so it wouldn't hurt if anyone else wants to
>>> review in the meantime.
>>>
>>
>>   ACK to the serie once the bug on 3/7 is fixed, if possible fix the
>> couple of other small nits 
>
> Nits fixed and complete series now pushed.
>
> --
> Eric Blake   eblake <at> redhat.com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
>
> --
> libvir-list mailing list
> libvir-list <at> redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 4/6] uml: Plug memory leak on umlStartVMDaemon() error path

On 12/01/2011 07:39 AM, Eric Blake wrote:
> On 11/29/2011 10:57 PM, ajia <at> redhat.com wrote:
>> From: Alex Jia<ajia <at> redhat.com>
>>
>> Detected by Coverity.  Leak introduced in commit 8866eed.
>>
>> Signed-off-by: Alex Jia<ajia <at> redhat.com>
>> ---
>>   src/uml/uml_driver.c |    1 +
>>   1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
>> index 073f362..fe6d5ba 100644
>> --- a/src/uml/uml_driver.c
>> +++ b/src/uml/uml_driver.c
>>  <at>  <at>  -1051,6 +1051,7  <at>  <at>  static int umlStartVMDaemon(virConnectPtr conn,
>>           VIR_FREE(vm->def->consoles[i]->info.alias);
>>           if (virAsprintf(&vm->def->consoles[i]->info.alias, "console%zu", i)<  0) {
>>               virReportOOMError();
>> +            VIR_FORCE_CLOSE(logfd);
>>               goto cleanup;
> Good catch, but while reading the function, I found another bug - if we
> failed to add the domain to autodestroy, we still returned success but
> without setting the domain to transient.  I'm pushing this more
> comprehensive fix:
Indeed, the return value hasn't been stored in 'ret' if we failed to add 
domain
to autodestroy. Eric, thanks.
> diff --git i/src/uml/uml_driver.c w/src/uml/uml_driver.c
> index fe6d5ba..faea313 100644

Re: [libvirt] [PATCH 6/6] util: Plug memory leak on virNetDevMacVLanCreateWithVPortProfile() error path

On 12/01/2011 07:56 AM, Eric Blake wrote:
> On 11/29/2011 10:57 PM, ajia <at> redhat.com wrote:
>> From: Alex Jia<ajia <at> redhat.com>
>>
>> Detected by Coverity. Leak introduced in commit 90074ec.
>>
>> Signed-off-by: Alex Jia<ajia <at> redhat.com>
>> ---
>>   src/util/virnetdevmacvlan.c |    1 +
>>   1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
>> index 12fc411..80a9f80 100644
>> --- a/src/util/virnetdevmacvlan.c
>> +++ b/src/util/virnetdevmacvlan.c
>>  <at>  <at>  -582,6 +582,7  <at>  <at>  create_name:
>>           virNetDevError(VIR_ERR_INTERNAL_ERROR,
>>                          _("cannot set bandwidth limits on %s"),
>>                          cr_ifname);
>> +        VIR_FORCE_CLOSE(rc);
>>           rc = -1;
>>           goto disassociate_exit;
> Close, but this botches the case when !withTap (then, rc is 0 and you
> just nuked stdin).  Here's what I'm squashing before pushing (and
> hopefully Coverity isn't too confused because we overload rc to be
> either an fd or just 0 depending on withTap):
Yeah, Coverity complains 'rc' is overwritten, it has a little confused 
for this.
> diff --git i/src/util/virnetdevmacvlan.c w/src/util/virnetdevmacvlan.c
> index 80a9f80..5e55b72 100644

Re: [libvirt] [PATCH] spec: mark directories in /var/run as ghosts

On Wed, Nov 30, 2011 at 03:34:42PM -0700, Eric Blake wrote:
> We have several directories that are created on the fly, and which
> only contain state relevant to a running libvirtd process (all
> located in /var/run).  Since the directories are created as needed,
> and make no sense without a running libvirtd, we want them deleted
> if libvirt is uninstalled.  And in F15 and newer, /var/run is on
> tmpfs (forcing us to recreate on the fly); which means that someone
> trying to verify a complete rpm will fail if the directory does not
> currently exist because libvirtd has not been started since boot.
> The solution, then, is to mark the directories as %ghost, so that
> rpm knows that we own them and will clean it up if libvirt is
> uninstalled, but will no longer create the directory for us at
> install, nor complain at verify time if the directory does not exist.
> 
> See https://bugzilla.redhat.com/show_bug.cgi?id=656611.
> 
> * libvirt.spec.in (%files): Add %ghost to temporary directories
> that we don't install, but want cleaned up on libvirt removal.
> ---
>  libvirt.spec.in |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index f61a243..06c949b 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
>  <at>  <at>  -1082,7 +1082,7  <at>  <at>  rm -f $RPM_BUILD_ROOT%{_sysconfdir}/sysctl.d/libvirtd
>  %{_datadir}/libvirt/networks/default.xml
>  %endif
> 

Re: [libvirt] [PATCH] virsh: Fix possible deadlock when virsh is about to exit

On Wed, Nov 30, 2011 at 02:59:56PM -0700, Eric Blake wrote:
> On 11/30/2011 12:42 PM, Jiri Denemark wrote:
> > Not only was ctl->quit accessed without a mutex but unfortunately,
> > virEventAddTimeout only interrupts the poll when event loop is running
> > so the hack needs to add a timeout that will make next poll return
> > immediately without blocking.
> > ---
> >  tools/virsh.c |   39 +++++++++++++++++++++++++++++++++------
> >  1 files changed, 33 insertions(+), 6 deletions(-)
> 
> ACK.

  ACK too, and pushed !

   thanks !

Daniel

--

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel <at> veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

[libvirt] Start of freeze for libvirt-0.9.8 and availability of rc1

  We are now entering the freeze for libvirt-0.9.8 .
For portability sake I think adding more support for PPC machines
should still be allowed for a few days, as long as it's not impacting
too much of the common code.

I have made a release candidate 1 tarball (and associated rpms) at
   ftp://libvirt.org/libvirt/libvirt-0.9.8-rc1.tar.gz
and the git tree is tagged.

I think I will make an rc2 on Mon or Thu and then try to
make the release around Thursday next week if things
looks good.

 Please give it a try !

   thanks in advance,

Daniel

--

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel <at> veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/