Re: [libvirt] [PATCHv3 04/43] snapshot: don't leak resources on qemu snapshot failure
On 08/24/2011 10:11 AM, Daniel P. Berrange wrote: On Wed, Aug 24, 2011 at 09:22:21AM -0600, Eric Blake wrote: https://bugzilla.redhat.com/show_bug.cgi?id=727709 mentions that if qemu fails to create the snapshot (such as what happens on Fedora 15 qemu, which has qmp but where savevm is only in hmp, and where libvirt is old enough to not try the hmp fallback), then 'virsh snapshot-list dom' will show a garbage snapshot entry, and the libvirt internal directory for storing snapshot metadata will have a bogus file. This fixes the fallout bug of polluting the snapshot-list with garbage on failure (the root cause of the F15 bug of not having fallback to hmp has already been fixed in newer libvirt releases). * src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Allocate memory before making snapshot, and cleanup on failure. Don't dereference NULL if transient domain exited during snapshot creation. --- src/qemu/qemu_driver.c | 43 --- 1 files changed, 24 insertions(+), 19 deletions(-) ACK I've now pushed this patch. Even though 2 and 3 were ack'ed, I'll wait to push them until I fix patch 1 (since otherwise it causes me some rebase conflicts). Where do I stand on getting reviews for the rest of the series, even as I continue to fix the problems indentified so far in the review? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 04/43] snapshot: don't leak resources on qemu snapshot failure
On Wed, Aug 24, 2011 at 09:22:21AM -0600, Eric Blake wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=727709 > mentions that if qemu fails to create the snapshot (such as what > happens on Fedora 15 qemu, which has qmp but where savevm is only > in hmp, and where libvirt is old enough to not try the hmp fallback), > then 'virsh snapshot-list dom' will show a garbage snapshot entry, > and the libvirt internal directory for storing snapshot metadata > will have a bogus file. > > This fixes the fallout bug of polluting the snapshot-list with > garbage on failure (the root cause of the F15 bug of not having > fallback to hmp has already been fixed in newer libvirt releases). > > * src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Allocate > memory before making snapshot, and cleanup on failure. Don't > dereference NULL if transient domain exited during snapshot creation. > --- > src/qemu/qemu_driver.c | 43 --- > 1 files changed, 24 insertions(+), 19 deletions(-) ACK 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list