Re: [libvirt] [PATCHv2] Don't allow two or more disks to be mapped to the same image file
On Thu, Mar 24, 2011 at 03:45:26PM -0600, Eric Blake wrote:
On 03/24/2011 02:46 AM, Hu Tao wrote:
> If two or more disks are mapped to the same image file, operating
> on these disks at the same time may corrupt data stored in the
> image file.
>
> changes:
>
> v2:
>
> - allow it for read-only disks
> - compare source files by inode number
>
> +
> + if (stat(disk->src, &stat1)) {
> + if (errno != ENOENT) {
> + /* Can't stat file, for safety treate it as conflicted */
s/treate/treat/
Won't this will fail on root-squash NFS from qemu:///system? (Or does
root-squash meant that root can still stat() but just not open() a file?)
I think it won't. man stat says:
No permissions are required on the file itself (to stat it)
But needs 'r' bit to open().
Overall, I'm worried that this patch is repeating some of danpb's bigger
efforts to integrate a sanlock disk contention avoidance [1]. If a
resource manager is properly hooked to all disks, then we can prevent
contention between domains (and not limit ourself to just single-domain
contention, as in this patch). On the other hand, this seems like an
easy enough check to do for a single domain whether or not we get the
sanlock code completed (that is, timing wise this looks like it could be
ready prior to 0.9.0 while Dan's work is bigger in scope and probably
missed the feature freeze for this month's release). So I'm not sure
whether to ack this.
Once danpb's sanlock patch is ready we can easily migrate from the
method of this patch to the sanlock. But what is more important is
to protect users' data. Although the chance is rare that a user
attaches a disk using an image file that is already in use, but if
it happens and important data is lost, it will become a disaster
(especially for a productive environment)
[1] https://www.redhat.com/archives/libvir-list/2011-January/msg00963.html
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org