uncommon situation

things that might or might not have happened before

Feb 11, 2024 - 6 minute read

Debugging SaltStack file module on FUSE

It all started with HomeAssistant. I wanted to deploy it as an LXC container on top of a Promxmox VE hypervisor. This required enabling USB passthru for the container to use my ZigBee controller.

Easy enough, just allow access to ACM devices and mount the stick by appending this to /etc/pve/lxc/101.conf:

...
lxc.cgroup2.devices.allow: c 166:* rwm
lxc.mount.entry: /dev/serial/by-id/usb-dresden_elektronik_ingenieurtechnik_GmbH_ConBee_II_DE2133183-if00 dev/ttyACM0 none bind,optional,create=file

Restart container, ZigBee dongle works, colorful lights flash all around the house, happiness. However, there’s one remaining chore: I use SaltStack as config management, so I need to backport these changes. The PVE host is already bootstrapped with Salt, so it shouldn’t be much of a challenge I thought (narrator: it was a challenge).

Previously I used salt-cloud to bring up these containers, hence my first idea was to extend the deployment profile with the required LXC options. However, the docs mention no such arguments and sadly the code has no signs of hidden options either. The module uses the PVE API, so I checked if there’s any feature which might not be exposed by salt-cloud. Unfortunately no such thing exists, just a lonely feature request describing the very thing I crave.

All right, so it seems the only option left is to manually edit the container config file mentioned earlier. Not great, not terrible.

First attempt: file.append state

As an infrequent Salt user, I now hit up the docs to learn how to ensure a certain line exists in a file. I quickly find the file.append state, which looks perfect for my use case:

/etc/pve/lxc/101.conf:
  file.append:
    - text:
      - "lxc.cgroup2.devices.allow: c 166:* rwm"
      - "lxc.mount.entry: /dev/serial/by-id/usb-dresden_elektronik_ingenieurtechnik_GmbH_ConBee_II_DE2133183-if00 dev/ttyACM0 none bind,optional,create=file"

Sounds good, doesn’t work. This rather useless message greets me when applying the state (in red, indicating something’s really gone south):

----------
          ID: /etc/pve/lxc/101.conf
    Function: file.append
      Result: False
     Comment: No text found to append. Nothing appended
     Started: 19:40:04.353299
    Duration: 2.25 ms
     Changes:

Of course, the file remains in an untouched state.

Second attempt: file.append module

All right, let’s try running it manually:

saltmaster# salt 'pve' file.append /etc/pve/lxc/101.conf "lxc.cgroup2.devices.allow: c 166:* rwm"
pve:
    Wrote 1 lines to "/etc/pve/lxc/101.conf"

This works, but unfortunately the file.append module’s behaviour differs from the state’s in that the module appends on every run, while the state only appends if the line is not yet present in the file. Naturally, we want the second behaviour for an idempotent definition. But this isn’t a huge problem, I combined it with an unless conditional in the sls file to replicate the file.append state behaviour with the (working) module:

file.append:
  module.run:
    - path: /etc/pve/lxc/101.conf
    - args:
      - "lxc.cgroup2.devices.allow: c 166:* rwm"
    - unless:
      - fun: file.search
        args:
          - /etc/pve/lxc/101.conf
          - "lxc.cgroup2.devices.allow: c 166:* rwm"

I cheated death. I thought. But the execution was less than ideal:

----------
          ID: file.append
    Function: module.run
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/opt/saltstack/salt/lib/python3.10/site-packages/salt/modules/file.py", line 2551, in replace
                  r_data = mmap.mmap(r_file.fileno(), 0, access=mmap.ACCESS_READ)
              OSError: [Errno 19] No such device

              During handling of the above exception, another exception occurred:

              Traceback (most recent call last):
                  ...
                  r_data = salt.utils.stringutils.to_bytes("".join(r_file))
              TypeError: sequence item 0: expected str instance, bytes found
     Started: 20:02:36.364000
    Duration: 313.006 ms
     Changes:

Hmm interesting… After this I tried the same sls with a different file path (something under /tmp), and much to my surprise, both the state and the module versions worked flawlessly. So what’s so special about the Proxmox LXC config that a simple file operation fails?

Down the rabbit hole

The failure of mmap() (a syscall which maps files into memory, in this case wrapped by the python library of the same name) indicated some lower level quirks with the filesystem, so I checked if it had some special mount options or something like that:

pve# mount | grep etc
/dev/fuse on /etc/pve type fuse (rw,nosuid,nodev,relatime,user_id=0,group_id=0,default_permissions,allow_other)

