Device: Don't remove an existing target directory when unmounting a `disk` device if the original dir hasn't been created by LXD

[gabrielmougard]

closes #12648 closes #12716

---

Overall description

When a disk device is removed while relying on a host directory and mapped to a target within a container or a VM, we detect if the target directory has been created by LXD or not in order to not delete the content of a target directory during the unmount. Additionaly, with the VM case, we cleanly unmount the target path inside the VM.

Container case

Example:

• Let say we mounted a custom host empty directory (test) on the existing /opt directory of the container, when unmounted (lxc device remove ...), the target /opt won't be removed, because we marked it as NOT being created by LXD at mount time.

• If, however, we created a custom empty target directory at mount time: lxc config device add u1 test disk source=/home/user/test path=/new_dir, the directory new_dir will be created on the target instance and if we decide to unmount test, the target /new_dir will be removed because is has been created by LXD

Particular case for VMs

In addition to that, this fix also cleanly unmount the target directory inside the VM through an new LXD-agent API call. Before that, here is what would have happened:

mkdir /tmp/empty-dir
lxc launch ubuntu:jammy v1 --vm
lxc config device add v1 empty-dir disk source=/tmp/empty-dir path=/opt
lxc config device remove v1 empty-dir

lxc shell v1 -- stat /opt
stat: cannot statx '/opt': Transport endpoint is not connected

This happens because the mounted device and its associated char device were removed using QEMU's QMP without unmounting the target.

Now, we inspect for the mounts and the over-mounts if any on the VM, and unmount them in the right order.

[gabrielmougard]

Working on the integration tests now.

[gabrielmougard]

Tests should be ready

[gabrielmougard]

Also, @tomponline, the deviceVolatileSetFunc function, which calls the VolatileSet, updates the DB. I think this is good to persist this information though. Removing an existing target dir like /opt for example (see the original post of the issue author) through an umount seems quite bad.

Or, we can introduce a new device option to say "not to track that kind of changes" so that I can set d.volatileSetPersistDisable = true before my volatile set call and then restore it to false after. This will surely be faster (no DB call) but the user accept a potential data loss on the target instance if he mounted on an existing target dir.

[gabrielmougard]

@tomponline I implemented the logic for the VM as well within the lxd-agent. I tested it on my side without an issue.

[tomponline]

Thanks @gabrielmougard please can you add a PR description with what has changed so the reviewer can compare the code to the intended design.

Please do include any new API endpoints you've added to the lxd-agent.

Cheers

[tomponline]

Please can you rebase @gabrielmougard

[gabrielmougard]

@tomponline that's interesting! I just didn't know about this eventsProcess() logic tbh. You're right, if we could handle the unmount logic as part of an event hook, that'd be better and it'll avoid adding new API endpoint. I'll see if I can move the logic.

[gabrielmougard]

@tomponline I moved the unmount logic bit into the eventsProcess() function as you required. I reproduced the integration tests on my side with a VM, no issue so far.

[gabrielmougard]

@tomponline a couple of thoughts on your questions.

[gabrielmougard]

@tomponline can you review this ? Here is the LXD-CI PR https://github.com/canonical/lxd-ci/pull/83 for the VM tests.

[gabrielmougard]

@tomponline @roosterfish can you review this please?

[github-actions[bot]]

Heads up @ru-fu - the "Documentation" label was applied to this issue.

[gabrielmougard]

@tomponline @roosterfish updated

[tomponline]

Please can you rebase