linux-api.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
  • * Re: [PATCH] Add a pair of system calls to make extended file stats available [ver #3]
           [not found] <20100630233614.32422.97038.stgit@warthog.procyon.org.uk>
           [not found] ` <20100630233614.32422.97038.stgit-S6HVgzuS8uM4Awkfq6JHfwNdhmdF6hFW@public.gmane.org>
    @ 2010-07-05 14:59 ` David Howells
           [not found]   ` <4859.1278341989-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
           [not found]   ` <9A36AB6B-7EC3-4789-89CD-D00715BEF34C-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org>
      2010-07-08 19:44 ` Michael Kerrisk
      2010-07-09 13:59 ` David Howells
      3 siblings, 2 replies; 8+ messages in thread
    From: David Howells @ 2010-07-05 14:59 UTC (permalink / raw)
      To: Michael Kerrisk
      Cc: linux-cifs, linux-api, samba-technical, linux-kernel, dhowells,
    	linux-fsdevel, linux-ext4
    
    Michael Kerrisk <mtk.manpages@gmail.com> wrote:
    
    > * Include information from the "inode_info" structure, most notably
    > i_flags, but perhaps other info as well.
    
    This thought has occurred to me, but are the contents of i_flags identical for
    all filesystems?  Certainly, i_flags doesn't seem to match the FS_IOC_GETFLAGS
    mask.  For example:
    
    	#define	FS_SECRM_FL			0x00000001
    
    vs:
    
    	#define S_SYNC		1	/* Writes are synced at once */
    
    I've also been asked to provide st_flags as for BSD, which aren't compatible
    either:-/.
    
    Some questions:
    
     (1) Does it make sense to rearrange the S_xxxx flags to match the numbers for
         FS_xxxx_FL?
    
     (2) Does it make sense to do the BSD st_flags compatibility in userspace?
    
     (3) Can we add a couple more flags to make Samba's life easier?  Say an
         archived bit and a hidden bit?
    
     (4) Do I actually need to provide a mask saying what st_flags are applicable
         to the file you've just queried?
    
     (5) How often are these flags required?  E.g. does it make more sense to keep
         them as an additional result, or does it make sense to stick them in the
         kstat and xstat structs, knowing that these are allocated on the kernel
         stack maybe as three times if an ecryptfs file?
    
    > * Return a bit mask indicating the presence of additional information
    > associated with the i-node. Here, I am thinking of flags that indicate
    > that the file has any of the following: capabilities, an ACL, and
    > extended attributes (obviously a superset of the previous). I could
    > imagine some apps that, having got the xstat info, would be interested
    > to obtain some of this other info.
    > 
    > Obviously, the above only make sense if the overhead of providing the
    > extra information is low.
    
    That might make sense as an 'additional result'.  These things may have to be
    probed for on disk or on a server, so you might not want to return them by
    default, and you may want to indicate what the filesystem can support vs what
    the file actually has:
    
    	u64	st_fs_additional_info;	/* what the filesystem supports */
    	u64	st_file_additional_info; /* what the file actually has */
    
    	#define XST_ADDINFO_CAPABILITY_MASK
    	#define XST_ADDINFO_ACL
    	#define XST_ADDINFO_XATTRS
    	#define XST_ADDINFO_SECLABEL
    
    David
    
    ^ permalink raw reply	[flat|nested] 8+ messages in thread
  • * Re: [PATCH] Add a pair of system calls to make extended file stats available [ver #3]
           [not found] <20100630233614.32422.97038.stgit@warthog.procyon.org.uk>
           [not found] ` <20100630233614.32422.97038.stgit-S6HVgzuS8uM4Awkfq6JHfwNdhmdF6hFW@public.gmane.org>
      2010-07-05 14:59 ` David Howells
    @ 2010-07-08 19:44 ` Michael Kerrisk
      2010-07-09 13:59 ` David Howells
      3 siblings, 0 replies; 8+ messages in thread
    From: Michael Kerrisk @ 2010-07-08 19:44 UTC (permalink / raw)
      To: David Howells
      Cc: linux-fsdevel, linux-cifs, linux-kernel, samba-technical,
    	linux-ext4, linux-api
    
    Hi David,
    
    A couple of comments below.
    
    On Thu, Jul 1, 2010 at 1:36 AM, David Howells <dhowells@redhat.com> wrote:
    > Add a pair of system calls to make extended file stats available, including
    > file creation time, inode version and data version where available through the
    > underlying filesystem.
    >
    > [This depends on the previously posted pair of patches to (a) constify a number
    >  of syscall string and buffer arguments and (b) rearrange AFS's use of
    >  i_version and i_generation].
    >
    > The following structures are defined for their use:
    >
    >        struct xstat_parameters {
    >                unsigned long long      request_mask;
    
    Poor name, since it's a value-result arg? Better maybe something like
    "field_mask"?
    
    >        };
    >
    >        struct xstat_dev {
    >                unsigned int            major, minor;
    >        };
    >
    >        struct xstat_time {
    >                unsigned long long      tv_sec, tv_nsec;
    >        };
    >
    >        struct xstat {
    >                unsigned int            st_mode;
    >                unsigned int            st_nlink;
    >                unsigned int            st_uid;
    >                unsigned int            st_gid;
    >                struct xstat_dev        st_rdev;
    >                struct xstat_dev        st_dev;
    >                struct xstat_time       st_atime;
    >                struct xstat_time       st_mtime;
    >                struct xstat_time       st_ctime;
    >                struct xstat_time       st_btime;
    >                unsigned long long      st_ino;
    >                unsigned long long      st_size;
    >                unsigned long long      st_blksize;
    >                unsigned long long      st_blocks;
    >                unsigned long long      st_gen;
    >                unsigned long long      st_data_version;
    >                unsigned long long      st_result_mask;
    >                unsigned long long      st_extra_results[0];
    >        };
    >
    > where st_btime is the file creation time, st_gen is the inode generation
    > (i_generation), st_data_version is the data version number (i_version),
    > request_mask and st_result_mask are bitmasks of data desired/provided and
    > st_extra_results[] is where as-yet undefined fields are appended.
    >
    > The defined bits in request_mask and st_result_mask are:
    >
    >        XSTAT_REQUEST_MODE              Want/got st_mode
    >        XSTAT_REQUEST_NLINK             Want/got st_nlink
    >        XSTAT_REQUEST_UID               Want/got st_uid
    >        XSTAT_REQUEST_GID               Want/got st_gid
    >        XSTAT_REQUEST_RDEV              Want/got st_rdev
    >        XSTAT_REQUEST_ATIME             Want/got st_atime
    >        XSTAT_REQUEST_MTIME             Want/got st_mtime
    >        XSTAT_REQUEST_CTIME             Want/got st_ctime
    >        XSTAT_REQUEST_INO               Want/got st_ino
    >        XSTAT_REQUEST_SIZE              Want/got st_size
    >        XSTAT_REQUEST_BLOCKS            Want/got st_blocks
    >        XSTAT_REQUEST__BASIC_STATS      The stuff in the normal stat struct
    >        XSTAT_REQUEST_BTIME             Want/got st_btime
    >        XSTAT_REQUEST_GEN               Want/got st_gen
    >        XSTAT_REQUEST_DATA_VERSION      Want/got st_data_version
    >        XSTAT_REQUEST__EXTENDED_STATS   The stuff in the xstat struct
    >        XSTAT_REQUEST__ALL_STATS        The defined set of requestables
    >
    > The system calls are:
    >
    >        ssize_t ret = xstat(int dfd,
    >                            const char *filename,
    >                            unsigned flags,
    >                            const struct xstat_parameters *params,
    >                            struct xstat *buffer,
    >                            size_t buflen);
    >
    >        ssize_t ret = fxstat(unsigned fd,
    >                             unsigned flags,
    >                             const struct xstat_parameters *params,
    >                             struct xstat *buffer,
    >                             size_t buflen);
    >
    >
    > The dfd, filename, flags and fd parameters indicate the file to query.  There
    > is no equivalent of lstat() as that can be emulated with xstat() by passing
    > AT_SYMLINK_NOFOLLOW in flags.
    >
    > AT_FORCE_ATTR_SYNC can also be set in flags.  This will require a network
    > filesystem to synchronise its attributes with the server.
    >
    > When the system call is executed, the request_mask bitmask is read from the
    > parameter block to work out what the user is requesting.  If params is NULL,
    > then request_mask will be assumed to be XSTAT_REQUEST__GET_ANYWAY.
    
    There is no XSTAT_REQUEST__GET_ANYWAY, AFAICS. I guess here you meant
    XSTAT_REQUEST__EXTENDED_STATS? Or?
    
    
    > The request_mask should be set by the caller to specify extra results that the
    > caller may desire.  These come in a number of classes:
    >
    >  (0) dev, blksize.
    >
    >     These are local data and are always available.
    >
    >  (1) mode, nlinks, uid, gid, [amc]time, ino, size, blocks.
    >
    >     These will be returned whether the caller asks for them or not.  The
    >     corresponding bits in result_mask will be set to indicate their presence.
    >
    >     If the caller didn't ask for them, then they may be approximated.  For
    >     example, NFS won't waste any time updating them from the server, unless as
    >     a byproduct of updating something requested.
    >
    >  (2) rdev.
    >
    >     As for class (1), but this won't be returned if the file is not a blockdev
    >     or chardev.  The bit will be cleared if the value is not returned.
    >
    >  (3) File creation time, inode generation and data version.
    >
    >     These will be returned if available whether the caller asked for them or
    >     not.  The corresponding bits in result_mask will be set or cleared as
    >     appropriate to indicate their presence.
    >
    >     If the caller didn't ask for them, then they may be approximated.  For
    >     example, NFS won't waste any time updating them from the server, unless
    >     as a byproduct of updating something requested.
    >
    >  (4) Extra results.
    >
    >     These will only be returned if the caller asked for them by setting their
    >     bits in request_mask.  They will be placed in the buffer after the xstat
    >     struct in ascending result_mask bit order.  Any bit set in request_mask
    >     mask will be left set in result_mask if the result is available and
    >     cleared otherwise.
    >
    >     The pointer into the results list will be rounded up to the nearest 8-byte
    >     boundary after each result is written in.  The size of each extra result
    >     is specific to the definition for that result.
    >
    >     No extra results are currently defined.
    >
    > If the buffer is insufficiently big, the syscall returns the amount of space it
    > will need to write the complete result set and returns a partial result in the
    > buffer.
    
    This case is almost certainly a user error, so why not simply return
    an error (-1 and ERANGE or E2BIG)? The above approach invites
    userspace errors of the form:
    
    if (xtat(...) < 0) { /* How users often check for error */
        /* I'll handle the error */
    } else {
        /* The call succeeded; I'm fine */
    }
    
    Instead, more complex error-handling is required for *every* call:
    
    ret = xstat(..., buflen);
    if (ret < 0 || ret > buflen)
        /* I'll handle the error */
    } else {
        /* The call succeeded; I'm fine */
    }
    
    If you are looking for a way to inform the user about the required
    buffer size, I think it would be better to take a leaf from the
    getxattr(2) book: if 'buflen' is zero, then do nothing with the output
    arg, but return the size that would be required.
    
    Cheers,
    
    Michael
    --
    To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at  http://vger.kernel.org/majordomo-info.html
    
    ^ permalink raw reply	[flat|nested] 8+ messages in thread
  • * Re: [PATCH] Add a pair of system calls to make extended file stats available [ver #3]
           [not found] <20100630233614.32422.97038.stgit@warthog.procyon.org.uk>
                       ` (2 preceding siblings ...)
      2010-07-08 19:44 ` Michael Kerrisk
    @ 2010-07-09 13:59 ` David Howells
      3 siblings, 0 replies; 8+ messages in thread
    From: David Howells @ 2010-07-09 13:59 UTC (permalink / raw)
      To: Michael Kerrisk
      Cc: dhowells, linux-fsdevel, linux-cifs, linux-kernel,
    	samba-technical, linux-ext4, linux-api
    
    Michael Kerrisk <mtk.manpages@gmail.com> wrote:
    
    > >        struct xstat_parameters {
    > >                unsigned long long      request_mask;
    > 
    > Poor name, since it's a value-result arg? Better maybe something like
    > "field_mask"?
    
    No.  The contents of xstat_parameters aren't changed.  request_mask is what
    you're asking for, result_mask in the xstat struct is what you actually got.
    
    result_mask may be more or less than request_mask as the filesystem isn't
    obliged to supply anything you didn't ask for, and may not be able to supply
    something you did ask for, and may give you stuff anyway that you didn't ask
    for if it's trivial to do so.
    
    > There is no XSTAT_REQUEST__GET_ANYWAY, AFAICS. I guess here you meant
    > XSTAT_REQUEST__EXTENDED_STATS? Or?
    
    Yep.  I forgot to change that in the patch description.
    
    > This case is almost certainly a user error, so why not simply return
    > an error (-1 and ERANGE or E2BIG)? The above approach invites
    > userspace errors of the form:
    > 
    > if (xtat(...) < 0) { /* How users often check for error */
    >     /* I'll handle the error */
    > } else {
    >     /* The call succeeded; I'm fine */
    > }
    
    I suppose.
    
    > If you are looking for a way to inform the user about the required
    > buffer size, I think it would be better to take a leaf from the
    > getxattr(2) book: if 'buflen' is zero, then do nothing with the output
    > arg, but return the size that would be required.
    
    That's reasonable.
    
    David
    --
    To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at  http://vger.kernel.org/majordomo-info.html
    
    ^ permalink raw reply	[flat|nested] 8+ messages in thread

  • end of thread, other threads:[~2010-07-09 13:59 UTC | newest]
    
    Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
    -- links below jump to the message on this page --
         [not found] <20100630233614.32422.97038.stgit@warthog.procyon.org.uk>
         [not found] ` <20100630233614.32422.97038.stgit-S6HVgzuS8uM4Awkfq6JHfwNdhmdF6hFW@public.gmane.org>
    2010-07-02  5:36   ` [PATCH] Add a pair of system calls to make extended file stats available [ver #3] Michael Kerrisk
    2010-07-02 15:49     ` Andreas Dilger
    2010-07-04  4:33     ` Michael Kerrisk
    2010-07-05 14:59 ` David Howells
         [not found]   ` <4859.1278341989-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    2010-07-07 14:57     ` Andreas Dilger
         [not found]   ` <9A36AB6B-7EC3-4789-89CD-D00715BEF34C-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org>
    2010-07-07 15:28     ` David Howells
    2010-07-08 19:44 ` Michael Kerrisk
    2010-07-09 13:59 ` David Howells
    

    This is a public inbox, see mirroring instructions
    for how to clone and mirror all data and code used for this inbox;
    as well as URLs for NNTP newsgroup(s).