On Wed, Nov 16, 2011 at 08:45:34AM -0700, 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. After an offline conversation with Eric about why file type is a good proxy for a filesystem that is unsuitable for a snapshot, I'm in agreement with this patch. The argument boils down to 1) block devices are really the only non-regular files that are useful for backing guest block devices 2) the vast majority of block devices on modern systems live in filesystems that are unsuitable for snapshots (e.g., devfs) 3) if a user has a use case that requires mknod'ing a block device in a filesystem capable of storing snapshots, they can override the libvirt check by supplying a filename which libvirt will honor. so, ACK to the design. Dave > >> 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. > > -- > Eric Blake eblake@xxxxxxxxxx +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list