On 2026/7/14 14:36, Giuseppe Scrivano wrote:
Gao Xiang <[email protected]> writes:

On 2026/7/14 08:49, Aleksa Sarai wrote:
On 2026-07-13, Giuseppe Scrivano <[email protected]> wrote:
thanks for the hints.

I'll prepare a v3 if you are fine with the version below:
No worries, and this seems more reasonable at a first glance.

diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 86fa5c6a0c70..72c85cc53085 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
...
@@ -437,6 +439,38 @@ static bool erofs_fc_set_dax_mode(struct fs_context *fc, 
unsigned int mode)
        return false;
   }
   +static int erofs_fc_parse_source(struct fs_context *fc,
+                                struct fs_parameter *param)
+{
+       struct erofs_sb_info *sbi = fc->s_fs_info;
+
+       if (fc->source || sbi->dif0.file)
+               return invalf(fc, "Multiple sources");
+
+       switch (param->type) {
+       case fs_value_is_string:
+               fc->source = param->string;
+               param->string = NULL;
+               return 0;
+       case fs_value_is_file: {
+               char *buf, *p;
+
+               sbi->dif0.file = get_file(param->file);
A very minor nit, but you can actually steal the file reference here
with
                sbi->dif0.file = no_free_ptr(param->file);
A few other places do this. (You'll also need to change the
param->file
reference below.)

+               buf = kmalloc(PATH_MAX, GFP_KERNEL);
+               if (!buf)
+                       return -ENOMEM;
+               p = file_path(param->file, buf, PATH_MAX);
+               fc->source = kstrdup(IS_ERR(p) ? "(fd)" : p, GFP_KERNEL);
I think that /proc/self/fd/%d would be a more useful name for
debugging
if file_path() fails (not that it is really possible here AFAICS). But
I'm not really too fussed.

Not quite sure if we should get in agreement with the format of this
one in advance (IOWs, users use source_fd and how fc->source looks like;
since other fses may follow the same practice if source_fd becomes common
later) since it's a user-visible field and I believe we shouldn't treat
this one as a dontcare field as some pseudo fses (since those fses don't
rely on `fc->source` by design but typically EROFS can rely on.)

I hope Christian and others could share move thought on this part too
before I land this feature for the next cycle.

would it be better to just return the error from file_path without any
fallback?

I hope Christian or other vfs folks can decide how to handle fc->source
string here, since in the long term, how to deal with source_fd should
be unique among different fses: just our current short-term
implementation lands into erofs directly for file-backed mounts to
fulfill composefs needs.

Thanks,
Gao Xiang


Thanks,
Giuseppe


Reply via email to