Re: [PATCH 36/38] vfs: Add a sample program for the new mount API [ver #10]

From: David Howells
Date: Tue Jul 31 2018 - 06:11:59 EST


Pavel Machek <pavel@xxxxxx> wrote:

> Proposal is "message %s foo %s\0param 1\0param2\0", only strings
> allowed.

I think that's too strict and you will need to allow integer values, IP
addresses and possibly other things also. It could certainly have a limited
set (e.g. no kernel pointers).

You also haven't proposed what you think the internal interface should look
like.

> That's simple enough, yet allows translations.

Yes and no. From the point of view of translating it, yes, it's easier, but
from the point of view of actually writing these things in the kernel, it's
not.

So, currently, I have, say:

cg_invalf(fc, "option or name mismatch, new: 0x%x \"%s\", old: 0x%x \"%s\"", ...);

Now I can split that to separate out the option change error from the option
change error:

cg_invalf(fc, "Name mismatch, new: \"%s\", old: \"%s\"", ...);
cg_invalf(fc, "Option mismatch, new: 0x%x, old: 0x%x", ...);

but we still need to communicate to the logging function how many arguments
we're actually passing. Now, I wouldn't consider this a fast-path, so
actually counting the arguments encoded in the format string is probably fine
- otherwise we have to actually pass the number of arguments in, e.g.:

cg_invalf(fc, "Option mismatch, new: 0x%x, old: 0x%x", 2, ...);

and then the kernel has to render the whole lot into

Another example, in ext4 we have:

printk(KERN_NOTICE "EXT4-fs (%s): last error at time %u: %.*s:%d",

so we could do something like:

infof(fc, "EXT4-fs (%s): last error at time %u: %.*s:%d", ...);

on mount. But the use of "%.*s" is a pain for your interface. We can't pass
qualifiers like "*" to userspace, so either the caller would have to copy the
string first or the logging routines would have to edit the format string.
Though I suppose we could leave the editing to userspace.

David