Convert manifests to new mount format

[pwmarcz]

Description of the changes

Follow-up to #411. Converts remaining manifests and documentation.

For the manifests, I used editor automation (regexp replace + a macro), but some manual adjustments were also necessary.

How to test this PR?

Read it carefully.

Here are some checks I did to find what to convert:

• git grep 'fs\.mount\.' - old table syntax
• git grep 'path = "[^/]' - non-absolute paths (but also shows paths with variables like "{{ arch_libdir }}/...")
• git grep 'type = "chroot"' - should not be necessary anymore

---

This change is 

[dimakuv]

Reviewed 43 of 43 files at r1, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @pwmarcz)

a discussion (no related file):
Do you plan to do the same change in the GSC and Examples repos?

...On the other hand, we probably should keep it as-is at least until the next release, otherwise GSC and Examples will fail with older Gramine versions. Can you please create a GitHub issue for each of these repos, and mention that GSC and Examples should be converted after v1.2 is released (or even v1.3, not sure)?

---

---

CI-Examples/blender/blender.manifest.template, line 20 at r1 (raw file):

  { path = "/blender_lib", uri = "file:{{ blender_dir }}/lib" },
  { path = "/usr/{{ arch_libdir }}", uri = "file:/usr/{{ arch_libdir }}" },
  { path = "{{ arch_libdir }}", uri = "file:{{ arch_libdir }}" },

Can we swap these two lines? We have these two lines in a different (and more intuitive) order in other manifest files.

---

CI-Examples/python/python.manifest.template, line 22 at r1 (raw file):

  { path = "{{ python.stdlib }}", uri = "file:{{ python.stdlib }}" },
  { path = "{{ python.distlib }}", uri = "file:{{ python.distlib }}" },
  { path = "/tmp", uri = "file:/tmp" },

I wonder if this works with tmpfs? If yes, maybe replace it (in a separate commit)? Not blocking.

---

LibOS/shim/test/ltp/manifest.template, line 10 at r1 (raw file):

loader.insecure__use_cmdline_argv = true

fs.root.uri = "file:{{ binary_dir }}"

How is it possible that there is no fs.root.path? Is it / by default? Maybe add it here explicitly?

---

LibOS/shim/test/ltp/manifest.template, line 14 at r1 (raw file):

fs.mount = [
  { path = "/etc", uri = "file:/etc" },
  { path = "/dev/shm", uri = "file:/tmp" },

Do we want to add some comment on this trick? Here we back the pseudo-files to create shared memory regions by the actual files under host-OS path /tmp.

---

LibOS/shim/test/regression/host_root_fs.manifest.template, line 13 at r1 (raw file):

  { path = "/lib", uri = "file:{{ gramine.runtimedir(libc) }}" },
  # overwrite host "/etc" - we don't want host-level configuration files, e.g. dynamic loader caches
  { type = "tmpfs", path = "/etc", uri = "file:unused" },

Why do you keep uri here? To check this syntax?

---

Pal/regression/Bootstrap6.manifest.template, line 7 at r1 (raw file):

loader.argv0_override = "{{ entrypoint }}"

fs.mount.root.uri = "file:"

Do you know what was that? This couldn't be used, right, since it is a LibOS-only construct?

[mkow]

Reviewable status: all files reviewed, 7 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv and @pwmarcz)

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Do you plan to do the same change in the GSC and Examples repos?

...On the other hand, we probably should keep it as-is at least until the next release, otherwise GSC and Examples will fail with older Gramine versions. Can you please create a GitHub issue for each of these repos, and mention that GSC and Examples should be converted after v1.2 is released (or even v1.3, not sure)?

I'd say it should be v1.2 for both repos (they track the most recent release).

---

[mkow]

Reviewed 43 of 43 files at r1, all commit messages.
Reviewable status: all files reviewed, 12 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv and @pwmarcz)

a discussion (no related file):
Please grep for mount., there's a hit in toml_utils.h (only partially relevant, fs.mount.smth is used as an example in a comment).

---

---

CI-Examples/bash/manifest.template, line 16 at r1 (raw file):

loader.insecure__use_cmdline_argv = true

