On Tue, May 03, 2022 at 05:39:46PM +0300, Amir Goldstein wrote:
> On Tue, May 3, 2022 at 3:23 PM Miklos Szeredi <[email protected]> wrote:
> >
> > This is a simplification of the getvalues(2) prototype and moving it to the
> > getxattr(2) interface, as suggested by Dave.
> >
> > The patch itself just adds the possibility to retrieve a single line of
> > /proc/$$/mountinfo (which was the basic requirement from which the fsinfo
> > patchset grew out of).
> >
> > But this should be able to serve Amir's per-sb iostats, as well as a host of
> > other cases where some statistic needs to be retrieved from some object. Note:
> > a filesystem object often represents other kinds of objects (such as processes
> > in /proc) so this is not limited to fs attributes.
> >
> > This also opens up the interface to setting attributes via setxattr(2).
> >
> > After some pondering I made the namespace so:
> >
> > : - root
> > bar - an attribute
> > foo: - a folder (can contain attributes and/or folders)
> >
> > The contents of a folder is represented by a null separated list of names.
> >
> > Examples:
> >
> > $ getfattr -etext -n ":" .
> > # file: .
> > :="mnt:\000mntns:"
> >
> > $ getfattr -etext -n ":mnt:" .
> > # file: .
> > :mnt:="info"
> >
> > $ getfattr -etext -n ":mnt:info" .
> > # file: .
> > :mnt:info="21 1 254:0 / / rw,relatime - ext4 /dev/root rw\012"
> >
> > $ getfattr -etext -n ":mntns:" .
> > # file: .
> > :mntns:="21:\00022:\00024:\00025:\00023:\00026:\00027:\00028:\00029:\00030:\00031:"
> >
> > $ getfattr -etext -n ":mntns:28:" .
> > # file: .
> > :mntns:28:="info"
> >
> > Comments?
> >
>
> I like that :)
>
> It should be noted that while this API mandates text keys,
> it does not mandate text values, so for example, sb iostats could be
> exported as text or as binary struct, or as individual text/binary records or
> all of the above.
Ugh, no, that would be a total mess. Don't go exporting random binary
structs depending on the file, that's going to be completely
unmaintainable. As it is, this is going to be hard enough with random
text fields.
As for this format, it needs to be required to be documented in
Documentation/ABI/ for each entry and key type so that we have a chance
of knowing what is going on and tracking how things are working and
validating stuff.
thanks,
greg k-h
On Tue, 3 May 2022 at 16:53, Greg KH <[email protected]> wrote:
>
> On Tue, May 03, 2022 at 05:39:46PM +0300, Amir Goldstein wrote:
> > It should be noted that while this API mandates text keys,
> > it does not mandate text values, so for example, sb iostats could be
> > exported as text or as binary struct, or as individual text/binary records or
> > all of the above.
>
> Ugh, no, that would be a total mess. Don't go exporting random binary
> structs depending on the file, that's going to be completely
> unmaintainable. As it is, this is going to be hard enough with random
> text fields.
>
> As for this format, it needs to be required to be documented in
> Documentation/ABI/ for each entry and key type so that we have a chance
> of knowing what is going on and tracking how things are working and
> validating stuff.
My preference would be a single text value for each key.
Contents of ":mnt:info" contradicts that, but mountinfo has a long
established, well documented format, and nothing prevents exporting
individual attributes with separate names as well (the getvalues(2)
patch did exactly that).
Thanks,
Miklos
On Tue, May 3, 2022 at 6:04 PM Miklos Szeredi <[email protected]> wrote:
>
> On Tue, 3 May 2022 at 16:53, Greg KH <[email protected]> wrote:
> >
> > On Tue, May 03, 2022 at 05:39:46PM +0300, Amir Goldstein wrote:
>
> > > It should be noted that while this API mandates text keys,
> > > it does not mandate text values, so for example, sb iostats could be
> > > exported as text or as binary struct, or as individual text/binary records or
> > > all of the above.
> >
> > Ugh, no, that would be a total mess. Don't go exporting random binary
> > structs depending on the file, that's going to be completely
> > unmaintainable. As it is, this is going to be hard enough with random
> > text fields.
> >
> > As for this format, it needs to be required to be documented in
> > Documentation/ABI/ for each entry and key type so that we have a chance
> > of knowing what is going on and tracking how things are working and
> > validating stuff.
>
> My preference would be a single text value for each key.
>
> Contents of ":mnt:info" contradicts that, but mountinfo has a long
> established, well documented format, and nothing prevents exporting
> individual attributes with separate names as well (the getvalues(2)
> patch did exactly that).
>
Right, the fun is that ":mnt:info" and ":mnt:info:" can co-exist.
Thanks,
Amir.
On Tue, May 03, 2022 at 05:04:06PM +0200, Miklos Szeredi wrote:
> On Tue, 3 May 2022 at 16:53, Greg KH <[email protected]> wrote:
> >
> > On Tue, May 03, 2022 at 05:39:46PM +0300, Amir Goldstein wrote:
>
> > > It should be noted that while this API mandates text keys,
> > > it does not mandate text values, so for example, sb iostats could be
> > > exported as text or as binary struct, or as individual text/binary records or
> > > all of the above.
> >
> > Ugh, no, that would be a total mess. Don't go exporting random binary
> > structs depending on the file, that's going to be completely
> > unmaintainable. As it is, this is going to be hard enough with random
> > text fields.
> >
> > As for this format, it needs to be required to be documented in
> > Documentation/ABI/ for each entry and key type so that we have a chance
> > of knowing what is going on and tracking how things are working and
> > validating stuff.
>
> My preference would be a single text value for each key.
Yes! That's the only sane way to maintain apis, and is why we do that
for sysfs.
If the key isn't present, there's no value, so userspace "knows" this
automatically and parsing this is trivial.
> Contents of ":mnt:info" contradicts that, but mountinfo has a long
> established, well documented format, and nothing prevents exporting
> individual attributes with separate names as well (the getvalues(2)
> patch did exactly that).
I understand, for "legacy" things like this, that's fine, but don't add
new fields or change them over time please, that way just gets us back
to the nightmare of preserving /proc/ file apis.
thanks,
greg k-h