Checking the source of the disk will exclude the case when the
source disk is block device while the snapshot target is given
manually by the users with "snapshot-create --xmlfile" or "virsh
snapshot-create-as --diskspec". Why not check the snapshot
directory where it will be created?
On 2011-11-16 23:45, Eric Blake wrote: On 11/15/2011 08:20 PM, Dave Allan wrote:After working on this some more, I think that identifying problematic file systems, like devtmpfs, is too tricky to be portable. But I think we can meet halfway - right now, the libvirt code blindly generates a filename if one was not provided, regardless of the file type of the backing file it will be wrapping. But maybe it would be sufficient to make the command error out if no qcow2 filename is provided and the backing file is not a regular file. As in this patch:Ok, now I understand the situation; thanks to everyone for the explanations. I'm somewhat uncomfortable using file type as a proxy for "troublesome filesystem" since although they are linked in this case, I'm not sure they're linked in all cases. Maybe I'm wrong about that. If there is no portable way to determine whether a particular filesystem is going to be a problem, I would just clearly document the limitations of letting libvirt choose the filename and the importance of not using that functionality on filesystems that cannot hold a useful snapshot.My patch would not be preventing snapshots of block devices, but merely requiring that the user supply the qcow2 filename that will wrap the block device. The problem with just documenting the issue is that someone will fail to read the documentation, perform a default snapshot that creates a problematic qcow2 file, then be upset that their domain fails to boot and that they lost all the changes made since the snapshot. I'm still in favor of this patch if anyone else thinks it is worthwhile.Subject: [PATCH] snapshot: refuse to generate names for non-regular backing files For whatever reason, the kernel allows you to create a regular file named /dev/sdc.12345; although this file will disappear the next time devtmpfs is remounted. If you let libvirt generate the name of the external snapshot for a disk image originally using the block device /dev/sdc, then the domain will be rendered unbootable once the qcow2 file is lost on the next devtmpfs remount. In this case, the user should have used 'virsh snapshot-create --xmlfile' or 'virsh snapshot-create-as --diskspec' to specify the name for the qcow2 file in a sane location, rather than relying on libvirt generating a name that is most likely to be wrong. We can help avoid naive mistakes by enforcing that the user provide the external name for any backing file that is not a regular file. * src/conf/domain_conf.c (virDomainSnapshotAlignDisks): Only generate names if backing file exists as regular file. Reported by MATSUDA Daiki. |
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list