[RFC] Revert partially "criu: handle symlink mount dests"

[giuseppe]

commit e0b0158 introduced a regression with podman system tests.

When the lookup in the chroot fails, fallback to the previous behavior of using the verbatim destination path.

Summary by Sourcery

Restore previous mount destination handling in CRIU integration by falling back to the original destination path when chroot_realpath fails, preventing regressions in Podman system tests.

Bug Fixes:

• Fallback to the verbatim mount destination path when chroot_realpath lookup fails instead of aborting.
• Apply the same fallback behavior for both checkpoint and restore CRIU operations.

[sourcery-ai]

Reviewer's Guide

This pull request modifies the CRIU integration in crun to revert part of a previous change by replacing a hard error on failing to resolve a mount destination under the container rootfs with a fallback to the original destination path, thereby fixing a regression in Podman system tests.

Sequence Diagram: Conditional Mount Path Handling in CRIU Functions

sequenceDiagram
    participant Func as "CRIU Function (Checkpoint/Restore)"
    participant Resolver as "chroot_realpath()"
    participant Wrapper as "libcriu_wrapper"

    Func->>Resolver: chroot_realpath(rootfs, original_dest, buf)
    Resolver-->>Func: absolute_resolved_path_or_null
    alt absolute_resolved_path_or_null is NOT NULL
        Func->>Func: dest_in_root = absolute_resolved_path + strlen(rootfs)
    else absolute_resolved_path_or_null is NULL (Fallback)
        Func->>Func: dest_in_root = original_dest
    end
    Func->>Wrapper: criu_add_ext_mount(dest_in_root, ...)

Loading

File-Level Changes

Change	Details	Files
Implement fallback for failed chroot realpath lookups when adding external mounts	In checkpoint routine, only adjust dest_in_root when realpath succeeds, otherwise fallback to the original mount destination In restore routine, apply the same conditional fallback logic instead of returning an error	src/libcrun/criu.c

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[kolyshkin]

Overall the patch LGTM, but I'm intrigued as to what are the failures, @giuseppe can you link to failed CI?

[kolyshkin]

Overall the patch LGTM, but I'm intrigued as to what are the failures, @giuseppe can you link to failed CI?

Nevermind, found it: #1756

[kolyshkin]

Hmm, I think this line is the source of the issue:

ret = append_paths (&rootfs, err, status->bundle, status->rootfs, NULL);

In here

• status->bundle is something like /var/lib/containers/storage/overlay-containers/XXXX/userdata
• status->rootfs is something like /var/lib/containers/storage/overlay/YYYY/merged

Both paths are absolute, and it does not make sense to join them. I traced this back to commit c7fe8f7.

My code in commit e0b0158 uses this as rootfs, which is obviously wrong. Will open a PR.

[kolyshkin]

Opened #1758.

Will take a look at why criu root is set to bundle + rootfs tomorrow (see previous comment). Maybe @adrianreber you remember something?

[adrianreber]

Opened #1758.

Will take a look at why criu root is set to bundle + rootfs tomorrow (see previous comment). Maybe @adrianreber you remember something?

Unfortunately I don't remember much about this code. The goal was to make it work. It definitely could be that it works just by accident.

Reading your comments now it definitely sounds wrong what the code does.

[giuseppe]

closing in favor of #1758