config.md: fix typo of context #807
Conversation
config.md
Outdated
| @@ -57,13 +57,13 @@ For Windows, see [mountvol][mountvol] and [SetVolumeMountPoint][set-volume-mount | |||
| * Windows: one mount destination MUST NOT be nested within another mount (e.g., c:\\foo and c:\\foo\\bar). | |||
| * Solaris: corresponds to "dir" of the fs resource in [zonecfg(1M)][zonecfg.1m]. | |||
| * **`type`** (string, OPTIONAL) The filesystem type of the filesystem to be mounted. | |||
| * Linux: valid *filesystemtype* supported by the kernel as listed in */proc/filesystems* (e.g., "minix", "ext2", "ext3", "jfs", "xfs", "reiserfs", "msdos", "proc", "nfs", "iso9660"). | |||
| * Linux: valid *filesystem type* supported by the kernel as listed in */proc/filesystems* (e.g., "minix", "ext2", "ext3", "jfs", "xfs", "reiserfs", "msdos", "proc", "nfs", "iso9660"). | |||
There was a problem hiding this comment.
I don't think we want this change, since the current spelling matches mount(2).
There was a problem hiding this comment.
Last sentence is "The filesystem type of the filesystem to be mounted" and we don't have filesystemtype in context, here looks weird.
Keeping consistent in context is easy for readers to understand. And I don't think it's so necessary to use parameter filesystemtype of mount for explanation in SPEC
There was a problem hiding this comment.
And I don't think it's so necessary to use parameter filesystemtype of mount for explanation…
I was thinking that the current entry (from the ancient 77d44b1) was really indented to be filesystemtype, since we don't have a precedence for italicizing English words. If we're moving away from mount(2)'s filesystemtype token, I think we should completely switch to English and use:
Linux: valid filesystem types supported by the kernel are listed…
Although I'm not clear on how “valid” and “supported by the kernel” interact. Maybe that will get sorted in #813.
There was a problem hiding this comment.
OK, let's leave filesystemtype alone, just keep list->array fix.
waiting any comments in #813
There was a problem hiding this comment.
waiting any comments in #813.
Briefly covered in the meeting with @mrunalp and @crosbymichael, this is not a statement about config validation. The spec has no opinions on what strings are valid values here. Once some RFC 2119 language lands (e.g. see here in the rejected #771, which I hope to revive after opencontainers/runc#1442 lands and I can catch runC up with current kernels), it will be clear that all the runtime has to do is hand this value off to the kernel. Whether the mount work or not is between the config author and the kernel. So we want to drop valid here; this line is just a hint to config authors about where to figure out what the kernel will accept, and has nothing to do with config validation or runtime compliance testing.
There was a problem hiding this comment.
So, the conclusion is to keep filesystemtype as it is?
There was a problem hiding this comment.
So, the conclusion is to keep filesystemtype as it is?
@mrunalp and @crosbymichael were fine splitting that up. I think we want:
. Linux: filesystem types supported by the kernel are listed…
There was a problem hiding this comment.
modified to the types of filesystem
Mashimiao
force-pushed
the
config-small-fix
branch
2 times, most recently
from
4afbd97
to
c3d3ab9
Compare
config.md
Outdated
| * **`type`** (string, OPTIONAL) The filesystem type of the filesystem to be mounted. | ||
| * Linux: valid *filesystemtype* supported by the kernel as listed in */proc/filesystems* (e.g., "minix", "ext2", "ext3", "jfs", "xfs", "reiserfs", "msdos", "proc", "nfs", "iso9660"). | ||
| * **`type`** (string, OPTIONAL) The type of the filesystem to be mounted. | ||
| * Linux: the types of filesystem supported by the kernel as listed in */proc/filesystems* (e.g., "minix", "ext2", "ext3", "jfs", "xfs", "reiserfs", "msdos", "proc", "nfs", "iso9660"). |
There was a problem hiding this comment.
This still reads awkwardly to me, since there are multiple filesystems listed in /proc/filesystems. Maybe we can drop the “type” echo:
- Linux: filesystems supported by the kernel are listed in
/proc/filesystems(e.g.,minix,ext2,ext3, …).
There was a problem hiding this comment.
I used
typesnottype.
But why use “type(s)” at all? I still prefer the more compact form from here, but if you want to use “type(s)” I'd recommend either:
Linux: filesystem types supported by the kernel are listed in
/proc/filesystems(e.g.,minix,ext2,ext3, …).
or:
Linux: types of filesystems supported by the kernel are listed in
/proc/filesystems(e.g.,minix,ext2,ext3, …).
replace `filesystemtype` with `type of filesystem` `list of strings` should be replaced by `array of strings` Signed-off-by: Ma Shimiao <mashimiao.fnst@cn.fujitsu.com>
Mashimiao
force-pushed
the
config-small-fix
branch
2 times, most recently
from
ee3e944
to
8baf688
Compare
|
@opencontainers/runtime-spec-maintainers PTAL |
|
LGTM |
1 similar comment
|
LGTM |
list of stringsshould be replaced byarray of stringsSigned-off-by: Ma Shimiao mashimiao.fnst@cn.fujitsu.com