[pve-devel] [RFC storage 2/2] Fix #1012: dir storage: add is_mountpoint option

[Wolfgang Bumiller]

While the mkdir option deals with the case where we don't
want to clobber a mount point with directories (like ZFS,
gluster or NFS), putting a directory storage directly onto a
mount point is still risky:
If the path exists - which it usually does even if not
mounted - the storage will be considered successfully
activated, but empty (or with unexpected content). Some
operations will then lead to unexpected problems: the
free_disk operation for instance only warns if the disk does
not exist, but does not throw an error. In this case the
configuration might be updated without the real disk being
deleted. Once it's mounted back in, later operations which
check existing disks which are not part of the current VM
configuration (like migration) might error unexpectedly.

This adds an 'is_mountpoint' option to directory storages
which assumes the directory is an externally managed mount
point (eg. fstab or zfs) and changes activate_storage() to
throw an error if the path is not mounted.
---
 PVE/Storage/DirPlugin.pm | 43 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/PVE/Storage/DirPlugin.pm b/PVE/Storage/DirPlugin.pm
index ab9e4bd..39f3b30 100644
--- a/PVE/Storage/DirPlugin.pm
+++ b/PVE/Storage/DirPlugin.pm
@@ -2,6 +2,7 @@ package PVE::Storage::DirPlugin;
 
 use strict;
 use warnings;
+use Cwd;
 use File::Path;
 use PVE::Storage::Plugin;
 use PVE::JSONSchema qw(get_standard_option);
@@ -33,6 +34,13 @@ sub properties {
 	    type => 'boolean',
 	    default => 'yes',
 	},
+	is_mountpoint => {
+	    description =>
+		"Assume the directory is an externally managed mountpoint. " .
+		"If nothing is mounted the storage will be considered offline.",
+	    type => 'boolean',
+	    default => 'no',
+	},
     };
 }
 
@@ -46,19 +54,52 @@ sub options {
 	content => { optional => 1 },
 	format => { optional => 1 },
 	mkdir => { optional => 1 },
+	is_mountpoint => { optional => 1 },
    };
 }
 
 # Storage implementation
+#
+
+# NOTE: should ProcFSTools::is_mounted accept an optional cache like this?
+sub path_is_mounted {
+    my ($mountpoint, $mountdata) = @_;
+
+    $mountpoint = Cwd::realpath($mountpoint); # symlinks
+    return 0 if !defined($mountpoint); # path does not exist
+
+    $mountdata = PVE::ProcFSTools::parse_proc_mounts() if !$mountdata;
+    return 1 if grep { $_->[1] eq $mountpoint } @$mountdata;
+    return undef;
+}
+
+sub status {
+    my ($class, $storeid, $scfg, $cache) = @_;
+
+    $cache->{mountdata} = PVE::ProcFSTools::parse_proc_mounts()
+	if !$cache->{mountdata};
+
+    my $path = $scfg->{path};
+
+    return undef if !path_is_mounted($path, $cache->{mountdata});
+
+    return $class->SUPER::status($storeid, $scfg, $cache);
+}
+
 
 sub activate_storage {
     my ($class, $storeid, $scfg, $cache) = @_;
 
+    my $path = $scfg->{path};
     if (!defined($scfg->{mkdir}) || $scfg->{mkdir}) {
-	my $path = $scfg->{path};
 	mkpath $path;
     }
 
+    if ($scfg->{is_mountpoint} && !path_is_mounted($path, $cache->{mountdata})) {
+	die "unable to activate storage '$storeid' - " .
+	    "directory is expected to be a mount point but is not mounted: '$path'\n";
+    }
+
     $class->SUPER::activate_storage($storeid, $scfg, $cache);    
 }
 
-- 
2.1.4