fs.mount = [

Just noticed this: Wouldn't it make more sense if this was named fs.mounts? (plural; probably not for this PR)

Code quote:

fs.mount

---

Documentation/manpages/gramine-manifest.rst, line 95 at r1 (raw file):

   fs.mount = [
     { path = "/lib",  uri = "file:{{ gramine.runtimedir() }}" },

Double space

---

LibOS/shim/test/fs/manifest.template, line 9 at r1 (raw file):

fs.mount = [
  { path = "/lib", uri = "file:{{ gramine.runtimedir() }}" },
  { path = "/{{ entrypoint }}", uri = "file:{{ binary_dir }}/{{ entrypoint }}" },

Shouldn't we ensure that entrypoint is absolute in gramine-manifest, instead of manually inserting a slash here?
Ditto for all other places like this one.

Code quote:

"/{{ entrypoint }}"

---

LibOS/shim/test/regression/attestation.manifest.template, line 8 at r1 (raw file):

loader.insecure__use_cmdline_argv = true

# Keep the deprecated `fs.mount` syntax for test purposes

Could you add "TODO: Update after v1.3 release" to make it easier to find later?

---

LibOS/shim/test/regression/host_root_fs.manifest.template, line 13 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Why do you keep uri here? To check this syntax?

If that's the case, then please add a comment about it.

[pwmarcz]

Reviewable status: 37 of 45 files reviewed, 12 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL), "fixup! " found in commit messages' one-liners (waiting on @dimakuv and @mkow)

a discussion (no related file):

Previously, mkow (Michał Kowalczyk) wrote…

I'd say it should be v1.2 for both repos (they track the most recent release).

I agree it should be v1.2. Ticketed: gramineproject/gsc#46, gramineproject/examples#17.

---

a discussion (no related file):

Previously, mkow (Michał Kowalczyk) wrote…

Please grep for mount., there's a hit in toml_utils.h (only partially relevant, fs.mount.smth is used as an example in a comment).

I changed these examples to fs.root.uri.

---

---

CI-Examples/bash/manifest.template, line 16 at r1 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

Just noticed this: Wouldn't it make more sense if this was named fs.mounts? (plural; probably not for this PR)

I like fs.mount. I don't think migrating from "mount" to "mounts" is a good idea, especially since we are not actually changing any "singular" thing to a "plural" thing, just a table to an array.

Also, "mounts" are not a thing according to our documentation, and IMO this would look akward (we use "mountpoints" or "mounted filesystems").

I'm interpreting this mount as a verb, not as a noun, i.e. "here's a list of things to mount".

---

CI-Examples/blender/blender.manifest.template, line 20 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Can we swap these two lines? We have these two lines in a different (and more intuitive) order in other manifest files.

Done.

---

CI-Examples/python/python.manifest.template, line 22 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

I wonder if this works with tmpfs? If yes, maybe replace it (in a separate commit)? Not blocking.

I think it works. I added a commit.

---

Documentation/manpages/gramine-manifest.rst, line 95 at r1 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

Double space

Fixed.

---

LibOS/shim/test/fs/manifest.template, line 9 at r1 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

Shouldn't we ensure that entrypoint is absolute in gramine-manifest, instead of manually inserting a slash here?
Ditto for all other places like this one.

{{ entrypoint }} is also used relative to {{ binary_dir }}. Passing it with a slash would work, but I think it's cleaner to treat the entrypoint variable as file name, not as an absolute path inside Gramine.

---

LibOS/shim/test/ltp/manifest.template, line 10 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

How is it possible that there is no fs.root.path? Is it / by default? Maybe add it here explicitly?

fs.root is a special manifest key for mounting /, so there is no such key as fs.root.path.

---

LibOS/shim/test/ltp/manifest.template, line 14 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Do we want to add some comment on this trick? Here we back the pseudo-files to create shared memory regions by the actual files under host-OS path /tmp.

Actually, I don't think it does anythng, LTP apparently falls back to creating a file in /tmp. I removed the mount (committed separately) and all tests still pass.

tst_test.c in LTP sources:

        if (access("/dev/shm", F_OK) == 0) {
                snprintf(shm_path, sizeof(shm_path), "/dev/shm/ltp_%s_%d",
                         tid, getpid());
        } else {
                char *tmpdir;

                if (!tst_tmpdir_created())
                        tst_tmpdir();

                tmpdir = tst_get_tmpdir();
                snprintf(shm_path, sizeof(shm_path), "%s/ltp_%s_%d",
                         tmpdir, tid, getpid());
                free(tmpdir);
        }

        ipc_fd = open(shm_path, O_CREAT | O_EXCL | O_RDWR, 0600);
        if (ipc_fd < 0)
                tst_brk(TBROK | TERRNO, "open(%s)", shm_path);
        SAFE_CHMOD(shm_path, 0666);

        SAFE_FTRUNCATE(ipc_fd, size);

        results = SAFE_MMAP(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, ipc_fd, 0);

---

LibOS/shim/test/regression/attestation.manifest.template, line 8 at r1 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

Could you add "TODO: Update after v1.3 release" to make it easier to find later?

I added a TODO, but mention "two versions after v1.2", same as TODOs in the code that implements the old syntax.

(I was hesitant to mention "v1.3" or "v1.4" since we had some disagreements recently regarding whether we should follow semantic versioning... which would mean having to bump one of the next versions to v2).

---

LibOS/shim/test/regression/host_root_fs.manifest.template, line 13 at r1 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

If that's the case, then please add a comment about it.

No, that was a mistake. I'm removing this.

---

Pal/regression/Bootstrap6.manifest.template, line 7 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Do you know what was that? This couldn't be used, right, since it is a LibOS-only construct?

It seems that PAL handled these, and fs.mount.root.uri was mandatory (it's like this in the first commit in graphene).

Methodology: (go to graphene, run git log -p -Sfs.mount.root.uri, and scroll to the very end :) )

[mkow]

Reviewed 8 of 8 files at r2, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL), "fixup! " found in commit messages' one-liners (waiting on @dimakuv and @pwmarcz)

---

CI-Examples/bash/manifest.template, line 16 at r1 (raw file):

I don't think migrating from "mount" to "mounts" is a good idea, especially since we are not actually changing any "singular" thing to a "plural" thing, just a table to an array.

It's actually a bit orthogonal to this PR, I think that in general this should be plural to be more "grammar correct". I.e. it's not that it should be plural because you made this change, previously I also disliked it, I'm just using the opportunity of changing the syntax to include more changes while we're doing it.

Also, "mounts" are not a thing according to our documentation, and IMO this would look akward (we use "mountpoints" or "mounted filesystems").

I consider "mount" as a shorthand for "mount point".

I'm interpreting this mount as a verb, not as a noun, i.e. "here's a list of things to mount".

The problem I have with this way of looking at it is that TOML is a declarative language, so here we're actually specifying a list of mounts.

---

LibOS/shim/test/fs/manifest.template, line 9 at r1 (raw file):

Previously, pwmarcz (Paweł Marczewski) wrote…

{{ entrypoint }} is also used relative to {{ binary_dir }}. Passing it with a slash would work, but I think it's cleaner to treat the entrypoint variable as file name, not as an absolute path inside Gramine.

Ouch. Ok, let's keep it as is, although it looks a bit ugly/hacky.

---

LibOS/shim/test/regression/attestation.manifest.template, line 8 at r1 (raw file):

Previously, pwmarcz (Paweł Marczewski) wrote…

I added a TODO, but mention "two versions after v1.2", same as TODOs in the code that implements the old syntax.

(I was hesitant to mention "v1.3" or "v1.4" since we had some disagreements recently regarding whether we should follow semantic versioning... which would mean having to bump one of the next versions to v2).

Makes sense, resolving.

[dimakuv]

Reviewed 6 of 8 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL), "fixup! " found in commit messages' one-liners (waiting on @mkow and @pwmarcz)

