lxc: read-only bind mounts

Factored the bind-mounting into a bindmount() function since
we don't want to leave a writable bind-mount behind if the
read-only remount fails.

The read-only flag is now also removed from the initial
mount flags and is added only for the remount command and is
added separately the non-bind mounts.

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index e701202e1929c073ec447cf1341edd6492c58d5e..a737fc0d3ab4195f4aa2d05053e9eddd0135de43 100644 (file)
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -2256,6 +2256,20 @@ sub run_with_loopdev {
     return $device;
 }
 
+sub bindmount {
+    my ($dir, $dest, $ro, @extra_opts) = @_;
+    PVE::Tools::run_command(['mount', '-o', 'bind', @extra_opts, $dir, $dest]);
+    if ($ro) {
+       eval { PVE::Tools::run_command(['mount', '-o', 'bind,remount,ro', $dest]); };
+       if (my $err = $@) {
+           warn "bindmount error\n";
+           # don't leave writable bind-mounts behind...
+           PVE::Tools::run_command(['umount', $dest]);
+           die $err;
+       }
+    }
+}
+
 # use $rootdir = undef to just return the corresponding mount path
 sub mountpoint_mount {
     my ($mountpoint, $rootdir, $storage_cfg, $snapname) = @_;
@@ -2286,10 +2300,7 @@ sub mountpoint_mount {
     if (defined($mountpoint->{acl})) {
        $optstring .= ($mountpoint->{acl} ? 'acl' : 'noacl');
     }
-    if ($mountpoint->{ro}) {
-       $optstring .= ',' if $optstring;
-       $optstring .= 'ro';
-    }
+    my $readonly = $mountpoint->{ro};
 
     my @extra_opts = ('-o', $optstring);
 
@@ -2314,10 +2325,7 @@ sub mountpoint_mount {
                        die "cannot mount subvol snapshots for storage type '$scfg->{type}'\n";
                    }
                } else {
-                   if ($mountpoint->{ro}) {
-                       die "read-only bind mounts not supported\n";
-                   }
-                   PVE::Tools::run_command(['mount', '-o', 'bind', @extra_opts, $path, $mount_path]);
+                   bindmount($path, $mount_path, $readonly, @extra_opts);
                    warn "cannot enable quota control for bind mounted subvolumes\n" if $quota;
                }
            }
@@ -2334,6 +2342,7 @@ sub mountpoint_mount {
                        if ($quota) {
                            push @extra_opts, '-o', 'usrjquota=aquota.user,grpjquota=aquota.group,jqfmt=vfsv0';
                        }
+                       push @extra_opts, '-o', 'ro' if $readonly;
                        PVE::Tools::run_command(['mount', @extra_opts, $path, $mount_path]);
                    }
                }
@@ -2354,18 +2363,13 @@ sub mountpoint_mount {
            die "unsupported image format '$format'\n";
        }
     } elsif ($type eq 'device') {
+                       push @extra_opts, '-o', 'ro' if $readonly;
        PVE::Tools::run_command(['mount', @extra_opts, $volid, $mount_path]) if $mount_path;
        return wantarray ? ($volid, 0, $volid) : $volid;
     } elsif ($type eq 'bind') {
-       if ($mountpoint->{ro}) {
-           die "read-only bind mounts not supported\n";
-           # Theoretically we'd have to execute both:
-           # mount -o bind $a $b
-           # mount -o bind,remount,ro $a $b
-       }
        die "directory '$volid' does not exist\n" if ! -d $volid;
        &$check_mount_path($volid);
-       PVE::Tools::run_command(['mount', '-o', 'bind', @extra_opts, $volid, $mount_path]) if $mount_path;
+       bindmount($volid, $mount_path, $readonly, @extra_opts) if $mount_path;
        warn "cannot enable quota control for bind mounts\n" if $quota;
        return wantarray ? ($volid, 0, undef) : $volid;
     }