]> git.proxmox.com Git - pve-container.git/blobdiff - src/PVE/LXC.pm
Refactor has_feature
[pve-container.git] / src / PVE / LXC.pm
index f964acaf93e67f49866e7d7931c25a1c6581f535..503a57fb872ada093e83b101349afe851950baf3 100644 (file)
@@ -4,17 +4,20 @@ use strict;
 use warnings;
 use POSIX qw(EINTR);
 
+use Socket;
+
 use File::Path;
 use File::Spec;
 use Cwd qw();
-use Fcntl ':flock';
+use Fcntl qw(O_RDONLY);
 
 use PVE::Cluster qw(cfs_register_file cfs_read_file);
+use PVE::Exception qw(raise_perm_exc);
 use PVE::Storage;
 use PVE::SafeSyslog;
 use PVE::INotify;
 use PVE::JSONSchema qw(get_standard_option);
-use PVE::Tools qw($IPV6RE $IPV4RE dir_glob_foreach);
+use PVE::Tools qw($IPV6RE $IPV4RE dir_glob_foreach lock_file lock_file_full);
 use PVE::Network;
 use PVE::AccessControl;
 use PVE::ProcFSTools;
@@ -38,6 +41,7 @@ my $rootfs_desc = {
     volume => {
        type => 'string',
        default_key => 1,
+       format => 'pve-lxc-mp-string',
        format_description => 'volume',
        description => 'Volume, device or directory to mount into the container.',
     },
@@ -54,6 +58,24 @@ my $rootfs_desc = {
        description => 'Volume size (read only value).',
        optional => 1,
     },
+    acl => {
+       type => 'boolean',
+       format_description => 'acl',
+       description => 'Explicitly enable or disable ACL support.',
+       optional => 1,
+    },
+    ro => {
+       type => 'boolean',
+       format_description => 'ro',
+       description => 'Read-only mountpoint (not supported with bind mounts)',
+       optional => 1,
+    },
+    quota => {
+       type => 'boolean',
+       format_description => '[0|1]',
+       description => 'Enable user quotas inside the container (not supported with zfs subvolumes)',
+       optional => 1,
+    },
 };
 
 PVE::JSONSchema::register_standard_option('pve-ct-rootfs', {
@@ -98,8 +120,8 @@ my $confdesc = {
     ostype => {
        optional => 1,
        type => 'string',
-       enum => ['debian', 'ubuntu', 'centos', 'fedora', 'opensuse', 'archlinux'],
-       description => "OS type. Corresponds to lxc setup scripts in /usr/share/lxc/config/<ostype>.common.conf.",
+       enum => ['debian', 'ubuntu', 'centos', 'fedora', 'opensuse', 'archlinux', 'alpine', 'unmanaged'],
+       description => "OS type. This is used to setup configuration inside the container, and corresponds to lxc setup scripts in /usr/share/lxc/config/<ostype>.common.conf. Value 'unmanaged' can be used to skip and OS specific setup.",
     },
     console => {
        optional => 1,
@@ -118,7 +140,7 @@ my $confdesc = {
     cpulimit => {
        optional => 1,
        type => 'number',
-       description => "Limit of CPU usage. Note if the computer has 2 CPUs, it has total of '2' CPU time. Value '0' indicates no CPU limit.",
+       description => "Limit of CPU usage. Note if the computer has 2 CPUs, it has total of '2' CPU time. Value '0' indicates no CPU limit.",
        minimum => 0,
        maximum => 128,
        default => 0,
@@ -126,7 +148,7 @@ my $confdesc = {
     cpuunits => {
        optional => 1,
        type => 'integer',
-       description => "CPU weight for a VM. Argument is used in the kernel fair scheduler. The larger the number is, the more CPU time this VM gets. Number is relative to weights of all the other running VMs.\n\nNOTE: You can disable fair-scheduler configuration by setting this to 0.",
+       description => "CPU weight for a VM. Argument is used in the kernel fair scheduler. The larger the number is, the more CPU time this VM gets. Number is relative to the weights of all the other running VMs.\n\nNOTE: You can disable fair-scheduler configuration by setting this to 0.",
        minimum => 0,
        maximum => 500000,
        default => 1024,
@@ -159,12 +181,12 @@ my $confdesc = {
     searchdomain => {
        optional => 1,
        type => 'string', format => 'dns-name-list',
-       description => "Sets DNS search domains for a container. Create will automatically use the setting from the host if you neither set searchdomain or nameserver.",
+       description => "Sets DNS search domains for a container. Create will automatically use the setting from the host if you neither set searchdomain nor nameserver.",
     },
     nameserver => {
        optional => 1,
        type => 'string', format => 'address-list',
-       description => "Sets DNS server IP address for a container. Create will automatically use the setting from the host if you neither set searchdomain or nameserver.",
+       description => "Sets DNS server IP address for a container. Create will automatically use the setting from the host if you neither set searchdomain nor nameserver.",
     },
     rootfs => get_standard_option('pve-ct-rootfs'),
     parent => {
@@ -189,7 +211,7 @@ my $confdesc = {
     protection => {
        optional => 1,
        type => 'boolean',
-       description => "Sets the protection flag of the container. This will prevent the remove operation. This will prevent the CT or CT's disk remove/update operation.",
+       description => "Sets the protection flag of the container. This will prevent the CT or CT's disk remove/update operation.",
        default => 0,
     },
     unprivileged => {
@@ -231,9 +253,10 @@ my $valid_lxc_conf_keys = {
     'lxc.mount' => 1,
     'lxc.mount.entry' => 1,
     'lxc.mount.auto' => 1,
-    'lxc.rootfs' => 1,
+    'lxc.rootfs' => 'lxc.rootfs is auto generated from rootfs',
     'lxc.rootfs.mount' => 1,
-    'lxc.rootfs.options' => 1,
+    'lxc.rootfs.options' => 'lxc.rootfs.options is not supported' .
+                            ', please use mountpoint options in the "rootfs" key',
     # lxc.cgroup.*
     'lxc.cap.drop' => 1,
     'lxc.cap.keep' => 1,
@@ -257,10 +280,6 @@ my $valid_lxc_conf_keys = {
     'lxc.start.order' => 1,
     'lxc.group' => 1,
     'lxc.environment' => 1,
-    'lxc.' => 1,
-    'lxc.' => 1,
-    'lxc.' => 1,
-    'lxc.' => 1,
 };
 
 my $netconf_desc = {
@@ -336,7 +355,14 @@ my $netconf_desc = {
        format_description => 'VlanNo',
        minimum => '2',
        maximum => '4094',
-       description => "VLAN tag foro this interface.",
+       description => "VLAN tag for this interface.",
+       optional => 1,
+    },
+    trunks => {
+       type => 'string',
+       pattern => qr/\d+(?:;\d+)*/,
+       format_description => 'vlanid[;vlanid...]',
+       description => "VLAN ids to pass through the interface",
        optional => 1,
     },
 };
@@ -351,10 +377,29 @@ for (my $i = 0; $i < $MAX_LXC_NETWORKS; $i++) {
     };
 }
 
+PVE::JSONSchema::register_format('pve-lxc-mp-string', \&verify_lxc_mp_string);
+sub verify_lxc_mp_string{
+    my ($mp, $noerr) = @_;
+
+    # do not allow:
+    # /./ or /../ 
+    # /. or /.. at the end
+    # ../ at the beginning
+    
+    if($mp =~ m@/\.\.?/@ ||
+       $mp =~ m@/\.\.?$@ ||
+       $mp =~ m@^\.\./@){
+       return undef if $noerr;
+       die "$mp contains illegal character sequences\n";
+    }
+    return $mp;
+}
+
 my $mp_desc = {
     %$rootfs_desc,
     mp => {
        type => 'string',
+       format => 'pve-lxc-mp-string',
        format_description => 'Path',
        description => 'Path to the mountpoint as seen from inside the container.',
     },
@@ -501,8 +546,11 @@ sub parse_pct_config {
        if ($line =~ m/^(lxc\.[a-z0-9_\-\.]+)(:|\s*=)\s*(.*?)\s*$/) {
            my $key = $1;
            my $value = $3;
-           if ($valid_lxc_conf_keys->{$key} || $key =~ m/^lxc\.cgroup\./) {
+           my $validity = $valid_lxc_conf_keys->{$key} || 0;
+           if ($validity eq 1 || $key =~ m/^lxc\.cgroup\./) {
                push @{$conf->{lxc}}, [$key, $value];
+           } elsif (my $errmsg = $validity) {
+               warn "vm $vmid - $key: $errmsg\n";
            } else {
                warn "vm $vmid - unable to parse config: $line\n";
            }
@@ -565,7 +613,7 @@ sub load_config {
     my $cfspath = cfs_config_path($vmid, $node);
 
     my $conf = PVE::Cluster::cfs_read_file($cfspath);
-    die "container $vmid does not exists\n" if !defined($conf);
+    die "container $vmid does not exist\n" if !defined($conf);
 
     return $conf;
 }
@@ -594,89 +642,49 @@ sub write_config {
 }
 
 # flock: we use one file handle per process, so lock file
-# can be called multiple times and succeeds for the same process.
+# can be called multiple times and will succeed for the same process.
 
 my $lock_handles =  {};
 my $lockdir = "/run/lock/lxc";
 
-sub lock_filename {
+sub config_file_lock {
     my ($vmid) = @_;
 
     return "$lockdir/pve-config-${vmid}.lock";
 }
 
-sub lock_aquire {
-    my ($vmid, $timeout) = @_;
-
-    $timeout = 10 if !$timeout;
-    my $mode = LOCK_EX;
+sub lock_config_full {
+    my ($vmid, $timeout, $code, @param) = @_;
 
-    my $filename = lock_filename($vmid);
+    my $filename = config_file_lock($vmid);
 
     mkdir $lockdir if !-d $lockdir;
 
-    my $lock_func = sub {
-       if (!$lock_handles->{$$}->{$filename}) {
-           my $fh = new IO::File(">>$filename") ||
-               die "can't open file - $!\n";
-           $lock_handles->{$$}->{$filename} = { fh => $fh, refcount => 0};
-       }
+    my $res = lock_file($filename, $timeout, $code, @param);
 
-       if (!flock($lock_handles->{$$}->{$filename}->{fh}, $mode |LOCK_NB)) {
-           print STDERR "trying to aquire lock...";
-           my $success;
-           while(1) {
-               $success = flock($lock_handles->{$$}->{$filename}->{fh}, $mode);
-               # try again on EINTR (see bug #273)
-               if ($success || ($! != EINTR)) {
-                   last;
-               }
-           }
-           if (!$success) {
-               print STDERR " failed\n";
-               die "can't aquire lock - $!\n";
-           }
+    die $@ if $@;
 
-           print STDERR " OK\n";
-       }
-       
-       $lock_handles->{$$}->{$filename}->{refcount}++;
-    };
-
-    eval { PVE::Tools::run_with_timeout($timeout, $lock_func); };
-    my $err = $@;
-    if ($err) {
-       die "can't lock file '$filename' - $err";
-    }
+    return $res;
 }
 
-sub lock_release {
-    my ($vmid) = @_;
+sub lock_config_mode {
+    my ($vmid, $timeout, $shared, $code, @param) = @_;
 
-    my $filename = lock_filename($vmid);
+    my $filename = config_file_lock($vmid);
 
-    if (my $fh = $lock_handles->{$$}->{$filename}->{fh}) {
-       my $refcount = --$lock_handles->{$$}->{$filename}->{refcount};
-       if ($refcount <= 0) {
-           $lock_handles->{$$}->{$filename} = undef;
-           close ($fh);
-       }
-    }
-}
+    mkdir $lockdir if !-d $lockdir;
 
-sub lock_container {
-    my ($vmid, $timeout, $code, @param) = @_;
+    my $res = lock_file_full($filename, $timeout, $shared, $code, @param);
 
-    my $res;
+    die $@ if $@;
 
-    lock_aquire($vmid, $timeout);
-    eval { $res = &$code(@param) };
-    my $err = $@;
-    lock_release($vmid);
+    return $res;
+}
 
-    die $err if $err;
+sub lock_config {
+    my ($vmid, $code, @param) = @_;
 
-    return $res;
+    return lock_config_full($vmid, 10, $code, @param);
 }
 
 sub option_exists {
@@ -698,18 +706,6 @@ sub json_config_properties {
     return $prop;
 }
 
-sub json_config_properties_no_rootfs {
-    my $prop = shift;
-
-    foreach my $opt (keys %$confdesc) {
-       next if $prop->{$opt};
-       next if $opt eq 'parent' || $opt eq 'snaptime' || $opt eq 'rootfs';
-       $prop->{$opt} = $confdesc->{$opt};
-    }
-
-    return $prop;
-}
-
 # container status helpers
 
 sub list_active_containers {
@@ -1009,7 +1005,7 @@ sub find_lxc_console_pids {
 
        my @args = split(/\0/, $cmdline);
 
-       # serach for lxc-console -n <vmid>
+       # search for lxc-console -n <vmid>
        return if scalar(@args) != 3;
        return if $args[1] ne '-n';
        return if $args[2] !~ m/^\d+$/;
@@ -1087,15 +1083,24 @@ sub update_lxc_config {
     my $custom_idmap = grep { $_->[0] eq 'lxc.id_map' } @{$conf->{lxc}};
 
     my $ostype = $conf->{ostype} || die "missing 'ostype' - internal error";
-    if ($ostype =~ /^(?:debian | ubuntu | centos | fedora | opensuse | archlinux)$/x) {
-       $raw .= "lxc.include = /usr/share/lxc/config/$ostype.common.conf\n";
+    if ($ostype =~ /^(?:debian | ubuntu | centos | fedora | opensuse | archlinux | alpine | unmanaged)$/x) {
+       my $inc ="/usr/share/lxc/config/$ostype.common.conf";
+       $inc ="/usr/share/lxc/config/common.conf" if !-f $inc;
+       $raw .= "lxc.include = $inc\n";
        if ($unprivileged || $custom_idmap) {
-           $raw .= "lxc.include = /usr/share/lxc/config/$ostype.userns.conf\n"
+           $inc = "/usr/share/lxc/config/$ostype.userns.conf";
+           $inc = "/usr/share/lxc/config/userns.conf" if !-f $inc;
+           $raw .= "lxc.include = $inc\n"
        }
     } else {
        die "implement me (ostype $ostype)";
     }
 
+    # WARNING: DO NOT REMOVE this without making sure that loop device nodes
+    # cannot be exposed to the container with r/w access (cgroup perms).
+    # When this is enabled mounts will still remain in the monitor's namespace
+    # after the container unmounted them and thus will not detach from their
+    # files while the container is running!
     $raw .= "lxc.monitor.unshare = 1\n";
 
     # Should we read them from /etc/subuid?
@@ -1112,7 +1117,7 @@ sub update_lxc_config {
     my $ttycount = get_tty_count($conf);
     $raw .= "lxc.tty = $ttycount\n";
 
-    # some init scripts expects a linux terminal (turnkey).
+    # some init scripts expect a linux terminal (turnkey).
     $raw .= "lxc.environment = TERM=linux\n";
     
     my $utsname = $conf->{hostname} || "CT$vmid";
@@ -1193,6 +1198,28 @@ sub verify_searchdomain_list {
     return join(' ', @list);
 }
 
+sub is_volume_in_use {
+    my ($config, $volid, $include_snapshots) = @_;
+    my $used = 0;
+
+    foreach_mountpoint($config, sub {
+       my ($ms, $mountpoint) = @_;
+       return if $used;
+       if ($mountpoint->{type} eq 'volume' && $mountpoint->{volume} eq $volid) {
+           $used = 1;
+       }
+    });
+
+    my $snapshots = $config->{snapshots};
+    if ($include_snapshots && $snapshots) {
+       foreach my $snap (keys %$snapshots) {
+           $used ||= is_volume_in_use($snapshots->{$snap}, $volid);
+       }
+    }
+
+    return $used;
+}
+
 sub add_unused_volume {
     my ($config, $volid) = @_;
 
@@ -1206,7 +1233,7 @@ sub add_unused_volume {
        }
     }
 
-    die "To many unused volume - please delete them first.\n" if !$key;
+    die "Too many unused volumes - please delete them first.\n" if !$key;
 
     $config->{$key} = $volid;
 
@@ -1269,11 +1296,11 @@ sub update_pct_config {
            } elsif ($opt =~ m/^mp(\d+)$/) {
                next if $hotplug_error->($opt);
                check_protection($conf, "can't remove CT $vmid drive '$opt'");
-               my $mountpoint = parse_ct_mountpoint($conf->{$opt});
-               if ($mountpoint->{type} eq 'volume') {
-                   add_unused_volume($conf, $mountpoint->{volume})
-               }
+               my $mp = parse_ct_mountpoint($conf->{$opt});
                delete $conf->{$opt};
+               if ($mp->{type} eq 'volume' && !is_volume_in_use($conf, $mp->{volume})) {
+                   add_unused_volume($conf, $mp->{volume});
+               }
            } elsif ($opt eq 'unprivileged') {
                die "unable to delete read-only option: '$opt'\n";
            } else {
@@ -1289,13 +1316,22 @@ sub update_pct_config {
     my $wanted_swap =  PVE::Tools::extract_param($param, 'swap');
     if (defined($wanted_memory) || defined($wanted_swap)) {
 
-       $wanted_memory //= ($conf->{memory} || 512);
-       $wanted_swap //=  ($conf->{swap} || 0);
+       my $old_memory = ($conf->{memory} || 512);
+       my $old_swap = ($conf->{swap} || 0);
+
+       $wanted_memory //= $old_memory;
+       $wanted_swap //= $old_swap;
 
         my $total = $wanted_memory + $wanted_swap;
        if ($running) {
-           write_cgroup_value("memory", $vmid, "memory.limit_in_bytes", int($wanted_memory*1024*1024));
-           write_cgroup_value("memory", $vmid, "memory.memsw.limit_in_bytes", int($total*1024*1024));
+           my $old_total = $old_memory + $old_swap;
+           if ($total > $old_total) {
+               write_cgroup_value("memory", $vmid, "memory.memsw.limit_in_bytes", int($total*1024*1024));
+               write_cgroup_value("memory", $vmid, "memory.limit_in_bytes", int($wanted_memory*1024*1024));
+           } else {
+               write_cgroup_value("memory", $vmid, "memory.limit_in_bytes", int($wanted_memory*1024*1024));
+               write_cgroup_value("memory", $vmid, "memory.memsw.limit_in_bytes", int($total*1024*1024));
+           }
        }
        $conf->{memory} = $wanted_memory;
        $conf->{swap} = $wanted_swap;
@@ -1303,6 +1339,8 @@ sub update_pct_config {
        write_config($vmid, $conf) if $running;
     }
 
+    my $used_volids = {};
+
     foreach my $opt (keys %$param) {
        my $value = $param->{$opt};
        if ($opt eq 'hostname') {
@@ -1343,22 +1381,59 @@ sub update_pct_config {
         } elsif ($opt =~ m/^mp(\d+)$/) {
            next if $hotplug_error->($opt);
            check_protection($conf, "can't update CT $vmid drive '$opt'");
+           my $old = $conf->{$opt};
            $conf->{$opt} = $value;
+           if (defined($old)) {
+               my $mp = parse_ct_mountpoint($old);
+               if ($mp->{type} eq 'volume' && !is_volume_in_use($conf, $mp->{volume})) {
+                   add_unused_volume($conf, $mp->{volume});
+               }
+           }
            $new_disks = 1;
+           my $mp = parse_ct_mountpoint($value);
+           $used_volids->{$mp->{volume}} = 1;
         } elsif ($opt eq 'rootfs') {
+           next if $hotplug_error->($opt);
            check_protection($conf, "can't update CT $vmid drive '$opt'");
-           die "implement me: $opt";
+           my $old = $conf->{$opt};
+           $conf->{$opt} = $value;
+           if (defined($old)) {
+               my $mp = parse_ct_rootfs($old);
+               if ($mp->{type} eq 'volume' && !is_volume_in_use($conf, $mp->{volume})) {
+                   add_unused_volume($conf, $mp->{volume});
+               }
+           }
+           my $mp = parse_ct_rootfs($value);
+           $used_volids->{$mp->{volume}} = 1;
        } elsif ($opt eq 'unprivileged') {
            die "unable to modify read-only option: '$opt'\n";
+       } elsif ($opt eq 'ostype') {
+           next if $hotplug_error->($opt);
+           $conf->{$opt} = $value;
        } else {
            die "implement me: $opt";
        }
        write_config($vmid, $conf) if $running;
     }
 
+    # Cleanup config:
+
+    # Remove unused disks after re-adding
+    foreach my $key (keys %$conf) {
+       next if $key !~ /^unused\d+/;
+       my $volid = $conf->{$key};
+       if ($used_volids->{$volid}) {
+           delete $conf->{$key};
+       }
+    }
+
+    # Apply deletions and creations of new volumes
     if (@deleted_volumes) {
        my $storage_cfg = PVE::Storage::config();
        foreach my $volume (@deleted_volumes) {
+           next if $used_volids->{$volume}; # could have been re-added, too
+           # also check for references in snapshots
+           next if is_volume_in_use($conf, $volume, 1);
            delete_mountpoint_volume($storage_cfg, $vmid, $volume);
        }
     }
@@ -1531,7 +1606,7 @@ sub update_net {
                    write_config($vmid, $conf);
                }
 
-               PVE::Network::tap_plug($veth, $newnet->{bridge}, $newnet->{tag}, $newnet->{firewall});
+               PVE::Network::tap_plug($veth, $newnet->{bridge}, $newnet->{tag}, $newnet->{firewall}, $newnet->{trunks});
                foreach (qw(bridge tag firewall)) {
                    $oldnet->{$_} = $newnet->{$_} if $newnet->{$_};
                }
@@ -1553,7 +1628,7 @@ sub hotplug_net {
     my $eth = $newnet->{name};
 
     PVE::Network::veth_create($veth, $vethpeer, $newnet->{bridge}, $newnet->{hwaddr});
-    PVE::Network::tap_plug($veth, $newnet->{bridge}, $newnet->{tag}, $newnet->{firewall});
+    PVE::Network::tap_plug($veth, $newnet->{bridge}, $newnet->{tag}, $newnet->{firewall}, $newnet->{trunks});
 
     # attach peer in container
     my $cmd = ['lxc-device', '-n', $vmid, 'add', $vethpeer, "$eth" ];
@@ -1685,8 +1760,8 @@ sub update_ipconfig {
 # Internal snapshots
 
 # NOTE: Snapshot create/delete involves several non-atomic
-# action, and can take a long time.
-# So we try to avoid locking the file and use 'lock' variable
+# actions, and can take a long time.
+# So we try to avoid locking the file and use the 'lock' variable
 # inside the config file instead.
 
 my $snapshot_copy_config = sub {
@@ -1700,13 +1775,37 @@ my $snapshot_copy_config = sub {
        next if $k eq 'lock';
        next if $k eq 'digest';
        next if $k eq 'description';
+       next if $k =~ m/^unused\d+$/;
 
        $dest->{$k} = $source->{$k};
     }
 };
 
-my $snapshot_prepare = sub {
-    my ($vmid, $snapname, $comment) = @_;
+my $snapshot_apply_config = sub {
+    my ($conf, $snap) = @_;
+
+    # copy snapshot list
+    my $newconf = {
+       snapshots => $conf->{snapshots},
+    };
+
+    # keep description and list of unused disks
+    foreach my $k (keys %$conf) {
+       next if !($k =~ m/^unused\d+$/ || $k eq 'description');
+       $newconf->{$k} = $conf->{$k};
+    }
+
+    &$snapshot_copy_config($snap, $newconf);
+
+    return $newconf;
+};
+
+sub snapshot_save_vmstate {
+    die "implement me - snapshot_save_vmstate\n";
+}
+
+sub snapshot_prepare {
+    my ($vmid, $snapname, $save_vmstate, $comment) = @_;
 
     my $snap;
 
@@ -1725,26 +1824,30 @@ my $snapshot_prepare = sub {
            if defined($conf->{snapshots}->{$snapname});
 
        my $storecfg = PVE::Storage::config();
-       die "snapshot feature is not available\n" if !has_feature('snapshot', $conf, $storecfg);
+       die "snapshot feature is not available\n"
+           if !has_feature('snapshot', $conf, $storecfg, undef, undef, $snapname eq 'vzdump');
 
        $snap = $conf->{snapshots}->{$snapname} = {};
 
+       if ($save_vmstate && check_running($vmid)) {
+           snapshot_save_vmstate($vmid, $conf, $snapname, $storecfg);
+       }
+
        &$snapshot_copy_config($conf, $snap);
 
-       $snap->{'snapstate'} = "prepare";
-       $snap->{'snaptime'} = time();
-       $snap->{'description'} = $comment if $comment;
-       $conf->{snapshots}->{$snapname} = $snap;
+       $snap->{snapstate} = "prepare";
+       $snap->{snaptime} = time();
+       $snap->{description} = $comment if $comment;
 
        write_config($vmid, $conf);
     };
 
-    lock_container($vmid, 10, $updatefn);
+    lock_config($vmid, $updatefn);
 
     return $snap;
-};
+}
 
-my $snapshot_commit = sub {
+sub snapshot_commit {
     my ($vmid, $snapname) = @_;
 
     my $updatefn = sub {
@@ -1754,25 +1857,27 @@ my $snapshot_commit = sub {
        die "missing snapshot lock\n"
            if !($conf->{lock} && $conf->{lock} eq 'snapshot');
 
-       die "snapshot '$snapname' does not exist\n"
-           if !defined($conf->{snapshots}->{$snapname});
+       my $snap = $conf->{snapshots}->{$snapname};
+       die "snapshot '$snapname' does not exist\n" if !defined($snap);
 
        die "wrong snapshot state\n"
-           if !($conf->{snapshots}->{$snapname}->{'snapstate'} && 
-                $conf->{snapshots}->{$snapname}->{'snapstate'} eq "prepare");
+           if !($snap->{snapstate} && $snap->{snapstate} eq "prepare");
 
-       delete $conf->{snapshots}->{$snapname}->{'snapstate'};
+       delete $snap->{snapstate};
        delete $conf->{lock};
-       $conf->{parent} = $snapname;
 
-       write_config($vmid, $conf);
+       my $newconf = &$snapshot_apply_config($conf, $snap);
+
+       $newconf->{parent} = $snapname;
+
+       write_config($vmid, $newconf);
     };
 
-    lock_container($vmid, 10 ,$updatefn);
-};
+    lock_config($vmid, $updatefn);
+}
 
 sub has_feature {
-    my ($feature, $conf, $storecfg, $snapname) = @_;
+    my ($feature, $conf, $storecfg, $snapname, $running, $backup_only) = @_;
     
     my $err;
 
@@ -1780,175 +1885,319 @@ sub has_feature {
        my ($ms, $mountpoint) = @_;
 
        return if $err; # skip further test
+       return if $backup_only && $ms ne 'rootfs' && !$mountpoint->{backup};
        
-       $err = 1 if !PVE::Storage::volume_has_feature($storecfg, $feature, $mountpoint->{volume}, $snapname);
-
-       # TODO: implement support for mountpoints
-       die "unable to handle mountpoint '$ms' - feature not implemented\n"
-           if $ms ne 'rootfs';
+       $err = 1 if !PVE::Storage::volume_has_feature($storecfg, $feature, $mountpoint->{volume}, $snapname, $running);
     });
 
     return $err ? 0 : 1;
 }
 
+my $enter_namespace = sub {
+    my ($vmid, $pid, $which, $type) = @_;
+    sysopen my $fd, "/proc/$pid/ns/$which", O_RDONLY
+       or die "failed to open $which namespace of container $vmid: $!\n";
+    PVE::Tools::setns(fileno($fd), $type)
+       or die "failed to enter $which namespace of container $vmid: $!\n";
+    close $fd;
+};
+
+my $do_syncfs = sub {
+    my ($vmid, $pid, $socket) = @_;
+
+    &$enter_namespace($vmid, $pid, 'mnt', PVE::Tools::CLONE_NEWNS);
+
+    # Tell the parent process to start reading our /proc/mounts
+    print {$socket} "go\n";
+    $socket->flush();
+
+    # Receive /proc/self/mounts
+    my $mountdata = do { local $/ = undef; <$socket> };
+    close $socket;
+
+    # Now sync all mountpoints...
+    my $mounts = PVE::ProcFSTools::parse_mounts($mountdata);
+    foreach my $mp (@$mounts) {
+       my ($what, $dir, $fs) = @$mp;
+       next if $fs eq 'fuse.lxcfs';
+       eval { PVE::Tools::sync_mountpoint($dir); };
+       warn $@ if $@;
+    }
+};
+
+sub sync_container_namespace {
+    my ($vmid) = @_;
+    my $pid = find_lxc_pid($vmid);
+
+    # SOCK_DGRAM is nicer for barriers but cannot be slurped
+    socketpair my $pfd, my $cfd, AF_UNIX, SOCK_STREAM, PF_UNSPEC
+       or die "failed to create socketpair: $!\n";
+
+    my $child = fork();
+    die "fork failed: $!\n" if !defined($child);
+
+    if (!$child) {
+       eval {
+           close $pfd;
+           &$do_syncfs($vmid, $pid, $cfd);
+       };
+       if (my $err = $@) {
+           warn $err;
+           POSIX::_exit(1);
+       }
+       POSIX::_exit(0);
+    }
+    close $cfd;
+    my $go = <$pfd>;
+    die "failed to enter container namespace\n" if $go ne "go\n";
+
+    open my $mounts, '<', "/proc/$child/mounts"
+       or die "failed to open container's /proc/mounts: $!\n";
+    my $mountdata = do { local $/ = undef; <$mounts> };
+    close $mounts;
+    print {$pfd} $mountdata;
+    close $pfd;
+
+    while (waitpid($child, 0) != $child) {}
+    die "failed to sync container namespace\n" if $? != 0;
+}
+
+sub check_freeze_needed {
+    my ($vmid, $config, $save_vmstate) = @_;
+
+    my $ret = check_running($vmid);
+    return ($ret, $ret);
+}
+
 sub snapshot_create {
-    my ($vmid, $snapname, $comment) = @_;
+    my ($vmid, $snapname, $save_vmstate, $comment) = @_;
 
-    my $snap = &$snapshot_prepare($vmid, $snapname, $comment);
+    my $snap = snapshot_prepare($vmid, $snapname, $save_vmstate, $comment);
+
+    $save_vmstate = 0 if !$snap->{vmstate};
 
     my $conf = load_config($vmid);
 
-    my $running = check_running($vmid);
-    
-    my $unfreeze = 0;
-    
+    my ($running, $freezefs) = check_freeze_needed($vmid, $conf, $snap->{vmstate});
+
+    my $drivehash = {};
+
     eval {
-       if ($running) {
+       if ($freezefs) {
            PVE::Tools::run_command(['/usr/bin/lxc-freeze', '-n', $vmid]);
-           $unfreeze = 1;
-           PVE::Tools::run_command(['/bin/sync']);
-       };
+           sync_container_namespace($vmid);
+       }
 
        my $storecfg = PVE::Storage::config();
-       my $rootinfo = parse_ct_rootfs($conf->{rootfs});
-       my $volid = $rootinfo->{volume};
+       foreach_mountpoint($conf, sub {
+           my ($ms, $mountpoint) = @_;
 
-       PVE::Storage::volume_snapshot($storecfg, $volid, $snapname);
-       &$snapshot_commit($vmid, $snapname);
+           return if $snapname eq 'vzdump' && $ms ne 'rootfs' && !$mountpoint->{backup};
+           PVE::Storage::volume_snapshot($storecfg, $mountpoint->{volume}, $snapname);
+           $drivehash->{$ms} = 1;
+       });
     };
     my $err = $@;
     
-    if ($unfreeze) {
-       eval { PVE::Tools::run_command(['/usr/bin/lxc-unfreeze', '-n', $vmid]); };
-       warn $@ if $@;
+    if ($running) {
+       if ($freezefs) {
+           eval { PVE::Tools::run_command(['/usr/bin/lxc-unfreeze', '-n', $vmid]); };
+           warn $@ if $@;
+       }
     }
     
     if ($err) {
-       snapshot_delete($vmid, $snapname, 1);
+       warn "snapshot create failed: starting cleanup\n";
+       eval { snapshot_delete($vmid, $snapname, 1, $drivehash); };
+       warn "$@" if $@;
        die "$err\n";
     }
+
+    snapshot_commit($vmid, $snapname);
 }
 
+# Note: $drivehash is only set when called from snapshot_create.
 sub snapshot_delete {
-    my ($vmid, $snapname, $force) = @_;
+    my ($vmid, $snapname, $force, $drivehash) = @_;
+
+    my $prepare = 1;
 
     my $snap;
+    my $unused = [];
 
-    my $conf;
+    my $unlink_parent = sub {
+       my ($confref, $new_parent) = @_;
+
+       if ($confref->{parent} && $confref->{parent} eq $snapname) {
+           if ($new_parent) {
+               $confref->{parent} = $new_parent;
+           } else {
+               delete $confref->{parent};
+           }
+       }
+    };
 
     my $updatefn =  sub {
+       my ($remove_drive) = @_;
 
-       $conf = load_config($vmid);
+       my $conf = load_config($vmid);
 
-       die "you can't delete a snapshot if vm is a template\n"
-           if is_template($conf);
+       if (!$drivehash) {
+           check_lock($conf);
+           die "you can't delete a snapshot if vm is a template\n"
+               if is_template($conf);
+       }
 
        $snap = $conf->{snapshots}->{$snapname};
 
-       check_lock($conf);
-
        die "snapshot '$snapname' does not exist\n" if !defined($snap);
 
-       $snap->{snapstate} = 'delete';
-
-       write_config($vmid, $conf);
-    };
-
-    lock_container($vmid, 10, $updatefn);
-
-    my $storecfg = PVE::Storage::config();
-
-    my $del_snap =  sub {
-
-       check_lock($conf);
+       # remove parent refs
+       if (!$prepare) {
+           &$unlink_parent($conf, $snap->{parent});
+           foreach my $sn (keys %{$conf->{snapshots}}) {
+               next if $sn eq $snapname;
+               &$unlink_parent($conf->{snapshots}->{$sn}, $snap->{parent});
+           }
+       }
 
-       if ($conf->{parent} eq $snapname) {
-           if ($conf->{snapshots}->{$snapname}->{snapname}) {
-               $conf->{parent} = $conf->{snapshots}->{$snapname}->{parent};
+       if ($remove_drive) {
+           if ($remove_drive eq 'vmstate') {
+               die "implement me - saving vmstate\n";
            } else {
-               delete $conf->{parent};
+               my $value = $snap->{$remove_drive};
+               my $mountpoint = $remove_drive eq 'rootfs' ? parse_ct_rootfs($value, 1) : parse_ct_mountpoint($value, 1);
+               delete $snap->{$remove_drive};
+               add_unused_volume($snap, $mountpoint->{volume})
+                   if (!is_volume_in_use($snap, $mountpoint->{volume}));
            }
        }
 
-       delete $conf->{snapshots}->{$snapname};
+       if ($prepare) {
+           $snap->{snapstate} = 'delete';
+       } else {
+           delete $conf->{snapshots}->{$snapname};
+           delete $conf->{lock} if $drivehash;
+           foreach my $volid (@$unused) {
+               add_unused_volume($conf, $volid)
+                   if (!is_volume_in_use($conf, $volid));
+           }
+       }
 
        write_config($vmid, $conf);
     };
 
-    my $rootfs = $conf->{snapshots}->{$snapname}->{rootfs};
-    my $rootinfo = parse_ct_rootfs($rootfs);
-    my $volid = $rootinfo->{volume};
+    lock_config($vmid, $updatefn);
 
-    eval {
-       PVE::Storage::volume_snapshot_delete($storecfg, $volid, $snapname);
+    # now remove vmstate file
+    # never set for LXC!
+    my $storecfg = PVE::Storage::config();
+
+    if ($snap->{vmstate}) {
+       die "implement me - saving vmstate\n";
     };
-    my $err = $@;
 
-    if(!$err || ($err && $force)) {
-       lock_container($vmid, 10, $del_snap);
-       if ($err) {
-           die "Can't delete snapshot: $vmid $snapname $err\n";
+    # now remove all volume snapshots
+    foreach_mountpoint($snap, sub {
+       my ($ms, $mountpoint) = @_;
+
+       return if $snapname eq 'vzdump' && $ms ne 'rootfs' && !$mountpoint->{backup};
+       if (!$drivehash || $drivehash->{$ms}) {
+           eval { PVE::Storage::volume_snapshot_delete($storecfg, $mountpoint->{volume}, $snapname); };
+           if (my $err = $@) {
+               die $err if !$force;
+               warn $err;
+           }
        }
-    }
+
+       # save changes (remove mp from snapshot)
+       lock_config($vmid, $updatefn, $ms) if !$force;
+       push @$unused, $mountpoint->{volume};
+    });
+
+    # now cleanup config
+    $prepare = 0;
+    lock_config($vmid, $updatefn);
 }
 
 sub snapshot_rollback {
     my ($vmid, $snapname) = @_;
 
+    my $prepare = 1;
+
     my $storecfg = PVE::Storage::config();
 
     my $conf = load_config($vmid);
 
-    die "you can't rollback if vm is a template\n" if is_template($conf);
+    my $get_snapshot_config = sub {
 
-    my $snap = $conf->{snapshots}->{$snapname};
+       die "you can't rollback if vm is a template\n" if is_template($conf);
 
-    die "snapshot '$snapname' does not exist\n" if !defined($snap);
+       my $res = $conf->{snapshots}->{$snapname};
 
-    my $rootfs = $snap->{rootfs};
-    my $rootinfo = parse_ct_rootfs($rootfs);
-    my $volid = $rootinfo->{volume};
+       die "snapshot '$snapname' does not exist\n" if !defined($res);
+
+       return $res;
+    };
 
-    PVE::Storage::volume_rollback_is_possible($storecfg, $volid, $snapname);
+    my $snap = &$get_snapshot_config();
+
+    foreach_mountpoint($snap, sub {
+       my ($ms, $mountpoint) = @_;
+
+       PVE::Storage::volume_rollback_is_possible($storecfg, $mountpoint->{volume}, $snapname);
+    });
 
     my $updatefn = sub {
 
-       die "unable to rollback to incomplete snapshot (snapstate = $snap->{snapstate})\n" 
-           if $snap->{snapstate};
+       $conf = load_config($vmid);
 
-       check_lock($conf);
+       $snap = &$get_snapshot_config();
 
-       system("lxc-stop -n $vmid --kill") if check_running($vmid);
+       die "unable to rollback to incomplete snapshot (snapstate = $snap->{snapstate})\n"
+           if $snap->{snapstate};
+
+       if ($prepare) {
+           check_lock($conf);
+           PVE::Tools::run_command(['/usr/bin/lxc-stop', '-n', $vmid, '--kill'])
+               if check_running($vmid);
+       }
 
        die "unable to rollback vm $vmid: vm is running\n"
            if check_running($vmid);
 
-       $conf->{lock} = 'rollback';
+       if ($prepare) {
+           $conf->{lock} = 'rollback';
+       } else {
+           die "got wrong lock\n" if !($conf->{lock} && $conf->{lock} eq 'rollback');
+           delete $conf->{lock};
+       }
 
        my $forcemachine;
 
-       # copy snapshot config to current config
-
-       my $tmp_conf = $conf;
-       &$snapshot_copy_config($tmp_conf->{snapshots}->{$snapname}, $conf);
-       $conf->{snapshots} = $tmp_conf->{snapshots};
-       delete $conf->{snaptime};
-       delete $conf->{snapname};
-       $conf->{parent} = $snapname;
+       if (!$prepare) {
+           # copy snapshot config to current config
+           $conf = &$snapshot_apply_config($conf, $snap);
+           $conf->{parent} = $snapname;
+       }
 
        write_config($vmid, $conf);
-    };
 
-    my $unlockfn = sub {
-       delete $conf->{lock};
-       write_config($vmid, $conf);
+       if (!$prepare && $snap->{vmstate}) {
+           die "implement me - save vmstate\n";
+       }
     };
 
-    lock_container($vmid, 10, $updatefn);
+    lock_config($vmid, $updatefn);
 
-    PVE::Storage::volume_snapshot_rollback($storecfg, $volid, $snapname);
+    foreach_mountpoint($snap, sub {
+       my ($ms, $mountpoint) = @_;
 
-    lock_container($vmid, 5, $unlockfn);
+       PVE::Storage::volume_snapshot_rollback($storecfg, $mountpoint->{volume}, $snapname);
+    });
+
+    $prepare = 0;
+    lock_config($vmid, $updatefn);
 }
 
 sub template_create {
@@ -1989,18 +2238,6 @@ sub mountpoint_names {
     return $reverse ? reverse @names : @names;
 }
 
-# The container might have *different* symlinks than the host. realpath/abs_path
-# use the actual filesystem to resolve links.
-sub sanitize_mountpoint {
-    my ($mp) = @_;
-    $mp = '/' . $mp; # we always start with a slash
-    $mp =~ s@/{2,}@/@g; # collapse sequences of slashes
-    $mp =~ s@/\./@@g; # collapse /./
-    $mp =~ s@/\.(/)?$@$1@; # collapse a trailing /. or /./
-    $mp =~ s@(.*)/[^/]+/\.\./@$1/@g; # collapse /../ without regard for symlinks
-    $mp =~ s@/\.\.(/)?$@$1@; # collapse trailing /.. or /../ disregarding symlinks
-    return $mp;
-}
 
 sub foreach_mountpoint_full {
     my ($conf, $reverse, $func) = @_;
@@ -2011,11 +2248,6 @@ sub foreach_mountpoint_full {
        my $mountpoint = $key eq 'rootfs' ? parse_ct_rootfs($value, 1) : parse_ct_mountpoint($value, 1);
        next if !defined($mountpoint);
 
-       $mountpoint->{mp} = sanitize_mountpoint($mountpoint->{mp});
-
-       my $path = $mountpoint->{volume};
-       $mountpoint->{volume} = sanitize_mountpoint($path) if $path =~ m|^/|;
-
        &$func($key, $mountpoint);
     }
 }
@@ -2033,16 +2265,20 @@ sub foreach_mountpoint_reverse {
 }
 
 sub check_ct_modify_config_perm {
-    my ($rpcenv, $authuser, $vmid, $pool, $key_list) = @_;
+    my ($rpcenv, $authuser, $vmid, $pool, $newconf, $delete) = @_;
 
-    return 1 if $authuser ne 'root@pam';
-
-    foreach my $opt (@$key_list) {
+    return 1 if $authuser eq 'root@pam';
 
+    my $check = sub {
+       my ($opt, $delete) = @_;
        if ($opt eq 'cpus' || $opt eq 'cpuunits' || $opt eq 'cpulimit') {
            $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.CPU']);
        } elsif ($opt eq 'rootfs' || $opt =~ /^mp\d+$/) {
            $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Disk']);
+           return if $delete;
+           my $data = $opt eq 'rootfs' ? parse_ct_rootfs($newconf->{$opt})
+                                       : parse_ct_mountpoint($newconf->{$opt});
+           raise_perm_exc("mountpoint type $data->{type}") if $data->{type} ne 'volume';
        } elsif ($opt eq 'memory' || $opt eq 'swap') {
            $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Memory']);
        } elsif ($opt =~ m/^net\d+$/ || $opt eq 'nameserver' ||
@@ -2051,6 +2287,13 @@ sub check_ct_modify_config_perm {
        } else {
            $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Options']);
        }
+    };
+
+    foreach my $opt (keys %$newconf) {
+       &$check($opt, 0);
+    }
+    foreach my $opt (@$delete) {
+       &$check($opt, 1);
     }
 
     return 1;
@@ -2143,6 +2386,41 @@ sub query_loopdev {
     return $found;
 }
 
+# Run a function with a file attached to a loop device.
+# The loop device is always detached afterwards (or set to autoclear).
+# Returns the loop device.
+sub run_with_loopdev {
+    my ($func, $file) = @_;
+    my $device;
+    my $parser = sub {
+       my $line = shift;
+       if ($line =~ m@^(/dev/loop\d+)$@) {
+           $device = $1;
+       }
+    };
+    PVE::Tools::run_command(['losetup', '--show', '-f', $file], outfunc => $parser);
+    die "failed to setup loop device for $file\n" if !$device;
+    eval { &$func($device); };
+    my $err = $@;
+    PVE::Tools::run_command(['losetup', '-d', $device]);
+    die $err if $err;
+    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) = @_;
@@ -2150,6 +2428,8 @@ sub mountpoint_mount {
     my $volid = $mountpoint->{volume};
     my $mount = $mountpoint->{mp};
     my $type = $mountpoint->{type};
+    my $quota = !$snapname && !$mountpoint->{ro} && $mountpoint->{quota};
+    my $mounted_dev;
     
     return if !$volid || !$mount;
 
@@ -2167,6 +2447,14 @@ sub mountpoint_mount {
 
     die "unknown snapshot path for '$volid'" if !$storage && defined($snapname);
 
+    my $optstring = '';
+    if (defined($mountpoint->{acl})) {
+       $optstring .= ($mountpoint->{acl} ? 'acl' : 'noacl');
+    }
+    my $readonly = $mountpoint->{ro};
+
+    my @extra_opts = ('-o', $optstring);
+
     if ($storage) {
 
        my $scfg = PVE::Storage::storage_config($storage_cfg, $storage);
@@ -2183,48 +2471,58 @@ sub mountpoint_mount {
                    if ($scfg->{type} eq 'zfspool') {
                        my $path_arg = $path;
                        $path_arg =~ s!^/+!!;
-                       PVE::Tools::run_command(['mount', '-o', 'ro', '-t', 'zfs', $path_arg, $mount_path]);
+                       PVE::Tools::run_command(['mount', '-o', 'ro', @extra_opts, '-t', 'zfs', $path_arg, $mount_path]);
                    } else {
                        die "cannot mount subvol snapshots for storage type '$scfg->{type}'\n";
                    }
                } else {
-                   PVE::Tools::run_command(['mount', '-o', 'bind', $path, $mount_path]);
+                   bindmount($path, $mount_path, $readonly, @extra_opts);
+                   warn "cannot enable quota control for bind mounted subvolumes\n" if $quota;
                }
            }
-           return wantarray ? ($path, 0) : $path;
+           return wantarray ? ($path, 0, $mounted_dev) : $path;
        } elsif ($format eq 'raw' || $format eq 'iso') {
+           my $domount = sub {
+               my ($path) = @_;
+               if ($mount_path) {
+                   if ($format eq 'iso') {
+                       PVE::Tools::run_command(['mount', '-o', 'ro', @extra_opts, $path, $mount_path]);
+                   } elsif ($isBase || defined($snapname)) {
+                       PVE::Tools::run_command(['mount', '-o', 'ro,noload', @extra_opts, $path, $mount_path]);
+                   } else {
+                       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]);
+                   }
+               }
+           };
            my $use_loopdev = 0;
-           my @extra_opts;
            if ($scfg->{path}) {
-               push @extra_opts, '-o', 'loop';
+               $mounted_dev = run_with_loopdev($domount, $path);
                $use_loopdev = 1;
            } elsif ($scfg->{type} eq 'drbd' || $scfg->{type} eq 'lvm' ||
                     $scfg->{type} eq 'rbd' || $scfg->{type} eq 'lvmthin') {
-               # do nothing
+               $mounted_dev = $path;
+               &$domount($path);
            } else {
                die "unsupported storage type '$scfg->{type}'\n";
            }
-           if ($mount_path) {
-               if ($format eq 'iso') {
-                   PVE::Tools::run_command(['mount', '-o', 'ro', @extra_opts, $path, $mount_path]);
-               } elsif ($isBase || defined($snapname)) {
-                   PVE::Tools::run_command(['mount', '-o', 'ro,noload', @extra_opts, $path, $mount_path]);
-               } else {
-                   PVE::Tools::run_command(['mount', @extra_opts, $path, $mount_path]);
-               }
-           }
-           return wantarray ? ($path, $use_loopdev) : $path;
+           return wantarray ? ($path, $use_loopdev, $mounted_dev) : $path;
        } else {
            die "unsupported image format '$format'\n";
        }
     } elsif ($type eq 'device') {
-       PVE::Tools::run_command(['mount', $volid, $mount_path]) if $mount_path;
-       return wantarray ? ($volid, 0) : $volid;
+                       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') {
        die "directory '$volid' does not exist\n" if ! -d $volid;
        &$check_mount_path($volid);
-       PVE::Tools::run_command(['mount', '-o', 'bind', $volid, $mount_path]) if $mount_path;
-       return wantarray ? ($volid, 0) : $volid;
+       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;
     }
     
     die "unsupported storage";