Re: [libvirt] 'build' on FS pool now unconditionally formats?

On 03/16/2010 06:22 AM, Daniel P. Berrange wrote: >On Mon, Mar 15, 2010 at 05:21:26PM -0400, Dave Allan wrote: >> >>Now that 0.7.7 is done, we need to revisit fs pool building. >> >>Because it is impossible to implement a check for arbitrary existing >>data, the only safe option is to require the overwrite flag in all >>cases. If we do not require the flag in all cases, virt-manager and >>virsh will format unknown partitions without providing any indication to >>the user that a destructive operation is about to take place. The only >>input from the user will be the selection of the partition. All other >>instances of destructive behavior require explicit confirmation from the >>user, or as Cole aptly put it, are loudly warned about by virt-manager. >> I wish that a safe alternative existed, but none does. > >There are two alternatives that are safe Unfortunately, no. There is no programatic way to detect the presence of arbitrary data on a partition. Any attempt to do so is false security. We can, as you point out, determine in some cases that the user *is* going to overwrite something, but we cannot determine in all cases that the user *is not* going to overwrite something. > 1. Do a per filesystem magic check on the volume in question. libvirt has > a list of filesystems that I knows about "ext2", "ext3", "ext4", > "ufs", "iso9660", "udf", "gfs", "gfs2", "vfat", "hfs+", "xfs", "ocfs2" > All of these will have an easily identified magic header that could > be positively checked for. > >Or > > 2. Do a check for the first 512 or 4096 bytes being all zeros. This > should > detect the absence of any filesystem AFAIK. And that's the problem. We can detect filesystems *that we know of*. We do not and cannot know the universe of partition formats that exist. Again, if we pretend we do all we're going to do is give users a false sense of security when the only real security is asking the user "You're about to destroy data, are you certain you have the right partition?"

>The semantics we should have for these APIs are > > - virStoragePoolBuild(flags=0) - format the > filesystem/volgroup/partition table > only if not already formatted. > - virStoragePoolBuild(flags=OVERWRITE) - unconditionally format > - virStoragePoolDelete() - wipe data such that Build(flags=0) is > guaranteed > to be successful next time. > >So I see several things that need to be implemented > > - Make disk& logical pool backends check for existing formatted data > - Implement the 'Delete' operation for all pool types, > - Add (checked) formatting of filesystem pools > - Add OVERWRITE flag to disk, logical, filesystem pool types to skip > the check > >>The attached patch implements this behavior. > >NACK in this form. We clearly disagree, so I think we need some additional voices to weigh in here.