Re: [Qemu-devel] [PATCH] HMP: snapshot_blkdev can not consider //root/sn1 and /root/sn1 as the same file

[Stefan Hajnoczi <stefanha@gmail.com>]

On Tue, Dec 10, 2013 at 09:58:44PM +0800, lijun wrote:
> On 12/09/2013 08:17 PM, Stefan Hajnoczi wrote:
> >On Mon, Dec 09, 2013 at 02:06:21PM +0800, jun muzi wrote:
> >My question was about the bug.  What is the problem you are seeing and
> >how do you reproduce it (including the HMP commands)?
> Now I am seeing is how to "distinguish network storage file mounted
> on different dir is the same file or not".
> Reproduce this bug using nfs mounted in different directories:
> Create a nfs server, and mount it as followings:
> # mount $IP:/home/ /mnt/dir1
> # mount $IP:/home/ /mnt/dir2
> (qemu) snapshot_blkdev drive-scsi0-0-0  /mnt/dir1/sn1
> Formatting '/mnt/dir1/sn1', fmt=qcow2 size=32212254720
> backing_file='/home/juli/RHEL-7.0-20131127.1.qcow2_v3'
> backing_fmt='qcow2' encryption=off cluster_size=65536
> lazy_refcounts=off
> (qemu) snapshot_blkdev drive-scsi0-0-0  /mnt/dir2/sn1
> Formatting '/mnt/dir2/sn1', fmt=qcow2 size=32212254720
> backing_file='/mnt/dir1/sn1' backing_fmt='qcow2' encryption=off
> cluster_size=65536 lazy_refcounts=off
> (qemu) info block
> drive-scsi0-0-0: removable=0 io-status=ok file=/mnt/dir2/sn1
> backing_file=/mnt/dir1/sn1 backing_file_depth=2 ro=0 drv=qcow2
> encrypted=0 bps=0 bps_rd=0 bps_wr=0 iops=0 iops_rd=0 iops_wr=0
> drive-virtio-disk1: removable=0 io-status=ok
> file=/home/juli/data_disk.qcow2_v3 ro=0 drv=qcow2 encrypted=0 bps=0
> bps_rd=0 bps_wr=0 iops=0 iops_rd=0 iops_wr=0
> ide1-cd0: removable=1 locked=0 tray-open=0 io-status=ok [not inserted]
> floppy0: removable=1 locked=0 tray-open=0 [not inserted]
> sd0: removable=1 locked=0 tray-open=0 [not inserted]
> (qemu) snapshot_blkdev drive-scsi0-0-0  /mnt/dir2/sn2
> Could not open '/mnt/dir2/sn1': Could not set AIO state: Too many
> open files: Too many open files

Okay, I think we got side-tracked worrying about identifying identical
files.  The problem in your example is more fundamental.

Here is what should have happened:

  (qemu) snapshot_blkdev drive-scsi0-0-0  /mnt/dir2/sn1
  Could not create '/mnt/dir2/sn1': File exists

The file was previously created with the name /mnt/dir1/sn1.  Running
snapshot_blkdev with /mnt/dir2/sn1 clobbers the file in your example.
This is bad because it corrupts the backing chain.

If QEMU uses O_CREAT | O_EXCL open(2) flags then creating the snapshot
file fails and the user will not mistakingly overwrite sn1.

However, QEMU does not use O_EXCL to create image files anywhere today
(qemu-img or QEMU monitor commands).  Returning an error is not
backwards-compatible since existing users might rely on clobbering files
(there are safe cases where it can be useful).

We could add an option to commands that create files but I'm not sure if
it's worth the effort since human users who are most at risk probably
won't provide this new option...

That said, if you want to add an O_EXCL option to bdrv_img_create(),
qemu-img commands, and monitor commands, I'm happy to review patches.

Stefan