Thomas,

The "return rc" was not intended to be removed.  I think I accidentally did
than when traversing the code.  The attached patch does not have the
problem.

Craig

On Thu, Nov 12, 2015 at 8:12 AM, LEIBOVICI Thomas <[email protected]>
wrote:

> Thanks for the patch.
> I think it can interest other users so I will integrate it to next v2.5.x
> and also port it to v3.
>
> I just have a question about the following change. Why did you remove the
> "return rc"?
>
> diff -urN tmp/robinhood-2.5.5/src/fs_scan/fs_scan.c
> robinhood-2.5.5/src/fs_scan/fs_scan.c
> --- tmp/robinhood-2.5.5/src/fs_scan/fs_scan.c 2015-04-16
> 14:32:52.561687000 +0000
> +++ robinhood-2.5.5/src/fs_scan/fs_scan.c 2015-11-11 22:56:13.445035000
> +0000
> @@ -793,7 +793,6 @@
>          if (!rc)
>              /* device id is not the one seen by client: change it */
>              inode->st_dev = fsdev;
> -        return rc;
>      }
>
> Thomas
>
>
>
> On 11/12/15 00:50, Craig Tierney - NOAA Affiliate wrote:
>
> Thomas,
>
> Thanks for the pointer to the code.  The fact that the code is old means
> it isn't something you want to support upstream.  Is that true?
>
> What about something less intrusive?  How about a way to say that
> last_access should be just the POSIX access time, and not the MAX of atime,
> mtime, and ctime?  I wrote a small patch that did this.
>
> Craig
>
> diff -urN tmp/robinhood-2.5.5/src/common/global_config.c
> robinhood-2.5.5/src/common/global_config.c
> --- tmp/robinhood-2.5.5/src/common/global_config.c 2014-11-25
> 09:02:52.000000000 +0000
> +++ robinhood-2.5.5/src/common/global_config.c 2015-11-11
> 20:36:45.003202000 +0000
> @@ -54,6 +54,7 @@
>      rh_strncpy(conf->lock_file, "/var/locks/robinhood.lock",
> RBH_PATH_MAX);
>      conf->stay_in_fs = TRUE;
>      conf->check_mounted = TRUE;
> +    conf->last_access_only_atime = FALSE;
>      conf->fs_key = FSKEY_FSNAME;
>
>  #if defined( _LUSTRE ) && defined( _MDS_STAT_SUPPORT )
> @@ -77,6 +78,7 @@
>      print_line( output, 1, "lock_file     :
>  \"/var/locks/robinhood.lock\"" );
>      print_line( output, 1, "stay_in_fs    :  TRUE" );
>      print_line( output, 1, "check_mounted :  TRUE" );
> +    print_line( output, 1, "last_access_only_atime :  FALSE" );
>
>  #if defined( _LUSTRE ) && defined( _MDS_STAT_SUPPORT )
>      print_line( output, 1, "direct_mds_stat :   FALSE" );
> @@ -92,7 +94,7 @@
>      global_config_t *conf = ( global_config_t * ) module_config;
>
>      static const char *allowed_params[] = {
> -        "fs_path", "fs_type", "lock_file", "stay_in_fs", "check_mounted",
> +        "fs_path", "fs_type", "lock_file", "stay_in_fs", "check_mounted",
> "last_access_only_atime",
>          "direct_mds_stat", "fs_key",
>  NULL
>      };
> @@ -163,6 +165,14 @@
>      else if ( rc != ENOENT )
>          conf->check_mounted = tmpval;
>
> +    /* /!\ last_access_only_atime is a piece of bit field, it should not
> be passed directly: using tmpval instead */
> +    rc = GetBoolParam( general_block, GLOBAL_CONFIG_BLOCK,
> "last_access_only_atime",
> +                       0, &tmpval, NULL, NULL, msg_out );
> +    if ( ( rc != 0 ) && ( rc != ENOENT ) )
> +        return rc;
> +    else if ( rc != ENOENT )
> +        conf->last_access_only_atime = tmpval;
> +
>      /* fs_key param */
>      char tmpstr[128];
>      rc = GetStringParam( general_block, GLOBAL_CONFIG_BLOCK, "fs_key",
> @@ -231,6 +241,13 @@
>          global_config.check_mounted = conf->check_mounted;
>      }
>
> +    if ( global_config.last_access_only_atime !=
> conf->last_access_only_atime )
> +    {
> +        DisplayLog( LVL_EVENT, "GlobalConfig", GLOBAL_CONFIG_BLOCK
> "::last_access_only_atime updated: %s->%s",
> +                    bool2str( global_config.last_access_only_atime ),
> bool2str( conf->last_access_only_atime ) );
> +        global_config.last_access_only_atime =
> conf->last_access_only_atime;
> +    }
> +
>  #if defined( _LUSTRE ) && defined( _MDS_STAT_SUPPORT )
>      if ( conf->direct_mds_stat != global_config.direct_mds_stat )
>      {
> @@ -274,6 +291,10 @@
>      fprintf( output, "\n" );
>      print_line( output, 1, "# check that the filesystem is mounted" );
>      print_line( output, 1, "check_mounted = TRUE ;" );
> +    fprintf( output, "\n" );
> +    print_line( output, 1, "# Set the last_access time by only the atime
> variable, and not MAX3(atime,mtime,ctime)" );
> +    print_line( output, 1, "# There are no guarantees that all
> filesystems will correctly store atime" );
> +    print_line( output, 1, "last_access_only_atime = FALSE ;" );
>
>  #if defined( _LUSTRE ) && defined( _MDS_STAT_SUPPORT )
>      fprintf( output, "\n" );
> diff -urN tmp/robinhood-2.5.5/src/common/RobinhoodMisc.c
> robinhood-2.5.5/src/common/RobinhoodMisc.c
> --- tmp/robinhood-2.5.5/src/common/RobinhoodMisc.c 2015-02-26
> 09:39:47.919878000 +0000
> +++ robinhood-2.5.5/src/common/RobinhoodMisc.c 2015-11-11
> 23:00:24.950352000 +0000
> @@ -444,6 +444,7 @@
>
>  void PosixStat2EntryAttr( struct stat *p_inode, attr_set_t * p_attr_set,
> int size_info )
>  {
> +
>      ATTR_MASK_SET( p_attr_set, owner );
>      uid2str( p_inode->st_uid, ATTR( p_attr_set, owner ) );
>
> @@ -464,9 +465,18 @@
>  #endif
>
>          /* times are also wrong when they come from the MDT device */
> +
> +
>          ATTR_MASK_SET( p_attr_set, last_access );
> -        ATTR( p_attr_set, last_access ) =
> -            MAX3( p_inode->st_atime, p_inode->st_mtime, p_inode->st_ctime
> );
> +
> +/* Vary the setting of last_access depending on value of
> global_config.last_access_only_atime */
> + if (global_config.last_access_only_atime) {
> +       ATTR( p_attr_set, last_access ) = p_inode->st_atime;
> + } else {
> +       ATTR( p_attr_set, last_access ) =
> +            MAX3( p_inode->st_atime, p_inode->st_mtime,
> p_inode->st_ctime );
> + }
> +
>
>          ATTR_MASK_SET( p_attr_set, last_mod );
>          ATTR( p_attr_set, last_mod ) = p_inode->st_mtime;
> diff -urN tmp/robinhood-2.5.5/src/fs_scan/fs_scan.c
> robinhood-2.5.5/src/fs_scan/fs_scan.c
> --- tmp/robinhood-2.5.5/src/fs_scan/fs_scan.c 2015-04-16
> 14:32:52.561687000 +0000
> +++ robinhood-2.5.5/src/fs_scan/fs_scan.c 2015-11-11 22:56:13.445035000
> +0000
> @@ -793,7 +793,6 @@
>          if (!rc)
>              /* device id is not the one seen by client: change it */
>              inode->st_dev = fsdev;
> -        return rc;
>      }
>      else
>  #endif
> diff -urN tmp/robinhood-2.5.5/src/include/global_config.h
> robinhood-2.5.5/src/include/global_config.h
> --- tmp/robinhood-2.5.5/src/include/global_config.h 2014-11-25
> 09:02:52.000000000 +0000
> +++ robinhood-2.5.5/src/include/global_config.h 2015-11-11
> 20:13:20.815117000 +0000
> @@ -49,6 +49,7 @@
>      /* behavior flags */
>      int            stay_in_fs:1;
>      int            check_mounted:1;
> +    int   last_access_only_atime:1;
>
>  #if defined( _LUSTRE ) && defined ( _MDS_STAT_SUPPORT )
>      /** Direct stat to MDS on Lustre filesystems */
>
> On Wed, Nov 11, 2015 at 2:20 PM, Thomas LEIBOVICI (mail perso) <
> <[email protected]>[email protected]> wrote:
>
>> This patch from Cray enables storing real atime/mtime/cime in rbh DB and
>> using them as rbh-find options :
>>
>>
>> https://github.com/fzago-cray/robinhood/commit/acc3d7144ecff99f16af6ab776380c9bcc57ec7d
>>
>> It is a little old (april 2013) but may still apply on current code.
>>
>> HTH
>> Regards
>>
>>
>> Le 11/11/2015 20:32, Craig Tierney - NOAA Affiliate a écrit :
>>
>> Thomas,
>>
>> Thanks for the response.  My problem is that we fake directory based
>> quotas by ensuring that the ownership of files matches the top level
>> directory.  This means we have to walk the filesystem and change the group
>> ownership if needed.  The problem is that this changes the mtime and
>> Robinhood then thinks the file was access, but it wasn't.
>>
>> Craig
>>
>> On Wed, Nov 11, 2015 at 12:21 PM, Thomas LEIBOVICI (mail perso) <
>> [email protected]> wrote:
>>
>>> Le 10/11/2015 23:31, Craig Tierney - NOAA Affiliate a écrit :
>>>
>>> Thomas,
>>>
>>> Thanks for the quick response.   To confirm, use of atime with rbh-find
>>> will compare times to the last_access time?
>>>
>>>
>>> Yes.
>>> I understand this can be confusing but last_access is taken as the last
>>> activity on the file (max(atime, mtime, ctime))
>>> which appears to be a suitable criteria for purge policies in particular.
>>>
>>> Regards.
>>>
>>>
>>> Craig
>>>
>>> On Tue, Nov 10, 2015 at 2:19 PM, Thomas LEIBOVICI (mail perso) <
>>> <[email protected]>[email protected]> wrote:
>>>
>>>> Hi Craig,
>>>>
>>>> rbh's last_mod = stat's mtime
>>>> rbh's last_access = max(atime, mtime, ctime)
>>>> with Lustre changelogs:
>>>>     rbh creation is the timestamp of the CREATE record of the entry
>>>> when scanning :
>>>>     rbh creation is the ctime of the entry the first time it was seen.
>>>>
>>>> Regards
>>>> Thomas
>>>>
>>>>
>>>> Le 10/11/2015 21:09, Craig Tierney - NOAA Affiliate wrote:
>>>>
>>>> Hello,
>>>>
>>>> I have a question regarding the difference in names of timestamps used
>>>> in Robinhood.  When I run stat on a file, it reports 3 times.  These are
>>>> Access, Change, and Modify.  When I run rbh-report --entry-info on the same
>>>> file, Robinhood uses creation, last_access, and last_mod.
>>>>
>>>> Are last_access from RBH and Access from stat supposed to be the same?
>>>> What about Modify and last_mod?  How does the timestamp Change from stat
>>>> and creation from RBH relate?
>>>>
>>>> Thanks,
>>>> Craig
>>>>
>>>>
>>>> ------------------------------------------------------------------------------
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> robinhood-support mailing 
>>>> [email protected]https://lists.sourceforge.net/lists/listinfo/robinhood-support
>>>>
>>>>
>>>>
>>>>
>>>> ------------------------------
>>>> [image: Avast logo] <http://www.avast.com/>
>>>>
>>>> L'absence de virus dans ce courrier électronique a été vérifiée par le
>>>> logiciel antivirus Avast.
>>>> <http://www.avast.com/>www.avast.com
>>>>
>>>>
>>>
>>>
>>>
>>> ------------------------------
>>> [image: Avast logo] <http://www.avast.com/>
>>>
>>> L'absence de virus dans ce courrier électronique a été vérifiée par le
>>> logiciel antivirus Avast.
>>> www.avast.com
>>>
>>>
>>
>>
>>
>> ------------------------------
>> [image: Avast logo] <http://www.avast.com/>
>>
>> L'absence de virus dans ce courrier électronique a été vérifiée par le
>> logiciel antivirus Avast.
>> www.avast.com
>>
>>
>
>
> ------------------------------------------------------------------------------
>
>
>
> _______________________________________________
> robinhood-support mailing 
> [email protected]https://lists.sourceforge.net/lists/listinfo/robinhood-support
>
>
>

Attachment: record_atime.patch
Description: Binary data

------------------------------------------------------------------------------
Presto, an open source distributed SQL query engine for big data, initially
developed by Facebook, enables you to easily query your data on Hadoop in a 
more interactive manner. Teradata is also now providing full enterprise
support for Presto. Download a free open source copy now.
http://pubads.g.doubleclick.net/gampad/clk?id=250295911&iu=/4140
_______________________________________________
robinhood-support mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/robinhood-support

Reply via email to