3:34 p.m.
I'm getting a seg fault when running virsh snapshot-create 1, but only
when virt-manager is open and connected.
Here is some of the debug info I was able to come up with -
http://fpaste.org/9GO6/ (bt)
http://fpaste.org/7gkH/ ('thread apply all bt)
* After the crash
(gdb) p mon->msg
$1 = (qemuMonitorMessagePtr) 0x0
nibbler:~ # libvirtd --version
libvirtd (libvirt) 0.8.0
Please let me know if there is any other information you need.
Stephen
3:53 p.m.
On Wed, Apr 21, 2010 at 14:34, Stephen Shaw <sshaw(a)decriptor.com> wrote:
I'm getting a seg fault when running virsh snapshot-create 1, but
only
when virt-manager is open and connected.
Here is some of the debug info I was able to come up with -
http://fpaste.org/9GO6/ (bt)
http://fpaste.org/7gkH/ ('thread apply all bt)
Here is the text since those seem to be expired:
################ bt
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff14d4910 (LWP 10112)]
0x000000000047c570 in qemuMonitorSend (mon=0x730570,
msg=0x7ffff14d3ac0) at qemu/qemu_monitor.c:708
708 while (!mon->msg->finished) {
(gdb) bt
#0 0x000000000047c570 in qemuMonitorSend (mon=0x730570,
msg=0x7ffff14d3ac0) at qemu/qemu_monitor.c:708
#1 0x000000000047edb5 in qemuMonitorCommandWithHandler (mon=0x730570,
cmd=0x4c4df5 "info balloon", passwordHandler=0,
passwordOpaque=0x0, scm_fd=-1, reply=0x7ffff14d3bd0) at
qemu/qemu_monitor_text.c:232
#2 0x000000000047ef29 in qemuMonitorCommandWithFd (mon=0x730570,
cmd=0x4c4df5 "info balloon", scm_fd=-1, reply=0x7ffff14d3bd0)
at qemu/qemu_monitor_text.c:266
#3 0x000000000047ef64 in qemuMonitorCommand (mon=0x730570,
cmd=0x4c4df5 "info balloon", reply=0x7ffff14d3bd0)
at qemu/qemu_monitor_text.c:273
#4 0x000000000047f8cc in qemuMonitorTextGetBalloonInfo (mon=0x730570,
currmem=0x7ffff14d3c70) at qemu/qemu_monitor_text.c:552
#5 0x000000000047d04b in qemuMonitorGetBalloonInfo (mon=0x730570,
currmem=0x7ffff14d3c70) at qemu/qemu_monitor.c:958
#6 0x000000000044fd80 in qemudDomainGetInfo (dom=0x762af0,
info=0x7ffff14d3d70) at qemu/qemu_driver.c:4462
#7 0x00007ffff7897f5e in virDomainGetInfo (domain=0x762af0,
info=0x7ffff14d3d70) at libvirt.c:3010
#8 0x0000000000422019 in remoteDispatchDomainGetInfo
(server=0x6fcbf0, client=0x7fffe8000900, conn=0x7309d0,
hdr=0x7fffe8140c20,
rerr=0x7ffff14d3de0, args=0x7ffff14d3ee0, ret=0x7ffff14d3e80) at
remote.c:1417
#9 0x000000000042c8f1 in remoteDispatchClientCall (server=0x6fcbf0,
client=0x7fffe8000900, msg=0x7fffe8100c10) at dispatch.c:506
#10 0x000000000042c49a in remoteDispatchClientRequest
(server=0x6fcbf0, client=0x7fffe8000900, msg=0x7fffe8100c10) at
dispatch.c:388
#11 0x000000000041a94f in qemudWorker (data=0x6ff9d0) at libvirtd.c:1553
#12 0x00007ffff67ac65d in start_thread () from /lib64/libpthread.so.0
#13 0x00007ffff651be1d in clone () from /lib64/libc.so.6
#14 0x0000000000000000 in ?? ()
################# thread apply all bt
Thread 7 (Thread 0x7ffff04d2910 (LWP 10114)):
#0 0x00007ffff67b1049 in pthread_cond_wait@(a)GLIBC_2.3.2 () from
/lib64/libpthread.so.0
#1 0x000000000043b6a7 in virCondWait (c=0x6fcc18, m=0x6fcbf0) at
util/threads-pthread.c:100
#2 0x000000000041a88f in qemudWorker (data=0x6ffa00) at libvirtd.c:1532
#3 0x00007ffff67ac65d in start_thread () from /lib64/libpthread.so.0
#4 0x00007ffff651be1d in clone () from /lib64/libc.so.6
#5 0x0000000000000000 in ?? ()
Thread 6 (Thread 0x7ffff0cd3910 (LWP 10113)):
#0 0x00007ffff67b1049 in pthread_cond_wait@(a)GLIBC_2.3.2 () from
/lib64/libpthread.so.0
#1 0x000000000043b6a7 in virCondWait (c=0x6fcc18, m=0x6fcbf0) at
util/threads-pthread.c:100
#2 0x000000000041a88f in qemudWorker (data=0x6ff9e8) at libvirtd.c:1532
#3 0x00007ffff67ac65d in start_thread () from /lib64/libpthread.so.0
#4 0x00007ffff651be1d in clone () from /lib64/libc.so.6
#5 0x0000000000000000 in ?? ()
Thread 5 (Thread 0x7ffff14d4910 (LWP 10112)):
#0 0x000000000047c570 in qemuMonitorSend (mon=0x730570,
msg=0x7ffff14d3ac0) at qemu/qemu_monitor.c:708
#1 0x000000000047edb5 in qemuMonitorCommandWithHandler (mon=0x730570,
cmd=0x4c4df5 "info balloon", passwordHandler=0,
passwordOpaque=0x0, scm_fd=-1, reply=0x7ffff14d3bd0) at
qemu/qemu_monitor_text.c:232
#2 0x000000000047ef29 in qemuMonitorCommandWithFd (mon=0x730570,
cmd=0x4c4df5 "info balloon", scm_fd=-1, reply=0x7ffff14d3bd0)
at qemu/qemu_monitor_text.c:266
#3 0x000000000047ef64 in qemuMonitorCommand (mon=0x730570,
cmd=0x4c4df5 "info balloon", reply=0x7ffff14d3bd0)
at qemu/qemu_monitor_text.c:273
#4 0x000000000047f8cc in qemuMonitorTextGetBalloonInfo (mon=0x730570,
currmem=0x7ffff14d3c70) at qemu/qemu_monitor_text.c:552
#5 0x000000000047d04b in qemuMonitorGetBalloonInfo (mon=0x730570,
currmem=0x7ffff14d3c70) at qemu/qemu_monitor.c:958
#6 0x000000000044fd80 in qemudDomainGetInfo (dom=0x762af0,
info=0x7ffff14d3d70) at qemu/qemu_driver.c:4462
#7 0x00007ffff7897f5e in virDomainGetInfo (domain=0x762af0,
info=0x7ffff14d3d70) at libvirt.c:3010
#8 0x0000000000422019 in remoteDispatchDomainGetInfo
(server=0x6fcbf0, client=0x7fffe8000900, conn=0x7309d0,
hdr=0x7fffe8140c20,
rerr=0x7ffff14d3de0, args=0x7ffff14d3ee0, ret=0x7ffff14d3e80) at
remote.c:1417
#9 0x000000000042c8f1 in remoteDispatchClientCall (server=0x6fcbf0,
client=0x7fffe8000900, msg=0x7fffe8100c10) at dispatch.c:506
#10 0x000000000042c49a in remoteDispatchClientRequest
(server=0x6fcbf0, client=0x7fffe8000900, msg=0x7fffe8100c10) at
dispatch.c:388
---Type <return> to continue, or q <return> to quit---
#11 0x000000000041a94f in qemudWorker (data=0x6ff9d0) at libvirtd.c:1553
#12 0x00007ffff67ac65d in start_thread () from /lib64/libpthread.so.0
#13 0x00007ffff651be1d in clone () from /lib64/libc.so.6
#14 0x0000000000000000 in ?? ()
Thread 4 (Thread 0x7ffff1cd5910 (LWP 10110)):
#0 0x00007ffff67b1049 in pthread_cond_wait@(a)GLIBC_2.3.2 () from
/lib64/libpthread.so.0
#1 0x000000000043b6a7 in virCondWait (c=0x6fcc18, m=0x6fcbf0) at
util/threads-pthread.c:100
#2 0x000000000041a88f in qemudWorker (data=0x6ff9b8) at libvirtd.c:1532
#3 0x00007ffff67ac65d in start_thread () from /lib64/libpthread.so.0
#4 0x00007ffff651be1d in clone () from /lib64/libc.so.6
#5 0x0000000000000000 in ?? ()
Thread 3 (Thread 0x7ffff24d6910 (LWP 10108)):
#0 0x00007ffff67b1049 in pthread_cond_wait@(a)GLIBC_2.3.2 () from
/lib64/libpthread.so.0
#1 0x000000000043b6a7 in virCondWait (c=0x6fcc18, m=0x6fcbf0) at
util/threads-pthread.c:100
#2 0x000000000041a88f in qemudWorker (data=0x6ff9a0) at libvirtd.c:1532
#3 0x00007ffff67ac65d in start_thread () from /lib64/libpthread.so.0
#4 0x00007ffff651be1d in clone () from /lib64/libc.so.6
#5 0x0000000000000000 in ?? ()
Thread 2 (Thread 0x7ffff2cd7910 (LWP 10107)):
#0 0x00007ffff6512d03 in poll () from /lib64/libc.so.6
#1 0x0000000000416cea in virEventRunOnce () at event.c:593
#2 0x000000000041bfd8 in qemudOneLoop () at libvirtd.c:2200
#3 0x000000000041c4ef in qemudRunLoop (opaque=0x6fcbf0) at libvirtd.c:2309
#4 0x00007ffff67ac65d in start_thread () from /lib64/libpthread.so.0
#5 0x00007ffff651be1d in clone () from /lib64/libc.so.6
#6 0x0000000000000000 in ?? ()
Thread 1 (Thread 0x7ffff7fd2790 (LWP 10104)):
#0 0x00007ffff67ad96d in pthread_join () from /lib64/libpthread.so.0
#1 0x000000000041f265 in main (argc=2, argv=0x7fffffffe3e8) at libvirtd.c:3227
4:16 p.m.
On 04/21/2010 04:34 PM, Stephen Shaw wrote:
I'm getting a seg fault when running virsh snapshot-create 1, but
only
when virt-manager is open and connected.
Here is some of the debug info I was able to come up with -
http://fpaste.org/9GO6/ (bt)
http://fpaste.org/7gkH/ ('thread apply all bt)
* After the crash
(gdb) p mon->msg
$1 = (qemuMonitorMessagePtr) 0x0
nibbler:~ # libvirtd --version
libvirtd (libvirt) 0.8.0
Please let me know if there is any other information you need.
Stephen
Thanks for the report. To be perfectly honest, I can't see how what
happened could happen :). But I'll take a closer look at it and see
if I can reproduce and see what is going on with it.
--
Chris Lalancette
1:04 a.m.
On Wed, Apr 21, 2010 at 15:16, Chris Lalancette <clalance(a)redhat.com> wrote:
On 04/21/2010 04:34 PM, Stephen Shaw wrote:
> I'm getting a seg fault when running virsh snapshot-create 1, but only
> when virt-manager is open and connected.
>
> Here is some of the debug info I was able to come up with -
> http://fpaste.org/9GO6/ (bt)
> http://fpaste.org/7gkH/ ('thread apply all bt)
>
> * After the crash
> (gdb) p mon->msg
> $1 = (qemuMonitorMessagePtr) 0x0
>
>
> nibbler:~ # libvirtd --version
> libvirtd (libvirt) 0.8.0
>
>
> Please let me know if there is any other information you need.
> Stephen
Thanks for the report. To be perfectly honest, I can't see how what
happened could happen :). But I'll take a closer look at it and see
if I can reproduce and see what is going on with it.
--
Chris Lalancette
If you need me to get any more information I can. Its seg faults
every time, so very easily reproducible.
Stephen
6:51 a.m.
On Wed, Apr 21, 2010 at 05:16:21PM -0400, Chris Lalancette wrote:
On 04/21/2010 04:34 PM, Stephen Shaw wrote:
> I'm getting a seg fault when running virsh snapshot-create 1, but only
> when virt-manager is open and connected.
>
> Here is some of the debug info I was able to come up with -
> http://fpaste.org/9GO6/ (bt)
> http://fpaste.org/7gkH/ ('thread apply all bt)
>
> * After the crash
> (gdb) p mon->msg
> $1 = (qemuMonitorMessagePtr) 0x0
>
>
> nibbler:~ # libvirtd --version
> libvirtd (libvirt) 0.8.0
>
>
> Please let me know if there is any other information you need.
> Stephen
Thanks for the report. To be perfectly honest, I can't see how what
happened could happen :). But I'll take a closer look at it and see
if I can reproduce and see what is going on with it.
Seems that virt-manager polls the status of the domain(s) in parallel
and somehow the processing of the (unrelated) 'info balloon' call
lead to some corruption of the monitor queue.
I would guess that snapshot-create forgets to lock something around
the monitor leading to a corruption of the monitor message queue
handling. but analyzing precisely the bug is gonna be painful as for all
threaded debugging, maybe run with full trace enabled and wtach
carefully all the locking output for the 2 commands.
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
7:34 a.m.
On Wed, Apr 21, 2010 at 05:16:21PM -0400, Chris Lalancette wrote:
On 04/21/2010 04:34 PM, Stephen Shaw wrote:
> I'm getting a seg fault when running virsh snapshot-create 1, but only
> when virt-manager is open and connected.
>
> Here is some of the debug info I was able to come up with -
> http://fpaste.org/9GO6/ (bt)
> http://fpaste.org/7gkH/ ('thread apply all bt)
>
> * After the crash
> (gdb) p mon->msg
> $1 = (qemuMonitorMessagePtr) 0x0
>
>
> nibbler:~ # libvirtd --version
> libvirtd (libvirt) 0.8.0
>
>
> Please let me know if there is any other information you need.
> Stephen
Thanks for the report. To be perfectly honest, I can't see how what
happened could happen :). But I'll take a closer look at it and see
if I can reproduce and see what is going on with it.
I see thread locking problems in the code
- qemuDomainSnapshotCreateXML() is calling monitor commands, but has
not run qemuDomainObjBeginJobWithDriver() to ensure exclusive
access to the monitor
- qemuDomainSnapshotDiscard has same problem
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
7:54 a.m.
On 04/22/2010 08:34 AM, Daniel P. Berrange wrote:
On Wed, Apr 21, 2010 at 05:16:21PM -0400, Chris Lalancette wrote:
> On 04/21/2010 04:34 PM, Stephen Shaw wrote:
>> I'm getting a seg fault when running virsh snapshot-create 1, but only
>> when virt-manager is open and connected.
>>
>> Here is some of the debug info I was able to come up with -
>> http://fpaste.org/9GO6/ (bt)
>> http://fpaste.org/7gkH/ ('thread apply all bt)
>>
>> * After the crash
>> (gdb) p mon->msg
>> $1 = (qemuMonitorMessagePtr) 0x0
>>
>>
>> nibbler:~ # libvirtd --version
>> libvirtd (libvirt) 0.8.0
>>
>>
>> Please let me know if there is any other information you need.
>> Stephen
>
> Thanks for the report. To be perfectly honest, I can't see how what
> happened could happen :). But I'll take a closer look at it and see
> if I can reproduce and see what is going on with it.
I see thread locking problems in the code
- qemuDomainSnapshotCreateXML() is calling monitor commands, but has
not run qemuDomainObjBeginJobWithDriver() to ensure exclusive
access to the monitor
- qemuDomainSnapshotDiscard has same problem
Yep, just fixing those now. I didn't quite understand the ObjBeginJob
before, but I think I'm understanding it now. This is probably the source of
the problems.
--
Chris Lalancette
8:09 a.m.
On Thu, Apr 22, 2010 at 08:54:30AM -0400, Chris Lalancette wrote:
On 04/22/2010 08:34 AM, Daniel P. Berrange wrote:
> On Wed, Apr 21, 2010 at 05:16:21PM -0400, Chris Lalancette wrote:
>> On 04/21/2010 04:34 PM, Stephen Shaw wrote:
>>> I'm getting a seg fault when running virsh snapshot-create 1, but only
>>> when virt-manager is open and connected.
>>>
>>> Here is some of the debug info I was able to come up with -
>>> http://fpaste.org/9GO6/ (bt)
>>> http://fpaste.org/7gkH/ ('thread apply all bt)
>>>
>>> * After the crash
>>> (gdb) p mon->msg
>>> $1 = (qemuMonitorMessagePtr) 0x0
>>>
>>>
>>> nibbler:~ # libvirtd --version
>>> libvirtd (libvirt) 0.8.0
>>>
>>>
>>> Please let me know if there is any other information you need.
>>> Stephen
>>
>> Thanks for the report. To be perfectly honest, I can't see how what
>> happened could happen :). But I'll take a closer look at it and see
>> if I can reproduce and see what is going on with it.
>
> I see thread locking problems in the code
>
> - qemuDomainSnapshotCreateXML() is calling monitor commands, but has
> not run qemuDomainObjBeginJobWithDriver() to ensure exclusive
> access to the monitor
>
> - qemuDomainSnapshotDiscard has same problem
Yep, just fixing those now. I didn't quite understand the ObjBeginJob
before, but I think I'm understanding it now. This is probably the source of
the problems.
There's some notes about the rules in src/qemu/THREADS.txt.
You must acquire locks on objects in the following order, not missing
any steps.
qemud_driver (qemudDriverLock)
virDomainObjPtr (implicit via virDomainObjFindByXXX)
qemuMonitorPrivatePtr (qemuDomainObjBeginJob)
qemuMonitorPtr (implicit via qemuDomainObjEnterMonitor)
Note that qemuDomainObjEnterMonitor() will release the locks on the
qemud_driver & virDomainObjPtr objects, once it has acquired the lock
on qemuMonitorPtr. The qemuMonitorPrivatePtr object has a condition
variable that ensures continued mutual exclusion, even though the
qemud_driver, virDomainObjPtr object are now unlocked.
So by missing qemuDomainObjBeginJob(), the condition variable was not
acquired, and mutual exclusion was not assured
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
8:19 a.m.
On Thu, Apr 22, 2010 at 07:09, Daniel P. Berrange <berrange(a)redhat.com> wrote:
On Thu, Apr 22, 2010 at 08:54:30AM -0400, Chris Lalancette wrote:
> On 04/22/2010 08:34 AM, Daniel P. Berrange wrote:
> > On Wed, Apr 21, 2010 at 05:16:21PM -0400, Chris Lalancette wrote:
> >> On 04/21/2010 04:34 PM, Stephen Shaw wrote:
> >>> I'm getting a seg fault when running virsh snapshot-create 1, but
only
> >>> when virt-manager is open and connected.
> >>>
> >>> Here is some of the debug info I was able to come up with -
> >>> http://fpaste.org/9GO6/ (bt)
> >>> http://fpaste.org/7gkH/ ('thread apply all bt)
> >>>
> >>> * After the crash
> >>> (gdb) p mon->msg
> >>> $1 = (qemuMonitorMessagePtr) 0x0
> >>>
> >>>
> >>> nibbler:~ # libvirtd --version
> >>> libvirtd (libvirt) 0.8.0
> >>>
> >>>
> >>> Please let me know if there is any other information you need.
> >>> Stephen
> >>
> >> Thanks for the report. To be perfectly honest, I can't see how what
> >> happened could happen :). But I'll take a closer look at it and see
> >> if I can reproduce and see what is going on with it.
> >
> > I see thread locking problems in the code
> >
> > - qemuDomainSnapshotCreateXML() is calling monitor commands, but has
> > not run qemuDomainObjBeginJobWithDriver() to ensure exclusive
> > access to the monitor
> >
> > - qemuDomainSnapshotDiscard has same problem
>
> Yep, just fixing those now. I didn't quite understand the ObjBeginJob
> before, but I think I'm understanding it now. This is probably the source of
> the problems.
There's some notes about the rules in src/qemu/THREADS.txt.
You must acquire locks on objects in the following order, not missing
any steps.
qemud_driver (qemudDriverLock)
virDomainObjPtr (implicit via virDomainObjFindByXXX)
qemuMonitorPrivatePtr (qemuDomainObjBeginJob)
qemuMonitorPtr (implicit via qemuDomainObjEnterMonitor)
Note that qemuDomainObjEnterMonitor() will release the locks on the
qemud_driver & virDomainObjPtr objects, once it has acquired the lock
on qemuMonitorPtr. The qemuMonitorPrivatePtr object has a condition
variable that ensures continued mutual exclusion, even though the
qemud_driver, virDomainObjPtr object are now unlocked.
So by missing qemuDomainObjBeginJob(), the condition variable was not
acquired, and mutual exclusion was not assured
Regards,
Daniel
Thanks for taking care of this. As soon as the fix is checked in I'll
rebuild libvirt and continue testing it.
Thanks,
Stephen
8:39 a.m.
On 04/22/2010 09:19 AM, Stephen Shaw wrote:
On Thu, Apr 22, 2010 at 07:09, Daniel P. Berrange
<berrange(a)redhat.com> wrote:
> On Thu, Apr 22, 2010 at 08:54:30AM -0400, Chris Lalancette wrote:
>> On 04/22/2010 08:34 AM, Daniel P. Berrange wrote:
>>> On Wed, Apr 21, 2010 at 05:16:21PM -0400, Chris Lalancette wrote:
>>>> On 04/21/2010 04:34 PM, Stephen Shaw wrote:
>>>>> I'm getting a seg fault when running virsh snapshot-create 1, but
only
>>>>> when virt-manager is open and connected.
>>>>>
>>>>> Here is some of the debug info I was able to come up with -
>>>>> http://fpaste.org/9GO6/ (bt)
>>>>> http://fpaste.org/7gkH/ ('thread apply all bt)
>>>>>
>>>>> * After the crash
>>>>> (gdb) p mon->msg
>>>>> $1 = (qemuMonitorMessagePtr) 0x0
>>>>>
>>>>>
>>>>> nibbler:~ # libvirtd --version
>>>>> libvirtd (libvirt) 0.8.0
>>>>>
>>>>>
>>>>> Please let me know if there is any other information you need.
>>>>> Stephen
>>>>
>>>> Thanks for the report. To be perfectly honest, I can't see how what
>>>> happened could happen :). But I'll take a closer look at it and see
>>>> if I can reproduce and see what is going on with it.
>>>
>>> I see thread locking problems in the code
>>>
>>> - qemuDomainSnapshotCreateXML() is calling monitor commands, but has
>>> not run qemuDomainObjBeginJobWithDriver() to ensure exclusive
>>> access to the monitor
>>>
>>> - qemuDomainSnapshotDiscard has same problem
>>
>> Yep, just fixing those now. I didn't quite understand the ObjBeginJob
>> before, but I think I'm understanding it now. This is probably the source
of
>> the problems.
>
> There's some notes about the rules in src/qemu/THREADS.txt.
>
> You must acquire locks on objects in the following order, not missing
> any steps.
>
> qemud_driver (qemudDriverLock)
> virDomainObjPtr (implicit via virDomainObjFindByXXX)
> qemuMonitorPrivatePtr (qemuDomainObjBeginJob)
> qemuMonitorPtr (implicit via qemuDomainObjEnterMonitor)
>
>
> Note that qemuDomainObjEnterMonitor() will release the locks on the
> qemud_driver & virDomainObjPtr objects, once it has acquired the lock
> on qemuMonitorPtr. The qemuMonitorPrivatePtr object has a condition
> variable that ensures continued mutual exclusion, even though the
> qemud_driver, virDomainObjPtr object are now unlocked.
>
> So by missing qemuDomainObjBeginJob(), the condition variable was not
> acquired, and mutual exclusion was not assured
>
> Regards,
> Daniel
Thanks for taking care of this. As soon as the fix is checked in I'll
rebuild libvirt and continue testing it.
I still have to go over the rules more completely, but the attached patch
seems to do more or less the right thing for me. If you could give it a
quick test, that would be great.
Thanks,
--
Chris Lalancette
8:55 a.m.
On Thu, Apr 22, 2010 at 09:39:57AM -0400, Chris Lalancette wrote:
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5f4adfd..4ec4bd6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11034,18 +11040,18 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
rc = qemuMonitorLoadSnapshot(priv->mon, snap->def->name);
qemuDomainObjExitMonitorWithDriver(driver, vm);
if (rc < 0)
- goto cleanup;
+ goto endjob;
}
else {
if (qemuDomainSnapshotSetActive(vm, driver->snapshotDir) < 0)
- goto cleanup;
+ goto endjob;
rc = qemudStartVMDaemon(snapshot->domain->conn, driver, vm, NULL,
-1);
if (qemuDomainSnapshotSetInactive(vm, driver->snapshotDir) < 0)
- goto cleanup;
+ goto endjob;
if (rc < 0)
- goto cleanup;
+ goto endjob;
}
if (snap->def->state == VIR_DOMAIN_PAUSED) {
@@ -11057,7 +11063,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
rc = qemuMonitorStopCPUs(priv->mon);
qemuDomainObjExitMonitorWithDriver(driver, vm);
if (rc < 0)
- goto cleanup;
+ goto endjob;
}
event = virDomainEventNewFromObj(vm,
@@ -11083,17 +11089,18 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
}
if (qemuDomainSnapshotSetActive(vm, driver->snapshotDir) < 0)
- goto cleanup;
+ goto endjob;
}
vm->state = snap->def->state;
ret = 0;
-cleanup:
- if (vm && qemuDomainObjEndJob(vm) == 0)
+endjob:
+ if (qemuDomainObjEndJob(vm) == 0)
vm = NULL;
+cleanup:
if (event)
qemuDomainEventQueue(driver, event);
if (vm)
Getting rid of the check for vm being non-NULL actually makes me
notice another bug. The place a little further up where you call
qemudShutdownVMDaemon() is missing the followup code to clear up
transient guests
if (!vm->persistent) {
if (qemuDomainObjEndJob(vm) > 0)
virDomainRemoveInactive(&driver->domains,
vm);
vm = NULL;
}
With this added, you'd still need the NULL check
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
9:45 a.m.
On Thu, Apr 22, 2010 at 07:55, Daniel P. Berrange <berrange(a)redhat.com> wrote:
On Thu, Apr 22, 2010 at 09:39:57AM -0400, Chris Lalancette wrote:
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 5f4adfd..4ec4bd6 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
<trim>
Ok, I just applied that patch in the spec file and rebuilt. I ran
virsh snapshot-create a couple time with virt-manager running and
connected. Exited cleanly with no apparent problems. I also ran
virsh snapshot-revert with no problems (hadn't tested this before so,
I'm not saying there was a problem before) and impressed. Though it
seems that being able to give a snapshot a name and not some random
numerical string would be very/extremely useful.
Thanks!
Stephen
11:49 a.m.
On 04/22/2010 10:45 AM, Stephen Shaw wrote:
On Thu, Apr 22, 2010 at 07:55, Daniel P. Berrange
<berrange(a)redhat.com> wrote:
> On Thu, Apr 22, 2010 at 09:39:57AM -0400, Chris Lalancette wrote:
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 5f4adfd..4ec4bd6 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
<trim>
Ok, I just applied that patch in the spec file and rebuilt. I ran
virsh snapshot-create a couple time with virt-manager running and
connected. Exited cleanly with no apparent problems. I also ran
virsh snapshot-revert with no problems (hadn't tested this before so,
I'm not saying there was a problem before) and impressed. Though it
seems that being able to give a snapshot a name and not some random
numerical string would be very/extremely useful.
Great, thanks for the testing! I'll clean the patch up with the
recommendations from Dan and post it for 0.8.1.
Note that you can specify a name when taking a snapshot; you just
need to provide the appropriate XML snippet. Something like the
following should work:
# cat<< EOF > snapshot.xml
<domainsnapshot>
<name>myname</name>
<description>this is my description</description>
</domainsnapshot>
EOF
# virsh snapshot-create myguest snapshot.xml
--
Chris Lalancette
11:54 a.m.
On Thu, Apr 22, 2010 at 10:49, Chris Lalancette <clalance(a)redhat.com> wrote:
On 04/22/2010 10:45 AM, Stephen Shaw wrote:
> On Thu, Apr 22, 2010 at 07:55, Daniel P. Berrange <berrange(a)redhat.com> wrote:
>> On Thu, Apr 22, 2010 at 09:39:57AM -0400, Chris Lalancette wrote:
>>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>>> index 5f4adfd..4ec4bd6 100644
>>> --- a/src/qemu/qemu_driver.c
>>> +++ b/src/qemu/qemu_driver.c
>
> <trim>
>
> Ok, I just applied that patch in the spec file and rebuilt. I ran
> virsh snapshot-create a couple time with virt-manager running and
> connected. Exited cleanly with no apparent problems. I also ran
> virsh snapshot-revert with no problems (hadn't tested this before so,
> I'm not saying there was a problem before) and impressed. Though it
> seems that being able to give a snapshot a name and not some random
> numerical string would be very/extremely useful.
Great, thanks for the testing! I'll clean the patch up with the
recommendations from Dan and post it for 0.8.1.
Note that you can specify a name when taking a snapshot; you just
need to provide the appropriate XML snippet. Something like the
following should work:
# cat<< EOF > snapshot.xml
<domainsnapshot>
<name>myname</name>
<description>this is my description</description>
</domainsnapshot>
EOF
# virsh snapshot-create myguest snapshot.xml
--
Chris Lalancette
ah ok, It would be nice if you could just optionally append it to the
end of the command ie. virsh snapshot-create 1 myname
thanks for putting that patch together and getting that fixed.
Something else I ran into as an aside was that if the driver type in
the xml wasn't set it wouldn't create the snapshot. It would be cool
if the code could detect the image type instead of relying solely on
that xml attribute(?).
Cheers,
Stephen
5332
days inactive
5333
days old
13 comments
4 participants
participants (4)
-
Chris Lalancette -
Daniel P. Berrange -
Daniel Veillard -
Stephen Shaw