On 11/13/2011 09:08 PM, MATSUDA, Daiki wrote:
> NACK. There is nothing inherently wrong with the source file not
being
> a qcow2 file. The whole point of creating a runtime snapshot is that
> the original file (of _any_ format) becomes the backing file of a new
> qcow2 file, so that the original file now serves as the snapshot.
> Forbidding a live snapshot of a raw source file interferes with this intent.
>
I have some tested since you replied. Certainly other image file, e.g.
raw type, has no problem after snapshot is taken in qcow2 format.
But in the case that direct disk block, e.g. /dev/sdc, is given for the
guest OS, the same error occurs. It may not be accepted by qemu-kvm.
I'm not following you. As I already said, there's nothing wrong with a
direct disk block as the backing file of a qcow2 image.
I do not understand which qemu-kvm or libvirt should be fixed. But at
least libvirt should be stop to take snapshot for block device with
following patch.
I still say NACK. Your patch is merely attempting to forbid a useful
case, rather than fix a demonstrated problem.
If I recall correctly, you started this thread when you used 'virsh
snapshot-create --disk-only' without arguments, and thus fell victim to
virsh picking the snapshot name for you, but trying to pick that name
under /dev instead of a more typical location for a non-device file.
But that's a usage error in you not taking time to provide virsh with
the alternate name to use for the qcow2 file, and not a flaw that needs
correcting in libvirt itself. Or even if there is a flaw in libvirt,
the fix is not to forbid snapshots of a raw block device, but to be
smarter about ensuring that any generated qcow2 file name is likely to
be correct (a qcow2 file created under /dev probably only makes sense if
the qcow2 data is being written atop a pre-existing block device, rather
than trying to use open(O_CREAT) to create a regular file in /dev).
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org