fuse-waked: for CAS, hardlinks backed by hardlinks, share virtual mode. by dtzSiFive · Pull Request #1836 · sifiveinc/wake

Will Dietz (dtzSiFive) · 2026-05-05T19:35:23Z

Modify hardlink test to run with CAS enabled, which before this change would cause it to fail (cannot write through hardlinks).

Extend test to check mode changes on one are observed on the other.

Will Dietz (dtzSiFive) · 2026-05-05T19:56:56Z


  if (it->second.is_visible(keyt.second)) return -EEXIST;

+  if (it->second.is_visible(keyf.second)) return -EACCES;


Drive-by: ban hardlinks originating from visible files for lack of structure presently to enforce RO access this way.

Fixes existing bug where you can modify visible files through a hardlink.

Wake snippet demonstrating:

export def datFile Unit = write "datFile" "hello world" export def modLink Unit = require Pass dat = datFile Unit makeExecPlan ("sh", "-c", "ln datFile modLink; echo asdf >> modLink; cat datFile", Nil) (dat, Nil) | setPlanLabel "test (hardlink): modLink" | setPlanShare False | runJobWith defaultRunner | getJobOutput export def test _ = require Pass out = modLink Unit Pass Unit

Note that since we -- before this PR -- block writes through hardlinks in CAS mode, this issue today only exists in non-CAS wake.

cc #1593 .

This change might deny benign jobs from performing reasonable behavior and should be considered carefully and perhaps separately 👍 .

Yeah I think this change is right, but don't know if there is any fallout due to this, so agreed that maybe we test and evaluate this in a separate PR

Actually, I think today attempts to hardlink visible "succeed" but fall through and don't do this in the staging directory but write through to workspace? The new -EEXIST check prevents this, as does the tentative is_visible explicit guard discussed here.

Will Dietz (dtzSiFive) · 2026-05-05T19:58:46Z

-      hardlinks.insert(std::string(to));
      return 0;
    }
+    return -EEXIST;


In CAS mode: never fallback to workspace!

This may be unreachable, it should be, but just be explicit here to avoid surprise behaviors while supporting both.

Would ENONET or EPERM be a better return for if a file is not staged?

Will Dietz (dtzSiFive) · 2026-05-05T20:03:58Z

Dare we set st_nlink based on this? 😉

Sam May (ag-eitilt)

This all looks good to me!

Sam May (ag-eitilt) · 2026-05-05T21:28:46Z

 trap 'rm link_file_src.txt link_file_dst.txt' EXIT

-STDOUT=$(${1}/wakebox -p input.json)
+export WAKE_CAS=1


Probably don't want to hardcode this in the test? I know we were saying that we'd rather push toward getting everything unified rather than supporting multiple code paths, but make test ; WAKE_CAS=1 make test should still leave us testing both paths over the full suite.

Ah, I see what you're getting at with this guard -- this is a breaking change between CAS and non-CAS. Can't say I like the different behaviour, but it's unlikely enough to not cause any issue. Instead of setting WAKE_CAS, though, can we instead check it, print a stderr warning about skipping the test, and then exit 0? (Also applies to #1838)

Mainline WAKE doesn't propagate WAKE_CAS=1 into the tests, whether this is a bug or not I am not sure. If I was certain I'd have addressed it.

But okay that works for me. I removed it in the multi-wake variant.

Good call!

Sam May (ag-eitilt) · 2026-05-05T21:31:24Z

 struct StagedFileData {
  std::string staging_path;
-  mode_t mode;
+  std::shared_ptr<mode_t> mode;
+
+  bool is_hardlink() const { return mode.use_count() > 1; }
 };


I don't like this C++ design choice... struct should be data, and if you want methods you reach for class/interface. But that's me getting grumpy at the language, not at this implementation.

I'll hoist it out to is_hardlink on StagedItem, that fits the current organization/style better anyway. Meant to revisit this, thanks!

Abrar Quazi (AbrarQuazi)

LGTM, thanks!

We should merge this to master and then maybe cherry pick to the feature/multi-wake branch so we can get to a beta release faster

Abrar Quazi (AbrarQuazi) · 2026-05-05T21:30:14Z

-      hardlinks.insert(std::string(to));
      return 0;
    }
+    return -EEXIST;


Would ENONET or EPERM be a better return for if a file is not staged?

Abrar Quazi (AbrarQuazi) · 2026-05-05T21:35:29Z


  if (it->second.is_visible(keyt.second)) return -EEXIST;

+  if (it->second.is_visible(keyf.second)) return -EACCES;


Yeah I think this change is right, but don't know if there is any fallout due to this, so agreed that maybe we test and evaluate this in a separate PR

Abrar Quazi (AbrarQuazi) · 2026-05-05T21:45:00Z

      it->second.files_wrote.insert(keyt.second);
-      // Both hardlink paths need direct_io to prevent kernel caching issues
-      hardlinks.insert(std::string(from));
-      hardlinks.insert(std::string(to));


Can the hardlinks set be completely removed? Or its still needed because the non-CAS mode still depends on it? If thats the case, maybe add a comment to remove this structure

It's for non-CAS. I'll add a comment.

Will Dietz (dtzSiFive) · 2026-05-05T22:05:05Z

Mixed signals on where we want to send these changes hahaha.

Applied y'all's feedback to multi-wake version. I'll sync those changes back here later.

Will Dietz (dtzSiFive) added 2 commits May 5, 2026 12:38

fuse-waked: for CAS, hardlinks backed by hardlinks, share virtual mode.

5ee1666

tests: update hardlink test to check mode, test WAKE_CAS=1.

0eb963e

Will Dietz (dtzSiFive) force-pushed the fix/hardlink-shared-mode branch from f4e96a5 to 0eb963e Compare May 5, 2026 19:38

Will Dietz (dtzSiFive) commented May 5, 2026

View reviewed changes

Will Dietz (dtzSiFive) requested review from Abrar Quazi (AbrarQuazi), Sam May (ag-eitilt) and Nathan Graybeal (ngraybeal) May 5, 2026 19:59

Will Dietz (dtzSiFive) mentioned this pull request May 5, 2026

fuse-waked: for CAS, hardlinks backed by hardlinks, share virtual mode. #1838

Merged

Sam May (ag-eitilt) approved these changes May 5, 2026

View reviewed changes

Abrar Quazi (AbrarQuazi) approved these changes May 5, 2026

View reviewed changes


		if (it->second.is_visible(keyt.second)) return -EEXIST;

		if (it->second.is_visible(keyf.second)) return -EACCES;

Conversation

Will Dietz (dtzSiFive) commented May 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Will Dietz (dtzSiFive) commented May 5, 2026

Uh oh!

Sam May (ag-eitilt) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Abrar Quazi (AbrarQuazi) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Will Dietz (dtzSiFive) commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants