Bug 676225

Summary:	should prevent creating snapshot image with the same file name as file in snapshot chain
Product:	Red Hat Enterprise Linux 6	Reporter:	Shirley Zhou <szhou>
Component:	qemu-kvm	Assignee:	Jes Sorensen <Jes.Sorensen>
Status:	CLOSED WONTFIX	QA Contact:	Virtualization Bugs <virt-bugs>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	6.1	CC:	kwolf, mkenneth, mshao, virt-maint
Target Milestone:	rc
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2011-02-14 14:25:55 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Bug Depends On:
Bug Blocks:	580946

Description Shirley Zhou 2011-02-09 05:54:20 UTC

Description of problem:
when creating snapshot image with same file name as file in snapshot chain, core dumped.

Version-Release number of selected component (if applicable):
qemu-kvm-0.12.1.2-2.144.el6.x86_64

How reproducible:
100%

Steps to Reproduce:
1.start a guest and create snapshot image with name sn1.img
CLI: -drive file=/root/snapshot/RHEl6-1.img,if=none,id=drive-ide0-0-0,format=qcow2,cache=none 
-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 

(qemu)  snapshot_blkdev drive-ide0-0-0 /root/snapshot/sn1.img
Formatting '/root/snapshot/sn1.img', fmt=qcow2 size=32212254720 backing_file='/root/snapshot/RHEl6-1.img' backing_fmt='qcow2' encryption=off cluster_size=0 

2. create snapshot image with name sn2.img
(qemu)  snapshot_blkdev drive-ide0-0-0 /root/snapshot/sn2.img
Formatting '/root/snapshot/sn2.img', fmt=qcow2 size=32212254720 backing_file='/root/snapshot/sn1.img' backing_fmt='qcow2' encryption=off cluster_size=0 

3.create snapshot image with name sn1.img (same as step1)
(qemu)  snapshot_blkdev drive-ide0-0-0 /root/snapshot/sn1.img
Formatting '/root/snapshot/sn1.img', fmt=qcow2 size=32212254720 backing_file='/root/snapshot/sn2.img' backing_fmt='qcow2' encryption=off cluster_size=0 
Aborted (core dumped)

(gdb) bt
#0  0x00007f5418fff9a5 in raise () from /lib64/libc.so.6
#1  0x00007f5419001185 in abort () from /lib64/libc.so.6
#2  0x00000000004139eb in do_snapshot_blkdev (mon=<value optimized out>, qdict=<value optimized out>, ret_data=<value optimized out>) at /usr/src/debug/qemu-kvm-0.12.1.2/monitor.c:1108
#3  0x0000000000418e39 in handle_user_command (mon=0x1ada230, cmdline=<value optimized out>) at /usr/src/debug/qemu-kvm-0.12.1.2/monitor.c:4377
#4  0x0000000000418e8a in monitor_command_cb (mon=0x1ada230, cmdline=<value optimized out>, opaque=<value optimized out>) at /usr/src/debug/qemu-kvm-0.12.1.2/monitor.c:4930
#5  0x00000000004a580b in readline_handle_byte (rs=0x2004600, ch=<value optimized out>) at readline.c:369
#6  0x00000000004190ac in monitor_read (opaque=<value optimized out>, buf=0x7fffe821f7d0 "\r", size=1) at /usr/src/debug/qemu-kvm-0.12.1.2/monitor.c:4916
#7  0x00000000004be1fb in qemu_chr_read (opaque=0x18e23e0) at qemu-char.c:171
#8  fd_chr_read (opaque=0x18e23e0) at qemu-char.c:657
#9  0x000000000040b95f in main_loop_wait (timeout=1000) at /usr/src/debug/qemu-kvm-0.12.1.2/vl.c:4424
#10 0x000000000042b29a in kvm_main_loop () at /usr/src/debug/qemu-kvm-0.12.1.2/qemu-kvm.c:2165
#11 0x000000000040ef0f in main_loop (argc=<value optimized out>, argv=<value optimized out>, envp=<value optimized out>) at /usr/src/debug/qemu-kvm-0.12.1.2/vl.c:4634
#12 main (argc=<value optimized out>, argv=<value optimized out>, envp=<value optimized out>) at /usr/src/debug/qemu-kvm-0.12.1.2/vl.c:6848

Actual results:
After step3, core dumped.

Expected results:
It should prevent create snapshot image with the same file name as files in snapshot chain.

Additional info:

Comment 2 Amit Shah 2011-02-09 06:17:19 UTC

This was introduced by commit d1671013d61fd9204f063a643cb07935c41622bb: "Introduce do_snapshot_blkdev() and monitor command to handle it."

Comment 3 Kevin Wolf 2011-02-09 09:05:46 UTC

You probably also shouldn't try to overwrite the image of a different disk in the VM, or the disk image of another VM, or your host kernel. The question is, how far can and should we go with sanity checking? We'll never be able to completely protect the user from doing stupid things.

We could probably avoid overwriting any existing file by default, but then I guess we'd need a force flag.

Comment 4 Jes Sorensen 2011-02-14 14:25:55 UTC

I added a basic test in the code to make sure we didn't try to overwrite
the base image, however this test was effectively free.

As Kevin points out too, there's an endless chain of options we would
have to consider. A filename could be a link to a filename we already
use etc. We would have to walk the full chain of backing files and stat()
each one of them to compare the output. It will add expensive overhead
without adding any real gain.

The real fix here is to make sure you don't try to specify an existing
image file or device.

Closing as WONTFIX