Nice, so it’s a FUSE. Reading the PVE docs I found that it is implemented in userspace by design, backed by an sqlite database and needed for clustered deployments (the db is replicated with corosync to all nodes of the cluster, so the config is always in sync).

But why doesn’t mmap() work? Let’s see what the kernel docs have to say, especially regarding the direct-io mode:

In direct-io mode the page cache is completely bypassed for reads and writes. No read-ahead takes place. Shared mmap is disabled.

Okay, so it’s possible to open() files with FUSE in a way that the shared mode of mmap() is not supported. The pmxcfs code confirms that this is the case with the PVE implementation:

static int cfs_fuse_open(const char *path, struct fuse_file_info *fi)
{
    cfs_debug("enter cfs_fuse_open %s", path);
    fi->direct_io = 1;
...

I wanted to confirm this theory so I ran strace during state execution on the PVE host (nevermind my ugly and probably much longer than necessary shell-fu):

pve# strace --silence=exit,attach -ttyy -e mmap,openat -f \
$(for i in `ps auxwww|grep salt-minion|grep -v grep|awk '{print $2}'`; do echo "-p $i "; done) 2>&1 \
|grep /etc/pve/nodes

[pid 347208] 20:42:23.899822 openat(AT_FDCWD</>, "/etc/pve/nodes/pve0/lxc/101.conf", O_RDONLY|O_CLOEXEC) = 66</etc/pve/nodes/pve0/lxc/101.conf>
[pid 347208] 20:42:23.901366 openat(AT_FDCWD</>, "/etc/pve/nodes/pve0/lxc/101.conf", O_RDONLY|O_CLOEXEC) = 66</etc/pve/nodes/pve0/lxc/101.conf>
[pid 347208] 20:42:23.901544 mmap(NULL, 526, PROT_READ, MAP_SHARED, 66</etc/pve/nodes/pve0/lxc/101.conf>, 0) = -1 ENODEV (No such device)

Bingo, it’s trying to do a shared mmap(), unsuccessfully.

Right, so the file.search module breaks because of mmap(), but why doesn’t the file.append state work? Let’s ask its code:

try:
        for chunk in text:
            if ignore_whitespace:
                if __salt__["file.search"](
                    name,
                    salt.utils.stringutils.build_whitespace_split_regex(chunk),
                    multiline=True,
                ):
                    continue
            elif __salt__["file.search"](name, chunk, multiline=True):
                continue

            for line_item in chunk.splitlines():
                append_lines.append("{}".format(line_item))

    except TypeError:
        return _error(ret, "No text found to append. Nothing appended")

Looks like it also makes use of the file.search module internally, but with the added “benefit” of concealing the underlying exceptions with a totally irrelevant one.

Final attempt

To solve the issue I dug around the file module code a bit more and found that it should actually be able to handle an mmap() failure:

 with salt.utils.files.fopen(path, mode="rb", buffering=bufsize) as r_file:
            try:
                # mmap throws a ValueError if the file is empty.
                r_data = mmap.mmap(r_file.fileno(), 0, access=mmap.ACCESS_READ)
            except (ValueError, OSError):
                # size of file in /proc is 0, but contains data
                r_data = salt.utils.stringutils.to_bytes("".join(r_file))

Sadly, the except branch contains a bug (also visible in the exception at the second attempt): it tries to convert r_file to bytes from string, but it’s already of type bytes because it was opened in rb mode. To correct this, I patched modules/file.py like this:

 with salt.utils.files.fopen(path, mode="rb", buffering=bufsize) as r_file:
            try:
                # mmap throws a ValueError if the file is empty.
                r_data = mmap.mmap(r_file.fileno(), 0, access=mmap.ACCESS_READ)
            except (ValueError, OSError):
                # size of file in /proc is 0, but contains data
                r_data = r_file.read()

Restart minion and test run with the lines missing from config:

----------
          ID: /etc/pve/lxc/101.conf
    Function: file.append
      Result: True
     Comment: Appended 2 lines
     Started: 21:21:26.644776
    Duration: 4.94 ms

Re-run:

----------
          ID: /etc/pve/lxc/101.conf
    Function: file.append
      Result: True
     Comment: File /etc/pve/lxc/101.conf is in correct state
     Started: 21:23:03.601713
    Duration: 4.034 ms

Looks like now I have the right behaviour. The only thing left to do is create an issue/PR for the Salt project to save me from patching after every update help the community.