Re: [Qemu-devel] New option for snapshot_blkdev to avoid image creation
Am 03.10.2011 18:09, schrieb Federico Simoncelli: In some situations might be useful to let qemu use an image that was prepared for a live snapshot. The advantage is that creating the snapshot file outside of the qemu process we can use the whole range of options provided by the format (eg for qcow2: encryption, cluster_size and preallocation). In fact, I don't think encryption works, nor does preallocation work with a backing file. I think you would have to check that the new image file isn't encrypted, doesn't have any clusters allocated, etc. I'm not sure that it makes sense to put the logic into an external program (i.e. make it not available for users not using management tools) when at the same you have to implement the same amount of checks in qemu that verify that the management tool actually did what it is supposed to. It is also possible to pre-set a relative path to the backing file (now it is created by default as absolute path). Your code performs a literal string comparison with the old file name used to open the image. So if it was relative on the qemu command line, the backing file link must be relative as well, and if it was absolute on the command line, it must be absolute. In the long run it can also avoid the danger of reimplementing qemu-img inside qemu (if we wanted to expose such options when a snapshot is requested). This is not reimplementing, it is reusing. bdrv_image_create() can do everything that qemu-img create can do. qemu-img has only a thin wrapper around it. The only thing you need to do is exposing the existing options in the monitor. Kevin
Re: [Qemu-devel] New option for snapshot_blkdev to avoid image creation
On Mon, Oct 03, 2011 at 04:09:01PM +, Federico Simoncelli wrote: In some situations might be useful to let qemu use an image that was prepared for a live snapshot. The advantage is that creating the snapshot file outside of the qemu process we can use the whole range of options provided by the format (eg for qcow2: encryption, cluster_size and preallocation). It is also possible to pre-set a relative path to the backing file (now it is created by default as absolute path). In the long run it can also avoid the danger of reimplementing qemu-img inside qemu (if we wanted to expose such options when a snapshot is requested). When the image file is created based on the backing file size: $ qemu-img create -f qcow2 -o backing_file=master.img vm001.qcow2 It turns out that bdrv_img_create() opens the backing file with read/write permissions. This is generally a bad idea but especially dangerous when the VM currently has the image file open already since image formats are not designed for multiple initiators (clustering). We wouldn't want any caches being written out or startup fsck-style operations to be performed on the backing file while the VM has it open. Please make sure to use read-only before applying this patch. Stefan
Re: [Qemu-devel] New option for snapshot_blkdev to avoid image creation
- Original Message - From: Stefan Hajnoczi stefa...@gmail.com To: Federico Simoncelli fsimo...@redhat.com Cc: qemu-devel@nongnu.org, aba...@redhat.com, dl...@redhat.com Sent: Tuesday, October 4, 2011 9:33:48 AM Subject: Re: [Qemu-devel] New option for snapshot_blkdev to avoid image creation On Mon, Oct 03, 2011 at 04:09:01PM +, Federico Simoncelli wrote: In some situations might be useful to let qemu use an image that was prepared for a live snapshot. The advantage is that creating the snapshot file outside of the qemu process we can use the whole range of options provided by the format (eg for qcow2: encryption, cluster_size and preallocation). It is also possible to pre-set a relative path to the backing file (now it is created by default as absolute path). In the long run it can also avoid the danger of reimplementing qemu-img inside qemu (if we wanted to expose such options when a snapshot is requested). When the image file is created based on the backing file size: $ qemu-img create -f qcow2 -o backing_file=master.img vm001.qcow2 It turns out that bdrv_img_create() opens the backing file with read/write permissions. This is generally a bad idea but especially dangerous when the VM currently has the image file open already since image formats are not designed for multiple initiators (clustering). Hi Stefan, are you sure that bdrv_img_create opens the backing file with read/write permissions? After a quick check I can't see that happening: $ ./qemu-img create -f qcow2 /tmp/master.img 1G Formatting '/tmp/master.img', fmt=qcow2 size=1073741824 encryption=off cluster_size=65536 $ strace -e trace=open ./qemu-img create -f qcow2 -o backing_file=/tmp/master.img /tmp/vm001.qcow2 [...] open(/tmp/vm001.qcow2, O_RDONLY|O_NONBLOCK) = -1 ENOENT (No such file or directory) open(/tmp/vm001.qcow2, O_RDONLY|O_NONBLOCK) = -1 ENOENT (No such file or directory) open(/tmp/master.img, O_RDONLY|O_NONBLOCK) = 3 open(/tmp/master.img, O_RDONLY|O_NONBLOCK) = 3 open(/tmp/master.img, O_RDONLY|O_DSYNC|O_CLOEXEC) = 3 open(/tmp/master.img, O_RDONLY|O_NONBLOCK) = 3 open(/tmp/master.img, O_RDONLY|O_NONBLOCK) = 3 open(/tmp/master.img, O_RDONLY|O_CLOEXEC) = 3 Formatting '/tmp/vm001.qcow2', fmt=qcow2 size=1073741824 backing_file='/tmp/master.img' encryption=off cluster_size=65536 open(/tmp/vm001.qcow2, O_RDONLY|O_NONBLOCK) = -1 ENOENT (No such file or directory) open(/tmp/vm001.qcow2, O_RDONLY|O_NONBLOCK) = -1 ENOENT (No such file or directory) open(/tmp/vm001.qcow2, O_WRONLY|O_CREAT|O_TRUNC, 0644) = 6 open(/tmp/vm001.qcow2, O_RDONLY|O_NONBLOCK) = 6 open(/tmp/vm001.qcow2, O_RDONLY|O_NONBLOCK) = 6 open(/tmp/vm001.qcow2, O_RDWR|O_DSYNC|O_CLOEXEC) = 6 open(/tmp/vm001.qcow2, O_RDONLY|O_NONBLOCK) = 6 open(/tmp/vm001.qcow2, O_RDONLY|O_NONBLOCK) = 6 open(/tmp/vm001.qcow2, O_RDWR|O_CLOEXEC) = 6 -- Federico
Re: [Qemu-devel] New option for snapshot_blkdev to avoid image creation
On Tue, Oct 04, 2011 at 04:27:42AM -0400, Federico Simoncelli wrote: - Original Message - From: Stefan Hajnoczi stefa...@gmail.com To: Federico Simoncelli fsimo...@redhat.com Cc: qemu-devel@nongnu.org, aba...@redhat.com, dl...@redhat.com Sent: Tuesday, October 4, 2011 9:33:48 AM Subject: Re: [Qemu-devel] New option for snapshot_blkdev to avoid image creation On Mon, Oct 03, 2011 at 04:09:01PM +, Federico Simoncelli wrote: In some situations might be useful to let qemu use an image that was prepared for a live snapshot. The advantage is that creating the snapshot file outside of the qemu process we can use the whole range of options provided by the format (eg for qcow2: encryption, cluster_size and preallocation). It is also possible to pre-set a relative path to the backing file (now it is created by default as absolute path). In the long run it can also avoid the danger of reimplementing qemu-img inside qemu (if we wanted to expose such options when a snapshot is requested). When the image file is created based on the backing file size: $ qemu-img create -f qcow2 -o backing_file=master.img vm001.qcow2 It turns out that bdrv_img_create() opens the backing file with read/write permissions. This is generally a bad idea but especially dangerous when the VM currently has the image file open already since image formats are not designed for multiple initiators (clustering). Hi Stefan, are you sure that bdrv_img_create opens the backing file with read/write permissions? You are absolutely right. BDRV_O_FLAGS does not have BDRV_O_RDWR so it is opening read-only. I missed this when reading the code earlier today. Stefan
[Qemu-devel] New option for snapshot_blkdev to avoid image creation
In some situations might be useful to let qemu use an image that was prepared for a live snapshot. The advantage is that creating the snapshot file outside of the qemu process we can use the whole range of options provided by the format (eg for qcow2: encryption, cluster_size and preallocation). It is also possible to pre-set a relative path to the backing file (now it is created by default as absolute path). In the long run it can also avoid the danger of reimplementing qemu-img inside qemu (if we wanted to expose such options when a snapshot is requested). -- Federico.
