[libvirt] seg fault when running snapshot-create
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
On Wed, Apr 21, 2010 at 14:34, Stephen Shaw <sshaw@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@@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@@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@@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@@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
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
On Wed, Apr 21, 2010 at 15:16, Chris Lalancette <clalance@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
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@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
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 :|
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
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 :|
On Thu, Apr 22, 2010 at 07:09, Daniel P. Berrange <berrange@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
On 04/22/2010 09:19 AM, Stephen Shaw wrote:
On Thu, Apr 22, 2010 at 07:09, Daniel P. Berrange <berrange@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
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 :|
On Thu, Apr 22, 2010 at 07:55, Daniel P. Berrange <berrange@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
On 04/22/2010 10:45 AM, Stephen Shaw wrote:
On Thu, Apr 22, 2010 at 07:55, Daniel P. Berrange <berrange@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
On Thu, Apr 22, 2010 at 10:49, Chris Lalancette <clalance@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@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
participants (4)
-
Chris Lalancette -
Daniel P. Berrange -
Daniel Veillard -
Stephen Shaw