---

CI-Examples/bash/manifest.template, line 16 at r1 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

I don't think migrating from "mount" to "mounts" is a good idea, especially since we are not actually changing any "singular" thing to a "plural" thing, just a table to an array.

It's actually a bit orthogonal to this PR, I think that in general this should be plural to be more "grammar correct". I.e. it's not that it should be plural because you made this change, previously I also disliked it, I'm just using the opportunity of changing the syntax to include more changes while we're doing it.

Also, "mounts" are not a thing according to our documentation, and IMO this would look akward (we use "mountpoints" or "mounted filesystems").

I consider "mount" as a shorthand for "mount point".

I'm interpreting this mount as a verb, not as a noun, i.e. "here's a list of things to mount".

The problem I have with this way of looking at it is that TOML is a declarative language, so here we're actually specifying a list of mounts.

I also like fs.mounts. And I agree with Michal that since we're changing all manifest files anyway in this PR, maybe we'll also add this synonym? I actually like the idea.

---

LibOS/shim/test/ltp/manifest.template, line 10 at r1 (raw file):

Previously, pwmarcz (Paweł Marczewski) wrote…

fs.root is a special manifest key for mounting /, so there is no such key as fs.root.path.

Ok. But please see my other comment (another manifest file erroneously uses fs.root.path).

Also, then this line in GSC is totally useless? https://github.com/gramineproject/gsc/blob/6a905d753c1b243ee66b0266d542f6a790f279ed/templates/entrypoint.common.manifest.template#L10

---

LibOS/shim/test/ltp/manifest.template, line 14 at r1 (raw file):

Previously, pwmarcz (Paweł Marczewski) wrote…

Actually, I don't think it does anythng, LTP apparently falls back to creating a file in /tmp. I removed the mount (committed separately) and all tests still pass.

