lxd/device: Disable virtiofsd idmap for shifted volumes

lxd/device/disk.go

@@ -1180,6 +1180,16 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) {
 					mount.FSType = "iso9660"
 				}
 
+				if shared.IsTrue(dbVolume.Config["security.shifted"]) {
+					// To be consistent with containers, we use the OwnerShift
+					// flag here even though it means something different for
+					// VMs. Containers use ID-mapped mounts because it makes
+					// UIDs and GIDs look the same on the host and in the
+					// container. For VMs, the same effect is achieved by just
+					// not running virtiofsd in a user namespace.
+					mount.OwnerShift = deviceConfig.MountOwnerShiftDynamic
+				}
+
 				revertFunc, mountedPath, _, err := d.mountPoolVolume()
 				if err != nil {
 					return nil, diskSourceNotFoundError{msg: "Failed mounting volume", err: err}
@@ -1257,9 +1267,20 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) {
 				mount.TargetPath = d.config["path"]
 				mount.FSType = "virtiofs"
 
-				rawIDMaps, err := idmap.ParseRawIdmap(d.inst.ExpandedConfig()["raw.idmap"])
-				if err != nil {
-					return nil, fmt.Errorf(`Failed parsing instance "raw.idmap": %w`, err)
+				// When security.shifted=true, the volume's files are owned by real users on the
+				// host (e.g. UID 0 not 1000000). For containers, the mount needs to be shifted to
+				// counteract the effect of entering a user namespace. But VMs don't use user
+				// namespaces, so we actually don't want to shift the virtiofsd process.
+				//
+				// Also, we should ignore raw.idmap for consistency with containers. If I create a
+				// file as user 1000 inside the container, the file on disk is owned by UID 1000.
+				// We don't care that container user is actually 1001000 in the root namespace.
+				var rawIDMaps []idmap.IdmapEntry
+				if mount.OwnerShift != deviceConfig.MountOwnerShiftDynamic {
+					rawIDMaps, err = idmap.ParseRawIdmap(d.inst.ExpandedConfig()["raw.idmap"])
+					if err != nil {
+						return nil, fmt.Errorf(`Failed parsing instance "raw.idmap": %w`, err)
+					}
 				}
 
 				// If we are using restricted parent source path mode, or if a non-empty set of

test/suites/vm.sh

@@ -179,6 +179,25 @@ test_vm_pcie_bus() {
   lxc config device remove v1 v1block
   lxc storage volume delete "${pool}" v1block
 
+  sub_test "Check security.shifted volumes are not remapped by virtiofsd in VMs"
+  # VMs do not use user namespaces, so files on a security.shifted volume keep their real
+  # on-disk ownership inside the VM (matching containers). virtiofsd must ignore raw.idmap for
+  # such volumes, otherwise the file below would appear owned by nobody instead of 123:456.
+  lxc config set v1 raw.idmap="both 1000000 0"
+  lxc storage volume create "${pool}" v1shift --type=filesystem size=1MiB security.shifted=true
+  lxc config device add v1 v1shift disk source=v1shift pool="${pool}" path=/mnt
+  lxc start v1
+  waitInstanceReady v1
+  lxc exec v1 -- findmnt /mnt -t virtiofs
+  volPath="${LXD_DIR}/storage-pools/${pool}/custom/default_v1shift"
+  touch "${volPath}/shifted-file"
+  chown 123:456 "${volPath}/shifted-file"
+  [ "$(lxc exec v1 -- stat /mnt/shifted-file -c '%u:%g')" = "123:456" ]
+  lxc stop -f v1
+  lxc config device remove v1 v1shift
+  lxc storage volume delete "${pool}" v1shift
+  lxc config unset v1 raw.idmap
+
   lxc storage volume create "${pool}" v1dir --type=filesystem size=1MiB
   lxc start v1
   lxc config device add v1 mydir disk source=v1dir pool="${pool}" path=/mnt