Demi Marie Obenour <demiobenour@gmail.com> writes:

> On 11/30/25 23:45, Alyssa Ross wrote:
>> This will solve the problem of mounts for VMs being very annoying to
>> clean up, as evidenced in run-appimage.  It was looking to be even
>> worse for Flatpak.
>
> Does the updater also need to be adjusted?

It does, thanks for pointing that out.  We really need a test for it!

>> Signed-off-by: Alyssa Ross <hi@alyssa.is>
>> ---
>>  .../template/data/service/dbus/run            |  2 ++
>>  .../template/data/service/vhost-user-fs/run   |  4 +--
>>  .../image/usr/bin/create-vm-dependencies      | 23 ++++++++++++++---
>>  host/rootfs/image/usr/bin/run-appimage        | 25 +++++++++----------
>>  4 files changed, 34 insertions(+), 20 deletions(-)
>> 
>> diff --git a/host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/dbus/run b/host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/dbus/run
>> index 351fc68..9b23192 100755
>> --- a/host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/dbus/run
>> +++ b/host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/dbus/run
>> @@ -4,6 +4,8 @@
>>  
>>  importas -i VM VM
>>  
>> +nsenter --mount=${VM}/mount
>> +
>>  dbus-daemon
>>    --config-file /usr/share/dbus-1/session.conf
>>    --print-address 3
>> diff --git a/host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/vhost-user-fs/run b/host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/vhost-user-fs/run
>> index 5d5ad7d..3848b0c 100755
>> --- a/host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/vhost-user-fs/run
>> +++ b/host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/vhost-user-fs/run
>> @@ -12,8 +12,6 @@ export TMPDIR /run
>>  
>>  # The VM should not be able to write directly into a tmpfs, but there
>>  # can be writable block-based bind mounted subdirectories.
>
> Comment should be moved to the code that does the bind mount.  Also,
> this comment is somewhat stale: the main reason to prevent writing
> into the toplevel fs/ folder is that host tools (like the updater)
> assume symlinks can't be created in it.

Good idea.

>> -unshare -m --propagation slave
>>  importas -i VM VM
>> -if { mount --rbind -o ro ${VM}/fs ${VM}/fs }
>> -
>> +nsenter --mount=${VM}/mount
>>  virtiofsd --fd 3 --shared-dir ${VM}/fs
>> diff --git a/host/rootfs/image/usr/bin/create-vm-dependencies b/host/rootfs/image/usr/bin/create-vm-dependencies
>> index f3a1b69..21681cd 100755
>> --- a/host/rootfs/image/usr/bin/create-vm-dependencies
>> +++ b/host/rootfs/image/usr/bin/create-vm-dependencies
>> @@ -1,11 +1,26 @@
>>  #!/bin/execlineb -S1
>>  # SPDX-License-Identifier: EUPL-1.2+
>> -# SPDX-FileCopyrightText: 2024 Alyssa Ross <hi@alyssa.is>
>> +# SPDX-FileCopyrightText: 2024-2025 Alyssa Ross <hi@alyssa.is>
>>  
>> -if { mount -m --rbind -o nofail /run/vm/by-id/${1}/config/fs /run/vm/by-id/${1}/fs/config }
>> +if { touch /run/vm/by-id/${1}/mount }
>> +if { mount --make-private --bind /run/vm/by-id/${1}/mount /run/vm/by-id/${1}/mount }
>>  
>> -if { mkdir -p /run/vm/by-id/${1}/doc-run/doc /run/vm/by-id/${1}/fs/doc }
>> -if { mount --rbind /run/vm/by-id/${1}/doc-run/doc /run/vm/by-id/${1}/fs/doc }
>> +if {
>> +  unshare --mount=/run/vm/by-id/${1}/mount
>> +
>> +  if {
>> +    mkdir -p
>> +      /run/vm/by-id/${1}/doc-run/doc
>> +      /run/vm/by-id/${1}/fs/config
>> +      /run/vm/by-id/${1}/fs/doc
> I would also create /run/vm/by-id/${1}/config/fs...
>
>> +  }
>> +
>> +  if { mount --make-shared --rbind /run/vm/by-id/${1} /run/vm/by-id/${1} }
>> +
>> +  if { mount --rbind -o ro /run/vm/by-id/${1}/fs /run/vm/by-id/${1}/fs }
>
> VMs should not be able to write to their own config, so this and the
> next like should be swapped.

Makes sense.

>> +  if { mount --rbind -o nofail /run/vm/by-id/${1}/config/fs /run/vm/by-id/${1}/fs/config }
>
> ...and remove this `-o nofail`.

Not sure about that, since config is likely to be on a different
filesystem we may not even be able to write to.  Either way, it's out of
scope here, because it already works this way before this change.