tst_test.c in LTP sources:

        if (access("/dev/shm", F_OK) == 0) {
                snprintf(shm_path, sizeof(shm_path), "/dev/shm/ltp_%s_%d",
                         tid, getpid());
        } else {
                char *tmpdir;

                if (!tst_tmpdir_created())
                        tst_tmpdir();

                tmpdir = tst_get_tmpdir();
                snprintf(shm_path, sizeof(shm_path), "%s/ltp_%s_%d",
                         tmpdir, tid, getpid());
                free(tmpdir);
        }

        ipc_fd = open(shm_path, O_CREAT | O_EXCL | O_RDWR, 0600);
        if (ipc_fd < 0)
                tst_brk(TBROK | TERRNO, "open(%s)", shm_path);
        SAFE_CHMOD(shm_path, 0666);

        SAFE_FTRUNCATE(ipc_fd, size);

        results = SAFE_MMAP(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, ipc_fd, 0);

Wow. Thanks for checking. And we had it from the dawn of times, and I always believed it...

---

LibOS/shim/test/regression/host_root_fs.manifest.template, line 7 at r2 (raw file):

loader.env.LD_LIBRARY_PATH = "/lib"

fs.root.path = "/"

Please remove this then (see my other comment).

---

Pal/regression/Bootstrap6.manifest.template, line 7 at r1 (raw file):

(it's like this in the first commit in graphene)

The infamous first commit of Graphene! So many memories.

[pwmarcz]

Reviewable status: 44 of 45 files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL), "fixup! " found in commit messages' one-liners (waiting on @dimakuv and @mkow)

---

CI-Examples/bash/manifest.template, line 16 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

I also like fs.mounts. And I agree with Michal that since we're changing all manifest files anyway in this PR, maybe we'll also add this synonym? I actually like the idea.

Okay. But then I don't think there should be any synonym, just single way of doing things. We're changing the syntax anyway, so we can allow fs.mounts only. Is that OK?

(Of course, the deprecated fs.mount.xyz stays the same for now).

---

LibOS/shim/test/ltp/manifest.template, line 10 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Ok. But please see my other comment (another manifest file erroneously uses fs.root.path).

Also, then this line in GSC is totally useless? https://github.com/gramineproject/gsc/blob/6a905d753c1b243ee66b0266d542f6a790f279ed/templates/entrypoint.common.manifest.template#L10

Yes.

---

LibOS/shim/test/ltp/manifest.template, line 14 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Wow. Thanks for checking. And we had it from the dawn of times, and I always believed it...

Maybe some older version of LTP needed it?

---

LibOS/shim/test/regression/host_root_fs.manifest.template, line 7 at r2 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please remove this then (see my other comment).

Done.

[mkow]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL), "fixup! " found in commit messages' one-liners (waiting on @dimakuv)

---

CI-Examples/bash/manifest.template, line 16 at r1 (raw file):

I don't think there should be any synonym, just single way of doing things.

Definitely, +1 from my side, I never wanted it to be a synonym, just to rename this option :P

[pwmarcz]

Reviewable status: 6 of 48 files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @dimakuv and @mkow)

---

CI-Examples/bash/manifest.template, line 16 at r1 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

I don't think there should be any synonym, just single way of doing things.

Definitely, +1 from my side, I never wanted it to be a synonym, just to rename this option :P

Done (and rebased: I put the rename to fs.mounts as first commit).

Quick check: git grep 'fs\.mount[^s]' should not show anything surprising.

[mkow]

Reviewed 42 of 42 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL), "fixup! " found in commit messages' one-liners (waiting on @dimakuv)

[dimakuv]

Reviewed 39 of 42 files at r4, all commit messages.
Reviewable status: all files reviewed, all discussions resolved, "fixup! " found in commit messages' one-liners

---

CI-Examples/bash/manifest.template, line 16 at r1 (raw file):

Previously, pwmarcz (Paweł Marczewski) wrote…

Done (and rebased: I put the rename to fs.mounts as first commit).

Quick check: git grep 'fs\.mount[^s]' should not show anything surprising.

OK, I'm happy with this solution. I just proposed a synonym (alias) because I thought it would be very cumbersome to have a TOML table fs.mount and a TOML array fs.mounts. But looks like it is not cumbersome to implement.

[dimakuv]

Reviewed 2 of 8 files at r2, 3 of 42 files at r4.
Reviewable status: all files reviewed, all discussions resolved, "fixup! " found in commit messages' one-liners

[dimakuv]

Reviewed all commit messages.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required)

[dimakuv]

Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required)

[mkow]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

---

CI-Examples/bash/manifest.template, line 16 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

OK, I'm happy with this solution. I just proposed a synonym (alias) because I thought it would be very cumbersome to have a TOML table fs.mount and a TOML array fs.mounts. But looks like it is not cumbersome to implement.

Ah, this is where the "alias" came from, I thought that Paweł was thinking that I suggested aliasing them :) (and I missed that Dmitrii's post included this)